diff --git a/common/src/main/java/org/keycloak/common/Profile.java b/common/src/main/java/org/keycloak/common/Profile.java index e1601bafed7..661670da819 100755 --- a/common/src/main/java/org/keycloak/common/Profile.java +++ b/common/src/main/java/org/keycloak/common/Profile.java @@ -135,6 +135,8 @@ public class Profile { OID4VC_VCI_PREAUTH_CODE("Support for credential offers with `pre-authorized_code` grant.", Type.EXPERIMENTAL, OID4VC_VCI), OID4VC_VCI_REST_CREDENTIAL_OFFER("Support for the REST endpoint to create credential offers.", Type.EXPERIMENTAL, OID4VC_VCI), + OID4VC_HAIP("OpenID4VC High Assurance Interoperability Profile 1.0", Type.EXPERIMENTAL, OID4VC_VCI), + OPENTELEMETRY("OpenTelemetry support", Type.DEFAULT), OPENTELEMETRY_LOGS("OpenTelemetry Logs support", Type.PREVIEW, OPENTELEMETRY), OPENTELEMETRY_METRICS("Micrometer to OpenTelemetry bridge support for metrics", Type.EXPERIMENTAL, OPENTELEMETRY), diff --git a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCAuthorizationCheckProvider.java b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCAuthorizationCheckProvider.java index 692a0f9ffd9..814f5b3d759 100644 --- a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCAuthorizationCheckProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCAuthorizationCheckProvider.java @@ -16,9 +16,9 @@ import org.keycloak.protocol.oid4vc.model.CredentialScopeRepresentation; import org.keycloak.protocol.oid4vc.model.CredentialsOffer; import org.keycloak.protocol.oid4vc.model.IssuerState; import org.keycloak.protocol.oid4vc.utils.CredentialScopeUtils; +import org.keycloak.protocol.oidc.endpoints.AuthorizationCheckException; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointCheckProvider; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker; -import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker.AuthorizationCheckException; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; import static org.keycloak.OAuth2Constants.ISSUER_STATE; diff --git a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/signing/SdJwtCredentialSigner.java b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/signing/SdJwtCredentialSigner.java index fc820700f0c..14bb307a361 100644 --- a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/signing/SdJwtCredentialSigner.java +++ b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/signing/SdJwtCredentialSigner.java @@ -22,7 +22,7 @@ import java.security.cert.X509Certificate; import java.util.Base64; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; +import javax.security.auth.x500.X500Principal; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.models.KeycloakSession; @@ -80,10 +80,10 @@ public class SdJwtCredentialSigner extends AbstractCredentialSigner { */ private void addX5cHeader(SdJwtCredentialBody sdJwtCredentialBody, SignatureSignerContext signer) { List certificateChain = signer.getCertificateChain(); - if (certificateChain != null && !certificateChain.isEmpty()) { List x5cList = certificateChain.stream() .filter(Objects::nonNull) + .filter(cert -> !isTrustAnchor(cert)) .map(cert -> { try { return Base64.getEncoder().encodeToString(cert.getEncoded()); @@ -91,7 +91,7 @@ public class SdJwtCredentialSigner extends AbstractCredentialSigner { throw new RuntimeException(e); } }) - .collect(Collectors.toList()); + .toList(); if (!x5cList.isEmpty()) { sdJwtCredentialBody.getIssuerSignedJWT().getJwsHeader().setX5c(x5cList); @@ -102,4 +102,17 @@ public class SdJwtCredentialSigner extends AbstractCredentialSigner { LOGGER.debugf("No certificate or certificate chain available for x5c header in SD-JWT credential."); } } + + private boolean isTrustAnchor(X509Certificate cert) { + boolean isTrustAnchor = false; + try { + int basicConstraints = cert.getBasicConstraints(); + X500Principal issuerPrincipal = cert.getIssuerX500Principal(); + X500Principal subjectPrincipal = cert.getSubjectX500Principal(); + isTrustAnchor = subjectPrincipal.equals(issuerPrincipal) && basicConstraints >= 0; + } catch (Exception e) { + // ignore + } + return isTrustAnchor; + } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index 64f9e9fe8d3..82c13734ebc 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -46,6 +46,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.ClientData; import org.keycloak.protocol.LoginProtocol; +import org.keycloak.protocol.oidc.endpoints.AuthorizationCheckException; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; import org.keycloak.protocol.oidc.utils.LogoutUtil; @@ -57,6 +58,7 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType; import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.adapters.action.PushNotBeforeAction; import org.keycloak.representations.idm.OAuth2ErrorRepresentation; +import org.keycloak.services.ErrorPageException; import org.keycloak.services.ServicesLogger; import org.keycloak.services.Urls; import org.keycloak.services.clientpolicy.ClientPolicyException; @@ -436,8 +438,8 @@ public class OIDCLoginProtocol implements LoginProtocol { try { checker.checkResponseType(); checker.checkRedirectUri(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { - checker.throwAsErrorPageException(null, ex); + } catch (AuthorizationCheckException ex) { + throw new ErrorPageException(session, ex.getError(), ex.getStatus(), ex.getErrorMessage(), ex.getParameters()); } setupResponseTypeAndMode(clientData.getResponseType(), clientData.getResponseMode()); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationCheckException.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationCheckException.java new file mode 100644 index 00000000000..3fb7535213d --- /dev/null +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationCheckException.java @@ -0,0 +1,49 @@ +package org.keycloak.protocol.oidc.endpoints; + +import java.text.MessageFormat; + +import jakarta.ws.rs.core.Response; + +// Exception propagated to the caller, which will allow caller to send proper error response based on the context (Browser OIDC Authorization Endpoint, PAR etc) +public class AuthorizationCheckException extends Exception { + + private final Response.Status status; + private final String error; + private final String errorMessage; + private final Object[] parameters; + + public AuthorizationCheckException(Response.Status status, String error, String errorDescription) { + this(error, status, errorDescription); + } + + public AuthorizationCheckException(String error, Response.Status status, String errorMessage, Object... params) { + this.status = status; + this.error = error; + this.errorMessage = errorMessage; + this.parameters = params; + } + + public Response.Status getStatus() { + return status; + } + + public String getError() { + return error; + } + + public String getErrorDescription() { + String errorDescription = errorMessage; + if (parameters.length > 0) { + errorDescription = MessageFormat.format(errorMessage, parameters); + } + return errorDescription; + } + + public String getErrorMessage() { + return errorMessage; + } + + public Object[] getParameters() { + return parameters; + } +} diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index 5701728d0e6..b9411566e98 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -17,8 +17,12 @@ package org.keycloak.protocol.oidc.endpoints; +import java.io.IOException; +import java.text.MessageFormat; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Properties; import java.util.function.BiConsumer; import jakarta.ws.rs.Consumes; @@ -48,12 +52,14 @@ import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor; +import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor.EndpointType; import org.keycloak.protocol.oidc.endpoints.request.RequestUriType; import org.keycloak.protocol.oidc.grants.device.endpoints.DeviceEndpoint; import org.keycloak.protocol.oidc.utils.AcrUtils; import org.keycloak.protocol.oidc.utils.OIDCRedirectUriBuilder; import org.keycloak.protocol.oidc.utils.OIDCResponseMode; import org.keycloak.protocol.oidc.utils.OIDCResponseType; +import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.services.ErrorPageException; import org.keycloak.services.Urls; import org.keycloak.services.clientpolicy.ClientPolicyException; @@ -65,6 +71,7 @@ import org.keycloak.services.resources.LoginActionsService; import org.keycloak.services.util.CacheControlUtil; import org.keycloak.services.util.LocaleUtil; import org.keycloak.sessions.AuthenticationSessionModel; +import org.keycloak.theme.Theme; import org.keycloak.util.TokenUtil; import org.jboss.logging.Logger; @@ -80,6 +87,9 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { public static final String CODE_AUTH_TYPE = "code"; + // Prefer error delivery on `redirect_uri` - rather than as HTML error page + public static final String AUTHORIZATION_PREFER_ERROR_ON_REDIRECT = "authorization.preferErrorOnRedirect"; + /** * Prefix used to store additional HTTP GET params from original client request into {@link AuthenticationSessionModel} note to be available later in Authenticators, RequiredActions etc. Prefix is used to * prevent collisions with internally used notes. @@ -93,6 +103,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { } private ClientModel client; + private AuthorizationEndpointChecker checker; private AuthenticationSessionModel authenticationSession; private Action action; @@ -100,7 +111,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { private OIDCResponseMode parsedResponseMode; private AuthorizationEndpointRequest request; - private String redirectUri; + private String parsedRedirectUri; public AuthorizationEndpoint(KeycloakSession session, EventBuilder event) { super(session, event); @@ -132,52 +143,58 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { } private Response process(final MultivaluedMap params) { - String clientId = AuthorizationEndpointRequestParserProcessor.getClientId(event, session, params); - - checkSsl(); - checkRealm(); try { - session.clientPolicy().triggerOnEvent(new PreAuthorizationRequestContext(clientId, params)); - } catch (ClientPolicyException cpe) { - event.detail(Details.REASON, Details.CLIENT_POLICY_ERROR); - event.detail(Details.CLIENT_POLICY_ERROR, cpe.getError()); - event.detail(Details.CLIENT_POLICY_ERROR_DETAIL, cpe.getErrorDetail()); - event.error(cpe.getError()); - throw new ErrorPageException(session, authenticationSession, cpe.getErrorStatus(), cpe.getErrorDetail()); + String clientId = AuthorizationEndpointRequestParserProcessor.getClientId(event, session, params); + + checkSsl(); + checkRealm(); + + try { + session.clientPolicy().triggerOnEvent(new PreAuthorizationRequestContext(clientId, params)); + } catch (ClientPolicyException cpe) { + event.detail(Details.REASON, Details.CLIENT_POLICY_ERROR); + event.detail(Details.CLIENT_POLICY_ERROR, cpe.getError()); + event.detail(Details.CLIENT_POLICY_ERROR_DETAIL, cpe.getErrorDetail()); + event.error(cpe.getError()); + throw new ErrorPageException(session, cpe.getError(), cpe.getErrorStatus(), cpe.getErrorDetail()); + } + checkClient(clientId); + + request = AuthorizationEndpointRequestParserProcessor.parseRequest(event, session, client, params, EndpointType.OIDC_AUTH_ENDPOINT); + + } catch (ErrorPageException ex) { + buildAuthorizationEndpointChecker(params); + return handleErrorPageException(ex); } - checkClient(clientId); - request = AuthorizationEndpointRequestParserProcessor.parseRequest(event, session, client, params, AuthorizationEndpointRequestParserProcessor.EndpointType.OIDC_AUTH_ENDPOINT); - - AuthorizationEndpointChecker checker = new AuthorizationEndpointChecker() - .event(event) - .client(client) - .realm(realm) - .request(request) - .session(session) - .params(params); + buildAuthorizationEndpointChecker(params); try { checker.checkRedirectUri(); - this.redirectUri = checker.getRedirectUri(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { - checker.throwAsErrorPageException(authenticationSession, ex); + parsedRedirectUri = checker.getRedirectUri(); + } catch (AuthorizationCheckException ex) { + ErrorPageException epex = new ErrorPageException(session, ex.getError(), ex.getStatus(), ex.getErrorMessage(), ex.getParameters()); + return handleErrorPageException(epex); } try { checker.checkResponseType(); - this.parsedResponseType = checker.getParsedResponseType(); - this.parsedResponseMode = checker.getParsedResponseMode(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { - OIDCResponseMode responseMode; + parsedResponseType = checker.getParsedResponseType(); + parsedResponseMode = checker.getParsedResponseMode(); + } catch (AuthorizationCheckException ex) { if (checker.isInvalidResponseType(ex)) { - responseMode = OIDCResponseMode.parseWhenInvalidResponseType(request.getResponseMode()); - } else { - responseMode = checker.getParsedResponseMode() != null ? checker.getParsedResponseMode() : OIDCResponseMode.QUERY; + parsedResponseMode = OIDCResponseMode.parseWhenInvalidResponseType(request.getResponseMode()); } - return redirectErrorToClient(responseMode, ex.getError(), ex.getErrorDescription()); + if (parsedResponseMode == null) { + parsedResponseMode = checker.getParsedResponseMode(); + } + if (parsedResponseMode == null) { + parsedResponseMode = OIDCResponseMode.QUERY; + } + return handleRedirectErrorToClient(ex.getError(), ex.getErrorDescription()); } + if (action == null) { action = AuthorizationEndpoint.Action.CODE; } @@ -192,8 +209,8 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { checker.checkOIDCParams(); checker.checkPKCEParams(); checker.checkProviderAddOns(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { - return redirectErrorToClient(parsedResponseMode, ex.getError(), ex.getErrorDescription()); + } catch (AuthorizationCheckException ex) { + return handleRedirectErrorToClient(ex.getError(), ex.getErrorDescription()); } // If DPoP Proof existed with PAR request, its public key needs to be matched with the one with Token Request afterward @@ -206,14 +223,14 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { authenticationSession = createAuthenticationSession(client, request.getState()); try { - session.clientPolicy().triggerOnEvent(new AuthorizationRequestContext(parsedResponseType, request, redirectUri, params, authenticationSession)); + session.clientPolicy().triggerOnEvent(new AuthorizationRequestContext(parsedResponseType, request, parsedRedirectUri, params, authenticationSession)); } catch (ClientPolicyException cpe) { event.detail(Details.REASON, Details.CLIENT_POLICY_ERROR); event.detail(Details.CLIENT_POLICY_ERROR, cpe.getError()); event.detail(Details.CLIENT_POLICY_ERROR_DETAIL, cpe.getErrorDetail()); event.error(cpe.getError()); new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authenticationSession, false); - return redirectErrorToClient(parsedResponseMode, cpe.getError(), cpe.getErrorDetail()); + return handleRedirectErrorToClient(cpe.getError(), cpe.getErrorDetail()); } updateAuthenticationSession(); @@ -242,6 +259,19 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { throw new RuntimeException("Unknown action " + action); } + private AuthorizationEndpointChecker buildAuthorizationEndpointChecker(MultivaluedMap params) { + if (checker == null) { + checker = new AuthorizationEndpointChecker() + .event(event) + .client(client) + .realm(realm) + .request(request) + .session(session) + .params(params); + } + return checker; + } + public AuthorizationEndpoint register(String tokenString) { event.event(EventType.REGISTER); action = Action.REGISTER; @@ -272,7 +302,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { return this; } - private void checkClient(String clientId) { + private ClientModel checkClient(String clientId) { if (clientId == null) { event.detail(Details.REASON, "Missing parameter: " + OIDCLoginProtocol.CLIENT_ID_PARAM); event.error(Errors.INVALID_REQUEST); @@ -310,24 +340,84 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { } session.getContext().setClient(client); + return client; } - private Response redirectErrorToClient(OIDCResponseMode responseMode, String error, String errorDescription) { + private Response handleErrorPageException(ErrorPageException epex) { + + // Do a second pass trying to obtain the required parameters for a valid redirect + // + try { + MultivaluedMap params = checker.getRequestParams(); + AuthorizationEndpointRequest request = checker.getAuthorizationEndpointRequest(); + if (client == null) { + String clientId = request != null ? request.getClientId() : params.getFirst(OIDCLoginProtocol.CLIENT_ID_PARAM); + client = checkClient(clientId); + checker.client(client); + } + if (parsedRedirectUri == null) { + checker.checkRedirectUri(); + parsedRedirectUri = checker.getRedirectUri(); + } + if (parsedResponseType == null || parsedResponseMode == null) { + checker.checkResponseType(); + parsedResponseType = checker.getParsedResponseType(); + parsedResponseMode = checker.getParsedResponseMode(); + } + } catch (Exception aux) { + // ignore + } + + // Prefer authorization error on redirect_uri + // https://github.com/keycloak/keycloak/issues/48542 + boolean preferErrorOnRedirect = false; + if (client != null) { + preferErrorOnRedirect = Boolean.parseBoolean(client.getAttribute(AUTHORIZATION_PREFER_ERROR_ON_REDIRECT)); + } + if (!preferErrorOnRedirect) + throw epex; + + // Get the localized error message (english) + // + String errorMessage = epex.getErrorMessage(); + try { + Theme theme = session.theme().getTheme(Theme.Type.LOGIN); + Properties localizedMessages = theme.getEnhancedMessages(realm, Locale.ENGLISH); + if (localizedMessages.containsKey(errorMessage)) { + errorMessage = localizedMessages.getProperty(errorMessage); + Object[] params = epex.getParameters(); + if (params.length > 0) { + errorMessage = MessageFormat.format(errorMessage, params); + } + } + } catch (IOException ioe) { + // ignore + } + + if (client != null && parsedRedirectUri != null && parsedResponseType != null && parsedResponseMode != null) { + return handleRedirectErrorToClient(epex.getError(), errorMessage); + } + + Response.Status status = epex.getStatus(); + var errRep = new OAuth2ErrorRepresentation(epex.getError(), errorMessage); + return Response.status(status).entity(errRep).type(MediaType.APPLICATION_JSON_TYPE).build(); + } + + private Response handleRedirectErrorToClient(String error, String errorDescription) { CacheControlUtil.noBackButtonCacheControlHeader(session); - OIDCRedirectUriBuilder errorResponseBuilder = OIDCRedirectUriBuilder.fromUri(redirectUri, responseMode, session, null) + OIDCRedirectUriBuilder errorResponseBuilder = OIDCRedirectUriBuilder.fromUri(parsedRedirectUri, parsedResponseMode, session, null) .addParam(OAuth2Constants.ERROR, error); if (errorDescription != null) { errorResponseBuilder.addParam(OAuth2Constants.ERROR_DESCRIPTION, errorDescription); } - if (request.getState() != null) { + if (request != null && request.getState() != null) { errorResponseBuilder.addParam(OAuth2Constants.STATE, request.getState()); } - OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientModel(client); - if (!clientConfig.isExcludeIssuerFromAuthResponse()) { + if (client != null && !OIDCAdvancedConfigWrapper.fromClientModel(client).isExcludeIssuerFromAuthResponse()) { errorResponseBuilder.addParam(OAuth2Constants.ISSUER, Urls.realmIssuer(session.getContext().getUri().getBaseUri(), realm.getName())); } @@ -336,7 +426,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { private void updateAuthenticationSession() { authenticationSession.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); - authenticationSession.setRedirectUri(redirectUri); + authenticationSession.setRedirectUri(parsedRedirectUri); authenticationSession.setAction(AuthenticationSessionModel.Action.AUTHENTICATE.name()); authenticationSession.setClientNote(OIDCLoginProtocol.RESPONSE_TYPE_PARAM, request.getResponseType()); authenticationSession.setClientNote(OIDCLoginProtocol.REDIRECT_URI_PARAM, request.getRedirectUri()); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointCheckProvider.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointCheckProvider.java index ef7259784f5..cdfa567afe2 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointCheckProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointCheckProvider.java @@ -1,6 +1,5 @@ package org.keycloak.protocol.oidc.endpoints; -import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker.AuthorizationCheckException; import org.keycloak.provider.Provider; public interface AuthorizationEndpointCheckProvider extends Provider { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointChecker.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointChecker.java index 7c5ef2e3ec0..070c0dda8a6 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointChecker.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpointChecker.java @@ -59,12 +59,10 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType; import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.representations.dpop.DPoP; import org.keycloak.services.CorsErrorResponseException; -import org.keycloak.services.ErrorPageException; import org.keycloak.services.ServicesLogger; import org.keycloak.services.cors.Cors; import org.keycloak.services.messages.Messages; import org.keycloak.services.util.DPoPUtil; -import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.util.TokenUtil; import org.keycloak.utils.StringUtil; @@ -160,25 +158,32 @@ public class AuthorizationEndpointChecker { return parsedResponseMode; } + public MultivaluedMap getRequestParams() { + return params; + } + public void checkRedirectUri() throws AuthorizationCheckException { - String redirectUriParam = request.getRedirectUri(); - boolean isOIDCRequest = TokenUtil.isOIDCRequest(request.getScope()); + + String redirectUriParam = request != null ? request.getRedirectUri() : params.getFirst(OIDCLoginProtocol.REDIRECT_URI_PARAM); + String scope = request != null ? request.getScope() : params.getFirst(OIDCLoginProtocol.SCOPE_PARAM); + + // The redirect_uri parameter is required for OIDC, but optional for OAuth2 + boolean isOIDCRequest = TokenUtil.isOIDCRequest(scope); event.detail(Details.REDIRECT_URI, redirectUriParam); - // redirect_uri parameter is required per OpenID Connect, but optional per OAuth2 this.redirectUri = RedirectUtils.verifyRedirectUri(session, redirectUriParam, client, isOIDCRequest); if (redirectUri == null) { event.error(Errors.INVALID_REDIRECT_URI); - throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM); + throw new AuthorizationCheckException(OAuthErrorException.INVALID_REDIRECT_URI, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM); } } - public void checkResponseType() throws AuthorizationCheckException { - String responseType = request.getResponseType(); + String responseTypeParam = request != null ? request.getResponseType() : params.getFirst(OIDCLoginProtocol.RESPONSE_TYPE_PARAM); + String responseModeParam = request != null ? request.getResponseMode() : params.getFirst(OIDCLoginProtocol.RESPONSE_MODE_PARAM); - if (responseType == null) { + if (responseTypeParam == null) { ServicesLogger.LOGGER.missingParameter(OAuth2Constants.RESPONSE_TYPE); String errorMessage = "Missing parameter: response_type"; event.detail(Details.REASON, errorMessage); @@ -186,19 +191,21 @@ public class AuthorizationEndpointChecker { throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage); } - event.detail(Details.RESPONSE_TYPE, responseType); + event.detail(Details.RESPONSE_TYPE, responseTypeParam); try { - this.parsedResponseType = OIDCResponseType.parse(responseType); + parsedResponseType = OIDCResponseType.parse(responseTypeParam); } catch (IllegalArgumentException iae) { - event.detail(Details.REASON, iae.getMessage()); + ServicesLogger.LOGGER.invalidParameter(OIDCLoginProtocol.RESPONSE_TYPE_PARAM); + String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.RESPONSE_TYPE_PARAM; + event.detail(Details.REASON, errorMessage); event.error(Errors.INVALID_REQUEST); - throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null); + throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, errorMessage); } - OIDCResponseMode parsedResponseMode = null; + OIDCResponseMode responseMode; try { - parsedResponseMode = OIDCResponseMode.parse(request.getResponseMode(), parsedResponseType); + responseMode = OIDCResponseMode.parse(responseModeParam, parsedResponseType); } catch (IllegalArgumentException iae) { ServicesLogger.LOGGER.invalidParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM); String errorMessage = "Invalid parameter: " + OIDCLoginProtocol.RESPONSE_MODE_PARAM; @@ -207,10 +214,10 @@ public class AuthorizationEndpointChecker { throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage); } - event.detail(Details.RESPONSE_MODE, parsedResponseMode.toString().toLowerCase()); + event.detail(Details.RESPONSE_MODE, responseMode.toString().toLowerCase()); // Disallowed by OIDC specs - if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) { + if (parsedResponseType.isImplicitOrHybridFlow() && responseMode == OIDCResponseMode.QUERY) { ServicesLogger.LOGGER.responseModeQueryNotAllowed(); String errorMessage = "Response_mode 'query' not allowed for implicit or hybrid flow"; event.detail(Details.REASON, errorMessage); @@ -218,9 +225,9 @@ public class AuthorizationEndpointChecker { throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, errorMessage); } - this.parsedResponseMode = parsedResponseMode; + parsedResponseMode = responseMode; - if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY_JWT && + if (parsedResponseType.isImplicitOrHybridFlow() && responseMode == OIDCResponseMode.QUERY_JWT && (!StringUtil.isNotBlank(client.getAttribute(OIDCConfigAttributes.AUTHORIZATION_ENCRYPTED_RESPONSE_ALG)) || !StringUtil.isNotBlank(client.getAttribute(OIDCConfigAttributes.AUTHORIZATION_ENCRYPTED_RESPONSE_ENC)))) { ServicesLogger.LOGGER.responseModeQueryJwtNotAllowed(); @@ -257,14 +264,14 @@ public class AuthorizationEndpointChecker { } } - public boolean isInvalidResponseType(AuthorizationEndpointChecker.AuthorizationCheckException ex) { + public boolean isInvalidResponseType(AuthorizationCheckException ex) { return "Missing parameter: response_type".equals(ex.getErrorDescription()) || OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE.equals(ex.getError()); } public void checkInvalidRequestMessage() throws AuthorizationCheckException { if (request.getInvalidRequestMessage() != null) { event.error(Errors.INVALID_REQUEST); - throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, Errors.INVALID_REQUEST, request.getInvalidRequestMessage()); + throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, request.getInvalidRequestMessage()); } } @@ -281,7 +288,7 @@ public class AuthorizationEndpointChecker { new AuthorizationDetailsProcessorManager(session).validateAuthorizationDetail(authDetailsParam); } catch (Exception e) { event.error(Errors.INVALID_REQUEST); - throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, Errors.INVALID_REQUEST, e.getMessage()); + throw new AuthorizationCheckException(Response.Status.BAD_REQUEST, OAuthErrorException.INVALID_REQUEST, e.getMessage()); } } } @@ -518,35 +525,8 @@ public class AuthorizationEndpointChecker { } } - public void throwAsErrorPageException(AuthenticationSessionModel authenticationSession, AuthorizationCheckException ex) { - throw new ErrorPageException(session, authenticationSession, ex.status, ex.error, ex.errorDescription); - } - public void throwAsCorsErrorResponseException(Cors cors, AuthorizationCheckException ex) { - event.detail("detail", ex.errorDescription).error(ex.error); - throw new CorsErrorResponseException(cors, ex.error, ex.errorDescription, ex.status); - } - - - // Exception propagated to the caller, which will allow caller to send proper error response based on the context (Browser OIDC Authorization Endpoint, PAR etc) - public static class AuthorizationCheckException extends Exception { - - private final Response.Status status; - private final String error; - private final String errorDescription; - - public AuthorizationCheckException(Response.Status status, String error, String errorDescription) { - this.status = status; - this.error = error; - this.errorDescription = errorDescription; - } - - public String getError() { - return error; - } - - public String getErrorDescription() { - return errorDescription; - } + event.detail("detail", ex.getErrorDescription()).error(ex.getError()); + throw new CorsErrorResponseException(cors, ex.getError(), ex.getErrorDescription(), ex.getStatus()); } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/grants/device/endpoints/DeviceEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/grants/device/endpoints/DeviceEndpoint.java index c4905d543e9..653bb676bb0 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/grants/device/endpoints/DeviceEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/grants/device/endpoints/DeviceEndpoint.java @@ -50,6 +50,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.SingleUseObjectProvider; import org.keycloak.protocol.AuthorizationEndpointBase; import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.endpoints.AuthorizationCheckException; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor; @@ -145,7 +146,7 @@ public class DeviceEndpoint extends AuthorizationEndpointBase implements RealmRe try { checker.checkPKCEParams(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { + } catch (AuthorizationCheckException ex) { throw new ErrorResponseException(ex.getError(), ex.getErrorDescription(), Response.Status.BAD_REQUEST); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java index de56de9341a..5575d0b1352 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java @@ -42,6 +42,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.SingleUseObjectProvider; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.OIDCLoginProtocolService; +import org.keycloak.protocol.oidc.endpoints.AuthorizationCheckException; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpointChecker; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; import org.keycloak.protocol.oidc.par.ParResponse; @@ -123,13 +124,13 @@ public class ParEndpoint extends AbstractParEndpoint { try { checker.checkRedirectUri(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { + } catch (AuthorizationCheckException ex) { throw throwErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Invalid parameter: redirect_uri", Response.Status.BAD_REQUEST); } try { checker.checkResponseType(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { + } catch (AuthorizationCheckException ex) { if (ex.getError().equals(OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE)) { throw throwErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Unsupported response type", Response.Status.BAD_REQUEST); } else { @@ -139,7 +140,7 @@ public class ParEndpoint extends AbstractParEndpoint { try { checker.checkValidScope(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { + } catch (AuthorizationCheckException ex) { // PAR throws this as "invalid_request" error throw throwErrorResponseException(OAuthErrorException.INVALID_REQUEST, ex.getErrorDescription(), Response.Status.BAD_REQUEST); } @@ -150,7 +151,7 @@ public class ParEndpoint extends AbstractParEndpoint { checker.checkOIDCParams(); checker.checkPKCEParams(); checker.checkParDPoPParams(); - } catch (AuthorizationEndpointChecker.AuthorizationCheckException ex) { + } catch (AuthorizationCheckException ex) { checker.throwAsCorsErrorResponseException(cors, ex); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java index f024d5f541b..8fc7ed92940 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java @@ -19,8 +19,10 @@ package org.keycloak.protocol.oidc.par.endpoints.request; import java.util.Map; +import java.util.Optional; import java.util.Set; +import org.keycloak.common.Profile; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -53,29 +55,19 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser { super(session); this.session = session; this.client = client; - SingleUseObjectProvider singleUseStore = session.singleUseObjects(); - String key; - try { - key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH); - } catch (RuntimeException re) { - logger.warnf(re,"Unable to parse request_uri: %s", requestUri); - throw new RuntimeException("Unable to parse request_uri"); - } - Map retrievedRequest = singleUseStore.remove(CACHE_KEY_PREFIX + key); - if (retrievedRequest == null) { - throw new RuntimeException("PAR not found. not issued or used multiple times."); - } - RealmModel realm = session.getContext().getRealm(); + Map parRequestParams = Optional.ofNullable(getRequestObject(session, requestUri)) + .orElseThrow(() -> new RuntimeException("PAR not found, not issued or used multiple times.")); + long created = Long.parseLong(parRequestParams.get(PAR_CREATED_TIME)); int expiresIn = realm.getParPolicy().getRequestUriLifespan(); - long created = Long.parseLong(retrievedRequest.get(PAR_CREATED_TIME)); - if (System.currentTimeMillis() - created < (expiresIn * 1000)) { - requestParams = retrievedRequest; + if (System.currentTimeMillis() - created < expiresIn * 1000L) { + removeRequestObject(session, requestUri); + requestParams = parRequestParams; } else { throw new RuntimeException("PAR expired."); } // If DPoP Proof existed with PAR request, its public key needs to be matched with the one with Token Request afterward - String dpopJkt = retrievedRequest.get(PAR_DPOP_PROOF_JKT); + String dpopJkt = parRequestParams.get(PAR_DPOP_PROOF_JKT); if (dpopJkt != null) { session.setAttribute(PAR_DPOP_PROOF_JKT, dpopJkt); } @@ -87,7 +79,7 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser { if (requestParam != null) { // parses the request object if PAR was registered using JAR - // parameters from requets object have precedence over those sent directly in the request + // parameters from the request object have precedence over those sent directly in the request new ParEndpointRequestObjectParser(session, requestParam, client).parseRequest(request); } else { super.parseRequest(request); @@ -114,4 +106,39 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser { return requestParams.keySet(); } + protected T replaceIfNotNull(T previousVal, T newVal) { + // FAPI says only parameters inside the request object should be used + // https://github.com/keycloak/keycloak/issues/48047 + if (Profile.isFeatureEnabled(Profile.Feature.OID4VC_HAIP)) { + return newVal; + } else { + return super.replaceIfNotNull(previousVal, newVal); + } + } + + public static Map getRequestObject(KeycloakSession session, String requestUri) { + String key = getRequestObjectKey(requestUri); + SingleUseObjectProvider singleUseStore = session.singleUseObjects(); + Map retrievedRequest = singleUseStore.get(CACHE_KEY_PREFIX + key); + return retrievedRequest; + } + + /** + * Authorization servers that enforce one-time use of request_uri values do so at the point of authorization, + * not at the point of visiting the authorization endpoint + * OpenID CT: fapi2-security-profile-final-par-ensure-reused-request-uri-prior-to-auth-completion-succeeds + */ + public static void removeRequestObject(KeycloakSession session, String requestUri) { + String key = getRequestObjectKey(requestUri); + session.singleUseObjects().remove(CACHE_KEY_PREFIX + key); + } + + private static String getRequestObjectKey(String requestUri) { + try { + return requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH); + } catch (RuntimeException re) { + logger.warnf(re,"Unable to parse request_uri: %s", requestUri); + throw new RuntimeException("Unable to parse request_uri"); + } + } } diff --git a/services/src/main/java/org/keycloak/services/ErrorPageException.java b/services/src/main/java/org/keycloak/services/ErrorPageException.java index 14b7a4d50de..329df3106c6 100644 --- a/services/src/main/java/org/keycloak/services/ErrorPageException.java +++ b/services/src/main/java/org/keycloak/services/ErrorPageException.java @@ -23,20 +23,53 @@ import jakarta.ws.rs.core.Response; import org.keycloak.models.KeycloakSession; import org.keycloak.sessions.AuthenticationSessionModel; +import static org.keycloak.OAuthErrorException.INVALID_REQUEST; + /** * @author Stian Thorgersen */ public class ErrorPageException extends WebApplicationException { + private String error; + private String errorMessage; + private Object[] parameters; + public ErrorPageException(KeycloakSession session, Response.Status status, String errorMessage, Object... parameters) { - super(errorMessage, ErrorPage.error(session, null, status, errorMessage, parameters)); + this(session, INVALID_REQUEST, status, errorMessage, parameters); + } + + public ErrorPageException(KeycloakSession session, String error, Response.Status status, String errorMessage, Object... parameters) { + this(session, null, error, status, errorMessage, parameters); } public ErrorPageException(KeycloakSession session, AuthenticationSessionModel authSession, Response.Status status, String errorMessage, Object... parameters) { + this(session, authSession, INVALID_REQUEST, status, errorMessage, parameters); + } + + public ErrorPageException(KeycloakSession session, AuthenticationSessionModel authSession, String error, Response.Status status, String errorMessage, Object... parameters) { super(errorMessage, ErrorPage.error(session, authSession, status, errorMessage, parameters)); + this.error = error; + this.errorMessage = errorMessage; + this.parameters = parameters; } public ErrorPageException(Response response) { super((Throwable) null, response); } + + public Response.Status getStatus() { + return Response.Status.fromStatusCode(getResponse().getStatus()); + } + + public String getError() { + return error; + } + + public String getErrorMessage() { + return errorMessage; + } + + public Object[] getParameters() { + return parameters; + } } diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java index 05d7109cab6..2853720ed88 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java @@ -18,6 +18,7 @@ package org.keycloak.services.clientpolicy.executor; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -28,7 +29,6 @@ import org.keycloak.OAuthErrorException; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.models.SingleUseObjectProvider; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpoint; import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest; @@ -36,7 +36,7 @@ import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest import org.keycloak.protocol.oidc.endpoints.request.AuthzEndpointRequestObjectParser; import org.keycloak.protocol.oidc.endpoints.request.AuthzEndpointRequestParser; import org.keycloak.protocol.oidc.endpoints.request.RequestUriType; -import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint; +import org.keycloak.protocol.oidc.par.endpoints.request.AuthzEndpointParParser; import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; import org.keycloak.services.clientpolicy.ClientPolicyContext; import org.keycloak.services.clientpolicy.ClientPolicyException; @@ -76,18 +76,13 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider requestParametersFromQuery = preAuthorizationRequestContext.getRequestParameters(); String requestUri = requestParametersFromQuery.getFirst(OIDCLoginProtocol.REQUEST_URI_PARAM); - if (requestUri == null) { - throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "request_uri not included."); - } - if (requestUri != null && AuthorizationEndpointRequestParserProcessor.getRequestUriType(requestUri) != RequestUriType.PAR) { + if (requestUri == null || AuthorizationEndpointRequestParserProcessor.getRequestUriType(requestUri) != RequestUriType.PAR) { throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request_uri not included."); } - String key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH); - SingleUseObjectProvider singleUseStore = session.singleUseObjects(); - Map requestParametersFromPAR = singleUseStore.get(ParEndpoint.CACHE_KEY_PREFIX + key); + Map requestParametersFromPAR = AuthzEndpointParParser.getRequestObject(session, requestUri); if (requestParametersFromPAR == null) { - throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR not found. not issued or used multiple times."); + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST_URI, "PAR not found, not issued or used multiple times."); } Set requestParametersNameFromPAR = new HashSet<>(); @@ -98,10 +93,16 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider queryKeys = requestParametersFromQuery.keySet().stream() + .filter(it -> !it.equals(OIDCLoginProtocol.REQUEST_URI_PARAM)) + .toList(); + + // FAPI says only parameters inside the request object should be used + // + for (String queryParam : queryKeys) { + if (!requestParametersNameFromPAR.contains(queryParam)) { + AuthzEndpointParParser.removeRequestObject(session, requestUri); + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST_OBJECT, "PAR request did not include query parameter: " + queryParam); } } } diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java index 6899d39aec1..d61da772751 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java @@ -48,7 +48,7 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider private static final Logger logger = Logger.getLogger(SecureRequestObjectExecutor.class); public static final Integer DEFAULT_AVAILABLE_PERIOD = Integer.valueOf(3600); // (sec) from FAPI 1.0 Advanced requirement - public static final Integer DEAULT_ALLOWED_CLOCK_SKEW = Integer.valueOf(15); // (sec) from FAPI 2.0 requirement + public static final Integer DEFAULT_ALLOWED_CLOCK_SKEW = Integer.valueOf(15); // (sec) from FAPI 2.0 requirement private final KeycloakSession session; private Configuration configuration; @@ -64,7 +64,7 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider configuration.setVerifyNbf(Boolean.TRUE); configuration.setAvailablePeriod(DEFAULT_AVAILABLE_PERIOD); configuration.setEncryptionRequired(Boolean.FALSE); - configuration.setAllowedClockSkew(DEAULT_ALLOWED_CLOCK_SKEW); + configuration.setAllowedClockSkew(DEFAULT_ALLOWED_CLOCK_SKEW); } else { configuration = config; if (config.isVerifyNbf() == null) { @@ -77,7 +77,7 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider configuration.setEncryptionRequired(Boolean.FALSE); } if (config.getAllowedClockSkew() == null) { - configuration.setAllowedClockSkew(DEAULT_ALLOWED_CLOCK_SKEW); + configuration.setAllowedClockSkew(DEFAULT_ALLOWED_CLOCK_SKEW); } } } @@ -160,7 +160,7 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider String requestParam = params.getFirst(OIDCLoginProtocol.REQUEST_PARAM); String requestUriParam = params.getFirst(OIDCLoginProtocol.REQUEST_URI_PARAM); - // check whether whether request object exists + // check whether the request object exists if (requestParam == null && requestUriParam == null) { logger.trace("request object not exist."); throwClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Missing parameter: 'request' or 'request_uri'", @@ -185,7 +185,7 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider // check whether "exp" claim exists if (requestObject.get("exp") == null) { - logger.trace("exp claim not incuded."); + logger.trace("exp claim not included."); throwClientPolicyException(INVALID_REQUEST_OBJECT, "Missing parameter in the 'request' object: exp", context); } @@ -289,4 +289,4 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider throw new ClientPolicyException(error, message); } -} \ No newline at end of file +} diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 8075912bbb2..295c21e6ace 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -475,22 +475,22 @@ public class LoginActionsService { if (clientID == null) { if (redirectUriParam != null) { logger.warn("Unsupported to send 'redirect_uri' parameter without providing 'client_id' parameter."); - throw new ErrorPageException(session, null, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER, OIDCLoginProtocol.CLIENT_ID_PARAM); + throw new ErrorPageException(session, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER, OIDCLoginProtocol.CLIENT_ID_PARAM); } client = SystemClientUtil.getSystemClient(realm); redirectUri = Urls.accountBase(session.getContext().getUri().getBaseUri()).path("/").build(realm.getName()).toString(); } else { client = session.clients().getClientByClientId(realm, clientID); if (client == null) { - throw new ErrorPageException(session, null, Response.Status.BAD_REQUEST, Messages.CLIENT_NOT_FOUND); + throw new ErrorPageException(session, Response.Status.BAD_REQUEST, Messages.CLIENT_NOT_FOUND); } if (!client.isEnabled()) { - throw new ErrorPageException(session, null, Response.Status.BAD_REQUEST, Messages.CLIENT_DISABLED); + throw new ErrorPageException(session, Response.Status.BAD_REQUEST, Messages.CLIENT_DISABLED); } if (redirectUriParam != null) { redirectUri = RedirectUtils.verifyRedirectUri(session, redirectUriParam, client); if (redirectUri == null) { - throw new ErrorPageException(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM); + throw new ErrorPageException(session, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM); } } } diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java index d837ca9d78b..71903b8eddb 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java @@ -638,6 +638,11 @@ public class OID4VCBasicWallet { return this; } + public AuthorizationEndpointRequest state(String state) { + loginForm.state(state); + return this; + } + public boolean openLoginForm() { loginForm.open(); String currUrl = oauth.getDriver().getCurrentUrl(); diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java index f775f9feeec..69023b3e964 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java @@ -26,6 +26,7 @@ import org.keycloak.OID4VCConstants; import org.keycloak.VCFormat; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.ClientPoliciesPoliciesResource; +import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientScopeResource; import org.keycloak.admin.client.resource.ClientScopesResource; import org.keycloak.admin.client.resource.RealmResource; @@ -177,10 +178,10 @@ public abstract class OID4VCIssuerTestBase { protected ManagedClient managedClient; @InjectClient(ref = OID4VCI_ABCA_CLIENT_ID, config = OID4VCAttestationBasedClient.class) - ManagedClient managedAttestationBasedClient; + protected ManagedClient managedAttestationBasedClient; @InjectClient(ref = OID4VCI_PUBLIC_CLIENT_ID, config = OID4VCPublicClient.class) - ManagedClient managedPublicClient; + protected ManagedClient managedPublicClient; @InjectOAuthClient protected OAuthClient oauth; @@ -232,7 +233,6 @@ public abstract class OID4VCIssuerTestBase { testUser.setRealmRoles(List.of(CREDENTIAL_OFFER_CREATE.getName())); realmResource.users().get(testUser.getId()).roles().realmLevel().add(List.of(credentialOfferRole)); } - } @BeforeEach @@ -444,13 +444,32 @@ public abstract class OID4VCIssuerTestBase { realmResource.update(realm); } - protected void setVerifiableCredentialsEnabled(boolean enabled) { + protected void setRealmVerifiableCredentialsEnabled(boolean enabled) { RealmResource realmResource = testRealm.admin(); RealmRepresentation realm = realmResource.toRepresentation(); realm.setVerifiableCredentialsEnabled(enabled); realmResource.update(realm); } + protected void setClientAttribute(ClientRepresentation client, String attrKey, String attrValue) { + setClientAttributes(client, Map.of(attrKey, attrValue)); + } + + protected void setClientAttributes(ClientRepresentation client, Map attrUpdate) { + Map attrs = client.getAttributes(); + boolean updateNeeded = attrUpdate.entrySet().stream() + .anyMatch(e -> !e.getValue().equals(attrs.get(e.getKey()))); + if (updateNeeded) { + ClientResource clientResource = testRealm.admin().clients().get(client.getId()); + client.getAttributes().putAll(attrUpdate); + clientResource.update(client); + } + } + + protected void setCredentialScopeAttribute(ClientScopeRepresentation credScope, String attrKey, String attrValue) { + setCredentialScopeAttributes(credScope, Map.of(attrKey, attrValue)); + } + protected void setCredentialScopeAttributes(ClientScopeRepresentation credScope, Map attrUpdate) { ClientScopeResource clientScopeResource = testRealm.admin().clientScopes().get(credScope.getId()); credScope = clientScopeResource.toRepresentation(); diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerWellKnownProviderTest.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerWellKnownProviderTest.java index 365e73485be..df84bb96caa 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerWellKnownProviderTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerWellKnownProviderTest.java @@ -524,7 +524,7 @@ public class OID4VCIssuerWellKnownProviderTest extends OID4VCIssuerTestBase { @Test public void testWellKnownEndpointDisabledWhenVerifiableCredentialsOff() { - setVerifiableCredentialsEnabled(false); + setRealmVerifiableCredentialsEnabled(false); try { CredentialIssuerMetadataResponse response = oauth.oid4vc() .issuerMetadataRequest() @@ -537,7 +537,7 @@ public class OID4VCIssuerWellKnownProviderTest extends OID4VCIssuerTestBase { assertEquals("OID4VCI functionality is disabled for this realm", error.getMessage()); } finally { - setVerifiableCredentialsEnabled(true); + setRealmVerifiableCredentialsEnabled(true); } } diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/haip/HAIPIssuerConformanceTest.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/haip/HAIPIssuerConformanceTest.java new file mode 100644 index 00000000000..83c558503f5 --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/haip/HAIPIssuerConformanceTest.java @@ -0,0 +1,238 @@ +package org.keycloak.tests.oid4vc.haip; + + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import org.keycloak.TokenVerifier; +import org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator; +import org.keycloak.crypto.KeyWrapper; +import org.keycloak.jose.jwk.JWK; +import org.keycloak.jose.jwk.JWKBuilder; +import org.keycloak.models.AuthenticatorConfigModel; +import org.keycloak.models.RealmModel; +import org.keycloak.protocol.oid4vc.model.CredentialResponse; +import org.keycloak.protocol.oid4vc.model.CredentialScopeRepresentation; +import org.keycloak.protocol.oid4vc.model.Proofs; +import org.keycloak.representations.JsonWebToken; +import org.keycloak.sdjwt.IssuerSignedJWT; +import org.keycloak.sdjwt.vp.SdJwtVP; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.annotations.TestSetup; +import org.keycloak.tests.oid4vc.OID4VCBasicWallet.AuthorizationEndpointRequest; +import org.keycloak.tests.oid4vc.OID4VCIssuerTestBase; +import org.keycloak.tests.oid4vc.OID4VCTestContext; +import org.keycloak.tests.oid4vc.abca.OIDCClientAttester; +import org.keycloak.tests.oid4vc.abca.OIDCMockClientAttester; +import org.keycloak.testsuite.util.oauth.AccessTokenResponse; +import org.keycloak.testsuite.util.oauth.AuthorizationEndpointResponse; +import org.keycloak.testsuite.util.oauth.PkceGenerator; +import org.keycloak.util.JsonSerialization; +import org.keycloak.util.TokenUtil; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.keycloak.OID4VCConstants.CLAIM_NAME_VCT; +import static org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator.OAUTH_CLIENT_ATTESTATION_CONFIG_ATTESTER_JWKS; +import static org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator.OAUTH_CLIENT_ATTESTATION_HEADER; +import static org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator.OAUTH_CLIENT_ATTESTATION_POP_HEADER; +import static org.keycloak.protocol.oidc.endpoints.AuthorizationEndpoint.AUTHORIZATION_PREFER_ERROR_ON_REDIRECT; +import static org.keycloak.tests.oid4vc.OID4VCProofTestUtils.createRsaKeyPair; +import static org.keycloak.tests.oid4vc.OID4VCTestContext.CLIENT_ATTESTER_ATTACHMENT_KEY; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Replicates various tests in oid4vci-1_0-issuer-haip-test-plan + */ +@KeycloakIntegrationTest(config = OID4VCIssuerTestBase.VCTestServerWithABCAEnabled.class) +public class HAIPIssuerConformanceTest extends OID4VCIssuerTestBase { + + private static OIDCClientAttester attester; + private static AttestationBasedClientAuthenticator.ABCAConfig abcaConfig; + + @TestSetup + public void configure() { + + var kw = createRsaKeyPair("openid-abca-attester-key"); + JWK jwk = JWKBuilder.create() + .kid(kw.getKid()) + .algorithm(kw.getAlgorithm()) + .rsa(kw.getPublicKey(), kw.getCertificate()); + + abcaConfig = new AttestationBasedClientAuthenticator.ABCAConfig().setKeys(List.of(jwk)); + attester = new OIDCMockClientAttester(kw); + } + + @BeforeEach + void beforeEach() { + String abcaConfigValue = JsonSerialization.valueAsString(abcaConfig); + runOnServer.run(session -> { + RealmModel realm = session.getContext().getRealm(); + AuthenticatorConfigModel configModel = new AuthenticatorConfigModel(); + configModel.setAlias(AttestationBasedClientAuthenticator.PROVIDER_ID); + configModel.setConfig(Map.of(OAUTH_CLIENT_ATTESTATION_CONFIG_ATTESTER_JWKS, abcaConfigValue)); + realm.addAuthenticatorConfig(configModel); + }); + setClientAttribute(abcaClient, AUTHORIZATION_PREFER_ERROR_ON_REDIRECT, String.valueOf(true)); + setClientPolicyEnabled(VCI_CLIENT_POLICY_HAIP, true); + oauth.client(abcaClient.getClientId(), null); + } + + /** + * oid4vci-1_0-issuer-happy-flow + * + * Validates the standard credential issuance flow using an emulated wallet, as defined by OpenID4VCI. + */ + @Test + public void testIssuerHappyFlow() throws Exception { + + var ctx = new OID4VCTestContext(abcaClient, sdJwtTypeCredentialScope); + ctx.putAttachment(CLIENT_ATTESTER_ATTACHMENT_KEY, attester); + + var pkce = PkceGenerator.s256(); + + // Generate ABCA Headers + // + KeyWrapper rsaKey = wallet.getRSAKeyPair(ctx); + String attestationJwt = wallet.buildClientAttestationJWT(ctx, rsaKey); + String attestationPoPJwt = wallet.buildClientAttestationPoPJWT(ctx, rsaKey); + + // Send PAR Request + // + String requestUri = oauth.pushedAuthorizationRequest() + .header(OAUTH_CLIENT_ATTESTATION_HEADER, attestationJwt) + .header(OAUTH_CLIENT_ATTESTATION_POP_HEADER, attestationPoPJwt) + .scopeParam(ctx.getScope()) + .codeChallenge(pkce) + .send().getRequestUri(); + assertNotNull(requestUri, "No requestUri"); + + // Send Authorization Request + // + String authCode = wallet.authorizationRequest() + .requestUri(requestUri) + .codeChallenge(pkce) + .send(ctx.getHolder(), TEST_PASSWORD) + .getCode(); + assertNotNull(authCode, "No auth code"); + + // Send Token Request + // + KeyWrapper ecKey = wallet.getECKeyPair(ctx); + String tokenEndpoint = oauth.getEndpoints().getToken(); + String dpopProof = wallet.generateSignedDPoPProof(tokenEndpoint, ecKey, null); + + AccessTokenResponse tokenResponse = wallet.accessTokenRequest(ctx, authCode) + .header(OAUTH_CLIENT_ATTESTATION_HEADER, attestationJwt) + .header(OAUTH_CLIENT_ATTESTATION_POP_HEADER, attestationPoPJwt) + .codeVerifier(pkce) + .dpopProof(dpopProof) + .send(); + String tokenType = tokenResponse.getTokenType(); + String accessToken = tokenResponse.getAccessToken(); + assertEquals(TokenUtil.TOKEN_TYPE_DPOP, tokenType); + assertNotNull(accessToken, "No access token"); + + String credentialIdentifier = ctx.getAuthorizedCredentialIdentifier(); + assertNotNull(credentialIdentifier, "No authorized credential identifier"); + + // Send Nonce Request + // + String nonce = wallet.nonceRequest().send().getNonce(); + Proofs jwtProof = wallet.generateJwtProof(ctx, ecKey, nonce); + + // Send Credential Request + // Note, we use the same EC key for DPoP and Holder identity + // + String credentialEndpoint = oauth.getEndpoints().getOid4vcCredential(); + dpopProof = wallet.generateSignedDPoPProof(credentialEndpoint, ecKey, accessToken); + + CredentialResponse credResponse = wallet.credentialRequest(ctx, tokenType, accessToken) + .credentialIdentifier(credentialIdentifier) + .dpopProof(dpopProof) + .proofs(jwtProof) + .send().getCredentialResponse(); + + // Verify Credential Response + // + verifyCredentialResponse(ctx, credResponse); + } + + /** + * fapi2-security-profile-final-state-only-outside-request-object-not-used + * + * Uses a request object that does not contain state, but state is passed in the url parameters to the authorization endpoint + * (hence state should be ignored, as FAPI says only parameters inside the request object should be used). + * The expected result is a successful authentication that returns neither state nor s_hash. + * + * It is also permissible to return an error message: invalid_request, invalid_request_object or access_denied. + */ + @Test + public void testOnlyParametersInsideRequestObjectAreUsed() throws Exception { + + var ctx = new OID4VCTestContext(abcaClient, sdJwtTypeCredentialScope); + ctx.putAttachment(CLIENT_ATTESTER_ATTACHMENT_KEY, attester); + + var pkce = PkceGenerator.s256(); + + // Generate ABCA Headers + // + KeyWrapper rsaKey = wallet.getRSAKeyPair(ctx); + String attestationJwt = wallet.buildClientAttestationJWT(ctx, rsaKey); + String attestationPoPJwt = wallet.buildClientAttestationPoPJWT(ctx, rsaKey); + + // Send PAR Request + // + String requestUri = oauth.pushedAuthorizationRequest() + .header(OAUTH_CLIENT_ATTESTATION_HEADER, attestationJwt) + .header(OAUTH_CLIENT_ATTESTATION_POP_HEADER, attestationPoPJwt) + .scopeParam(ctx.getScope()) + .codeChallenge(pkce) + .send().getRequestUri(); + assertNotNull(requestUri, "No requestUri"); + + // Send Authorization Request + // + AuthorizationEndpointRequest authRequest = wallet.authorizationRequest() + .requestUri(requestUri) + .codeChallenge(pkce) + .state("123456"); + assertFalse(authRequest.openLoginForm(), "Error expected"); + AuthorizationEndpointResponse authResponse = authRequest.parseLoginResponse(); + + assertNull(authResponse.getCode(), "Expected no auth code"); + assertEquals("invalid_request_object", authResponse.getError()); + assertEquals("PAR request did not include query parameter: state", authResponse.getErrorDescription()); + } + + // Private --------------------------------------------------------------------------------------------------------- + + private void verifyCredentialResponse(OID4VCTestContext ctx, CredentialResponse credResponse) throws Exception { + + CredentialScopeRepresentation credScope = ctx.getCredentialScope(); + String issuer = wallet.getIssuerMetadata(ctx).getCredentialIssuer(); + CredentialResponse.Credential credObj = credResponse.getCredentials().get(0); + assertNotNull(credObj, "The first credential in the array should not be null"); + + SdJwtVP sdJwtVP = SdJwtVP.of(credObj.getCredential().toString()); + IssuerSignedJWT issuerSignedJWT = sdJwtVP.getIssuerSignedJWT(); + JsonWebToken vcSdJwt = TokenVerifier.create(issuerSignedJWT.getJws(), JsonWebToken.class).getToken(); + Map otherClaims = vcSdJwt.getOtherClaims(); + assertEquals(issuer, vcSdJwt.getIssuer()); + assertEquals(credScope.getVct(), otherClaims.get(CLAIM_NAME_VCT)); + + Map claims = sdJwtVP.getClaims().values().stream().collect(Collectors.toMap( + arrayNode -> arrayNode.get(1).asText(), + arrayNode -> arrayNode.get(2).asText() + )); + assertEquals("Alice", claims.get("firstName")); + assertEquals("Wonderland", claims.get("lastName")); + assertEquals("alice@email.cz", claims.get("email")); + } +} diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCAuthorizationDetailsFlowTestBase.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCAuthorizationDetailsFlowTestBase.java index d7c5e513a45..72cdfe03906 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCAuthorizationDetailsFlowTestBase.java +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCAuthorizationDetailsFlowTestBase.java @@ -1,7 +1,6 @@ package org.keycloak.tests.oid4vc.issuance.signing; import java.util.List; -import java.util.Map; import java.util.function.Function; import java.util.function.Supplier; @@ -187,7 +186,7 @@ public abstract class OID4VCAuthorizationDetailsFlowTestBase extends OID4VCIssue // String wasCredentialIdentifier = ctx.getCredentialScope().getCredentialIdentifier(); if (!wasCredentialIdentifier.equals(credIdentifier)) { - setCredentialScopeAttributes(ctx.getCredentialScope(), Map.of(VC_IDENTIFIER, credIdentifier)); + setCredentialScopeAttribute(ctx.getCredentialScope(), VC_IDENTIFIER, credIdentifier); } try { @@ -244,7 +243,7 @@ public abstract class OID4VCAuthorizationDetailsFlowTestBase extends OID4VCIssue // Restore the vc.credential_identifier attribute value if (!wasCredentialIdentifier.equals(credIdentifier)) { - setCredentialScopeAttributes(ctx.getCredentialScope(), Map.of(VC_IDENTIFIER, wasCredentialIdentifier)); + setCredentialScopeAttribute(ctx.getCredentialScope(), VC_IDENTIFIER, wasCredentialIdentifier); } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2DPoPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2DPoPTest.java index c23b81cea8a..30d33e6b9a1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2DPoPTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2DPoPTest.java @@ -223,7 +223,7 @@ public class FAPI2DPoPTest extends AbstractFAPI2Test { // without PAR request - should fail oauth.openLoginForm(); - assertBrowserWithError("request_uri not included."); + assertBrowserWithError("PAR request_uri not included."); pkceGenerator = PkceGenerator.s256(); @@ -252,11 +252,11 @@ public class FAPI2DPoPTest extends AbstractFAPI2Test { .send(); assertEquals(201, pResp.getStatusCode()); oauth.loginForm().requestUri(pResp.getRequestUri()).param("custom", "value").open(); - assertBrowserWithError("PAR request did not include necessary parameters"); + assertBrowserWithError("PAR request did not include query parameter: custom"); // duplicated usage of a PAR request - should fail oauth.loginForm().requestUri(pResp.getRequestUri()).open(); - assertBrowserWithError("PAR not found. not issued or used multiple times."); + assertBrowserWithError("PAR not found, not issued or used multiple times."); // send a pushed authorization request // use RSA key for DPoP proof but not send dpop_jkt diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2Test.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2Test.java index e9c4966792c..1c772e57d8f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2Test.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI2Test.java @@ -161,7 +161,7 @@ public class FAPI2Test extends AbstractFAPI2Test { // without PAR request - should fail oauth.openLoginForm(); - assertBrowserWithError("request_uri not included."); + assertBrowserWithError("PAR request_uri not included."); pkceGenerator = PkceGenerator.s256(); @@ -189,11 +189,11 @@ public class FAPI2Test extends AbstractFAPI2Test { assertEquals(201, pResp.getStatusCode()); requestUri = pResp.getRequestUri(); oauth.loginForm().requestUri(requestUri).state("testFAPI2SecurityProfileLoginWithMTLS").open(); - assertBrowserWithError("PAR request did not include necessary parameters"); + assertBrowserWithError("PAR request did not include query parameter: state"); // duplicated usage of a PAR request - should fail oauth.loginForm().requestUri(requestUri).state("testFAPI2SecurityProfileLoginWithMTLS").codeChallenge(pkceGenerator).open(); - assertBrowserWithError("PAR not found. not issued or used multiple times."); + assertBrowserWithError("PAR not found, not issued or used multiple times."); // send a pushed authorization request pResp = oauth.pushedAuthorizationRequest().nonce("123456").codeChallenge(pkceGenerator).send(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java index 4e6bffc3cef..56dd6bfa04e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java @@ -508,7 +508,7 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { @Test public void testSecureRequestObjectExecutor() throws Exception { Integer availablePeriod = SecureRequestObjectExecutor.DEFAULT_AVAILABLE_PERIOD + 400; - Integer allowedClockSkew = SecureRequestObjectExecutor.DEAULT_ALLOWED_CLOCK_SKEW + 15; // 30 sec + Integer allowedClockSkew = SecureRequestObjectExecutor.DEFAULT_ALLOWED_CLOCK_SKEW + 15; // 30 sec // register profiles String json = (new ClientProfilesBuilder()).addProfile( @@ -1701,12 +1701,12 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { oauth.client(clientBetaId); oauth.loginForm().state("randomstatesomething").requestUri(requestUri).open(); assertTrue(errorPage.isCurrent()); - assertEquals("PAR request did not include necessary parameters", errorPage.getError()); + assertEquals("PAR request did not include query parameter: state", errorPage.getError()); EventAssertion.assertError(events.poll()) - .type(EventType.LOGIN_ERROR).error(OAuthErrorException.INVALID_REQUEST) + .type(EventType.LOGIN_ERROR).error(OAuthErrorException.INVALID_REQUEST_OBJECT) .details(Details.REASON, Details.CLIENT_POLICY_ERROR) - .details(Details.CLIENT_POLICY_ERROR, OAuthErrorException.INVALID_REQUEST) - .details(Details.CLIENT_POLICY_ERROR_DETAIL, "PAR request did not include necessary parameters").clientId(null) + .details(Details.CLIENT_POLICY_ERROR, OAuthErrorException.INVALID_REQUEST_OBJECT) + .details(Details.CLIENT_POLICY_ERROR_DETAIL, "PAR request did not include query parameter: state").clientId(null) .userId(null); oauth.client(clientBetaId, "secretBeta"); @@ -1752,12 +1752,12 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { // only query parameters include state parameter oauth.loginForm().requestUri(requestUri).state("mystate2").open(); assertTrue(errorPage.isCurrent()); - assertEquals("PAR request did not include necessary parameters", errorPage.getError()); + assertEquals("PAR request did not include query parameter: state", errorPage.getError()); EventAssertion.assertError(events.poll()) - .type(EventType.LOGIN_ERROR).error(OAuthErrorException.INVALID_REQUEST) + .type(EventType.LOGIN_ERROR).error(OAuthErrorException.INVALID_REQUEST_OBJECT) .details(Details.REASON, Details.CLIENT_POLICY_ERROR) - .details(Details.CLIENT_POLICY_ERROR, OAuthErrorException.INVALID_REQUEST) - .details(Details.CLIENT_POLICY_ERROR_DETAIL, "PAR request did not include necessary parameters").clientId(null) + .details(Details.CLIENT_POLICY_ERROR, OAuthErrorException.INVALID_REQUEST_OBJECT) + .details(Details.CLIENT_POLICY_ERROR_DETAIL, "PAR request did not include query parameter: state").clientId(null) .userId(null); // Pushed Authorization Request with state parameter