Refactor dependencies and improve thread safety (#65)

Removed unused JWT and Azure Functions Worker dependencies in `Program.cs` and `SampleIsolatedFunctions.V4.csproj`. Updated authentication scheme in `TestFunction.cs`. Improved thread safety in `FunctionsAuthorizationProvider.cs` and `FunctionsAuthorizationMetadataMiddleware.cs` using `string.Intern` and `KeyedMonitor`. Upgraded `Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore` to version `1.3.2`. Added `ConcurrentTests` to test middleware concurrency. Related issues: #62, #64 Co-authored-by: Arturo Martinez <[email protected]>
dark-loop · Aug 18, 2024 · f5020f3 · f5020f3
1 parent 81033d4
commit f5020f3
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 31 deletions.
diff --git a/.build/release.props b/.build/release.props
@@ -6,9 +6,9 @@
     <Company>DarkLoop</Company>
     <Copyright>DarkLoop - All rights reserved</Copyright>
     <Product>DarkLoop's Azure Functions Authorization</Product>
-    <IsPreview>false</IsPreview>
+    <IsPreview>true</IsPreview>
     <AssemblyVersion>4.0.0.0</AssemblyVersion>
-    <Version>4.1.1</Version>
+    <Version>4.1.2</Version>
     <FileVersion>$(Version).0</FileVersion>
     <RepositoryUrl>https://github.com/dark-loop/functions-authorize</RepositoryUrl>
     <License>https://github.com/dark-loop/functions-authorize/blob/master/LICENSE</License>

diff --git a/sample/SampleIsolatedFunctions.V4/Program.cs b/sample/SampleIsolatedFunctions.V4/Program.cs
@@ -2,10 +2,8 @@
 //  Copyright (c) DarkLoop. All rights reserved.
 // </copyright>
 
-using System.IdentityModel.Tokens.Jwt;
 using Common.Tests;
 using DarkLoop.Azure.Functions.Authorization;
-using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.Azure.Functions.Worker;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;

diff --git a/sample/SampleIsolatedFunctions.V4/SampleIsolatedFunctions.V4.csproj b/sample/SampleIsolatedFunctions.V4/SampleIsolatedFunctions.V4.csproj
@@ -9,10 +9,6 @@
   </PropertyGroup>
   <ItemGroup>
     <FrameworkReference Include="Microsoft.AspNetCore.App" />
-    <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="6.0.0" />
-    <PackageReference Include="Microsoft.Azure.Functions.Worker" Version="1.20.1" />
-    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http" Version="3.1.0" />
-    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore" Version="1.2.0" />
     <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Timer" Version="4.1.0" />
     <PackageReference Include="Microsoft.Azure.Functions.Worker.Sdk" Version="1.16.4" />
     <PackageReference Include="Microsoft.ApplicationInsights.WorkerService" Version="2.21.0" />

diff --git a/sample/SampleIsolatedFunctions.V4/TestFunction.cs b/sample/SampleIsolatedFunctions.V4/TestFunction.cs
@@ -14,7 +14,7 @@
 
 namespace SampleIsolatedFunctions.V4
 {
-    [FunctionAuthorize(AuthenticationSchemes = "Bearer")]
+    [FunctionAuthorize(AuthenticationSchemes = "FunctionsBearer")]
     public class TestFunction
     {
         private readonly ILogger<TestFunction> _logger;

diff --git a/src/abstractions/FunctionsAuthorizationProvider.cs b/src/abstractions/FunctionsAuthorizationProvider.cs
@@ -70,8 +70,8 @@ public async Task<FunctionAuthorizationFilter> GetAuthorizationAsync(string func
                 return filter!;
             }
 
-            var asyncKey = $"fap:{functionName}";
-
+            // ensuring key is interned before entering monitor since key is compared as object
+            var asyncKey = string.Intern($"fap:{functionName}");
             await KeyedMonitor.EnterAsync(asyncKey, unblockOnFirstExit: true);
 
             try

diff --git a/src/isolated/DarkLoop.Azure.Functions.Authorization.Isolated.csproj b/src/isolated/DarkLoop.Azure.Functions.Authorization.Isolated.csproj
@@ -21,9 +21,8 @@
 
   <ItemGroup>
     <FrameworkReference Include="Microsoft.AspNetCore.App" />
-    <PackageReference Include="Microsoft.Azure.Functions.Worker.Core" Version="1.15.0" />
     <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Abstractions" Version="1.3.0" />
-    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore" Version="1.1.0" />
+    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore" Version="1.3.2" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/isolated/Metadata/FunctionsAuthorizationMetadataMiddleware.cs b/src/isolated/Metadata/FunctionsAuthorizationMetadataMiddleware.cs
@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using DarkLoop.Azure.Functions.Authorization.Extensions;
 using DarkLoop.Azure.Functions.Authorization.Features;
+using DarkLoop.Azure.Functions.Authorization.Internal;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.Azure.Functions.Worker;
 using Microsoft.Azure.Functions.Worker.Middleware;
@@ -39,9 +40,9 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next
                 return;
             }
 
-            if(!_options.IsFunctionRegistered(context.FunctionDefinition.Name))
+            if (!_options.IsFunctionRegistered(context.FunctionDefinition.Name))
             {
-                RegisterHttpTriggerAuthorization(context);
+                await RegisterHttpTriggerAuthorizationAsync(context);
             }
 
             context.Features.Set<IFunctionsAuthorizationFeature>(
@@ -50,27 +51,45 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next
             await next(context);
         }
 
-        private void RegisterHttpTriggerAuthorization(FunctionContext context)
+        private async Task RegisterHttpTriggerAuthorizationAsync(FunctionContext context)
         {
-            var functionName = context.FunctionDefinition.Name;
-            var declaringTypeName = context.FunctionDefinition.EntryPoint.LastIndexOf('.') switch
+            // Middleware can be hit concurrently, we need to ensure this functionality
+            // is thread-safe on a per function basis.
+            // Ensuring key is interned before entering monitor since key is compared as object
+            var monitorKey = string.Intern($"famm:{context.FunctionId}");
+            await KeyedMonitor.EnterAsync(monitorKey);
+
+            try
             {
-                -1 => string.Empty,
-                var index => context.FunctionDefinition.EntryPoint[..index]
-            };
+                if (_options.IsFunctionRegistered(context.FunctionDefinition.Name))
+                {
+                    return;
+                }
+
+                var functionName = context.FunctionDefinition.Name;
+                var declaringTypeName = context.FunctionDefinition.EntryPoint.LastIndexOf('.') switch
+                {
+                    -1 => string.Empty,
+                    var index => context.FunctionDefinition.EntryPoint[..index]
+                };
 
-            var methodName = context.FunctionDefinition.EntryPoint[(declaringTypeName.Length + 1)..];
-            var assemblies = AppDomain.CurrentDomain.GetAssemblies();
-            var method = assemblies.Select(a => a.GetType(declaringTypeName, throwOnError: false))
-                .FirstOrDefault(t => t is not null)?
-                .GetMethod(methodName, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static) ??
-                throw new MethodAccessException(
-                    $"Method instance for function '{context.FunctionDefinition.Name}' " +
-                    $"cannot be found or cannot be accessed due to its protection level.");
+                var methodName = context.FunctionDefinition.EntryPoint[(declaringTypeName.Length + 1)..];
+                var assemblies = AppDomain.CurrentDomain.GetAssemblies();
+                var method = assemblies.Select(a => a.GetType(declaringTypeName, throwOnError: false))
+                    .FirstOrDefault(t => t is not null)?
+                    .GetMethod(methodName, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static) ??
+                    throw new MethodAccessException(
+                        $"Method instance for function '{context.FunctionDefinition.Name}' " +
+                        $"cannot be found or cannot be accessed due to its protection level.");
 
-            var declaringType = method.DeclaringType!;
+                var declaringType = method.DeclaringType!;
 
-            _options.RegisterFunctionAuthorizationAttributesMetadata<AuthorizeAttribute>(functionName, declaringType, method);
+                _options.RegisterFunctionAuthorizationAttributesMetadata<AuthorizeAttribute>(functionName, declaringType, method);
+            }
+            finally
+            {
+                KeyedMonitor.Exit(monitorKey);
+            }
         }
     }
 }
