Skip to content

Commit

Permalink
Warnings for temporary admin user and service account
Browse files Browse the repository at this point in the history
* UI banner, labels and log messages are shown when temporary admin account is used
* added UI tests that check the elements' presence

Co-authored-by: Václav Muzikář <[email protected]>
Signed-off-by: Peter Zaoral <[email protected]>
  • Loading branch information
Pepo48 and vmuzikar committed Aug 19, 2024
1 parent 95e649f commit cc0d747
Show file tree
Hide file tree
Showing 17 changed files with 176 additions and 16 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/js-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ jobs:
env:
KC_BOOTSTRAP_ADMIN_USERNAME: admin
KC_BOOTSTRAP_ADMIN_PASSWORD: admin
KC_BOOTSTRAP_ADMIN_CLIENT_ID: temporary-admin-service
KC_BOOTSTRAP_ADMIN_CLIENT_SECRET: temporary-admin-service

- name: Start LDAP server
run: pnpm --fail-if-no-match --filter ${{ env.WORKSPACE }} cy:ldap-server &
Expand Down
18 changes: 18 additions & 0 deletions js/apps/admin-ui/cypress/e2e/clients_test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,24 @@ describe("Clients test", () => {
);
});

it("Should check temporary admin service label (non)existence", () => {
commonPage.sidebar().goToRealm("master");
commonPage.sidebar().goToClients();
commonPage
.tableToolbarUtils()
.searchItem("temporary-admin-service", false);
commonPage.tableUtils().checkRowItemExists("temporary-admin-service");
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", () => {
commonPage
.tableUtils()
Expand Down
16 changes: 16 additions & 0 deletions js/apps/admin-ui/cypress/e2e/users_test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ describe("User creation", () => {
masthead.checkNotificationMessage("The user has been created");
});

it("Should check temporary admin user existence", () => {
const commonPage = new CommonPage();

// check banner visibility first
cy.get(".pf-v5-c-banner").should(
"contain.text",
"You are logged in as a temporary admin user.",
);

commonPage.tableToolbarUtils().searchItem("admin", false);
commonPage.tableUtils().checkRowItemExists("admin");
commonPage
.tableUtils()
.checkTemporaryAdminLabelExists("temporary-admin-label");
});

it("Create user with groups test", () => {
itemIdWithGroups += uuid();
// Add user from search bar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ export default class TablePage extends CommonElements {
return this;
}

checkTemporaryAdminLabelExists(labelId: string, exist = true) {
cy.get(
(this.#tableInModal ? ".pf-v5-c-modal-box.pf-m-md " : "") +
this.#tableRowItem,
)
.find(`#${labelId}`)
.should((!exist ? "not." : "") + "exist");
return this;
}

checkRowItemValueByItemName(itemName: string, column: number, value: string) {
cy.get(
(this.#tableInModal ? ".pf-v5-c-modal-box.pf-m-md " : "") +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3216,4 +3216,7 @@ emailVerificationHelp=Specifies independent timeout for email verification.
idpAccountEmailVerificationHelp=Specifies independent timeout for IdP account email verification.
forgotPasswordHelp=Specifies independent timeout for forgot password.
executeActionsHelp=Specifies independent timeout for execute actions.
validatingX509CertsHelp=The public certificates Keycloak uses to validate the signatures of SAML requests and responses from the external IDP when Use metadata descriptor URL is OFF. Multiple certificates can be entered separated by comma (,). The certificates can be re-imported from the Metadata descriptor URL clicking the Import Keys action in the identity provider page. The action downloads the current certificates in the metadata endpoint and assigns them to the config in this same option. You need to click Save to definitely store the re-imported certificates.
validatingX509CertsHelp=The public certificates Keycloak uses to validate the signatures of SAML requests and responses from the external IDP when Use metadata descriptor URL is OFF. Multiple certificates can be entered separated by comma (,). The certificates can be re-imported from the Metadata descriptor URL clicking the Import Keys action in the identity provider page. The action downloads the current certificates in the metadata endpoint and assigns them to the config in this same option. You need to click Save to definitely store the re-imported certificates.
loggedInAsTempAdminUser=You are logged in as a temporary admin user. To harden security, create a permanent admin account and delete the temporary one.
temporaryAdmin=Temporary admin user account. Ensure it is replaced with a permanent admin user account as soon as possible.
temporaryService=Temporary admin service account. Ensure it is replaced with a permanent admin service account as soon as possible.
2 changes: 2 additions & 0 deletions js/apps/admin-ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { WhoAmIContextProvider } from "./context/whoami/WhoAmI";
import type { Environment } from "./environment";
import { SubGroups } from "./groups/SubGroupsContext";
import { AuthWall } from "./root/AuthWall";
import { Banners } from "./Banners";

const AppContexts = ({ children }: PropsWithChildren) => (
<ErrorBoundaryProvider>
Expand Down Expand Up @@ -65,6 +66,7 @@ export const App = () => {
breadcrumb={<PageBreadCrumbs />}
mainContainerId={mainPageContentId}
>
<Banners />
<ErrorBoundaryFallback fallback={ErrorRenderer}>
<Suspense fallback={<KeycloakSpinner />}>
<AuthWall>
Expand Down
26 changes: 26 additions & 0 deletions js/apps/admin-ui/src/Banners.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Banner, Flex, FlexItem } from "@patternfly/react-core";
import { ExclamationTriangleIcon } from "@patternfly/react-icons";
import { useWhoAmI } from "./context/whoami/WhoAmI";
import { useTranslation } from "react-i18next";

const WarnBanner = (msg: string) => {
const { t } = useTranslation();

return (
<Banner screenReaderText={t(msg)} variant="gold" isSticky>
<Flex spaceItems={{ default: "spaceItemsSm" }}>
<FlexItem>
<ExclamationTriangleIcon />
</FlexItem>
<FlexItem>{t(msg)}</FlexItem>
</Flex>
</Banner>
);
};

export const Banners = () => {
const { whoAmI } = useWhoAmI();

if (whoAmI.isTemporary()) return WarnBanner("loggedInAsTempAdminUser");
// more banners in the future?
};
10 changes: 10 additions & 0 deletions js/apps/admin-ui/src/clients/ClientsSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import {
Tab,
TabTitleText,
ToolbarItem,
Tooltip,
} from "@patternfly/react-core";
import { WarningTriangleIcon } from "@patternfly/react-icons";
import { IRowData, TableText, cellWidth } from "@patternfly/react-table";
import { useState } from "react";
import { useTranslation } from "react-i18next";
Expand Down Expand Up @@ -58,6 +60,14 @@ const ClientDetailLink = (client: ClientRepresentation) => {
</Badge>
)}
</Link>
{client.attributes?.["temporary_admin"] === "true" && (
<Tooltip content={t("temporaryService")}>
<WarningTriangleIcon
className="pf-v5-u-ml-sm"
id="temporary-admin-label"
/>
</Tooltip>
)}
</TableText>
);
};
Expand Down
18 changes: 15 additions & 3 deletions js/apps/admin-ui/src/components/users/UserDataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,23 @@ export type UserAttribute = {
};

const UserDetailLink = (user: BruteUser) => {
const { t } = useTranslation();
const { realm } = useRealm();
return (
<Link to={toUser({ realm, id: user.id!, tab: "settings" })}>
{user.username} <StatusRow user={user} />
</Link>
<>
<Link to={toUser({ realm, id: user.id!, tab: "settings" })}>
{user.username}
<StatusRow user={user} />
</Link>
{user.attributes?.["temporary_admin"][0] === "true" && (
<Tooltip content={t("temporaryAdmin")}>
<WarningTriangleIcon
className="pf-v5-u-ml-sm"
id="temporary-admin-label"
/>
</Tooltip>
)}
</>
);
};

Expand Down
4 changes: 4 additions & 0 deletions js/apps/admin-ui/src/context/whoami/WhoAmI.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ export class WhoAmI {

return this.#me.realm_access;
}

public isTemporary(): boolean {
return this.#me?.temporary ?? false;
}
}

type WhoAmIProps = {
Expand Down
4 changes: 4 additions & 0 deletions js/apps/keycloak-server/scripts/start-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const LOCAL_DIST_NAME = "keycloak-999.0.0-SNAPSHOT.tar.gz";
const SCRIPT_EXTENSION = process.platform === "win32" ? ".bat" : ".sh";
const ADMIN_USERNAME = "admin";
const ADMIN_PASSWORD = "admin";
const CLIENT_ID = "temporary-admin-service";
const CLIENT_SECRET = "temporary-admin-service";

const options = {
local: {
Expand All @@ -39,6 +41,8 @@ async function startServer() {
const env = {
KC_BOOTSTRAP_ADMIN_USERNAME: ADMIN_USERNAME,
KC_BOOTSTRAP_ADMIN_PASSWORD: ADMIN_PASSWORD,
KC_BOOTSTRAP_ADMIN_CLIENT_ID: CLIENT_ID,
KC_BOOTSTRAP_ADMIN_CLIENT_SECRET: CLIENT_SECRET,
...process.env,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ export default interface WhoAmIRepresentation {
locale: string;
createRealm: boolean;
realm_access: { [key: string]: AccessType[] };
temporary: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -265,10 +266,21 @@ 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;
}

public static void addAttributeToBriefRep(UserModel user, UserRepresentation userRep, String attributeName) {
String userAttributeValue = user.getFirstAttribute(attributeName);
if (Boolean.parseBoolean(userAttributeValue)) {
if (userRep.getAttributes() == null) {
userRep.setAttributes(new HashMap<>());
}
userRep.getAttributes().put(attributeName, Collections.singletonList(user.getFirstAttribute(attributeName)));
}
}

public static EventRepresentation toRepresentation(Event event) {
EventRepresentation rep = new EventRepresentation();
rep.setTime(event.getTime());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,25 @@
package org.keycloak.events.log;

import org.jboss.logging.Logger;
import org.keycloak.Config;
import org.keycloak.common.util.StackUtil;
import org.keycloak.events.Event;
import org.keycloak.events.EventListenerProvider;
import org.keycloak.events.EventListenerTransaction;
import org.keycloak.events.admin.AdminEvent;
import org.keycloak.models.KeycloakContext;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.utils.StringUtil;

import jakarta.ws.rs.core.Cookie;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.UriInfo;
import java.util.Map;
import java.util.function.Supplier;

import static org.keycloak.models.Constants.TEMP_ADMIN_ATTR_NAME;

/**
* @author <a href="mailto:[email protected]">Stian Thorgersen</a>
Expand Down Expand Up @@ -135,6 +140,24 @@ private void logEvent(Event event) {

logger.log(logger.isTraceEnabled() ? Logger.Level.TRACE : level, sb.toString());
}

if (event.getRealmName().equals(Config.getAdminRealm())) {
Supplier<RealmModel> getRealm = () -> session.realms().getRealm(event.getRealmId());
switch (event.getType()) {
case LOGIN:
var user = session.users().getUserById(getRealm.get(), event.getUserId());
if (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.");
}
break;
case CLIENT_LOGIN:
var client = session.clients().getClientByClientId(getRealm.get(), event.getClientId());
if (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.");
}
break;
}
}
}

private void logAdminEvent(AdminEvent adminEvent, boolean includeRepresentation) {
Expand Down Expand Up @@ -176,7 +199,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();
Expand All @@ -199,7 +222,7 @@ private void setKeycloakContext(StringBuilder sb) {
}
sb.append("]");
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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 $
Expand Down Expand Up @@ -137,15 +139,15 @@ public boolean createTemporaryMasterRealmAdminUser(String username, String passw
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());
adminUser.setSingleAttribute(TEMP_ADMIN_ATTR_NAME, 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());
Expand Down Expand Up @@ -176,15 +178,15 @@ public boolean createTemporaryMasterRealmAdminService(String clientId, String cl

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

adminClientModel.setAttribute(TEMP_ADMIN_ATTR_NAME, Boolean.TRUE.toString());
// 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());
Expand Down
Loading

0 comments on commit cc0d747

Please sign in to comment.