keycloak-memoizeit

KEYCLOAK-3290 UserInfoEndpoint error responses don't have

7/8/2016 7:15:07 AM

Details

diff --git a/core/src/main/java/org/keycloak/OAuthErrorException.java b/core/src/main/java/org/keycloak/OAuthErrorException.java
index c5f0173..710dbf1 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 2d6438f..0981806 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 d84cf57..a79e679 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 ab60db1..06989cd 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);
         }
 
-        ClientModel clientModel = realm.getClientByClientId(token.getIssuedFor());
+        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());
+        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<String, Object> claims = new HashMap<String, Object>();
         claims.putAll(userInfo.getOtherClaims());
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 e210626..4e6901f 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/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java
index 4536eef..fb71a2e 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/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 88d7f3f..dfade95 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 994ce3a..268333f 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");
     }
 }