Skip to content

Port Throw when UseAuthorization is incorrectly configured #14893

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

Merged
merged 2 commits into from
Oct 10, 2019

Conversation

pranavkm
Copy link
Contributor

Description

Endpoint routing throws if an endpoint requires Auth or Cors middleware to execute but is not configured. #14049 pointed out that this does not throw if the Auth or Cors middleware are configured to execute before the endpoint routing middleware. In this case, an endpoint that requires authorization will run unauthenticated.

This change ports the runtime portion of #14401 to 3.0 to correctly flag this scenario.

Customer Impact

Endpoint routing will throw (result in a 500 response) if an endpoint requires authorization but the middleware is incorrectly configured.

Regression?

No. The middleware is new to 3.0.

Risk

Low. While the impact is very visible as an exception, this prevents an insecure configuration.

@@ -0,0 +1,240 @@
// Copyright (c) .NET Foundation. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntegrationTerst.cs

@mkArtakMSFT mkArtakMSFT added Servicing-consider Shiproom approval is required for the issue area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Oct 10, 2019
@analogrelay analogrelay added this to the 3.0.x milestone Oct 10, 2019
@leecow leecow modified the milestones: 3.0.x, 3.0.1 Oct 10, 2019
@leecow
Copy link
Member

leecow commented Oct 10, 2019

3.0.1 approved ... merge.

@analogrelay analogrelay added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Oct 10, 2019
@pranavkm
Copy link
Contributor Author

@aspnet/build could you merge this?

@mkArtakMSFT mkArtakMSFT merged commit 8c39137 into release/3.0 Oct 10, 2019
@mkArtakMSFT mkArtakMSFT deleted the prkrishn/auth-patch branch October 10, 2019 21:14
@mkArtakMSFT
Copy link
Contributor

Thanks @pranavkm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants