jboss-logging listener adjustments + final feedback improvements

Signed-off-by: Peter Zaoral <[email protected]>
Pepo48 · Aug 19, 2024 · 95998c4 · 95998c4
1 parent d6bae4b
commit 95998c4
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 11 deletions.
diff --git a/js/apps/admin-ui/cypress/e2e/clients_test.spec.ts b/js/apps/admin-ui/cypress/e2e/clients_test.spec.ts
@@ -115,7 +115,7 @@ describe("Clients test", () => {
       );
     });
 
-    it("Should check temporary admin service existence", () => {
+    it("Should check temporary admin service label (non)existence", () => {
       commonPage.sidebar().goToRealm("master");
       commonPage.sidebar().goToClients();
       commonPage
@@ -125,6 +125,12 @@ describe("Clients test", () => {
       commonPage
         .tableUtils()
         .checkTemporaryAdminLabelExists("temporary-admin-label");
+
+      commonPage.tableToolbarUtils().searchItem("admin-cli", false);
+      commonPage.tableUtils().checkRowItemExists("admin-cli");
+      commonPage
+        .tableUtils()
+        .checkTemporaryAdminLabelExists("temporary-admin-label", false);
     });
 
     it("Should list client scopes", () => {

diff --git a/js/apps/admin-ui/cypress/support/pages/admin-ui/components/TablePage.ts b/js/apps/admin-ui/cypress/support/pages/admin-ui/components/TablePage.ts
@@ -148,13 +148,13 @@ export default class TablePage extends CommonElements {
     return this;
   }
 
-  checkTemporaryAdminLabelExists(labelId: string) {
+  checkTemporaryAdminLabelExists(labelId: string, exist = true) {
     cy.get(
       (this.#tableInModal ? ".pf-v5-c-modal-box.pf-m-md " : "") +
         this.#tableRowItem,
     )
       .find(`#${labelId}`)
-      .should("exist");
+      .should((!exist ? "not." : "") + "exist");
     return this;
   }
 

diff --git a/js/apps/admin-ui/src/Banners.tsx b/js/apps/admin-ui/src/Banners.tsx
@@ -7,7 +7,7 @@ const WarnBanner = (msg: string) => {
   const { t } = useTranslation();
 
   return (
-    <Banner screenReaderText={t(msg)} isSticky>
+    <Banner screenReaderText={t(msg)} variant="gold" isSticky>
       <Flex spaceItems={{ default: "spaceItemsSm" }}>
         <FlexItem>
           <ExclamationTriangleIcon />

diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/BruteForceUsersResource.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/BruteForceUsersResource.java
@@ -29,8 +29,6 @@
 import org.keycloak.services.resources.admin.permissions.UserPermissionEvaluator;
 import org.keycloak.utils.SearchQueryUtils;
 
-import static org.keycloak.services.managers.ApplianceBootstrap.TEMP_ADMIN_ATTR_NAME;
-
 public class BruteForceUsersResource {
     private static final Logger logger = Logger.getLogger(BruteForceUsersResource.class);
     private static final String SEARCH_ID_PARAMETER = "id:";
@@ -169,7 +167,6 @@ private Stream<BruteUser> toRepresentation(RealmModel realm, UserPermissionEvalu
                     ModelToRepresentation.toBriefRepresentation(user) :
                     ModelToRepresentation.toRepresentation(session, realm, user);
             userRep.setAccess(usersEvaluator.getAccess(user));
-            ModelToRepresentation.addAttributeToBriefRep(user, userRep, TEMP_ADMIN_ATTR_NAME);
             return userRep;
         }).map(this::getBruteForceStatus);
     }

diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java
@@ -184,4 +184,7 @@ public final class Constants {
 
     //attribute name used to mark a client as realm client
     public static final String REALM_CLIENT = "realm_client";
+
+    //attribute name used to mark a temporary admin user/service account as temporary
+    public static final String TEMP_ADMIN_ATTR_NAME = "temporary_admin";
 }
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -68,6 +68,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import static org.keycloak.models.light.LightweightUserAdapter.isLightweightUser;
+import static org.keycloak.models.Constants.TEMP_ADMIN_ATTR_NAME;
 
 /**
  * @author <a href="mailto:[email protected]">Bill Burke</a>
@@ -265,6 +266,7 @@ public static UserRepresentation toBriefRepresentation(UserModel user) {
         rep.setEnabled(user.isEnabled());
         rep.setEmailVerified(user.isEmailVerified());
         rep.setFederationLink(user.getFederationLink());
+        addAttributeToBriefRep(user, rep, TEMP_ADMIN_ATTR_NAME);
 
         return rep;
     }

diff --git a/services/src/main/java/org/keycloak/events/log/JBossLoggingEventListenerProvider.java b/services/src/main/java/org/keycloak/events/log/JBossLoggingEventListenerProvider.java
@@ -22,9 +22,13 @@
 import org.keycloak.events.Event;
 import org.keycloak.events.EventListenerProvider;
 import org.keycloak.events.EventListenerTransaction;
+import org.keycloak.events.EventType;
 import org.keycloak.events.admin.AdminEvent;
+import org.keycloak.models.ClientModel;
 import org.keycloak.models.KeycloakContext;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
 import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.utils.StringUtil;
 
@@ -33,6 +37,8 @@
 import jakarta.ws.rs.core.UriInfo;
 import java.util.Map;
 
+import static org.keycloak.models.Constants.TEMP_ADMIN_ATTR_NAME;
+
 /**
  * @author <a href="mailto:[email protected]">Stian Thorgersen</a>
  */
@@ -135,6 +141,18 @@ private void logEvent(Event event) {
 
             logger.log(logger.isTraceEnabled() ? Logger.Level.TRACE : level, sb.toString());
         }
+
+        RealmModel realm = session.realms().getRealm(event.getRealmId());
+        UserModel user = session.users().getUserById(realm, event.getUserId());
+        ClientModel client = session.clients().getClientByClientId(realm, event.getClientId());
+
+        if (EventType.LOGIN.equals(event.getType()) && Boolean.parseBoolean(user.getFirstAttribute(TEMP_ADMIN_ATTR_NAME))) {
+            logger.warn(user.getUsername() + " is a temporary admin user account. To harden security, create a permanent account and delete the temporary one.");
+        }
+
+        if (EventType.CLIENT_LOGIN.equals(event.getType()) && Boolean.parseBoolean(client.getAttribute(TEMP_ADMIN_ATTR_NAME))) {
+            logger.warn(client.getClientId() + " is a temporary admin service account. To harden security, create a permanent account and delete the temporary one.");
+        }
     }
 
     private void logAdminEvent(AdminEvent adminEvent, boolean includeRepresentation) {
@@ -176,7 +194,7 @@ private void logAdminEvent(AdminEvent adminEvent, boolean includeRepresentation)
     @Override
     public void close() {
     }
-    
+
     private void setKeycloakContext(StringBuilder sb) {
         KeycloakContext context = session.getContext();
         UriInfo uriInfo = context.getUri();
@@ -199,7 +217,7 @@ private void setKeycloakContext(StringBuilder sb) {
             }
             sb.append("]");
         }
-        
+
     }
 
 }
diff --git a/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java b/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java
@@ -37,6 +37,8 @@
 import org.keycloak.userprofile.UserProfileProvider;
 import org.keycloak.utils.StringUtil;
 
+import static org.keycloak.models.Constants.TEMP_ADMIN_ATTR_NAME;
+
 /**
  * @author <a href="mailto:[email protected]">Bill Burke</a>
  * @version $Revision: 1 $
@@ -46,7 +48,6 @@ public class ApplianceBootstrap {
     public static final String DEFAULT_TEMP_ADMIN_USERNAME = "temp-admin";
     public static final String DEFAULT_TEMP_ADMIN_SERVICE = "temp-admin";
     public static final int DEFAULT_TEMP_ADMIN_EXPIRATION = 120;
-    public static final String TEMP_ADMIN_ATTR_NAME = "temporary_admin";
 
     private final KeycloakSession session;
 

diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AdminConsole.java b/services/src/main/java/org/keycloak/services/resources/admin/AdminConsole.java
@@ -68,7 +68,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import static org.keycloak.services.managers.ApplianceBootstrap.TEMP_ADMIN_ATTR_NAME;
+import static org.keycloak.models.Constants.TEMP_ADMIN_ATTR_NAME;
 
 /**
  * @author <a href="mailto:[email protected]">Bill Burke</a>