Forbid requests whose token has no subject claim, instead of failing with 500

src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs

@@ -6,6 +6,7 @@ namespace ServiceControl.Hosting.Auth;
 using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.Extensions.Logging;
 using ServiceControl.Infrastructure;
 using ServiceControl.Infrastructure.Auth;
 
@@ -22,7 +23,8 @@ namespace ServiceControl.Hosting.Auth;
 /// </summary>
 public sealed class PermissionVerbHandler(
     IAuthorizationAuditLog auditLog,
-    OpenIdConnectSettings oidcSettings)
+    OpenIdConnectSettings oidcSettings,
+    ILogger<PermissionVerbHandler> logger)
     : AuthorizationHandler<PermissionRequirement>
 {
     protected override Task HandleRequirementAsync(
@@ -37,8 +39,25 @@ protected override Task HandleRequirementAsync(
             return Task.CompletedTask;
         }
 
-        var subjectId = context.User.RequireClaim(oidcSettings.SubjectIdClaim, "Authentication.SubjectIdClaim");
-        var subjectName = context.User.RequireClaim(oidcSettings.SubjectNameClaim, "Authentication.SubjectNameClaim");
+        var subjectId = context.User.FindFirst(oidcSettings.SubjectIdClaim)?.Value;
+        var subjectName = context.User.FindFirst(oidcSettings.SubjectNameClaim)?.Value;
+
+        // The audit log needs both values to identify the caller. Without them the request is
+        // forbidden (403), not an unhandled exception (500).
+        if (string.IsNullOrEmpty(subjectId) || string.IsNullOrEmpty(subjectName))
+        {
+            var (claimType, settingName) = string.IsNullOrEmpty(subjectId)
+                ? (oidcSettings.SubjectIdClaim, "Authentication.SubjectIdClaim")
+                : (oidcSettings.SubjectNameClaim, "Authentication.SubjectNameClaim");
+
+            logger.LogWarning(
+                "Access denied: the token has no '{ClaimType}' claim, which is configured by {SettingName}. Configure the identity provider to emit this claim, or change the setting to a claim that the identity provider emits",
+                claimType, settingName);
+
+            context.Fail(new AuthorizationFailureReason(this, $"The token has no '{claimType}' claim, which is configured by {settingName}"));
+            return Task.CompletedTask;
+        }
+
         var roles = context.User.FindAll(ClaimTypes.Role).Select(claim => claim.Value).ToArray();
         var permission = requirement.Permission;
 

src/ServiceControl.Infrastructure.Tests/Auth/PermissionPolicyProviderTests.cs

@@ -77,6 +77,21 @@ public async Task Admins_are_allowed_every_permission_when_role_based_authorizat
         Assert.That(await DeniedPermissions(host, Authenticated(RolePermissions.Admin), TestContext.CurrentContext.CancellationToken), Is.Empty);
     }
 
+    // The audit log needs the subject ID and name to identify the caller. A token without them must
+    // get 403, not an unhandled exception that ASP.NET Core turns into 500.
+    [TestCase("sub")]
+    [TestCase("preferred_username")]
+    public async Task Admins_without_a_subject_claim_are_denied_every_permission(string missingClaim)
+    {
+        using var host = BuildHost(authenticationEnabled: true, roleBasedAuthorizationEnabled: true);
+
+        var user = new ClaimsPrincipal(new ClaimsIdentity(
+            Authenticated(RolePermissions.Admin).Claims.Where(claim => claim.Type != missingClaim),
+            authenticationType: "test"));
+
+        Assert.That(await AllowedPermissions(host, user, TestContext.CurrentContext.CancellationToken), Is.Empty);
+    }
+
     static readonly ClaimsPrincipal Anonymous = new(new ClaimsIdentity());
 
     static ClaimsPrincipal Authenticated(params string[] roles) =>