From 61d1c498ef501f57d5701432896ad5b1f46d9d4e Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Thu, 7 Apr 2016 13:46:30 +0200 Subject: [PATCH 01/26] KEYCLOAK-2751 Separate HTTP status codes for REST API errors --- ...nstallationClientRegistrationProvider.java | 41 +----- .../ClientRegistrationAuth.java | 133 ++++++++++++------ .../ClientRegistrationTokenUtils.java | 15 +- .../client/AdapterInstallationConfigTest.java | 8 +- .../client/ClientRegistrationTest.java | 18 ++- .../client/InitialAccessTokenTest.java | 6 +- .../client/RegistrationAccessTokenTest.java | 16 +-- 7 files changed, 129 insertions(+), 108 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/clientregistration/AdapterInstallationClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/AdapterInstallationClientRegistrationProvider.java index 65a4339423..86a71e33fd 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/AdapterInstallationClientRegistrationProvider.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/AdapterInstallationClientRegistrationProvider.java @@ -17,18 +17,17 @@ package org.keycloak.services.clientregistration; -import org.keycloak.authentication.AuthenticationProcessor; -import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; -import org.keycloak.protocol.oidc.utils.AuthorizeClientUtil; -import org.keycloak.services.ForbiddenException; import org.keycloak.services.managers.ClientManager; import org.keycloak.services.managers.RealmManager; -import javax.ws.rs.*; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -52,12 +51,7 @@ public class AdapterInstallationClientRegistrationProvider implements ClientRegi event.event(EventType.CLIENT_INFO); ClientModel client = session.getContext().getRealm().getClientByClientId(clientId); - - if (auth.isAuthenticated()) { - auth.requireView(client); - } else { - authenticateClient(client); - } + auth.requireView(client); ClientManager clientManager = new ClientManager(new RealmManager(session)); Object rep = clientManager.toInstallationRepresentation(session.getContext().getRealm(), client, session.getContext().getAuthServerUrl()); @@ -80,29 +74,4 @@ public class AdapterInstallationClientRegistrationProvider implements ClientRegi public void close() { } - private void authenticateClient(ClientModel client) { - if (client.isPublicClient()) { - return; - } - - AuthenticationProcessor processor = AuthorizeClientUtil.getAuthenticationProcessor(session, event); - - Response response = processor.authenticateClient(); - if (response != null) { - event.client(client.getClientId()).error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); - } - - ClientModel authClient = processor.getClient(); - if (client == null) { - event.client(client.getClientId()).error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); - } - - if (!authClient.getClientId().equals(client.getClientId())) { - event.client(client.getClientId()).error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); - } - } - } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java index 5dc72854a1..5564ef282d 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java @@ -17,20 +17,28 @@ package org.keycloak.services.clientregistration; -import com.sun.xml.bind.v2.runtime.reflect.opt.Const; +import org.jboss.resteasy.spi.Failure; +import org.jboss.resteasy.spi.NotFoundException; import org.jboss.resteasy.spi.UnauthorizedException; import org.keycloak.Config; +import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.common.util.Time; import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; -import org.keycloak.models.*; +import org.keycloak.models.AdminRoles; +import org.keycloak.models.ClientInitialAccessModel; +import org.keycloak.models.ClientModel; +import org.keycloak.models.Constants; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.protocol.oidc.utils.AuthorizeClientUtil; import org.keycloak.representations.JsonWebToken; import org.keycloak.services.ForbiddenException; import org.keycloak.util.TokenUtil; import javax.ws.rs.core.HttpHeaders; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; -import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -49,8 +57,6 @@ public class ClientRegistrationAuth { public ClientRegistrationAuth(KeycloakSession session, EventBuilder event) { this.session = session; this.event = event; - - init(); } private void init() { @@ -67,41 +73,39 @@ public class ClientRegistrationAuth { return; } - jwt = ClientRegistrationTokenUtils.parseToken(realm, uri, split[1]); + jwt = ClientRegistrationTokenUtils.verifyToken(realm, uri, split[1]); + if (jwt == null) { + throw unauthorized(); + } if (isInitialAccessToken()) { initialAccessModel = session.sessions().getClientInitialAccessModel(session.getContext().getRealm(), jwt.getId()); if (initialAccessModel == null) { - throw new ForbiddenException(); + throw unauthorized(); } } } - public boolean isAuthenticated() { - return jwt != null; - } - - public boolean isBearerToken() { - return TokenUtil.TOKEN_TYPE_BEARER.equals(jwt.getType()); + private boolean isBearerToken() { + return jwt != null && TokenUtil.TOKEN_TYPE_BEARER.equals(jwt.getType()); } public boolean isInitialAccessToken() { - return ClientRegistrationTokenUtils.TYPE_INITIAL_ACCESS_TOKEN.equals(jwt.getType()); + return jwt != null && ClientRegistrationTokenUtils.TYPE_INITIAL_ACCESS_TOKEN.equals(jwt.getType()); } public boolean isRegistrationAccessToken() { - return ClientRegistrationTokenUtils.TYPE_REGISTRATION_ACCESS_TOKEN.equals(jwt.getType()); + return jwt != null && ClientRegistrationTokenUtils.TYPE_REGISTRATION_ACCESS_TOKEN.equals(jwt.getType()); } public void requireCreate() { - if (!isAuthenticated()) { - event.error(Errors.NOT_ALLOWED); - throw new UnauthorizedException(); - } + init(); if (isBearerToken()) { if (hasRole(AdminRoles.MANAGE_CLIENTS, AdminRoles.CREATE_CLIENT)) { return; + } else { + throw forbidden(); } } else if (isInitialAccessToken()) { if (initialAccessModel.getRemainingCount() > 0) { @@ -111,58 +115,55 @@ public class ClientRegistrationAuth { } } - event.error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); + throw unauthorized(); } public void requireView(ClientModel client) { - if (!isAuthenticated()) { - event.error(Errors.NOT_ALLOWED); - throw new UnauthorizedException(); - } - - if (client == null) { - event.error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); - } + init(); if (isBearerToken()) { if (hasRole(AdminRoles.MANAGE_CLIENTS, AdminRoles.VIEW_CLIENTS)) { + if (client == null) { + throw notFound(); + } return; + } else { + throw forbidden(); } } else if (isRegistrationAccessToken()) { - if (client.getRegistrationToken() != null && client.getRegistrationToken().equals(jwt.getId())) { + if (client.getRegistrationToken() != null && client != null && client.getRegistrationToken().equals(jwt.getId())) { + return; + } + } else if (isInitialAccessToken()) { + throw unauthorized(); + } else { + if (authenticateClient(client)) { return; } } - event.error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); + throw unauthorized(); } public void requireUpdate(ClientModel client) { - if (!isAuthenticated()) { - event.error(Errors.NOT_ALLOWED); - throw new UnauthorizedException(); - } - - if (client == null) { - event.error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); - } + init(); if (isBearerToken()) { if (hasRole(AdminRoles.MANAGE_CLIENTS)) { + if (client == null) { + throw notFound(); + } return; + } else { + throw forbidden(); } } else if (isRegistrationAccessToken()) { - if (client.getRegistrationToken() != null && client.getRegistrationToken().equals(jwt.getId())) { + if (client.getRegistrationToken() != null && client != null && client.getRegistrationToken().equals(jwt.getId())) { return; } } - event.error(Errors.NOT_ALLOWED); - throw new ForbiddenException(); + throw unauthorized(); } public ClientInitialAccessModel getInitialAccessModel() { @@ -207,4 +208,46 @@ public class ClientRegistrationAuth { } } + private boolean authenticateClient(ClientModel client) { + if (client.isPublicClient()) { + return true; + } + + AuthenticationProcessor processor = AuthorizeClientUtil.getAuthenticationProcessor(session, event); + + Response response = processor.authenticateClient(); + if (response != null) { + event.client(client.getClientId()).error(Errors.NOT_ALLOWED); + throw unauthorized(); + } + + ClientModel authClient = processor.getClient(); + if (client == null) { + event.client(client.getClientId()).error(Errors.NOT_ALLOWED); + throw unauthorized(); + } + + if (!authClient.getClientId().equals(client.getClientId())) { + event.client(client.getClientId()).error(Errors.NOT_ALLOWED); + throw unauthorized(); + } + + return true; + } + + private Failure unauthorized() { + event.error(Errors.NOT_ALLOWED); + return new UnauthorizedException(); + } + + private Failure forbidden() { + event.error(Errors.NOT_ALLOWED); + return new ForbiddenException(); + } + + private Failure notFound() { + event.error(Errors.NOT_ALLOWED); + return new NotFoundException("Client not found"); + } + } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationTokenUtils.java b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationTokenUtils.java index 2cd74ba2e7..2fe65cc8fd 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationTokenUtils.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationTokenUtils.java @@ -28,7 +28,6 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.JsonWebToken; -import org.keycloak.services.ForbiddenException; import org.keycloak.services.Urls; import org.keycloak.util.TokenUtil; @@ -57,37 +56,37 @@ public class ClientRegistrationTokenUtils { return createToken(realm, uri, model.getId(), TYPE_INITIAL_ACCESS_TOKEN, model.getExpiration() > 0 ? model.getTimestamp() + model.getExpiration() : 0); } - public static JsonWebToken parseToken(RealmModel realm, UriInfo uri, String token) { + public static JsonWebToken verifyToken(RealmModel realm, UriInfo uri, String token) { JWSInput input; try { input = new JWSInput(token); } catch (JWSInputException e) { - throw new ForbiddenException(e); + return null; } if (!RSAProvider.verify(input, realm.getPublicKey())) { - throw new ForbiddenException("Invalid signature"); + return null; } JsonWebToken jwt; try { jwt = input.readJsonContent(JsonWebToken.class); } catch (JWSInputException e) { - throw new ForbiddenException(e); + return null; } if (!getIssuer(realm, uri).equals(jwt.getIssuer())) { - throw new ForbiddenException("Issuer doesn't match"); + return null; } if (!jwt.isActive()) { - throw new ForbiddenException("Expired token"); + return null; } if (!(TokenUtil.TOKEN_TYPE_BEARER.equals(jwt.getType()) || TYPE_INITIAL_ACCESS_TOKEN.equals(jwt.getType()) || TYPE_REGISTRATION_ACCESS_TOKEN.equals(jwt.getType()))) { - throw new ForbiddenException("Invalid token type"); + return null; } return jwt; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AdapterInstallationConfigTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AdapterInstallationConfigTest.java index 427202548d..0a4fe4bea7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AdapterInstallationConfigTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AdapterInstallationConfigTest.java @@ -103,9 +103,9 @@ public class AdapterInstallationConfigTest extends AbstractClientRegistrationTes try { reg.getAdapterConfig(client.getClientId()); - fail("Expected 403"); + fail("Expected 401"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } @@ -115,9 +115,9 @@ public class AdapterInstallationConfigTest extends AbstractClientRegistrationTes try { reg.getAdapterConfig(client2.getClientId()); - fail("Expected 403"); + fail("Expected 401"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java index 70bfed0e0a..0ef5e26c1e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java @@ -126,8 +126,14 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest { @Test public void getClientNotFound() throws ClientRegistrationException { authManageClients(); + assertNull(reg.get("invalid")); + } + + @Test + public void getClientNotFoundNoAccess() throws ClientRegistrationException { + authNoAccess(); try { - reg.get(CLIENT_ID); + reg.get("invalid"); fail("Expected 403"); } catch (ClientRegistrationException e) { assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); @@ -181,10 +187,14 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest { public void updateClientNotFound() throws ClientRegistrationException { authManageClients(); try { - updateClient(); - fail("Expected 403"); + ClientRepresentation client = new ClientRepresentation(); + client.setClientId("invalid"); + + reg.update(client); + + fail("Expected 404"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(404, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/InitialAccessTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/InitialAccessTokenTest.java index 4f4d204b11..ad33f52e9e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/InitialAccessTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/InitialAccessTokenTest.java @@ -58,7 +58,7 @@ public class InitialAccessTokenTest extends AbstractClientRegistrationTest { try { reg.create(rep); } catch (ClientRegistrationException e) { - Assert.assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + Assert.assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } @@ -79,7 +79,7 @@ public class InitialAccessTokenTest extends AbstractClientRegistrationTest { try { reg.create(rep); } catch (ClientRegistrationException e) { - Assert.assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + Assert.assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } @@ -113,7 +113,7 @@ public class InitialAccessTokenTest extends AbstractClientRegistrationTest { try { reg.create(rep); } catch (ClientRegistrationException e) { - Assert.assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + Assert.assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/RegistrationAccessTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/RegistrationAccessTokenTest.java index c6fb960594..96a0010c93 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/RegistrationAccessTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/RegistrationAccessTokenTest.java @@ -59,8 +59,8 @@ public class RegistrationAccessTokenTest extends AbstractClientRegistrationTest try { reg.get(client.getClientId()); fail("Expected 403"); - } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + } catch (Exception e) { + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } return null; @@ -82,9 +82,9 @@ public class RegistrationAccessTokenTest extends AbstractClientRegistrationTest reg.auth(Auth.token("invalid")); try { reg.get(client.getClientId()); - fail("Expected 403"); + fail("Expected 401"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } } @@ -109,9 +109,9 @@ public class RegistrationAccessTokenTest extends AbstractClientRegistrationTest reg.auth(Auth.token("invalid")); try { reg.update(client); - fail("Expected 403"); + fail("Expected 401"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } assertEquals("http://root", getClient(client.getId()).getRootUrl()); @@ -128,9 +128,9 @@ public class RegistrationAccessTokenTest extends AbstractClientRegistrationTest reg.auth(Auth.token("invalid")); try { reg.delete(client); - fail("Expected 403"); + fail("Expected 401"); } catch (ClientRegistrationException e) { - assertEquals(403, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); + assertEquals(401, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode()); } assertNotNull(getClient(client.getId())); } From 74f98f9eb296269408e7edfab3d4e93959abefc6 Mon Sep 17 00:00:00 2001 From: Konstantin Gribov Date: Wed, 30 Mar 2016 21:42:21 +0300 Subject: [PATCH 02/26] Add service account support to Keycloak admin client Added grant_type=client_credentials support to keycloak-admin-client so `keycloak-admin-client` can be used with service client account. Fixes #KEYCLOAK-2236 --- .../org/keycloak/admin/client/Config.java | 27 +++++++++- .../org/keycloak/admin/client/Keycloak.java | 29 +++++------ .../admin/client/KeycloakBuilder.java | 43 ++++++++++++---- .../admin/client/token/TokenManager.java | 51 +++++++++++-------- 4 files changed, 103 insertions(+), 47 deletions(-) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/Config.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/Config.java index ae3edaacc8..4e628e5a4c 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/Config.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/Config.java @@ -17,6 +17,9 @@ package org.keycloak.admin.client; +import static org.keycloak.OAuth2Constants.CLIENT_CREDENTIALS; +import static org.keycloak.OAuth2Constants.PASSWORD; + /** * @author rodrigo.sasaki@icarros.com.br */ @@ -28,14 +31,21 @@ public class Config { private String password; private String clientId; private String clientSecret; + private String grantType; public Config(String serverUrl, String realm, String username, String password, String clientId, String clientSecret) { + this(serverUrl, realm, username, password, clientId, clientSecret, PASSWORD); + } + + public Config(String serverUrl, String realm, String username, String password, String clientId, String clientSecret, String grantType) { this.serverUrl = serverUrl; this.realm = realm; this.username = username; this.password = password; this.clientId = clientId; this.clientSecret = clientSecret; + this.grantType = grantType; + checkGrantType(grantType); } public String getServerUrl() { @@ -86,8 +96,23 @@ public class Config { this.clientSecret = clientSecret; } - public boolean isPublicClient(){ + public boolean isPublicClient() { return clientSecret == null; } + public String getGrantType() { + return grantType; + } + + public void setGrantType(String grantType) { + this.grantType = grantType; + checkGrantType(grantType); + } + + public static void checkGrantType(String grantType) { + if (!PASSWORD.equals(grantType) && !CLIENT_CREDENTIALS.equals(grantType)) { + throw new IllegalArgumentException("Unsupported grantType: " + grantType + + " (only " + PASSWORD + " and " + CLIENT_CREDENTIALS + " are supported)"); + } + } } diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java index 52c55619e4..5e30a664e4 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java @@ -28,25 +28,25 @@ import org.keycloak.admin.client.token.TokenManager; import java.net.URI; +import static org.keycloak.OAuth2Constants.PASSWORD; + /** * Provides a Keycloak client. By default, this implementation uses a {@link ResteasyClient RESTEasy client} with the * default {@link ResteasyClientBuilder} settings. To customize the underling client, use a {@link KeycloakBuilder} to * create a Keycloak client. * - * @see KeycloakBuilder - * * @author rodrigo.sasaki@icarros.com.br + * @see KeycloakBuilder */ public class Keycloak { - private final Config config; private final TokenManager tokenManager; private final ResteasyWebTarget target; private final ResteasyClient client; - Keycloak(String serverUrl, String realm, String username, String password, String clientId, String clientSecret, ResteasyClient resteasyClient){ - config = new Config(serverUrl, realm, username, password, clientId, clientSecret); - client = resteasyClient != null ? resteasyClient : new ResteasyClientBuilder().connectionPoolSize(10).build(); + Keycloak(String serverUrl, String realm, String username, String password, String clientId, String clientSecret, String grantType, ResteasyClient resteasyClient) { + config = new Config(serverUrl, realm, username, password, clientId, clientSecret, grantType); + client = resteasyClient != null ? resteasyClient : new ResteasyClientBuilder().build(); tokenManager = new TokenManager(config, client); @@ -55,27 +55,27 @@ public class Keycloak { target.register(new BearerAuthFilter(tokenManager)); } - public static Keycloak getInstance(String serverUrl, String realm, String username, String password, String clientId, String clientSecret){ - return new Keycloak(serverUrl, realm, username, password, clientId, clientSecret, null); + public static Keycloak getInstance(String serverUrl, String realm, String username, String password, String clientId, String clientSecret) { + return new Keycloak(serverUrl, realm, username, password, clientId, clientSecret, PASSWORD, null); } - public static Keycloak getInstance(String serverUrl, String realm, String username, String password, String clientId){ - return new Keycloak(serverUrl, realm, username, password, clientId, null, null); + public static Keycloak getInstance(String serverUrl, String realm, String username, String password, String clientId) { + return new Keycloak(serverUrl, realm, username, password, clientId, null, PASSWORD, null); } - public RealmsResource realms(){ + public RealmsResource realms() { return target.proxy(RealmsResource.class); } - public RealmResource realm(String realmName){ + public RealmResource realm(String realmName) { return realms().realm(realmName); } - public ServerInfoResource serverInfo(){ + public ServerInfoResource serverInfo() { return target.proxy(ServerInfoResource.class); } - public TokenManager tokenManager(){ + public TokenManager tokenManager() { return tokenManager; } @@ -98,5 +98,4 @@ public class Keycloak { public void close() { client.close(); } - } diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/KeycloakBuilder.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/KeycloakBuilder.java index 5a61ffb933..e192d9abce 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/KeycloakBuilder.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/KeycloakBuilder.java @@ -20,15 +20,17 @@ package org.keycloak.admin.client; import org.jboss.resteasy.client.jaxrs.ResteasyClient; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; +import static org.keycloak.OAuth2Constants.CLIENT_CREDENTIALS; +import static org.keycloak.OAuth2Constants.PASSWORD; + /** * Provides a {@link Keycloak} client builder with the ability to customize the underlying * {@link ResteasyClient RESTEasy client} used to communicate with the Keycloak server. - * + *

*

Example usage with a connection pool size of 20:

- * * 
  *   Keycloak keycloak = KeycloakBuilder.builder()
- *     .serverUrl("https:/sso.example.com/auth")
+ *     .serverUrl("https://sso.example.com/auth")
  *     .realm("realm")
  *     .username("user")
  *     .password("pass")
@@ -37,6 +39,16 @@ import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder;
  *     .resteasyClient(new ResteasyClientBuilder().connectionPoolSize(20).build())
  *     .build();
  *
+ *

Example usage with grant_type=client_credentials

+ * 
+ *   Keycloak keycloak = KeycloakBuilder.builder()
+ *     .serverUrl("https://sso.example.com/auth")
+ *     .realm("example")
+ *     .grantType(OAuth2Constants.CLIENT_CREDENTIALS)
+ *     .clientId("client")
+ *     .clientSecret("secret")
+ *     .build();
+ *
* * @author Scott Rossillo * @see ResteasyClientBuilder @@ -48,6 +60,7 @@ public class KeycloakBuilder { private String password; private String clientId; private String clientSecret; + private String grantType = PASSWORD; private ResteasyClient resteasyClient; public KeycloakBuilder serverUrl(String serverUrl) { @@ -60,6 +73,12 @@ public class KeycloakBuilder { return this; } + public KeycloakBuilder grantType(String grantType) { + Config.checkGrantType(grantType); + this.grantType = grantType; + return this; + } + public KeycloakBuilder username(String username) { this.username = username; return this; @@ -97,19 +116,25 @@ public class KeycloakBuilder { throw new IllegalStateException("realm required"); } - if (username == null) { - throw new IllegalStateException("username required"); - } + if (PASSWORD.equals(grantType)) { + if (username == null) { + throw new IllegalStateException("username required"); + } - if (password == null) { - throw new IllegalStateException("password required"); + if (password == null) { + throw new IllegalStateException("password required"); + } + } else if (CLIENT_CREDENTIALS.equals(grantType)) { + if (clientSecret == null) { + throw new IllegalStateException("clientSecret required with grant_type=client_credentials"); + } } if (clientId == null) { throw new IllegalStateException("clientId required"); } - return new Keycloak(serverUrl, realm, username, password, clientId, clientSecret, resteasyClient); + return new Keycloak(serverUrl, realm, username, password, clientId, clientSecret, grantType, resteasyClient); } private KeycloakBuilder() { diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/token/TokenManager.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/token/TokenManager.java index e32568139c..bb32daea90 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/token/TokenManager.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/token/TokenManager.java @@ -27,11 +27,12 @@ import org.keycloak.representations.AccessTokenResponse; import javax.ws.rs.BadRequestException; import javax.ws.rs.core.Form; +import static org.keycloak.OAuth2Constants.*; + /** * @author rodrigo.sasaki@icarros.com.br */ public class TokenManager { - private static final long DEFAULT_MIN_VALIDITY = 30; private AccessTokenResponse currentToken; @@ -39,61 +40,67 @@ public class TokenManager { private long minTokenValidity = DEFAULT_MIN_VALIDITY; private final Config config; private final TokenService tokenService; + private final String accessTokenGrantType; - public TokenManager(Config config, ResteasyClient client){ + public TokenManager(Config config, ResteasyClient client) { this.config = config; ResteasyWebTarget target = client.target(config.getServerUrl()); - if(!config.isPublicClient()){ + if (!config.isPublicClient()) { target.register(new BasicAuthFilter(config.getClientId(), config.getClientSecret())); } - tokenService = target.proxy(TokenService.class); + this.tokenService = target.proxy(TokenService.class); + this.accessTokenGrantType = config.getGrantType(); + + if (CLIENT_CREDENTIALS.equals(accessTokenGrantType) && config.isPublicClient()) { + throw new IllegalArgumentException("Can't use " + GRANT_TYPE + "=" + CLIENT_CREDENTIALS + " with public client"); + } } - public String getAccessTokenString(){ + public String getAccessTokenString() { return getAccessToken().getToken(); } - public synchronized AccessTokenResponse getAccessToken(){ - if(currentToken == null){ + public synchronized AccessTokenResponse getAccessToken() { + if (currentToken == null) { grantToken(); - }else if(tokenExpired()){ + } else if (tokenExpired()) { refreshToken(); } return currentToken; } - public AccessTokenResponse grantToken(){ - Form form = new Form() - .param("grant_type", "password") - .param("username", config.getUsername()) + public AccessTokenResponse grantToken() { + Form form = new Form().param(GRANT_TYPE, accessTokenGrantType); + if (PASSWORD.equals(accessTokenGrantType)) { + form.param("username", config.getUsername()) .param("password", config.getPassword()); + } - if(config.isPublicClient()){ - form.param("client_id", config.getClientId()); + if (config.isPublicClient()) { + form.param(CLIENT_ID, config.getClientId()); } int requestTime = Time.currentTime(); synchronized (this) { - currentToken = tokenService.grantToken( config.getRealm(), form.asMap() ); + currentToken = tokenService.grantToken(config.getRealm(), form.asMap()); expirationTime = requestTime + currentToken.getExpiresIn(); } return currentToken; } - public AccessTokenResponse refreshToken(){ - Form form = new Form() - .param("grant_type", "refresh_token") - .param("refresh_token", currentToken.getRefreshToken()); + public AccessTokenResponse refreshToken() { + Form form = new Form().param(GRANT_TYPE, REFRESH_TOKEN) + .param(REFRESH_TOKEN, currentToken.getRefreshToken()); - if(config.isPublicClient()){ - form.param("client_id", config.getClientId()); + if (config.isPublicClient()) { + form.param(CLIENT_ID, config.getClientId()); } try { int requestTime = Time.currentTime(); synchronized (this) { - currentToken = tokenService.refreshToken( config.getRealm(), form.asMap() ); + currentToken = tokenService.refreshToken(config.getRealm(), form.asMap()); expirationTime = requestTime + currentToken.getExpiresIn(); } return currentToken; From ddd93df5031fc79320dc3dbcf33cd59062c4e71a Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Thu, 7 Apr 2016 16:21:30 +0200 Subject: [PATCH 03/26] KEYCLOAK-2646 Admin console shows 'Page not found' after password reset --- adapters/oidc/js/src/main/resources/keycloak.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/adapters/oidc/js/src/main/resources/keycloak.js b/adapters/oidc/js/src/main/resources/keycloak.js index 6aa116aebb..fa35049298 100755 --- a/adapters/oidc/js/src/main/resources/keycloak.js +++ b/adapters/oidc/js/src/main/resources/keycloak.js @@ -192,7 +192,7 @@ redirectUri += (redirectUri.indexOf('?') == -1 ? '?' : '&') + 'prompt=' + options.prompt; } - sessionStorage.oauthState = JSON.stringify({ state: state, nonce: nonce, redirectUri: encodeURIComponent(redirectUri) }); + localStorage.oauthState = JSON.stringify({ state: state, nonce: nonce, redirectUri: encodeURIComponent(redirectUri) }); var action = 'auth'; if (options && options.action == 'register') { @@ -689,10 +689,10 @@ function parseCallback(url) { var oauth = new CallbackParser(url, kc.responseMode).parseUri(); - var sessionState = sessionStorage.oauthState && JSON.parse(sessionStorage.oauthState); + var sessionState = localStorage.oauthState && JSON.parse(localStorage.oauthState); if (sessionState && (oauth.code || oauth.error || oauth.access_token || oauth.id_token) && oauth.state && oauth.state == sessionState.state) { - delete sessionStorage.oauthState; + delete localStorage.oauthState; oauth.redirectUri = sessionState.redirectUri; oauth.storedNonce = sessionState.nonce; From b576703533d79d1b1a92e1696e4c2c43eef3c47c Mon Sep 17 00:00:00 2001 From: Vaclav Muzikar Date: Thu, 7 Apr 2016 18:01:18 +0200 Subject: [PATCH 04/26] KEYCLOAK-2772 Add deletion test to GroupTest --- .../testsuite/admin/group/GroupTest.java | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java index ad3793ed0c..1e2abdb6d7 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java @@ -37,6 +37,7 @@ import java.io.IOException; import java.net.URI; import java.util.LinkedList; import java.util.List; +import java.util.Set; /** @@ -188,19 +189,6 @@ public class GroupTest extends AbstractGroupTest { Assert.assertEquals(1, level3Group.getRealmRoles().size()); Assert.assertTrue(level3Group.getRealmRoles().contains("level3Role")); - try { - GroupRepresentation notFound = realm.getGroupByPath("/notFound"); - Assert.fail(); - } catch (NotFoundException e) { - - } - try { - GroupRepresentation notFound = realm.getGroupByPath("/top/notFound"); - Assert.fail(); - } catch (NotFoundException e) { - - } - UserRepresentation user = realm.users().search("direct-login", -1, -1).get(0); realm.users().get(user.getId()).joinGroup(level3Group.getId()); List membership = realm.users().get(user.getId()).groups(); @@ -231,5 +219,27 @@ public class GroupTest extends AbstractGroupTest { realm.removeDefaultGroup(level3Group.getId()); defaultGroups = realm.getDefaultGroups(); Assert.assertEquals(0, defaultGroups.size()); + + realm.groups().group(topGroup.getId()).remove(); + + try { + realm.getGroupByPath("/top/level2/level3"); + Assert.fail("Group should not have been found"); + } + catch (NotFoundException e) {} + + try { + realm.getGroupByPath("/top/level2"); + Assert.fail("Group should not have been found"); + } + catch (NotFoundException e) {} + + try { + realm.getGroupByPath("/top"); + Assert.fail("Group should not have been found"); + } + catch (NotFoundException e) {} + + Assert.assertNull(login("direct-login", "resource-owner", "secret", user.getId()).getRealmAccess()); } } From 1cf10f1f11db13f8561cb8a5be933469794507c6 Mon Sep 17 00:00:00 2001 From: Konstantin Gribov Date: Thu, 7 Apr 2016 20:24:03 +0300 Subject: [PATCH 05/26] Revert accidentally removed ResteasyClient configuration in admin Fixes accidentally removed in PR #2449 ResteasyClient pool size parameter in `org.keycloak.admin.client.Keycloak`. --- .../src/main/java/org/keycloak/admin/client/Keycloak.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java index 5e30a664e4..f86170e521 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/Keycloak.java @@ -46,7 +46,7 @@ public class Keycloak { Keycloak(String serverUrl, String realm, String username, String password, String clientId, String clientSecret, String grantType, ResteasyClient resteasyClient) { config = new Config(serverUrl, realm, username, password, clientId, clientSecret, grantType); - client = resteasyClient != null ? resteasyClient : new ResteasyClientBuilder().build(); + client = resteasyClient != null ? resteasyClient : new ResteasyClientBuilder().connectionPoolSize(10).build(); tokenManager = new TokenManager(config, client); From 51efc3c1847e797ddaa1a6be7b0d8b7e8f581304 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 8 Apr 2016 06:59:39 +0200 Subject: [PATCH 06/26] KEYCLOAK-2774 Adapters logout section points to removed tokens endpoint --- docbook/auth-server-docs/reference/en/en-US/modules/logout.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docbook/auth-server-docs/reference/en/en-US/modules/logout.xml b/docbook/auth-server-docs/reference/en/en-US/modules/logout.xml index 9f5bfa37eb..ea6eb4e627 100755 --- a/docbook/auth-server-docs/reference/en/en-US/modules/logout.xml +++ b/docbook/auth-server-docs/reference/en/en-US/modules/logout.xml @@ -20,7 +20,7 @@ There are multiple ways you can logout from a web application. For Java EE servlet containers, you can call HttpServletRequest.logout(). - For any other browser application, you can point the browser at the url http://auth-server/auth/realms/{realm-name}/tokens/logout?redirect_uri=encodedRedirectUri. + For any other browser application, you can point the browser at the url http://auth-server/auth/realms/{realm-name}/protocol/openid-connect/logout?redirect_uri=encodedRedirectUri. This will log you out if you have an SSO session with your browser. \ No newline at end of file From faaf20de9571031c532bd127ca4c54df53ecfe20 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 8 Apr 2016 09:00:36 +0200 Subject: [PATCH 07/26] KEYCLOAK-2683 Remove QRCodeResource and embed QR code in image --- .../freemarker/FreeMarkerAccountProvider.java | 2 +- .../account/freemarker/model/TotpBean.java | 47 ++------ .../FreeMarkerLoginFormsProvider.java | 2 +- .../login/freemarker/model/TotpBean.java | 27 ++--- .../resources/KeycloakApplication.java | 1 - .../services/resources/QRCodeResource.java | 106 ------------------ .../java/org/keycloak/utils/TotpUtils.java | 69 ++++++++++++ .../resources/theme/base/account/totp.ftl | 2 +- .../theme/base/login/login-config-totp.ftl | 2 +- .../account/resources/css/account.css | 11 +- 10 files changed, 100 insertions(+), 169 deletions(-) mode change 100755 => 100644 services/src/main/java/org/keycloak/forms/account/freemarker/model/TotpBean.java delete mode 100755 services/src/main/java/org/keycloak/services/resources/QRCodeResource.java create mode 100644 services/src/main/java/org/keycloak/utils/TotpUtils.java diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/FreeMarkerAccountProvider.java b/services/src/main/java/org/keycloak/forms/account/freemarker/FreeMarkerAccountProvider.java index 893e816592..58c39ec568 100755 --- a/services/src/main/java/org/keycloak/forms/account/freemarker/FreeMarkerAccountProvider.java +++ b/services/src/main/java/org/keycloak/forms/account/freemarker/FreeMarkerAccountProvider.java @@ -188,7 +188,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { switch (page) { case TOTP: - attributes.put("totp", new TotpBean(session, realm, user, baseUri)); + attributes.put("totp", new TotpBean(session, realm, user)); break; case FEDERATED_IDENTITY: attributes.put("federatedIdentity", new AccountFederatedIdentityBean(session, realm, user, uriInfo.getBaseUri(), stateChecker)); diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/model/TotpBean.java b/services/src/main/java/org/keycloak/forms/account/freemarker/model/TotpBean.java old mode 100755 new mode 100644 index 1312e5d3cf..197f98558e --- a/services/src/main/java/org/keycloak/forms/account/freemarker/model/TotpBean.java +++ b/services/src/main/java/org/keycloak/forms/account/freemarker/model/TotpBean.java @@ -14,12 +14,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.keycloak.forms.account.freemarker.model; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.Base32; +import org.keycloak.models.utils.HmacOTP; +import org.keycloak.utils.TotpUtils; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -34,35 +37,15 @@ public class TotpBean { private final String totpSecret; private final String totpSecretEncoded; + private final String totpSecretQrCode; private final boolean enabled; - private final String contextUrl; - private final String keyUri; - public TotpBean(KeycloakSession session, RealmModel realm, UserModel user, URI baseUri) { + public TotpBean(KeycloakSession session, RealmModel realm, UserModel user) { this.enabled = session.users().configuredForCredentialType(realm.getOTPPolicy().getType(), realm, user); - this.contextUrl = baseUri.getPath(); - this.totpSecret = randomString(20); - this.totpSecretEncoded = Base32.encode(totpSecret.getBytes()); - this.keyUri = realm.getOTPPolicy().getKeyURI(realm, user, this.totpSecret); - } - - private static String randomString(int length) { - String chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVW1234567890"; - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < length; i++) { - char c = chars.charAt(random.nextInt(chars.length())); - sb.append(c); - } - return sb.toString(); - } - - private static final SecureRandom random; - - static - { - random = new SecureRandom(); - random.nextInt(); + this.totpSecret = HmacOTP.generateSecret(20); + this.totpSecretEncoded = TotpUtils.encode(totpSecret); + this.totpSecretQrCode = TotpUtils.qrCode(totpSecret, realm, user); } public boolean isEnabled() { @@ -74,19 +57,11 @@ public class TotpBean { } public String getTotpSecretEncoded() { - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < totpSecretEncoded.length(); i += 4) { - sb.append(totpSecretEncoded.substring(i, i + 4 < totpSecretEncoded.length() ? i + 4 : totpSecretEncoded.length())); - if (i + 4 < totpSecretEncoded.length()) { - sb.append(" "); - } - } - return sb.toString(); + return totpSecretEncoded; } - public String getTotpSecretQrCodeUrl() throws UnsupportedEncodingException { - String contents = URLEncoder.encode(keyUri, "utf-8"); - return contextUrl + "qrcode" + "?size=246x246&contents=" + contents; + public String getTotpSecretQrCode() { + return totpSecretQrCode; } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index 1a81870ee0..44ee44d77d 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -279,7 +279,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { switch (page) { case LOGIN_CONFIG_TOTP: - attributes.put("totp", new TotpBean(realm, user, baseUri)); + attributes.put("totp", new TotpBean(realm, user)); break; case LOGIN_UPDATE_PROFILE: UpdateProfileContext userCtx = (UpdateProfileContext) attributes.get(LoginFormsProvider.UPDATE_PROFILE_CONTEXT_ATTR); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java index a403176eec..d1e1d9ce45 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java @@ -20,6 +20,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.Base32; import org.keycloak.models.utils.HmacOTP; +import org.keycloak.utils.TotpUtils; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -32,17 +33,15 @@ public class TotpBean { private final String totpSecret; private final String totpSecretEncoded; + private final String totpSecretQrCode; private final boolean enabled; - private final String contextUrl; - private final String keyUri; - public TotpBean(RealmModel realm, UserModel user, URI baseUri) { + public TotpBean(RealmModel realm, UserModel user) { this.enabled = user.isOtpEnabled(); - this.contextUrl = baseUri.getPath(); - + this.totpSecret = HmacOTP.generateSecret(20); - this.totpSecretEncoded = Base32.encode(totpSecret.getBytes()); - this.keyUri = realm.getOTPPolicy().getKeyURI(realm, user, this.totpSecret); + this.totpSecretEncoded = TotpUtils.encode(totpSecret); + this.totpSecretQrCode = TotpUtils.qrCode(totpSecret, realm, user); } public boolean isEnabled() { @@ -54,19 +53,11 @@ public class TotpBean { } public String getTotpSecretEncoded() { - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < totpSecretEncoded.length(); i += 4) { - sb.append(totpSecretEncoded.substring(i, i + 4 < totpSecretEncoded.length() ? i + 4 : totpSecretEncoded.length())); - if (i + 4 < totpSecretEncoded.length()) { - sb.append(" "); - } - } - return sb.toString(); + return totpSecretEncoded; } - public String getTotpSecretQrCodeUrl() throws UnsupportedEncodingException { - String contents = URLEncoder.encode(keyUri, "utf-8"); - return contextUrl + "qrcode" + "?size=246x246&contents=" + contents; + public String getTotpSecretQrCode() { + return totpSecretQrCode; } } diff --git a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java index 4de67ed3ad..e979465a04 100644 --- a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java +++ b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java @@ -82,7 +82,6 @@ public class KeycloakApplication extends Application { singletons.add(new ServerVersionResource()); singletons.add(new RealmsResource()); singletons.add(new AdminRoot()); - classes.add(QRCodeResource.class); classes.add(ThemeResource.class); classes.add(JsResource.class); diff --git a/services/src/main/java/org/keycloak/services/resources/QRCodeResource.java b/services/src/main/java/org/keycloak/services/resources/QRCodeResource.java deleted file mode 100755 index cf19c30edd..0000000000 --- a/services/src/main/java/org/keycloak/services/resources/QRCodeResource.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright 2016 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.services.resources; - -import com.google.zxing.BarcodeFormat; -import com.google.zxing.WriterException; -import com.google.zxing.client.j2se.MatrixToImageWriter; -import com.google.zxing.common.BitMatrix; -import com.google.zxing.qrcode.QRCodeWriter; -import org.keycloak.services.util.CacheControlUtil; - -import javax.servlet.ServletException; -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.Response; -import javax.ws.rs.core.StreamingOutput; -import java.io.IOException; -import java.io.OutputStream; - -/** - * Create a barcode image - * - * @author Stian Thorgersen - */ -@Path("/qrcode") -public class QRCodeResource { - - /** - * Create a bar code image - * - * @param contents - * @param size - * @return - * @throws ServletException - * @throws IOException - * @throws WriterException - */ - @GET - @Produces("image/png") - public Response createQrCode(@QueryParam("contents") String contents, @QueryParam("size") String size) throws ServletException, IOException, WriterException { - int width = 256; - int height = 256; - - if (size != null) { - String[] s = size.split("x"); - try { - width = Integer.parseInt(s[0]); - height = Integer.parseInt(s[1]); - } catch (Throwable t) { - return Response.status(Response.Status.BAD_REQUEST).build(); - } - } - - if (contents == null) { - return Response.status(Response.Status.BAD_REQUEST).build(); - } - - if (width > 1000 || height > 1000 || contents.length() > 1000) { - return Response.status(Response.Status.BAD_REQUEST).build(); - } - - QRCodeWriter writer = new QRCodeWriter(); - final BitMatrix bitMatrix = writer.encode(contents, BarcodeFormat.QR_CODE, width, height); - - StreamingOutput stream = new StreamingOutput() { - @Override - public void write(OutputStream os) throws IOException, - WebApplicationException { - MatrixToImageWriter.writeToStream(bitMatrix, "png", os); - } - }; - - /* - * This response is served with extra headers that tell the browser to not do any caching. - * The reason is that this page will include a QR code that can give an attacker access to - * the time based tokens, so it's best to take precautions and make sure there are no copies - * of the QR code lost in a cache. - */ - CacheControl cacheControl = CacheControlUtil.noCache(); - - return Response.ok(stream) // - .cacheControl(cacheControl) // - .header("Pragma","no-cache") // - .header("Expires", "0") // - .build(); - } -} diff --git a/services/src/main/java/org/keycloak/utils/TotpUtils.java b/services/src/main/java/org/keycloak/utils/TotpUtils.java new file mode 100644 index 0000000000..f9f3b60d3a --- /dev/null +++ b/services/src/main/java/org/keycloak/utils/TotpUtils.java @@ -0,0 +1,69 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.utils; + +import com.google.zxing.BarcodeFormat; +import com.google.zxing.client.j2se.MatrixToImageWriter; +import com.google.zxing.common.BitMatrix; +import com.google.zxing.qrcode.QRCodeWriter; +import org.keycloak.common.util.Base64; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.utils.Base32; + +import java.io.ByteArrayOutputStream; +import java.net.URLEncoder; + +/** + * @author Stian Thorgersen + */ +public class TotpUtils { + + public static String encode(String totpSecret) { + String encoded = Base32.encode(totpSecret.getBytes()); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < encoded.length(); i += 4) { + sb.append(encoded.substring(i, i + 4 < encoded.length() ? i + 4 : encoded.length())); + if (i + 4 < encoded.length()) { + sb.append(" "); + } + } + return sb.toString(); + } + + public static String qrCode(String totpSecret, RealmModel realm, UserModel user) { + try { + String keyUri = realm.getOTPPolicy().getKeyURI(realm, user, totpSecret); + + int width = 246; + int height = 246; + + QRCodeWriter writer = new QRCodeWriter(); + final BitMatrix bitMatrix = writer.encode(keyUri, BarcodeFormat.QR_CODE, width, height); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + MatrixToImageWriter.writeToStream(bitMatrix, "png", bos); + bos.close(); + + return Base64.encodeBytes(bos.toByteArray()); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + +} diff --git a/themes/src/main/resources/theme/base/account/totp.ftl b/themes/src/main/resources/theme/base/account/totp.ftl index 1cfefba5e8..8ab36fa8b7 100755 --- a/themes/src/main/resources/theme/base/account/totp.ftl +++ b/themes/src/main/resources/theme/base/account/totp.ftl @@ -30,7 +30,7 @@

  • ${msg("totpStep2")}

    - Figure: Barcode
    + Figure: Barcode
    ${totp.totpSecretEncoded}
  • diff --git a/themes/src/main/resources/theme/base/login/login-config-totp.ftl b/themes/src/main/resources/theme/base/login/login-config-totp.ftl index 649a8339b1..07a88019f0 100755 --- a/themes/src/main/resources/theme/base/login/login-config-totp.ftl +++ b/themes/src/main/resources/theme/base/login/login-config-totp.ftl @@ -34,7 +34,7 @@

  • ${msg("loginTotpStep2")}

    - Figure: Barcode
    + Figure: Barcode
    ${totp.totpSecretEncoded}
  • diff --git a/themes/src/main/resources/theme/keycloak/account/resources/css/account.css b/themes/src/main/resources/theme/keycloak/account/resources/css/account.css index c1353d7972..be0c31e608 100644 --- a/themes/src/main/resources/theme/keycloak/account/resources/css/account.css +++ b/themes/src/main/resources/theme/keycloak/account/resources/css/account.css @@ -204,17 +204,20 @@ ol li { ol li img { margin-top: 15px; - width: 180px; margin-bottom: 5px; border: 1px solid #eee; } ol li span { - bottom: 80px; - left: 200px; + padding: 15px; + background-color: #f5f5f5; + border: 1px solid #eee; + top: 46px; + left: 270px; + right: 50px; position: absolute; font-family: courier, ​monospace; - font-size: 13px; + font-size: 25px; } hr + .form-horizontal { From 38952e5b3d3f2de2f9e5a4c269b0e8b4e5534f80 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 8 Apr 2016 07:20:54 +0200 Subject: [PATCH 08/26] KEYCLOAK-2614 Refactor database lock to use 'SELECT FOR UPDATE' pessimistic locking --- .../DefaultLiquibaseConnectionProvider.java | 4 + .../CustomLockDatabaseChangeLogGenerator.java | 86 +++++++++++++ .../liquibase/lock/CustomLockService.java | 119 +++++++++++------- .../liquibase/lock/DummyLockService.java | 10 ++ .../lock/LiquibaseDBLockProvider.java | 34 ++--- .../lock/LiquibaseDBLockProviderFactory.java | 10 +- .../mongo/lock/MongoDBLockProvider.java | 5 + .../models/dblock/DBLockProvider.java | 9 ++ .../services/managers/DBLockManager.java | 46 +++---- .../resources/KeycloakApplication.java | 10 +- .../keycloak/testsuite/model/DBLockTest.java | 10 +- 11 files changed, 232 insertions(+), 111 deletions(-) create mode 100644 model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockDatabaseChangeLogGenerator.java diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java index a011b4153a..ff3747b2a0 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java @@ -39,6 +39,7 @@ import org.keycloak.Config; import org.keycloak.connections.jpa.updater.liquibase.LiquibaseJpaUpdaterProvider; import org.keycloak.connections.jpa.updater.liquibase.PostgresPlusDatabase; import org.keycloak.connections.jpa.updater.liquibase.lock.CustomInsertLockRecordGenerator; +import org.keycloak.connections.jpa.updater.liquibase.lock.CustomLockDatabaseChangeLogGenerator; import org.keycloak.connections.jpa.updater.liquibase.lock.CustomLockService; import org.keycloak.connections.jpa.updater.liquibase.lock.DummyLockService; import org.keycloak.models.KeycloakSession; @@ -93,6 +94,9 @@ public class DefaultLiquibaseConnectionProvider implements LiquibaseConnectionPr // Change command for creating lock and drop DELETE lock record from it SqlGeneratorFactory.getInstance().register(new CustomInsertLockRecordGenerator()); + + // Use "SELECT FOR UPDATE" for locking database + SqlGeneratorFactory.getInstance().register(new CustomLockDatabaseChangeLogGenerator()); } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockDatabaseChangeLogGenerator.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockDatabaseChangeLogGenerator.java new file mode 100644 index 0000000000..09d5495a69 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockDatabaseChangeLogGenerator.java @@ -0,0 +1,86 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.connections.jpa.updater.liquibase.lock; + +import liquibase.database.Database; +import liquibase.database.core.DB2Database; +import liquibase.database.core.H2Database; +import liquibase.database.core.MSSQLDatabase; +import liquibase.database.core.MySQLDatabase; +import liquibase.database.core.OracleDatabase; +import liquibase.database.core.PostgresDatabase; +import liquibase.sql.Sql; +import liquibase.sql.UnparsedSql; +import liquibase.sqlgenerator.SqlGeneratorChain; +import liquibase.sqlgenerator.core.LockDatabaseChangeLogGenerator; +import liquibase.statement.core.LockDatabaseChangeLogStatement; +import org.jboss.logging.Logger; + +/** + * We use "SELECT FOR UPDATE" pessimistic locking (Same algorithm like Hibernate LockMode.PESSIMISTIC_WRITE ) + * + * @author Marek Posolda + */ +public class CustomLockDatabaseChangeLogGenerator extends LockDatabaseChangeLogGenerator { + + private static final Logger logger = Logger.getLogger(CustomLockDatabaseChangeLogGenerator.class); + + @Override + public int getPriority() { + return super.getPriority() + 1; // Ensure bigger priority than LockDatabaseChangeLogGenerator + } + + @Override + public Sql[] generateSql(LockDatabaseChangeLogStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { + + Sql selectForUpdateSql = generateSelectForUpdate(database); + + return new Sql[] { selectForUpdateSql }; + } + + + private Sql generateSelectForUpdate(Database database) { + String catalog = database.getLiquibaseCatalogName(); + String schema = database.getLiquibaseSchemaName(); + String rawLockTableName = database.getDatabaseChangeLogLockTableName(); + + String lockTableName = database.escapeTableName(catalog, schema, rawLockTableName); + String idColumnName = database.escapeColumnName(catalog, schema, rawLockTableName, "ID"); + + String sqlBase = "SELECT " + idColumnName + " FROM " + lockTableName; + String sqlWhere = " WHERE " + idColumnName + "=1"; + + String sql; + if (database instanceof MySQLDatabase || database instanceof PostgresDatabase || database instanceof H2Database || + database instanceof OracleDatabase) { + sql = sqlBase + sqlWhere + " FOR UPDATE"; + } else if (database instanceof MSSQLDatabase) { + sql = sqlBase + " WITH (UPDLOCK, ROWLOCK)" + sqlWhere; + } else if (database instanceof DB2Database) { + sql = sqlBase + sqlWhere + " FOR READ ONLY WITH RS USE AND KEEP UPDATE LOCKS"; + } else { + sql = sqlBase + sqlWhere; + logger.warnf("No direct support for database %s . Database lock may not work correctly", database.getClass().getName()); + } + + logger.debugf("SQL command for pessimistic lock: %s", sql); + + return new UnparsedSql(sql); + } + +} diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java index 0c246ca9a3..3efbb239ab 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java @@ -18,25 +18,16 @@ package org.keycloak.connections.jpa.updater.liquibase.lock; import java.lang.reflect.Field; -import java.text.DateFormat; -import java.util.Arrays; -import java.util.Collections; -import java.util.Date; -import liquibase.database.Database; import liquibase.database.core.DerbyDatabase; import liquibase.exception.DatabaseException; -import liquibase.exception.LockException; import liquibase.executor.Executor; import liquibase.executor.ExecutorService; -import liquibase.lockservice.DatabaseChangeLogLock; import liquibase.lockservice.StandardLockService; -import liquibase.logging.LogFactory; -import liquibase.sql.visitor.AbstractSqlVisitor; -import liquibase.sql.visitor.SqlVisitor; import liquibase.statement.core.CreateDatabaseChangeLogLockTableStatement; import liquibase.statement.core.DropTableStatement; import liquibase.statement.core.InitializeDatabaseChangeLogLockTableStatement; +import liquibase.statement.core.LockDatabaseChangeLogStatement; import liquibase.statement.core.RawSqlStatement; import org.jboss.logging.Logger; import org.keycloak.common.util.Time; @@ -51,24 +42,6 @@ public class CustomLockService extends StandardLockService { private static final Logger log = Logger.getLogger(CustomLockService.class); - private long changeLogLocRecheckTimeMillis = -1; - - @Override - public void setChangeLogLockRecheckTime(long changeLogLocRecheckTime) { - super.setChangeLogLockRecheckTime(changeLogLocRecheckTime); - this.changeLogLocRecheckTimeMillis = changeLogLocRecheckTime; - } - - // Bug in StandardLockService.getChangeLogLockRecheckTime() - @Override - public Long getChangeLogLockRecheckTime() { - if (changeLogLocRecheckTimeMillis == -1) { - return super.getChangeLogLockRecheckTime(); - } else { - return changeLogLocRecheckTimeMillis; - } - } - @Override public void init() throws DatabaseException { boolean createdTable = false; @@ -84,8 +57,8 @@ public class CustomLockService extends StandardLockService { database.commit(); } catch (DatabaseException de) { log.warn("Failed to create lock table. Maybe other transaction created in the meantime. Retrying..."); - if (log.isDebugEnabled()) { - log.debug(de.getMessage(), de); //Log details at debug level + if (log.isTraceEnabled()) { + log.trace(de.getMessage(), de); //Log details at trace level } database.rollback(); throw new LockRetryException(de); @@ -115,8 +88,8 @@ public class CustomLockService extends StandardLockService { } catch (DatabaseException de) { log.warn("Failed to insert first record to the lock table. Maybe other transaction inserted in the meantime. Retrying..."); - if (log.isDebugEnabled()) { - log.debug(de.getMessage(), de); // Log details at debug level + if (log.isTraceEnabled()) { + log.trace(de.getMessage(), de); // Log details at trace level } database.rollback(); throw new LockRetryException(de); @@ -140,34 +113,88 @@ public class CustomLockService extends StandardLockService { } @Override - public void waitForLock() throws LockException { + public void waitForLock() { boolean locked = false; long startTime = Time.toMillis(Time.currentTime()); long timeToGiveUp = startTime + (getChangeLogLockWaitTime()); + boolean nextAttempt = true; - while (!locked && Time.toMillis(Time.currentTime()) < timeToGiveUp) { + while (nextAttempt) { locked = acquireLock(); if (!locked) { int remainingTime = ((int)(timeToGiveUp / 1000)) - Time.currentTime(); - log.debugf("Waiting for changelog lock... Remaining time: %d seconds", remainingTime); - try { - Thread.sleep(getChangeLogLockRecheckTime()); - } catch (InterruptedException e) { - e.printStackTrace(); + if (remainingTime > 0) { + log.debugf("Will try to acquire log another time. Remaining time: %d seconds", remainingTime); + } else { + nextAttempt = false; } + } else { + nextAttempt = false; } } if (!locked) { - DatabaseChangeLogLock[] locks = listLocks(); - String lockedBy; - if (locks.length > 0) { - DatabaseChangeLogLock lock = locks[0]; - lockedBy = lock.getLockedBy() + " since " + DateFormat.getDateTimeInstance(DateFormat.SHORT, DateFormat.SHORT).format(lock.getLockGranted()); + int timeout = ((int)(getChangeLogLockWaitTime() / 1000)); + throw new IllegalStateException("Could not acquire change log lock within specified timeout " + timeout + " seconds. Currently locked by other transaction"); + } + } + + @Override + public boolean acquireLock() { + if (hasChangeLogLock) { + // We already have a lock + return true; + } + + Executor executor = ExecutorService.getInstance().getExecutor(database); + + try { + database.rollback(); + + // Ensure table created and lock record inserted + this.init(); + } catch (DatabaseException de) { + throw new IllegalStateException("Failed to retrieve lock", de); + } + + try { + log.debug("Trying to lock database"); + executor.execute(new LockDatabaseChangeLogStatement()); + log.debug("Successfully acquired database lock"); + + hasChangeLogLock = true; + database.setCanCacheLiquibaseTableInfo(true); + return true; + + } catch (DatabaseException de) { + log.warn("Lock didn't yet acquired. Will possibly retry to acquire lock. Details: " + de.getMessage()); + if (log.isTraceEnabled()) { + log.debug(de.getMessage(), de); + } + return false; + } + } + + + @Override + public void releaseLock() { + try { + if (hasChangeLogLock) { + log.debug("Going to release database lock"); + database.commit(); } else { - lockedBy = "UNKNOWN"; + log.warn("Attempt to release lock, which is not owned by current transaction"); + } + } catch (Exception e) { + log.error("Database error during release lock", e); + } finally { + try { + hasChangeLogLock = false; + database.setCanCacheLiquibaseTableInfo(false); + database.rollback(); + } catch (DatabaseException e) { + ; } - throw new LockException("Could not acquire change log lock. Currently locked by " + lockedBy); } } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/DummyLockService.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/DummyLockService.java index 61ef19fdfa..954822a5ae 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/DummyLockService.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/DummyLockService.java @@ -17,6 +17,7 @@ package org.keycloak.connections.jpa.updater.liquibase.lock; +import liquibase.exception.DatabaseException; import liquibase.exception.LockException; import liquibase.lockservice.StandardLockService; @@ -27,6 +28,15 @@ import liquibase.lockservice.StandardLockService; */ public class DummyLockService extends StandardLockService { + @Override + public int getPriority() { + return Integer.MAX_VALUE; + } + + @Override + public void init() throws DatabaseException { + } + @Override public void waitForLock() throws LockException { } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProvider.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProvider.java index 8769053102..f44641e14b 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProvider.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProvider.java @@ -46,7 +46,7 @@ public class LiquibaseDBLockProvider implements DBLockProvider { private final LiquibaseDBLockProviderFactory factory; private final KeycloakSession session; - private LockService lockService; + private CustomLockService lockService; private Connection dbConnection; private int maxAttempts = DEFAULT_MAX_ATTEMPTS; @@ -69,7 +69,6 @@ public class LiquibaseDBLockProvider implements DBLockProvider { this.lockService = new CustomLockService(); lockService.setChangeLogLockWaitTime(factory.getLockWaitTimeoutMillis()); - lockService.setChangeLogLockRecheckTime(factory.getLockRecheckTimeMillis()); lockService.setDatabase(liquibase.getDatabase()); } catch (LiquibaseException exception) { safeRollbackConnection(); @@ -94,16 +93,15 @@ public class LiquibaseDBLockProvider implements DBLockProvider { lockService.waitForLock(); this.maxAttempts = DEFAULT_MAX_ATTEMPTS; return; - } catch (LockException le) { - if (le.getCause() != null && le.getCause() instanceof LockRetryException) { - // Indicates we should try to acquire lock again in different transaction - restart(); - maxAttempts--; - } else { - throw new IllegalStateException("Failed to retrieve lock", le); - - // TODO: Possibility to forcefully retrieve lock after timeout instead of just give-up? - } + } catch (LockRetryException le) { + // Indicates we should try to acquire lock again in different transaction + safeRollbackConnection(); + restart(); + maxAttempts--; + } catch (RuntimeException re) { + safeRollbackConnection(); + safeCloseConnection(); + throw re; } } } @@ -111,14 +109,16 @@ public class LiquibaseDBLockProvider implements DBLockProvider { @Override public void releaseLock() { - try { - lockService.releaseLock(); - } catch (LockException e) { - logger.error("Could not release lock", e); - } + lockService.releaseLock(); lockService.reset(); } + @Override + public boolean supportsForcedUnlock() { + // Implementation based on "SELECT FOR UPDATE" can't force unlock as it's locked by other transaction + return false; + } + @Override public void destroyLockInfo() { try { diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProviderFactory.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProviderFactory.java index 5ce8be3f47..3026f7daf2 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/LiquibaseDBLockProviderFactory.java @@ -31,24 +31,17 @@ public class LiquibaseDBLockProviderFactory implements DBLockProviderFactory { private static final Logger logger = Logger.getLogger(LiquibaseDBLockProviderFactory.class); - private long lockRecheckTimeMillis; private long lockWaitTimeoutMillis; - protected long getLockRecheckTimeMillis() { - return lockRecheckTimeMillis; - } - protected long getLockWaitTimeoutMillis() { return lockWaitTimeoutMillis; } @Override public void init(Config.Scope config) { - int lockRecheckTime = config.getInt("lockRecheckTime", 2); int lockWaitTimeout = config.getInt("lockWaitTimeout", 900); - this.lockRecheckTimeMillis = Time.toMillis(lockRecheckTime); this.lockWaitTimeoutMillis = Time.toMillis(lockWaitTimeout); - logger.debugf("Liquibase lock provider configured with lockWaitTime: %d seconds, lockRecheckTime: %d seconds", lockWaitTimeout, lockRecheckTime); + logger.debugf("Liquibase lock provider configured with lockWaitTime: %d seconds", lockWaitTimeout); } @Override @@ -63,7 +56,6 @@ public class LiquibaseDBLockProviderFactory implements DBLockProviderFactory { @Override public void setTimeouts(long lockRecheckTimeMillis, long lockWaitTimeoutMillis) { - this.lockRecheckTimeMillis = lockRecheckTimeMillis; this.lockWaitTimeoutMillis = lockWaitTimeoutMillis; } diff --git a/model/mongo/src/main/java/org/keycloak/connections/mongo/lock/MongoDBLockProvider.java b/model/mongo/src/main/java/org/keycloak/connections/mongo/lock/MongoDBLockProvider.java index e3383fece0..1b368893d7 100644 --- a/model/mongo/src/main/java/org/keycloak/connections/mongo/lock/MongoDBLockProvider.java +++ b/model/mongo/src/main/java/org/keycloak/connections/mongo/lock/MongoDBLockProvider.java @@ -124,6 +124,11 @@ public class MongoDBLockProvider implements DBLockProvider { } } + @Override + public boolean supportsForcedUnlock() { + return true; + } + @Override public void destroyLockInfo() { db.getCollection(DB_LOCK_COLLECTION).remove(new BasicDBObject()); diff --git a/server-spi/src/main/java/org/keycloak/models/dblock/DBLockProvider.java b/server-spi/src/main/java/org/keycloak/models/dblock/DBLockProvider.java index b5fb417aca..d9dc131f25 100644 --- a/server-spi/src/main/java/org/keycloak/models/dblock/DBLockProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/dblock/DBLockProvider.java @@ -34,9 +34,18 @@ public interface DBLockProvider extends Provider { void waitForLock(); + /** + * Release previously acquired lock + */ void releaseLock(); + /** + * @return true if provider supports forced unlock at startup + */ + boolean supportsForcedUnlock(); + + /** * Will destroy whole state of DB lock (drop table/collection to track locking). * */ diff --git a/services/src/main/java/org/keycloak/services/managers/DBLockManager.java b/services/src/main/java/org/keycloak/services/managers/DBLockManager.java index 1909c36f41..8aace4bb14 100644 --- a/services/src/main/java/org/keycloak/services/managers/DBLockManager.java +++ b/services/src/main/java/org/keycloak/services/managers/DBLockManager.java @@ -34,52 +34,38 @@ public class DBLockManager { protected static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER; - public void waitForLock(KeycloakSessionFactory sessionFactory) { - KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { + private final KeycloakSession session; - @Override - public void run(KeycloakSession session) { - DBLockProvider lock = getDBLock(session); - lock.waitForLock(); - } - - }); + public DBLockManager(KeycloakSession session) { + this.session = session; } - public void releaseLock(KeycloakSessionFactory sessionFactory) { - KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { - - @Override - public void run(KeycloakSession session) { - DBLockProvider lock = getDBLock(session); - lock.releaseLock(); - } - - }); - } - - - public void checkForcedUnlock(KeycloakSessionFactory sessionFactory) { + public void checkForcedUnlock() { if (Boolean.getBoolean("keycloak.dblock.forceUnlock")) { - logger.forcedReleaseDBLock(); - releaseLock(sessionFactory); + DBLockProvider lock = getDBLock(); + if (lock.supportsForcedUnlock()) { + logger.forcedReleaseDBLock(); + lock.releaseLock(); + } else { + throw new IllegalStateException("Forced unlock requested, but provider " + lock + " doesn't support it"); + } } } // Try to detect ID from realmProvider - public DBLockProvider getDBLock(KeycloakSession session) { - String realmProviderId = getRealmProviderId(session); + public DBLockProvider getDBLock() { + String realmProviderId = getRealmProviderId(); return session.getProvider(DBLockProvider.class, realmProviderId); } - public DBLockProviderFactory getDBLockFactory(KeycloakSession session) { - String realmProviderId = getRealmProviderId(session); + public DBLockProviderFactory getDBLockFactory() { + String realmProviderId = getRealmProviderId(); return (DBLockProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(DBLockProvider.class, realmProviderId); } - private String getRealmProviderId(KeycloakSession session) { + private String getRealmProviderId() { RealmProviderFactory realmProviderFactory = (RealmProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(RealmProvider.class); return realmProviderFactory.getId(); } diff --git a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java index e979465a04..0294e9c55e 100644 --- a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java +++ b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java @@ -25,6 +25,7 @@ import org.keycloak.Config; import org.keycloak.exportimport.ExportImportManager; import org.keycloak.migration.MigrationModelManager; import org.keycloak.models.*; +import org.keycloak.models.dblock.DBLockProvider; import org.keycloak.services.managers.DBLockManager; import org.keycloak.models.utils.PostMigrationEvent; import org.keycloak.models.utils.RepresentationToModel; @@ -91,9 +92,10 @@ public class KeycloakApplication extends Application { ExportImportManager exportImportManager; - DBLockManager dbLockManager = new DBLockManager(); - dbLockManager.checkForcedUnlock(sessionFactory); - dbLockManager.waitForLock(sessionFactory); + DBLockManager dbLockManager = new DBLockManager(sessionFactory.create()); + dbLockManager.checkForcedUnlock(); + DBLockProvider dbLock = dbLockManager.getDBLock(); + dbLock.waitForLock(); try { migrateModel(); @@ -130,7 +132,7 @@ public class KeycloakApplication extends Application { importAddUser(); } finally { - dbLockManager.releaseLock(sessionFactory); + dbLock.releaseLock(); } if (exportImportManager.isRunExport()) { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/DBLockTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/DBLockTest.java index bc0cb99c8d..14fde53979 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/DBLockTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/DBLockTest.java @@ -54,17 +54,17 @@ public class DBLockTest extends AbstractModelTest { super.before(); // Set timeouts for testing - DBLockManager lockManager = new DBLockManager(); - DBLockProviderFactory lockFactory = lockManager.getDBLockFactory(session); + DBLockManager lockManager = new DBLockManager(session); + DBLockProviderFactory lockFactory = lockManager.getDBLockFactory(); lockFactory.setTimeouts(LOCK_RECHECK_MILLIS, LOCK_TIMEOUT_MILLIS); // Drop lock table, just to simulate racing threads for create lock table and insert lock record into it. - lockManager.getDBLock(session).destroyLockInfo(); + lockManager.getDBLock().destroyLockInfo(); commit(); } - // @Test // TODO: Running -Dtest=DBLockTest,UserModelTest might cause issues sometimes. Reenable this once DB lock is refactored. + @Test public void testLockConcurrently() throws Exception { long startupTime = System.currentTimeMillis(); @@ -112,7 +112,7 @@ public class DBLockTest extends AbstractModelTest { } private void lock(KeycloakSession session, Semaphore semaphore) { - DBLockProvider dbLock = new DBLockManager().getDBLock(session); + DBLockProvider dbLock = new DBLockManager(session).getDBLock(); dbLock.waitForLock(); try { semaphore.increase(); From 34e125ea8067fb6a052c85145c309cedfa169f9f Mon Sep 17 00:00:00 2001 From: mhajas Date: Thu, 31 Mar 2016 14:18:36 +0200 Subject: [PATCH 09/26] Fix urls on eap6 --- .../arquillian/provider/URLProvider.java | 2 +- .../page/AbstractPageWithInjectedUrl.java | 2 +- .../example/AbstractDemoExampleAdapterTest.java | 15 +++++---------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/provider/URLProvider.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/provider/URLProvider.java index 301fc866a6..424705ccd5 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/provider/URLProvider.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/provider/URLProvider.java @@ -74,7 +74,7 @@ public class URLProvider extends URLResourceProvider { } try { - if ("true".equals(System.getProperty("app.server.eap6"))) { + if ("eap6".equals(System.getProperty("app.server"))) { if (url == null) { url = new URL("http://localhost:8080/"); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPageWithInjectedUrl.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPageWithInjectedUrl.java index fcbdc977d7..e678db4341 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPageWithInjectedUrl.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPageWithInjectedUrl.java @@ -32,7 +32,7 @@ public abstract class AbstractPageWithInjectedUrl extends AbstractPage { //EAP6 URL fix protected URL createInjectedURL(String url) { - if (System.getProperty("app.server.eap6","false").equals("false")) { + if (!System.getProperty("app.server").equals("eap6")) { return null; } try { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractDemoExampleAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractDemoExampleAdapterTest.java index fafbf2942b..b1df6e0a34 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractDemoExampleAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractDemoExampleAdapterTest.java @@ -344,16 +344,8 @@ public abstract class AbstractDemoExampleAdapterTest extends AbstractExampleAdap String serverLogPath = null; - if (System.getProperty("app.server.wildfly", "false").equals("true")) { - serverLogPath = System.getProperty("app.server.wildfly.home") + "/standalone/log/server.log"; - } - - if (System.getProperty("app.server.eap6", "false").equals("true")) { - serverLogPath = System.getProperty("app.server.eap6.home") + "/standalone/log/server.log"; - } - - if (System.getProperty("app.server.eap7", "false").equals("true")) { - serverLogPath = System.getProperty("app.server.eap7.home") + "/standalone/log/server.log"; + if (System.getProperty("app.server").equals("wildfly") || System.getProperty("app.server").equals("eap6") || System.getProperty("app.server").equals("eap")) { + serverLogPath = System.getProperty("app.server.home") + "/standalone/log/server.log"; } String appServerUrl; @@ -364,6 +356,7 @@ public abstract class AbstractDemoExampleAdapterTest extends AbstractExampleAdap } if (serverLogPath != null) { + log.info("Checking app server log at: " + serverLogPath); File serverLog = new File(serverLogPath); String serverLogContent = FileUtils.readFileToString(serverLog); UserRepresentation bburke = ApiUtil.findUserByUsername(testRealmResource(), "bburke@redhat.com"); @@ -373,6 +366,8 @@ public abstract class AbstractDemoExampleAdapterTest extends AbstractExampleAdap assertTrue(matcher.find()); assertTrue(serverLogContent.contains("User '" + bburke.getId() + "' invoking '" + appServerUrl + "database/customers' on client 'database-service'")); + } else { + log.info("Checking app server log on app-server: \"" + System.getProperty("app.server") + "\" is not supported."); } } } From 4ed2c4e5a810d41c28172ebe168bbadad431a032 Mon Sep 17 00:00:00 2001 From: mhajas Date: Thu, 31 Mar 2016 12:45:00 +0200 Subject: [PATCH 10/26] Fix timeouts in DemoServlets tests --- .../adapter/servlet/AbstractDemoServletsAdapterTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractDemoServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractDemoServletsAdapterTest.java index 181d5029fc..ab7b1f5dc5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractDemoServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractDemoServletsAdapterTest.java @@ -25,7 +25,6 @@ import org.junit.Ignore; import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.common.Version; -import org.keycloak.common.util.Time; import org.keycloak.constants.AdapterConstants; import org.keycloak.protocol.oidc.OIDCLoginProtocolService; import org.keycloak.representations.VersionRepresentation; @@ -49,6 +48,7 @@ import java.util.concurrent.TimeUnit; import static org.junit.Assert.*; import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlEquals; import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWithLoginUrlOf; +import static org.keycloak.testsuite.util.WaitUtils.pause; /** * @@ -226,7 +226,8 @@ public abstract class AbstractDemoServletsAdapterTest extends AbstractServletsAd demoRealmRep.setSsoSessionIdleTimeout(1); testRealmResource().update(demoRealmRep); -// Thread.sleep(2000); + pause(2000); + productPortal.navigateTo(); assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); @@ -253,16 +254,16 @@ public abstract class AbstractDemoServletsAdapterTest extends AbstractServletsAd demoRealmRep.setSsoSessionIdleTimeout(1); testRealmResource().update(demoRealmRep); - Time.setOffset(2); + pause(2000); productPortal.navigateTo(); assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); // need to cleanup so other tests don't fail, so invalidate http sessions on remote clients. demoRealmRep.setSsoSessionIdleTimeout(originalIdle); + testRealmResource().update(demoRealmRep); // note: sessions invalidated after each test, see: AbstractKeycloakTest.afterAbstractKeycloakTest() - Time.setOffset(0); } @Test From 8209c749183fad91327c39d8cbe4b9db50334f95 Mon Sep 17 00:00:00 2001 From: mhajas Date: Fri, 1 Apr 2016 11:01:18 +0200 Subject: [PATCH 11/26] Fix DemoServlet test --- .../adapter/AbstractServletsAdapterTest.java | 5 ++++ .../resources/adapter-test/demorealm.json | 4 +-- .../secure-portal/WEB-INF/keycloak.json | 23 ++++++++++++------ .../secure-portal/WEB-INF/keystore.jks | Bin 0 -> 2045 bytes 4 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keystore.jks diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java index 1e708b67a3..a3badef973 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java @@ -48,6 +48,11 @@ public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { .addAsWebInfResource(keycloakJSON, "keycloak.json") .addAsWebInfResource(jbossDeploymentStructure, JBOSS_DEPLOYMENT_STRUCTURE_XML); + URL keystore = AbstractServletsAdapterTest.class.getResource(webInfPath + "keystore.jks"); + if (keystore != null) { + deployment.addAsWebInfResource(keystore, "classes/keystore.jks"); + } + addContextXml(deployment, name); return deployment; diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json index bb6ce6ebff..fbe54ceb30 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json @@ -174,8 +174,8 @@ "redirectUris": [ "/secure-portal/*" ], - "attributes": { - "jwt.credential.certificate": "MIICnTCCAYUCBgFPPLDaTzANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDDAdjbGllbnQxMB4XDTE1MDgxNzE3MjI0N1oXDTI1MDgxNzE3MjQyN1owEjEQMA4GA1UEAwwHY2xpZW50MTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAIUjjgv+V3s96O+Za9002Lp/trtGuHBeaeVL9dFKMKzO2MPqdRmHB4PqNlDdd28Rwf5Xn6iWdFpyUKOnI/yXDLhdcuFpR0sMNK/C9Lt+hSpPFLuzDqgtPgDotlMxiHIWDOZ7g9/gPYNXbNvjv8nSiyqoguoCQiiafW90bPHsiVLdP7ZIUwCcfi1qQm7FhxRJ1NiW5dvUkuCnnWEf0XR+Wzc5eC9EgB0taLFiPsSEIlWMm5xlahYyXkPdNOqZjiRnrTWm5Y4uk8ZcsD/KbPTf/7t7cQXipVaswgjdYi1kK2/zRwOhg1QwWFX/qmvdd+fLxV0R6VqRDhn7Qep2cxwMxLsCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAKE6OA46sf20bz8LZPoiNsqRwBUDkaMGXfnob7s/hJZIIwDEx0IAQ3uKsG7q9wb+aA6s+v7S340zb2k3IxuhFaHaZpAd4CyR5cn1FHylbzoZ7rI/3ASqHDqpljdJaFqPH+m7nZWtyDvtZf+gkZ8OjsndwsSBK1d/jMZPp29qYbl1+XfO7RCp/jDqro/R3saYFaIFiEZPeKn1hUJn6BO48vxH1xspSu9FmlvDOEAOz4AuM58z4zRMP49GcFdCWr1wkonJUHaSptJaQwmBwLFUkCbE5I1ixGMb7mjEud6Y5jhfzJiZMo2U8RfcjNbrN0diZl3jB6LQIwESnhYSghaTjNQ==" + "attributes" : { + "jwt.credential.certificate" : "MIICqTCCAZECBgFT0Ngs/DANBgkqhkiG9w0BAQsFADAYMRYwFAYDVQQDDA1zZWN1cmUtcG9ydGFsMB4XDTE2MDQwMTA4MDA0MVoXDTI2MDQwMTA4MDIyMVowGDEWMBQGA1UEAwwNc2VjdXJlLXBvcnRhbDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJa4GixpmzP511AmI0eLPLORyJwXS8908MUvdG3hmh8jMOIhe28XjIFeZSY09vFxh22F2SUMjxU/B2Hw4PDJUkebuNR7rXhOIYCJAo6eEZzjSBY/wngFtfm74zJ/eLCobBtDvIld7jobdHTfE1Oz9+GzvtG0k7cm7ubrLT0J4I1UsFZj3b//3wa+O0vNaTwHC1Jz/m59VbtXqyO4xEzIdl416cnGCmEmk5qd5h1de2UoLi/CTad8HftIJhzN1qhlySzW/9Ha70aYlDH2hiibDsXDTrNaMdaaLik7I8Rv/nIbggysG863PKZo8wknDe62QctH5VYSSktiy4gjSJkGh7ECAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAZnnx+AHQ8txugGcFK8gWjildDgk+v31fBHBDvmLQaSzsUaIOJaK4wnlwUI+VfR46HmBXhjlDCobFLUptd+kz0G7xapcIn3b5jLrySUUD7L+LAp1vNOQU4mKhTGS3IEvNB73D3GH9rQ+M3KEcoN3f99fNKqKsUdxbmZqGf4VOQ57PUfLBw4PJJGlROPosBc7ivPRyeYnKekhoCTynq30BAD1FA1BA8ppcY4ZVGADPTAgMJxpglpFY9LiqCwdLAGW1ttnsyIJ7DpT+kybhhk7c+MU7gyQdv8xPnMR0bSCB9hndowgBn5oZ393aMscwMNCzwJ0aWBs1sUyn3X0RIsu9Jg==" } }, { diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keycloak.json b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keycloak.json index 6b8d13f1e2..de290de301 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keycloak.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keycloak.json @@ -1,10 +1,17 @@ { - "realm" : "demo", - "resource" : "secure-portal", - "realm-public-key" : "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB", - "auth-server-url" : "http://localhost:8180/auth", - "ssl-required" : "external", - "credentials" : { - "secret": "password" - } + "realm": "demo", + "realm-public-key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB", + "auth-server-url": "http://localhost:8180/auth", + "ssl-required": "external", + "resource": "secure-portal", + "credentials": { + "jwt": { + "client-key-password": "password", + "client-keystore-file": "classpath:keystore.jks", + "client-keystore-password": "password", + "client-key-alias": "secure-portal", + "token-timeout": 10, + "client-keystore-type": "jks" + } + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keystore.jks b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..399be7a48ad57bada96b492d6f4c5f21305a1ee0 GIT binary patch literal 2045 zcmaKsc{J1u8^`B|84Sak*`8FmfyKKPx(J*#S?M6>_3m6JJ$BTS!?3;#3tmp8=t5X& zCvP`ICO<4)nmK4zK(iUN($UJocfc`@EU|f|cWH(iUM8_k99cmu#NFI|xyg@#-K}KT z(!iEhQha?zV1~|#-(Q9PV(DB#%Tq~S*z&cLs;Qeu<{QK{ie{(y#<@KYG%#39#`+8XED0`8o*jG2x&Ff4x4`_Zr_Jv}!}yKke+zq_s};@fhtEg1^?>IS*p z&aP7Uf+0&DAk+XaJs;rP$E`iRhzEx|a+ zD>Dz3Jf?L4b9&}hJK{F4%{?7RP&glMD=S`SRBetN7a=(Cw8KKO0k@Gu-;85 zJ>^Y|T7Fig9U6xXd2qTJPx<=LwF7Iz%7 zi`V?Plb^|ih8xwdJVY3GBIar za~O`8Wj8J5N-iu6O7pLh+-Wb&GOx_6lGE+)3$~u0u2Ae)kOigN8ZbJ>tu_PjMRq3g z5+=jNs;O-&B?u;RdCtg|MqU2#JxqZhQt16X_g&I=TFGpO=`cv_vS*ch@WR&Khl*^H zalw6q2JIH1`aV0t-tj6}!zYAWDB&yY!xe-DX^~P|X-(A(2ygc&nJjAm27%X5TPO-{ zgHImcwy^Kl#I2?j!10n2FOjmZB%t%bF4!bF!pwu+R$0hvMux|4-FHk*Nu3Qx>r7|UVQw|z+xhA5v%@0mK8Rd&~ z(V7ac)fPZSfeU;uAJRi5>I3HBX4md*+hcjR{EHmwxPS3-#^k~ZVT3$qS!0{Hb+sd7 zt}9uwv!H@h5gXAiOu~c(*I|Rqt~}9d9UtMlg_c+yB#VJ*Wjpkb+29dm46bz4|JJ4N+xK!Pvm zW6>u2$b5_@2|YrSfV)DF|3FfgkA^|SwJCNK9t;2^z##NN%mK7KMA%J4Sn_|VidKQc zFvrnGXbjpIjW)t~!C{8~wT6Zmum7`20@VI_W*@=ABtROp{{~cu1OTA?b|qbW;jx`b z7o@svhI#Ydp(3~=lgJq`AQI;3ht<&^Rg)s&nW?_PNTcnIXexm`r6HN4U?qBq!{!X1 zvn_0YpY%BXjB3gqq3jZwq6NDHRxjhhtvk;b3~$Ca)PyTrci!uZ^* z_eTn1lu43x%D+7|uL|7~*MzOLoqA*Y$z9gLG2l&_x?KT;+9)IpfPjA{v|q%2LICJ; z!p1jXbdwd45(3s4I*_g9BPDLxlk6uFW!)Vxiq~B^TP~$h-u^Nn$|dK1vWlsSzX!#{ z8cG?^b0Az>IyM@y5k?a$joZn5wrOu8ywZ~)R2*sar~Jo&GF;HpBaS1YU45)eKOReG zvdRwqIy1+c9MLYXJInGepi^#=&sdi*&u+fxyES|if7WOYO|1-(lBDhl@{seCGpug| zX19(adfqx0^%Dt4QnwXnD#UdbGK>Sm}Z?wh>o7lPsh5rVBZ?mGZVJ TQ-MZp=uyEzlNGk^y0HHZ3|gv* literal 0 HcmV?d00001 From 013af68d746a985c3e3db003d17c080b903b4a45 Mon Sep 17 00:00:00 2001 From: mhajas Date: Fri, 1 Apr 2016 12:17:39 +0200 Subject: [PATCH 12/26] Better stability in JSConsole test --- .../adapter/example/AbstractJSConsoleExampleAdapterTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractJSConsoleExampleAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractJSConsoleExampleAdapterTest.java index e4c9226f97..e0d87f7b7f 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractJSConsoleExampleAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/AbstractJSConsoleExampleAdapterTest.java @@ -168,6 +168,7 @@ public abstract class AbstractJSConsoleExampleAdapterTest extends AbstractExampl jsConsoleExamplePage.init(); jsConsoleExamplePage.getProfile(); + pause(500); assertTrue(jsConsoleExamplePage.getOutputText().contains("Failed to load profile")); jsConsoleExamplePage.logIn(); @@ -306,7 +307,7 @@ public abstract class AbstractJSConsoleExampleAdapterTest extends AbstractExampl logInAndInit("implicit"); - pause(5000); + pause(6000); assertTrue(jsConsoleExamplePage.getEventsText().contains("Access token expired")); } From 00bb2de0326251e4cf1c0f87eb4089ec13be0372 Mon Sep 17 00:00:00 2001 From: Vlasta Ramik Date: Tue, 29 Mar 2016 11:32:08 +0200 Subject: [PATCH 13/26] added nexus-staging-maven-plugin --- pom.xml | 53 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/pom.xml b/pom.xml index 0ddcf4cdf4..a414a02418 100644 --- a/pom.xml +++ b/pom.xml @@ -1,19 +1,19 @@ +~ Copyright 2016 Red Hat, Inc. and/or its affiliates +~ and other contributors as indicated by the @author tags. +~ +~ Licensed under the Apache License, Version 2.0 (the "License"); +~ you may not use this file except in compliance with the License. +~ You may obtain a copy of the License at +~ +~ http://www.apache.org/licenses/LICENSE-2.0 +~ +~ Unless required by applicable law or agreed to in writing, software +~ distributed under the License is distributed on an "AS IS" BASIS, +~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +~ See the License for the specific language governing permissions and +~ limitations under the License. +--> @@ -107,6 +107,7 @@ 1.7.2 2.3.7 1.0.1.Final + 1.6.5 -Xms512m -Xmx2048m -XX:MetaspaceSize=96m -XX:MaxMetaspaceSize=256m @@ -1213,6 +1214,16 @@ + + org.sonatype.plugins + nexus-staging-maven-plugin + ${nexus.staging.plugin.version} + true + + https://repository.jboss.org/nexus + jboss-releases-repository + + org.apache.maven.plugins maven-release-plugin @@ -1351,5 +1362,17 @@ + + + nexus-staging + + + + org.sonatype.plugins + nexus-staging-maven-plugin + + + + From 3c5fcebce959766ac11ea1feea913425bb4c49a3 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Fri, 8 Apr 2016 16:34:46 +0200 Subject: [PATCH 14/26] KEYCLOAK-2785: Admin client should be able to delete a user. The delete user service should be exposed in the admin client. --- .../admin/client/resource/UsersResource.java | 11 ++++------- .../org/keycloak/testsuite/admin/UserTest.java | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UsersResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UsersResource.java index 9497f3a67a..93a0638b8c 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UsersResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UsersResource.java @@ -19,13 +19,7 @@ package org.keycloak.admin.client.resource; import org.keycloak.representations.idm.UserRepresentation; -import javax.ws.rs.Consumes; -import javax.ws.rs.GET; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; -import javax.ws.rs.QueryParam; +import javax.ws.rs.*; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.util.List; @@ -59,4 +53,7 @@ public interface UsersResource { @Path("{id}") UserResource get(@PathParam("id") String id); + @Path("{id}") + @DELETE + Response delete(@PathParam("id") String id); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java index eafe169a74..c2f4b7c010 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -239,6 +239,20 @@ public class UserTest extends AbstractClientTest { assertEquals(9, count.intValue()); } + @Test + public void delete() { + Response response = realm.users().delete( createUser() ); + assertEquals(204, response.getStatus()); + response.close(); + } + + @Test + public void deleteNonExistent() { + Response response = realm.users().delete( "does-not-exist" ); + assertEquals(404, response.getStatus()); + response.close(); + } + @Test public void searchPaginated() { createUsers(); From 2d3580e2d40ad4d35d3ac0543dccc4153c48d7fe Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 8 Apr 2016 19:06:52 +0200 Subject: [PATCH 15/26] KEYCLOAK-2769 Fix NPE during 'Identity Broker cancelled' and instead show keycloak 'we are sorry' page --- .../resources/IdentityBrokerService.java | 4 +- .../broker/AbstractIdentityProviderTest.java | 4 ++ .../OIDCKeyCloakServerBrokerBasicTest.java | 68 +++++++++++++++++-- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 27d47b5e1e..4a9d99383c 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -531,7 +531,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override public Response cancelled(String code) { ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { return redirectToErrorPage(Messages.INVALID_CODE); } @@ -541,7 +541,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override public Response error(String code, String message) { ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { return redirectToErrorPage(Messages.INVALID_CODE); } return browserAuthentication(clientCode.getClientSession(), message); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java index b45213aa26..f5f80f5011 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java @@ -40,6 +40,7 @@ import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionSt import org.keycloak.testsuite.pages.AccountFederatedIdentityPage; import org.keycloak.testsuite.pages.AccountPasswordPage; import org.keycloak.testsuite.pages.AccountUpdateProfilePage; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.pages.OAuthGrantPage; @@ -109,6 +110,9 @@ public abstract class AbstractIdentityProviderTest { @WebResource protected AccountFederatedIdentityPage accountFederatedIdentityPage; + @WebResource + protected ErrorPage errorPage; + protected KeycloakSession session; protected int logoutTimeOffset = 0; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index ad3e9ffeeb..68d903cafc 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -17,12 +17,16 @@ package org.keycloak.testsuite.broker; +import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.common.util.Time; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.representations.AccessTokenResponse; +import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.services.Urls; @@ -30,7 +34,6 @@ import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.Constants; import org.keycloak.testsuite.KeycloakServer; import org.keycloak.testsuite.pages.AccountApplicationsPage; -import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.rule.AbstractKeycloakRule; import org.keycloak.testsuite.rule.WebResource; import org.keycloak.util.JsonSerialization; @@ -38,6 +41,7 @@ import org.openqa.selenium.NoSuchElementException; import javax.ws.rs.core.UriBuilder; import java.io.IOException; +import java.util.List; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -68,9 +72,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP } }; - @WebResource - private OAuthGrantPage grantPage; - @WebResource protected AccountApplicationsPage accountApplicationsPage; @@ -139,6 +140,65 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP keycloak.realm("realm-with-broker").identityProviders().get("kc-oidc-idp").update(idp); } + + @Test + public void testAccessDeniedError() throws Exception { + // Disable update profile + setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF); + + Keycloak keycloak1 = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); + Keycloak keycloak2 = Keycloak.getInstance("http://localhost:8082/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); + + // Require broker to show consent screen + RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); + List clients = brokeredRealm.clients().findByClientId("broker-app"); + Assert.assertEquals(1, clients.size()); + ClientRepresentation brokerApp = clients.get(0); + brokerApp.setConsentRequired(true); + brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp); + + // Change timeouts on realm-with-broker to lower values + RealmResource realmWithBroker = keycloak1.realm("realm-with-broker"); + RealmRepresentation realmBackup = realmWithBroker.toRepresentation(); + RealmRepresentation realm = realmWithBroker.toRepresentation(); + realm.setAccessCodeLifespanLogin(30);; + realm.setAccessCodeLifespan(30); + realm.setAccessCodeLifespanUserAction(30); + realmWithBroker.update(realm); + + // Login to broker + loginIDP("test-user"); + + // Set time offset and manually enforce removeExpired sessions TODO: Will need custom REST endpoints for that on integration-arquillian + Time.setOffset(60); + try { + brokerServerRule.stopSession(this.session, true); + this.session = brokerServerRule.startSession(); + + session.sessions().removeExpired(getRealm()); + + brokerServerRule.stopSession(this.session, true); + this.session = brokerServerRule.startSession(); + + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert classic error page ( We're sorry ) displayed. TODO: Should be improved? See KEYCLOAK-2740 + errorPage.assertCurrent(); + + } finally { + Time.setOffset(0); + } + + // Revert consent + brokerApp.setConsentRequired(false); + brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp); + + // Revert timeouts + realmWithBroker.update(realmBackup); + } + @Test public void testSuccessfulAuthenticationWithoutUpdateProfile() { super.testSuccessfulAuthenticationWithoutUpdateProfile(); From 5440688dd94c9311fd9eef39642fb5b458f17fd8 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 11 Apr 2016 07:42:49 +0200 Subject: [PATCH 16/26] KEYCLOAK-2766 Add missing id to AuthenticatorConfigRepresentation --- .../idm/AuthenticatorConfigRepresentation.java | 10 ++++++++-- .../keycloak/models/utils/ModelToRepresentation.java | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/idm/AuthenticatorConfigRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/AuthenticatorConfigRepresentation.java index dda7418152..258f9257ed 100755 --- a/core/src/main/java/org/keycloak/representations/idm/AuthenticatorConfigRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/AuthenticatorConfigRepresentation.java @@ -27,9 +27,17 @@ import java.util.Map; */ public class AuthenticatorConfigRepresentation implements Serializable { + private String id; private String alias; private Map config = new HashMap(); + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } public String getAlias() { return alias; @@ -39,8 +47,6 @@ public class AuthenticatorConfigRepresentation implements Serializable { this.alias = alias; } - - public Map getConfig() { return config; } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 64079b0a1f..7f7f7e0d04 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -717,6 +717,7 @@ public class ModelToRepresentation { public static AuthenticatorConfigRepresentation toRepresentation(AuthenticatorConfigModel model) { AuthenticatorConfigRepresentation rep = new AuthenticatorConfigRepresentation(); + rep.setId(model.getId()); rep.setAlias(model.getAlias()); rep.setConfig(model.getConfig()); return rep; From a48ace4d70d1f8e13df6cecfd781b0b5a62ed6b1 Mon Sep 17 00:00:00 2001 From: Pavel Drozd Date: Mon, 11 Apr 2016 08:04:19 +0200 Subject: [PATCH 17/26] KEYCLOAK-2794 - changed version of undertow-embedded. --- testsuite/integration-arquillian/tests/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/integration-arquillian/tests/pom.xml b/testsuite/integration-arquillian/tests/pom.xml index 35059eb4c7..36d4863581 100644 --- a/testsuite/integration-arquillian/tests/pom.xml +++ b/testsuite/integration-arquillian/tests/pom.xml @@ -749,7 +749,7 @@ org.jboss.arquillian.container undertow-embedded - 1.0.0.Alpha1-SNAPSHOT + 1.0.0.Alpha2 From dafea0cad6036f20873143488fe22204738f0a66 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 11 Apr 2016 09:18:57 +0200 Subject: [PATCH 18/26] KEYCLOAK-2700 Import existing realm fails due to can't delete group --- .../keycloak/models/jpa/JpaRealmProvider.java | 4 ++++ .../keycloak/testsuite/admin/RealmTest.java | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 5da9b4e4cd..3172665dde 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -127,6 +127,10 @@ public class JpaRealmProvider implements RealmProvider { em.refresh(realm); RealmAdapter adapter = new RealmAdapter(session, em, realm); session.users().preRemove(adapter); + + realm.getDefaultGroups().clear(); + em.flush(); + int num = em.createNamedQuery("deleteGroupRoleMappingsByRealm") .setParameter("realm", realm).executeUpdate(); num = em.createNamedQuery("deleteGroupAttributesByRealm") diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/RealmTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/RealmTest.java index 6b842b04b3..40ed6ee661 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/RealmTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/RealmTest.java @@ -27,6 +27,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.services.managers.RealmManager; @@ -35,6 +36,7 @@ import org.keycloak.util.JsonSerialization; import javax.ws.rs.BadRequestException; import javax.ws.rs.NotFoundException; +import javax.ws.rs.core.Response; import java.io.IOException; import java.util.Collections; import java.util.LinkedList; @@ -441,4 +443,22 @@ public class RealmTest extends AbstractClientTest { assertEquals(certificate, rep.getCertificate()); } + @Test + // KEYCLOAK-2700 + public void deleteRealmWithDefaultGroups() throws IOException { + RealmRepresentation rep = new RealmRepresentation(); + rep.setRealm("foo"); + + GroupRepresentation group = new GroupRepresentation(); + group.setName("default1"); + group.setPath("/default1"); + + rep.setGroups(Collections.singletonList(group)); + rep.setDefaultGroups(Collections.singletonList("/default1")); + + keycloak.realms().create(rep); + + keycloak.realm(rep.getRealm()).remove(); + } + } From f4f95cd096fc12350f3bfadc0865b04f31ddb415 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 11 Apr 2016 10:05:32 +0200 Subject: [PATCH 19/26] KEYCLOAK-2795 Testsuite fails sometimes with NPE in LockServiceFactory.getLockService --- .../conn/DefaultLiquibaseConnectionProvider.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java index ff3747b2a0..b9018f8991 100644 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/conn/DefaultLiquibaseConnectionProvider.java @@ -69,6 +69,7 @@ public class DefaultLiquibaseConnectionProvider implements LiquibaseConnectionPr protected void baseLiquibaseInitialization() { ServiceLocator sl = ServiceLocator.getInstance(); + sl.setResourceAccessor(new ClassLoaderResourceAccessor(getClass().getClassLoader())); if (!System.getProperties().containsKey("liquibase.scan.packages")) { if (sl.getPackages().remove("liquibase.core")) { @@ -85,6 +86,10 @@ public class DefaultLiquibaseConnectionProvider implements LiquibaseConnectionPr sl.getPackages().remove("liquibase.ext"); sl.getPackages().remove("liquibase.sdk"); + + String lockPackageName = DummyLockService.class.getPackage().getName(); + logger.debugf("Added package %s to liquibase", lockPackageName); + sl.addPackageToScan(lockPackageName); } LogFactory.setInstance(new LogWrapper()); @@ -129,10 +134,6 @@ public class DefaultLiquibaseConnectionProvider implements LiquibaseConnectionPr String changelog = (database instanceof DB2Database) ? LiquibaseJpaUpdaterProvider.DB2_CHANGELOG : LiquibaseJpaUpdaterProvider.CHANGELOG; logger.debugf("Using changelog file: %s", changelog); - // We wrap liquibase update in CustomLockService provided by DBLockProvider. No need to lock inside liquibase itself. - // NOTE: This can't be done in baseLiquibaseInitialization() as liquibase always restarts lock service - LockServiceFactory.getInstance().register(new DummyLockService()); - return new Liquibase(changelog, new ClassLoaderResourceAccessor(getClass().getClassLoader()), database); } From dc2d71489d111c1c37e1403f3c4b1c0a2677dffa Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 11 Apr 2016 18:08:10 +0200 Subject: [PATCH 20/26] KEYCLOAK-2769 Better error handling of expired code in IdentityBrokerService --- .../keycloak/services/messages/Messages.java | 2 + .../resources/IdentityBrokerService.java | 99 +++++++++++++------ .../OIDCKeyCloakServerBrokerBasicTest.java | 30 +++++- .../keycloak/testsuite/pages/ErrorPage.java | 9 ++ .../main/resources/theme/base/login/error.ftl | 2 +- .../login/messages/messages_en.properties | 1 + 6 files changed, 108 insertions(+), 35 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java index d9a2fe3577..c585c62c13 100755 --- a/services/src/main/java/org/keycloak/services/messages/Messages.java +++ b/services/src/main/java/org/keycloak/services/messages/Messages.java @@ -154,6 +154,8 @@ public class Messages { public static final String IDENTITY_PROVIDER_LINK_SUCCESS = "identityProviderLinkSuccess"; + public static final String STALE_CODE = "staleCodeMessage"; + public static final String IDENTITY_PROVIDER_NOT_UNIQUE = "identityProviderNotUniqueMessage"; public static final String REALM_SUPPORTS_NO_CREDENTIALS = "realmSupportsNoCredentialsMessage"; diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 4a9d99383c..9834edeb27 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -146,7 +146,12 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } try { - ClientSessionCode clientSessionCode = parseClientSessionCode(code); + ParsedCodeContext parsedCode = parseClientSessionCode(code); + if (parsedCode.response != null) { + return parsedCode.response; + } + + ClientSessionCode clientSessionCode = parsedCode.clientSessionCode; IdentityProvider identityProvider = getIdentityProvider(session, realmModel, providerId); Response response = identityProvider.performLogin(createAuthenticationRequest(providerId, clientSessionCode)); @@ -245,14 +250,14 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } public Response authenticated(BrokeredIdentityContext context) { - ClientSessionCode clientCode = null; IdentityProviderModel identityProviderConfig = context.getIdpConfig(); - try { - clientCode = parseClientSessionCode(context.getCode()); - } catch (Exception e) { - return redirectToErrorPage(Messages.IDENTITY_PROVIDER_AUTHENTICATION_FAILED, e, identityProviderConfig.getProviderId()); + ParsedCodeContext parsedCode = parseClientSessionCode(context.getCode()); + if (parsedCode.response != null) { + return parsedCode.response; } + ClientSessionCode clientCode = parsedCode.clientSessionCode; + String providerId = identityProviderConfig.getAlias(); if (!identityProviderConfig.isStoreToken()) { if (isDebugEnabled()) { @@ -329,8 +334,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @GET @Path("/after-first-broker-login") public Response afterFirstBrokerLogin(@QueryParam("code") String code) { - ClientSessionCode clientCode = parseClientSessionCode(code); - ClientSessionModel clientSession = clientCode.getClientSession(); + ParsedCodeContext parsedCode = parseClientSessionCode(code); + if (parsedCode.response != null) { + return parsedCode.response; + } + ClientSessionModel clientSession = parsedCode.clientSessionCode.getClientSession(); try { this.event.detail(Details.CODE_ID, clientSession.getId()) @@ -443,8 +451,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @GET @Path("/after-post-broker-login") public Response afterPostBrokerLoginFlow(@QueryParam("code") String code) { - ClientSessionCode clientCode = parseClientSessionCode(code); - ClientSessionModel clientSession = clientCode.getClientSession(); + ParsedCodeContext parsedCode = parseClientSessionCode(code); + if (parsedCode.response != null) { + return parsedCode.response; + } + ClientSessionModel clientSession = parsedCode.clientSessionCode.getClientSession(); try { SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromClientSession(clientSession, PostBrokerLoginConstants.PBL_BROKERED_IDENTITY_CONTEXT); @@ -530,20 +541,23 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override public Response cancelled(String code) { - ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { - return redirectToErrorPage(Messages.INVALID_CODE); + ParsedCodeContext parsedCode = parseClientSessionCode(code); + if (parsedCode.response != null) { + return parsedCode.response; } + ClientSessionCode clientCode = parsedCode.clientSessionCode; return browserAuthentication(clientCode.getClientSession(), null); } @Override public Response error(String code, String message) { - ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { - return redirectToErrorPage(Messages.INVALID_CODE); + ParsedCodeContext parsedCode = parseClientSessionCode(code); + if (parsedCode.response != null) { + return parsedCode.response; } + ClientSessionCode clientCode = parsedCode.clientSessionCode; + return browserAuthentication(clientCode.getClientSession(), message); } @@ -604,36 +618,41 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } - private ClientSessionCode parseClientSessionCode(String code) { + private ParsedCodeContext parseClientSessionCode(String code) { ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode != null && clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + if (clientCode != null) { ClientSessionModel clientSession = clientCode.getClientSession(); - if (clientSession != null) { - ClientModel client = clientSession.getClient(); + if (clientSession.getUserSession() != null) { + this.event.session(clientSession.getUserSession()); + } - if (client == null) { - throw new IdentityBrokerException("Invalid client"); - } + ClientModel client = clientSession.getClient(); + + if (client != null) { logger.debugf("Got authorization code from client [%s].", client.getClientId()); this.event.client(client); this.session.getContext().setClient(client); - if (clientSession.getUserSession() != null) { - this.event.session(clientSession.getUserSession()); + if (!clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + logger.debugf("Authorization code is not valid. Client session ID: %s, Client session's action: %s", clientSession.getId(), clientSession.getAction()); + Response staleCodeError = redirectToErrorPage(Messages.STALE_CODE); + return ParsedCodeContext.response(staleCodeError); } - } - if (isDebugEnabled()) { - logger.debugf("Authorization code is valid."); - } + if (isDebugEnabled()) { + logger.debugf("Authorization code is valid."); + } - return clientCode; + return ParsedCodeContext.clientSessionCode(clientCode); + } } - throw new IdentityBrokerException("Invalid code, please login again through your client."); + logger.debugf("Authorization code is not valid. Code: %s", code); + Response staleCodeError = redirectToErrorPage(Messages.STALE_CODE); + return ParsedCodeContext.response(staleCodeError); } private AuthenticationRequest createAuthenticationRequest(String providerId, ClientSessionCode clientSessionCode) { @@ -808,4 +827,22 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal this.session.getTransaction().rollback(); } } + + + private static class ParsedCodeContext { + private ClientSessionCode clientSessionCode; + private Response response; + + public static ParsedCodeContext clientSessionCode(ClientSessionCode clientSessionCode) { + ParsedCodeContext ctx = new ParsedCodeContext(); + ctx.clientSessionCode = clientSessionCode; + return ctx; + } + + public static ParsedCodeContext response(Response response) { + ParsedCodeContext ctx = new ParsedCodeContext(); + ctx.response = response; + return ctx; + } + } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index 68d903cafc..e2fbfa2217 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -44,6 +44,7 @@ import java.io.IOException; import java.util.List; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -142,7 +143,7 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP @Test - public void testAccessDeniedError() throws Exception { + public void testConsentDeniedWithExpiredClientSession() throws Exception { // Disable update profile setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF); @@ -169,9 +170,26 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP // Login to broker loginIDP("test-user"); - // Set time offset and manually enforce removeExpired sessions TODO: Will need custom REST endpoints for that on integration-arquillian + // Set time offset Time.setOffset(60); try { + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert error page with backToApplication link displayed + errorPage.assertCurrent(); + errorPage.clickBackToApplication(); + + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); + + + // Login to broker again + loginIDP("test-user"); + + // Set time offset and manually remove expiredSessions TODO: Will require custom endpoint when migrate to integration-arquillian + Time.setOffset(120); + brokerServerRule.stopSession(this.session, true); this.session = brokerServerRule.startSession(); @@ -184,8 +202,14 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP grantPage.assertCurrent(); grantPage.cancel(); - // Assert classic error page ( We're sorry ) displayed. TODO: Should be improved? See KEYCLOAK-2740 + // Assert error page without backToApplication link (clientSession expired and was removed on the server) errorPage.assertCurrent(); + try { + errorPage.clickBackToApplication(); + fail("Not expected to have link backToApplication available"); + } catch (NoSuchElementException nsee) { + // Expected; + } } finally { Time.setOffset(0); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java index b8213319c6..1cee3062e9 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java @@ -16,8 +16,10 @@ */ package org.keycloak.testsuite.pages; +import org.junit.Assert; import org.keycloak.testsuite.OAuthClient; import org.keycloak.testsuite.rule.WebResource; +import org.openqa.selenium.By; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -32,10 +34,17 @@ public class ErrorPage extends AbstractPage { @FindBy(className = "instruction") private WebElement errorMessage; + @FindBy(id = "backToApplication") + private WebElement backToApplicationLink; + public String getError() { return errorMessage.getText(); } + public void clickBackToApplication() { + backToApplicationLink.click(); + } + public boolean isCurrent() { return driver.getTitle().equals("We're sorry..."); } diff --git a/themes/src/main/resources/theme/base/login/error.ftl b/themes/src/main/resources/theme/base/login/error.ftl index 95de521af8..c069e26bd7 100755 --- a/themes/src/main/resources/theme/base/login/error.ftl +++ b/themes/src/main/resources/theme/base/login/error.ftl @@ -8,7 +8,7 @@

    ${message.summary}

    <#if client?? && client.baseUrl?has_content> -

    ${msg("backToApplication")}

    +

    ${msg("backToApplication")}

    diff --git a/themes/src/main/resources/theme/base/login/messages/messages_en.properties b/themes/src/main/resources/theme/base/login/messages/messages_en.properties index ee8a767095..ec02b1c7ce 100755 --- a/themes/src/main/resources/theme/base/login/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/login/messages/messages_en.properties @@ -202,6 +202,7 @@ invalidCodeMessage=An error occurred, please login again through your applicatio identityProviderUnexpectedErrorMessage=Unexpected error when authenticating with identity provider identityProviderNotFoundMessage=Could not find an identity provider with the identifier. identityProviderLinkSuccess=Your account was successfully linked with {0} account {1} . +staleCodeMessage=This page is no longer valid, please go back to your application and login again realmSupportsNoCredentialsMessage=Realm does not support any credential type. identityProviderNotUniqueMessage=Realm supports multiple identity providers. Could not determine which identity provider should be used to authenticate with. emailVerifiedMessage=Your email address has been verified. From d14867f785add7b1813ed0f9ca58e60abdea833f Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 11 Apr 2016 22:51:05 +0200 Subject: [PATCH 21/26] KEYCLOAK-2801 Redirected to login theme error page after failed social linking from account management --- .../admin/client/resource/UserResource.java | 11 + .../keycloak/services/messages/Messages.java | 2 + .../resources/IdentityBrokerService.java | 17 +- .../OIDCKeyCloakServerBrokerBasicTest.java | 82 ------- ...DCKeycloakServerBrokerWithConsentTest.java | 227 ++++++++++++++++++ .../account/messages/messages_en.properties | 1 + 6 files changed, 257 insertions(+), 83 deletions(-) create mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java index 51e9a459bd..b02dd0d9b2 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java @@ -17,6 +17,7 @@ package org.keycloak.admin.client.resource; +import org.jboss.resteasy.annotations.cache.NoCache; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; import org.keycloak.representations.idm.GroupRepresentation; @@ -35,6 +36,7 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.util.List; +import java.util.Map; /** * @author rodrigo.sasaki@icarros.com.br @@ -132,4 +134,13 @@ public interface UserResource { @Path("role-mappings") public RoleMappingResource roles(); + + @GET + @Path("consents") + public List> getConsents(); + + @DELETE + @Path("consents/{client}") + public void revokeConsent(@PathParam("client") String clientId); + } diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java index c585c62c13..7fae66d96b 100755 --- a/services/src/main/java/org/keycloak/services/messages/Messages.java +++ b/services/src/main/java/org/keycloak/services/messages/Messages.java @@ -156,6 +156,8 @@ public class Messages { public static final String STALE_CODE = "staleCodeMessage"; + public static final String STALE_CODE_ACCOUNT = "staleCodeAccountMessage"; + public static final String IDENTITY_PROVIDER_NOT_UNIQUE = "identityProviderNotUniqueMessage"; public static final String REALM_SUPPORTS_NO_CREDENTIALS = "realmSupportsNoCredentialsMessage"; diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 9834edeb27..414644f4e0 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -638,7 +638,22 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal if (!clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { logger.debugf("Authorization code is not valid. Client session ID: %s, Client session's action: %s", clientSession.getId(), clientSession.getAction()); - Response staleCodeError = redirectToErrorPage(Messages.STALE_CODE); + + Response staleCodeError; + + // Linking identityProvider from account mgmt + if (clientSession.getUserSession() != null && client.getClientId().equals(ACCOUNT_MANAGEMENT_CLIENT_ID)) { + + this.event.event(EventType.FEDERATED_IDENTITY_LINK); + UserModel user = clientSession.getUserSession().getUser(); + this.event.user(user); + this.event.detail(Details.USERNAME, user.getUsername()); + + staleCodeError = redirectToAccountErrorPage(clientSession, Messages.STALE_CODE_ACCOUNT); + } else { + staleCodeError = redirectToErrorPage(Messages.STALE_CODE); + } + return ParsedCodeContext.response(staleCodeError); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index e2fbfa2217..9110a90402 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -141,88 +141,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP keycloak.realm("realm-with-broker").identityProviders().get("kc-oidc-idp").update(idp); } - - @Test - public void testConsentDeniedWithExpiredClientSession() throws Exception { - // Disable update profile - setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF); - - Keycloak keycloak1 = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); - Keycloak keycloak2 = Keycloak.getInstance("http://localhost:8082/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); - - // Require broker to show consent screen - RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); - List clients = brokeredRealm.clients().findByClientId("broker-app"); - Assert.assertEquals(1, clients.size()); - ClientRepresentation brokerApp = clients.get(0); - brokerApp.setConsentRequired(true); - brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp); - - // Change timeouts on realm-with-broker to lower values - RealmResource realmWithBroker = keycloak1.realm("realm-with-broker"); - RealmRepresentation realmBackup = realmWithBroker.toRepresentation(); - RealmRepresentation realm = realmWithBroker.toRepresentation(); - realm.setAccessCodeLifespanLogin(30);; - realm.setAccessCodeLifespan(30); - realm.setAccessCodeLifespanUserAction(30); - realmWithBroker.update(realm); - - // Login to broker - loginIDP("test-user"); - - // Set time offset - Time.setOffset(60); - try { - // User rejected consent - grantPage.assertCurrent(); - grantPage.cancel(); - - // Assert error page with backToApplication link displayed - errorPage.assertCurrent(); - errorPage.clickBackToApplication(); - - assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); - - - // Login to broker again - loginIDP("test-user"); - - // Set time offset and manually remove expiredSessions TODO: Will require custom endpoint when migrate to integration-arquillian - Time.setOffset(120); - - brokerServerRule.stopSession(this.session, true); - this.session = brokerServerRule.startSession(); - - session.sessions().removeExpired(getRealm()); - - brokerServerRule.stopSession(this.session, true); - this.session = brokerServerRule.startSession(); - - // User rejected consent - grantPage.assertCurrent(); - grantPage.cancel(); - - // Assert error page without backToApplication link (clientSession expired and was removed on the server) - errorPage.assertCurrent(); - try { - errorPage.clickBackToApplication(); - fail("Not expected to have link backToApplication available"); - } catch (NoSuchElementException nsee) { - // Expected; - } - - } finally { - Time.setOffset(0); - } - - // Revert consent - brokerApp.setConsentRequired(false); - brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp); - - // Revert timeouts - realmWithBroker.update(realmBackup); - } - @Test public void testSuccessfulAuthenticationWithoutUpdateProfile() { super.testSuccessfulAuthenticationWithoutUpdateProfile(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java new file mode 100644 index 0000000000..c2606e3c0d --- /dev/null +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java @@ -0,0 +1,227 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.broker; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.common.util.Time; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.services.managers.RealmManager; +import org.keycloak.testsuite.KeycloakServer; +import org.keycloak.testsuite.pages.AccountFederatedIdentityPage; +import org.keycloak.testsuite.rule.AbstractKeycloakRule; +import org.keycloak.testsuite.rule.WebResource; +import org.openqa.selenium.NoSuchElementException; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * @author Marek Posolda + */ +public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityProviderTest { + + private static final int PORT = 8082; + + private static Keycloak keycloak1; + private static Keycloak keycloak2; + + @ClassRule + public static AbstractKeycloakRule oidcServerRule = new AbstractKeycloakRule() { + + @Override + protected void configureServer(KeycloakServer server) { + server.getConfig().setPort(PORT); + } + + @Override + protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { + server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json")); + + // Disable update profile + RealmModel realm = getRealm(session); + setUpdateProfileFirstLogin(realm, IdentityProviderRepresentation.UPFLM_OFF); + } + + @Override + protected String[] getTestRealms() { + return new String[] { "realm-with-oidc-identity-provider" }; + } + }; + + + @BeforeClass + public static void before() { + keycloak1 = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); + keycloak2 = Keycloak.getInstance("http://localhost:8082/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); + + // Require broker to show consent screen + RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); + List clients = brokeredRealm.clients().findByClientId("broker-app"); + Assert.assertEquals(1, clients.size()); + ClientRepresentation brokerApp = clients.get(0); + brokerApp.setConsentRequired(true); + brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp); + + + // Change timeouts on realm-with-broker to lower values + RealmResource realmWithBroker = keycloak1.realm("realm-with-broker"); + RealmRepresentation realmRep = realmWithBroker.toRepresentation(); + realmRep.setAccessCodeLifespanLogin(30);; + realmRep.setAccessCodeLifespan(30); + realmRep.setAccessCodeLifespanUserAction(30); + realmWithBroker.update(realmRep); + } + + + @Override + protected String getProviderId() { + return "kc-oidc-idp"; + } + + + // KEYCLOAK-2769 + @Test + public void testConsentDeniedWithExpiredClientSession() throws Exception { + // Login to broker + loginIDP("test-user"); + + // Set time offset + Time.setOffset(60); + try { + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert error page with backToApplication link displayed + errorPage.assertCurrent(); + errorPage.clickBackToApplication(); + + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); + + } finally { + Time.setOffset(0); + } + } + + + // KEYCLOAK-2769 + @Test + public void testConsentDeniedWithExpiredAndClearedClientSession() throws Exception { + // Login to broker again + loginIDP("test-user"); + + // Set time offset + Time.setOffset(60); + try { + // Manually remove expiredSessions TODO: Will require custom endpoint when migrate to integration-arquillian + brokerServerRule.stopSession(this.session, true); + this.session = brokerServerRule.startSession(); + + session.sessions().removeExpired(getRealm()); + + brokerServerRule.stopSession(this.session, true); + this.session = brokerServerRule.startSession(); + + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert error page without backToApplication link (clientSession expired and was removed on the server) + errorPage.assertCurrent(); + try { + errorPage.clickBackToApplication(); + fail("Not expected to have link backToApplication available"); + } catch (NoSuchElementException nsee) { + // Expected; + } + + } finally { + Time.setOffset(0); + } + } + + + // KEYCLOAK-2801 + @Test + public void testAccountManagementLinkingAndExpiredClientSession() throws Exception { + // Login as pedroigor to account management + accountFederatedIdentityPage.realm("realm-with-broker"); + accountFederatedIdentityPage.open(); + assertTrue(driver.getTitle().equals("Log in to realm-with-broker")); + loginPage.login("pedroigor", "password"); + assertTrue(accountFederatedIdentityPage.isCurrent()); + + // Link my "pedroigor" identity with "test-user" from brokered Keycloak + accountFederatedIdentityPage.clickAddProvider(getProviderId()); + + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/")); + this.loginPage.login("test-user", "password"); + + // Set time offset + Time.setOffset(60); + try { + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert account error page with "staleCodeAccount" error displayed + accountFederatedIdentityPage.assertCurrent(); + Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError()); + + + // Try to link one more time + accountFederatedIdentityPage.clickAddProvider(getProviderId()); + + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/")); + this.loginPage.login("test-user", "password"); + + Time.setOffset(120); + + // User granted consent + grantPage.assertCurrent(); + grantPage.accept(); + + // Assert account error page with "staleCodeAccount" error displayed + accountFederatedIdentityPage.assertCurrent(); + Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError()); + + } finally { + Time.setOffset(0); + + // Revoke consent + RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); + List users = brokeredRealm.users().search("test-user", 0, 1); + brokeredRealm.users().get(users.get(0).getId()).revokeConsent("broker-app"); + } + } +} diff --git a/themes/src/main/resources/theme/base/account/messages/messages_en.properties b/themes/src/main/resources/theme/base/account/messages/messages_en.properties index 5803146529..27a4565ba9 100755 --- a/themes/src/main/resources/theme/base/account/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/account/messages/messages_en.properties @@ -135,6 +135,7 @@ federatedIdentityRemovingLastProviderMessage=You can''t remove last federated id identityProviderRedirectErrorMessage=Failed to redirect to identity provider. identityProviderRemovedMessage=Identity provider removed successfully. identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user. +staleCodeAccountMessage=The page expired. Please try one more time accountDisabledMessage=Account is disabled, contact admin. From fc9506719368799782ff06245aa0428eac62bc3b Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 11 Apr 2016 23:28:16 +0200 Subject: [PATCH 22/26] KEYCLOAK-2802 NPE during identity broker cancelled from account mgmt --- .../admin/client/resource/UserResource.java | 1 - .../resources/IdentityBrokerService.java | 40 +++++++---- .../OIDCKeyCloakServerBrokerBasicTest.java | 6 -- ...DCKeycloakServerBrokerWithConsentTest.java | 71 ++++++++++++++----- .../account/messages/messages_en.properties | 3 +- 5 files changed, 83 insertions(+), 38 deletions(-) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java index b02dd0d9b2..df13c0b756 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java @@ -17,7 +17,6 @@ package org.keycloak.admin.client.resource; -import org.jboss.resteasy.annotations.cache.NoCache; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; import org.keycloak.representations.idm.GroupRepresentation; diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 414644f4e0..d186f285d6 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -547,6 +547,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } ClientSessionCode clientCode = parsedCode.clientSessionCode; + Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), Messages.CONSENT_DENIED); + if (accountManagementFailedLinking != null) { + return accountManagementFailedLinking; + } + return browserAuthentication(clientCode.getClientSession(), null); } @@ -558,6 +563,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } ClientSessionCode clientCode = parsedCode.clientSessionCode; + Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), message); + if (accountManagementFailedLinking != null) { + return accountManagementFailedLinking; + } + return browserAuthentication(clientCode.getClientSession(), message); } @@ -639,20 +649,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal if (!clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { logger.debugf("Authorization code is not valid. Client session ID: %s, Client session's action: %s", clientSession.getId(), clientSession.getAction()); - Response staleCodeError; + // Check if error happened during login or during linking from account management + Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), Messages.STALE_CODE_ACCOUNT); + Response staleCodeError = (accountManagementFailedLinking != null) ? accountManagementFailedLinking : redirectToErrorPage(Messages.STALE_CODE); - // Linking identityProvider from account mgmt - if (clientSession.getUserSession() != null && client.getClientId().equals(ACCOUNT_MANAGEMENT_CLIENT_ID)) { - - this.event.event(EventType.FEDERATED_IDENTITY_LINK); - UserModel user = clientSession.getUserSession().getUser(); - this.event.user(user); - this.event.detail(Details.USERNAME, user.getUsername()); - - staleCodeError = redirectToAccountErrorPage(clientSession, Messages.STALE_CODE_ACCOUNT); - } else { - staleCodeError = redirectToErrorPage(Messages.STALE_CODE); - } return ParsedCodeContext.response(staleCodeError); } @@ -670,6 +670,20 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return ParsedCodeContext.response(staleCodeError); } + private Response checkAccountManagementFailedLinking(ClientSessionModel clientSession, String error, Object... parameters) { + if (clientSession.getUserSession() != null && clientSession.getClient() != null && clientSession.getClient().getClientId().equals(ACCOUNT_MANAGEMENT_CLIENT_ID)) { + + this.event.event(EventType.FEDERATED_IDENTITY_LINK); + UserModel user = clientSession.getUserSession().getUser(); + this.event.user(user); + this.event.detail(Details.USERNAME, user.getUsername()); + + return redirectToAccountErrorPage(clientSession, error, parameters); + } else { + return null; + } + } + private AuthenticationRequest createAuthenticationRequest(String providerId, ClientSessionCode clientSessionCode) { ClientSessionModel clientSession = null; String relayState = null; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index 9110a90402..63cf639d2d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -17,16 +17,12 @@ package org.keycloak.testsuite.broker; -import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; import org.keycloak.admin.client.Keycloak; -import org.keycloak.admin.client.resource.RealmResource; -import org.keycloak.common.util.Time; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.representations.AccessTokenResponse; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.services.Urls; @@ -41,10 +37,8 @@ import org.openqa.selenium.NoSuchElementException; import javax.ws.rs.core.UriBuilder; import java.io.IOException; -import java.util.List; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java index c2606e3c0d..d648cdd538 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java @@ -20,15 +20,12 @@ package org.keycloak.testsuite.broker; import java.util.List; import org.junit.Assert; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.keycloak.admin.client.Keycloak; -import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.common.util.Time; -import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.ClientRepresentation; @@ -37,9 +34,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.KeycloakServer; -import org.keycloak.testsuite.pages.AccountFederatedIdentityPage; import org.keycloak.testsuite.rule.AbstractKeycloakRule; -import org.keycloak.testsuite.rule.WebResource; import org.openqa.selenium.NoSuchElementException; import static org.junit.Assert.assertTrue; @@ -175,11 +170,7 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro @Test public void testAccountManagementLinkingAndExpiredClientSession() throws Exception { // Login as pedroigor to account management - accountFederatedIdentityPage.realm("realm-with-broker"); - accountFederatedIdentityPage.open(); - assertTrue(driver.getTitle().equals("Log in to realm-with-broker")); - loginPage.login("pedroigor", "password"); - assertTrue(accountFederatedIdentityPage.isCurrent()); + loginToAccountManagement("pedroigor"); // Link my "pedroigor" identity with "test-user" from brokered Keycloak accountFederatedIdentityPage.clickAddProvider(getProviderId()); @@ -196,7 +187,7 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro // Assert account error page with "staleCodeAccount" error displayed accountFederatedIdentityPage.assertCurrent(); - Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError()); + Assert.assertEquals("The page expired. Please try one more time.", accountFederatedIdentityPage.getError()); // Try to link one more time @@ -213,15 +204,61 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro // Assert account error page with "staleCodeAccount" error displayed accountFederatedIdentityPage.assertCurrent(); - Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError()); + Assert.assertEquals("The page expired. Please try one more time.", accountFederatedIdentityPage.getError()); } finally { Time.setOffset(0); - - // Revoke consent - RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); - List users = brokeredRealm.users().search("test-user", 0, 1); - brokeredRealm.users().get(users.get(0).getId()).revokeConsent("broker-app"); } + + // Revoke consent + RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider"); + List users = brokeredRealm.users().search("test-user", 0, 1); + brokeredRealm.users().get(users.get(0).getId()).revokeConsent("broker-app"); + } + + + @Test + public void testLoginCancelConsent() throws Exception { + // Try to login + loginIDP("test-user"); + + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert back on login page + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/")); + assertTrue(driver.getTitle().equals("Log in to realm-with-broker")); + } + + + // KEYCLOAK-2802 + @Test + public void testAccountManagementLinkingCancelConsent() throws Exception { + // Login as pedroigor to account management + loginToAccountManagement("pedroigor"); + + // Link my "pedroigor" identity with "test-user" from brokered Keycloak + accountFederatedIdentityPage.clickAddProvider(getProviderId()); + + assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/")); + this.loginPage.login("test-user", "password"); + + // User rejected consent + grantPage.assertCurrent(); + grantPage.cancel(); + + // Assert account error page with "consentDenied" error displayed + accountFederatedIdentityPage.assertCurrent(); + Assert.assertEquals("Consent denied.", accountFederatedIdentityPage.getError()); + } + + + private void loginToAccountManagement(String username) { + accountFederatedIdentityPage.realm("realm-with-broker"); + accountFederatedIdentityPage.open(); + assertTrue(driver.getTitle().equals("Log in to realm-with-broker")); + loginPage.login(username, "password"); + assertTrue(accountFederatedIdentityPage.isCurrent()); } } diff --git a/themes/src/main/resources/theme/base/account/messages/messages_en.properties b/themes/src/main/resources/theme/base/account/messages/messages_en.properties index 27a4565ba9..8c0727fd79 100755 --- a/themes/src/main/resources/theme/base/account/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/account/messages/messages_en.properties @@ -135,7 +135,8 @@ federatedIdentityRemovingLastProviderMessage=You can''t remove last federated id identityProviderRedirectErrorMessage=Failed to redirect to identity provider. identityProviderRemovedMessage=Identity provider removed successfully. identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user. -staleCodeAccountMessage=The page expired. Please try one more time +staleCodeAccountMessage=The page expired. Please try one more time. +consentDenied=Consent denied. accountDisabledMessage=Account is disabled, contact admin. From 788bdfb29b621fb82666f12b058268df5d960f5f Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 12 Apr 2016 07:06:50 +0200 Subject: [PATCH 23/26] KEYCLOAK-2770 Close mail transport after sending message --- .../keycloak/email/DefaultEmailSenderProvider.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/email/DefaultEmailSenderProvider.java b/services/src/main/java/org/keycloak/email/DefaultEmailSenderProvider.java index dc35f392e1..a2469ec279 100644 --- a/services/src/main/java/org/keycloak/email/DefaultEmailSenderProvider.java +++ b/services/src/main/java/org/keycloak/email/DefaultEmailSenderProvider.java @@ -25,6 +25,7 @@ import org.keycloak.models.UserModel; import org.keycloak.services.ServicesLogger; import javax.mail.Message; +import javax.mail.MessagingException; import javax.mail.Multipart; import javax.mail.Session; import javax.mail.Transport; @@ -54,6 +55,7 @@ public class DefaultEmailSenderProvider implements EmailSenderProvider { @Override public void send(RealmModel realm, UserModel user, String subject, String textBody, String htmlBody) throws EmailException { + Transport transport = null; try { String address = user.getEmail(); Map config = realm.getSmtpConfig(); @@ -114,7 +116,7 @@ public class DefaultEmailSenderProvider implements EmailSenderProvider { msg.saveChanges(); msg.setSentDate(new Date()); - Transport transport = session.getTransport("smtp"); + transport = session.getTransport("smtp"); if (auth) { transport.connect(config.get("user"), config.get("password")); } else { @@ -124,6 +126,14 @@ public class DefaultEmailSenderProvider implements EmailSenderProvider { } catch (Exception e) { logger.failedToSendEmail(e); throw new EmailException(e); + } finally { + if (transport != null) { + try { + transport.close(); + } catch (MessagingException e) { + logger.warn("Failed to close transport", e); + } + } } } From 45231b69a7eff7e9f27b1ff95c03775c8773f21c Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 12 Apr 2016 08:04:06 +0200 Subject: [PATCH 24/26] KEYCLOAK-2803 Fix failure to add execution to client flow --- .../resources/admin/AuthenticationManagementResource.java | 8 +++++++- .../base/admin/resources/partials/create-execution.html | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java index 54378bf181..1e42aad8bf 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java @@ -389,10 +389,16 @@ public class AuthenticationManagementResource { String provider = data.get("provider"); // make sure provider is one of the registered providers - ProviderFactory f = session.getKeycloakSessionFactory().getProviderFactory(Authenticator.class, provider); + ProviderFactory f; + if (parentFlow.getProviderId().equals(AuthenticationFlow.CLIENT_FLOW)) { + f = session.getKeycloakSessionFactory().getProviderFactory(ClientAuthenticator.class, provider); + } else { + f = session.getKeycloakSessionFactory().getProviderFactory(Authenticator.class, provider); + } if (f == null) { throw new BadRequestException("No authentication provider found for id: " + provider); } + AuthenticationExecutionModel execution = new AuthenticationExecutionModel(); execution.setParentFlow(parentFlow.getId()); execution.setRequirement(AuthenticationExecutionModel.Requirement.DISABLED); diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/create-execution.html b/themes/src/main/resources/theme/base/admin/resources/partials/create-execution.html index 6956088ab5..dd333a9269 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/create-execution.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/create-execution.html @@ -1,6 +1,8 @@
    +

    {{:: 'create-authenticator-execution' | translate}}

    +

    {{:: 'create-authenticator-execution' | translate}}

    {{:: 'create-form-action-execution' | translate}}

    From a22c8c1d8e7e85130a46674db098b9da2ed7670b Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 12 Apr 2016 10:02:35 +0200 Subject: [PATCH 25/26] KEYCLOAK-2799 Show error for identity brokering login if user is disabled --- .../resources/IdentityBrokerService.java | 18 +++++ .../broker/AbstractIdentityProviderTest.java | 5 -- .../AbstractKeycloakIdentityProviderTest.java | 71 ++++++++++++++++++- .../OIDCKeyCloakServerBrokerBasicTest.java | 10 +++ .../keycloak/testsuite/pages/ErrorPage.java | 2 +- 5 files changed, 98 insertions(+), 8 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index d186f285d6..a9fb79f1a7 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -34,6 +34,7 @@ import org.keycloak.common.ClientConnection; import org.keycloak.common.util.ObjectUtil; import org.keycloak.common.util.Time; import org.keycloak.events.Details; +import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.forms.login.LoginFormsProvider; @@ -323,6 +324,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return Response.status(302).location(redirect).build(); } else { + Response response = validateUser(federatedUser, realmModel); + if (response != null) { + return response; + } + updateFederatedIdentity(context, federatedUser); clientSession.setAuthenticatedUser(federatedUser); @@ -330,6 +336,18 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } } + public Response validateUser(UserModel user, RealmModel realm) { + if (!user.isEnabled()) { + event.error(Errors.USER_DISABLED); + return ErrorPage.error(session, Messages.ACCOUNT_DISABLED); + } + if (realm.isBruteForceProtected()) { + event.error(Errors.USER_TEMPORARILY_DISABLED); + return ErrorPage.error(session, Messages.ACCOUNT_DISABLED); + } + return null; + } + // Callback from LoginActionsService after first login with broker was done and Keycloak account is successfully linked/created @GET @Path("/after-first-broker-login") diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java index f5f80f5011..4ccf3cbee0 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java @@ -172,10 +172,6 @@ public abstract class AbstractIdentityProviderTest { Time.setOffset(0); } - String afterLogoutUrl = driver.getCurrentUrl(); - String afterLogoutPageSource = driver.getPageSource(); - System.out.println("afterLogoutUrl: " + afterLogoutUrl); - //System.out.println("after logout page source: " + afterLogoutPageSource); driver.navigate().to("http://localhost:8081/test-app"); @@ -219,7 +215,6 @@ public abstract class AbstractIdentityProviderTest { String currentUrl = this.driver.getCurrentUrl(); assertTrue(currentUrl.startsWith("http://localhost:8082/auth/")); - System.out.println(this.driver.getCurrentUrl()); // log in to identity provider this.loginPage.login(username, "password"); doAfterProviderAuthentication(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java index 76911d252d..1af21f51f0 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java @@ -38,6 +38,7 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserFederationProviderModel; @@ -69,6 +70,73 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent Assert.assertEquals("617-666-7777", user.getFirstAttribute("mobile")); } + @Test + public void testDisabledUser() { + setUpdateProfileFirstLogin(session.realms().getRealmByName("realm-with-broker"), IdentityProviderRepresentation.UPFLM_OFF); + + driver.navigate().to("http://localhost:8081/test-app"); + loginPage.clickSocial(getProviderId()); + loginPage.login("test-user", "password"); + System.out.println(driver.getPageSource()); + driver.navigate().to("http://localhost:8081/test-app/logout"); + + try { + KeycloakSession session = brokerServerRule.startSession(); + session.users().getUserByUsername("test-user", session.realms().getRealmByName("realm-with-broker")).setEnabled(false); + brokerServerRule.stopSession(session, true); + + driver.navigate().to("http://localhost:8081/test-app"); + + loginPage.clickSocial(getProviderId()); + loginPage.login("test-user", "password"); + + assertTrue(errorPage.isCurrent()); + assertEquals("Account is disabled, contact admin.", errorPage.getError()); + } finally { + KeycloakSession session = brokerServerRule.startSession(); + session.users().getUserByUsername("test-user", session.realms().getRealmByName("realm-with-broker")).setEnabled(true); + brokerServerRule.stopSession(session, true); + } + } + + @Test + public void testTemporarilyDisabledUser() { + setUpdateProfileFirstLogin(session.realms().getRealmByName("realm-with-broker"), IdentityProviderRepresentation.UPFLM_OFF); + + driver.navigate().to("http://localhost:8081/test-app"); + loginPage.clickSocial(getProviderId()); + loginPage.login("test-user", "password"); + driver.navigate().to("http://localhost:8081/test-app/logout"); + + try { + KeycloakSession session = brokerServerRule.startSession(); + RealmModel brokerRealm = session.realms().getRealmByName("realm-with-broker"); + brokerRealm.setBruteForceProtected(true); + brokerRealm.setFailureFactor(2); + brokerServerRule.stopSession(session, true); + + driver.navigate().to("http://localhost:8081/test-app"); + loginPage.login("test-user", "fail"); + loginPage.login("test-user", "fail"); + + driver.navigate().to("http://localhost:8081/test-app"); + + assertTrue(driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); + + loginPage.clickSocial(getProviderId()); + loginPage.login("test-user", "password"); + + assertTrue(errorPage.isCurrent()); + assertEquals("Account is disabled, contact admin.", errorPage.getError()); + } finally { + KeycloakSession session = brokerServerRule.startSession(); + RealmModel brokerRealm = session.realms().getRealmByName("realm-with-broker"); + brokerRealm.setBruteForceProtected(false); + brokerRealm.setFailureFactor(0); + brokerServerRule.stopSession(session, true); + } + } + @Test public void testSuccessfulAuthenticationUpdateProfileOnMissing_nothingMissing() { IdentityProviderModel identityProviderModel = getIdentityProviderModel(); @@ -362,7 +430,6 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent revokeGrant(); // Logout from account management - System.out.println("*** logout from account management"); accountFederatedIdentityPage.logout(); assertTrue(driver.getTitle().equals("Log in to realm-with-broker")); assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); @@ -502,7 +569,7 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent driver.navigate().to("http://localhost:8081/test-app/logout"); String currentUrl = this.driver.getCurrentUrl(); - System.out.println("after logout currentUrl: " + currentUrl); +// System.out.println("after logout currentUrl: " + currentUrl); assertTrue(currentUrl.startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth")); unconfigureUserRetrieveToken("test-user"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index 63cf639d2d..562675138e 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -115,6 +115,16 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP super.testSuccessfulAuthentication(); } + @Test + public void testDisabledUser() { + super.testDisabledUser(); + } + + @Test + public void testTemporarilyDisabledUser() { + super.testTemporarilyDisabledUser(); + } + @Test public void testLogoutWorksWithTokenTimeout() { Keycloak keycloak = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java index 1cee3062e9..3077db4c89 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java @@ -46,7 +46,7 @@ public class ErrorPage extends AbstractPage { } public boolean isCurrent() { - return driver.getTitle().equals("We're sorry..."); + return driver.getTitle() != null && driver.getTitle().equals("We're sorry..."); } @Override From e159254cd22402aff6e60809eec2bfeed19b102c Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 12 Apr 2016 15:06:48 +0200 Subject: [PATCH 26/26] KEYCLOAK-2807 Fix server info providers page --- .../models/KeycloakSessionFactory.java | 4 ++ .../DefaultKeycloakSessionFactory.java | 8 +++ .../admin/info/ServerInfoAdminResource.java | 5 +- .../testsuite/admin/ServerInfoTest.java | 65 +++++++++++++++++++ .../testsuite/admin/ServerInfoTest.java | 61 ----------------- 5 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java delete mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java diff --git a/server-spi/src/main/java/org/keycloak/models/KeycloakSessionFactory.java b/server-spi/src/main/java/org/keycloak/models/KeycloakSessionFactory.java index 3aa15d810e..0028ae7b8f 100755 --- a/server-spi/src/main/java/org/keycloak/models/KeycloakSessionFactory.java +++ b/server-spi/src/main/java/org/keycloak/models/KeycloakSessionFactory.java @@ -20,8 +20,10 @@ package org.keycloak.models; import org.keycloak.provider.Provider; import org.keycloak.provider.ProviderEventManager; import org.keycloak.provider.ProviderFactory; +import org.keycloak.provider.Spi; import java.util.List; +import java.util.Set; /** * @author Bill Burke @@ -30,6 +32,8 @@ import java.util.List; public interface KeycloakSessionFactory extends ProviderEventManager { KeycloakSession create(); + Set getSpis(); + ProviderFactory getProviderFactory(Class clazz); ProviderFactory getProviderFactory(Class clazz, String id); diff --git a/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java b/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java index 3ca473b33d..90b495cc71 100755 --- a/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java +++ b/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java @@ -40,6 +40,7 @@ public class DefaultKeycloakSessionFactory implements KeycloakSessionFactory { private static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER; + private Set spis = new HashSet<>(); private Map, String> provider = new HashMap, String>(); private Map, Map> factoriesMap = new HashMap, Map>(); protected CopyOnWriteArrayList listeners = new CopyOnWriteArrayList(); @@ -80,6 +81,8 @@ public class DefaultKeycloakSessionFactory implements KeycloakSessionFactory { protected void loadSPIs(ProviderManager pm, ServiceLoader load) { for (Spi spi : load) { + spis.add(spi); + Map factories = new HashMap(); factoriesMap.put(spi.getProviderClass(), factories); @@ -138,6 +141,11 @@ public class DefaultKeycloakSessionFactory implements KeycloakSessionFactory { return provider.get(clazz); } + @Override + public Set getSpis() { + return spis; + } + @Override public ProviderFactory getProviderFactory(Class clazz) { return getProviderFactory(clazz, provider.get(clazz)); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java index 4454ccbfe6..3ea8fd13cf 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java @@ -95,10 +95,7 @@ public class ServerInfoAdminResource { private void setProviders(ServerInfoRepresentation info) { LinkedHashMap spiReps = new LinkedHashMap<>(); - List spis = new LinkedList<>(); - for (Spi spi : ServiceLoader.load(Spi.class)) { - spis.add(spi); - } + List spis = new LinkedList<>(session.getKeycloakSessionFactory().getSpis()); Collections.sort(spis, new Comparator() { @Override public int compare(Spi s1, Spi s2) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java new file mode 100644 index 0000000000..5e1a041b44 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.admin; + +import org.junit.Test; +import org.keycloak.common.Version; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.info.ServerInfoRepresentation; +import org.keycloak.testsuite.AbstractKeycloakTest; + +import java.util.List; + +import static org.junit.Assert.*; + +/** + * @author Stian Thorgersen + */ +public class ServerInfoTest extends AbstractKeycloakTest { + + @Test + public void testServerInfo() { + ServerInfoRepresentation info = adminClient.serverInfo().getInfo(); + assertNotNull(info); + + assertNotNull(info.getProviders()); + assertNotNull(info.getProviders().get("realm")); + assertNotNull(info.getProviders().get("user")); + assertNotNull(info.getProviders().get("authenticator")); + + assertNotNull(info.getThemes()); + assertNotNull(info.getThemes().get("account")); + assertNotNull(info.getThemes().get("admin")); + assertNotNull(info.getThemes().get("email")); + assertNotNull(info.getThemes().get("login")); + assertNotNull(info.getThemes().get("welcome")); + + assertNotNull(info.getEnums()); + + assertNotNull(info.getMemoryInfo()); + assertNotNull(info.getSystemInfo()); + + assertEquals(Version.VERSION, info.getSystemInfo().getVersion()); + assertNotNull(info.getSystemInfo().getServerTime()); + assertNotNull(info.getSystemInfo().getUptime()); + } + + @Override + public void addTestRealms(List testRealms) { + } +} diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java deleted file mode 100644 index a217a0480d..0000000000 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright 2016 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.testsuite.admin; - -import org.junit.Assert; -import org.junit.Rule; -import org.junit.Test; -import org.keycloak.common.Version; -import org.keycloak.representations.info.ServerInfoRepresentation; -import org.keycloak.testsuite.OAuthClient; -import org.keycloak.testsuite.rule.WebResource; -import org.keycloak.testsuite.rule.WebRule; -import org.openqa.selenium.WebDriver; - -/** - * @author Stian Thorgersen - */ -public class ServerInfoTest extends AbstractClientTest { - - @Rule - public WebRule webRule = new WebRule(this); - - @WebResource - protected WebDriver driver; - - @WebResource - protected OAuthClient oauth; - - @Test - public void testServerInfo() { - ServerInfoRepresentation info = keycloak.serverInfo().getInfo(); - - Assert.assertNotNull(info); - Assert.assertNotNull(info.getProviders()); - Assert.assertNotNull(info.getThemes()); - Assert.assertNotNull(info.getEnums()); - - Assert.assertNotNull(info.getMemoryInfo()); - Assert.assertNotNull(info.getSystemInfo()); - - Assert.assertEquals(Version.VERSION, info.getSystemInfo().getVersion()); - Assert.assertNotNull(info.getSystemInfo().getServerTime()); - Assert.assertNotNull(info.getSystemInfo().getUptime()); - } - -}