Ensure correct OpenID Connect error responses (#16982)

umbraco · Sep 4, 2024 · eff520c · eff520c
1 parent 874055e
commit eff520c
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 13 deletions.
diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/Security/MemberController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/Security/MemberController.cs
@@ -79,19 +79,28 @@ public async Task<IActionResult> Authorize()
         // the Authorize endpoint is not allowed unless authorization code flow is enabled.
         if (_deliveryApiSettings.MemberAuthorization?.AuthorizationCodeFlow?.Enabled is not true)
         {
-            return BadRequest("Member authorization is not allowed.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "Not allowed", ErrorDescription = "Member authorization is not allowed."
+            });
         }
 
         OpenIddictRequest? request = HttpContext.GetOpenIddictServerRequest();
         if (request is null)
         {
-            return BadRequest("Unable to obtain OpenID data from the current request.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "No context found", ErrorDescription = "Unable to obtain context from the current request."
+            });
         }
 
         // make sure this endpoint ONLY handles member authentication
         if (request.ClientId is not Constants.OAuthClientIds.Member)
         {
-            return BadRequest("The specified client ID cannot be used here.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "Invalid 'client ID'", ErrorDescription = "The specified 'client_id' is not valid."
+            });
         }
 
         return request.IdentityProvider.IsNullOrWhiteSpace()
@@ -106,7 +115,10 @@ public async Task<IActionResult> Token()
         OpenIddictRequest? request = HttpContext.GetOpenIddictServerRequest();
         if (request is null)
         {
-            return BadRequest("Unable to obtain OpenID data from the current request.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "No context found", ErrorDescription = "Unable to obtain context from the current request."
+            });
         }
 
         // authorization code flow or refresh token flow?
@@ -117,7 +129,10 @@ public async Task<IActionResult> Token()
 
             return authenticateResult is { Succeeded: true, Principal: not null }
                 ? new SignInResult(OpenIddictServerAspNetCoreDefaults.AuthenticationScheme, authenticateResult.Principal)
-                : BadRequest("The supplied authorization was not be verified.");
+                : BadRequest(new OpenIddictResponse
+                {
+                    Error = "Authorization failed", ErrorDescription = "The supplied authorization could not be verified."
+                });
         }
 
         // client credentials flow?
@@ -128,7 +143,10 @@ public async Task<IActionResult> Token()
             MemberIdentityUser? member = await _memberClientCredentialsManager.FindMemberAsync(request.ClientId!);
             return member is not null
                 ? await SignInMember(member, request)
-                : BadRequest("Invalid client or client configuration.");
+                : BadRequest(new OpenIddictResponse
+                {
+                    Error = "Authorization failed", ErrorDescription = "Invalid 'client_id' or client configuration."
+                });
         }
 
         throw new InvalidOperationException("The requested grant type is not supported.");
@@ -159,7 +177,10 @@ private async Task<IActionResult> AuthorizeInternal(OpenIddictRequest request)
         if (member is null)
         {
             _logger.LogError("The member with username {userName} was successfully authorized, but could not be retrieved by the member manager", userName);
-            return BadRequest("The member could not be found.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "Authorization failed", ErrorDescription = "The member associated with the supplied 'client_id' could not be found."
+            });
         }
 
         return await SignInMember(member, request);
@@ -187,7 +208,10 @@ private async Task<IActionResult> AuthorizeExternal(OpenIddictRequest request)
             if (member is null)
             {
                 _logger.LogError("A member was successfully authorized using external authentication, but could not be retrieved by the member manager");
-                return BadRequest("The member could not be found.");
+                return BadRequest(new OpenIddictResponse
+                {
+                    Error = "Authorization failed", ErrorDescription = "The member associated with the supplied 'client_id' could not be found."
+                });
             }
 
             // update member authentication tokens if succeeded

diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs
@@ -169,13 +169,19 @@ public async Task<IActionResult> Authorize(CancellationToken cancellationToken)
         OpenIddictRequest? request = context.GetOpenIddictServerRequest();
         if (request == null)
         {
-            return BadRequest("Unable to obtain OpenID data from the current request");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "No context found", ErrorDescription = "Unable to obtain context from the current request."
+            });
         }
 
         // make sure we keep member authentication away from this endpoint
         if (request.ClientId is Constants.OAuthClientIds.Member)
         {
-            return BadRequest("The specified client ID cannot be used here.");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "Invalid 'client ID'", ErrorDescription = "The specified 'client_id' is not valid."
+            });
         }
 
         return request.IdentityProvider.IsNullOrWhiteSpace()
@@ -192,7 +198,10 @@ public async Task<IActionResult> Token()
         OpenIddictRequest? request = context.GetOpenIddictServerRequest();
         if (request == null)
         {
-            return BadRequest("Unable to obtain OpenID data from the current request");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "No context found", ErrorDescription = "Unable to obtain context from the current request."
+            });
         }
 
         if (request.IsAuthorizationCodeGrantType() || request.IsRefreshTokenGrantType())
@@ -202,7 +211,10 @@ public async Task<IActionResult> Token()
 
             return authenticateResult is { Succeeded: true, Principal: not null }
                 ? new SignInResult(OpenIddictServerAspNetCoreDefaults.AuthenticationScheme, authenticateResult.Principal)
-                : BadRequest("The supplied authorization could not be verified.");
+                : BadRequest(new OpenIddictResponse
+                {
+                    Error = "Authorization failed", ErrorDescription = "The supplied authorization could not be verified."
+                });
         }
 
         if (request.IsClientCredentialsGrantType())
@@ -223,7 +235,10 @@ public async Task<IActionResult> Token()
 
             // if this happens, the OpenIddict applications have somehow gone out of sync with the Umbraco users
             _logger.LogError("The user associated with the client ID ({clientId}) could not be found", request.ClientId);
-            return BadRequest("The user associated with the client ID could not be found");
+            return BadRequest(new OpenIddictResponse
+            {
+                Error = "Authorization failed", ErrorDescription = "The user associated with the supplied 'client_id' could not be found."
+            });
         }
 
         throw new InvalidOperationException("The requested grant type is not supported.");