From eff520c7eb5ba3e7992accccee31d56335d2bdd1 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 4 Sep 2024 08:31:50 +0200 Subject: [PATCH] Ensure correct OpenID Connect error responses (#16982) --- .../Controllers/Security/MemberController.cs | 40 +++++++++++++++---- .../Security/BackOfficeController.cs | 25 +++++++++--- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/Security/MemberController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/Security/MemberController.cs index f7296a950f..a5b085073b 100644 --- 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 class MemberController : DeliveryApiControllerBase // 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 class MemberController : DeliveryApiControllerBase 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 class MemberController : DeliveryApiControllerBase 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 class MemberController : DeliveryApiControllerBase 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 @@ public class MemberController : DeliveryApiControllerBase 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 @@ public class MemberController : DeliveryApiControllerBase 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 index 217890a539..b948c034da 100644 --- 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 class BackOfficeController : SecurityControllerBase 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 class BackOfficeController : SecurityControllerBase 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 class BackOfficeController : SecurityControllerBase 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 class BackOfficeController : SecurityControllerBase // 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.");