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

Print available scheme names in exception message for invalid scheme #961

Merged
merged 1 commit into from
Nov 16, 2017
Merged

Print available scheme names in exception message for invalid scheme #961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 117 additions & 9 deletions src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using System.Security.Claims;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
Expand Down Expand Up @@ -62,7 +63,7 @@ public virtual async Task<AuthenticateResult> AuthenticateAsync(HttpContext cont
var handler = await Handlers.GetHandlerAsync(context, scheme);
if (handler == null)
{
throw new InvalidOperationException($"No authentication handler is configured to authenticate for the scheme: {scheme}");
throw await CreateMissingHandlerException(scheme);
}

var result = await handler.AuthenticateAsync();
Expand Down Expand Up @@ -96,7 +97,7 @@ public virtual async Task ChallengeAsync(HttpContext context, string scheme, Aut
var handler = await Handlers.GetHandlerAsync(context, scheme);
if (handler == null)
{
throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}");
throw await CreateMissingHandlerException(scheme);
}

await handler.ChallengeAsync(properties);
Expand Down Expand Up @@ -124,7 +125,7 @@ public virtual async Task ForbidAsync(HttpContext context, string scheme, Authen
var handler = await Handlers.GetHandlerAsync(context, scheme);
if (handler == null)
{
throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}");
throw await CreateMissingHandlerException(scheme);
}

await handler.ForbidAsync(properties);
Expand Down Expand Up @@ -155,13 +156,19 @@ public virtual async Task SignInAsync(HttpContext context, string scheme, Claims
}
}

var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignInHandler;
var handler = await Handlers.GetHandlerAsync(context, scheme);
if (handler == null)
{
throw new InvalidOperationException($"No IAuthenticationSignInHandler is configured to handle sign in for the scheme: {scheme}");
throw await CreateMissingSignInHandlerException(scheme);
}

var signInHandler = handler as IAuthenticationSignInHandler;
if (signInHandler == null)
{
throw await CreateMismatchedSignInHandlerException(scheme, handler);
}

await handler.SignInAsync(principal, properties);
await signInHandler.SignInAsync(principal, properties);
}

/// <summary>
Expand All @@ -183,13 +190,114 @@ public virtual async Task SignOutAsync(HttpContext context, string scheme, Authe
}
}

var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignOutHandler;
var handler = await Handlers.GetHandlerAsync(context, scheme);
if (handler == null)
{
throw new InvalidOperationException($"No IAuthenticationSignOutHandler is configured to handle sign out for the scheme: {scheme}");
throw await CreateMissingSignOutHandlerException(scheme);
}

var signOutHandler = handler as IAuthenticationSignOutHandler;
if (signOutHandler == null)
{
throw await CreateMismatchedSignOutHandlerException(scheme, handler);
}

await signOutHandler.SignOutAsync(properties);
}

private async Task<Exception> CreateMissingHandlerException(string scheme)
{
var schemes = string.Join(", ", (await Schemes.GetAllSchemesAsync()).Select(sch => sch.Name));

var footer = $" Did you forget to call AddAuthentication().Add[SomeAuthHandler](\"{scheme}\",...)?";

if (string.IsNullOrEmpty(schemes))
{
return new InvalidOperationException(
$"No authentication handlers are registered." + footer);
}

return new InvalidOperationException(
$"No authentication handler is registered for the scheme '{scheme}'. The registered schemes are: {schemes}." + footer);
}

private async Task<string> GetAllSignInSchemeNames()
{
return string.Join(", ", (await Schemes.GetAllSchemesAsync())
.Where(sch => typeof(IAuthenticationSignInHandler).IsAssignableFrom(sch.HandlerType))
.Select(sch => sch.Name));
}

private async Task<Exception> CreateMissingSignInHandlerException(string scheme)
{
var schemes = await GetAllSignInSchemeNames();

// CookieAuth is the only implementation of sign-in.
var footer = $" Did you forget to call AddAuthentication().AddCookies(\"{scheme}\",...)?";

if (string.IsNullOrEmpty(schemes))
{
return new InvalidOperationException(
$"No sign-in authentication handlers are registered." + footer);
}

return new InvalidOperationException(
$"No sign-in authentication handler is registered for the scheme '{scheme}'. The registered sign-in schemes are: {schemes}." + footer);
}

private async Task<Exception> CreateMismatchedSignInHandlerException(string scheme, IAuthenticationHandler handler)
{
var schemes = await GetAllSignInSchemeNames();

var mismatchError = $"The authentication handler registered for scheme '{scheme}' is '{handler.GetType().Name}' which cannot be used for SignInAsync. ";

if (string.IsNullOrEmpty(schemes))
{
// CookieAuth is the only implementation of sign-in.
return new InvalidOperationException(mismatchError
+ $"Did you intended to call AddAuthentication().AddCookies(\"Cookies\") and SignInAsync(\"Cookies\",...)?");
Copy link
Member

Choose a reason for hiding this comment

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

Typo: intended => intend

Copy link
Member

Choose a reason for hiding this comment

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

Or should just be consistent and use forget?

}

return new InvalidOperationException(mismatchError + $"The registered sign-in schemes are: {schemes}.");
}

