From d28c6e1dbbd6156a6168e331ec8137044cd05ee9 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Wed, 14 Oct 2015 14:44:50 -0700 Subject: [PATCH] Changes for error handling in Authentication --- .../Authentication/AuthenticationManager.cs | 19 ++++++++++------- .../Authentication/AuthenticateContext.cs | 12 +++++++++-- .../Authentication/ChallengeContext.cs | 4 ++-- .../Authentication/SignInContext.cs | 4 ++-- .../Authentication/SignOutContext.cs | 4 ++-- .../DefaultAuthenticationManager.cs | 21 +++++++++---------- .../DefaultAuthenticationManagerTests.cs | 4 ++-- 7 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.AspNet.Http.Abstractions/Authentication/AuthenticationManager.cs b/src/Microsoft.AspNet.Http.Abstractions/Authentication/AuthenticationManager.cs index a18fe99c97..497ad3e25d 100644 --- a/src/Microsoft.AspNet.Http.Abstractions/Authentication/AuthenticationManager.cs +++ b/src/Microsoft.AspNet.Http.Abstractions/Authentication/AuthenticationManager.cs @@ -11,6 +11,11 @@ namespace Microsoft.AspNet.Http.Authentication { public abstract class AuthenticationManager { + /// + /// Constant used to represent the automatic scheme + /// + public const string AutomaticScheme = "Automatic"; + public abstract IEnumerable GetAuthenticationSchemes(); public abstract Task AuthenticateAsync(AuthenticateContext context); @@ -34,14 +39,14 @@ namespace Microsoft.AspNet.Http.Authentication public virtual Task ChallengeAsync(AuthenticationProperties properties) { - return ChallengeAsync(authenticationScheme: string.Empty, properties: properties); + return ChallengeAsync(authenticationScheme: AutomaticScheme, properties: properties); } public virtual Task ChallengeAsync(string authenticationScheme) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } return ChallengeAsync(authenticationScheme: authenticationScheme, properties: null); @@ -50,9 +55,9 @@ namespace Microsoft.AspNet.Http.Authentication // Leave it up to authentication handler to do the right thing for the challenge public virtual Task ChallengeAsync(string authenticationScheme, AuthenticationProperties properties) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } return ChallengeAsync(authenticationScheme, properties, ChallengeBehavior.Automatic); @@ -60,9 +65,9 @@ namespace Microsoft.AspNet.Http.Authentication public virtual Task SignInAsync(string authenticationScheme, ClaimsPrincipal principal) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } if (principal == null) diff --git a/src/Microsoft.AspNet.Http.Features/Authentication/AuthenticateContext.cs b/src/Microsoft.AspNet.Http.Features/Authentication/AuthenticateContext.cs index c3ef43c071..187c372fad 100644 --- a/src/Microsoft.AspNet.Http.Features/Authentication/AuthenticateContext.cs +++ b/src/Microsoft.AspNet.Http.Features/Authentication/AuthenticateContext.cs @@ -11,9 +11,9 @@ namespace Microsoft.AspNet.Http.Features.Authentication { public AuthenticateContext(string authenticationScheme) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } AuthenticationScheme = authenticationScheme; @@ -29,6 +29,8 @@ namespace Microsoft.AspNet.Http.Features.Authentication public IDictionary Description { get; private set; } + public Exception Error { get; private set; } + public virtual void Authenticated(ClaimsPrincipal principal, IDictionary properties, IDictionary description) { Accepted = true; @@ -41,5 +43,11 @@ namespace Microsoft.AspNet.Http.Features.Authentication { Accepted = true; } + + public virtual void Failed(Exception error) + { + Error = error; + Accepted = true; + } } } diff --git a/src/Microsoft.AspNet.Http.Features/Authentication/ChallengeContext.cs b/src/Microsoft.AspNet.Http.Features/Authentication/ChallengeContext.cs index d632a5722e..5989ab4245 100644 --- a/src/Microsoft.AspNet.Http.Features/Authentication/ChallengeContext.cs +++ b/src/Microsoft.AspNet.Http.Features/Authentication/ChallengeContext.cs @@ -15,9 +15,9 @@ namespace Microsoft.AspNet.Http.Features.Authentication public ChallengeContext(string authenticationScheme, IDictionary properties, ChallengeBehavior behavior) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } AuthenticationScheme = authenticationScheme; diff --git a/src/Microsoft.AspNet.Http.Features/Authentication/SignInContext.cs b/src/Microsoft.AspNet.Http.Features/Authentication/SignInContext.cs index f4bdef8dcb..08591ffa82 100644 --- a/src/Microsoft.AspNet.Http.Features/Authentication/SignInContext.cs +++ b/src/Microsoft.AspNet.Http.Features/Authentication/SignInContext.cs @@ -11,9 +11,9 @@ namespace Microsoft.AspNet.Http.Features.Authentication { public SignInContext(string authenticationScheme, ClaimsPrincipal principal, IDictionary properties) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } if (principal == null) diff --git a/src/Microsoft.AspNet.Http.Features/Authentication/SignOutContext.cs b/src/Microsoft.AspNet.Http.Features/Authentication/SignOutContext.cs index ba27794c3e..ea89cf9f31 100644 --- a/src/Microsoft.AspNet.Http.Features/Authentication/SignOutContext.cs +++ b/src/Microsoft.AspNet.Http.Features/Authentication/SignOutContext.cs @@ -10,9 +10,9 @@ namespace Microsoft.AspNet.Http.Features.Authentication { public SignOutContext(string authenticationScheme, IDictionary properties) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } AuthenticationScheme = authenticationScheme; diff --git a/src/Microsoft.AspNet.Http/Authentication/DefaultAuthenticationManager.cs b/src/Microsoft.AspNet.Http/Authentication/DefaultAuthenticationManager.cs index 4dcb81916c..f7de7fcd49 100644 --- a/src/Microsoft.AspNet.Http/Authentication/DefaultAuthenticationManager.cs +++ b/src/Microsoft.AspNet.Http/Authentication/DefaultAuthenticationManager.cs @@ -54,7 +54,6 @@ namespace Microsoft.AspNet.Http.Authentication.Internal } var handler = HttpAuthenticationFeature.Handler; - if (handler != null) { await handler.AuthenticateAsync(context); @@ -62,15 +61,15 @@ namespace Microsoft.AspNet.Http.Authentication.Internal if (!context.Accepted) { - throw new InvalidOperationException($"The following authentication scheme was not accepted: {context.AuthenticationScheme}"); + throw new InvalidOperationException($"No authentication handler is configured to authenticate for the scheme: {context.AuthenticationScheme}"); } } public override async Task ChallengeAsync(string authenticationScheme, AuthenticationProperties properties, ChallengeBehavior behavior) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } var handler = HttpAuthenticationFeature.Handler; @@ -83,15 +82,15 @@ namespace Microsoft.AspNet.Http.Authentication.Internal if (!challengeContext.Accepted) { - throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}"); + throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}"); } } public override async Task SignInAsync(string authenticationScheme, ClaimsPrincipal principal, AuthenticationProperties properties) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } if (principal == null) @@ -109,15 +108,15 @@ namespace Microsoft.AspNet.Http.Authentication.Internal if (!signInContext.Accepted) { - throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}"); + throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}"); } } public override async Task SignOutAsync(string authenticationScheme, AuthenticationProperties properties) { - if (authenticationScheme == null) + if (string.IsNullOrEmpty(authenticationScheme)) { - throw new ArgumentNullException(nameof(authenticationScheme)); + throw new ArgumentException(nameof(authenticationScheme)); } var handler = HttpAuthenticationFeature.Handler; @@ -130,7 +129,7 @@ namespace Microsoft.AspNet.Http.Authentication.Internal if (!signOutContext.Accepted) { - throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}"); + throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}"); } } } diff --git a/test/Microsoft.AspNet.Http.Tests/Authentication/DefaultAuthenticationManagerTests.cs b/test/Microsoft.AspNet.Http.Tests/Authentication/DefaultAuthenticationManagerTests.cs index a48e80ea03..2b3f666740 100644 --- a/test/Microsoft.AspNet.Http.Tests/Authentication/DefaultAuthenticationManagerTests.cs +++ b/test/Microsoft.AspNet.Http.Tests/Authentication/DefaultAuthenticationManagerTests.cs @@ -12,7 +12,7 @@ using Xunit; namespace Microsoft.AspNet.Http.Authentication.Internal { - public class AuthenticationManagerTests + public class DefaultAuthenticationManagerTests { [Fact] @@ -23,7 +23,7 @@ namespace Microsoft.AspNet.Http.Authentication.Internal } [Theory] - [InlineData("")] + [InlineData("Automatic")] [InlineData("Foo")] public async Task ChallengeWithNoAuthMiddlewareMayThrow(string scheme) {