From 8bf0bdc42efe32fde4f1fb38390b7dfc8eac39a5 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 16 Jun 2017 14:29:37 -0700 Subject: [PATCH 1/7] Split SignIn/Signout into their own interfaces --- .../AuthenticateResult.cs | 24 +-- .../IAuthenticationHandler.cs | 16 -- .../IAuthenticationRequestHandler.cs | 1 - .../IAuthenticationSignInHandler.cs | 22 +++ .../IAuthenticationSignOutHandler.cs | 21 +++ .../AuthenticationSchemeProvider.cs | 15 +- .../AuthenticationService.cs | 8 +- .../AuthenticationServiceTests.cs | 171 ++++++++++++++++++ 8 files changed, 235 insertions(+), 43 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs create mode 100644 src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignOutHandler.cs create mode 100644 test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs index bb9bbb97..454416eb 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Authentication /// public class AuthenticateResult { - private AuthenticateResult() { } + protected AuthenticateResult() { } /// /// If a ticket was produced, authenticate was successful. @@ -21,7 +21,7 @@ private AuthenticateResult() { } /// /// The authentication ticket. /// - public AuthenticationTicket Ticket { get; private set; } + public AuthenticationTicket Ticket { get; protected set; } /// /// Gets the claims-principal with authenticated user identities. @@ -36,18 +36,12 @@ private AuthenticateResult() { } /// /// Holds failure information from the authentication. /// - public Exception Failure { get; private set; } - - /// - /// Indicates that stage of authentication was directly handled by user intervention and no - /// further processing should be attempted. - /// - public bool Handled { get; private set; } + public Exception Failure { get; protected set; } /// /// Indicates that there was no information returned for this authentication scheme. /// - public bool Nothing { get; private set; } + public bool Nothing { get; protected set; } /// /// Indicates that authentication was successful. @@ -63,16 +57,6 @@ public static AuthenticateResult Success(AuthenticationTicket ticket) return new AuthenticateResult() { Ticket = ticket }; } - /// - /// Indicates that stage of authentication was directly handled by user intervention and no - /// further processing should be attempted. - /// - /// The result. - public static AuthenticateResult Handle() - { - return new AuthenticateResult() { Handled = true }; - } - /// /// Indicates that there was no information returned for this authentication scheme. /// diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationHandler.cs index af92cc76..aeb373e1 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationHandler.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -39,20 +38,5 @@ public interface IAuthenticationHandler /// The that contains the extra meta-data arriving with the authentication. /// A task. Task ForbidAsync(AuthenticationProperties properties); - - /// - /// Handle sign in. - /// - /// The user. - /// The that contains the extra meta-data arriving with the authentication. - /// A task. - Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties); - - /// - /// Signout behavior. - /// - /// The that contains the extra meta-data arriving with the authentication. - /// A task. - Task SignOutAsync(AuthenticationProperties properties); } } diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationRequestHandler.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationRequestHandler.cs index fffe08f4..fb1b227a 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationRequestHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationRequestHandler.cs @@ -10,7 +10,6 @@ namespace Microsoft.AspNetCore.Authentication /// public interface IAuthenticationRequestHandler : IAuthenticationHandler { - /// /// Returns true if request processing should stop. /// diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs new file mode 100644 index 00000000..8f631659 --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Security.Claims; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Authentication +{ + /// + /// Used to determine if a handler supports SignIn. + /// + public interface IAuthenticationSignInHandler : IAuthenticationHandler + { + /// + /// Handle sign in. + /// + /// The user. + /// The that contains the extra meta-data arriving with the authentication. + /// A task. + Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties); + } +} diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignOutHandler.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignOutHandler.cs new file mode 100644 index 00000000..f76d116a --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignOutHandler.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Authentication +{ + /// + /// Used to determine if a handler supports SignOut. + /// + public interface IAuthenticationSignOutHandler : IAuthenticationHandler + { + /// + /// Signout behavior. + /// + /// The that contains the extra meta-data arriving with the authentication. + /// A task. + Task SignOutAsync(AuthenticationProperties properties); + } + +} diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index e56247e1..74a52257 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Options; @@ -36,6 +35,8 @@ public AuthenticationSchemeProvider(IOptions options) private IDictionary _map = new Dictionary(StringComparer.Ordinal); private List _requestHandlers = new List(); + private List _signOutHandlers = new List(); + private List _signInHandlers = new List(); /// /// Returns the scheme that will be used by default for . @@ -167,6 +168,14 @@ public void AddScheme(AuthenticationScheme scheme) { _requestHandlers.Add(scheme); } + if (typeof(IAuthenticationSignInHandler).IsAssignableFrom(scheme.HandlerType)) + { + _signInHandlers.Add(scheme); + } + if (typeof(IAuthenticationSignOutHandler).IsAssignableFrom(scheme.HandlerType)) + { + _signOutHandlers.Add(scheme); + } _map[scheme.Name] = scheme; } } @@ -186,7 +195,9 @@ public void RemoveScheme(string name) if (_map.ContainsKey(name)) { var scheme = _map[name]; - _requestHandlers.Remove(_requestHandlers.Where(s => s.Name == name).FirstOrDefault()); + _requestHandlers.Remove(scheme); + _signInHandlers.Remove(scheme); + _signOutHandlers.Remove(scheme); _map.Remove(name); } } diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs index 326b277f..e88af496 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs @@ -155,10 +155,10 @@ public virtual async Task SignInAsync(HttpContext context, string scheme, Claims } } - var handler = await Handlers.GetHandlerAsync(context, scheme); + var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignInHandler; if (handler == null) { - throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}"); + throw new InvalidOperationException($"No IAuthenticationSignInHandler is configured to handle sign in for the scheme: {scheme}"); } await handler.SignInAsync(principal, properties); @@ -178,10 +178,10 @@ public virtual async Task SignOutAsync(HttpContext context, string scheme, Authe throw new ArgumentException(nameof(scheme)); } - var handler = await Handlers.GetHandlerAsync(context, scheme); + var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignOutHandler; if (handler == null) { - throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}"); + throw new InvalidOperationException($"No IAuthenticationSignOutHandler is configured to handle sign out for the scheme: {scheme}"); } await handler.SignOutAsync(properties); diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs new file mode 100644 index 00000000..2c2f7598 --- /dev/null +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs @@ -0,0 +1,171 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Authentication +{ + public class AuthenticationServiceTests + { + [Fact] + public async Task CanOnlySignInIfSignInHandler() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("uber", "whatever"); + o.AddScheme("base", "whatever"); + o.AddScheme("signin", "whatever"); + o.AddScheme("signout", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.SignInAsync("uber", new ClaimsPrincipal(), null); + await Assert.ThrowsAsync(() => context.SignInAsync("base", new ClaimsPrincipal(), null)); + await context.SignInAsync("signin", new ClaimsPrincipal(), null); + await Assert.ThrowsAsync(() => context.SignInAsync("signout", new ClaimsPrincipal(), null)); + } + + [Fact] + public async Task CanOnlySignOutIfSignOutHandler() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("uber", "whatever"); + o.AddScheme("base", "whatever"); + o.AddScheme("signin", "whatever"); + o.AddScheme("signout", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.SignOutAsync("uber"); + await Assert.ThrowsAsync(() => context.SignOutAsync("base")); + await context.SignOutAsync("signout"); + await Assert.ThrowsAsync(() => context.SignOutAsync("signin")); + } + + private class BaseHandler : IAuthenticationHandler + { + public Task AuthenticateAsync() + { + return Task.FromResult(AuthenticateResult.None()); + } + + public Task ChallengeAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task ForbidAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) + { + return Task.FromResult(0); + } + } + + private class SignInHandler : IAuthenticationSignInHandler + { + public Task AuthenticateAsync() + { + return Task.FromResult(AuthenticateResult.None()); + } + + public Task ChallengeAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task ForbidAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) + { + return Task.FromResult(0); + } + + public Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties) + { + return Task.FromResult(0); + } + } + + public class SignOutHandler : IAuthenticationSignOutHandler + { + public Task AuthenticateAsync() + { + return Task.FromResult(AuthenticateResult.None()); + } + + public Task ChallengeAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task ForbidAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) + { + return Task.FromResult(0); + } + + public Task SignOutAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + } + + private class UberHandler : IAuthenticationHandler, IAuthenticationRequestHandler, IAuthenticationSignInHandler, IAuthenticationSignOutHandler + { + public Task AuthenticateAsync() + { + return Task.FromResult(AuthenticateResult.None()); + } + + public Task ChallengeAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task ForbidAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task HandleRequestAsync() + { + return Task.FromResult(false); + } + + public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) + { + return Task.FromResult(0); + } + + public Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties) + { + return Task.FromResult(0); + } + + public Task SignOutAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } + } + + } +} From 0d92b28cb335865704569391ebef0240ec4aabe4 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 15 Jun 2017 15:32:37 -0700 Subject: [PATCH 2/7] Fallback to DefaultAuthenticateScheme --- .../AuthenticationSchemeProvider.cs | 16 ++++------------ .../AuthenticationSchemeProviderTests.cs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index 74a52257..ef705c29 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -60,7 +60,7 @@ public Task GetDefaultAuthenticateSchemeAsync() /// /// Returns the scheme that will be used by default for . /// This is typically specified via . - /// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned. + /// Otherwise, this will fallback to . /// /// The scheme that will be used by default for . public Task GetDefaultChallengeSchemeAsync() @@ -69,11 +69,7 @@ public Task GetDefaultChallengeSchemeAsync() { return GetSchemeAsync(_options.DefaultChallengeScheme); } - if (_map.Count == 1) - { - return Task.FromResult(_map.Values.First()); - } - return Task.FromResult(null); + return GetDefaultAuthenticateSchemeAsync(); } /// @@ -94,7 +90,7 @@ public Task GetDefaultForbidSchemeAsync() /// /// Returns the scheme that will be used by default for . /// This is typically specified via . - /// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned. + /// Otherwise, this will fallback to . /// /// The scheme that will be used by default for . public Task GetDefaultSignInSchemeAsync() @@ -103,11 +99,7 @@ public Task GetDefaultSignInSchemeAsync() { return GetSchemeAsync(_options.DefaultSignInScheme); } - if (_map.Count == 1) - { - return Task.FromResult(_map.Values.First()); - } - return Task.FromResult(null); + return GetDefaultAuthenticateSchemeAsync(); } /// diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs index 3810f833..9fe65bd2 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs @@ -60,6 +60,24 @@ public async Task DefaultSchemesFallbackToOnlyScheme() Assert.Equal("single", (await provider.GetDefaultSignOutSchemeAsync()).Name); } + [Fact] + public async Task DefaultSchemesFallbackToAuthenticateScheme() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.DefaultAuthenticateScheme = "B"; + o.AddScheme("A", "whatever"); + o.AddScheme("B", "whatever"); + }).BuildServiceProvider(); + + var provider = services.GetRequiredService(); + Assert.Equal("B", (await provider.GetDefaultForbidSchemeAsync()).Name); + Assert.Equal("B", (await provider.GetDefaultAuthenticateSchemeAsync()).Name); + Assert.Equal("B", (await provider.GetDefaultChallengeSchemeAsync()).Name); + Assert.Equal("B", (await provider.GetDefaultSignInSchemeAsync()).Name); + Assert.Equal("B", (await provider.GetDefaultSignOutSchemeAsync()).Name); + } + [Fact] public async Task DefaultSchemesAreSet() { From f8a2cfa8719161c5f83c34227a695ae7367596c9 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 16 Jun 2017 14:44:24 -0700 Subject: [PATCH 3/7] Updates --- .../AuthenticationHttpContextExtensions.cs | 9 ++- .../AuthenticationService.cs | 9 ++- .../AuthenticationServiceTests.cs | 69 +++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationHttpContextExtensions.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationHttpContextExtensions.cs index cb8f1fb4..050e6c57 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationHttpContextExtensions.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationHttpContextExtensions.cs @@ -76,7 +76,7 @@ public static Task ForbidAsync(this HttpContext context, string scheme) => context.ForbidAsync(scheme, properties: null); /// - /// Extension method for Forbid. + /// Extension method for Forbid using the scheme.. /// /// The context. /// The task. @@ -142,6 +142,13 @@ public static Task SignInAsync(this HttpContext context, ClaimsPrincipal princip public static Task SignInAsync(this HttpContext context, string scheme, ClaimsPrincipal principal, AuthenticationProperties properties) => context.RequestServices.GetRequiredService().SignInAsync(context, scheme, principal, properties); + /// + /// Extension method for SignOut using the . + /// + /// The context. + /// The task. + public static Task SignOutAsync(this HttpContext context) => context.SignOutAsync(scheme: null, properties: null); + /// /// Extension method for SignOut. /// diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs index e88af496..e9fcfc0b 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs @@ -173,9 +173,14 @@ public virtual async Task SignInAsync(HttpContext context, string scheme, Claims /// A task. public virtual async Task SignOutAsync(HttpContext context, string scheme, AuthenticationProperties properties) { - if (string.IsNullOrEmpty(scheme)) + if (scheme == null) { - throw new ArgumentException(nameof(scheme)); + var defaultScheme = await Schemes.GetDefaultSignOutSchemeAsync(); + scheme = defaultScheme?.Name; + if (scheme == null) + { + throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignOutScheme found."); + } } var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignOutHandler; diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs index 2c2f7598..fca3541f 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs @@ -50,6 +50,75 @@ public async Task CanOnlySignOutIfSignOutHandler() await Assert.ThrowsAsync(() => context.SignOutAsync("signin")); } + [Fact] + public async Task ServicesWithDefaultIAuthenticationHandlerMethodsTest() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("base", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.AuthenticateAsync(); + await context.ChallengeAsync(); + await context.ForbidAsync(); + await Assert.ThrowsAsync(() => context.SignOutAsync()); + await Assert.ThrowsAsync(() => context.SignInAsync(new ClaimsPrincipal())); + } + + [Fact] + public async Task ServicesWithDefaultUberMethodsTest() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("base", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.AuthenticateAsync(); + await context.ChallengeAsync(); + await context.ForbidAsync(); + await context.SignOutAsync(); + await context.SignInAsync(new ClaimsPrincipal()); + } + + [Fact] + public async Task ServicesWithDefaultSignInMethodsTest() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("base", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.AuthenticateAsync(); + await context.ChallengeAsync(); + await context.ForbidAsync(); + await Assert.ThrowsAsync(() => context.SignOutAsync()); + await context.SignInAsync(new ClaimsPrincipal()); + } + + [Fact] + public async Task ServicesWithDefaultSignOutMethodsTest() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("base", "whatever"); + }).BuildServiceProvider(); + var context = new DefaultHttpContext(); + context.RequestServices = services; + + await context.AuthenticateAsync(); + await context.ChallengeAsync(); + await context.ForbidAsync(); + await context.SignOutAsync(); + await Assert.ThrowsAsync(() => context.SignInAsync(new ClaimsPrincipal())); + } + + private class BaseHandler : IAuthenticationHandler { public Task AuthenticateAsync() From cd17a2251db2127631268ef484b9a731cd7eb8d5 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 16 Jun 2017 18:37:26 -0700 Subject: [PATCH 4/7] Correctly fallback for SignIn/SignOut --- .../AuthenticationSchemeProvider.cs | 8 +++++ .../AuthenticationSchemeProviderTests.cs | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index ef705c29..fec36aae 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -99,6 +99,10 @@ public Task GetDefaultSignInSchemeAsync() { return GetSchemeAsync(_options.DefaultSignInScheme); } + if (_signInHandlers.Count() == 1) + { + return Task.FromResult(_signInHandlers[0]); + } return GetDefaultAuthenticateSchemeAsync(); } @@ -114,6 +118,10 @@ public Task GetDefaultSignOutSchemeAsync() { return GetSchemeAsync(_options.DefaultSignOutScheme); } + if (_signOutHandlers.Count() == 1) + { + return Task.FromResult(_signOutHandlers[0]); + } return GetDefaultSignInSchemeAsync(); } diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs index 9fe65bd2..1cbd3b2e 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs @@ -1,3 +1,4 @@ + // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. @@ -101,6 +102,25 @@ public async Task DefaultSchemesAreSet() Assert.Equal("A", (await provider.GetDefaultSignOutSchemeAsync()).Name); } + [Fact] + public async Task SignInSignOutDefaultsToOnlyOne() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("basic", "whatever"); + o.AddScheme("signout", "whatever"); + o.AddScheme("signin", "whatever"); + o.DefaultAuthenticateScheme = "basic"; + }).BuildServiceProvider(); + + var provider = services.GetRequiredService(); + Assert.Equal("basic", (await provider.GetDefaultForbidSchemeAsync()).Name); + Assert.Equal("basic", (await provider.GetDefaultAuthenticateSchemeAsync()).Name); + Assert.Equal("basic", (await provider.GetDefaultChallengeSchemeAsync()).Name); + Assert.Equal("signin", (await provider.GetDefaultSignInSchemeAsync()).Name); + Assert.Equal("signout", (await provider.GetDefaultSignOutSchemeAsync()).Name); + } + private class Handler : IAuthenticationHandler { public Task AuthenticateAsync() @@ -122,12 +142,26 @@ public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) { throw new NotImplementedException(); } + } + private class SignInHandler : Handler, IAuthenticationSignInHandler + { public Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties) { throw new NotImplementedException(); } + } + + private class SignOutHandler : Handler, IAuthenticationSignOutHandler + { + public Task SignOutAsync(AuthenticationProperties properties) + { + throw new NotImplementedException(); + } + } + private class Uberhandler : SignInHandler, IAuthenticationSignOutHandler + { public Task SignOutAsync(AuthenticationProperties properties) { throw new NotImplementedException(); From 2a2b27ed683c78138e1cb9ef597ea6a8d7df6275 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 22 Jun 2017 11:24:22 -0700 Subject: [PATCH 5/7] Ensure handlers implement signin/signout --- .../AuthenticateResult.cs | 6 +-- .../IClaimsTransformation.cs | 2 +- .../AuthenticationSchemeProvider.cs | 51 +++++++++++-------- .../AuthenticationSchemeProviderTests.cs | 40 ++++++++++++--- .../AuthenticationServiceTests.cs | 8 +-- 5 files changed, 72 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs index 454416eb..78daa7db 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs @@ -41,7 +41,7 @@ protected AuthenticateResult() { } /// /// Indicates that there was no information returned for this authentication scheme. /// - public bool Nothing { get; protected set; } + public bool Ignored { get; protected set; } /// /// Indicates that authentication was successful. @@ -61,9 +61,9 @@ public static AuthenticateResult Success(AuthenticationTicket ticket) /// Indicates that there was no information returned for this authentication scheme. /// /// The result. - public static AuthenticateResult None() + public static AuthenticateResult Ignore() { - return new AuthenticateResult() { Nothing = true }; + return new AuthenticateResult() { Ignored = true }; } /// diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IClaimsTransformation.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IClaimsTransformation.cs index 3aed710a..83719015 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/IClaimsTransformation.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IClaimsTransformation.cs @@ -18,4 +18,4 @@ public interface IClaimsTransformation /// The transformed principal. Task TransformAsync(ClaimsPrincipal principal); } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index fec36aae..bd1a5279 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -44,7 +44,7 @@ public AuthenticationSchemeProvider(IOptions options) /// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned. /// /// The scheme that will be used by default for . - public Task GetDefaultAuthenticateSchemeAsync() + public virtual Task GetDefaultAuthenticateSchemeAsync() { if (_options.DefaultAuthenticateScheme != null) { @@ -63,7 +63,7 @@ public Task GetDefaultAuthenticateSchemeAsync() /// Otherwise, this will fallback to . /// /// The scheme that will be used by default for . - public Task GetDefaultChallengeSchemeAsync() + public virtual Task GetDefaultChallengeSchemeAsync() { if (_options.DefaultChallengeScheme != null) { @@ -78,7 +78,7 @@ public Task GetDefaultChallengeSchemeAsync() /// Otherwise, this will fallback to . /// /// The scheme that will be used by default for . - public Task GetDefaultForbidSchemeAsync() + public virtual Task GetDefaultForbidSchemeAsync() { if (_options.DefaultForbidScheme != null) { @@ -90,39 +90,41 @@ public Task GetDefaultForbidSchemeAsync() /// /// Returns the scheme that will be used by default for . /// This is typically specified via . - /// Otherwise, this will fallback to . + /// If only a single sign in handler scheme exists, that will be used, if more than one exists, + /// this will fallback to . /// /// The scheme that will be used by default for . - public Task GetDefaultSignInSchemeAsync() + public virtual async Task GetDefaultSignInSchemeAsync() { if (_options.DefaultSignInScheme != null) { - return GetSchemeAsync(_options.DefaultSignInScheme); + return EnsureAssignable(await GetSchemeAsync(_options.DefaultSignInScheme)); } - if (_signInHandlers.Count() == 1) + if (_signInHandlers.Count == 1) { - return Task.FromResult(_signInHandlers[0]); + return _signInHandlers[0]; } - return GetDefaultAuthenticateSchemeAsync(); + return EnsureAssignable(await GetDefaultAuthenticateSchemeAsync()); } /// /// Returns the scheme that will be used by default for . /// This is typically specified via . - /// Otherwise, this will fallback to . + /// If only a single sign out handler scheme exists, that will be used, if more than one exists, + /// this will fallback to if that supoorts sign out. /// /// The scheme that will be used by default for . - public Task GetDefaultSignOutSchemeAsync() + public virtual async Task GetDefaultSignOutSchemeAsync() { if (_options.DefaultSignOutScheme != null) { - return GetSchemeAsync(_options.DefaultSignOutScheme); + return EnsureAssignable(await GetSchemeAsync(_options.DefaultSignOutScheme)); } - if (_signOutHandlers.Count() == 1) + if (_signOutHandlers.Count == 1) { - return Task.FromResult(_signOutHandlers[0]); + return _signOutHandlers[0]; } - return GetDefaultSignInSchemeAsync(); + return EnsureAssignable(await GetDefaultSignInSchemeAsync()); } /// @@ -130,7 +132,7 @@ public Task GetDefaultSignOutSchemeAsync() /// /// The name of the authenticationScheme. /// The scheme or null if not found. - public Task GetSchemeAsync(string name) + public virtual Task GetSchemeAsync(string name) { if (_map.ContainsKey(name)) { @@ -143,7 +145,7 @@ public Task GetSchemeAsync(string name) /// Returns the schemes in priority order for request handling. /// /// The schemes in priority order for request handling - public Task> GetRequestHandlerSchemesAsync() + public virtual Task> GetRequestHandlerSchemesAsync() { return Task.FromResult>(_requestHandlers); } @@ -152,7 +154,7 @@ public Task> GetRequestHandlerSchemesAsync() /// Registers a scheme for use by . /// /// The scheme. - public void AddScheme(AuthenticationScheme scheme) + public virtual void AddScheme(AuthenticationScheme scheme) { if (_map.ContainsKey(scheme.Name)) { @@ -184,7 +186,7 @@ public void AddScheme(AuthenticationScheme scheme) /// Removes a scheme, preventing it from being used by . /// /// The name of the authenticationScheme being removed. - public void RemoveScheme(string name) + public virtual void RemoveScheme(string name) { if (!_map.ContainsKey(name)) { @@ -203,9 +205,16 @@ public void RemoveScheme(string name) } } - public Task> GetAllSchemesAsync() + public virtual Task> GetAllSchemesAsync() + => Task.FromResult>(_map.Values); + + private static AuthenticationScheme EnsureAssignable(AuthenticationScheme scheme) where T : class { - return Task.FromResult>(_map.Values); + if (scheme != null && typeof(T).IsAssignableFrom(scheme.HandlerType)) + { + return scheme; + } + return null; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs index 1cbd3b2e..3ae358c0 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs @@ -18,7 +18,7 @@ public async Task DefaultSignOutFallsbackToSignIn() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("signin", "whatever"); + o.AddScheme("signin", "whatever"); o.AddScheme("foobly", "whatever"); o.DefaultSignInScheme = "signin"; }).BuildServiceProvider(); @@ -50,7 +50,7 @@ public async Task DefaultSchemesFallbackToOnlyScheme() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("single", "whatever"); + o.AddScheme("single", "whatever"); }).BuildServiceProvider(); var provider = services.GetRequiredService(); @@ -68,7 +68,7 @@ public async Task DefaultSchemesFallbackToAuthenticateScheme() { o.DefaultAuthenticateScheme = "B"; o.AddScheme("A", "whatever"); - o.AddScheme("B", "whatever"); + o.AddScheme("B", "whatever"); }).BuildServiceProvider(); var provider = services.GetRequiredService(); @@ -84,9 +84,9 @@ public async Task DefaultSchemesAreSet() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("A", "whatever"); - o.AddScheme("B", "whatever"); - o.AddScheme("C", "whatever"); + o.AddScheme("A", "whatever"); + o.AddScheme("B", "whatever"); + o.AddScheme("C", "whatever"); o.DefaultChallengeScheme = "A"; o.DefaultForbidScheme = "B"; o.DefaultSignInScheme = "C"; @@ -102,6 +102,19 @@ public async Task DefaultSchemesAreSet() Assert.Equal("A", (await provider.GetDefaultSignOutSchemeAsync()).Name); } + [Fact] + public async Task DefaultSignInOutWillReturnNullIfNotSignInHandler() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("A", "whatever"); + }).BuildServiceProvider(); + + var provider = services.GetRequiredService(); + Assert.Null(await provider.GetDefaultSignInSchemeAsync()); + Assert.Null(await provider.GetDefaultSignOutSchemeAsync()); + } + [Fact] public async Task SignInSignOutDefaultsToOnlyOne() { @@ -121,6 +134,21 @@ public async Task SignInSignOutDefaultsToOnlyOne() Assert.Equal("signout", (await provider.GetDefaultSignOutSchemeAsync()).Name); } + [Fact] + public async Task SignOutWillNotDefaultsToSignInThatDoesNotSignOut() + { + var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => + { + o.AddScheme("signin", "whatever"); + o.DefaultSignInScheme = "signin"; + }).BuildServiceProvider(); + + var provider = services.GetRequiredService(); + Assert.Equal("signin", (await provider.GetDefaultSignInSchemeAsync()).Name); + Assert.Null(await provider.GetDefaultSignOutSchemeAsync()); + } + + private class Handler : IAuthenticationHandler { public Task AuthenticateAsync() diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs index fca3541f..f498b20c 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs @@ -123,7 +123,7 @@ private class BaseHandler : IAuthenticationHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.None()); + return Task.FromResult(AuthenticateResult.Ignore()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -146,7 +146,7 @@ private class SignInHandler : IAuthenticationSignInHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.None()); + return Task.FromResult(AuthenticateResult.Ignore()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -174,7 +174,7 @@ public class SignOutHandler : IAuthenticationSignOutHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.None()); + return Task.FromResult(AuthenticateResult.Ignore()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -202,7 +202,7 @@ private class UberHandler : IAuthenticationHandler, IAuthenticationRequestHandle { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.None()); + return Task.FromResult(AuthenticateResult.Ignore()); } public Task ChallengeAsync(AuthenticationProperties properties) From 25fcbb56dba452f696fd9ffbfed1415415a52dab Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 22 Jun 2017 13:46:22 -0700 Subject: [PATCH 6/7] React --- .../AuthenticateResult.cs | 6 ++--- .../IAuthenticationSignInHandler.cs | 2 +- .../AuthenticationSchemeProvider.cs | 2 +- .../AuthenticationSchemeProviderTests.cs | 24 +++++++------------ .../AuthenticationServiceTests.cs | 21 +++++++++------- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs index 78daa7db..1da6a0b9 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs @@ -41,7 +41,7 @@ protected AuthenticateResult() { } /// /// Indicates that there was no information returned for this authentication scheme. /// - public bool Ignored { get; protected set; } + public bool None { get; protected set; } /// /// Indicates that authentication was successful. @@ -61,9 +61,9 @@ public static AuthenticateResult Success(AuthenticationTicket ticket) /// Indicates that there was no information returned for this authentication scheme. /// /// The result. - public static AuthenticateResult Ignore() + public static AuthenticateResult NoResult() { - return new AuthenticateResult() { Ignored = true }; + return new AuthenticateResult() { None = true }; } /// diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs index 8f631659..69b88032 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/IAuthenticationSignInHandler.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Authentication /// /// Used to determine if a handler supports SignIn. /// - public interface IAuthenticationSignInHandler : IAuthenticationHandler + public interface IAuthenticationSignInHandler : IAuthenticationSignOutHandler { /// /// Handle sign in. diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index bd1a5279..46c63dce 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -124,7 +124,7 @@ public virtual async Task GetDefaultSignOutSchemeAsync() { return _signOutHandlers[0]; } - return EnsureAssignable(await GetDefaultSignInSchemeAsync()); + return await GetDefaultSignInSchemeAsync(); } /// diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs index 3ae358c0..4eeaedce 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs @@ -18,7 +18,7 @@ public async Task DefaultSignOutFallsbackToSignIn() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("signin", "whatever"); + o.AddScheme("signin", "whatever"); o.AddScheme("foobly", "whatever"); o.DefaultSignInScheme = "signin"; }).BuildServiceProvider(); @@ -50,7 +50,7 @@ public async Task DefaultSchemesFallbackToOnlyScheme() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("single", "whatever"); + o.AddScheme("single", "whatever"); }).BuildServiceProvider(); var provider = services.GetRequiredService(); @@ -68,7 +68,7 @@ public async Task DefaultSchemesFallbackToAuthenticateScheme() { o.DefaultAuthenticateScheme = "B"; o.AddScheme("A", "whatever"); - o.AddScheme("B", "whatever"); + o.AddScheme("B", "whatever"); }).BuildServiceProvider(); var provider = services.GetRequiredService(); @@ -84,9 +84,9 @@ public async Task DefaultSchemesAreSet() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("A", "whatever"); - o.AddScheme("B", "whatever"); - o.AddScheme("C", "whatever"); + o.AddScheme("A", "whatever"); + o.AddScheme("B", "whatever"); + o.AddScheme("C", "whatever"); o.DefaultChallengeScheme = "A"; o.DefaultForbidScheme = "B"; o.DefaultSignInScheme = "C"; @@ -131,7 +131,7 @@ public async Task SignInSignOutDefaultsToOnlyOne() Assert.Equal("basic", (await provider.GetDefaultAuthenticateSchemeAsync()).Name); Assert.Equal("basic", (await provider.GetDefaultChallengeSchemeAsync()).Name); Assert.Equal("signin", (await provider.GetDefaultSignInSchemeAsync()).Name); - Assert.Equal("signout", (await provider.GetDefaultSignOutSchemeAsync()).Name); + Assert.Equal("signin", (await provider.GetDefaultSignOutSchemeAsync()).Name); // Defaults to single sign in scheme } [Fact] @@ -139,16 +139,14 @@ public async Task SignOutWillNotDefaultsToSignInThatDoesNotSignOut() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { - o.AddScheme("signin", "whatever"); + o.AddScheme("signin", "whatever"); o.DefaultSignInScheme = "signin"; }).BuildServiceProvider(); var provider = services.GetRequiredService(); - Assert.Equal("signin", (await provider.GetDefaultSignInSchemeAsync()).Name); Assert.Null(await provider.GetDefaultSignOutSchemeAsync()); } - private class Handler : IAuthenticationHandler { public Task AuthenticateAsync() @@ -178,23 +176,19 @@ public Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties propertie { throw new NotImplementedException(); } - } - private class SignOutHandler : Handler, IAuthenticationSignOutHandler - { public Task SignOutAsync(AuthenticationProperties properties) { throw new NotImplementedException(); } } - private class Uberhandler : SignInHandler, IAuthenticationSignOutHandler + private class SignOutHandler : Handler, IAuthenticationSignOutHandler { public Task SignOutAsync(AuthenticationProperties properties) { throw new NotImplementedException(); } } - } } diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs index f498b20c..471053e6 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationServiceTests.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Authentication public class AuthenticationServiceTests { [Fact] - public async Task CanOnlySignInIfSignInHandler() + public async Task CanOnlySignInIfSupported() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { @@ -32,7 +32,7 @@ public async Task CanOnlySignInIfSignInHandler() } [Fact] - public async Task CanOnlySignOutIfSignOutHandler() + public async Task CanOnlySignOutIfSupported() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { @@ -47,7 +47,7 @@ public async Task CanOnlySignOutIfSignOutHandler() await context.SignOutAsync("uber"); await Assert.ThrowsAsync(() => context.SignOutAsync("base")); await context.SignOutAsync("signout"); - await Assert.ThrowsAsync(() => context.SignOutAsync("signin")); + await context.SignOutAsync("signin"); } [Fact] @@ -97,7 +97,7 @@ public async Task ServicesWithDefaultSignInMethodsTest() await context.AuthenticateAsync(); await context.ChallengeAsync(); await context.ForbidAsync(); - await Assert.ThrowsAsync(() => context.SignOutAsync()); + await context.SignOutAsync(); await context.SignInAsync(new ClaimsPrincipal()); } @@ -123,7 +123,7 @@ private class BaseHandler : IAuthenticationHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Ignore()); + return Task.FromResult(AuthenticateResult.NoResult()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -146,7 +146,7 @@ private class SignInHandler : IAuthenticationSignInHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Ignore()); + return Task.FromResult(AuthenticateResult.NoResult()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -168,13 +168,18 @@ public Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties propertie { return Task.FromResult(0); } + + public Task SignOutAsync(AuthenticationProperties properties) + { + return Task.FromResult(0); + } } public class SignOutHandler : IAuthenticationSignOutHandler { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Ignore()); + return Task.FromResult(AuthenticateResult.NoResult()); } public Task ChallengeAsync(AuthenticationProperties properties) @@ -202,7 +207,7 @@ private class UberHandler : IAuthenticationHandler, IAuthenticationRequestHandle { public Task AuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Ignore()); + return Task.FromResult(AuthenticateResult.NoResult()); } public Task ChallengeAsync(AuthenticationProperties properties) From 45c82a3e952fd9943ca116c0a455fc66713d7d37 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 22 Jun 2017 14:46:25 -0700 Subject: [PATCH 7/7] Remove interface checks in SchemeProvider --- .../AuthenticationSchemeProvider.cs | 25 ++++++------------- .../AuthenticationSchemeProviderTests.cs | 17 ++----------- 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs index 46c63dce..be02a5c3 100644 --- a/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs +++ b/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationSchemeProvider.cs @@ -94,17 +94,17 @@ public virtual Task GetDefaultForbidSchemeAsync() /// this will fallback to . /// /// The scheme that will be used by default for . - public virtual async Task GetDefaultSignInSchemeAsync() + public virtual Task GetDefaultSignInSchemeAsync() { if (_options.DefaultSignInScheme != null) { - return EnsureAssignable(await GetSchemeAsync(_options.DefaultSignInScheme)); + return GetSchemeAsync(_options.DefaultSignInScheme); } if (_signInHandlers.Count == 1) { - return _signInHandlers[0]; + return Task.FromResult(_signInHandlers[0]); } - return EnsureAssignable(await GetDefaultAuthenticateSchemeAsync()); + return GetDefaultAuthenticateSchemeAsync(); } /// @@ -114,17 +114,17 @@ public virtual async Task GetDefaultSignInSchemeAsync() /// this will fallback to if that supoorts sign out. /// /// The scheme that will be used by default for . - public virtual async Task GetDefaultSignOutSchemeAsync() + public virtual Task GetDefaultSignOutSchemeAsync() { if (_options.DefaultSignOutScheme != null) { - return EnsureAssignable(await GetSchemeAsync(_options.DefaultSignOutScheme)); + return GetSchemeAsync(_options.DefaultSignOutScheme); } if (_signOutHandlers.Count == 1) { - return _signOutHandlers[0]; + return Task.FromResult(_signOutHandlers[0]); } - return await GetDefaultSignInSchemeAsync(); + return GetDefaultSignInSchemeAsync(); } /// @@ -207,14 +207,5 @@ public virtual void RemoveScheme(string name) public virtual Task> GetAllSchemesAsync() => Task.FromResult>(_map.Values); - - private static AuthenticationScheme EnsureAssignable(AuthenticationScheme scheme) where T : class - { - if (scheme != null && typeof(T).IsAssignableFrom(scheme.HandlerType)) - { - return scheme; - } - return null; - } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs index 4eeaedce..fc647138 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationSchemeProviderTests.cs @@ -102,19 +102,6 @@ public async Task DefaultSchemesAreSet() Assert.Equal("A", (await provider.GetDefaultSignOutSchemeAsync()).Name); } - [Fact] - public async Task DefaultSignInOutWillReturnNullIfNotSignInHandler() - { - var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => - { - o.AddScheme("A", "whatever"); - }).BuildServiceProvider(); - - var provider = services.GetRequiredService(); - Assert.Null(await provider.GetDefaultSignInSchemeAsync()); - Assert.Null(await provider.GetDefaultSignOutSchemeAsync()); - } - [Fact] public async Task SignInSignOutDefaultsToOnlyOne() { @@ -135,7 +122,7 @@ public async Task SignInSignOutDefaultsToOnlyOne() } [Fact] - public async Task SignOutWillNotDefaultsToSignInThatDoesNotSignOut() + public async Task SignOutWillDefaultsToSignInThatDoesNotSignOut() { var services = new ServiceCollection().AddOptions().AddAuthenticationCore(o => { @@ -144,7 +131,7 @@ public async Task SignOutWillNotDefaultsToSignInThatDoesNotSignOut() }).BuildServiceProvider(); var provider = services.GetRequiredService(); - Assert.Null(await provider.GetDefaultSignOutSchemeAsync()); + Assert.NotNull(await provider.GetDefaultSignOutSchemeAsync()); } private class Handler : IAuthenticationHandler