From 7ce6f12fe3df1632d9787ca3f9bf6b7c1758f53e Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Thu, 8 Aug 2024 08:20:33 -0400 Subject: [PATCH] fix: adds a check for duplicate users/clients to simplify cmd errors (#31583) also changes temp-admin-service to temp-admin closes: #31160 Signed-off-by: Steve Hawkins --- .../jaxrs/QuarkusKeycloakApplication.java | 27 +++--- .../org/keycloak/services/ServicesLogger.java | 11 ++- .../services/managers/ApplianceBootstrap.java | 82 ++++++++++++------- 3 files changed, 73 insertions(+), 47 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/QuarkusKeycloakApplication.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/QuarkusKeycloakApplication.java index fc4751b8a7..d7895750f9 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/QuarkusKeycloakApplication.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/QuarkusKeycloakApplication.java @@ -17,27 +17,22 @@ package org.keycloak.quarkus.runtime.integration.jaxrs; -import io.quarkus.runtime.ShutdownEvent; -import io.quarkus.runtime.StartupEvent; -import io.smallrye.common.annotation.Blocking; - -import org.keycloak.Config; import org.keycloak.config.BootstrapAdminOptions; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.KeycloakSessionTask; -import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.platform.Platform; import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.command.AbstractNonServerCommand; import org.keycloak.quarkus.runtime.configuration.Configuration; import org.keycloak.quarkus.runtime.integration.QuarkusKeycloakSessionFactory; import org.keycloak.quarkus.runtime.integration.QuarkusPlatform; -import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.ApplianceBootstrap; import org.keycloak.services.resources.KeycloakApplication; import org.keycloak.utils.StringUtil; +import io.quarkus.runtime.ShutdownEvent; +import io.quarkus.runtime.StartupEvent; +import io.smallrye.common.annotation.Blocking; import jakarta.enterprise.event.Observes; import jakarta.ws.rs.ApplicationPath; @@ -85,23 +80,23 @@ public class QuarkusKeycloakApplication extends KeycloakApplication { try { //Integer expiration = Configuration.getOptionalKcValue(BootstrapAdminOptions.EXPIRATION.getKey()).map(Integer::valueOf).orElse(null); - if (StringUtil.isNotBlank(adminPassword)) { - createTemporaryMasterRealmAdminUser(adminUsername, adminPassword, /*expiration,*/ session); + if (StringUtil.isNotBlank(adminPassword) && !createTemporaryMasterRealmAdminUser(adminUsername, adminPassword, /*expiration,*/ session)) { + throw new RuntimeException("Aborting startup and the creation of the master realm, because the temporary admin user account could not be created."); } - if (StringUtil.isNotBlank(clientSecret)) { - createTemporaryMasterRealmAdminService(clientId, clientSecret, /*expiration,*/ session); + if (StringUtil.isNotBlank(clientSecret) && !createTemporaryMasterRealmAdminService(clientId, clientSecret, /*expiration,*/ session)) { + throw new RuntimeException("Aborting startup and the creation of the master realm, because the temporary admin service account could not be created."); } } catch (NumberFormatException e) { throw new RuntimeException("Invalid admin expiration value provided. An integer is expected.", e); } } - public void createTemporaryMasterRealmAdminUser(String adminUserName, String adminPassword, /*Integer adminExpiration,*/ KeycloakSession session) { - new ApplianceBootstrap(session).createTemporaryMasterRealmAdminUser(adminUserName, adminPassword /*, adminExpiration*/, false); + public boolean createTemporaryMasterRealmAdminUser(String adminUserName, String adminPassword, /*Integer adminExpiration,*/ KeycloakSession session) { + return new ApplianceBootstrap(session).createTemporaryMasterRealmAdminUser(adminUserName, adminPassword /*, adminExpiration*/, false); } - public void createTemporaryMasterRealmAdminService(String clientId, String clientSecret, /*Integer adminExpiration,*/ KeycloakSession session) { - new ApplianceBootstrap(session).createTemporaryMasterRealmAdminService(clientId, clientSecret /*, adminExpiration*/); + public boolean createTemporaryMasterRealmAdminService(String clientId, String clientSecret, /*Integer adminExpiration,*/ KeycloakSession session) { + return new ApplianceBootstrap(session).createTemporaryMasterRealmAdminService(clientId, clientSecret /*, adminExpiration*/); } } diff --git a/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java index 1bbe1cea55..27e72fde1b 100644 --- a/services/src/main/java/org/keycloak/services/ServicesLogger.java +++ b/services/src/main/java/org/keycloak/services/ServicesLogger.java @@ -88,7 +88,7 @@ public interface ServicesLogger extends BasicLogger { @LogMessage(level = ERROR) @Message(id=10, value="Failed to add user '%s' to realm '%s': user with username exists") void addUserFailedUserExists(String user, String realm); - + @LogMessage(level = ERROR) @Message(id=11, value="Failed to add user '%s' to realm '%s'") void addUserFailed(@Cause Throwable t, String user, String realm); @@ -465,10 +465,15 @@ public interface ServicesLogger extends BasicLogger { void scriptEngineCreated(String engineName, String engineVersion, String mimeType); @LogMessage(level = DEBUG) - @Message(id=107, value="Skipping create admin user. Admin already exists in realm '%s'.") - void addAdminUserFailedAdminExists(String realm); + @Message(id=107, value="Skipping create admin user. User(s) already exist in realm '%s'.") + void addAdminUserFailedUsersExist(String realm); @LogMessage(level = WARN) @Message(id=108, value="URI '%s' doesn't match any trustedHost or trustedDomain") void uriDoesntMatch(String uri); + + @LogMessage(level = ERROR) + @Message(id=109, value="Failed to add client '%s' to realm '%s': client with client ID exists") + void addClientFailedClientExists(String clientId, String realm); + } diff --git a/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java b/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java index 907016957f..907030d8f5 100755 --- a/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java +++ b/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java @@ -23,6 +23,7 @@ import org.keycloak.models.AdminRoles; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserCredentialModel; @@ -113,7 +114,14 @@ public class ApplianceBootstrap { return true; } - public void createTemporaryMasterRealmAdminUser(String username, String password, /*Integer expriationMinutes,*/ boolean initialUser) { + /** + * Create a temporary admin user + * @param username + * @param password + * @param initialUser if true only create the user if no other users exist + * @return false if the user could not be created + */ + public boolean createTemporaryMasterRealmAdminUser(String username, String password, /*Integer expriationMinutes,*/ boolean initialUser) { RealmModel realm = session.realms().getRealmByName(Config.getAdminRealm()); session.getContext().setRealm(realm); @@ -121,26 +129,38 @@ public class ApplianceBootstrap { //expriationMinutes = expriationMinutes == null ? DEFAULT_TEMP_ADMIN_EXPIRATION : expriationMinutes; if (initialUser && session.users().getUsersCount(realm) > 0) { - ServicesLogger.LOGGER.addAdminUserFailedAdminExists(Config.getAdminRealm()); - return; + ServicesLogger.LOGGER.addAdminUserFailedUsersExist(Config.getAdminRealm()); + return false; } - UserModel adminUser = session.users().addUser(realm, username); - adminUser.setEnabled(true); - // TODO: is this appropriate, does it need to be managed? - // adminUser.setSingleAttribute("temporary_admin", Boolean.TRUE.toString()); - // also set the expiration - could be relative to a creation timestamp, or computed - - UserCredentialModel usrCredModel = UserCredentialModel.password(password); - adminUser.credentialManager().updateCredential(usrCredModel); - - RoleModel adminRole = realm.getRole(AdminRoles.ADMIN); - adminUser.grantRole(adminRole); - - ServicesLogger.LOGGER.createdTemporaryAdminUser(username); + try { + UserModel adminUser = session.users().addUser(realm, username); + adminUser.setEnabled(true); + // TODO: is this appropriate, does it need to be managed? + // adminUser.setSingleAttribute("temporary_admin", Boolean.TRUE.toString()); + // also set the expiration - could be relative to a creation timestamp, or computed + + UserCredentialModel usrCredModel = UserCredentialModel.password(password); + adminUser.credentialManager().updateCredential(usrCredModel); + + RoleModel adminRole = realm.getRole(AdminRoles.ADMIN); + adminUser.grantRole(adminRole); + + ServicesLogger.LOGGER.createdTemporaryAdminUser(username); + } catch (ModelDuplicateException e) { + ServicesLogger.LOGGER.addUserFailedUserExists(username, Config.getAdminRealm()); + return false; + } + return true; } - public void createTemporaryMasterRealmAdminService(String clientId, String clientSecret /*, Integer expriationMinutes*/) { + /** + * Create a temporary admin service account + * @param clientId the client ID + * @param clientSecret the client secret + * @return false if the service account could not be created + */ + public boolean createTemporaryMasterRealmAdminService(String clientId, String clientSecret /*, Integer expriationMinutes*/) { RealmModel realm = session.realms().getRealmByName(Config.getAdminRealm()); session.getContext().setRealm(realm); @@ -154,17 +174,23 @@ public class ApplianceBootstrap { adminClient.setPublicClient(false); adminClient.setSecret(clientSecret); - ClientModel adminClientModel = ClientManager.createClient(session, realm, adminClient); - - new ClientManager(new RealmManager(session)).enableServiceAccount(adminClientModel); - UserModel serviceAccount = session.users().getServiceAccount(adminClientModel); - RoleModel adminRole = realm.getRole(AdminRoles.ADMIN); - serviceAccount.grantRole(adminRole); - - // TODO: set temporary - // also set the expiration - could be relative to a creation timestamp, or computed - - ServicesLogger.LOGGER.createdTemporaryAdminService(clientId); + try { + ClientModel adminClientModel = ClientManager.createClient(session, realm, adminClient); + + new ClientManager(new RealmManager(session)).enableServiceAccount(adminClientModel); + UserModel serviceAccount = session.users().getServiceAccount(adminClientModel); + RoleModel adminRole = realm.getRole(AdminRoles.ADMIN); + serviceAccount.grantRole(adminRole); + + // TODO: set temporary + // also set the expiration - could be relative to a creation timestamp, or computed + + ServicesLogger.LOGGER.createdTemporaryAdminService(clientId); + } catch (ModelDuplicateException e) { + ServicesLogger.LOGGER.addClientFailedClientExists(clientId, Config.getAdminRealm()); + return false; + } + return true; } public void createMasterRealmUser(String username, String password) {