Skip to content
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

Windows Authentication with Kestrel can ignore Authorization due to Middleware Order #14049

Closed
RickStrahl opened this issue Sep 16, 2019 · 10 comments

Comments

@RickStrahl
Copy link

commented Sep 16, 2019

Scenario:

  • Using Windows Authentication with Kestrel (on Windows)
  • Results in [Authorize] can pass through as an unauthorized request
    ( in certain situations)

image

I ended up with this scenario after using IIS Express which worked fine to produce a Windows Identity object. Switching to Kestrel (on Windows) I got the result above.

There are two problems here:

  • Context.User.Identity returns ClaimsIdentity instead of WindowsIdentity
  • Context.User.Identiy.IsAuthenticated is false inside of a controller with [Authorize]

After a Twitter discussion with Damien and Barry it turns out that this specific scenario was caused by a specific middleware order, but only on Kestrel. The same code below works when using IIS Express.

In ConfigureServices():

 services.AddAuthentication(NegotiateDefaults.AuthenticationScheme)
                                        .AddNegotiate();

In Configure():

            app.UseAuthentication(); 
            app.UseAuthorization();

            app.UseCors("CorsPolicy");


            // THIS IS THE PROBLEM - has to happen BEFORE .UseAuthorization
            app.UseRouting();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
            });

Note .UseRouting() comes after .UseAuthorization() and .UseAuthentication() and that's what's apparently causing the non-authorization on the controller.

Switching the configuration code to:

            app.UseRouting();

            app.UseAuthentication(); 
            app.UseAuthorization();

            app.UseCors("CorsPolicy");


            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
            });

makes the problem go away. WIth this in place both IIS Express and Kestrel now work with Windows Auth and produce a proper WindowsIdentity.

Expected Behavior

That the middleware order needs to be a certain way is understandable if confusing and non-discoverable, although it would be nice if there was some way to notify via error or warning if the order is not viable (if possible).

The more serious problem though is that the [Authorize] attribute on the controller in the failure scenario fails to block the request completely. Instead it passes through the request as if there was no authorization present at all. This should fail.

@DamianEdwards

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

@RickStrahl I can't reproduce the behavior where IIS Express provides an identity but Kestrel doesn't, when the order of the middleware is wrong. Can you share your launchSettings.json configuration for IIS/IIS Express please?

@pranavkm pranavkm added the area-mvc label Sep 16, 2019
@DamianEdwards

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

I can reproduce the IIS Express behavior of always getting an identity (even when the middleware are in the wrong order) but only if I disable anonymous authentication. This essentially forces IIS Express to always authenticate so you always get an identity flowing from IIS Express.

@DamianEdwards

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

Somewhat related to #9041 and #8526

@RickStrahl

This comment has been minimized.

Copy link
Author

commented Sep 17, 2019

@DamianEdwards - Double checked and the following is true:

  • If I have anonymousAuthentication=true in IISExpress Windows Auth works with the 'wrong' order
  • If I have anonymousAuthentication=false IISExpressWindows Auth doesn't work with the 'wrong' order and fails in the same way as the Kestrel

Swapping the order properly makes it work on all platforms.

@DamianEdwards

This comment has been minimized.

Copy link
Member

commented Sep 18, 2019

Thanks @RickStrahl. To be clear, with IIS Express, when Windows authentication is enabled and anonymous disabled, it will always attempt to authenticate the user, irrespective of any challenge originating from the application. So while you may indeed end up with an identity being created, that doesn't mean the endpoint itself is being authorized if you get the middleware in the wrong order (i.e. the authentication, presence of an identity, and authorization of the endpoint, are all orthogonal).

We're looking at making a slight change to the current check we have to tighten this up and have the app throw an exception in the case that an endpoint with authorization metadata is to be executed, but the authorization middleware wasn't executed in the context of an endpoint being set. Today it just checks that the endpoint middleware executed anywhere in the pipeline, but that's not enough.

@davidfowl

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

This is patch worthy

@damienbod

This comment has been minimized.

Copy link

commented Sep 19, 2019

I have the same problem using Bearer token auth

public void Configure(IApplicationBuilder app) 
{
            app.UseCors("corsGlobalPolicy");
     
            app.UseStaticFiles();
            app.UseHttpsRedirection();

            app.UseAuthentication();
            app.UseAuthorization();

            app.UseRouting(); // no auth active because this is after the UseAuth..,

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
            });
}

If I switch the UseRouting above the auth stuff, then the auth stuff works, otherwise all requests pass through.

@davidfowl

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

If I switch the UseRouting above the auth stuff, then the auth stuff works, otherwise all requests pass through.

That's by design but it should throw.

@mkArtakMSFT mkArtakMSFT added this to the 3.1.0-preview2 milestone Sep 19, 2019
@mkArtakMSFT mkArtakMSFT added the bug label Sep 19, 2019
@pranavkm pranavkm added the cost: S label Sep 20, 2019
@DamianEdwards

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

@mkArtakMSFT are we in a position to prepare this as a patch for 3.0.x, rather than waiting for 3.1?

pranavkm added a commit that referenced this issue Sep 24, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Sep 24, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Sep 24, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Sep 24, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Sep 30, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Oct 1, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
pranavkm added a commit that referenced this issue Oct 1, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
@mkArtakMSFT mkArtakMSFT added the Working label Oct 1, 2019
mkArtakMSFT added a commit that referenced this issue Oct 1, 2019
* Update AuthZ & Cors middlewares to only set endpoint routing metadata when
  executing in the context of endpoint routing
* Add analyzers for incorrect UseAuth use

Fixes #14049
@pranavkm pranavkm added Done and removed Working labels Oct 1, 2019
@pranavkm pranavkm closed this Oct 1, 2019
@mkArtakMSFT

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

@mkArtakMSFT are we in a position to prepare this as a patch for 3.0.x, rather than waiting for 3.1?

We'll be fixing this as part of 3.0.2 (November) release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.