Add a feature for accessing the AuthenticateResult (#33408)

BrennanConroy · web-flow · commit dbf84eaa5a8f · 2021-06-25T19:31:20.000-07:00
diff --git a/src/Http/Authentication.Abstractions/src/IAuthenticateResultFeature.cs b/src/Http/Authentication.Abstractions/src/IAuthenticateResultFeature.cs
@@ -0,0 +1,19 @@
+// 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 Microsoft.AspNetCore.Http.Features.Authentication;
+
+namespace Microsoft.AspNetCore.Authentication
+{
+    /// <summary>
+    /// Used to capture the <see cref="AuthenticateResult"/> from the authorization middleware.
+    /// </summary>
+    public interface IAuthenticateResultFeature
+    {
+        /// <summary>
+        /// The <see cref="AuthenticateResult"/> from the authorization middleware.
+        /// Set to null if the <see cref="IHttpAuthenticationFeature.User"/> property is set after the authorization middleware.
+        /// </summary>
+        AuthenticateResult? AuthenticateResult { get; set; }
+    }
+}
diff --git a/src/Http/Authentication.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Authentication.Abstractions/src/PublicAPI.Unshipped.txt
@@ -1 +1,4 @@
 #nullable enable
+Microsoft.AspNetCore.Authentication.IAuthenticateResultFeature
+Microsoft.AspNetCore.Authentication.IAuthenticateResultFeature.AuthenticateResult.get -> Microsoft.AspNetCore.Authentication.AuthenticateResult?
+Microsoft.AspNetCore.Authentication.IAuthenticateResultFeature.AuthenticateResult.set -> void
diff --git a/src/Security/Authentication/Core/src/AuthenticationFeatures.cs b/src/Security/Authentication/Core/src/AuthenticationFeatures.cs
@@ -0,0 +1,42 @@
+// 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 Microsoft.AspNetCore.Http.Features.Authentication;
+
+namespace Microsoft.AspNetCore.Authentication
+{
+    /// <summary>
+    /// Keeps the User and AuthenticationResult consistent with each other
+    /// </summary>
+    internal sealed class AuthenticationFeatures : IAuthenticateResultFeature, IHttpAuthenticationFeature
+    {
+        private ClaimsPrincipal? _user;
+        private AuthenticateResult? _result;
+
+        public AuthenticationFeatures(AuthenticateResult result)
+        {
+            AuthenticateResult = result;
+        }
+
+        public AuthenticateResult? AuthenticateResult
+        {
+            get => _result;
+            set
+            {
+                _result = value;
+                _user = _result?.Principal;
+            }
+        }
+
+        public ClaimsPrincipal? User
+        {
+            get => _user;
+            set
+            {
+                _user = value;
+                _result = null;
+            }
+        }
+    }
+}
diff --git a/src/Security/Authentication/Core/src/AuthenticationMiddleware.cs b/src/Security/Authentication/Core/src/AuthenticationMiddleware.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features.Authentication;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.AspNetCore.Authentication
@@ -71,6 +72,12 @@ public async Task Invoke(HttpContext context)
                 {
                     context.User = result.Principal;
                 }
+                if (result?.Succeeded ?? false)
+                {
+                    var authFeatures = new AuthenticationFeatures(result);
+                    context.Features.Set<IHttpAuthenticationFeature>(authFeatures);
+                    context.Features.Set<IAuthenticateResultFeature>(authFeatures);
+                }
             }
 
             await _next(context);
diff --git a/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs b/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs
@@ -3,12 +3,15 @@
 using System;
 using System.Security.Claims;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.TestHost;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Authentication
@@ -54,6 +57,126 @@ public async Task OnlyInvokesCanHandleRequestHandlers()
             Assert.Equal(607, (int)response.StatusCode);
         }
 