diff --git a/test/Isolated.Tests/ConcurrentTests.cs b/test/Isolated.Tests/ConcurrentTests.cs
@@ -0,0 +1,37 @@
+// <copyright file="IntegrationTests.cs" company="DarkLoop" author="Arturo Martinez">
+//  Copyright (c) DarkLoop. All rights reserved.
+// </copyright>
+
+using System.Net;
+
+namespace Isolated.Tests
+{
+    [TestClass]
+    public class ConcurrentTests
+    {
+        [TestMethod]
+        [Ignore("This is to test middleware concurrency")]
+        public async Task TestFunctionAuthorizationMetadataCollectionAsync()
+        {
+            // Arrange
+            var client = new HttpClient { BaseAddress = new Uri("http://localhost:7005/") };
+
+            // Act
+            var message1 = new HttpRequestMessage(HttpMethod.Get, "api/testfunction");
+            var message2 = new HttpRequestMessage(HttpMethod.Get, "api/testfunction");
+            var message3 = new HttpRequestMessage(HttpMethod.Get, "api/testfunction");
+            var message4 = new HttpRequestMessage(HttpMethod.Get, "api/testfunction");
+            var request1 = client.SendAsync(message1);
+            var request2 = client.SendAsync(message2);
+            var request3 = client.SendAsync(message3);
+            var request4 = client.SendAsync(message4);
+
+            // Assert
+            await Task.WhenAll(request1, request2, request3, request4);
+            Assert.AreEqual(HttpStatusCode.Unauthorized, request1.Result.StatusCode);
+            Assert.AreEqual(HttpStatusCode.Unauthorized, request2.Result.StatusCode);
+            Assert.AreEqual(HttpStatusCode.Unauthorized, request3.Result.StatusCode);
+            Assert.AreEqual(HttpStatusCode.Unauthorized, request4.Result.StatusCode);
+        }
+    }
+}