From 6d37a313979a97bd6d0ef7b3da8c283bcffeddc3 Mon Sep 17 00:00:00 2001 From: Anthony Popelar Date: Fri, 19 Jul 2024 09:51:31 -0700 Subject: [PATCH 1/4] Basic support for getting AuthenticateResult from HttpContext feature --- .../FunctionAuthorizationFeature.cs | 50 +++++++++++++++++++ .../FunctionsFeatureCollectionExtension.cs | 28 +++++++++++ src/in-proc/FunctionsAuthorizationExecutor.cs | 2 + .../FunctionsAuthorizationMiddleware.cs | 3 ++ test/Common.Tests/HttpUtils.cs | 4 +- .../FunctionsAuthorizationExecutorTests.cs | 41 +++++++++++++++ .../FunctionsAuthorizationMiddlewareTests.cs | 42 ++++++++++++++++ 7 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 src/abstractions/FunctionAuthorizationFeature.cs create mode 100644 src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs diff --git a/src/abstractions/FunctionAuthorizationFeature.cs b/src/abstractions/FunctionAuthorizationFeature.cs new file mode 100644 index 0000000..caaf76b --- /dev/null +++ b/src/abstractions/FunctionAuthorizationFeature.cs @@ -0,0 +1,50 @@ +// +// Copyright (c) DarkLoop. All rights reserved. +// + +using DarkLoop.Azure.Functions.Authorization.Internal; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http.Features.Authentication; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Text; +using System.Threading.Tasks; + +namespace DarkLoop.Azure.Functions.Authorization +{ + internal sealed class FunctionAuthorizationFeature : IAuthenticateResultFeature, IHttpAuthenticationFeature + { + private ClaimsPrincipal? _principal; + private AuthenticateResult? _authenticateResult; + + public FunctionAuthorizationFeature(AuthenticateResult result) + { + Check.NotNull(result, nameof(result)); + + AuthenticateResult = result; + } + + + public AuthenticateResult? AuthenticateResult + { + get => _authenticateResult; + set + { + _authenticateResult = value; + _principal = value?.Principal; + } + } + + public ClaimsPrincipal? User + { + get => _principal; + set + { + _authenticateResult = null; + _principal = value; + } + } + } +} diff --git a/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs b/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs new file mode 100644 index 0000000..69ab07c --- /dev/null +++ b/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs @@ -0,0 +1,28 @@ +// +// Copyright (c) DarkLoop. All rights reserved. +// + +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Features.Authentication; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace DarkLoop.Azure.Functions.Authorization.Internal +{ + internal static class FunctionsFeatureCollectionExtension + { + public static void SetAuthenticationFeatures(this IFeatureCollection features, AuthenticateResult result) + { + // A single object is used to handle both of these features so that they stay in sync. + // This is in line with what asp core normally does. + var feature = new FunctionAuthorizationFeature(result); + + features.Set(feature); + features.Set(feature); + } + } +} diff --git a/src/in-proc/FunctionsAuthorizationExecutor.cs b/src/in-proc/FunctionsAuthorizationExecutor.cs index cb656b3..3ed1667 100644 --- a/src/in-proc/FunctionsAuthorizationExecutor.cs +++ b/src/in-proc/FunctionsAuthorizationExecutor.cs @@ -79,6 +79,8 @@ public async Task ExecuteAuthorizationAsync(FunctionExecutingContext context, Ht var authenticateResult = await _policyEvaluator.AuthenticateAsync(filter.Policy, httpContext); + httpContext.Features.SetAuthenticationFeatures(authenticateResult); + // still authenticating in case token is sent to set context user but skipping authorization if (filter.AllowAnonymous) { diff --git a/src/isolated/FunctionsAuthorizationMiddleware.cs b/src/isolated/FunctionsAuthorizationMiddleware.cs index dc2f58a..12ea8fe 100644 --- a/src/isolated/FunctionsAuthorizationMiddleware.cs +++ b/src/isolated/FunctionsAuthorizationMiddleware.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using DarkLoop.Azure.Functions.Authorization.Internal; using DarkLoop.Azure.Functions.Authorization.Properties; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Http.Extensions; @@ -83,6 +84,8 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next var authenticateResult = await _policyEvaluator.AuthenticateAsync(filter.Policy, httpContext); + httpContext.Features.SetAuthenticationFeatures(authenticateResult); + if (filter.AllowAnonymous) { await next(context); diff --git a/test/Common.Tests/HttpUtils.cs b/test/Common.Tests/HttpUtils.cs index f15de2b..12808f1 100644 --- a/test/Common.Tests/HttpUtils.cs +++ b/test/Common.Tests/HttpUtils.cs @@ -25,11 +25,12 @@ public static HttpContext SetupHttpContext(IServiceProvider services) var streamReader = PipeReader.Create(requestStream); var requestHeaders = new HeaderDictionary(); var streamWriter = PipeWriter.Create(responseStream); + var features = new FeatureCollection(); httpContextMock.SetupGet(x => x.RequestServices).Returns(services); httpContextMock.SetupGet(x => x.Request).Returns(requestMock.Object); httpContextMock.SetupGet(x => x.Response).Returns(responseMock.Object); - httpContextMock.SetupGet(x => x.Features).Returns(Mock.Of()); + httpContextMock.SetupGet(x => x.Features).Returns(features); httpContextMock.SetupGet(x => x.Items).Returns(new Dictionary()); requestMock.SetupGet(x => x.RouteValues).Returns(new RouteValueDictionary()); requestMock.SetupGet(x => x.Body).Returns(requestStream); @@ -43,5 +44,6 @@ public static HttpContext SetupHttpContext(IServiceProvider services) return httpContextMock.Object; } + } } diff --git a/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs b/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs index c6584c3..953b081 100644 --- a/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs +++ b/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.AspNetCore.Http.Features.Authentication; using Microsoft.Azure.WebJobs.Host; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -237,6 +238,46 @@ public async Task AuthorizationExecutorShouldThrowWhenFailedAuthenticationAndDoe policyEvaluatorMock.Verify(evaluator => evaluator.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + [TestMethod("AuthorizationExecutor: should throw when failed authentication and does not allow anonymous with Forbid")] + public async Task AuthorizationExecutorShouldSetHttpFeaturesOnSuccessOrFailure() + { + // Arrange + var options = _services!.GetRequiredService>(); + options.CurrentValue.AuthorizationDisabled = false; + + var authorizationProviderMock = new Mock(); + authorizationProviderMock + .Setup(provider => provider.GetAuthorizationAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new FunctionAuthorizationFilter(new AuthorizationPolicyBuilder().RequireAssertion(_ => false).Build(), false)); + + var policyEvaluatorMock = new Mock(); + policyEvaluatorMock + .Setup(evaluator => evaluator.AuthenticateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(new System.Security.Claims.ClaimsPrincipal(), ""))); + policyEvaluatorMock + .Setup(evaluator => evaluator.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(PolicyAuthorizationResult.Success()); + + var executor = new FunctionsAuthorizationExecutor( + authorizationProviderMock.Object, + _services!.GetRequiredService(), + _services!.GetRequiredService(), + policyEvaluatorMock.Object, + _services!.GetRequiredService>(), + _services!.GetRequiredService>()); + + var functionId = Guid.NewGuid(); + var httpContext = HttpUtils.SetupHttpContext(_services!); + var context = SetupExecutingContext(functionId, "TestFunction", httpContext.Request); + + // Act + await executor.ExecuteAuthorizationAsync(context, httpContext); + + // Assert + Assert.IsNotNull(httpContext.Features.Get()?.AuthenticateResult); + Assert.IsNotNull(httpContext.Features.Get()?.User); + } + private static FunctionExecutingContext SetupExecutingContext(Guid functionId, string functionName, HttpRequest request) { var args = new Dictionary diff --git a/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs b/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs index e166c25..0392b6b 100644 --- a/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs +++ b/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.AspNetCore.Http.Features.Authentication; using Microsoft.Azure.Functions.Worker; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -237,6 +238,47 @@ await middleware.Invoke(context, async fc => policyEvaluatorMock.Verify(evaluator => evaluator.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + [TestMethod("AuthorizationMiddleware: on success should set appropriate HttpContext features")] + public async Task AuthorizationMiddlewareOnSuccessShouldSetAppropriateHttpContextFeatures() + { + // Arrange + var authorizationProviderMock = new Mock(); + authorizationProviderMock + .Setup(provider => provider.GetAuthorizationAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new FunctionAuthorizationFilter(new AuthorizationPolicyBuilder().RequireAssertion(_ => false).Build(), false)); + + var policyEvaluatorMock = new Mock(); + policyEvaluatorMock + .Setup(evaluator => evaluator.AuthenticateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(new System.Security.Claims.ClaimsPrincipal(), "fakeauth"))); + policyEvaluatorMock + .Setup(evaluator => evaluator.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(PolicyAuthorizationResult.Success()); + + var middleware = new FunctionsAuthorizationMiddleware( + authorizationProviderMock.Object, + _services!.GetRequiredService(), + _services!.GetRequiredService(), + policyEvaluatorMock.Object, + _services!.GetRequiredService>(), + _services!.GetRequiredService>()); + + var functionId = "098039841"; + var entryPoint = $"{typeof(FakeFunctionClass).FullName}.TestFunction"; + var httpContext = HttpUtils.SetupHttpContext(_services!); + var context = SetupFunctionContext(functionId, "TestFunction", entryPoint, "httpTrigger", "request", httpContext); + + // Act + await middleware.Invoke(context, async fc => + { + await Task.CompletedTask; + }); + + // Assert + Assert.IsNotNull(httpContext.Features.Get()?.AuthenticateResult); + Assert.IsNotNull(httpContext.Features.Get()?.User); + } + private FunctionContext SetupFunctionContext( string functionId, string functionName, string entryPoint, string triggerType, string boundTriggerParamName, HttpContext? httpContext = null) { From b10cceb9390b217f618bcc9ab3c0568cb7b0e1bd Mon Sep 17 00:00:00 2001 From: Anthony Popelar Date: Fri, 19 Jul 2024 13:42:07 -0700 Subject: [PATCH 2/4] Cleanup, comments, and support for FunctionContext --- .../FunctionAuthorizationFeature.cs | 14 ++++---- .../FunctionsFeatureCollectionExtension.cs | 17 +++++---- .../FunctionsAuthorizationMiddleware.cs | 7 +++- .../FunctionsAuthorizationExecutorTests.cs | 2 +- .../Fakes/FakeInvocationFeatures.cs | 36 +++++++++++++++++++ .../FunctionsAuthorizationMiddlewareTests.cs | 8 +++-- 6 files changed, 68 insertions(+), 16 deletions(-) create mode 100644 test/Isolated.Tests/Fakes/FakeInvocationFeatures.cs diff --git a/src/abstractions/FunctionAuthorizationFeature.cs b/src/abstractions/FunctionAuthorizationFeature.cs index caaf76b..f5edee2 100644 --- a/src/abstractions/FunctionAuthorizationFeature.cs +++ b/src/abstractions/FunctionAuthorizationFeature.cs @@ -5,20 +5,21 @@ using DarkLoop.Azure.Functions.Authorization.Internal; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http.Features.Authentication; -using System; -using System.Collections.Generic; -using System.Linq; using System.Security.Claims; -using System.Text; -using System.Threading.Tasks; namespace DarkLoop.Azure.Functions.Authorization { + // This was designed with maximum compatibility with ASP.NET core. It keeps + // two separate features in sync with each other automatically. internal sealed class FunctionAuthorizationFeature : IAuthenticateResultFeature, IHttpAuthenticationFeature { private ClaimsPrincipal? _principal; private AuthenticateResult? _authenticateResult; + /// + /// Construct an instance of the feature with the given AuthenticateResult + /// + /// public FunctionAuthorizationFeature(AuthenticateResult result) { Check.NotNull(result, nameof(result)); @@ -26,7 +27,7 @@ public FunctionAuthorizationFeature(AuthenticateResult result) AuthenticateResult = result; } - + /// public AuthenticateResult? AuthenticateResult { get => _authenticateResult; @@ -37,6 +38,7 @@ public AuthenticateResult? AuthenticateResult } } + /// public ClaimsPrincipal? User { get => _principal; diff --git a/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs b/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs index 69ab07c..db64504 100644 --- a/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs +++ b/src/abstractions/Internal/FunctionsFeatureCollectionExtension.cs @@ -5,17 +5,20 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Features.Authentication; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; namespace DarkLoop.Azure.Functions.Authorization.Internal { + // This functionality is used internally to emulate Asp.net's treatment of AuthenticateResult internal static class FunctionsFeatureCollectionExtension { - public static void SetAuthenticationFeatures(this IFeatureCollection features, AuthenticateResult result) + /// + /// Store the given AuthenticateResult in the IFeatureCollection accessible via + /// IAuthenticateResultFeature and IHttpAuthenticationFeature + /// + /// The feature collection to add to + /// The authentication to expose in the feature collection + /// The object associated with the features + public static FunctionAuthorizationFeature SetAuthenticationFeatures(this IFeatureCollection features, AuthenticateResult result) { // A single object is used to handle both of these features so that they stay in sync. // This is in line with what asp core normally does. @@ -23,6 +26,8 @@ public static void SetAuthenticationFeatures(this IFeatureCollection features, A features.Set(feature); features.Set(feature); + + return feature; } } } diff --git a/src/isolated/FunctionsAuthorizationMiddleware.cs b/src/isolated/FunctionsAuthorizationMiddleware.cs index 12ea8fe..7490e79 100644 --- a/src/isolated/FunctionsAuthorizationMiddleware.cs +++ b/src/isolated/FunctionsAuthorizationMiddleware.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.AspNetCore.Http.Features.Authentication; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Middleware; using Microsoft.Extensions.Logging; @@ -84,7 +85,11 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next var authenticateResult = await _policyEvaluator.AuthenticateAsync(filter.Policy, httpContext); - httpContext.Features.SetAuthenticationFeatures(authenticateResult); + var authenticateFeature = httpContext.Features.SetAuthenticationFeatures(authenticateResult); + + // We also make the features available in the FunctionContext + context.Features.Set(authenticateFeature); + context.Features.Set(authenticateFeature); if (filter.AllowAnonymous) { diff --git a/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs b/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs index 953b081..65de4ca 100644 --- a/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs +++ b/test/InProc.Tests/FunctionsAuthorizationExecutorTests.cs @@ -238,7 +238,7 @@ public async Task AuthorizationExecutorShouldThrowWhenFailedAuthenticationAndDoe policyEvaluatorMock.Verify(evaluator => evaluator.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } - [TestMethod("AuthorizationExecutor: should throw when failed authentication and does not allow anonymous with Forbid")] + [TestMethod("AuthorizationExecutor: should set http features on authenticate success or failure")] public async Task AuthorizationExecutorShouldSetHttpFeaturesOnSuccessOrFailure() { // Arrange diff --git a/test/Isolated.Tests/Fakes/FakeInvocationFeatures.cs b/test/Isolated.Tests/Fakes/FakeInvocationFeatures.cs new file mode 100644 index 0000000..f2b7351 --- /dev/null +++ b/test/Isolated.Tests/Fakes/FakeInvocationFeatures.cs @@ -0,0 +1,36 @@ +// +// Copyright (c) DarkLoop. All rights reserved. +// + +using Microsoft.Azure.Functions.Worker; +using System.Collections; + +namespace Isolated.Tests.Fakes +{ + internal sealed class FakeInvocationFeatures : IInvocationFeatures + { + private Dictionary _underlyingSet = new Dictionary(); + + public T? Get() + { + _underlyingSet.TryGetValue(typeof(T), out var feature); + + return (T?)feature; + } + + public IEnumerator> GetEnumerator() + { + return _underlyingSet.GetEnumerator(); + } + + public void Set(T instance) + { + _underlyingSet.Add(typeof(T), instance); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _underlyingSet.GetEnumerator(); + } + } +} diff --git a/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs b/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs index 0392b6b..b77273a 100644 --- a/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs +++ b/test/Isolated.Tests/FunctionsAuthorizationMiddlewareTests.cs @@ -276,7 +276,10 @@ await middleware.Invoke(context, async fc => // Assert Assert.IsNotNull(httpContext.Features.Get()?.AuthenticateResult); - Assert.IsNotNull(httpContext.Features.Get()?.User); + Assert.IsNotNull(httpContext.Features.Get()?.User); + + Assert.IsNotNull(context.Features.Get()?.AuthenticateResult); + Assert.IsNotNull(context.Features.Get()?.User); } private FunctionContext SetupFunctionContext( @@ -296,10 +299,11 @@ private FunctionContext SetupFunctionContext( } var context = new Mock(); + var features = new FakeInvocationFeatures(); context.Setup(context => context.FunctionId).Returns(functionId); context.Setup(context => context.FunctionDefinition.Name).Returns(functionName); context.Setup(context => context.FunctionDefinition.EntryPoint).Returns(entryPoint); - context.Setup(context => context.Features).Returns(Mock.Of()); + context.Setup(context => context.Features).Returns(features); context.Setup(context => context.Items).Returns(items); context .Setup(contextMock => contextMock.FunctionDefinition.InputBindings) From 01c5f9e8c250b08921a4c3fe29b2025b8d14ddf3 Mon Sep 17 00:00:00 2001 From: Anthony Popelar Date: Fri, 19 Jul 2024 13:42:24 -0700 Subject: [PATCH 3/4] Update ChangeLog and release version to match --- .build/release.props | 2 +- ChangeLog.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.build/release.props b/.build/release.props index 6ec2fe7..21447f8 100644 --- a/.build/release.props +++ b/.build/release.props @@ -8,7 +8,7 @@ DarkLoop's Azure Functions Authorization false 4.0.0.0 - 4.1.0 + 4.1.1 $(Version).0 https://github.com/dark-loop/functions-authorize https://github.com/dark-loop/functions-authorize/blob/master/LICENSE diff --git a/ChangeLog.md b/ChangeLog.md index dfc3db3..93c4d43 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,9 @@ # Change log Change log stars with version 3.1.3 +## 4.1.1 +After authenticate but before authorize IAuthenticateResultFeature and IHttpAuthenticationFeature are now both set in HttpContext.Features and (for isolated Azure Functions) FunctionContext.Features. + ## 4.1.0 - ### [Breaking] Removing support for `Bearer` scheme and adding `FunctionsBearer` Recent security updates in the Azure Functions runtime are clashing with the use of the default, well known `Bearer` scheme.
From 4970ab70f1947cccae41f9556652ebf611cd7452 Mon Sep 17 00:00:00 2001 From: Anthony Popelar Date: Fri, 19 Jul 2024 14:27:07 -0700 Subject: [PATCH 4/4] Swap preview flag to true --- .build/release.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.build/release.props b/.build/release.props index 21447f8..d101a20 100644 --- a/.build/release.props +++ b/.build/release.props @@ -6,7 +6,7 @@ DarkLoop DarkLoop - All rights reserved DarkLoop's Azure Functions Authorization - false + true 4.0.0.0 4.1.1 $(Version).0