From a8e74196d1ca8a360bea52e9f76cb056d859f68f Mon Sep 17 00:00:00 2001 From: rmartinc Date: Fri, 13 Sep 2019 16:23:51 +0200 Subject: [PATCH] KEYCLOAK-4923: Client Service Account Roles are not exported --- .../exportimport/util/ExportOptions.java | 12 +- .../exportimport/util/ExportUtils.java | 87 ++++---- .../resources/admin/RealmAdminResource.java | 5 +- .../partialexport/PartialExportTest.java | 37 +++- .../export/partialexport-testrealm.json | 187 +++++++++++++++++- 5 files changed, 289 insertions(+), 39 deletions(-) diff --git a/services/src/main/java/org/keycloak/exportimport/util/ExportOptions.java b/services/src/main/java/org/keycloak/exportimport/util/ExportOptions.java index b23f756c90..ba4704a29b 100644 --- a/services/src/main/java/org/keycloak/exportimport/util/ExportOptions.java +++ b/services/src/main/java/org/keycloak/exportimport/util/ExportOptions.java @@ -8,14 +8,16 @@ public class ExportOptions { private boolean usersIncluded = true; private boolean clientsIncluded = true; private boolean groupsAndRolesIncluded = true; + private boolean onlyServiceAccountsIncluded = false; public ExportOptions() { } - public ExportOptions(boolean users, boolean clients, boolean groupsAndRoles) { + public ExportOptions(boolean users, boolean clients, boolean groupsAndRoles, boolean onlyServiceAccounts) { usersIncluded = users; clientsIncluded = clients; groupsAndRolesIncluded = groupsAndRoles; + onlyServiceAccountsIncluded = onlyServiceAccounts; } public boolean isUsersIncluded() { @@ -30,6 +32,10 @@ public class ExportOptions { return groupsAndRolesIncluded; } + public boolean isOnlyServiceAccountsIncluded() { + return onlyServiceAccountsIncluded; + } + public void setUsersIncluded(boolean value) { usersIncluded = value; } @@ -41,4 +47,8 @@ public class ExportOptions { public void setGroupsAndRolesIncluded(boolean value) { groupsAndRolesIncluded = value; } + + public void setOnlyServiceAccountsIncluded(boolean value) { + onlyServiceAccountsIncluded = value; + } } diff --git a/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java b/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java index 45e22ba47b..1c93da36e8 100755 --- a/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java +++ b/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java @@ -85,10 +85,7 @@ import com.fasterxml.jackson.databind.SerializationFeature; public class ExportUtils { public static RealmRepresentation exportRealm(KeycloakSession session, RealmModel realm, boolean includeUsers, boolean internal) { - ExportOptions opts = new ExportOptions(false, true, true); - if (includeUsers) { - opts.setUsersIncluded(true); - } + ExportOptions opts = new ExportOptions(includeUsers, true, true, false); return exportRealm(session, realm, opts, internal); } @@ -250,7 +247,7 @@ public class ExportUtils { List allUsers = session.users().getUsers(realm, true); List users = new LinkedList<>(); for (UserModel user : allUsers) { - UserRepresentation userRep = exportUser(session, realm, user, options); + UserRepresentation userRep = exportUser(session, realm, user, options, internal); users.add(userRep); } @@ -267,6 +264,21 @@ public class ExportUtils { rep.setFederatedUsers(federatedUsers); } + } else if (options.isClientsIncluded() && options.isOnlyServiceAccountsIncluded()) { + List users = new LinkedList<>(); + for (ClientModel app : clients) { + if (app.isServiceAccountsEnabled() && !app.isPublicClient() && !app.isBearerOnly()) { + UserModel user = session.users().getServiceAccount(app); + if (user != null) { + UserRepresentation userRep = exportUser(session, realm, user, options, internal); + users.add(userRep); + } + } + } + + if (users.size() > 0) { + rep.setUsers(users); + } } // components @@ -474,7 +486,7 @@ public class ExportUtils { * @param user * @return fully exported user representation */ - public static UserRepresentation exportUser(KeycloakSession session, RealmModel realm, UserModel user, ExportOptions options) { + public static UserRepresentation exportUser(KeycloakSession session, RealmModel realm, UserModel user, ExportOptions options, boolean internal) { UserRepresentation userRep = ModelToRepresentation.toRepresentation(session, realm, user); // Social links @@ -489,40 +501,45 @@ public class ExportUtils { } // Role mappings - Set roles = user.getRoleMappings(); - List realmRoleNames = new ArrayList<>(); - Map> clientRoleNames = new HashMap<>(); - for (RoleModel role : roles) { - if (role.getContainer() instanceof RealmModel) { - realmRoleNames.add(role.getName()); - } else { - ClientModel client = (ClientModel)role.getContainer(); - String clientId = client.getClientId(); - List currentClientRoles = clientRoleNames.get(clientId); - if (currentClientRoles == null) { - currentClientRoles = new ArrayList<>(); - clientRoleNames.put(clientId, currentClientRoles); - } + if (options.isGroupsAndRolesIncluded()) { + Set roles = user.getRoleMappings(); + List realmRoleNames = new ArrayList<>(); + Map> clientRoleNames = new HashMap<>(); + for (RoleModel role : roles) { + if (role.getContainer() instanceof RealmModel) { + realmRoleNames.add(role.getName()); + } else { + ClientModel client = (ClientModel)role.getContainer(); + String clientId = client.getClientId(); + List currentClientRoles = clientRoleNames.get(clientId); + if (currentClientRoles == null) { + currentClientRoles = new ArrayList<>(); + clientRoleNames.put(clientId, currentClientRoles); + } - currentClientRoles.add(role.getName()); + currentClientRoles.add(role.getName()); + } + } + + if (realmRoleNames.size() > 0) { + userRep.setRealmRoles(realmRoleNames); + } + if (clientRoleNames.size() > 0) { + userRep.setClientRoles(clientRoleNames); } } - if (realmRoleNames.size() > 0) { - userRep.setRealmRoles(realmRoleNames); - } - if (clientRoleNames.size() > 0) { - userRep.setClientRoles(clientRoleNames); + // Credentials - extra security, do not export credentials if service accounts + if (internal) { + List creds = session.userCredentialManager().getStoredCredentials(realm, user); + List credReps = new ArrayList<>(); + for (CredentialModel cred : creds) { + CredentialRepresentation credRep = exportCredential(cred); + credReps.add(credRep); + } + userRep.setCredentials(credReps); } - // Credentials - List creds = session.userCredentialManager().getStoredCredentials(realm, user); - List credReps = new ArrayList<>(); - for (CredentialModel cred : creds) { - CredentialRepresentation credRep = exportCredential(cred); - credReps.add(credRep); - } - userRep.setCredentials(credReps); userRep.setFederationLink(user.getFederationLink()); // Grants @@ -591,7 +608,7 @@ public class ExportUtils { generator.writeStartArray(); for (UserModel user : usersToExport) { - UserRepresentation userRep = ExportUtils.exportUser(session, realm, user, options); + UserRepresentation userRep = ExportUtils.exportUser(session, realm, user, options, true); generator.writeObject(userRep); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index b99ae25191..70e527ca84 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -1123,7 +1123,10 @@ public class RealmAdminResource { auth.clients().requireView(); } - ExportOptions options = new ExportOptions(false, clientsExported, groupsAndRolesExported); + // service accounts are exported if the clients are exported + // this means that if clients is true but groups/roles is false the service account is exported without roles + // the other option is just include service accounts if clientsExported && groupsAndRolesExported + ExportOptions options = new ExportOptions(false, clientsExported, groupsAndRolesExported, clientsExported); RealmRepresentation rep = ExportUtils.exportRealm(session, realm, options, false); return stripForExport(session, rep); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java index 63a7860d5c..c7422aa8b2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java @@ -1,5 +1,6 @@ package org.keycloak.testsuite.admin.partialexport; +import java.util.Arrays; import org.junit.Test; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.representations.idm.ClientRepresentation; @@ -18,6 +19,9 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import org.hamcrest.Matchers; +import org.keycloak.common.constants.ServiceAccountConstants; +import org.keycloak.representations.idm.UserRepresentation; /** @@ -40,6 +44,7 @@ public class PartialExportTest extends AbstractAdminTest { // exportGroupsAndRoles == false, exportClients == false RealmRepresentation rep = adminClient.realm(EXPORT_TEST_REALM).partialExport(false, false); + Assert.assertNull("Users are null", rep.getUsers()); Assert.assertNull("Default groups are empty", rep.getDefaultGroups()); Assert.assertNull("Groups are empty", rep.getGroups()); @@ -55,6 +60,7 @@ public class PartialExportTest extends AbstractAdminTest { // exportGroupsAndRoles == true, exportClients == false rep = adminClient.realm(EXPORT_TEST_REALM).partialExport(true, false); + Assert.assertNull("Users are null", rep.getUsers()); Assert.assertNull("Default groups are empty", rep.getDefaultGroups()); Assert.assertNotNull("Groups not empty", rep.getGroups()); checkGroups(rep.getGroups()); @@ -75,6 +81,9 @@ public class PartialExportTest extends AbstractAdminTest { // exportGroupsAndRoles == false, exportClients == true rep = adminClient.realm(EXPORT_TEST_REALM).partialExport(false, true); + Assert.assertNotNull("The service accout user should be exported", rep.getUsers()); + Assert.assertEquals("Only one client has a service account", 1, rep.getUsers().size()); + checkServiceAccountRoles(rep.getUsers().get(0), false); // export but without roles Assert.assertNull("Default groups are empty", rep.getDefaultGroups()); Assert.assertNull("Groups are empty", rep.getGroups()); Assert.assertNotNull("Default roles not empty", rep.getDefaultRoles()); @@ -90,6 +99,10 @@ public class PartialExportTest extends AbstractAdminTest { // exportGroupsAndRoles == true, exportClients == true rep = adminClient.realm(EXPORT_TEST_REALM).partialExport(true, true); + // service accounts are only exported if roles/groups and clients are asked to be exported + Assert.assertNotNull("The service accout user should be exported", rep.getUsers()); + Assert.assertEquals("Only one client has a service account", 1, rep.getUsers().size()); + checkServiceAccountRoles(rep.getUsers().get(0), true); // exported with roles Assert.assertNull("Default groups are empty", rep.getDefaultGroups()); Assert.assertNotNull("Groups not empty", rep.getGroups()); checkGroups(rep.getGroups()); @@ -115,6 +128,25 @@ public class PartialExportTest extends AbstractAdminTest { checkSecretsAreMasked(rep); } + private void checkServiceAccountRoles(UserRepresentation serviceAccount, boolean rolesExpected) { + Assert.assertTrue("User is a service account", serviceAccount.getUsername().startsWith(ServiceAccountConstants.SERVICE_ACCOUNT_USER_PREFIX)); + Assert.assertNull("Password should be null", serviceAccount.getCredentials()); + if (rolesExpected) { + List realmRoles = serviceAccount.getRealmRoles(); + Assert.assertThat("Realm roles are OK", realmRoles, Matchers.containsInAnyOrder("uma_authorization", "user", "offline_access")); + + Map> clientRoles = serviceAccount.getClientRoles(); + Assert.assertNotNull("Client roles are exported", clientRoles); + Assert.assertThat("Client roles for test-app-service-account are OK", clientRoles.get("test-app-service-account"), + Matchers.containsInAnyOrder("test-app-service-account", "test-app-service-account-parent")); + Assert.assertThat("Client roles for account are OK", clientRoles.get("account"), + Matchers.containsInAnyOrder("manage-account", "view-profile")); + } else { + Assert.assertNull("Service account should be exported without realm roles", serviceAccount.getRealmRoles()); + Assert.assertNull("Service account should be exported without client roles", serviceAccount.getClientRoles()); + } + } + private void checkSecretsAreMasked(RealmRepresentation rep) { // Client secret @@ -216,10 +248,13 @@ public class PartialExportTest extends AbstractAdminTest { Assert.assertTrue("customer-admin-composite-role / realm / customer-user-premium", cmp.getRealm().contains("customer-user-premium")); Assert.assertTrue("customer-admin-composite-role / client['test-app'] / customer-admin", cmp.getClient().get("test-app").contains("customer-admin")); - roles = collectRoles(clientRoles.get("test-app-scope")); Assert.assertTrue("Client role test-app-disallowed-by-scope for test-app-scope", roles.containsKey("test-app-disallowed-by-scope")); Assert.assertTrue("Client role test-app-allowed-by-scope for test-app-scope", roles.containsKey("test-app-allowed-by-scope")); + + roles = collectRoles(clientRoles.get("test-app-service-account")); + Assert.assertThat("Client roles are OK for test-app-service-account", roles.keySet(), + Matchers.containsInAnyOrder("test-app-service-account", "test-app-service-account-parent", "test-app-service-account-child")); } private Map collectRoles(List roles) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json index 933a170666..6e7ab3a3b4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json @@ -120,6 +120,39 @@ "clientRole": true, "containerId": "f3ff0b0d-e922-4874-a34c-cdfa1b3305fe" } + ], + "test-app-service-account": [ + { + "name": "test-app-service-account", + "description": "test-app-service-account", + "composite": false, + "clientRole": true, + "containerId": "9f39a1b4-8ca1-45e1-943d-9149c5bdcca4", + "attributes": {} + }, + { + "name": "test-app-service-account-child", + "description": "test-app-service-account-child", + "composite": false, + "clientRole": true, + "containerId": "9f39a1b4-8ca1-45e1-943d-9149c5bdcca4", + "attributes": {} + }, + { + "name": "test-app-service-account-parent", + "description": "test-app-service-account-parent", + "composite": true, + "composites": { + "client": { + "test-app-service-account": [ + "test-app-service-account-child" + ] + } + }, + "clientRole": true, + "containerId": "9f39a1b4-8ca1-45e1-943d-9149c5bdcca4", + "attributes": {} + } ] } }, @@ -209,6 +242,61 @@ "user": "user", "password": "secret" }, + "users": [ + { + "username" : "bburke", + "enabled": true, + "email" : "bburke@redhat.com", + "credentials" : [ + { + "type" : "password", + "value" : "password" + } + ], + "attributes" : { + "phone": "617" + }, + "realmRoles": [ + "user" + ], + "applicationRoles": { + "test-app": [ + "sample-client-role" + ] + } + }, + { + "username": "service-account-test-app-service-account", + "enabled": true, + "totp": false, + "emailVerified": false, + "email": "service-account-test-app-service-account@placeholder.org", + "serviceAccountClientId": "test-app-service-account", + "credentials" : [ + { + "type" : "password", + "value" : "password" + } + ], + "realmRoles": [ + "uma_authorization", + "user", + "offline_access" + ], + "clientRoles": { + "test-app-service-account": [ + "test-app-service-account", + "test-app-service-account-parent" + ], + "account": [ + "manage-account", + "view-profile" + ] + }, + "notBefore": 0, + "groups": [] + } + ], "scopeMappings": [ { "client": "test-app", @@ -606,6 +694,103 @@ "useTemplateConfig": false, "useTemplateScope": false, "useTemplateMappers": false + }, + { + "clientId": "test-app-service-account", + "rootUrl": "http://localhost:8180/auth/realms/master/app-service-account", + "adminUrl": "http://localhost:8180/auth/realms/master/app-service-account", + "surrogateAuthRequired": false, + "enabled": true, + "clientAuthenticatorType": "client-secret", + "secret": "password", + "redirectUris": [ + "http://localhost:8180/auth/realms/master/app-service-account/*" + ], + "webOrigins": [ + "http://localhost:8180" + ], + "notBefore": 0, + "bearerOnly": false, + "consentRequired": false, + "standardFlowEnabled": true, + "implicitFlowEnabled": false, + "directAccessGrantsEnabled": true, + "serviceAccountsEnabled": true, + "publicClient": false, + "frontchannelLogout": false, + "protocol": "openid-connect", + "attributes": { + "saml.assertion.signature": "false", + "saml.force.post.binding": "false", + "saml.multivalued.roles": "false", + "saml.encrypt": "false", + "saml.server.signature": "false", + "saml.server.signature.keyinfo.ext": "false", + "exclude.session.state.from.auth.response": "false", + "saml_force_name_id_format": "false", + "saml.client.signature": "false", + "tls.client.certificate.bound.access.tokens": "false", + "saml.authnstatement": "false", + "display.on.consent.screen": "false", + "saml.onetimeuse.condition": "false" + }, + "authenticationFlowBindingOverrides": {}, + "fullScopeAllowed": true, + "nodeReRegistrationTimeout": -1, + "protocolMappers": [ + { + "name": "Client Host", + "protocol": "openid-connect", + "protocolMapper": "oidc-usersessionmodel-note-mapper", + "consentRequired": false, + "config": { + "user.session.note": "clientHost", + "id.token.claim": "true", + "access.token.claim": "true", + "claim.name": "clientHost", + "jsonType.label": "String" + } + }, + { + "name": "Client ID", + "protocol": "openid-connect", + "protocolMapper": "oidc-usersessionmodel-note-mapper", + "consentRequired": false, + "config": { + "user.session.note": "clientId", + "id.token.claim": "true", + "access.token.claim": "true", + "claim.name": "clientId", + "jsonType.label": "String" + } + }, + { + "name": "Client IP Address", + "protocol": "openid-connect", + "protocolMapper": "oidc-usersessionmodel-note-mapper", + "consentRequired": false, + "config": { + "user.session.note": "clientAddress", + "id.token.claim": "true", + "access.token.claim": "true", + "claim.name": "clientAddress", + "jsonType.label": "String" + } + } + ], + "defaultClientScopes": [ + "web-origins", + "role_list", + "profile", + "roles", + "email" + ], + "optionalClientScopes": [ + "address", + "phone", + "offline_access", + "microprofile-jwt" + ] }], "components": { "org.keycloak.keys.KeyProvider": [ @@ -986,4 +1171,4 @@ } } ] -} \ No newline at end of file +}