From dad12635f67e77b94127d1a7343a68b11972f54b Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Wed, 5 Dec 2018 22:01:48 +0100 Subject: [PATCH] KEYCLOAK-9014 Fix displayed applications --- .../org/keycloak/models/OrderedModel.java | 7 ++ .../freemarker/model/ApplicationsBean.java | 31 +++-- .../updaters/ClientAttributeUpdater.java | 65 +++++++--- .../updaters/RealmAttributeUpdater.java | 29 ++--- .../testsuite/updaters/RoleScopeUpdater.java | 63 ++++++++++ .../updaters/ServerResourceUpdater.java | 112 ++++++++++++++++++ .../updaters/UserAttributeUpdater.java | 40 ++++--- .../account/AccountFormServiceTest.java | 67 +++++++++-- .../servlet/OfflineServletsAdapterTest.java | 5 +- .../servlet/SAMLServletAdapterTest.java | 9 +- .../broker/KcSamlSignedBrokerTest.java | 20 +--- .../keycloak/testsuite/forms/LogoutTest.java | 5 +- .../testsuite/forms/ResetPasswordTest.java | 4 +- .../saml/IncludeOneTimeUseConditionTest.java | 7 +- .../keycloak/testsuite/saml/LogoutTest.java | 4 +- 15 files changed, 353 insertions(+), 115 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RoleScopeUpdater.java create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ServerResourceUpdater.java diff --git a/server-spi/src/main/java/org/keycloak/models/OrderedModel.java b/server-spi/src/main/java/org/keycloak/models/OrderedModel.java index 3134ea5c7b..603e7a5255 100644 --- a/server-spi/src/main/java/org/keycloak/models/OrderedModel.java +++ b/server-spi/src/main/java/org/keycloak/models/OrderedModel.java @@ -29,6 +29,13 @@ public interface OrderedModel { class OrderedModelComparator implements Comparator { + public static final OrderedModelComparator INSTANCE = new OrderedModelComparator(); + + @SuppressWarnings("unchecked") + public static OrderedModelComparator getInstance() { + return INSTANCE; + } + @Override public int compare(OM o1, OM o2) { int o1order = parseOrder(o1); diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java b/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java index 3a5aa86c1f..baaa5257cd 100755 --- a/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java +++ b/services/src/main/java/org/keycloak/forms/account/freemarker/model/ApplicationsBean.java @@ -18,13 +18,11 @@ package org.keycloak.forms.account.freemarker.model; import org.keycloak.common.util.MultivaluedHashMap; -import org.keycloak.forms.login.freemarker.model.OAuthGrantBean; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrderedModel; -import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserConsentModel; @@ -39,7 +37,6 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Set; -import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -47,36 +44,44 @@ import java.util.stream.Collectors; */ public class ApplicationsBean { - private List applications = new LinkedList(); + private List applications = new LinkedList<>(); public ApplicationsBean(KeycloakSession session, RealmModel realm, UserModel user) { Set offlineClients = new UserSessionManager(session).findClientsWithOfflineToken(realm, user); for (ClientModel client : getApplications(session, realm, user)) { - Set availableRoles = new HashSet<>(); + if (isAdminClient(client) && ! AdminPermissions.realms(session, realm, user).isAdmin()) { + continue; + } // Construct scope parameter with all optional scopes to see all potentially available roles Set allClientScopes = new HashSet<>(client.getClientScopes(true, true).values()); allClientScopes.addAll(client.getClientScopes(false, true).values()); allClientScopes.add(client); - availableRoles = TokenManager.getAccess(user, client, allClientScopes); + Set availableRoles = TokenManager.getAccess(user, client, allClientScopes); + + // Don't show applications, which user doesn't have access into (any available roles) + // unless this is can be changed by approving/revoking consent + if (! isAdminClient(client) && availableRoles.isEmpty() && ! client.isConsentRequired()) { + continue; + } List realmRolesAvailable = new LinkedList<>(); MultivaluedHashMap resourceRolesAvailable = new MultivaluedHashMap<>(); processRoles(availableRoles, realmRolesAvailable, resourceRolesAvailable); - List orderedScopes = new ArrayList<>(); + List orderedScopes = new LinkedList<>(); if (client.isConsentRequired()) { UserConsentModel consent = session.users().getConsentByClient(realm, user.getId(), client.getId()); if (consent != null) { orderedScopes.addAll(consent.getGrantedClientScopes()); - orderedScopes.sort(new OrderedModel.OrderedModelComparator<>()); } } List clientScopesGranted = orderedScopes.stream() + .sorted(OrderedModel.OrderedModelComparator.getInstance()) .map(ClientScopeModel::getConsentScreenText) .collect(Collectors.toList()); @@ -89,6 +94,11 @@ public class ApplicationsBean { } } + public static boolean isAdminClient(ClientModel client) { + return client.getClientId().equals(Constants.ADMIN_CLI_CLIENT_ID) + || client.getClientId().equals(Constants.ADMIN_CONSOLE_CLIENT_ID); + } + private Set getApplications(KeycloakSession session, RealmModel realm, UserModel user) { Set clients = new HashSet<>(); @@ -98,11 +108,6 @@ public class ApplicationsBean { continue; } - if (client.getClientId().equals(Constants.ADMIN_CLI_CLIENT_ID) - || client.getClientId().equals(Constants.ADMIN_CONSOLE_CLIENT_ID)) { - if (!AdminPermissions.realms(session, realm, user).isAdmin()) continue; - } - clients.add(client); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java index 106aa5b0b2..0445b5eb1b 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java @@ -3,10 +3,14 @@ package org.keycloak.testsuite.updaters; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.ClientRepresentation; -import java.io.Closeable; +import org.keycloak.representations.idm.ClientScopeRepresentation; import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; @@ -14,29 +18,40 @@ import static org.junit.Assert.assertThat; * * @author hmlnarik */ -public class ClientAttributeUpdater { +public class ClientAttributeUpdater extends ServerResourceUpdater { - private final ClientResource clientResource; - - private final ClientRepresentation rep; - private final ClientRepresentation origRep; + private final RealmResource realmResource; public static ClientAttributeUpdater forClient(Keycloak adminClient, String realm, String clientId) { - ClientsResource clients = adminClient.realm(realm).clients(); + RealmResource realmRes = adminClient.realm(realm); + ClientsResource clients = realmRes.clients(); List foundClients = clients.findByClientId(clientId); assertThat(foundClients, hasSize(1)); ClientResource clientRes = clients.get(foundClients.get(0).getId()); - return new ClientAttributeUpdater(clientRes); + return new ClientAttributeUpdater(clientRes, realmRes); } - public ClientAttributeUpdater(ClientResource clientResource) { - this.clientResource = clientResource; - this.origRep = clientResource.toRepresentation(); - this.rep = clientResource.toRepresentation(); + private ClientAttributeUpdater(ClientResource resource, RealmResource realmResource) { + super(resource, resource::toRepresentation, resource::update); if (this.rep.getAttributes() == null) { this.rep.setAttributes(new HashMap<>()); } + this.realmResource = realmResource; + } + + @Override + protected void performUpdate(ClientRepresentation from, ClientRepresentation to) { + super.performUpdate(from, to); + updateViaAddRemove(from.getDefaultClientScopes(), to.getDefaultClientScopes(), this::getConversionForScopeNameToId, resource::addDefaultClientScope, resource::removeDefaultClientScope); + updateViaAddRemove(from.getOptionalClientScopes(), to.getOptionalClientScopes(), this::getConversionForScopeNameToId, resource::addOptionalClientScope, resource::removeOptionalClientScope); + } + + private Function getConversionForScopeNameToId() { + Map scopeNameToIdMap = realmResource.clientScopes().findAll().stream() + .collect(Collectors.toMap(ClientScopeRepresentation::getName, ClientScopeRepresentation::getId)); + + return scopeNameToIdMap::get; } public ClientAttributeUpdater setClientId(String clientId) { @@ -64,9 +79,27 @@ public class ClientAttributeUpdater { return this; } - public Closeable update() { - clientResource.update(rep); - - return () -> clientResource.update(origRep); + public ClientAttributeUpdater setFullScopeAllowed(Boolean fullScopeAllowed) { + rep.setFullScopeAllowed(fullScopeAllowed); + return this; } + + public ClientAttributeUpdater setDefaultClientScopes(List defaultClientScopes) { + rep.setDefaultClientScopes(defaultClientScopes); + return this; + } + + public ClientAttributeUpdater setOptionalClientScopes(List optionalClientScopes) { + rep.setOptionalClientScopes(optionalClientScopes); + return this; + } + + public RoleScopeUpdater realmRoleScope() { + return new RoleScopeUpdater(resource.getScopeMappings().realmLevel()); + } + + public RoleScopeUpdater clientRoleScope(String clientUUID) { + return new RoleScopeUpdater(resource.getScopeMappings().clientLevel(clientUUID)); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java index 1b9df30fc0..fad4f9a02b 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java @@ -2,35 +2,22 @@ package org.keycloak.testsuite.updaters; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.RealmRepresentation; -import java.io.Closeable; import java.util.HashMap; -import java.util.function.Consumer; +import java.util.List; /** * * @author hmlnarik */ -public class RealmAttributeUpdater { +public class RealmAttributeUpdater extends ServerResourceUpdater { - private final RealmResource realmResource; - - private final RealmRepresentation rep; - private final RealmRepresentation origRep; - - public RealmAttributeUpdater(RealmResource realmResource) { - this.realmResource = realmResource; - this.origRep = realmResource.toRepresentation(); - this.rep = realmResource.toRepresentation(); + public RealmAttributeUpdater(RealmResource resource) { + super(resource, resource::toRepresentation, resource::update); if (this.rep.getAttributes() == null) { this.rep.setAttributes(new HashMap<>()); } } - public RealmAttributeUpdater updateWith(Consumer updater) { - updater.accept(this.rep); - return this; - } - public RealmAttributeUpdater setAttribute(String name, String value) { this.rep.getAttributes().put(name, value); return this; @@ -51,9 +38,9 @@ public class RealmAttributeUpdater { return this; } - public Closeable update() { - realmResource.update(rep); - - return () -> realmResource.update(origRep); + public RealmAttributeUpdater setDefaultDefaultClientScopes(List defaultClientScopes) { + rep.setDefaultDefaultClientScopes(defaultClientScopes); + return this; } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RoleScopeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RoleScopeUpdater.java new file mode 100644 index 0000000000..dd295b0949 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RoleScopeUpdater.java @@ -0,0 +1,63 @@ +/* + * Copyright 2018 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.updaters; + +import org.keycloak.admin.client.resource.RoleScopeResource; +import org.keycloak.representations.idm.RoleRepresentation; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * + * @author hmlnarik + */ +public class RoleScopeUpdater extends ServerResourceUpdater> { + + public RoleScopeUpdater(RoleScopeResource resource) { + super(resource, resource::listAll, null); + this.updater = this::update; + } + + public RoleScopeUpdater add(RoleRepresentation representation) { + rep.add(representation); + return this; + } + + public RoleScopeUpdater remove(RoleRepresentation representation) { + rep.add(representation); + return this; + } + + private void update(List expectedRoles) { + List currentRoles = resource.listAll(); + + Set currentRoleIds = currentRoles.stream().map(RoleRepresentation::getId).collect(Collectors.toSet()); + Set expectedRoleIds = expectedRoles.stream().map(RoleRepresentation::getId).collect(Collectors.toSet()); + + List toAdd = expectedRoles.stream() + .filter(role -> ! currentRoleIds.contains(role.getId())) + .collect(Collectors.toList()); + List toRemove = currentRoles.stream() + .filter(role -> ! expectedRoleIds.contains(role.getId())) + .collect(Collectors.toList()); + + resource.add(toAdd); + resource.remove(toRemove); + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ServerResourceUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ServerResourceUpdater.java new file mode 100644 index 0000000000..d8d06213d5 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ServerResourceUpdater.java @@ -0,0 +1,112 @@ +/* + * Copyright 2018 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.updaters; + +import java.io.Closeable; +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +/** + * + * @author hmlnarik + */ +public abstract class ServerResourceUpdater implements Closeable { + + protected final Res resource; + protected final Rep rep; + protected final Rep origRep; + protected Consumer updater; + protected boolean updated = false; + + public ServerResourceUpdater(Res resource, Supplier representationGenerator, Consumer updater) { + this.resource = resource; + this.updater = updater; + + // origRep and rep need to be two different instances + this.origRep = representationGenerator.get(); + this.rep = representationGenerator.get(); + } + + public Res getResource() { + return resource; + } + + public T update() { + performUpdate(origRep, rep); + this.updated = true; + return (T) this; + } + + protected void performUpdate(Rep from, Rep to) { + updater.accept(to); + } + + public T updateWith(Consumer representationUpdater) { + representationUpdater.accept(this.rep); + return (T) this; + } + + @Override + public void close() throws IOException { + if (! this.updated) { + throw new IOException("Attempt to revert changes that were never applied - have you called " + this.getClass().getName() + ".update()?"); + } + performUpdate(rep, origRep); + } + + /** + * This function performs a set of single {@code add} and {@code remove} operations that represent the changes needed to + * get collection {@code from} to the state of collection {@code to}. Since this is intended to work on collections of + * names of objects but the {@code add} and {@code remove} functions operate on IDs of those objects, a conversion + * is performed. + * + * @param Type of the objects in the collections (e.g. names) + * @param Type of the objects required by add/remove functions (e.g. IDs) + * @param from Initial collection + * @param to Target collection + * @param client2ServerConvertorGenerator Producer of the convertor. If not needed, just use {@code () -> Functions::identity}. + * This is intentionally a lazy-evaluated function variable because the conversion map often needs to be obtained from the + * server which can be slow operation. This function is called only if the two collections differ. + * @param add Function to add + * @param remove Function to remove + */ + public static void updateViaAddRemove(Collection from, Collection to, Supplier> client2ServerConvertorGenerator, Consumer add, Consumer remove) { + if (Objects.equals(from, to)) { + return; + } + + Function client2ServerConvertor = client2ServerConvertorGenerator.get(); + + Set current = from == null ? Collections.EMPTY_SET : from.stream().map(client2ServerConvertor).collect(Collectors.toSet()); + Set expected = to == null ? Collections.EMPTY_SET : to.stream().map(client2ServerConvertor).collect(Collectors.toSet()); + + expected.stream() + .filter(role -> ! current.contains(role)) + .forEach(add); + current.stream() + .filter(role -> ! expected.contains(role)) + .forEach(remove); + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java index be1182ab35..9603dadab2 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java @@ -1,29 +1,37 @@ package org.keycloak.testsuite.updaters; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.models.UserModel; +import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.UserRepresentation; -import java.io.Closeable; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.stream.Collectors; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertThat; /** * * @author hmlnarik */ -public class UserAttributeUpdater { +public class UserAttributeUpdater extends ServerResourceUpdater { - private final UserResource userResource; + public static UserAttributeUpdater forUserByUsername(Keycloak adminClient, String realm, String userName) { + UsersResource users = adminClient.realm(realm).users(); + List foundUsers = users.search(userName); + assertThat(foundUsers, hasSize(1)); + UserResource userRes = users.get(foundUsers.get(0).getId()); - private final UserRepresentation rep; - private final UserRepresentation origRep; + return new UserAttributeUpdater(userRes); + } - public UserAttributeUpdater(UserResource userResource) { - this.userResource = userResource; - this.origRep = userResource.toRepresentation(); - this.rep = userResource.toRepresentation(); + public UserAttributeUpdater(UserResource resource) { + super(resource, resource::toRepresentation, resource::update); if (this.rep.getAttributes() == null) { this.rep.setAttributes(new HashMap<>()); } @@ -49,12 +57,6 @@ public class UserAttributeUpdater { return this; } - public Closeable update() { - userResource.update(rep); - - return () -> userResource.update(origRep); - } - public UserAttributeUpdater setRequiredActions(UserModel.RequiredAction... requiredAction) { rep.setRequiredActions(Arrays.stream(requiredAction) .map(action -> action.name()) @@ -62,4 +64,12 @@ public class UserAttributeUpdater { ); return this; } + + public RoleScopeUpdater realmRoleScope() { + return new RoleScopeUpdater(resource.roles().realmLevel()); + } + + public RoleScopeUpdater clientRoleScope(String clientUUID) { + return new RoleScopeUpdater(resource.roles().clientLevel(clientUUID)); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java index cfae10337d..755a0422b6 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java @@ -57,11 +57,16 @@ import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.RegisterPage; +import org.keycloak.testsuite.updaters.ClientAttributeUpdater; +import org.keycloak.testsuite.updaters.RoleScopeUpdater; import org.keycloak.testsuite.util.IdentityProviderBuilder; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.UIUtils; import org.keycloak.testsuite.util.UserBuilder; +import java.io.Closeable; +import java.io.IOException; +import java.util.Collections; import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; @@ -86,6 +91,9 @@ import static org.junit.Assert.assertTrue; */ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest { + public static final String ROOT_URL_CLIENT = "root-url-client"; + public static final String REALM_NAME = "test"; + @Override public void configureTestRealm(RealmRepresentation testRealm) { //UserRepresentation user = findUserInRealmRep(testRealm, "test-user@localhost"); @@ -1067,7 +1075,50 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest { events.clear(); } + @Test + public void applicationsVisibilityNoScopesNoConsent() throws Exception { + try (ClientAttributeUpdater cau = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, ROOT_URL_CLIENT) + .setConsentRequired(false) + .setFullScopeAllowed(false) + .setDefaultClientScopes(Collections.EMPTY_LIST) + .setOptionalClientScopes(Collections.EMPTY_LIST) + .update(); + RoleScopeUpdater rsu = cau.realmRoleScope().update()) { + applicationsPage.open(); + loginPage.login("john-doh@localhost", "password"); + applicationsPage.assertCurrent(); + Map apps = applicationsPage.getApplications(); + Assert.assertThat(apps.keySet(), containsInAnyOrder( + /* "root-url-client", */ "Account", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant")); + + rsu.add(testRealm().roles().get("user").toRepresentation()) + .update(); + + driver.navigate().refresh(); + apps = applicationsPage.getApplications(); + Assert.assertThat(apps.keySet(), containsInAnyOrder( + "root-url-client", "Account", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant")); + } + } + + @Test + public void applicationsVisibilityNoScopesAndConsent() throws Exception { + try (ClientAttributeUpdater cau = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, ROOT_URL_CLIENT) + .setConsentRequired(true) + .setFullScopeAllowed(false) + .setDefaultClientScopes(Collections.EMPTY_LIST) + .setOptionalClientScopes(Collections.EMPTY_LIST) + .update()) { + applicationsPage.open(); + loginPage.login("john-doh@localhost", "password"); + applicationsPage.assertCurrent(); + + Map apps = applicationsPage.getApplications(); + Assert.assertThat(apps.keySet(), containsInAnyOrder( + "root-url-client", "Account", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant")); + } + } // More tests (including revoke) are in OAuthGrantTest and OfflineTokenTest @Test @@ -1076,19 +1127,19 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest { loginPage.login("test-user@localhost", "password"); events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=applications").assertEvent(); - Assert.assertTrue(applicationsPage.isCurrent()); + applicationsPage.assertCurrent(); Map apps = applicationsPage.getApplications(); Assert.assertThat(apps.keySet(), containsInAnyOrder("root-url-client", "Account", "Broker", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant")); AccountApplicationsPage.AppEntry accountEntry = apps.get("Account"); - Assert.assertEquals(4, accountEntry.getRolesAvailable().size()); - Assert.assertTrue(accountEntry.getRolesAvailable().contains("Manage account links in Account")); - Assert.assertTrue(accountEntry.getRolesAvailable().contains("Manage account in Account")); - Assert.assertTrue(accountEntry.getRolesAvailable().contains("View profile in Account")); - Assert.assertTrue(accountEntry.getRolesAvailable().contains("Offline access")); - Assert.assertEquals(1, accountEntry.getClientScopesGranted().size()); - Assert.assertTrue(accountEntry.getClientScopesGranted().contains("Full Access")); + Assert.assertThat(accountEntry.getRolesAvailable(), containsInAnyOrder( + "Manage account links in Account", + "Manage account in Account", + "View profile in Account", + "Offline access" + )); + Assert.assertThat(accountEntry.getClientScopesGranted(), containsInAnyOrder("Full Access")); Assert.assertEquals("http://localhost:8180/auth/realms/test/account", accountEntry.getHref()); AccountApplicationsPage.AppEntry testAppEntry = apps.get("test-app"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java index e278e96442..58fca76f16 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java @@ -24,6 +24,7 @@ import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.util.ClientManager; import org.keycloak.testsuite.utils.io.IOUtil; import org.keycloak.util.TokenUtil; +import org.hamcrest.Matchers; import org.openqa.selenium.By; import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; @@ -181,8 +182,8 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { accountAppPage.open(); AccountApplicationsPage.AppEntry offlineClient = accountAppPage.getApplications().get("offline-client"); - Assert.assertTrue(offlineClient.getClientScopesGranted().contains(OAuthGrantPage.OFFLINE_ACCESS_CONSENT_TEXT)); - Assert.assertTrue(offlineClient.getAdditionalGrants().contains("Offline Token")); + Assert.assertThat(offlineClient.getClientScopesGranted(), Matchers.hasItem(OAuthGrantPage.OFFLINE_ACCESS_CONSENT_TEXT)); + Assert.assertThat(offlineClient.getAdditionalGrants(), Matchers.hasItem("Offline Token")); //This was necessary to be introduced, otherwise other testcases will fail offlineTokenPage.logout(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java index 431efb4045..1d59eedfd3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java @@ -818,8 +818,7 @@ public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { @Test public void salesPostEncSignedAssertionsAndDocumentTest() throws Exception { - ClientRepresentation salesPostEncClient = testRealmResource().clients().findByClientId(SalesPostEncServlet.CLIENT_NAME).get(0); - try (Closeable client = new ClientAttributeUpdater(testRealmResource().clients().get(salesPostEncClient.getId())) + try (Closeable client = ClientAttributeUpdater.forClient(adminClient, testRealmPage.getAuthRealm(), SalesPostEncServlet.CLIENT_NAME) .setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, "true") .setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "true") .update()) { @@ -831,8 +830,7 @@ public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { @Test public void salesPostEncRejectConsent() throws Exception { - ClientRepresentation salesPostEncClient = testRealmResource().clients().findByClientId(SalesPostEncServlet.CLIENT_NAME).get(0); - try (Closeable client = new ClientAttributeUpdater(testRealmResource().clients().get(salesPostEncClient.getId())) + try (Closeable client = ClientAttributeUpdater.forClient(adminClient, testRealmPage.getAuthRealm(), SalesPostEncServlet.CLIENT_NAME) .setConsentRequired(true) .update()) { new SamlClientBuilder() @@ -853,8 +851,7 @@ public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { @Test public void salesPostRejectConsent() throws Exception { - ClientRepresentation salesPostClient = testRealmResource().clients().findByClientId(SalesPostServlet.CLIENT_NAME).get(0); - try (Closeable client = new ClientAttributeUpdater(testRealmResource().clients().get(salesPostClient.getId())) + try (Closeable client = ClientAttributeUpdater.forClient(adminClient, testRealmPage.getAuthRealm(), SalesPostServlet.CLIENT_NAME) .setConsentRequired(true) .update()) { new SamlClientBuilder() diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java index 486a4e5444..b8e7d24001 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java @@ -117,14 +117,6 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest { } public void withSignedEncryptedAssertions(Runnable testBody, boolean signedAssertion, boolean encryptedAssertion) throws Exception { - ClientRepresentation client = adminClient.realm(bc.providerRealmName()) - .clients() - .findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext)) - .get(0); - - final ClientResource clientResource = realmsResouce().realm(bc.providerRealmName()).clients().get(client.getId()); - Assert.assertThat(clientResource, Matchers.notNullValue()); - String providerCert = KeyUtils.getActiveKey(adminClient.realm(bc.providerRealmName()).keys().getKeyMetadata(), Algorithm.RS256).getCertificate(); Assert.assertThat(providerCert, Matchers.notNullValue()); @@ -138,7 +130,7 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest { .setAttribute(SAMLIdentityProviderConfig.WANT_AUTHN_REQUESTS_SIGNED, "false") .setAttribute(SAMLIdentityProviderConfig.SIGNING_CERTIFICATE_KEY, providerCert) .update(); - Closeable clientUpdater = new ClientAttributeUpdater(clientResource) + Closeable clientUpdater = ClientAttributeUpdater.forClient(adminClient, bc.providerRealmName(), bc.getIDPClientIdInProviderRealm(suiteContext)) .setAttribute(SamlConfigAttributes.SAML_ENCRYPT, Boolean.toString(encryptedAssertion)) .setAttribute(SamlConfigAttributes.SAML_ENCRYPTION_CERTIFICATE_ATTRIBUTE, consumerCert) .setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false") // only sign assertions @@ -269,14 +261,6 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest { @Test public void testWithExpiredBrokerCertificate() throws Exception { - ClientRepresentation client = adminClient.realm(bc.providerRealmName()) - .clients() - .findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext)) - .get(0); - - final ClientResource clientResource = realmsResouce().realm(bc.providerRealmName()).clients().get(client.getId()); - Assert.assertThat(clientResource, Matchers.notNullValue()); - try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource) .setAttribute(SAMLIdentityProviderConfig.VALIDATE_SIGNATURE, Boolean.toString(true)) .setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_SIGNED, Boolean.toString(true)) @@ -284,7 +268,7 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest { .setAttribute(SAMLIdentityProviderConfig.WANT_AUTHN_REQUESTS_SIGNED, "true") .setAttribute(SAMLIdentityProviderConfig.SIGNING_CERTIFICATE_KEY, AbstractSamlTest.SAML_CLIENT_SALES_POST_SIG_EXPIRED_CERTIFICATE) .update(); - Closeable clientUpdater = new ClientAttributeUpdater(clientResource) + Closeable clientUpdater = ClientAttributeUpdater.forClient(adminClient, bc.providerRealmName(), bc.getIDPClientIdInProviderRealm(suiteContext)) .setAttribute(SamlConfigAttributes.SAML_ENCRYPT, Boolean.toString(false)) .setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "true") .setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, Boolean.toString(true)) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java index 54e0714850..a4b29f169c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java @@ -244,11 +244,8 @@ public class LogoutTest extends AbstractTestRealmKeycloakTest { // KEYCLOAK-5982 @Test public void testLogoutWhenAccountClientRenamed() throws IOException { - // Rename client "account" - ClientResource accountClient = ApiUtil.findClientByClientId(adminClient.realm("test"), Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); - // Temporarily rename client "account" . Revert it back after the test - try (Closeable accountClientUpdater = new ClientAttributeUpdater(accountClient) + try (Closeable accountClientUpdater = ClientAttributeUpdater.forClient(adminClient, "test", Constants.ACCOUNT_MANAGEMENT_CLIENT_ID) .setClientId("account-changed") .update()) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 5f36170e05..2774b150a3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -1011,10 +1011,8 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { // KEYCLOAK-5982 @Test public void resetPasswordLinkOpenedInNewBrowserAndAccountClientRenamed() throws IOException, MessagingException { - ClientResource accountClient = ApiUtil.findClientByClientId(adminClient.realm("test"), Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); - // Temporarily rename client "account" . Revert it back after the test - try (Closeable accountClientUpdater = new ClientAttributeUpdater(accountClient) + try (Closeable accountClientUpdater = ClientAttributeUpdater.forClient(adminClient, "test", Constants.ACCOUNT_MANAGEMENT_CLIENT_ID) .setClientId("account-changed") .update()) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/IncludeOneTimeUseConditionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/IncludeOneTimeUseConditionTest.java index 5c9ee3e296..0562efe0ab 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/IncludeOneTimeUseConditionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/IncludeOneTimeUseConditionTest.java @@ -59,12 +59,7 @@ public class IncludeOneTimeUseConditionTest extends AbstractSamlTest private void testOneTimeUseConditionIncluded(Boolean oneTimeUseConditionShouldBeIncluded) throws IOException { - ClientsResource clients = adminClient.realm(REALM_NAME).clients(); - List foundClients = clients.findByClientId(SAML_CLIENT_ID_SALES_POST); - assertThat(foundClients, hasSize(1)); - ClientResource clientRes = clients.get(foundClients.get(0).getId()); - - try (Closeable c = new ClientAttributeUpdater(clientRes) + try (Closeable c = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) .setAttribute(SamlConfigAttributes.SAML_ONETIMEUSE_CONDITION, oneTimeUseConditionShouldBeIncluded.toString()) .update()) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java index 09fb266d6c..69e4ecd25a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java @@ -16,7 +16,6 @@ */ package org.keycloak.testsuite.saml; -import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.broker.saml.SAMLIdentityProviderConfig; import org.keycloak.broker.saml.SAMLIdentityProviderFactory; @@ -361,10 +360,9 @@ public class LogoutTest extends AbstractSamlTest { @Test public void testLogoutPropagatesToSamlIdentityProvider() throws IOException { final RealmResource realm = adminClient.realm(REALM_NAME); - final ClientsResource clients = realm.clients(); try ( - Closeable sales = new ClientAttributeUpdater(clients.get(salesRep.getId())) + Closeable sales = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) .setFrontchannelLogout(true) .removeAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE) .setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "http://url")