Use ImpersonationToken for IIS Windows Auth #58041
Conversation
BrennanConroy
added
feature-iis
BrennanConroy
requested review from
halter73,
JamesNK and
mgravell
as code owners
dotnet-policy-service
bot
added
the
pending-ci-rerun
|
What can we do to build confidence here? It sounds like there's no expert to ask, but maybe we could have a feature flag in case it turns out to have been a mistake? How would we / an app author know that it was a mistake? |
| #pragma warning disable SYSLIB0014 // Type or member is obsolete | ||
| var request = HttpWebRequest.CreateHttp($"{deploymentResult.HttpClient.BaseAddress}Auth"); | ||
| #pragma warning restore SYSLIB0014 // Type or member is obsolete | ||
| request.ImpersonationLevel = impersonationLevel; |
There was a problem hiding this comment.
I was going to say ImpersonationLevel isn't supported, but went to check the source code in case anything has changed and was surprised to see that they use reflection to enable it for HttpWebRequest, even though HttpClientHandler/SocketsHttpHandler don't expose the api. Time to open an issue as this is an important missing feature that WCF needs.
|
@amcasey, I took a look at how this value is used in the rest of the code. The only opportunity to fail is when a WindowsIdentity instance is created in the aspnetcore side of things. That class checks the handle is a valid user access token handle before duplicating it internally, during construction. It's constructed on either HttpContext creation (in process), or at invocation of an incoming request (out of process). Either way, if this code does something which breaks things, requests won't even get dispatched, and you'll probably get a 500 response as the request will fail very early on. |
|
@BrennanConroy any updates? Can we get this merged? |
There was a problem hiding this comment.
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (2)
- src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp: Language not supported
- src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp: Language not supported
|
Is there any chance that this will be merged for the .NET 10 release? |
BrennanConroy
force-pushed
the
brecon/impersonation
branch
from
c13def5 to
94a9634
Compare
| @@ -136,7 +136,7 @@ public RequiresIISAttribute(IISCapability capabilities) | |||
| IsMet &= available; | |||
| if (!available) | |||
| { | |||
| SkipReason += $"The machine does have {module.Capability} available."; | |||
| SkipReason += $"The machine does not have {module.Capability} available."; | |||
BrennanConroy
removed
the
pending-ci-rerun
Closes #54175
The doc comments on
GetPrimaryTokenandGetImpersonationTokenare next to useless so can't really comment on the difference between them. However, did look at the old .NET FX IIS code and it was usingGetImpersonationTokenso I feel a bit better about this change.Looks like the same sort of issue is filed on corewcf as well CoreWCF/CoreWCF#757
cc @mconnew