diff --git a/src/Microsoft.Identity.Web/Policy/ScopeAuthorizationHandler.cs b/src/Microsoft.Identity.Web/Policy/ScopeAuthorizationHandler.cs index af8d416a2..6f9caf4f5 100644 --- a/src/Microsoft.Identity.Web/Policy/ScopeAuthorizationHandler.cs +++ b/src/Microsoft.Identity.Web/Policy/ScopeAuthorizationHandler.cs @@ -60,7 +60,7 @@ protected override Task HandleRequirementAsync( if (configurationKey != null) { - scopes = _configuration[configurationKey]?.Split(' '); + scopes = _configuration[configurationKey]?.Split(' ') ?? Array.Empty(); } if (scopes is null) diff --git a/src/Microsoft.Identity.Web/Policy/ScopeOrAppPermissionAuthorizationHandler.cs b/src/Microsoft.Identity.Web/Policy/ScopeOrAppPermissionAuthorizationHandler.cs index ae3dd9e64..9c8c8b1ac 100644 --- a/src/Microsoft.Identity.Web/Policy/ScopeOrAppPermissionAuthorizationHandler.cs +++ b/src/Microsoft.Identity.Web/Policy/ScopeOrAppPermissionAuthorizationHandler.cs @@ -63,7 +63,7 @@ protected override Task HandleRequirementAsync( if (scopeConfigurationKey != null) { - scopes = _configuration[scopeConfigurationKey]?.Split(' '); + scopes = _configuration[scopeConfigurationKey]?.Split(' ') ?? Array.Empty(); } if (scopes is null) @@ -75,7 +75,7 @@ protected override Task HandleRequirementAsync( if (appPermissionConfigurationKey != null) { - appPermissions = _configuration.GetValue(appPermissionConfigurationKey)?.Split(' '); + appPermissions = _configuration[appPermissionConfigurationKey]?.Split(' ') ?? Array.Empty(); } if (appPermissions is null) diff --git a/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopeOrAppPermissionPolicyTests.cs b/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopeOrAppPermissionPolicyTests.cs index 30547454f..80dfdf6c3 100644 --- a/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopeOrAppPermissionPolicyTests.cs +++ b/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopeOrAppPermissionPolicyTests.cs @@ -6,6 +6,7 @@ using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Memory; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -153,6 +154,74 @@ public async Task VerifyAppHasAnyAcceptedScopeOrAppPermission_TestAsync() Assert.True(allowed.Succeeded); } + [Fact] + public async Task DeclaredConfigurationKeysWithoutValues_FailAsync() + { + // Arrange + var requirement = new ScopeOrAppPermissionAuthorizationRequirement + { + RequiredScopesConfigurationKey = $"{ConfigSectionName}:MissingScope", + RequiredAppPermissionsConfigurationKey = $"{ConfigSectionName}:MissingAppPermission", + }; + var context = CreateAuthorizationContext( + requirement, + ClaimConstants.Role, + AppPermission); + var handler = new ScopeOrAppPermissionAuthorizationHandler(new ConfigurationBuilder().Build()); + + // Act + await handler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } + + [Fact] + public async Task ValidAppPermissionSucceedsWhenDeclaredScopeConfigurationIsMissingAsync() + { + // Arrange + var requirement = new ScopeOrAppPermissionAuthorizationRequirement( + scopeAllowedValues: null, + new[] { AppPermission }) + { + RequiredScopesConfigurationKey = $"{ConfigSectionName}:MissingScope", + }; + var context = CreateAuthorizationContext( + requirement, + ClaimConstants.Role, + AppPermission); + var handler = new ScopeOrAppPermissionAuthorizationHandler(new ConfigurationBuilder().Build()); + + // Act + await handler.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Fact] + public async Task ValidScopeSucceedsWhenDeclaredAppPermissionConfigurationIsMissingAsync() + { + // Arrange + var requirement = new ScopeOrAppPermissionAuthorizationRequirement( + new[] { Scope }, + appPermissionAllowedValues: null) + { + RequiredAppPermissionsConfigurationKey = $"{ConfigSectionName}:MissingAppPermission", + }; + var context = CreateAuthorizationContext( + requirement, + ClaimConstants.Scp, + Scope); + var handler = new ScopeOrAppPermissionAuthorizationHandler(new ConfigurationBuilder().Build()); + + // Act + await handler.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + private IAuthorizationService BuildAuthorizationService( string policy, string? appPermission, @@ -202,5 +271,16 @@ private IAuthorizationService BuildAuthorizationService( _provider = hostBuilder.Build().Services; return _provider.GetRequiredService(); } + + private static AuthorizationHandlerContext CreateAuthorizationContext( + ScopeOrAppPermissionAuthorizationRequirement requirement, + string claimType, + string claimValue) + { + var user = new ClaimsPrincipal( + new CaseSensitiveClaimsIdentity(new[] { new Claim(claimType, claimValue) })); + + return new AuthorizationHandlerContext(new[] { requirement }, user, resource: null); + } } } diff --git a/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopePolicyTests.cs b/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopePolicyTests.cs index e5c8ccf66..f6ccc830a 100644 --- a/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopePolicyTests.cs +++ b/tests/Microsoft.Identity.Web.Test/Resource/RequiredScopePolicyTests.cs @@ -5,6 +5,8 @@ using System.Collections.Generic; using System.Security.Claims; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Memory; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -116,6 +118,50 @@ public async Task VerifyUserHasAnyAcceptedScope_RequiredScopesMissingAsync() Assert.True(allowed.Succeeded); } + [Fact] + public async Task RequirementConfigurationKeyWithoutValue_FailsAsync() + { + // Arrange + var requirement = new ScopeAuthorizationRequirement(new[] { SingleScope }) + { + RequiredScopesConfigurationKey = $"{ConfigSectionName}:MissingScope", + }; + var user = new ClaimsPrincipal( + new CaseSensitiveClaimsIdentity(new[] { new Claim(ClaimConstants.Scp, SingleScope) })); + var context = new AuthorizationHandlerContext(new[] { requirement }, user, resource: null); + var handler = new ScopeAuthorizationHandler(new ConfigurationBuilder().Build()); + + // Act + await handler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } + + [Fact] + public async Task EndpointConfigurationKeyWithoutValue_FailsAsync() + { + // Arrange + var requirement = new ScopeAuthorizationRequirement(); + var user = new ClaimsPrincipal( + new CaseSensitiveClaimsIdentity(new[] { new Claim(ClaimConstants.Scp, SingleScope) })); + var endpoint = new Endpoint( + _ => Task.CompletedTask, + new EndpointMetadataCollection(new RequiredScopeAttribute + { + RequiredScopesConfigurationKey = $"{ConfigSectionName}:MissingScope", + }), + displayName: null); + var context = new AuthorizationHandlerContext(new[] { requirement }, user, endpoint); + var handler = new ScopeAuthorizationHandler(new ConfigurationBuilder().Build()); + + // Act + await handler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } + [Fact] public async Task IncorrectPolicyName_FailsAsync() {