+        [Fact]
+        public async Task IAuthenticateResultFeature_SetOnSuccessfulAuthenticate()
+        {
+            var authenticationService = new Mock<IAuthenticationService>();
+            authenticationService.Setup(s => s.AuthenticateAsync(It.IsAny<HttpContext>(), It.IsAny<string>()))
+                .Returns(Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), "custom"))));
+            var schemeProvider = new Mock<IAuthenticationSchemeProvider>();
+            schemeProvider.Setup(p => p.GetDefaultAuthenticateSchemeAsync())
+                .Returns(Task.FromResult(new AuthenticationScheme("custom", "custom", typeof(JwtBearerHandler))));
+            var middleware = new AuthenticationMiddleware(c => Task.CompletedTask, schemeProvider.Object);
+            var context = GetHttpContext(authenticationService: authenticationService.Object);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            var authenticateResultFeature = context.Features.Get<IAuthenticateResultFeature>();
+            Assert.NotNull(authenticateResultFeature);
+            Assert.NotNull(authenticateResultFeature.AuthenticateResult);
+            Assert.True(authenticateResultFeature.AuthenticateResult.Succeeded);
+            Assert.Same(context.User, authenticateResultFeature.AuthenticateResult.Principal);
+        }
+
+        [Fact]
+        public async Task IAuthenticateResultFeature_NotSetOnUnsuccessfulAuthenticate()
+        {
+            var authenticationService = new Mock<IAuthenticationService>();
+            authenticationService.Setup(s => s.AuthenticateAsync(It.IsAny<HttpContext>(), It.IsAny<string>()))
+                .Returns(Task.FromResult(AuthenticateResult.Fail("not authenticated")));
+            var schemeProvider = new Mock<IAuthenticationSchemeProvider>();
+            schemeProvider.Setup(p => p.GetDefaultAuthenticateSchemeAsync())
+                .Returns(Task.FromResult(new AuthenticationScheme("custom", "custom", typeof(JwtBearerHandler))));
+            var middleware = new AuthenticationMiddleware(c => Task.CompletedTask, schemeProvider.Object);
+            var context = GetHttpContext(authenticationService: authenticationService.Object);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            var authenticateResultFeature = context.Features.Get<IAuthenticateResultFeature>();
+            Assert.Null(authenticateResultFeature);
+        }
+
+        [Fact]
+        public async Task IAuthenticateResultFeature_NullResultWhenUserSetAfter()
+        {
+            var authenticationService = new Mock<IAuthenticationService>();
+            authenticationService.Setup(s => s.AuthenticateAsync(It.IsAny<HttpContext>(), It.IsAny<string>()))
+                .Returns(Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), "custom"))));
+            var schemeProvider = new Mock<IAuthenticationSchemeProvider>();
+            schemeProvider.Setup(p => p.GetDefaultAuthenticateSchemeAsync())
+                .Returns(Task.FromResult(new AuthenticationScheme("custom", "custom", typeof(JwtBearerHandler))));
+            var middleware = new AuthenticationMiddleware(c => Task.CompletedTask, schemeProvider.Object);
+            var context = GetHttpContext(authenticationService: authenticationService.Object);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            var authenticateResultFeature = context.Features.Get<IAuthenticateResultFeature>();
+            Assert.NotNull(authenticateResultFeature);
+            Assert.NotNull(authenticateResultFeature.AuthenticateResult);
+            Assert.True(authenticateResultFeature.AuthenticateResult.Succeeded);
+            Assert.Same(context.User, authenticateResultFeature.AuthenticateResult.Principal);
+
+            context.User = new ClaimsPrincipal();
+            Assert.Null(authenticateResultFeature.AuthenticateResult);
+        }
+
+        [Fact]
+        public async Task IAuthenticateResultFeature_SettingResultSetsUser()
+        {
+            var authenticationService = new Mock<IAuthenticationService>();
+            authenticationService.Setup(s => s.AuthenticateAsync(It.IsAny<HttpContext>(), It.IsAny<string>()))
+                .Returns(Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), "custom"))));
+            var schemeProvider = new Mock<IAuthenticationSchemeProvider>();
+            schemeProvider.Setup(p => p.GetDefaultAuthenticateSchemeAsync())
+                .Returns(Task.FromResult(new AuthenticationScheme("custom", "custom", typeof(JwtBearerHandler))));
+            var middleware = new AuthenticationMiddleware(c => Task.CompletedTask, schemeProvider.Object);
+            var context = GetHttpContext(authenticationService: authenticationService.Object);
+
+            // Act
+            await middleware.Invoke(context);
+
+            // Assert
+            var authenticateResultFeature = context.Features.Get<IAuthenticateResultFeature>();
+            Assert.NotNull(authenticateResultFeature);
+            Assert.NotNull(authenticateResultFeature.AuthenticateResult);
+            Assert.True(authenticateResultFeature.AuthenticateResult.Succeeded);
+            Assert.Same(context.User, authenticateResultFeature.AuthenticateResult.Principal);
+
+            var newTicket = new AuthenticationTicket(new ClaimsPrincipal(), "");
+            authenticateResultFeature.AuthenticateResult = AuthenticateResult.Success(newTicket);
+            Assert.Same(context.User, newTicket.Principal);
+        }
+
+        private HttpContext GetHttpContext(
+            Action<IServiceCollection> registerServices = null,
+            IAuthenticationService authenticationService = null)
+        {
+            // ServiceProvider
+            var serviceCollection = new ServiceCollection();
+
+            authenticationService = authenticationService ?? Mock.Of<IAuthenticationService>();
+
+            serviceCollection.AddSingleton(authenticationService);
+            serviceCollection.AddOptions();
+            serviceCollection.AddLogging();
+            serviceCollection.AddAuthentication();
+            registerServices?.Invoke(serviceCollection);
+
+            var serviceProvider = serviceCollection.BuildServiceProvider();
+
+            //// HttpContext
+            var httpContext = new DefaultHttpContext();
+            httpContext.RequestServices = serviceProvider;
+
+            return httpContext;
+        }
+
         private class ThreeOhFiveHandler : StatusCodeHandler {
             public ThreeOhFiveHandler() : base(305) { }
         }
