From 116f5f5795df4b4f1466ee34bbb592d712d52daa Mon Sep 17 00:00:00 2001 From: sebastien blanc Date: Mon, 17 Oct 2016 14:05:36 +0200 Subject: [PATCH 1/3] KEYCLOAK-3625: conform to oidc specs --- .../oidc/js/src/main/resources/keycloak.js | 39 ++++++++----------- .../main/resources/login-status-iframe.html | 11 +++--- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/adapters/oidc/js/src/main/resources/keycloak.js b/adapters/oidc/js/src/main/resources/keycloak.js index 2d8f4218c8..9e2e412047 100755 --- a/adapters/oidc/js/src/main/resources/keycloak.js +++ b/adapters/oidc/js/src/main/resources/keycloak.js @@ -29,7 +29,7 @@ var loginIframe = { enable: true, - callbackMap: [], + callbackList: [], interval: 5 }; @@ -830,33 +830,25 @@ document.body.appendChild(iframe); var messageCallback = function(event) { + + if (event.origin !== loginIframe.iframeOrigin) { return; } - try { - var data = JSON.parse(event.data); - } catch (err) { - return; + + for (i = loginIframe.callbackList.length - 1; i >= 0; --i) { + var promise = loginIframe.callbackList[i]; + if (event.data == "unchanged") { + promise.setSuccess(); + } else { + kc.clearToken(); + promise.setError(); + } + loginIframe.callbackList.splice(i, 1); } - if (!data.callbackId) { - return; - } - var promise = loginIframe.callbackMap[data.callbackId]; - if (!promise) { - return; - } - - delete loginIframe.callbackMap[data.callbackId]; - - if ((!kc.sessionId || kc.sessionId == data.session) && data.loggedIn) { - promise.setSuccess(); - } else { - kc.clearToken(); - promise.setError(); - } }; window.addEventListener('message', messageCallback, false); @@ -873,10 +865,11 @@ function checkLoginIframe() { var promise = createPromise(); - if (loginIframe.iframe && loginIframe.iframeOrigin) { + if (loginIframe.iframe && loginIframe.iframeOrigin && loginIframe.callbackList.length !== 0) { var msg = {}; msg.callbackId = createCallbackId(); - loginIframe.callbackMap[msg.callbackId] = promise; + msg.sessionId = kc.sessionId; + loginIframe.callbackList.push(promise); var origin = loginIframe.iframeOrigin; loginIframe.iframe.contentWindow.postMessage(JSON.stringify(msg), origin); } else { diff --git a/adapters/oidc/js/src/main/resources/login-status-iframe.html b/adapters/oidc/js/src/main/resources/login-status-iframe.html index fe7eda1cc5..895f2d787c 100755 --- a/adapters/oidc/js/src/main/resources/login-status-iframe.html +++ b/adapters/oidc/js/src/main/resources/login-status-iframe.html @@ -35,14 +35,15 @@ } var data = JSON.parse(event.data); - data.loggedIn = false; var cookie = getCookie('KEYCLOAK_SESSION'); - if (cookie) { - data.loggedIn = true; - data.session = cookie; + var state; + if (!data.sessionId || data.sessionId == cookie) { + state = "unchanged"; + } else { + state = "changed"; } - event.source.postMessage(JSON.stringify(data), + event.source.postMessage(state, event.origin); } window.addEventListener("message", receiveMessage, false); From 32df5225cf290853304bfd517ac91faff2c01f4f Mon Sep 17 00:00:00 2001 From: sebastien blanc Date: Tue, 18 Oct 2016 11:50:51 +0200 Subject: [PATCH 2/3] add check on list size --- adapters/oidc/js/src/main/resources/keycloak.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/adapters/oidc/js/src/main/resources/keycloak.js b/adapters/oidc/js/src/main/resources/keycloak.js index 9e2e412047..2839d750ba 100755 --- a/adapters/oidc/js/src/main/resources/keycloak.js +++ b/adapters/oidc/js/src/main/resources/keycloak.js @@ -865,13 +865,15 @@ function checkLoginIframe() { var promise = createPromise(); - if (loginIframe.iframe && loginIframe.iframeOrigin && loginIframe.callbackList.length !== 0) { + if (loginIframe.iframe && loginIframe.iframeOrigin ) { var msg = {}; msg.callbackId = createCallbackId(); msg.sessionId = kc.sessionId; loginIframe.callbackList.push(promise); var origin = loginIframe.iframeOrigin; - loginIframe.iframe.contentWindow.postMessage(JSON.stringify(msg), origin); + if(loginIframe.callbackList.length == 1) { + loginIframe.iframe.contentWindow.postMessage(JSON.stringify(msg), origin); + } } else { promise.setSuccess(); } From 1b24d2edd807583fb5581b1c37614c16eb18af09 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Wed, 19 Oct 2016 12:07:31 +0200 Subject: [PATCH 3/3] KEYCLOAK-3625 More work on the issue --- .../oidc/js/src/main/resources/keycloak.js | 23 ++--- .../main/resources/login-status-iframe.html | 80 +++++++++++---- .../protocol/oidc/OIDCWellKnownProvider.java | 1 + .../endpoints/LoginStatusIframeEndpoint.java | 98 +++++++------------ .../OIDCConfigurationRepresentation.java | 11 +++ .../oauth/LoginStatusIframeEndpointTest.java | 57 ++++++++++- 6 files changed, 175 insertions(+), 95 deletions(-) diff --git a/adapters/oidc/js/src/main/resources/keycloak.js b/adapters/oidc/js/src/main/resources/keycloak.js index 2839d750ba..01f0523511 100755 --- a/adapters/oidc/js/src/main/resources/keycloak.js +++ b/adapters/oidc/js/src/main/resources/keycloak.js @@ -824,32 +824,31 @@ setTimeout(check, loginIframe.interval * 1000); } - var src = getRealmUrl() + '/protocol/openid-connect/login-status-iframe.html?client_id=' + encodeURIComponent(kc.clientId) + '&origin=' + getOrigin(); + var src = getRealmUrl() + '/protocol/openid-connect/login-status-iframe.html'; iframe.setAttribute('src', src ); iframe.style.display = 'none'; document.body.appendChild(iframe); var messageCallback = function(event) { - - if (event.origin !== loginIframe.iframeOrigin) { return; } + if (event.data != "unchanged") { + kc.clearToken(); + } - for (i = loginIframe.callbackList.length - 1; i >= 0; --i) { + for (var i = loginIframe.callbackList.length - 1; i >= 0; --i) { var promise = loginIframe.callbackList[i]; if (event.data == "unchanged") { promise.setSuccess(); } else { - kc.clearToken(); promise.setError(); } loginIframe.callbackList.splice(i, 1); } - - }; + window.addEventListener('message', messageCallback, false); var check = function() { @@ -866,14 +865,12 @@ var promise = createPromise(); if (loginIframe.iframe && loginIframe.iframeOrigin ) { - var msg = {}; - msg.callbackId = createCallbackId(); - msg.sessionId = kc.sessionId; + var msg = kc.clientId + ' ' + kc.sessionId; loginIframe.callbackList.push(promise); var origin = loginIframe.iframeOrigin; - if(loginIframe.callbackList.length == 1) { - loginIframe.iframe.contentWindow.postMessage(JSON.stringify(msg), origin); - } + if (loginIframe.callbackList.length == 1) { + loginIframe.iframe.contentWindow.postMessage(msg, origin); + } } else { promise.setSuccess(); } diff --git a/adapters/oidc/js/src/main/resources/login-status-iframe.html b/adapters/oidc/js/src/main/resources/login-status-iframe.html index 895f2d787c..8794d54795 100755 --- a/adapters/oidc/js/src/main/resources/login-status-iframe.html +++ b/adapters/oidc/js/src/main/resources/login-status-iframe.html @@ -15,10 +15,56 @@ ~ limitations under the License. --> + + \ No newline at end of file + + + \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java index c2f4296769..0485e3f11d 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java @@ -90,6 +90,7 @@ public class OIDCWellKnownProvider implements WellKnownProvider { config.setUserinfoEndpoint(uriBuilder.clone().path(OIDCLoginProtocolService.class, "issueUserInfo").build(realm.getName(), OIDCLoginProtocol.LOGIN_PROTOCOL).toString()); config.setLogoutEndpoint(uriBuilder.clone().path(OIDCLoginProtocolService.class, "logout").build(realm.getName(), OIDCLoginProtocol.LOGIN_PROTOCOL).toString()); config.setJwksUri(uriBuilder.clone().path(OIDCLoginProtocolService.class, "certs").build(realm.getName(), OIDCLoginProtocol.LOGIN_PROTOCOL).toString()); + config.setCheckSessionIframe(uriBuilder.clone().path(OIDCLoginProtocolService.class, "getLoginStatusIframe").build(realm.getName(), OIDCLoginProtocol.LOGIN_PROTOCOL).toString()); config.setRegistrationEndpoint(RealmsResource.clientRegistrationUrl(uriInfo).path(ClientRegistrationService.class, "provider").build(realm.getName(), OIDCClientRegistrationProviderFactory.ID).toString()); config.setIdTokenSigningAlgValuesSupported(DEFAULT_ID_TOKEN_SIGNING_ALG_VALUES_SUPPORTED); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java index b41f52d721..c46ba108d3 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java @@ -17,27 +17,25 @@ package org.keycloak.protocol.oidc.endpoints; -import org.jboss.resteasy.spi.NotFoundException; -import org.keycloak.Config; -import org.keycloak.common.util.StreamUtil; import org.keycloak.common.util.UriUtils; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.protocol.oidc.utils.RedirectUtils; +import org.keycloak.models.UserSessionModel; +import org.keycloak.protocol.oidc.utils.WebOriginsUtils; +import org.keycloak.services.util.CacheControlUtil; import org.keycloak.services.util.P3PHelper; import javax.ws.rs.GET; +import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.CacheControl; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; -import java.io.IOException; import java.io.InputStream; +import java.util.Set; /** * @author Stian Thorgersen @@ -58,62 +56,40 @@ public class LoginStatusIframeEndpoint { @GET @Produces(MediaType.TEXT_HTML) - public Response getLoginStatusIframe(@QueryParam("client_id") String client_id, - @QueryParam("origin") String origin) { - if (client_id == null || origin == null) { - throw new WebApplicationException(Response.Status.BAD_REQUEST); - } - - if (!UriUtils.isOrigin(origin)) { - throw new WebApplicationException(Response.Status.BAD_REQUEST); - } - - ClientModel client = realm.getClientByClientId(client_id); - if (client == null) { - throw new WebApplicationException(Response.Status.BAD_REQUEST); - } - - InputStream is = getClass().getClassLoader().getResourceAsStream("login-status-iframe.html"); - if (is == null) throw new NotFoundException("Could not find login-status-iframe.html "); - - boolean valid = false; - for (String o : client.getWebOrigins()) { - if (o.equals("*") || o.equals(origin)) { - valid = true; - break; - } - } - - for (String r : RedirectUtils.resolveValidRedirects(uriInfo, client.getRootUrl(), client.getRedirectUris())) { - int i = r.indexOf('/', 8); - if (i != -1) { - r = r.substring(0, i); - } - - if (r.equals(origin)) { - valid = true; - break; - } - } - - if (!valid) { - throw new WebApplicationException(Response.Status.BAD_REQUEST); - } - - try { - String file = StreamUtil.readString(is); - file = file.replace("ORIGIN", origin); - + public Response getLoginStatusIframe() { + InputStream resource = getClass().getClassLoader().getResourceAsStream("login-status-iframe.html"); + if (resource != null) { P3PHelper.addP3PHeader(session); - - CacheControl cacheControl = new CacheControl(); - cacheControl.setNoTransform(false); - cacheControl.setMaxAge(Config.scope("theme").getInt("staticMaxAge", -1)); - - return Response.ok(file).cacheControl(cacheControl).build(); - } catch (IOException e) { - throw new WebApplicationException(e, Response.Status.BAD_REQUEST); + return Response.ok(resource).type(MediaType.TEXT_HTML_TYPE).cacheControl(CacheControlUtil.getDefaultCacheControl()).build(); + } else { + return Response.status(Response.Status.NOT_FOUND).build(); } } + @GET + @Path("init") + public Response preCheck(@QueryParam("client_id") String clientId, @QueryParam("origin") String origin, @QueryParam("session_state") String sessionState) { + try { + RealmModel realm = session.getContext().getRealm(); + String sessionId = sessionState.split("/")[2]; + UserSessionModel userSession = session.sessions().getUserSession(realm, sessionId); + if (userSession == null) { + return Response.status(Response.Status.NOT_FOUND).build(); + } + + ClientModel client = session.realms().getClientByClientId(clientId, realm); + if (client != null) { + Set validWebOrigins = WebOriginsUtils.resolveValidWebOrigins(uriInfo, client); + validWebOrigins.add(UriUtils.getOrigin(uriInfo.getRequestUri())); + + if (validWebOrigins.contains(origin)) { + return Response.noContent().build(); + } + } + } catch (Throwable t) { + } + + return Response.status(Response.Status.FORBIDDEN).build(); + } + } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/representations/OIDCConfigurationRepresentation.java b/services/src/main/java/org/keycloak/protocol/oidc/representations/OIDCConfigurationRepresentation.java index 181e0d28df..cb94c1cab0 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/representations/OIDCConfigurationRepresentation.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/representations/OIDCConfigurationRepresentation.java @@ -52,6 +52,9 @@ public class OIDCConfigurationRepresentation { @JsonProperty("jwks_uri") private String jwksUri; + @JsonProperty("check_session_iframe") + private String checkSessionIframe; + @JsonProperty("grant_types_supported") private List grantTypesSupported; @@ -150,6 +153,14 @@ public class OIDCConfigurationRepresentation { this.jwksUri = jwksUri; } + public String getCheckSessionIframe() { + return checkSessionIframe; + } + + public void setCheckSessionIframe(String checkSessionIframe) { + this.checkSessionIframe = checkSessionIframe; + } + public String getLogoutEndpoint() { return logoutEndpoint; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java index 437e7d88ab..2d0a269c8e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java @@ -53,7 +53,7 @@ import static org.junit.Assert.assertTrue; public class LoginStatusIframeEndpointTest extends AbstractKeycloakTest { @Test - public void checkIframeP3PHeader() throws IOException { + public void checkIframe() throws IOException { CookieStore cookieStore = new BasicCookieStore(); CloseableHttpClient client = HttpClients.custom().setDefaultCookieStore(cookieStore).build(); @@ -115,17 +115,66 @@ public class LoginStatusIframeEndpointTest extends AbstractKeycloakTest { } assertNotNull(sessionCookie); - get = new HttpGet( - suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html?client_id=" + Constants.ADMIN_CONSOLE_CLIENT_ID + "&origin=" + suiteContext.getAuthServerInfo().getContextRoot()); + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html"); response = client.execute(get); assertEquals(200, response.getStatusLine().getStatusCode()); s = IOUtils.toString(response.getEntity().getContent()); - assertTrue(s.contains("function getCookie(cname)")); + assertTrue(s.contains("function getCookie()")); assertEquals("CP=\"This is not a P3P policy!\"", response.getFirstHeader("P3P").getValue()); response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init"); + response = client.execute(get); + assertEquals(403, response.getStatusLine().getStatusCode()); + response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init?" + + "client_id=invalid" + + "&session_state=" + sessionCookie.getValue() + + "&origin=" + suiteContext.getAuthServerInfo().getContextRoot() + ); + response = client.execute(get); + assertEquals(403, response.getStatusLine().getStatusCode()); + response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init?" + + "client_id=" + Constants.ADMIN_CONSOLE_CLIENT_ID + + "&session_state=invalid" + + "&origin=" + suiteContext.getAuthServerInfo().getContextRoot() + ); + response = client.execute(get); + assertEquals(403, response.getStatusLine().getStatusCode()); + response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init?" + + "client_id=" + Constants.ADMIN_CONSOLE_CLIENT_ID + + "&session_state=" + sessionCookie.getValue() + + "&origin=http://invalid" + ); + response = client.execute(get); + assertEquals(403, response.getStatusLine().getStatusCode()); + response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init?" + + "client_id=" + Constants.ADMIN_CONSOLE_CLIENT_ID + + "&session_state=master/random/random" + + "&origin=" + suiteContext.getAuthServerInfo().getContextRoot() + ); + response = client.execute(get); + assertEquals(404, response.getStatusLine().getStatusCode()); + response.close(); + + get = new HttpGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/realms/master/protocol/openid-connect/login-status-iframe.html/init?" + + "client_id=" + Constants.ADMIN_CONSOLE_CLIENT_ID + + "&session_state=" + sessionCookie.getValue() + + "&origin=" + suiteContext.getAuthServerInfo().getContextRoot() + ); + response = client.execute(get); + assertEquals(204, response.getStatusLine().getStatusCode()); + response.close(); } finally { client.close(); }