Skip to content

Windows Authentication Events #3805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
norachuga opened this issue Apr 5, 2018 · 17 comments
Closed

Windows Authentication Events #3805

norachuga opened this issue Apr 5, 2018 · 17 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-iis Includes: IIS, ANCM

Comments

@norachuga
Copy link

It would be nice if the AuthenticationHandler provided an event or events.

These are available on most, if not all, of the other AspNetCore authentication providers. This feature was also available in the old .NET: WindowsAuthenticationModule.Authenticate Event

This would enable you to perform one-time actions when authentication has occurred. The unsavory alternative is to add middleware that must check on every request to see if authentication has occurred and set/check a flag indicating that the desired actions have not been already been ran.

@Tratcher
Copy link
Member

Tratcher commented Apr 5, 2018

Interesting idea. Your comments about a one time action confuse me though. Windows Auth happens on every request*, it's not a one time event like OAuth or forms auth. What kind of action did you plan to take?

`* technically windows auth can happen once per connection and be recycled on each request, but that information isn't surfaced to us, we don't know if this is the first request or 10th on a connection. And the user authenticates again when they open another connection, even if the first is still open.

@Tratcher
Copy link
Member

Tratcher commented Apr 5, 2018

Note the description of the old WindowsAuthentication_OnAuthenticate method implies it's also invoked per request.

@norachuga
Copy link
Author

Well that's embarrassing! I apologize, I am in the process of porting something from WS-Federation to Windows Auth, and it's my first time working with it.

If there was an event, I would have quickly figured out how often it authenticates ;)

Having it per-request actually works fine with how I plan to use it. It could still be duplicated with custom middleware, but it would be more logical to be able to "finish out" my authentication within the actual authentication handler instead of creating a FinishAuthenticationMiddleWare to do it.

@Tratcher
Copy link
Member

Tratcher commented Apr 6, 2018

IClaimsTransformation is one way of doing this. It's run by the UseAuthentication middleware after authentication. @HaoK do you have a quick doc or sample for this?

@HaoK
Copy link
Member

HaoK commented Apr 6, 2018

@norachuga
Copy link
Author

Thank you, @HaoK. This seems to be what I'm looking for, but I'm having a little bit of trouble.

TransformAsync never gets called when Anonymous Authentication is enabled. If anonymous authentication is disabled, it will work.

Can you explain why that would be the case? I would expect TransformAsync to still be called on requests where [Authorize] is involved.

@HaoK
Copy link
Member

HaoK commented Apr 9, 2018

AnonymousAuthentication = true means don't do authentication so as a result claims transformation won't run since its run after a successful Authenticate

@norachuga
Copy link
Author

I don't know what issue I was having yesterday, but while fiddling with something else, it began working. I'm also able to get it to work fine in the ClaimsTransformation AuthSample. Sorry!

However, there's another quirk with IClaimsTransformation that is proving a bit problematic. I am injecting IHttpContextAccessor into the constructor in order to access Session. TransformAsync gets called 2-4 times per request (which is fine), but Session is not consistently available in every call.

The first call will always throw an InvalidOperationException "Session has not been configured for this application or request.". The second call will have a working Session. Beyond that, sometimes the last call will have the same exception. I have not yet been able to put together a pattern for when the 3rd or 4th call will have Session available.

Due to this, I think I will have to rely on using custom middleware so I can be sure that it takes place post-authentication and has Session available.

@Tratcher
Copy link
Member

That sounds like a Startup ordering issue. Session is not available until after the UseSession middleware. That said, beware of storing any auth information in session.

It's likely that the first call happens in the IISIntegration middleware, the second in the auth middleware. No idea about 3 and 4, are you sure those are the same request and not a parallel request for something like favicon.ico?

For Windows Auth you can either get rid of the auth middleware, or turn off auto-auth in the IIS middleware to prevent redundant calls.

            services.Configure<IISOptions>(options =>
            {
                options.AutomaticAuthentication = false;
            });

@norachuga
Copy link
Author

You're right about the further calls, they are for CSS/JS components. Those then get cached, which explains the inconsistency in number of calls. Don't worry, I'm not storing auth information into session. I just need to make use of the session ID.

As for the first two calls, I don't believe it's a startup ordering issue.

I am testing this using the provided sample. The only changes I have made were to add the Microsoft.AspNetCore.Session package, services.AddSession() and app.UseSession() in their respective places, then a singleton binding for IHttpContextAccessor, which is then injected into the IClaimsTransformation.

    public void ConfigureServices(IServiceCollection services)
    {
        services.AddSession();
        services.AddAuthentication(IISDefaults.AuthenticationScheme);
        services.AddMvc();

        // claims transformation is run after every Authenticate call
        services.AddTransient<IClaimsTransformation, ClaimsTransformer>();
        services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();
        services.Configure<IISOptions>(options =>
        {
            options.AutomaticAuthentication = false;
        });
    }

    public void Configure(IApplicationBuilder app, IHostingEnvironment env)
    {
       [ ... exception page stuff ... ]

        app.UseStaticFiles();
        app.UseSession();
        app.UseAuthentication();
        app.UseMvc([ ... default route ... ]);
    }

With this startup configuration, the first call to TransformAsync will be unable to use Session.

I realize I'm getting a bit beyond the scope of my original question. I don't want to waste anyone's time, so let me know if this is no longer an IISIntegration issue.

@Tratcher
Copy link
Member

Yes, this is still an ordering issue but it might not be one you can fix. IISIntegration calls AuthenticateAsync before any other middleware in Configure.
https://github.com/aspnet/IISIntegration/blob/6e54256fca6f2ae25525bfbc0a7ca966e0d1780d/src/Microsoft.AspNetCore.Server.IISIntegration/IISMiddleware.cs#L133-L137

Hmm, AutomaticAuthentication isn't checked early enough to prevent this, nor can we move it because it really needs to run to make sure the handle gets registered and disposed properly.

@shirhatti
Copy link
Contributor

@Tratcher What's the verdict here? Something we can fix in IISIntegration?

@Tratcher
Copy link
Member

Adding events would be doable, but their value would be pretty limtied. You could add events for AuthenticateAsync, ChallengeAsync, and ForbidAsync, but I'm not sure how much you could do in those scenarios.
https://github.com/aspnet/IISIntegration/blob/3761667e3611dd07c1be9e2ea113a21a3df15e29/src/Microsoft.AspNetCore.Server.IISIntegration/AuthenticationHandler.cs

Recommend backlog for the events feature.

As for the ordering issue with session... https://github.com/aspnet/IISIntegration/issues/761#issuecomment-380120276
https://github.com/aspnet/IISIntegration/issues/761#issuecomment-380183841
We could restructure this if we really needed to so that IISIntegration loaded the user without calling GetUser. However, that would break anyone currently relying on IClaimsTransformation getting called without UseAuthentication.

Also recommending backlog for the initialization issue.

@norachuga or @shirhatti can you split these into two seperate bugs?

@shirhatti
Copy link
Contributor

@Tratcher I'm assuming we won't be able to do it since it's a breaking change, right?

@Tratcher
Copy link
Member

Tratcher commented May 4, 2018

Right, but not a huge one. Backlog until 3.0.

@natemcmaster natemcmaster transferred this issue from aspnet/IISIntegration Nov 1, 2018
@natemcmaster natemcmaster added this to the 3.0.0 milestone Nov 1, 2018
@muratg
Copy link
Contributor

muratg commented Nov 2, 2018

@pakrym could you ping @Tratcher to understand this better?

@Tratcher
Copy link
Member

Tratcher commented Nov 2, 2018

For out of proc I recommend we don't change anything. The events would be of quite limited value and are mostly covered by claims transformation.

For in proc this is even less relevant as the user is created before the pipeline even begins and there are knobs to control that.

Let's close this.

@pakrym pakrym closed this as completed Nov 3, 2018
@Eilon Eilon added feature-iis Includes: IIS, ANCM and removed area-iis labels Nov 6, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
@amcasey amcasey added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-iis Includes: IIS, ANCM
Projects
None yet
Development

No branches or pull requests

9 participants