From a4315f40761e22a5d2694eee3b4297393fa55a8b Mon Sep 17 00:00:00 2001 From: Martin Hardselius Date: Fri, 15 Sep 2017 10:17:45 +0200 Subject: [PATCH 1/2] Fix introspection error for pairwise access tokens When access tokens containing a pairwise sub are introspected, user related checks are using that sub to fetch the UserModel instead of fetching the user from the UserSession. No corresponding user is found (or possibly even another user) and the token is reported inactive. Resolves: KEYCLOAK-5494 --- .../keycloak/protocol/oidc/TokenManager.java | 21 +++++++++---------- .../OIDCPairwiseClientRegistrationTest.java | 20 ++++++++++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 769947a0b8..10c9516d16 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -203,17 +203,6 @@ public class TokenManager { return false; } - UserModel user = session.users().getUserById(token.getSubject(), realm); - if (user == null) { - return false; - } - if (!user.isEnabled()) { - return false; - } - if (token.getIssuedAt() < session.users().getNotBeforeOfUser(realm, user)) { - return false; - } - ClientModel client = realm.getClientByClientId(token.getIssuedFor()); if (client == null || !client.isEnabled() || token.getIssuedAt() < client.getNotBefore()) { return false; @@ -224,6 +213,16 @@ public class TokenManager { return true; } + UserModel user = userSession.getUser(); + if (user == null) { + return false; + } + if (!user.isEnabled()) { + return false; + } + if (token.getIssuedAt() < session.users().getNotBeforeOfUser(realm, user)) { + return false; + } userSession = new UserSessionCrossDCManager(session).getUserSessionWithClient(realm, token.getSessionState(), true, client.getId()); if (AuthenticationManager.isOfflineSessionValid(realm, userSession)) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java index 0601879004..45bcb7bb27 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java @@ -18,6 +18,8 @@ package org.keycloak.testsuite.client; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.lang.StringUtils; import org.junit.Before; import org.junit.Test; @@ -44,9 +46,11 @@ import org.keycloak.testsuite.util.ClientManager; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.UserInfoClientUtil; import org.keycloak.testsuite.util.UserManager; +import org.keycloak.util.JsonSerialization; import javax.ws.rs.client.Client; import javax.ws.rs.core.Response; +import java.io.IOException; import java.util.ArrayList; import java.util.Base64; import java.util.Collections; @@ -407,6 +411,22 @@ public class OIDCPairwiseClientRegistrationTest extends AbstractClientRegistrati Assert.assertEquals(idToken.getIssuedFor(), refreshedIdToken.getIssuedFor()); } + @Test + public void introspectPairwiseAccessToken() throws Exception { + // Create a pairwise client + OIDCClientRepresentation pairwiseClient = createPairwise(); + + // Login to pairwise client + OAuthClient.AccessTokenResponse accessTokenResponse = login(pairwiseClient, "test-user@localhost", "password"); + + String introspectionResponse = oauth.introspectAccessTokenWithClientCredential(pairwiseClient.getClientId(), pairwiseClient.getClientSecret(), accessTokenResponse.getAccessToken()); + + ObjectMapper objectMapper = new ObjectMapper(); + JsonNode jsonNode = objectMapper.readTree(introspectionResponse); + Assert.assertEquals(true, jsonNode.get("active").asBoolean()); + Assert.assertEquals("test-user@localhost", jsonNode.get("email").asText()); + } + @Test public void refreshPairwiseTokenDeletedUser() throws Exception { String userId = createUser(REALM_NAME, "delete-me@localhost", "password"); From 6b687c43188b2a0c43e5262025ad833e7b20de54 Mon Sep 17 00:00:00 2001 From: Martin Hardselius Date: Mon, 18 Sep 2017 11:26:57 +0200 Subject: [PATCH 2/2] Fix offline validation errors Refactored token validation method to run user checks only if the user session is valid. --- .../keycloak/protocol/oidc/TokenManager.java | 22 +++++++++++-------- .../OIDCPairwiseClientRegistrationTest.java | 2 ++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 10c9516d16..00b02c0336 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -208,11 +208,20 @@ public class TokenManager { return false; } - UserSessionModel userSession = new UserSessionCrossDCManager(session).getUserSessionWithClient(realm, token.getSessionState(), false, client.getId()); + UserSessionModel userSession = new UserSessionCrossDCManager(session).getUserSessionWithClient(realm, token.getSessionState(), false, client.getId()); if (AuthenticationManager.isSessionValid(realm, userSession)) { - return true; + return isUserValid(session, realm, token, userSession); } + userSession = new UserSessionCrossDCManager(session).getUserSessionWithClient(realm, token.getSessionState(), true, client.getId()); + if (AuthenticationManager.isOfflineSessionValid(realm, userSession)) { + return isUserValid(session, realm, token, userSession); + } + + return false; + } + + private boolean isUserValid(KeycloakSession session, RealmModel realm, AccessToken token, UserSessionModel userSession) { UserModel user = userSession.getUser(); if (user == null) { return false; @@ -223,15 +232,10 @@ public class TokenManager { if (token.getIssuedAt() < session.users().getNotBeforeOfUser(realm, user)) { return false; } - - userSession = new UserSessionCrossDCManager(session).getUserSessionWithClient(realm, token.getSessionState(), true, client.getId()); - if (AuthenticationManager.isOfflineSessionValid(realm, userSession)) { - return true; - } - - return false; + return true; } + public RefreshResult refreshAccessToken(KeycloakSession session, UriInfo uriInfo, ClientConnection connection, RealmModel realm, ClientModel authorizedClient, String encodedRefreshToken, EventBuilder event, HttpHeaders headers) throws OAuthErrorException { RefreshToken refreshToken = verifyRefreshToken(session, realm, encodedRefreshToken); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java index 45bcb7bb27..fa6ed907ab 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.lang.StringUtils; import org.junit.Before; import org.junit.Test; +import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.client.registration.Auth; @@ -38,6 +39,7 @@ import org.keycloak.representations.idm.ClientInitialAccessPresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.oidc.OIDCClientRepresentation; +import org.keycloak.representations.oidc.TokenMetadataRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls;