Make WindowsIdentity.RunImpersonated closer in behavior to Netfx#30377
Make WindowsIdentity.RunImpersonated closer in behavior to Netfx#30377kouvel merged 3 commits intodotnet:masterfrom kouvel:ImpersonateFix
Conversation
Followup to #30346, this is closer to Netfx behavior for invalid handles
| SafeLocalAllocHandle.InvalidHandle, 0, out dwLength); | ||
| if (Marshal.GetLastWin32Error() == Interop.Errors.ERROR_INVALID_HANDLE) | ||
| throw new ArgumentException(SR.Argument_InvalidImpersonationToken); | ||
|
|
There was a problem hiding this comment.
What impact does moving this code have? Is there a test that fails before the change and passes after and on .NET Framework (and can you add that to the PR)?
There was a problem hiding this comment.
The additional check makes it possible for an ArgumentException to be thrown in cases where GetTokenInformation fails due to invalid handle or handle that is not an access token. Mainly it's just about making the checks similar to those done in netfx. Added a test.
| (uint)TokenInformationClass.TokenType, | ||
| IntPtr.Zero, | ||
| 0, | ||
| out dwLength) && |
There was a problem hiding this comment.
It's strange that we're initializing dwLength to sizeof(uint) but then passing it as out, which suggests it didn't need to be initialized. We should either a) just leave it as uint dwLength; or b) change the GetTokenInformation declaration to take the last argument as ref instead of out, based on whatever the Win32 function expects.
There was a problem hiding this comment.
Yea I just left it the way it was. It's a out-only value an in some places the local is assigned to the current buffer size and is passed in as (..., dwLength, ref dwLength). Anyway it's not incorrect and there are several places to clean up, I'll leave this for a separate PR.
| [Fact] | ||
| public static void RunImpersonatedTest_InvalidHandle() | ||
| { | ||
| Assert.Throws<ArgumentException>(() => |
There was a problem hiding this comment.
Nit: consider moving this to just wrap the RunImpersonated call rather than the whole using
…nImpersonated (#30346) (#30379) * Duplicate the access token passed to WindowsIdentity.RunImpersonated (#30346) So that callbacks for async work initiated while impersonated may continue to impersonate even after the original access token had been disposed. Port of #30346 and #30377 to release/2.1 Fixes https://github.com/dotnet/corefx/issues/30275 * Small fix * Address feedback * Change to use ref counting, remove assert (no null check on access token parameter) * Manual add/release * Add test * Change test
…net/corefx#30377) Make WindowsIdentity.RunImpersonated closer in behavior to Netfx Followup to dotnet/corefx#30346, this is closer to Netfx behavior for invalid handles Commit migrated from dotnet/corefx@ca6adc3
2 participants
Followup to #30346, this is closer to Netfx behavior for invalid handles