diff --git a/core/src/main/java/org/keycloak/OAuthErrorException.java b/core/src/main/java/org/keycloak/OAuthErrorException.java index c5f0173fd4..710dbf1181 100755 --- a/core/src/main/java/org/keycloak/OAuthErrorException.java +++ b/core/src/main/java/org/keycloak/OAuthErrorException.java @@ -28,6 +28,8 @@ public class OAuthErrorException extends Exception { public static final String INVALID_SCOPE = "invalid_grant"; public static final String UNAUTHORIZED_CLIENT = "unauthorized_client"; public static final String UNSUPPORTED_GRANT_TYPE = "unsupported_grant_type"; + public static final String INVALID_TOKEN = "invalid_token"; + public static final String INSUFFICIENT_SCOPE = "insufficient_scope"; public OAuthErrorException(String error, String description, String message, Throwable cause) { super(message, cause); diff --git a/server-spi/src/main/java/org/keycloak/events/Errors.java b/server-spi/src/main/java/org/keycloak/events/Errors.java index 2d6438fd2d..0981806a83 100755 --- a/server-spi/src/main/java/org/keycloak/events/Errors.java +++ b/server-spi/src/main/java/org/keycloak/events/Errors.java @@ -65,6 +65,7 @@ public interface Errors { String SSL_REQUIRED = "ssl_required"; String USER_SESSION_NOT_FOUND = "user_session_not_found"; + String SESSION_EXPIRED = "session_expired"; String EMAIL_SEND_FAILED = "email_send_failed"; String INVALID_EMAIL = "invalid_email"; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index d84cf57932..a79e67980e 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -152,7 +152,7 @@ public class TokenEndpoint { private void checkSsl() { if (!uriInfo.getBaseUri().getScheme().equals("https") && realm.getSslRequired().isRequired(clientConnection)) { - throw new ErrorResponseException("invalid_request", "HTTPS required", Response.Status.FORBIDDEN); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "HTTPS required", Response.Status.FORBIDDEN); } } @@ -168,13 +168,13 @@ public class TokenEndpoint { clientAuthAttributes = clientAuth.getClientAuthAttributes(); if (client.isBearerOnly()) { - throw new ErrorResponseException("invalid_client", "Bearer-only not allowed", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_CLIENT, "Bearer-only not allowed", Response.Status.BAD_REQUEST); } } private void checkGrantType() { if (grantType == null) { - throw new ErrorResponseException("invalid_request", "Missing form parameter: " + OIDCLoginProtocol.GRANT_TYPE_PARAM, Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Missing form parameter: " + OIDCLoginProtocol.GRANT_TYPE_PARAM, Response.Status.BAD_REQUEST); } if (grantType.equals(OAuth2Constants.AUTHORIZATION_CODE)) { @@ -200,20 +200,17 @@ public class TokenEndpoint { String code = formParams.getFirst(OAuth2Constants.CODE); if (code == null) { event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_request", "Missing parameter: " + OAuth2Constants.CODE, Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Missing parameter: " + OAuth2Constants.CODE, Response.Status.BAD_REQUEST); } ClientSessionCode accessCode = ClientSessionCode.parse(code, session, realm); if (accessCode == null) { String[] parts = code.split("\\."); if (parts.length == 2) { - try { - event.detail(Details.CODE_ID, new String(parts[1])); - } catch (Throwable t) { - } + event.detail(Details.CODE_ID, parts[1]); } event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_grant", "Code not found", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Code not found", Response.Status.BAD_REQUEST); } ClientSessionModel clientSession = accessCode.getClientSession(); @@ -221,16 +218,15 @@ public class TokenEndpoint { String codeExchanged = clientSession.getNote(CODE_EXCHANGED); if (codeExchanged != null && Boolean.parseBoolean(codeExchanged)) { - logger.codeUsedAlready(code); session.sessions().removeClientSession(realm, clientSession); event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_grant", "Code used already", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Code used already", Response.Status.BAD_REQUEST); } if (!accessCode.isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name(), ClientSessionCode.ActionType.CLIENT)) { event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_grant", "Code is expired", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Code is expired", Response.Status.BAD_REQUEST); } accessCode.setAction(null); @@ -239,17 +235,17 @@ public class TokenEndpoint { if (userSession == null) { event.error(Errors.USER_SESSION_NOT_FOUND); - throw new ErrorResponseException("invalid_grant", "User session not found", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User session not found", Response.Status.BAD_REQUEST); } UserModel user = userSession.getUser(); if (user == null) { event.error(Errors.USER_NOT_FOUND); - throw new ErrorResponseException("invalid_grant", "User not found", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User not found", Response.Status.BAD_REQUEST); } if (!user.isEnabled()) { event.error(Errors.USER_DISABLED); - throw new ErrorResponseException("invalid_grant", "User disabled", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User disabled", Response.Status.BAD_REQUEST); } event.user(userSession.getUser()); @@ -258,22 +254,22 @@ public class TokenEndpoint { String redirectUri = clientSession.getNote(OIDCLoginProtocol.REDIRECT_URI_PARAM); if (redirectUri != null && !redirectUri.equals(formParams.getFirst(OAuth2Constants.REDIRECT_URI))) { event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_grant", "Incorrect redirect_uri", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Incorrect redirect_uri", Response.Status.BAD_REQUEST); } if (!client.getClientId().equals(clientSession.getClient().getClientId())) { event.error(Errors.INVALID_CODE); - throw new ErrorResponseException("invalid_grant", "Auth error", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Auth error", Response.Status.BAD_REQUEST); } if (!client.isStandardFlowEnabled()) { event.error(Errors.NOT_ALLOWED); - throw new ErrorResponseException("invalid_grant", "Client not allowed to exchange code", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Client not allowed to exchange code", Response.Status.BAD_REQUEST); } if (!AuthenticationManager.isSessionValid(realm, userSession)) { event.error(Errors.USER_SESSION_NOT_FOUND); - throw new ErrorResponseException("invalid_grant", "Session not active", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Session not active", Response.Status.BAD_REQUEST); } updateClientSession(clientSession); @@ -368,12 +364,12 @@ public class TokenEndpoint { if (!client.isDirectAccessGrantsEnabled()) { event.error(Errors.NOT_ALLOWED); - throw new ErrorResponseException("invalid_grant", "Client not allowed for direct access grants", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Client not allowed for direct access grants", Response.Status.BAD_REQUEST); } if (client.isConsentRequired()) { event.error(Errors.CONSENT_DENIED); - throw new ErrorResponseException("invalid_client", "Client requires user consent", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_CLIENT, "Client requires user consent", Response.Status.BAD_REQUEST); } String scope = formParams.getFirst(OAuth2Constants.SCOPE); @@ -401,7 +397,7 @@ public class TokenEndpoint { UserModel user = clientSession.getAuthenticatedUser(); if (user.getRequiredActions() != null && user.getRequiredActions().size() > 0) { event.error(Errors.RESOLVE_REQUIRED_ACTIONS); - throw new ErrorResponseException("invalid_grant", "Account is not fully set up", Response.Status.BAD_REQUEST); + throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Account is not fully set up", Response.Status.BAD_REQUEST); } processor.attachSession(); @@ -423,15 +419,15 @@ public class TokenEndpoint { public Response buildClientCredentialsGrant() { if (client.isBearerOnly()) { event.error(Errors.INVALID_CLIENT); - throw new ErrorResponseException("unauthorized_client", "Bearer-only client not allowed to retrieve service account", Response.Status.UNAUTHORIZED); + throw new ErrorResponseException(OAuthErrorException.UNAUTHORIZED_CLIENT, "Bearer-only client not allowed to retrieve service account", Response.Status.UNAUTHORIZED); } if (client.isPublicClient()) { event.error(Errors.INVALID_CLIENT); - throw new ErrorResponseException("unauthorized_client", "Public client not allowed to retrieve service account", Response.Status.UNAUTHORIZED); + throw new ErrorResponseException(OAuthErrorException.UNAUTHORIZED_CLIENT, "Public client not allowed to retrieve service account", Response.Status.UNAUTHORIZED); } if (!client.isServiceAccountsEnabled()) { event.error(Errors.INVALID_CLIENT); - throw new ErrorResponseException("unauthorized_client", "Client not enabled to retrieve service account", Response.Status.UNAUTHORIZED); + throw new ErrorResponseException(OAuthErrorException.UNAUTHORIZED_CLIENT, "Client not enabled to retrieve service account", Response.Status.UNAUTHORIZED); } UserModel clientUser = session.users().getServiceAccount(client); @@ -449,7 +445,7 @@ public class TokenEndpoint { if (!clientUser.isEnabled()) { event.error(Errors.USER_DISABLED); - throw new ErrorResponseException("invalid_request", "User '" + clientUsername + "' disabled", Response.Status.UNAUTHORIZED); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "User '" + clientUsername + "' disabled", Response.Status.UNAUTHORIZED); } String scope = formParams.getFirst(OAuth2Constants.SCOPE); 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 ab60db1a16..06989cdf5a 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 @@ -24,6 +24,7 @@ import org.keycloak.OAuthErrorException; import org.keycloak.RSATokenVerifier; import org.keycloak.common.VerificationException; import org.keycloak.events.Details; +import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.ClientModel; @@ -49,7 +50,6 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.UriInfo; import java.util.HashMap; import java.util.Map; @@ -122,30 +122,61 @@ public class UserInfoEndpoint { .event(EventType.USER_INFO_REQUEST) .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN); + if (tokenString == null) { + event.error(Errors.INVALID_TOKEN); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Token not provided", Response.Status.BAD_REQUEST); + } + AccessToken token = null; try { token = RSATokenVerifier.verifyToken(tokenString, realm.getPublicKey(), Urls.realmIssuer(uriInfo.getBaseUri(), realm.getName()), true, true); } catch (VerificationException e) { - throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Token invalid: " + e.getMessage(), Status.FORBIDDEN); + event.error(Errors.INVALID_TOKEN); + throw new ErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Token invalid: " + e.getMessage(), Response.Status.UNAUTHORIZED); } UserSessionModel userSession = session.sessions().getUserSession(realm, token.getSessionState()); ClientSessionModel clientSession = session.sessions().getClientSession(token.getClientSession()); - if (userSession == null || clientSession == null || !AuthenticationManager.isSessionValid(realm, userSession)) { - throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Token invalid", Status.FORBIDDEN); + + if (userSession == null) { + event.error(Errors.USER_SESSION_NOT_FOUND); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "User session not found", Response.Status.BAD_REQUEST); + } + + event.session(userSession); + + UserModel userModel = userSession.getUser(); + if (userModel == null) { + event.error(Errors.USER_NOT_FOUND); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "User not found", Response.Status.BAD_REQUEST); + } + + event.user(userModel) + .detail(Details.USERNAME, userModel.getUsername()); + + + if (clientSession == null || !AuthenticationManager.isSessionValid(realm, userSession)) { + event.error(Errors.SESSION_EXPIRED); + throw new ErrorResponseException(OAuthErrorException.INVALID_TOKEN, "Session expired", Response.Status.UNAUTHORIZED); } ClientModel clientModel = realm.getClientByClientId(token.getIssuedFor()); - UserModel userModel = userSession.getUser(); + if (clientModel == null) { + event.error(Errors.CLIENT_NOT_FOUND); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Client not found", Response.Status.BAD_REQUEST); + } + + event.client(clientModel); + + if (!clientModel.isEnabled()) { + event.error(Errors.CLIENT_DISABLED); + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Client disabled", Response.Status.BAD_REQUEST); + } + AccessToken userInfo = new AccessToken(); tokenManager.transformAccessToken(session, userInfo, realm, clientModel, userModel, userSession, clientSession); - event - .detail(Details.USERNAME, userModel.getUsername()) - .client(clientModel) - .session(userSession) - .user(userModel) - .success(); + event.success(); Map claims = new HashMap(); claims.putAll(userInfo.getOtherClaims()); diff --git a/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java index 4536eefe0f..fb71a2e2ad 100644 --- a/services/src/main/java/org/keycloak/services/ServicesLogger.java +++ b/services/src/main/java/org/keycloak/services/ServicesLogger.java @@ -402,8 +402,4 @@ public interface ServicesLogger extends BasicLogger { @LogMessage(level = ERROR) @Message(id=90, value="Failed to close ProviderSession") void failedToCloseProviderSession(@Cause Throwable t); - - @LogMessage(level = WARN) - @Message(id=91, value="Attempt to re-use code '%s'") - void codeUsedAlready(String code); } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index e21062609d..4e6901f313 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -84,7 +84,7 @@ public class AuthenticationManager { } int currentTime = Time.currentTime(); int max = userSession.getStarted() + realm.getSsoSessionMaxLifespan(); - return userSession != null && userSession.getLastSessionRefresh() + realm.getSsoSessionIdleTimeout() > currentTime && max > currentTime; + return userSession.getLastSessionRefresh() + realm.getSsoSessionIdleTimeout() > currentTime && max > currentTime; } public static void expireUserSessionCookie(KeycloakSession session, UserSessionModel userSession, RealmModel realm, UriInfo uriInfo, HttpHeaders headers, ClientConnection connection) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java index 88d7f3f2fb..dfade959ed 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java @@ -365,7 +365,7 @@ public class AccessTokenTest extends AbstractKeycloakTest { // Check that userInfo can't be invoked with invalidated accessToken userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(jaxrsClient, accessToken); - assertEquals(Response.Status.FORBIDDEN.getStatusCode(), userInfoResponse.getStatus()); + assertEquals(Response.Status.UNAUTHORIZED.getStatusCode(), userInfoResponse.getStatus()); userInfoResponse.close(); // Check that tokenIntrospection can't be invoked with invalidated accessToken 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 994ce3ae6b..268333f0e5 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 @@ -16,10 +16,14 @@ */ package org.keycloak.testsuite.oidc; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.keycloak.OAuth2Constants; +import org.keycloak.events.Details; +import org.keycloak.events.Errors; +import org.keycloak.events.EventType; import org.keycloak.protocol.oidc.OIDCLoginProtocolService; import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.UserInfo; @@ -44,7 +48,6 @@ import java.net.URI; import java.util.List; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson; import static org.keycloak.testsuite.util.OAuthClient.AUTH_SERVER_ROOT; @@ -160,10 +163,18 @@ public class UserInfoTest extends AbstractKeycloakTest { Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(client, accessTokenResponse.getToken()); - assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus()); + assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); response.close(); + events.expect(EventType.USER_INFO_REQUEST_ERROR) + .error(Errors.USER_SESSION_NOT_FOUND) + .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(); } @@ -178,7 +189,15 @@ public class UserInfoTest extends AbstractKeycloakTest { response.close(); - assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus()); + assertEquals(Status.UNAUTHORIZED.getStatusCode(), response.getStatus()); + + events.expect(EventType.USER_INFO_REQUEST_ERROR) + .error(Errors.INVALID_TOKEN) + .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(); @@ -206,10 +225,17 @@ public class UserInfoTest extends AbstractKeycloakTest { response.close(); + events.clear(); + return accessTokenResponse; } private void testSuccessfulUserInfoResponse(Response response) { + events.expect(EventType.USER_INFO_REQUEST) + .session(Matchers.notNullValue(String.class)) + .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN) + .detail(Details.USERNAME, "test-user@localhost") + .assertEvent(); UserInfoClientUtil.testSuccessfulUserInfoResponse(response, "test-user@localhost", "test-user@localhost"); } }