From cc2117bf7c996f9cf2c9fe0cc7e32629a7007174 Mon Sep 17 00:00:00 2001 From: Dmitry Telegin Date: Tue, 6 Sep 2022 20:49:15 +0100 Subject: [PATCH] UserInfo endpoint not fully standards compliant Closes #14184 --- .../testsuite/integration/ClusteringTest.java | 5 +- .../oidc/endpoints/UserInfoEndpoint.java | 117 ++++--- .../org/keycloak/services/resources/Cors.java | 67 +--- .../java/org/keycloak/utils/OAuth2Error.java | 305 ++++++++++++++++++ .../keycloak/testsuite/util/OAuthClient.java | 16 +- .../testsuite/oauth/OAuth2OnlyTest.java | 1 + .../testsuite/oauth/RefreshTokenTest.java | 16 +- ...urceOwnerPasswordCredentialsGrantTest.java | 3 +- .../keycloak/testsuite/oidc/UserInfoTest.java | 110 ++++++- 9 files changed, 516 insertions(+), 124 deletions(-) create mode 100644 services/src/main/java/org/keycloak/utils/OAuth2Error.java diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/ClusteringTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/ClusteringTest.java index 9a7123a15c..05335d1cf7 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/ClusteringTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/integration/ClusteringTest.java @@ -100,7 +100,7 @@ public class ClusteringTest extends BaseOperatorTest { } // local debug commands: - // export TOKEN=$(curl --data "grant_type=password&client_id=token-test-client&username=test&password=test" http://localhost:8080/realms/token-test/protocol/openid-connect/token | jq -r '.access_token') + // export TOKEN=$(curl --data "grant_type=password&client_id=token-test-client&username=test&password=test&scope=openid" http://localhost:8080/realms/token-test/protocol/openid-connect/token | jq -r '.access_token') // // curl http://localhost:8080/realms/token-test/protocol/openid-connect/userinfo -H "Authorization: bearer $TOKEN" // @@ -170,6 +170,7 @@ public class ClusteringTest extends BaseOperatorTest { .param("client_id", "token-test-client") .param("username", "test") .param("password", "test") + .param("scope", "openid") .post("https://localhost:" + portForward.getLocalPort() + "/realms/token-test/protocol/openid-connect/token") .body() .jsonPath() @@ -208,7 +209,7 @@ public class ClusteringTest extends BaseOperatorTest { var tokenUrl = "https://" + service.getName() + "." + namespace + ":" + Constants.KEYCLOAK_HTTPS_PORT + "/realms/token-test/protocol/openid-connect/token"; Log.info("Checking url: " + tokenUrl); - var tokenOutput = K8sUtils.inClusterCurl(k8sclient, namespace, "--insecure", "-s", "--data", "grant_type=password&client_id=token-test-client&username=test&password=test", tokenUrl); + var tokenOutput = K8sUtils.inClusterCurl(k8sclient, namespace, "--insecure", "-s", "--data", "grant_type=password&client_id=token-test-client&username=test&password=test&scope=openid", tokenUrl); Log.info("Curl Output with token: " + tokenOutput); JsonNode tokenAnswer = Serialization.jsonMapper().readTree(tokenOutput); assertThat(tokenAnswer.hasNonNull("access_token")).isTrue(); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/UserInfoEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/UserInfoEndpoint.java index 6326bbcb80..0f59fb2ac3 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/UserInfoEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/UserInfoEndpoint.java @@ -20,7 +20,6 @@ import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.spi.HttpResponse; import org.keycloak.OAuth2Constants; -import org.keycloak.OAuthErrorException; import org.keycloak.TokenCategory; import org.keycloak.TokenVerifier; import org.keycloak.common.ClientConnection; @@ -55,7 +54,6 @@ import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.TokenManager; import org.keycloak.protocol.oidc.TokenManager.NotBeforeCheck; import org.keycloak.representations.AccessToken; -import org.keycloak.services.CorsErrorResponseException; import org.keycloak.services.Urls; import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.services.clientpolicy.context.UserInfoRequestContext; @@ -70,6 +68,7 @@ import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.util.JsonSerialization; import org.keycloak.util.TokenUtil; import org.keycloak.utils.MediaType; +import org.keycloak.utils.OAuth2Error; import javax.ws.rs.GET; import javax.ws.rs.OPTIONS; @@ -83,7 +82,6 @@ import javax.ws.rs.core.MultivaluedMap; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.security.Key; -import java.util.Collections; import java.util.Map; /** @@ -106,12 +104,15 @@ public class UserInfoEndpoint { private final org.keycloak.protocol.oidc.TokenManager tokenManager; private final AppAuthManager appAuthManager; private final RealmModel realm; + private final OAuth2Error error; private Cors cors; + private String authorization; public UserInfoEndpoint(org.keycloak.protocol.oidc.TokenManager tokenManager, RealmModel realm) { this.realm = realm; this.tokenManager = tokenManager; this.appAuthManager = new AppAuthManager(); + this.error = new OAuth2Error().json(false).realm(realm); } @Path("/") @@ -124,57 +125,57 @@ public class UserInfoEndpoint { @GET @NoCache public Response issueUserInfoGet(@Context final HttpHeaders headers) { + setupCors(); String accessToken = this.appAuthManager.extractAuthorizationHeaderTokenOrReturnNull(headers); - return issueUserInfo(accessToken); + authorization(accessToken); + return issueUserInfo(); } @Path("/") @POST @NoCache public Response issueUserInfoPost() { + setupCors(); + // Try header first HttpHeaders headers = request.getHttpHeaders(); String accessToken = this.appAuthManager.extractAuthorizationHeaderTokenOrReturnNull(headers); + authorization(accessToken); - // Fallback to form parameter - if (accessToken == null) { + try { MultivaluedMap formParams = request.getDecodedFormParameters(); checkAccessTokenDuplicated(formParams); accessToken = formParams.getFirst(OAuth2Constants.ACCESS_TOKEN); + authorization(accessToken); + } catch (IllegalArgumentException e) { + // not application/x-www-form-urlencoded, ignore } - return issueUserInfo(accessToken); + return issueUserInfo(); } - // This method won't add allowedOrigins to the cors. Assumption is that allowedOrigins are already set to the "cors" object when this method is called - private CorsErrorResponseException newUnauthorizedErrorResponseException(String oauthError, String errorMessage) { - // See: https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError - response.getOutputHeaders().put(HttpHeaders.WWW_AUTHENTICATE, Collections.singletonList(String.format("Bearer realm=\"%s\", error=\"%s\", error_description=\"%s\"", realm.getName(), oauthError, errorMessage))); - return new CorsErrorResponseException(cors, oauthError, errorMessage, Response.Status.UNAUTHORIZED); - } - - private Response issueUserInfo(String tokenString) { - cors = Cors.add(request).auth().allowedMethods(request.getHttpMethod()).auth().exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS); + private Response issueUserInfo() { + cors.allowAllOrigins(); try { - session.clientPolicy().triggerOnEvent(new UserInfoRequestContext(tokenString)); + session.clientPolicy().triggerOnEvent(new UserInfoRequestContext(authorization)); } catch (ClientPolicyException cpe) { - throw new CorsErrorResponseException(cors.allowAllOrigins(), cpe.getError(), cpe.getErrorDetail(), cpe.getErrorStatus()); + throw error.error(cpe.getError()).errorDescription(cpe.getErrorDetail()).status(cpe.getErrorStatus()).build(); } EventBuilder event = new EventBuilder(realm, session, clientConnection) .event(EventType.USER_INFO_REQUEST) .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN); - if (tokenString == null) { + if (authorization == null) { event.error(Errors.INVALID_TOKEN); - throw new CorsErrorResponseException(cors.allowAllOrigins(), OAuthErrorException.INVALID_REQUEST, "Token not provided", Response.Status.BAD_REQUEST); + throw error.unauthorized(); } AccessToken token; ClientModel clientModel = null; try { - TokenVerifier verifier = TokenVerifier.create(tokenString, AccessToken.class).withDefaultChecks() + TokenVerifier verifier = TokenVerifier.create(authorization, AccessToken.class).withDefaultChecks() .realmUrl(Urls.realmIssuer(session.getContext().getUri().getBaseUri(), realm.getName())); SignatureVerifierContext verifierContext = session.getProvider(SignatureProvider.class, verifier.getHeader().getAlgorithm().name()).verifier(verifier.getHeader().getKeyId()); @@ -182,10 +183,15 @@ public class UserInfoEndpoint { token = verifier.verify().getToken(); + if (!TokenUtil.hasScope(token.getScope(), OAuth2Constants.SCOPE_OPENID)) { + event.error(Errors.ACCESS_DENIED); + throw error.insufficientScope("Missing openid scope"); + } + clientModel = realm.getClientByClientId(token.getIssuedFor()); if (clientModel == null) { event.error(Errors.CLIENT_NOT_FOUND); - throw new CorsErrorResponseException(cors.allowAllOrigins(), OAuthErrorException.INVALID_REQUEST, "Client not found", Response.Status.BAD_REQUEST); + throw error.invalidToken("Client not found"); } cors.allowedOrigins(session, clientModel); @@ -198,12 +204,12 @@ public class UserInfoEndpoint { cors.allowAllOrigins(); } event.error(Errors.INVALID_TOKEN); - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Token verification failed"); + throw error.invalidToken("Token verification failed"); } - if (!clientModel.getProtocol().equals(OIDCLoginProtocol.LOGIN_PROTOCOL)) { + if (!clientModel.getProtocol().equals(OIDCLoginProtocol.LOGIN_PROTOCOL)) { event.error(Errors.INVALID_CLIENT); - throw new CorsErrorResponseException(cors, Errors.INVALID_CLIENT, "Wrong client protocol.", Response.Status.BAD_REQUEST); + throw error.invalidToken("Wrong client protocol"); } session.getContext().setClient(clientModel); @@ -212,7 +218,7 @@ public class UserInfoEndpoint { if (!clientModel.isEnabled()) { event.error(Errors.CLIENT_DISABLED); - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, "Client disabled", Response.Status.BAD_REQUEST); + throw error.invalidToken("Client disabled"); } UserSessionModel userSession = findValidSession(token, event, clientModel); @@ -220,18 +226,23 @@ public class UserInfoEndpoint { UserModel userModel = userSession.getUser(); if (userModel == null) { event.error(Errors.USER_NOT_FOUND); - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, "User not found", Response.Status.BAD_REQUEST); + throw error.invalidToken("User not found"); } event.user(userModel) .detail(Details.USERNAME, userModel.getUsername()); + if (!userModel.isEnabled()) { + event.error(Errors.USER_DISABLED); + throw error.invalidToken("User disabled"); + } + // KEYCLOAK-6771 Certificate Bound Token // https://tools.ietf.org/html/draft-ietf-oauth-mtls-08#section-3 if (OIDCAdvancedConfigWrapper.fromClientModel(clientModel).isUseMtlsHokToken()) { if (!MtlsHoKTokenUtil.verifyTokenBindingWithClientCertificate(token, request, session)) { event.error(Errors.NOT_ALLOWED); - throw newUnauthorizedErrorResponseException(OAuthErrorException.UNAUTHORIZED_CLIENT, "Client certificate missing, or its thumbprint and one in the refresh token did NOT match"); + throw error.invalidToken("Client certificate missing, or its thumbprint and one in the refresh token did NOT match"); } } @@ -242,7 +253,7 @@ public class UserInfoEndpoint { ClientSessionContext clientSessionCtx = DefaultClientSessionContext.fromClientSessionScopeParameter(clientSession, session); AccessToken userInfo = new AccessToken(); - + tokenManager.transformUserInfoAccessToken(session, userInfo, userSession, clientSessionCtx); Map claims = tokenManager.generateUserInfoClaims(userInfo, userModel); @@ -266,12 +277,7 @@ public class UserInfoEndpoint { responseBuilder = Response.ok(cfg.isUserInfoEncryptionRequired() ? jweFromContent(signedUserInfo, "JWT") : signedUserInfo).header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JWT); } catch (RuntimeException re) { - if ("can not get encryption KEK".equals(re.getMessage())) { - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, - "can not get encryption KEK", Response.Status.BAD_REQUEST); - } else { - throw re; - } + throw error.status(Response.Status.INTERNAL_SERVER_ERROR).build(); } event.detail(Details.SIGNATURE_REQUIRED, "true"); event.detail(Details.SIGNATURE_ALGORITHM, cfg.getUserInfoSignedResponseAlg()); @@ -279,15 +285,8 @@ public class UserInfoEndpoint { try { responseBuilder = Response.ok(jweFromContent(JsonSerialization.writeValueAsString(claims), null)) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JWT); - } catch (RuntimeException re) { - if ("can not get encryption KEK".equals(re.getMessage())) { - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, - "can not get encryption KEK", Response.Status.BAD_REQUEST); - } else { - throw re; - } - } catch (IOException e) { - throw new RuntimeException(e); + } catch (RuntimeException | IOException ex) { + throw error.status(Response.Status.INTERNAL_SERVER_ERROR).build(); } event.detail(Details.SIGNATURE_REQUIRED, "false"); @@ -335,7 +334,7 @@ public class UserInfoEndpoint { // create a transient session UserModel user = TokenManager.lookupUserFromStatelessToken(session, realm, token); if (user == null) { - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_REQUEST, "User not found"); + throw error.invalidToken("User not found"); } UserSessionModel userSession = session.sessions().createUserSession(KeycloakModelUtils.generateId(), realm, user, user.getUsername(), clientConnection.getRemoteAddr(), ServiceAccountConstants.CLIENT_AUTH, false, null, null, UserSessionModel.SessionPersistenceState.TRANSIENT); @@ -372,7 +371,7 @@ public class UserInfoEndpoint { if (userSession == null && offlineUserSession == null) { event.error(Errors.USER_SESSION_NOT_FOUND); - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_REQUEST, "User session not found or doesn't have client attached on it"); + throw error.invalidToken("User session not found or doesn't have client attached on it"); } if (userSession != null) { @@ -382,19 +381,19 @@ public class UserInfoEndpoint { } event.error(Errors.SESSION_EXPIRED); - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Session expired"); + throw error.invalidToken("Session expired"); } - private void checkTokenIssuedAt(AccessToken token, UserSessionModel userSession, EventBuilder event, ClientModel client) throws CorsErrorResponseException { + private void checkTokenIssuedAt(AccessToken token, UserSessionModel userSession, EventBuilder event, ClientModel client) { if (token.isIssuedBeforeSessionStart(userSession.getStarted())) { event.error(Errors.INVALID_TOKEN); - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Stale token"); + throw error.invalidToken("Stale token"); } AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId()); if (token.isIssuedBeforeSessionStart(clientSession.getStarted())) { event.error(Errors.INVALID_TOKEN); - throw newUnauthorizedErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Stale token"); + throw error.invalidToken("Stale token"); } } @@ -402,8 +401,22 @@ public class UserInfoEndpoint { // If access_token is not provided, error is thrown in issueUserInfo(). // Only checks duplication of access token parameter in this function. if (formParams.containsKey(OAuth2Constants.ACCESS_TOKEN) && formParams.get(OAuth2Constants.ACCESS_TOKEN).size() != 1) { - cors = Cors.add(request).auth().allowedMethods(request.getHttpMethod()).auth().exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS); - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, "duplicated parameter", Response.Status.BAD_REQUEST); + throw error.invalidRequest("Duplicate parameter"); + } + } + + private void setupCors() { + cors = Cors.add(request).auth().allowedMethods(request.getHttpMethod()).auth().exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS); + error.cors(cors); + } + + private void authorization(String accessToken) { + if (accessToken != null) { + if (authorization == null) { + authorization = accessToken; + } else { + throw error.cors(cors.allowAllOrigins()).invalidRequest("More than one method used for including an access token"); + } } } } diff --git a/services/src/main/java/org/keycloak/services/resources/Cors.java b/services/src/main/java/org/keycloak/services/resources/Cors.java index ce6199ff84..740a78a479 100755 --- a/services/src/main/java/org/keycloak/services/resources/Cors.java +++ b/services/src/main/java/org/keycloak/services/resources/Cors.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.ResponseBuilder; import org.jboss.logging.Logger; @@ -134,53 +135,17 @@ public class Cors { } public Response build() { - String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER); - if (origin == null) { - logger.trace("No origin header ignoring"); - return builder.build(); - } - - if (!preflight && (allowedOrigins == null || (!allowedOrigins.contains(origin) && !allowedOrigins.contains(ACCESS_CONTROL_ALLOW_ORIGIN_WILDCARD)))) { - if (logger.isDebugEnabled()) { - logger.debugv("Invalid CORS request: origin {0} not in allowed origins {1}", origin, allowedOrigins); - } - return builder.build(); - } - - builder.header(ACCESS_CONTROL_ALLOW_ORIGIN, origin); - - if (preflight) { - if (allowedMethods != null) { - builder.header(ACCESS_CONTROL_ALLOW_METHODS, CollectionUtil.join(allowedMethods)); - } else { - builder.header(ACCESS_CONTROL_ALLOW_METHODS, DEFAULT_ALLOW_METHODS); - } - } - - if (!preflight && exposedHeaders != null) { - builder.header(ACCESS_CONTROL_EXPOSE_HEADERS, CollectionUtil.join(exposedHeaders)); - } - - builder.header(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.toString(auth)); - - if (preflight) { - if (auth) { - builder.header(ACCESS_CONTROL_ALLOW_HEADERS, String.format("%s, %s", DEFAULT_ALLOW_HEADERS, AUTHORIZATION_HEADER)); - } else { - builder.header(ACCESS_CONTROL_ALLOW_HEADERS, DEFAULT_ALLOW_HEADERS); - } - } - - if (preflight) { - builder.header(ACCESS_CONTROL_MAX_AGE, DEFAULT_MAX_AGE); - } - + build(builder::header); logger.debug("Added CORS headers to response"); - return builder.build(); } public void build(HttpResponse response) { + build(response.getOutputHeaders()::add); + logger.debug("Added CORS headers to response"); + } + + public void build(BiConsumer addHeader) { String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER); if (origin == null) { logger.trace("No origin header ignoring"); @@ -194,35 +159,33 @@ public class Cors { return; } - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_ORIGIN, origin); + addHeader.accept(ACCESS_CONTROL_ALLOW_ORIGIN, origin); if (preflight) { if (allowedMethods != null) { - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_METHODS, CollectionUtil.join(allowedMethods)); + addHeader.accept(ACCESS_CONTROL_ALLOW_METHODS, CollectionUtil.join(allowedMethods)); } else { - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_METHODS, DEFAULT_ALLOW_METHODS); + addHeader.accept(ACCESS_CONTROL_ALLOW_METHODS, DEFAULT_ALLOW_METHODS); } } if (!preflight && exposedHeaders != null) { - response.getOutputHeaders().add(ACCESS_CONTROL_EXPOSE_HEADERS, CollectionUtil.join(exposedHeaders)); + addHeader.accept(ACCESS_CONTROL_EXPOSE_HEADERS, CollectionUtil.join(exposedHeaders)); } - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.toString(auth)); + addHeader.accept(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.toString(auth)); if (preflight) { if (auth) { - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_HEADERS, String.format("%s, %s", DEFAULT_ALLOW_HEADERS, AUTHORIZATION_HEADER)); + addHeader.accept(ACCESS_CONTROL_ALLOW_HEADERS, String.format("%s, %s", DEFAULT_ALLOW_HEADERS, AUTHORIZATION_HEADER)); } else { - response.getOutputHeaders().add(ACCESS_CONTROL_ALLOW_HEADERS, DEFAULT_ALLOW_HEADERS); + addHeader.accept(ACCESS_CONTROL_ALLOW_HEADERS, DEFAULT_ALLOW_HEADERS); } } if (preflight) { - response.getOutputHeaders().add(ACCESS_CONTROL_MAX_AGE, DEFAULT_MAX_AGE); + addHeader.accept(ACCESS_CONTROL_MAX_AGE, DEFAULT_MAX_AGE); } - - logger.debug("Added CORS headers to response"); } } diff --git a/services/src/main/java/org/keycloak/utils/OAuth2Error.java b/services/src/main/java/org/keycloak/utils/OAuth2Error.java new file mode 100644 index 0000000000..1b38ffd6e4 --- /dev/null +++ b/services/src/main/java/org/keycloak/utils/OAuth2Error.java @@ -0,0 +1,305 @@ +/* + * Copyright 2022 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.utils; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.stream.Collectors; +import javax.ws.rs.BadRequestException; +import javax.ws.rs.ForbiddenException; +import javax.ws.rs.InternalServerErrorException; +import javax.ws.rs.NotAuthorizedException; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; + +import org.keycloak.OAuthErrorException; +import org.keycloak.models.RealmModel; +import org.keycloak.representations.idm.OAuth2ErrorRepresentation; +import org.keycloak.services.resources.Cors; + +import static javax.ws.rs.core.HttpHeaders.WWW_AUTHENTICATE; + +/** + * @author Dmitry Telegin + */ +public class OAuth2Error { + + private static final Map> STATUS_MAP = new HashMap<>(); + + private RealmModel realm; + private String error; + private String errorDescription; + private Optional cors = Optional.empty(); + + private Class clazz; + private Response.Status status; + private boolean json = true; + + static { + STATUS_MAP.put(Response.Status.BAD_REQUEST, BadRequestException.class); + STATUS_MAP.put(Response.Status.UNAUTHORIZED, NotAuthorizedException.class); + STATUS_MAP.put(Response.Status.FORBIDDEN, ForbiddenException.class); + STATUS_MAP.put(Response.Status.INTERNAL_SERVER_ERROR, InternalServerErrorException.class); + } + + public OAuth2Error realm(RealmModel realm) { + this.realm = realm; + return this; + } + + public OAuth2Error error(String error) { + + this.error = error; + + switch (error) { + case OAuthErrorException.INVALID_GRANT: + case OAuthErrorException.INVALID_REQUEST: + case OAuthErrorException.UNAUTHORIZED_CLIENT: + case OAuthErrorException.UNSUPPORTED_GRANT_TYPE: + case OAuthErrorException.INVALID_SCOPE: + status = Response.Status.BAD_REQUEST; + break; + case OAuthErrorException.INVALID_CLIENT: + case OAuthErrorException.INVALID_TOKEN: + status = Response.Status.UNAUTHORIZED; + break; + case OAuthErrorException.INSUFFICIENT_SCOPE: + status = Response.Status.FORBIDDEN; + break; + case OAuthErrorException.SERVER_ERROR: + status = Response.Status.INTERNAL_SERVER_ERROR; + break; + default: + throw new IllegalArgumentException("Unrecognized OAuth 2.0 error: " + error); + } + + return this; + } + + public OAuth2Error errorDescription(String errorDescription) { + this.errorDescription = errorDescription; + return this; + } + + public OAuth2Error cors(Cors cors) { + this.cors = Optional.ofNullable(cors); + return this; + } + + public OAuth2Error status(Response.Status status) { + this.status = status; + return this; + } + + public OAuth2Error json(boolean json) { + this.json = json; + return this; + } + + public WebApplicationException build() { + clazz = STATUS_MAP.getOrDefault(status, WebApplicationException.class); + Response.ResponseBuilder builder = Response.status(status); + + try { + Constructor constructor = clazz.getConstructor(new Class[] { Response.class }); + cors.ifPresent(_cors -> { _cors.build(builder::header); }); + + if (json) { + OAuth2ErrorRepresentation errorRep = new OAuth2ErrorRepresentation(error, errorDescription); + builder.entity(errorRep).type(MediaType.APPLICATION_JSON_TYPE); + } else { + WWWAuthenticate.BearerChallenge bearer = new WWWAuthenticate.BearerChallenge(); + bearer.setRealm(realm.getName()); + bearer.setError(error); + bearer.setErrorDescription(errorDescription); + WWWAuthenticate wwwAuthenticate = new WWWAuthenticate(bearer); + wwwAuthenticate.build(builder::header); + builder.entity(""); + } + + return constructor.newInstance(builder.build()); + } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) { + throw new InternalServerErrorException(ex); + } + } + + public WebApplicationException insufficientScope(String errorDescription) { + return this.error(OAuthErrorException.INSUFFICIENT_SCOPE).errorDescription(errorDescription).build(); + } + + public WebApplicationException invalidToken(String errorDescription) { + return this.error(OAuthErrorException.INVALID_TOKEN).errorDescription(errorDescription).build(); + } + + public WebApplicationException invalidRequest(String errorDescription) { + return this.error(OAuthErrorException.INVALID_REQUEST).errorDescription(errorDescription).build(); + } + + public WebApplicationException unauthorized() { + return this.status(Response.Status.UNAUTHORIZED).build(); + } + + private static class WWWAuthenticate { + + private final List challenges; + private Challenge master; + private boolean singleHeader = true; + + public WWWAuthenticate(Challenge challenge, Challenge... moreChallenges) { + challenges = new ArrayList<>(1 + ((moreChallenges == null) ? 0 : moreChallenges.length)); + challenges.add(challenge); + if (moreChallenges != null) { + challenges.addAll(Arrays.asList(moreChallenges)); + } + master = challenge; + } + + public void addChallenge(Challenge challenge) { + challenges.add(challenge); + } + + public void setMasterChallenge(Challenge challenge) { + if (challenges.contains(challenge)) { + master = challenge; + } else { + throw new IllegalArgumentException("Unknown challenge: " + challenge); + } + } + + public void setMasterChallenge(String scheme) { + master = challenges.stream() + .filter(c -> c.getScheme().equals(scheme)) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("Unknown challenge: " + scheme)); + } + + public Challenge getMasterChallenge() { + return master; + } + + public boolean isSingleHeader() { + return singleHeader; + } + + public void setSingleHeader(boolean singleHeader) { + this.singleHeader = singleHeader; + } + + public void setAttribute(String attribute, String value) { + challenges.forEach(c -> c.setAttribute(attribute, value)); + } + + public void build(BiConsumer addHeader) { + if (singleHeader) { + String header = challenges.stream() + .map(Challenge::toString) + .collect(Collectors.joining(", ")); + addHeader.accept(WWW_AUTHENTICATE, header); + } else { + challenges.forEach(c -> addHeader.accept(WWW_AUTHENTICATE, c)); + } + } + + public static abstract class Challenge { + + private final Map attributes = new LinkedHashMap<>(); + + public void setAttribute(String attribute, String value) { + if (value != null) { + attributes.put(attribute, value); + } + } + + public abstract String getScheme(); + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(getScheme()); + + if (!attributes.isEmpty()) { + sb.append(" ").append( + attributes.entrySet().stream() + .map(e -> String.format("%s=\"%s\"", e.getKey(), e.getValue())) + .collect(Collectors.joining(", ")) + ); + } + + return sb.toString(); + } + + } + + public static class BasicChallenge extends Challenge { + + private static final String BASIC_SCHEME = "Basic"; + private static final String REALM_ATTRIBUTE = "realm"; + + public void setRealm(String realm) { + setAttribute(REALM_ATTRIBUTE, realm); + } + + @Override + public String getScheme() { + return BASIC_SCHEME; + } + + } + + public static class BearerChallenge extends BasicChallenge { + + private static final String BEARER_SCHEME = "Bearer"; + + private static final String ERROR_ATTRIBUTE = "error"; + private static final String ERROR_DESCRIPTION_ATTRIBUTE = "error_description"; + private static final String ERROR_URI_ATTRIBUTE = "error_uri"; + private static final String SCOPE_ATTRIBUTE = "scope"; + + public void setError(String error) { + setAttribute(ERROR_ATTRIBUTE, error); + } + + public void setErrorDescription(String errorDescription) { + setAttribute(ERROR_DESCRIPTION_ATTRIBUTE, errorDescription); + } + + public void setErrorUri(String errorUri) { + setAttribute(ERROR_URI_ATTRIBUTE, errorUri); + } + + public void setScope(String scope) { + setAttribute(SCOPE_ATTRIBUTE, scope); + } + + @Override + public String getScheme() { + return BEARER_SCHEME; + } + + } + + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java index 34e6c575b0..b3e170dd7e 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java @@ -621,8 +621,10 @@ public class OAuthClient { if (clientSessionHost != null) { parameters.add(new BasicNameValuePair(AdapterConstants.CLIENT_SESSION_HOST, clientSessionHost)); } - if (scope != null) { - parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scope)); + + String scopeParam = openid ? TokenUtil.attachOIDCScope(scope) : scope; + if (scopeParam != null && !scopeParam.isEmpty()) { + parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scopeParam)); } if (userAgent != null) { @@ -751,8 +753,9 @@ public class OAuthClient { List parameters = new LinkedList<>(); parameters.add(new BasicNameValuePair(OAuth2Constants.GRANT_TYPE, OAuth2Constants.CLIENT_CREDENTIALS)); - if (scope != null) { - parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scope)); + String scopeParam = openid ? TokenUtil.attachOIDCScope(scope) : scope; + if (scopeParam != null && !scopeParam.isEmpty()) { + parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scopeParam)); } UrlEncodedFormEntity formEntity; @@ -1032,8 +1035,9 @@ public class OAuthClient { post.addHeader("Origin", origin); } - if (scope != null) { - parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scope)); + String scopeParam = openid ? TokenUtil.attachOIDCScope(scope) : scope; + if (scopeParam != null && !scopeParam.isEmpty()) { + parameters.add(new BasicNameValuePair(OAuth2Constants.SCOPE, scopeParam)); } if (nonce != null) { parameters.add(new BasicNameValuePair(OIDCLoginProtocol.NONCE_PARAM, scope)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2OnlyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2OnlyTest.java index 4f5d0dba43..fd933453dc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2OnlyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2OnlyTest.java @@ -109,6 +109,7 @@ public class OAuth2OnlyTest extends AbstractTestRealmKeycloakTest { * @see AccessTokenTest#testAuthorizationNegotiateHeaderIgnored() */ oauth.init(driver); + oauth.openid(false); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java index 2d0b87ea42..83e72866db 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.oauth; import com.fasterxml.jackson.databind.JsonNode; +import org.hamcrest.CoreMatchers; import org.jboss.arquillian.graphene.page.Page; import org.junit.Assert; import org.junit.Before; @@ -42,7 +43,6 @@ import org.keycloak.protocol.oidc.OIDCLoginProtocolService; import org.keycloak.representations.AccessToken; import org.keycloak.representations.IDToken; import org.keycloak.representations.RefreshToken; -import org.keycloak.representations.UserInfo; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -59,6 +59,7 @@ import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.RealmManager; import org.keycloak.testsuite.util.TokenSignatureUtil; +import org.keycloak.testsuite.util.UserInfoClientUtil; import org.keycloak.testsuite.util.UserManager; import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.util.BasicAuthHelper; @@ -79,6 +80,7 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -637,9 +639,15 @@ public class RefreshTokenTest extends AbstractKeycloakTest { Assert.assertFalse(jsonNode.get("active").asBoolean()); // Try userInfo with the same old access token. Should fail as well - UserInfo userInfo = oauth.doUserInfoRequest(response1.getAccessToken()); - Assert.assertNull(userInfo.getSubject()); - Assert.assertEquals(userInfo.getOtherClaims().get(OAuth2Constants.ERROR), OAuthErrorException.INVALID_TOKEN); +// UserInfo userInfo = oauth.doUserInfoRequest(response1.getAccessToken()); + Client client = AdminClientUtil.createResteasyClient(); + Response userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, response1.getAccessToken()); + assertEquals(Response.Status.UNAUTHORIZED.getStatusCode(), userInfoResponse.getStatus()); + String wwwAuthHeader = userInfoResponse.getHeaderString(HttpHeaders.WWW_AUTHENTICATE); + assertNotNull(wwwAuthHeader); + assertThat(wwwAuthHeader, CoreMatchers.containsString("Bearer")); + assertThat(wwwAuthHeader, CoreMatchers.containsString("error=\"" + OAuthErrorException.INVALID_TOKEN + "\"")); + events.clear(); // Try to refresh with one of the old refresh tokens before SSO re-authentication - should fail diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java index 2400d69980..678221fc9d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java @@ -97,6 +97,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT @Override public void beforeAbstractKeycloakTest() throws Exception { super.beforeAbstractKeycloakTest(); + oauth.openid(false); } @Override @@ -234,7 +235,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest extends AbstractKeycloakT @Test @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true) - public void grantAccessTokenWithUnassignedDynamicScope() throws Exception {; + public void grantAccessTokenWithUnassignedDynamicScope() throws Exception { oauth.scope("unknown-scope:123"); oauth.clientId("resource-owner-public"); OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "direct-login", "password"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java index 2b2906d91a..881acd2e0a 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java @@ -64,6 +64,7 @@ import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.UserInfo; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.Urls; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; @@ -496,7 +497,7 @@ public class UserInfoTest extends AbstractKeycloakTest { assertNotNull(wwwAuthHeader); assertThat(wwwAuthHeader, CoreMatchers.containsString("Bearer")); assertThat(wwwAuthHeader, CoreMatchers.containsString("realm=\"" + realmName + "\"")); - assertThat(wwwAuthHeader, CoreMatchers.containsString("error=\"" + OAuthErrorException.INVALID_REQUEST + "\"")); + assertThat(wwwAuthHeader, CoreMatchers.containsString("error=\"" + OAuthErrorException.INVALID_TOKEN + "\"")); response.close(); @@ -652,7 +653,7 @@ public class UserInfoTest extends AbstractKeycloakTest { Client client = AdminClientUtil.createResteasyClient(); try { - AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client, true); + AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client, true, true); testingClient.testing().removeUserSessions("test"); @@ -700,8 +701,10 @@ public class UserInfoTest extends AbstractKeycloakTest { try { Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, ""); + String wwwAuthHeader = response.getHeaderString(HttpHeaders.WWW_AUTHENTICATE); + assertEquals(Status.UNAUTHORIZED.getStatusCode(), response.getStatus()); + assertEquals(wwwAuthHeader, "Bearer realm=\"test\""); response.close(); - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); } finally { client.close(); } @@ -727,6 +730,92 @@ public class UserInfoTest extends AbstractKeycloakTest { } } + @Test + public void testUnsuccessfulUserInfoRequestWithMultipleTokens() { + Client client = AdminClientUtil.createResteasyClient(); + + try { + AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client); + String accessToken = accessTokenResponse.getToken(); + + Form form = new Form(); + form.param("access_token", accessToken); + + WebTarget userInfoTarget = UserInfoClientUtil.getUserInfoWebTarget(client); + Response response = userInfoTarget.request() + .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken) + .post(Entity.form(form)); + response.close(); + assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + } finally { + client.close(); + } + } + + @Test + public void testUnsuccessfulUserInfoRequestWithoutOpenIDScope() { + Client client = AdminClientUtil.createResteasyClient(); + + try { + AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client, false, false); + Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken()); + response.close(); + + assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus()); + + String wwwAuthHeader = response.getHeaderString(HttpHeaders.WWW_AUTHENTICATE); + assertNotNull(wwwAuthHeader); + assertThat(wwwAuthHeader, CoreMatchers.containsString("Bearer")); + assertThat(wwwAuthHeader, CoreMatchers.containsString("error=\"" + OAuthErrorException.INSUFFICIENT_SCOPE + "\"")); + + events.expect(EventType.USER_INFO_REQUEST_ERROR) + .error(Errors.ACCESS_DENIED) + .client((String) null) + .user(Matchers.nullValue(String.class)) + .session(Matchers.nullValue(String.class)) + .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN) + .assertEvent(); + } finally { + client.close(); + } + } + + @Test + public void testUnsuccessfulUserInfoRequestWithDisabledUser() { + Client client = AdminClientUtil.createResteasyClient(); + RealmResource realm = adminClient.realm("test"); + UserResource userResource = ApiUtil.findUserByUsernameId(realm, "test-user@localhost"); + UserRepresentation user = userResource.toRepresentation(); + + try { + AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client); + user.setEnabled(false); + userResource.update(user); + Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken()); + response.close(); + + assertEquals(Status.UNAUTHORIZED.getStatusCode(), response.getStatus()); + + String wwwAuthHeader = response.getHeaderString(HttpHeaders.WWW_AUTHENTICATE); + assertNotNull(wwwAuthHeader); + assertThat(wwwAuthHeader, CoreMatchers.containsString("Bearer")); + assertThat(wwwAuthHeader, CoreMatchers.containsString("error=\"" + OAuthErrorException.INVALID_TOKEN + "\"")); + + events.expect(EventType.USER_INFO_REQUEST_ERROR) + .error(Errors.USER_DISABLED) + .client("test-app") + .user(user.getId()) + .session(Matchers.notNullValue(String.class)) + .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN) + .assertEvent(); + } finally { + client.close(); + } + + user.setEnabled(true); + userResource.update(user); + } + @Test public void testUserInfoRequestWithSamlClient() throws Exception { // obtain an access token @@ -743,7 +832,7 @@ public class UserInfoTest extends AbstractKeycloakTest { Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessToken); response.close(); - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals(Status.UNAUTHORIZED.getStatusCode(), response.getStatus()); events.expect(EventType.USER_INFO_REQUEST) .error(Errors.INVALID_CLIENT) .client((String) null) @@ -776,21 +865,28 @@ public class UserInfoTest extends AbstractKeycloakTest { } private AccessTokenResponse executeGrantAccessTokenRequest(Client client) { - return executeGrantAccessTokenRequest(client, false); + return executeGrantAccessTokenRequest(client, false, true); } - private AccessTokenResponse executeGrantAccessTokenRequest(Client client, boolean requestOfflineToken) { + private AccessTokenResponse executeGrantAccessTokenRequest(Client client, boolean requestOfflineToken, boolean openid) { UriBuilder builder = UriBuilder.fromUri(AUTH_SERVER_ROOT); URI grantUri = OIDCLoginProtocolService.tokenUrl(builder).build("test"); WebTarget grantTarget = client.target(grantUri); String header = BasicAuthHelper.createHeader("test-app", "password"); Form form = new Form(); + String scope = null; form.param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.PASSWORD) .param("username", "test-user@localhost") .param("password", "password"); if( requestOfflineToken) { - form.param("scope", "offline_access"); + scope = OAuth2Constants.OFFLINE_ACCESS; + } + if (openid) { + scope = TokenUtil.attachOIDCScope(scope); + } + if (scope != null) { + form.param(OAuth2Constants.SCOPE, scope); } Response response = grantTarget.request()