@@ -77,7 +200,7 @@ public StatusCodeHandler(int code)
             {
                 _code = code;
             }
-            
+
             public Task<AuthenticateResult> AuthenticateAsync()
             {
                 throw new NotImplementedException();
diff --git a/src/Security/Authorization/Policy/src/AuthenticationFeatures.cs b/src/Security/Authorization/Policy/src/AuthenticationFeatures.cs
@@ -0,0 +1,43 @@
+// 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 Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Http.Features.Authentication;
+
+namespace Microsoft.AspNetCore.Authorization.Policy
+{
+    /// <summary>
+    /// Keeps the User and AuthenticationResult consistent with each other
+    /// </summary>
+    internal sealed class AuthenticationFeatures : IAuthenticateResultFeature, IHttpAuthenticationFeature
+    {
+        private ClaimsPrincipal? _user;
+        private AuthenticateResult? _result;
+
+        public AuthenticationFeatures(AuthenticateResult result)
+        {
+            AuthenticateResult = result;
+        }
+
+        public AuthenticateResult? AuthenticateResult
+        {
+            get => _result;
+            set
+            {
+                _result = value;
+                _user = _result?.Principal;
+            }
+        }
+
+        public ClaimsPrincipal? User
+        {
+            get => _user;
+            set
+            {
+                _user = value;
+                _result = null;
+            }
+        }
+    }
+}
diff --git a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs
@@ -3,8 +3,10 @@
 
 using System;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authorization.Policy;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features.Authentication;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.AspNetCore.Authorization
@@ -29,7 +31,7 @@ public class AuthorizationMiddleware
         /// </summary>
         /// <param name="next">The next middleware in the application middleware pipeline.</param>
         /// <param name="policyProvider">The <see cref="IAuthorizationPolicyProvider"/>.</param>
-        public AuthorizationMiddleware(RequestDelegate next, IAuthorizationPolicyProvider policyProvider) 
+        public AuthorizationMiddleware(RequestDelegate next, IAuthorizationPolicyProvider policyProvider)
         {
             _next = next ?? throw new ArgumentNullException(nameof(next));
             _policyProvider = policyProvider ?? throw new ArgumentNullException(nameof(policyProvider));
@@ -64,12 +66,26 @@ public async Task Invoke(HttpContext context)
                 return;
             }
 
-            // Policy evaluator has transient lifetime so it fetched from request services instead of injecting in constructor
+            // Policy evaluator has transient lifetime so it's fetched from request services instead of injecting in constructor
             var policyEvaluator = context.RequestServices.GetRequiredService<IPolicyEvaluator>();
 
             var authenticateResult = await policyEvaluator.AuthenticateAsync(policy, context);
 
-            // Allow Anonymous skips all authorization
+            if (authenticateResult?.Succeeded ?? false)
+            {
+                if (context.Features.Get<IAuthenticateResultFeature>() is IAuthenticateResultFeature authenticateResultFeature)
+                {
+                    authenticateResultFeature.AuthenticateResult = authenticateResult;
+                }
+                else
+                {
+                    var authFeatures = new AuthenticationFeatures(authenticateResult);
+                    context.Features.Set<IHttpAuthenticationFeature>(authFeatures);
+                    context.Features.Set<IAuthenticateResultFeature>(authFeatures);
+                }
+            }
+
+            // Allow Anonymous still wants to run authorization to populate the User but skips any failure/challenge handling
             if (endpoint?.Metadata.GetMetadata<IAllowAnonymous>() != null)
             {
                 await _next(context);
@@ -85,8 +101,8 @@ public async Task Invoke(HttpContext context)
             {
                 resource = context;
             }
-            
-            var authorizeResult = await policyEvaluator.AuthorizeAsync(policy, authenticateResult, context, resource);
+
+            var authorizeResult = await policyEvaluator.AuthorizeAsync(policy, authenticateResult!, context, resource);
             var authorizationMiddlewareResultHandler = context.RequestServices.GetRequiredService<IAuthorizationMiddlewareResultHandler>();
             await authorizationMiddlewareResultHandler.HandleAsync(_next, context, policy, authorizeResult);
         }
diff --git a/src/Security/Authorization/Policy/src/PolicyEvaluator.cs b/src/Security/Authorization/Policy/src/PolicyEvaluator.cs
@@ -38,19 +38,29 @@ public virtual async Task<AuthenticateResult> AuthenticateAsync(AuthorizationPol
             if (policy.AuthenticationSchemes != null && policy.AuthenticationSchemes.Count > 0)
             {
                 ClaimsPrincipal? newPrincipal = null;
+                DateTimeOffset? minExpiresUtc = null;
                 foreach (var scheme in policy.AuthenticationSchemes)
                 {
                     var result = await context.AuthenticateAsync(scheme);
                     if (result != null && result.Succeeded)
                     {
                         newPrincipal = SecurityHelper.MergeUserPrincipal(newPrincipal, result.Principal);
+
+                        if (minExpiresUtc is null || result.Properties?.ExpiresUtc < minExpiresUtc)
+                        {
+                            minExpiresUtc = result.Properties?.ExpiresUtc;
+                        }
                     }
                 }
 
                 if (newPrincipal != null)
                 {
                     context.User = newPrincipal;
-                    return AuthenticateResult.Success(new AuthenticationTicket(newPrincipal, string.Join(";", policy.AuthenticationSchemes)));
+                    var ticket = new AuthenticationTicket(newPrincipal, string.Join(";", policy.AuthenticationSchemes));
+                    // ExpiresUtc is the easiest property to reason about when dealing with multiple schemes
+                    // SignalR will use this property to evaluate auth expiration for long running connections
+                    ticket.Properties.ExpiresUtc = minExpiresUtc;
+                    return AuthenticateResult.Success(ticket);
                 }
                 else
                 {
diff --git a/src/Security/Authorization/test/AuthorizationMiddlewareTests.cs b/src/Security/Authorization/test/AuthorizationMiddlewareTests.cs

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`using System;`
`5`	`5`	`using System.Threading.Tasks;`
`6`	`6`	`using Microsoft.AspNetCore.Http;`
	`7`	`+using Microsoft.AspNetCore.Http.Features.Authentication;`
`7`	`8`	`using Microsoft.Extensions.DependencyInjection;`
`8`	`9`
`9`	`10`	`namespace Microsoft.AspNetCore.Authentication`
`@@ -71,6 +72,12 @@ public async Task Invoke(HttpContext context)`
`71`	`72`	`{`
`72`	`73`	`context.User = result.Principal;`
`73`	`74`	`}`
	`75`	`+ if (result?.Succeeded ?? false)`
	`76`	`+ {`
	`77`	`+ var authFeatures = new AuthenticationFeatures(result);`
	`78`	`+ context.Features.Set<IHttpAuthenticationFeature>(authFeatures);`
	`79`	`+ context.Features.Set<IAuthenticateResultFeature>(authFeatures);`
	`80`	`+ }`
`74`	`81`	`}`
`75`	`82`
`76`	`83`	`await _next(context);`
Original file line number	Diff line number	Diff line change
`@@ -38,19 +38,29 @@ public virtual async Task<AuthenticateResult> AuthenticateAsync(AuthorizationPol`
`38`	`38`	`if (policy.AuthenticationSchemes != null && policy.AuthenticationSchemes.Count > 0)`
`39`	`39`	`{`
`40`	`40`	`ClaimsPrincipal? newPrincipal = null;`
	`41`	`+ DateTimeOffset? minExpiresUtc = null;`
`41`	`42`	`foreach (var scheme in policy.AuthenticationSchemes)`
`42`	`43`	`{`
`43`	`44`	`var result = await context.AuthenticateAsync(scheme);`
`44`	`45`	`if (result != null && result.Succeeded)`
`45`	`46`	`{`
`46`	`47`	`newPrincipal = SecurityHelper.MergeUserPrincipal(newPrincipal, result.Principal);`
	`48`	`+`
	`49`	`+ if (minExpiresUtc is null \|\| result.Properties?.ExpiresUtc < minExpiresUtc)`
	`50`	`+ {`
	`51`	`+ minExpiresUtc = result.Properties?.ExpiresUtc;`
	`52`	`+ }`
`47`	`53`	`}`
`48`	`54`	`}`
`49`	`55`
`50`	`56`	`if (newPrincipal != null)`
`51`	`57`	`{`
`52`	`58`	`context.User = newPrincipal;`
`53`		`- return AuthenticateResult.Success(new AuthenticationTicket(newPrincipal, string.Join(";", policy.AuthenticationSchemes)));`
	`59`	`+ var ticket = new AuthenticationTicket(newPrincipal, string.Join(";", policy.AuthenticationSchemes));`
	`60`	`+ // ExpiresUtc is the easiest property to reason about when dealing with multiple schemes`
	`61`	`+ // SignalR will use this property to evaluate auth expiration for long running connections`
	`62`	`+ ticket.Properties.ExpiresUtc = minExpiresUtc;`
	`63`	`+ return AuthenticateResult.Success(ticket);`
`54`	`64`	`}`
`55`	`65`	`else`
`56`	`66`	`{`