private async Task<string> GetAllSignOutSchemeNames()
{
return string.Join(", ", (await Schemes.GetAllSchemesAsync())
.Where(sch => typeof(IAuthenticationSignOutHandler).IsAssignableFrom(sch.HandlerType))
.Select(sch => sch.Name));
}

private async Task<Exception> CreateMissingSignOutHandlerException(string scheme)
{
var schemes = await GetAllSignOutSchemeNames();

var footer = $" Did you forget to call AddAuthentication().AddCookies(\"{scheme}\",...)?";

if (string.IsNullOrEmpty(schemes))
{
// CookieAuth is the most common implementation of sign-out, but OpenIdConnect and WsFederation also support it.
return new InvalidOperationException($"No sign-out authentication handlers are registered." + footer);
}

return new InvalidOperationException(
$"No sign-out authentication handler is registered for the scheme '{scheme}'. The registered sign-out schemes are: {schemes}." + footer);
}

private async Task<Exception> CreateMismatchedSignOutHandlerException(string scheme, IAuthenticationHandler handler)
{
var schemes = await GetAllSignOutSchemeNames();

var mismatchError = $"The authentication handler registered for scheme '{scheme}' is '{handler.GetType().Name}' which cannot be used for {nameof(SignOutAsync)}. ";

if (string.IsNullOrEmpty(schemes))
{
// CookieAuth is the most common implementation of sign-out, but OpenIdConnect and WsFederation also support it.
return new InvalidOperationException(mismatchError
+ $"Did you intended to call AddAuthentication().AddCookies(\"Cookies\") and {nameof(SignOutAsync)}(\"Cookies\",...)?");
Copy link
Member

Choose a reason for hiding this comment

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

Typo here too

}

await handler.SignOutAsync(properties);
return new InvalidOperationException(mismatchError + $"The registered sign-out schemes are: {schemes}.");
}
}
}
66 changes: 60 additions & 6 deletions test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,51 @@ namespace Microsoft.AspNetCore.Authentication
{
public class AuthenticationServiceTests
{
[Fact]
public async Task AuthenticateThrowsForSchemeMismatch()
{
var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o =>
{
o.AddScheme<BaseHandler>("base", "whatever");
}).BuildServiceProvider();
var context = new DefaultHttpContext();
context.RequestServices = services;

await context.AuthenticateAsync("base");
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.AuthenticateAsync("missing"));
Assert.Contains("base", ex.Message);
}

[Fact]
public async Task ChallengeThrowsForSchemeMismatch()
{
var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o =>
{
o.AddScheme<BaseHandler>("base", "whatever");
}).BuildServiceProvider();
var context = new DefaultHttpContext();
context.RequestServices = services;

await context.ChallengeAsync("base");
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.ChallengeAsync("missing"));
Assert.Contains("base", ex.Message);
}

[Fact]
public async Task ForbidThrowsForSchemeMismatch()
{
var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o =>
{
o.AddScheme<BaseHandler>("base", "whatever");
}).BuildServiceProvider();
var context = new DefaultHttpContext();
context.RequestServices = services;

await context.ForbidAsync("base");
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.ForbidAsync("missing"));
Assert.Contains("base", ex.Message);
}

[Fact]
public async Task CanOnlySignInIfSupported()
{
Expand All @@ -26,9 +71,13 @@ public async Task CanOnlySignInIfSupported()
context.RequestServices = services;

await context.SignInAsync("uber", new ClaimsPrincipal(), null);
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync("base", new ClaimsPrincipal(), null));
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync("base", new ClaimsPrincipal(), null));
Assert.Contains("uber", ex.Message);
Assert.Contains("signin", ex.Message);
await context.SignInAsync("signin", new ClaimsPrincipal(), null);
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync("signout", new ClaimsPrincipal(), null));
ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync("signout", new ClaimsPrincipal(), null));
Assert.Contains("uber", ex.Message);
Assert.Contains("signin", ex.Message);
}

[Fact]
Expand All @@ -45,7 +94,9 @@ public async Task CanOnlySignOutIfSupported()
context.RequestServices = services;

await context.SignOutAsync("uber");
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignOutAsync("base"));
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignOutAsync("base"));
Assert.Contains("uber", ex.Message);
Assert.Contains("signout", ex.Message);
await context.SignOutAsync("signout");
await context.SignOutAsync("signin");
}
Expand All @@ -64,8 +115,10 @@ public async Task ServicesWithDefaultIAuthenticationHandlerMethodsTest()
await context.AuthenticateAsync();
await context.ChallengeAsync();
await context.ForbidAsync();
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignOutAsync());
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync(new ClaimsPrincipal()));
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignOutAsync());
Assert.Contains("cannot be used for SignOutAsync", ex.Message);
ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync(new ClaimsPrincipal()));
Assert.Contains("cannot be used for SignInAsync", ex.Message);
}

[Fact]
Expand Down Expand Up @@ -119,7 +172,8 @@ public async Task ServicesWithDefaultSignOutMethodsTest()
await context.ChallengeAsync();
await context.ForbidAsync();
await context.SignOutAsync();
await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync(new ClaimsPrincipal()));
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => context.SignInAsync(new ClaimsPrincipal()));
Assert.Contains("cannot be used for SignInAsync", ex.Message);
}

[Fact]
Expand Down