Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure correct OpenID Connect error responses #16982

Merged
merged 2 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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?
Expand All @@ -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?
Expand All @@ -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.");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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.");
Expand Down
Loading