From f7d00fc2e9ddb4d57dc0fcb112120e4f1d0d6af1 Mon Sep 17 00:00:00 2001 From: Yoshiyuki Tabata Date: Fri, 24 Apr 2020 10:54:42 +0900 Subject: [PATCH] KEYCLOAK-13844 "exp" claim should not be "0" when using offline token --- .../keycloak/protocol/oidc/TokenManager.java | 30 +++++++-- .../testsuite/oauth/OfflineTokenTest.java | 61 ++++++++++++++++++- 2 files changed, 85 insertions(+), 6 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 c6f87b8f1c..b0240e8d0d 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -648,12 +648,16 @@ public class TokenManager { token.setSessionState(session.getId()); - token.expiration(getTokenExpiration(realm, client, session, clientSession)); + ClientScopeModel offlineAccessScope = KeycloakModelUtils.getClientScopeByName(realm, OAuth2Constants.OFFLINE_ACCESS); + boolean offlineTokenRequested = offlineAccessScope == null ? false + : clientSessionCtx.getClientScopeIds().contains(offlineAccessScope.getId()); + token.expiration(getTokenExpiration(realm, client, session, clientSession, offlineTokenRequested)); return token; } - private int getTokenExpiration(RealmModel realm, ClientModel client, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) { + private int getTokenExpiration(RealmModel realm, ClientModel client, UserSessionModel userSession, + AuthenticatedClientSessionModel clientSession, boolean offlineTokenRequested) { boolean implicitFlow = false; String responseType = clientSession.getNote(OIDCLoginProtocol.RESPONSE_TYPE_PARAM); if (responseType != null) { @@ -681,9 +685,16 @@ public class TokenManager { expiration = Time.currentTime() + tokenLifespan; } - if (!userSession.isOffline()) { - int sessionExpires = userSession.getStarted() + (userSession.isRememberMe() && realm.getSsoSessionMaxLifespanRememberMe() > 0 ? - realm.getSsoSessionMaxLifespanRememberMe() : realm.getSsoSessionMaxLifespan()); + if (userSession.isOffline() || offlineTokenRequested) { + if (realm.isOfflineSessionMaxLifespanEnabled()) { + int sessionExpires = userSession.getStarted() + realm.getOfflineSessionMaxLifespan(); + expiration = expiration <= sessionExpires ? expiration : sessionExpires; + } + } else { + int sessionExpires = userSession.getStarted() + + (userSession.isRememberMe() && realm.getSsoSessionMaxLifespanRememberMe() > 0 + ? realm.getSsoSessionMaxLifespanRememberMe() + : realm.getSsoSessionMaxLifespan()); expiration = expiration <= sessionExpires ? expiration : sessionExpires; } @@ -779,6 +790,8 @@ public class TokenManager { refreshToken = new RefreshToken(accessToken); refreshToken.type(TokenUtil.TOKEN_TYPE_OFFLINE); + if (realm.isOfflineSessionMaxLifespanEnabled()) + refreshToken.expiration(getOfflineExpiration()); sessionManager.createOrUpdateOfflineSession(clientSessionCtx.getClientSession(), userSession); } else { refreshToken = new RefreshToken(accessToken); @@ -828,6 +841,13 @@ public class TokenManager { return expiration <= sessionExpires ? expiration : sessionExpires; } + private int getOfflineExpiration() { + int expiration = Time.currentTime() + realm.getOfflineSessionIdleTimeout(); + int sessionExpires = userSession.getStarted() + realm.getOfflineSessionMaxLifespan(); + + return expiration <= sessionExpires ? expiration : sessionExpires; + } + public AccessTokenResponseBuilder generateIDToken() { if (accessToken == null) { throw new IllegalStateException("accessToken not set"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java index a2cbcb5990..4a3ff18c94 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java @@ -70,6 +70,9 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertFalse; @@ -81,6 +84,9 @@ import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsername; import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsernameId; import static org.keycloak.testsuite.util.OAuthClient.APP_ROOT; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + /** * @author Marek Posolda */ @@ -557,7 +563,9 @@ public class OfflineTokenTest extends AbstractKeycloakTest { // Login as admin and see consents of user UserResource user = ApiUtil.findUserByUsernameId(appRealm, "test-user@localhost"); List> consents = user.getConsents(); - assertTrue(consents.isEmpty()); + for (Map consent : consents) { + assertNotEquals(consent.get("clientId"), "offline-client-2"); + } } @Test @@ -657,6 +665,17 @@ public class OfflineTokenTest extends AbstractKeycloakTest { return prev; } + private int[] changeSessionSettings(int ssoSessionIdle, int accessTokenLifespan) { + int prev[] = new int[2]; + RealmRepresentation rep = adminClient.realm("test").toRepresentation(); + prev[0] = rep.getOfflineSessionMaxLifespan().intValue(); + prev[1] = rep.getOfflineSessionIdleTimeout().intValue(); + RealmBuilder realmBuilder = RealmBuilder.create(); + realmBuilder.ssoSessionIdleTimeout(ssoSessionIdle).accessTokenLifespan(accessTokenLifespan); + adminClient.realm("test").update(realmBuilder.build()); + return prev; + } + @Test public void offlineTokenBrowserFlowMaxLifespanExpired() throws Exception { // expect that offline session expired by max lifespan @@ -822,4 +841,44 @@ public class OfflineTokenTest extends AbstractKeycloakTest { } + @Test + public void testShortOfflineSessionMax() throws Exception { + int prevOfflineSession[] = null; + int prevSession[] = null; + try { + prevOfflineSession = changeOfflineSessionSettings(true, 60, 30); + prevSession = changeSessionSettings(1800, 300); + + oauth.scope(OAuth2Constants.OFFLINE_ACCESS); + oauth.clientId("offline-client"); + oauth.redirectUri(offlineClientAppUri); + oauth.doLogin("test-user@localhost", "password"); + + events.expectLogin().client("offline-client").detail(Details.REDIRECT_URI, offlineClientAppUri).assertEvent(); + + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + + OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "secret1"); + String offlineTokenString = tokenResponse.getRefreshToken(); + RefreshToken offlineToken = oauth.parseRefreshToken(offlineTokenString); + + Assert.assertThat(tokenResponse.getExpiresIn(), allOf(greaterThanOrEqualTo(59), lessThanOrEqualTo(60))); + Assert.assertThat(tokenResponse.getRefreshExpiresIn(), allOf(greaterThanOrEqualTo(29), lessThanOrEqualTo(30))); + assertEquals(TokenUtil.TOKEN_TYPE_OFFLINE, offlineToken.getType()); + + String introspectionResponse = oauth.introspectAccessTokenWithClientCredential("test-app", "password", + tokenResponse.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()); + Assert.assertThat(jsonNode.get("exp").asInt() - getCurrentTime(), + allOf(greaterThanOrEqualTo(59), lessThanOrEqualTo(60))); + + } finally { + changeOfflineSessionSettings(false, prevOfflineSession[0], prevOfflineSession[1]); + changeSessionSettings(prevSession[0], prevSession[1]); + } + } + }