From f4ddfe4a528b1e569a6a0f960aecbcf5f096625f Mon Sep 17 00:00:00 2001 From: mposolda Date: Wed, 20 Jul 2016 17:11:50 +0200 Subject: [PATCH] KEYCLOAK-3318 Support for prompt=login. More tests for prompt parameter --- .../java/org/keycloak/util/TokenUtil.java | 16 ++ .../browser/CookieAuthenticator.java | 14 +- .../protocol/oidc/OIDCLoginProtocol.java | 5 + .../oidc/endpoints/AuthorizationEndpoint.java | 2 +- .../keycloak/testsuite/forms/LoginTest.java | 19 -- .../oidc/OIDCAdvancedRequestParamsTest.java | 166 +++++++++++++++++- .../broker-test/test-realm-with-broker.json | 2 - 7 files changed, 197 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/org/keycloak/util/TokenUtil.java b/core/src/main/java/org/keycloak/util/TokenUtil.java index 6168019443..f649b5ee49 100644 --- a/core/src/main/java/org/keycloak/util/TokenUtil.java +++ b/core/src/main/java/org/keycloak/util/TokenUtil.java @@ -69,6 +69,22 @@ public class TokenUtil { } + public static boolean hasPrompt(String promptParam, String targetPrompt) { + if (promptParam == null || targetPrompt == null) { + return false; + } + + String[] prompts = promptParam.split(" "); + for (String prompt : prompts) { + if (targetPrompt.equals(prompt)) { + return true; + } + } + return false; + } + + + /** * Return refresh token or offline token * diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java index ca6d7e6f5d..24d708acab 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java @@ -28,6 +28,7 @@ import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.util.TokenUtil; /** * @author Bill Burke @@ -49,8 +50,8 @@ public class CookieAuthenticator implements Authenticator { if (authResult == null) { context.attempted(); } else { - // Cookie re-authentication is skipped if authTime is too old. - if (isAuthTimeExpired(authResult.getSession(), context.getClientSession())) { + // Cookie re-authentication is skipped if re-authentication is required + if (requireReauthentication(authResult.getSession(), context.getClientSession())) { context.attempted(); } else { ClientSessionModel clientSession = context.getClientSession(); @@ -83,6 +84,15 @@ public class CookieAuthenticator implements Authenticator { } + protected boolean requireReauthentication(UserSessionModel userSession, ClientSessionModel clientSession) { + return isPromptLogin(clientSession) || isAuthTimeExpired(userSession, clientSession); + } + + protected boolean isPromptLogin(ClientSessionModel clientSession) { + String prompt = clientSession.getNote(OIDCLoginProtocol.PROMPT_PARAM); + return TokenUtil.hasPrompt(prompt, OIDCLoginProtocol.PROMPT_VALUE_LOGIN); + } + protected boolean isAuthTimeExpired(UserSessionModel userSession, ClientSessionModel clientSession) { String authTime = userSession.getNote(AuthenticationManager.AUTH_TIME); String maxAge = clientSession.getNote(OIDCLoginProtocol.MAX_AGE_PARAM); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index e6520db0a1..39dc288d8c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -65,6 +65,11 @@ public class OIDCLoginProtocol implements LoginProtocol { public static final String RESPONSE_MODE_PARAM = "response_mode"; + public static final String PROMPT_VALUE_NONE = "none"; + public static final String PROMPT_VALUE_LOGIN = "login"; + public static final String PROMPT_VALUE_CONSENT = "consent"; + public static final String PROMPT_VALUE_SELECT_ACCOUNT = "select_account"; + private static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER; protected KeycloakSession session; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index f754c3f48d..2092079de8 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -382,7 +382,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { this.event.event(EventType.LOGIN); clientSession.setNote(Details.AUTH_TYPE, CODE_AUTH_TYPE); - return handleBrowserAuthenticationRequest(clientSession, new OIDCLoginProtocol(session, realm, uriInfo, headers, event), prompt != null && prompt.equals("none"), false); + return handleBrowserAuthenticationRequest(clientSession, new OIDCLoginProtocol(session, realm, uriInfo, headers, event), TokenUtil.hasPrompt(prompt, OIDCLoginProtocol.PROMPT_VALUE_NONE), false); } private Response buildRegister() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java index f3ed5f609a..accbd37e33 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java @@ -327,25 +327,6 @@ public class LoginTest extends TestRealmKeycloakTest { events.expectLogin().user(userId).assertEvent(); } - @Test - public void loginPromptNone() { - driver.navigate().to(oauth.getLoginFormUrl().toString() + "&prompt=none"); - - assertFalse(loginPage.isCurrent()); - assertTrue(appPage.isCurrent()); - - loginPage.open(); - loginPage.login("login-test", "password"); - Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); - - events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); - - driver.navigate().to(oauth.getLoginFormUrl().toString() + "&prompt=none"); - Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); - - events.expectLogin().user(userId).removeDetail(Details.USERNAME).assertEvent(); - } - private void setPasswordPolicy(String policy) { RealmRepresentation realmRep = adminClient.realm("test").toRepresentation(); realmRep.setPasswordPolicy(policy); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCAdvancedRequestParamsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCAdvancedRequestParamsTest.java index e6a72a02da..bdc582ce06 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCAdvancedRequestParamsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCAdvancedRequestParamsTest.java @@ -19,23 +19,31 @@ package org.keycloak.testsuite.oidc; import java.util.List; + +import org.jboss.arquillian.graphene.page.Page; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.keycloak.OAuth2Constants; +import org.keycloak.OAuthErrorException; import org.keycloak.common.util.Time; import org.keycloak.events.Details; +import org.keycloak.models.Constants; import org.keycloak.representations.IDToken; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.TestRealmKeycloakTest; import org.keycloak.testsuite.admin.AbstractAdminTest; +import org.keycloak.testsuite.pages.AccountUpdateProfilePage; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.util.ClientManager; import org.keycloak.testsuite.util.OAuthClient; -import org.keycloak.testsuite.util.RealmBuilder; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * Test for supporting advanced parameters of OIDC specs (max_age, nonce, prompt, ...) @@ -47,6 +55,18 @@ public class OIDCAdvancedRequestParamsTest extends TestRealmKeycloakTest { @Rule public AssertEvents events = new AssertEvents(this); + @Page + protected AppPage appPage; + + @Page + protected LoginPage loginPage; + + @Page + protected AccountUpdateProfilePage profilePage; + + @Page + protected OAuthGrantPage grantPage; + @Override public void configureTestRealm(RealmRepresentation testRealm) { @@ -71,6 +91,9 @@ public class OIDCAdvancedRequestParamsTest extends TestRealmKeycloakTest { testRealms.add(realm); } + + // Max_age + @Test public void testMaxAge1() { // Open login form and login successfully @@ -131,4 +154,141 @@ public class OIDCAdvancedRequestParamsTest extends TestRealmKeycloakTest { Assert.assertEquals(authTime, authTimeUpdated); } + + // Prompt + + @Test + public void promptNoneNotLogged() { + // Send request with prompt=none + driver.navigate().to(oauth.getLoginFormUrl() + "&prompt=none"); + + assertFalse(loginPage.isCurrent()); + assertTrue(appPage.isCurrent()); + + events.assertEmpty(); + + // Assert error response was sent because not logged in + OAuthClient.AuthorizationCodeResponse resp = new OAuthClient.AuthorizationCodeResponse(oauth); + Assert.assertNull(resp.getCode()); + Assert.assertEquals(OAuthErrorException.LOGIN_REQUIRED, resp.getError()); + + + } + + @Test + public void promptNoneSuccess() { + // Login user + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + EventRepresentation loginEvent = events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent(); + IDToken idToken = sendTokenRequestAndGetIDToken(loginEvent); + int authTime = idToken.getAuthTime(); + + // Set time offset + setTimeOffset(10); + + // Assert user still logged with previous authTime + driver.navigate().to(oauth.getLoginFormUrl() + "&prompt=none"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + loginEvent = events.expectLogin().removeDetail(Details.USERNAME).assertEvent(); + idToken = sendTokenRequestAndGetIDToken(loginEvent); + int authTime2 = idToken.getAuthTime(); + + Assert.assertEquals(authTime, authTime2); + } + + + // Prompt=none with consent required for client + @Test + public void promptNoneConsentRequired() throws Exception { + // Require consent + ClientManager.realm(adminClient.realm("test")).clientId("test-app").consentRequired(true); + + try { + // login to account mgmt. + profilePage.open(); + assertTrue(loginPage.isCurrent()); + loginPage.login("test-user@localhost", "password"); + profilePage.assertCurrent(); + + events.expectLogin().client(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID) + .removeDetail(Details.REDIRECT_URI) + .detail(Details.USERNAME, "test-user@localhost").assertEvent(); + + // Assert error shown when trying prompt=none and consent not yet retrieved + driver.navigate().to(oauth.getLoginFormUrl() + "&prompt=none"); + assertTrue(appPage.isCurrent()); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + OAuthClient.AuthorizationCodeResponse resp = new OAuthClient.AuthorizationCodeResponse(oauth); + Assert.assertNull(resp.getCode()); + Assert.assertEquals(OAuthErrorException.INTERACTION_REQUIRED, resp.getError()); + + // Confirm consent + driver.navigate().to(oauth.getLoginFormUrl()); + grantPage.assertCurrent(); + grantPage.accept(); + + events.expectLogin() + .detail(Details.USERNAME, "test-user@localhost") + .detail(Details.CONSENT, Details.CONSENT_VALUE_CONSENT_GRANTED) + .assertEvent(); + + // Consent not required anymore. Login with prompt=none should success + driver.navigate().to(oauth.getLoginFormUrl() + "&prompt=none"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + resp = new OAuthClient.AuthorizationCodeResponse(oauth); + Assert.assertNotNull(resp.getCode()); + Assert.assertNull(resp.getError()); + + events.expectLogin() + .detail(Details.USERNAME, "test-user@localhost") + .detail(Details.CONSENT, Details.CONSENT_VALUE_PERSISTED_CONSENT) + .assertEvent(); + + } finally { + // revert require consent + ClientManager.realm(adminClient.realm("test")).clientId("test-app").consentRequired(false); + } + } + + + // prompt=login + @Test + public void promptLogin() { + // Login user + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + EventRepresentation loginEvent = events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent(); + IDToken idToken = sendTokenRequestAndGetIDToken(loginEvent); + int authTime = idToken.getAuthTime(); + + // Set time offset + setTimeOffset(10); + + // Assert need to re-authenticate with prompt=login + driver.navigate().to(oauth.getLoginFormUrl() + "&prompt=login"); + + loginPage.assertCurrent(); + loginPage.login("test-user@localhost", "password"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + loginEvent = events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent(); + idToken = sendTokenRequestAndGetIDToken(loginEvent); + int authTimeUpdated = idToken.getAuthTime(); + + // Assert that authTime was updated + Assert.assertTrue(authTime + 10 <= authTimeUpdated); + + } + + + // prompt=consent + } diff --git a/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json b/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json index 84bcaf8ee5..99c0245349 100755 --- a/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json +++ b/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json @@ -178,7 +178,6 @@ "config": { "clientId": "broker-app", "clientSecret": "secret", - "prompt": "login", "authorizationUrl": "http://localhost:8082/auth/realms/realm-with-oidc-identity-provider/protocol/openid-connect/auth", "tokenUrl": "http://localhost:8082/auth/realms/realm-with-oidc-identity-provider/protocol/openid-connect/token", "userInfoUrl": "http://localhost:8082/auth/realms/realm-with-oidc-identity-provider/protocol/openid-connect/userinfo", @@ -196,7 +195,6 @@ "config": { "clientId": "broker-app", "clientSecret": "secret", - "prompt": "login", "authorizationUrl": "http://localhost:8082/auth/realms/realm-with-oidc-idp-property-mappers/protocol/openid-connect/auth", "tokenUrl": "http://localhost:8082/auth/realms/realm-with-oidc-idp-property-mappers/protocol/openid-connect/token", "userInfoUrl": "http://localhost:8082/auth/realms/realm-with-oidc-idp-property-mappers/protocol/openid-connect/userinfo",