Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

ForbidAsync uses challenge schemes instead of forbid schemes #917

Closed
davidfowl opened this issue Aug 19, 2017 · 7 comments
Closed

ForbidAsync uses challenge schemes instead of forbid schemes #917

davidfowl opened this issue Aug 19, 2017 · 7 comments
Assignees

Comments

@davidfowl
Copy link
Member

While looking through the code I noticed that nothing uses the configured forbid schemes by default (GetDefaultForbidSchemeAsync).

public virtual async Task ForbidAsync(HttpContext context, string scheme, AuthenticationProperties properties)
{
if (scheme == null)
{
var defaultChallengeScheme = await Schemes.GetDefaultChallengeSchemeAsync();
scheme = defaultChallengeScheme?.Name;
if (scheme == null)
{
throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultChallengeScheme found.");
}
}

Seems like ForbidAsync should use the specificed ForbidScheme (if any).

/cc @HaoK

@jkotalik
Copy link
Contributor

See aspnet/Security#1376

@jkotalik
Copy link
Contributor

@Eilon @muratg This is probably a 2.0.1 candidate.

@Eilon
Copy link
Member

Eilon commented Aug 21, 2017

Do we feel this is a common case? How bad is the workaround?

@davidfowl
Copy link
Member Author

@Eilon yes it's very common. The work around isn't that bad but it's non obvious. The application crashes (Stackoverflow) as a result of the recommended configuration.

@davidfowl
Copy link
Member Author

@Eilon this alone doesn't actually fix the issue though. This issue aspnet/Security#1376 requires that this be fixed aspnet/Security#1378 as well.

@muratg muratg added this to the 2.0.1 milestone Aug 21, 2017
@Eilon
Copy link
Member

Eilon commented Aug 21, 2017

Got it, this does sound patch-worthy.

@Eilon Eilon modified the milestones: 2.1.0, 2.0.1 Aug 24, 2017
@Eilon
Copy link
Member

Eilon commented Aug 24, 2017

@jkotalik it looks like you fixed this for 2.1.0 (dev) already. If that's correct, please mark as done and close. Thanks!

@muratg muratg modified the milestones: 2.1.0, 2.1.0-preview1 Dec 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants