8000 Improve details for user error events in OIDC protocol endpoints by thomasdarimont · Pull Request #29167 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Improve details for user error events in OIDC protocol endpoints #29167

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.

Sign up for GitHub

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

Merged
ahus1 merged 12 commits into from
May 6, 2024
Merged

Improve details for user error events in OIDC protocol endpoints #29167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d280380
Improve details for user error events in protocol endpoints (#29166)
thomasdarimont Apr 25, 2024
574045c
Adapt test to check for proper error type in event
thomasdarimont Apr 30, 2024
b97ace6
Adapt test to check for more detailed error response.
thomasdarimont Apr 30, 2024
0042442
Unified error messages for PKCE error responses
thomasdarimont Apr 30, 2024
729d731
Add debug logging with actual invalid scope
thomasdarimont Apr 30, 2024
4cc360c
Cleanup
thomasdarimont Apr 30, 2024
d31016b
Adapt code according to code review
thomasdarimont May 1, 2024
201fa52
Cleanup
thomasdarimont May 1, 2024
c187344
Revise error message
thomasdarimont May 1, 2024
57a13fe
Cleanup
thomasdarimont May 1, 2024
b06d2db
Revised error events and logging in PkceUtils
thomasdarimont May 2, 2024
5d139d5
Cleanup
thomasdarimont May 2, 2024
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
6 changes: 5 additions & 1 deletion services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
8000
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public AuthorizationEndpoint forgotCredentials() {

private void checkClient(String clientId) {
if (clientId == null) {
event.detail(Details.REASON, "Missing parameter: " + OIDCLoginProtocol.CLIENT_ID_PARAM);
event.error(Errors.INVALID_REQUEST);
throw new ErrorPageException(session, authenticationSession, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER, OIDCLoginProtocol.CLIENT_ID_PARAM);
}
Expand Down Expand Up @@ -257,8 +258,10 @@ private void checkClient(String clientId) {
protocol = OIDCLoginProtocol.LOGIN_PROTOCOL;
}
if (!protocol.equals(OIDCLoginProtocol.LOGIN_PROTOCOL)) {
String errorMessage = "Wrong client protocol.";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_CLIENT);
throw new ErrorPageException(session, authenticationSession, Response.Status.BAD_REQUEST, "Wrong client protocol.");
throw new ErrorPageException(session, authenticationSession, Response.Status.BAD_REQUEST, errorMessage);
}

session.getContext().setClient(client);
Expand Down Expand Up @@ -314,6 +317,7 @@ private void updateAuthenticationSession() {
boolean essential = Boolean.parseBoolean(authenticationSession.getClientNote(Constants.FORCE_LEVEL_OF_AUTHENTICATION));
if (essential) {
logger.errorf("Requested essential acr value '%s' is not a number and it is not mapped in the ACR-To-Loa mappings of realm or client. Please doublecheck ACR-to-LOA mapping or correct ACR passed in the 'claims' parameter.", acr);
event.detail(Details.REASON, "Invalid requested essential acr value");
event.error(Errors.INVALID_REQUEST);
throw new ErrorPageException(session, authenticationSession, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.CLAIMS_PARAM);
} else {
Expand Down
67 changes: 50 additions & 17 deletions ...ices/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,18 @@ public void checkResponseType() throws AuthorizationCheckException {

if (responseType == null) {
ServicesLogger.LOGGER.missingParameter(OAuth2Constants.RESPONSE_TYPE);
String errorMessage = "Missing parameter: response_type";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Missing parameter: response_type");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

event.detail(Details.RESPONSE_TYPE, responseType);

try {
this.parsedResponseType = OIDCResponseType.parse(responseType);
} catch (IllegalArgumentException iae) {
event.detail(Details.REASON, iae.getMessage());
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null);
}
Expand All @@ -157,17 +160,21 @@ public void checkResponseType() throws AuthorizationCheckException {
parsedResponseMode = OIDCResponseMode.parse(request.getResponseMode(), parsedResponseType);
} catch (IllegalArgumentException iae) {
ServicesLogger.LOGGER.invalidParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM);
String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.RESPONSE_MODE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: response_mode");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

event.detail(Details.RESPONSE_MODE, parsedResponseMode.toString().toLowerCase());

// Disallowed by OIDC specs
if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) {
ServicesLogger.LOGGER.responseModeQueryNotAllowed();
String errorMessage = "Response_mode 'query' not allowed for implicit or hybrid flow";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query' not allowed for implicit or hybrid flow");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

this.parsedResponseMode = parsedResponseMode;
Expand All @@ -176,20 +183,26 @@ public void checkResponseType() throws AuthorizationCheckException {
(!StringUtil.isNotBlank(client.getAttribute(OIDCConfigAttributes.AUTHORIZATION_ENCRYPTED_RESPONSE_ALG)) ||
!StringUtil.isNotBlank(client.getAttribute(OIDCConfigAttributes.AUTHORIZATION_ENCRYPTED_RESPONSE_ENC)))) {
ServicesLogger.LOGGER.responseModeQueryJwtNotAllowed();
String errorMessage = "Response_mode 'query.jwt' is allowed only when the authorization response token is encrypted";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query.jwt' is allowed only when the authorization response token is encrypted");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

if ((parsedResponseType.hasResponseType(OIDCResponseType.CODE) || parsedResponseType.hasResponseType(OIDCResponseType.NONE)) && !client.isStandardFlowEnabled()) {
ServicesLogger.LOGGER.flowNotAllowed("Standard");
String errorMessage = "Client is not allowed to initiate browser login with given response_type. Standard flow is disabled for the client.";
event.detail(Details.REASON, errorMessage);
event.error(Errors.NOT_ALLOWED);
throw new AuthorizationCheckException(Response.Status.UNAUTHORIZED, OAuthErrorException.UNAUTHORIZED_CLIENT, "Client is not allowed to initiate browser login with given response_type. Standard flow is disabled for the client.");
throw new AuthorizationCheckException(Response.Status.UNAUTHORIZED, OAuthErrorException.UNAUTHORIZED_CLIENT, errorMessage);
}

if (parsedResponseType.isImplicitOrHybridFlow() && !client.isImplicitFlowEnabled()) {
ServicesLogger.LOGGER.flowNotAllowed("Implicit");
String errorMessage = "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.";
event.detail(Details.REASON, errorMessage);
event.error(Errors.NOT_ALLOWED);
throw new AuthorizationCheckException(Response.Status.UNAUTHORIZED, OAuthErrorException.UNAUTHORIZED_CLIENT, "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.");
throw new AuthorizationCheckException(Response.Status.UNAUTHORIZED, OAuthErrorException.UNAUTHORIZED_CLIENT, errorMessage);
}
}

Expand Down Expand Up @@ -219,8 +232,10 @@ public void checkValidScope() throws AuthorizationCheckException {
}
if (!validScopes) {
ServicesLogger.LOGGER.invalidParameter(OIDCLoginProtocol.SCOPE_PARAM);
String errorMessage = "Invalid scopes: " + request.getScope();
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_SCOPE, "Invalid scopes: " + request.getScope());
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_SCOPE, errorMessage);
}
}

Expand All @@ -233,8 +248,10 @@ public void checkOIDCParams() throws AuthorizationCheckException {

if (parsedResponseType.hasResponseType(OIDCResponseType.ID_TOKEN) && request.getNonce() == null) {
ServicesLogger.LOGGER.missingParameter(OIDCLoginProtocol.NONCE_PARAM);
String errorMessage = "Missing parameter: " + OIDCLoginProtocol.NONCE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Missing parameter: nonce");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

return;
Expand Down Expand Up @@ -270,8 +287,10 @@ public void checkParRequired() throws AuthorizationCheckException {
return;
}
ServicesLogger.LOGGER.missingParameter(OIDCLoginProtocol.REQUEST_URI_PARAM);
String errorMessage = "Pushed Authorization Request is only allowed.";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Pushed Authorization Request is only allowed.");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

// https://tools.ietf.org/html/rfc7636#section-4
Expand All @@ -292,34 +311,44 @@ private void checkParamsForPkceEnforcedClient(String codeChallengeMethod, String
// check whether code challenge method is specified
if (codeChallengeMethod == null) {
logger.info("PKCE enforced Client without code challenge method.");
String errorMessage = "Missing parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Missing parameter: code_challenge_method");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
// check whether specified code challenge method is configured one in advance
if (!codeChallengeMethod.equals(pkceCodeChallengeMethod)) {
logger.info("PKCE enforced Client code challenge method is not configured one.");
logger.info("PKCE enforced Client code challenge method is not matching the configured one.");
String errorMessage = "Invalid parameter: code challenge method is not matching the configured one";
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code challenge method is not configured one");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
// check whether code challenge is specified
if (codeChallenge == null) {
logger.info("PKCE supporting Client without code challenge");
String errorMessage = "Missing parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Missing parameter: code_challenge");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
// check whether code challenge is formatted along with the PKCE specification
if (!isValidPkceCodeChallenge(codeChallenge)) {
logger.infof("PKCE supporting Client with invalid code challenge specified in PKCE, codeChallenge = %s", codeChallenge);
String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code_challenge");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
}

private void checkParamsForPkceNotEnforcedClient(String codeChallengeMethod, String pkceCodeChallengeMethod, String codeChallenge) throws AuthorizationCheckException {
if (codeChallenge == null && codeChallengeMethod != null) {
logger.info("PKCE supporting Client without code challenge");
String errorMessage = "Missing parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Missing parameter: code_challenge");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}

// based on code_challenge value decide whether this client(RP) supports PKCE
Expand All @@ -333,8 +362,10 @@ private void checkParamsForPkceNotEnforcedClient(String codeChallengeMethod, Str
// plain or S256
if (!codeChallengeMethod.equals(OIDCLoginProtocol.PKCE_METHOD_S256) && !codeChallengeMethod.equals(OIDCLoginProtocol.PKCE_METHOD_PLAIN)) {
logger.infof("PKCE supporting Client with invalid code challenge method not specified in PKCE, codeChallengeMethod = %s", codeChallengeMethod);
String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code_challenge_method");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
} else {
// https://tools.ietf.org/html/rfc7636#section-4.3
Expand All @@ -344,8 +375,10 @@ private void checkParamsForPkceNotEnforcedClient(String codeChallengeMethod, Str

if (!isValidPkceCodeChallenge(codeChallenge)) {
logger.infof("PKCE supporting Client with invalid code challenge specified in PKCE, codeChallenge = %s", codeChallenge);
String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.CODE_CHALLENGE_PARAM;
event.detail(Details.REASON, errorMessage);
event.error(Errors.INVALID_REQUEST);
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code_challenge");
throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage);
}
}

Expand Down
Loading
0