Changes for error handling in Authentication

HaoK · HaoK · commit d28c6e1dbbd6 · 2015-10-14T14:44:50.000-07:00
diff --git 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
     {
+        /// <summary>
+        /// Constant used to represent the automatic scheme
+        /// </summary>
+        public const string AutomaticScheme = "Automatic";
+
         public abstract IEnumerable<AuthenticationDescription> GetAuthenticationSchemes();
 
         public abstract Task AuthenticateAsync(AuthenticateContext context);
@@ -34,14 +39,14 @@ public virtual Task ChallengeAsync()
 
         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,19 +55,19 @@ public virtual Task ChallengeAsync(string authenticationScheme)
         // 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);
         }
 
         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
@@ -11,9 +11,9 @@ public class AuthenticateContext
     {
         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 @@ public AuthenticateContext(string authenticationScheme)
 
         public IDictionary<string, object> Description { get; private set; }
 
+        public Exception Error { get; private set; }
+
         public virtual void Authenticated(ClaimsPrincipal principal, IDictionary<string, string> properties, IDictionary<string, object> description)
         {
             Accepted = true;
@@ -41,5 +43,11 @@ public virtual void NotAuthenticated()
         {
             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
@@ -15,9 +15,9 @@ public ChallengeContext(string authenticationScheme)
 
         public ChallengeContext(string authenticationScheme, IDictionary<string, string> 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
@@ -11,9 +11,9 @@ public class SignInContext
     {
         public SignInContext(string authenticationScheme, ClaimsPrincipal principal, IDictionary<string, string> 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
@@ -10,9 +10,9 @@ public class SignOutContext
     {
         public SignOutContext(string authenticationScheme, IDictionary<string, string> 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
@@ -54,23 +54,22 @@ public override async Task AuthenticateAsync(AuthenticateContext context)
             }
 
             var handler = HttpAuthenticationFeature.Handler;
-
             if (handler != null)
             {
                 await handler.AuthenticateAsync(context);
             }
 
             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 @@ public override async Task ChallengeAsync(string authenticationScheme, Authentic
 
             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 @@ public override async Task SignInAsync(string authenticationScheme, ClaimsPrinci
 
             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 @@ public override async Task SignOutAsync(string authenticationScheme, Authenticat
 
             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
@@ -12,7 +12,7 @@
 
 namespace Microsoft.AspNet.Http.Authentication.Internal
 {
-    public class AuthenticationManagerTests
+    public class DefaultAuthenticationManagerTests
     {
 
         [Fact]
@@ -23,7 +23,7 @@ public async Task AuthenticateWithNoAuthMiddlewareThrows()
         }
 
         [Theory]
-        [InlineData("")]
+        [InlineData("Automatic")]
         [InlineData("Foo")]
         public async Task ChallengeWithNoAuthMiddlewareMayThrow(string scheme)
         {

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,11 @@ namespace Microsoft.AspNet.Http.Authentication`
`11`	`11`	`{`
`12`	`12`	`public abstract class AuthenticationManager`
`13`	`13`	`{`
	`14`	`+ /// <summary>`
	`15`	`+ /// Constant used to represent the automatic scheme`
	`16`	`+ /// </summary>`
	`17`	`+ public const string AutomaticScheme = "Automatic";`
	`18`	`+`
`14`	`19`	`public abstract IEnumerable<AuthenticationDescription> GetAuthenticationSchemes();`
`15`	`20`
`16`	`21`	`public abstract Task AuthenticateAsync(AuthenticateContext context);`
`@@ -34,14 +39,14 @@ public virtual Task ChallengeAsync()`
`34`	`39`
`35`	`40`	`public virtual Task ChallengeAsync(AuthenticationProperties properties)`
`36`	`41`	`{`
`37`		`- return ChallengeAsync(authenticationScheme: string.Empty, properties: properties);`
	`42`	`+ return ChallengeAsync(authenticationScheme: AutomaticScheme, properties: properties);`
`38`	`43`	`}`
`39`	`44`
`40`	`45`	`public virtual Task ChallengeAsync(string authenticationScheme)`
`41`	`46`	`{`
`42`		`- if (authenticationScheme == null)`
	`47`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`43`	`48`	`{`
`44`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`49`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`45`	`50`	`}`
`46`	`51`
`47`	`52`	`return ChallengeAsync(authenticationScheme: authenticationScheme, properties: null);`
`@@ -50,19 +55,19 @@ public virtual Task ChallengeAsync(string authenticationScheme)`
`50`	`55`	`// Leave it up to authentication handler to do the right thing for the challenge`
`51`	`56`	`public virtual Task ChallengeAsync(string authenticationScheme, AuthenticationProperties properties)`
`52`	`57`	`{`
`53`		`- if (authenticationScheme == null)`
	`58`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`54`	`59`	`{`
`55`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`60`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`56`	`61`	`}`
`57`	`62`
`58`	`63`	`return ChallengeAsync(authenticationScheme, properties, ChallengeBehavior.Automatic);`
`59`	`64`	`}`
`60`	`65`
`61`	`66`	`public virtual Task SignInAsync(string authenticationScheme, ClaimsPrincipal principal)`
`62`	`67`	`{`
`63`		`- if (authenticationScheme == null)`
	`68`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`64`	`69`	`{`
`65`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`70`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`66`	`71`	`}`
`67`	`72`
`68`	`73`	`if (principal == null)`
Original file line number	Diff line number	Diff line change
`@@ -11,9 +11,9 @@ public class AuthenticateContext`
`11`	`11`	`{`
`12`	`12`	`public AuthenticateContext(string authenticationScheme)`
`13`	`13`	`{`
`14`		`- if (authenticationScheme == null)`
	`14`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`15`	`15`	`{`
`16`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`16`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`AuthenticationScheme = authenticationScheme;`
`@@ -29,6 +29,8 @@ public AuthenticateContext(string authenticationScheme)`
`29`	`29`
`30`	`30`	`public IDictionary<string, object> Description { get; private set; }`
`31`	`31`
	`32`	`+ public Exception Error { get; private set; }`
	`33`	`+`
`32`	`34`	`public virtual void Authenticated(ClaimsPrincipal principal, IDictionary<string, string> properties, IDictionary<string, object> description)`
`33`	`35`	`{`
`34`	`36`	`Accepted = true;`
`@@ -41,5 +43,11 @@ public virtual void NotAuthenticated()`
`41`	`43`	`{`
`42`	`44`	`Accepted = true;`
`43`	`45`	`}`
	`46`	`+`
	`47`	`+ public virtual void Failed(Exception error)`
	`48`	`+ {`
	`49`	`+ Error = error;`
	`50`	`+ Accepted = true;`
	`51`	`+ }`
`44`	`52`	`}`
`45`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,9 +15,9 @@ public ChallengeContext(string authenticationScheme)`
`15`	`15`
`16`	`16`	`public ChallengeContext(string authenticationScheme, IDictionary<string, string> properties, ChallengeBehavior behavior)`
`17`	`17`	`{`
`18`		`- if (authenticationScheme == null)`
	`18`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`19`	`19`	`{`
`20`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`20`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`AuthenticationScheme = authenticationScheme;`
Original file line number	Diff line number	Diff line change
`@@ -11,9 +11,9 @@ public class SignInContext`
`11`	`11`	`{`
`12`	`12`	`public SignInContext(string authenticationScheme, ClaimsPrincipal principal, IDictionary<string, string> properties)`
`13`	`13`	`{`
`14`		`- if (authenticationScheme == null)`
	`14`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`15`	`15`	`{`
`16`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`16`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`if (principal == null)`
Original file line number	Diff line number	Diff line change
`@@ -10,9 +10,9 @@ public class SignOutContext`
`10`	`10`	`{`
`11`	`11`	`public SignOutContext(string authenticationScheme, IDictionary<string, string> properties)`
`12`	`12`	`{`
`13`		`- if (authenticationScheme == null)`
	`13`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`14`	`14`	`{`
`15`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`15`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`AuthenticationScheme = authenticationScheme;`
Original file line number	Diff line number	Diff line change
`@@ -54,23 +54,22 @@ public override async Task AuthenticateAsync(AuthenticateContext context)`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`var handler = HttpAuthenticationFeature.Handler;`
`57`		`-`
`58`	`57`	`if (handler != null)`
`59`	`58`	`{`
`60`	`59`	`await handler.AuthenticateAsync(context);`
`61`	`60`	`}`
`62`	`61`
`63`	`62`	`if (!context.Accepted)`
`64`	`63`	`{`
`65`		`- throw new InvalidOperationException($"The following authentication scheme was not accepted: {context.AuthenticationScheme}");`
	`64`	`+ throw new InvalidOperationException($"No authentication handler is configured to authenticate for the scheme: {context.AuthenticationScheme}");`
`66`	`65`	`}`
`67`	`66`	`}`
`68`	`67`
`69`	`68`	`public override async Task ChallengeAsync(string authenticationScheme, AuthenticationProperties properties, ChallengeBehavior behavior)`
`70`	`69`	`{`
`71`		`- if (authenticationScheme == null)`
	`70`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`72`	`71`	`{`
`73`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`72`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`74`	`73`	`}`
`75`	`74`
`76`	`75`	`var handler = HttpAuthenticationFeature.Handler;`
`@@ -83,15 +82,15 @@ public override async Task ChallengeAsync(string authenticationScheme, Authentic`
`83`	`82`
`84`	`83`	`if (!challengeContext.Accepted)`
`85`	`84`	`{`
`86`		`- throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}");`
	`85`	`+ throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}");`
`87`	`86`	`}`
`88`	`87`	`}`
`89`	`88`
`90`	`89`	`public override async Task SignInAsync(string authenticationScheme, ClaimsPrincipal principal, AuthenticationProperties properties)`
`91`	`90`	`{`
`92`		`- if (authenticationScheme == null)`
	`91`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`93`	`92`	`{`
`94`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`93`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`95`	`94`	`}`
`96`	`95`
`97`	`96`	`if (principal == null)`
`@@ -109,15 +108,15 @@ public override async Task SignInAsync(string authenticationScheme, ClaimsPrinci`
`109`	`108`
`110`	`109`	`if (!signInContext.Accepted)`
`111`	`110`	`{`
`112`		`- throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}");`
	`111`	`+ throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}");`
`113`	`112`	`}`
`114`	`113`	`}`
`115`	`114`
`116`	`115`	`public override async Task SignOutAsync(string authenticationScheme, AuthenticationProperties properties)`
`117`	`116`	`{`
`118`		`- if (authenticationScheme == null)`
	`117`	`+ if (string.IsNullOrEmpty(authenticationScheme))`
`119`	`118`	`{`
`120`		`- throw new ArgumentNullException(nameof(authenticationScheme));`
	`119`	`+ throw new ArgumentException(nameof(authenticationScheme));`
`121`	`120`	`}`
`122`	`121`
`123`	`122`	`var handler = HttpAuthenticationFeature.Handler;`
`@@ -130,7 +129,7 @@ public override async Task SignOutAsync(string authenticationScheme, Authenticat`
`130`	`129`
`131`	`130`	`if (!signOutContext.Accepted)`
`132`	`131`	`{`
`133`		`- throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}");`
	`132`	`+ throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {authenticationScheme}");`
`134`	`133`	`}`
`135`	`134`	`}`
`136`	`135`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`
`13`	`13`	`namespace Microsoft.AspNet.Http.Authentication.Internal`
`14`	`14`	`{`
`15`		`- public class AuthenticationManagerTests`
	`15`	`+ public class DefaultAuthenticationManagerTests`
`16`	`16`	`{`
`17`	`17`
`18`	`18`	`[Fact]`
`@@ -23,7 +23,7 @@ public async Task AuthenticateWithNoAuthMiddlewareThrows()`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`[Theory]`
`26`		`- [InlineData("")]`
	`26`	`+ [InlineData("Automatic")]`
`27`	`27`	`[InlineData("Foo")]`
`28`	`28`	`public async Task ChallengeWithNoAuthMiddlewareMayThrow(string scheme)`
`29`	`29`	`{`