diff --git a/messages/en-US.json b/messages/en-US.json index d75c3bdc2..5199aee71 100644 --- a/messages/en-US.json +++ b/messages/en-US.json @@ -1176,6 +1176,10 @@ "idpJmespathAboutDescriptionLink": "Learn more about JMESPath", "idpJmespathLabel": "Identifier Path", "idpJmespathLabelDescription": "The path to the user identifier in the ID token", + "idpIdentifierChangeTitle": "Identifier Path Change Warning", + "idpIdentifierChangeDescription": "You are about to change the identifier path. This will affect how existing users are mapped. Users who previously signed in through this identity provider may no longer be recognized as the same users.", + "idpIdentifierChangeConfirmMessage": "I confirm", + "idpIdentifierChangeWarningText": "This will affect how existing users are mapped", "idpJmespathEmailPathOptional": "Email Path (Optional)", "idpJmespathEmailPathOptionalDescription": "The path to the user's email in the ID token", "idpJmespathNamePathOptional": "Name Path (Optional)", diff --git a/server/lib/blueprints/findOrgUser.ts b/server/lib/blueprints/findOrgUser.ts new file mode 100644 index 000000000..1fc054fd1 --- /dev/null +++ b/server/lib/blueprints/findOrgUser.ts @@ -0,0 +1,38 @@ +import { and, asc, eq, or } from "drizzle-orm"; +import { Transaction, User, userOrgs, users } from "@server/db"; + +export async function findOrgUserByIdentifier( + trx: Transaction, + orgId: string, + identifier: string +): Promise { + const [match] = await trx + .select() + .from(users) + .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) + .where( + and( + or(eq(users.username, identifier), eq(users.email, identifier)), + eq(userOrgs.orgId, orgId) + ) + ) + .orderBy(asc(users.dateCreated), asc(users.userId)) + .limit(1); + + return match?.user ?? null; +} + +export async function resolveOrgUserIds( + trx: Transaction, + orgId: string, + identifiers: string[] +): Promise { + const userIds = new Set(); + for (const identifier of identifiers) { + const user = await findOrgUserByIdentifier(trx, orgId, identifier); + if (user) { + userIds.add(user.userId); + } + } + return [...userIds]; +} diff --git a/server/lib/blueprints/privateResources.ts b/server/lib/blueprints/privateResources.ts index 65cf199d6..085fed836 100644 --- a/server/lib/blueprints/privateResources.ts +++ b/server/lib/blueprints/privateResources.ts @@ -11,15 +11,14 @@ import { siteNetworks, siteResources, Transaction, - userOrgs, - users, userSiteResources, networks } from "@server/db"; import { sites } from "@server/db"; -import { eq, and, ne, inArray, or, isNotNull } from "drizzle-orm"; +import { eq, and, ne, inArray, isNotNull } from "drizzle-orm"; import { Config } from "./types"; import { getOrCreateLabelIds, syncSiteResourceLabels } from "./labels"; +import { resolveOrgUserIds } from "./findOrgUser"; import logger from "@server/logger"; import { defaultRoleAllowedActions } from "@server/routers/role/createRole"; import { getNextAvailableAliasAddress } from "../ip"; @@ -389,28 +388,22 @@ export async function updatePrivateResources( .where(eq(userSiteResources.siteResourceId, siteResourceId)); if (resourceData.users.length > 0) { - // get userIds from username - const usersToUpdate = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or( - inArray(users.username, resourceData.users), - inArray(users.email, resourceData.users) - ), - eq(userOrgs.orgId, orgId) - ) - ); + const userIds = await resolveOrgUserIds( + trx, + orgId, + resourceData.users + ); - const userIds = usersToUpdate.map((user) => user.user.userId); - - await trx - .insert(userSiteResources) - .values( - userIds.map((userId) => ({ userId, siteResourceId })) - ); + if (userIds.length > 0) { + await trx + .insert(userSiteResources) + .values( + userIds.map((userId) => ({ + userId, + siteResourceId + })) + ); + } } // Get all admin role IDs for this org to exclude from deletion @@ -721,28 +714,22 @@ export async function updatePrivateResources( } if (resourceData.users.length > 0) { - // get userIds from username - const usersToUpdate = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or( - inArray(users.username, resourceData.users), - inArray(users.email, resourceData.users) - ), - eq(userOrgs.orgId, orgId) - ) - ); + const userIds = await resolveOrgUserIds( + trx, + orgId, + resourceData.users + ); - const userIds = usersToUpdate.map((user) => user.user.userId); - - await trx - .insert(userSiteResources) - .values( - userIds.map((userId) => ({ userId, siteResourceId })) - ); + if (userIds.length > 0) { + await trx + .insert(userSiteResources) + .values( + userIds.map((userId) => ({ + userId, + siteResourceId + })) + ); + } } if (resourceData.machines.length > 0) { diff --git a/server/lib/blueprints/publicResources.ts b/server/lib/blueprints/publicResources.ts index a76bcc26c..7822c2b8a 100644 --- a/server/lib/blueprints/publicResources.ts +++ b/server/lib/blueprints/publicResources.ts @@ -46,11 +46,12 @@ import { encrypt } from "@server/lib/crypto"; import logger from "@server/logger"; import { defaultRoleAllowedActions } from "@server/routers/role/createRole"; import { pickPort } from "@server/routers/target/helpers"; -import { and, asc, eq, isNotNull, ne, or } from "drizzle-orm"; +import { and, asc, eq, isNotNull, ne } from "drizzle-orm"; import { tierMatrix } from "../billing/tierMatrix"; import { isValidCIDR, isValidIP, isValidUrlGlobPattern } from "../validators"; import { Config, isTargetsOnlyResource, TargetData } from "./types"; import { getOrCreateLabelIds, syncResourceLabels } from "./labels"; +import { findOrgUserByIdentifier } from "./findOrgUser"; import { LimitId } from "../billing"; import { usageService } from "../billing/usageService"; import { syncInferenceAiConfig } from "./aiProviders"; @@ -1563,29 +1564,19 @@ async function syncUserResources( .where(eq(userResources.resourceId, resourceId)); for (const username of ssoUsers) { - const [user] = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or(eq(users.username, username), eq(users.email, username)), - eq(userOrgs.orgId, orgId) - ) - ) - .limit(1); + const user = await findOrgUserByIdentifier(trx, orgId, username); if (!user) { throw new Error(`User not found: ${username} in org ${orgId}`); } const existingUserResource = existingUserResources.find( - (rr) => rr.userId === user.user.userId + (rr) => rr.userId === user.userId ); if (!existingUserResource) { await trx.insert(userResources).values({ - userId: user.user.userId, + userId: user.userId, resourceId: resourceId }); } @@ -1955,29 +1946,19 @@ async function syncUserPolicies( .where(eq(userPolicies.resourcePolicyId, policyId)); for (const username of ssoUsers) { - const [user] = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or(eq(users.username, username), eq(users.email, username)), - eq(userOrgs.orgId, orgId) - ) - ) - .limit(1); + const user = await findOrgUserByIdentifier(trx, orgId, username); if (!user) { throw new Error(`User not found: ${username} in org ${orgId}`); } const existingUserPolicy = existingUserPoliciesList.find( - (up) => up.userId === user.user.userId + (up) => up.userId === user.userId ); if (!existingUserPolicy) { await trx.insert(userPolicies).values({ - userId: user.user.userId, + userId: user.userId, resourcePolicyId: policyId }); } diff --git a/server/lib/blueprints/resourcePolicies.ts b/server/lib/blueprints/resourcePolicies.ts index 87e9101cc..4a883f64b 100644 --- a/server/lib/blueprints/resourcePolicies.ts +++ b/server/lib/blueprints/resourcePolicies.ts @@ -13,7 +13,7 @@ import { userPolicies, users } from "@server/db"; -import { eq, and, or } from "drizzle-orm"; +import { eq, and } from "drizzle-orm"; import { Config, ResourcePolicyData } from "./types"; import logger from "@server/logger"; import { getUniqueResourcePolicyName } from "@server/db/names"; @@ -22,6 +22,7 @@ import { idpExistsForOrg } from "@server/lib/idp/idpExistsForOrg"; import { isValidCIDR, isValidIP, isValidUrlGlobPattern } from "../validators"; import { isLicensedOrSubscribed } from "#dynamic/lib/isLicencedOrSubscribed"; import { tierMatrix } from "../billing/tierMatrix"; +import { findOrgUserByIdentifier } from "./findOrgUser"; export type ResourcePoliciesResults = { resourcePolicyId: number; @@ -466,17 +467,7 @@ async function syncUserPolicies( .where(eq(userPolicies.resourcePolicyId, policyId)); for (const username of ssoUsers) { - const [user] = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or(eq(users.username, username), eq(users.email, username)), - eq(userOrgs.orgId, orgId) - ) - ) - .limit(1); + const user = await findOrgUserByIdentifier(trx, orgId, username); if (!user) { logger.warn( @@ -486,12 +477,12 @@ async function syncUserPolicies( } const alreadyExists = existingUserPolicies.some( - (up) => up.userId === user.user.userId + (up) => up.userId === user.userId ); if (!alreadyExists) { await trx.insert(userPolicies).values({ - userId: user.user.userId, + userId: user.userId, resourcePolicyId: policyId }); } @@ -536,17 +527,7 @@ async function addUserPolicies( trx: Transaction ) { for (const username of ssoUsers) { - const [user] = await trx - .select() - .from(users) - .innerJoin(userOrgs, eq(users.userId, userOrgs.userId)) - .where( - and( - or(eq(users.username, username), eq(users.email, username)), - eq(userOrgs.orgId, orgId) - ) - ) - .limit(1); + const user = await findOrgUserByIdentifier(trx, orgId, username); if (!user) { logger.warn( @@ -556,7 +537,7 @@ async function addUserPolicies( } await trx.insert(userPolicies).values({ - userId: user.user.userId, + userId: user.userId, resourcePolicyId: policyId }); } diff --git a/src/app/[orgId]/settings/(private)/idp/[idpId]/general/page.tsx b/src/app/[orgId]/settings/(private)/idp/[idpId]/general/page.tsx index b2ad61d67..e7e52b068 100644 --- a/src/app/[orgId]/settings/(private)/idp/[idpId]/general/page.tsx +++ b/src/app/[orgId]/settings/(private)/idp/[idpId]/general/page.tsx @@ -46,6 +46,7 @@ import { AxiosResponse } from "axios"; import { ListRolesResponse } from "@server/routers/role"; import AutoProvisionConfigWidget from "@app/components/AutoProvisionConfigWidget"; import IdpAutoProvisionUsersDescription from "@app/components/IdpAutoProvisionUsersDescription"; +import IdpIdentifierChangeDialog from "@app/components/IdpIdentifierChangeDialog"; import { PaidFeaturesAlert } from "@app/components/PaidFeaturesAlert"; import { tierMatrix } from "@server/lib/billing/tierMatrix"; import { @@ -75,6 +76,12 @@ export default function GeneralPage() { >([createMappingBuilderRule()]); const [rawRoleExpression, setRawRoleExpression] = useState(""); const [variant, setVariant] = useState<"oidc" | "google" | "azure">("oidc"); + const [originalIdentifierPath, setOriginalIdentifierPath] = useState(""); + const [identifierConfirmOpen, setIdentifierConfirmOpen] = useState(false); + const [pendingPayload, setPendingPayload] = useState | null>(null); const dashboardRedirectUrl = `${env.app.dashboardUrl}/auth/idp/${idpId}/oidc/callback`; const [redirectUrl, setRedirectUrl] = useState( @@ -184,6 +191,9 @@ export default function GeneralPage() { const data = res.data.data; const roleMapping = data.idpOrg.roleMapping; const idpVariant = data.idpOidcConfig?.variant || "oidc"; + setOriginalIdentifierPath( + data.idpOidcConfig?.identifierPath ?? "sub" + ); setRedirectUrl(res.data.data.redirectUrl); // Set the variant @@ -378,18 +388,56 @@ export default function GeneralPage() { }; } - const res = await api.post( - `/org/${orgId}/idp/${idpId}/oidc`, - payload - ); + const nextIdentifierPath = + variant === "oidc" + ? (data as OidcFormValues).identifierPath + : undefined; - if (res.status === 200) { - toast({ - title: t("success"), - description: t("idpUpdatedDescription") - }); - router.refresh(); + if ( + typeof nextIdentifierPath === "string" && + nextIdentifierPath !== originalIdentifierPath + ) { + setPendingPayload(payload); + setIdentifierConfirmOpen(true); + return; } + + await persistIdp(payload); + } catch (e) { + toast({ + title: t("error"), + description: formatAxiosError(e), + variant: "destructive" + }); + } finally { + setLoading(false); + } + } + + async function persistIdp(payload: Record) { + const res = await api.post(`/org/${orgId}/idp/${idpId}/oidc`, payload); + + if (res.status === 200) { + if (typeof payload.identifierPath === "string") { + setOriginalIdentifierPath(payload.identifierPath); + } + toast({ + title: t("success"), + description: t("idpUpdatedDescription") + }); + router.refresh(); + } + } + + async function confirmIdentifierChange() { + if (!pendingPayload) { + return; + } + + setLoading(true); + try { + await persistIdp(pendingPayload); + setPendingPayload(null); } catch (e) { toast({ title: t("error"), @@ -407,6 +455,16 @@ export default function GeneralPage() { return ( <> + { + setIdentifierConfirmOpen(open); + if (!open) { + setPendingPayload(null); + } + }} + onConfirm={confirmIdentifierChange} + /> diff --git a/src/app/admin/idp/[idpId]/general/page.tsx b/src/app/admin/idp/[idpId]/general/page.tsx index c9506b027..8b7ed226f 100644 --- a/src/app/admin/idp/[idpId]/general/page.tsx +++ b/src/app/admin/idp/[idpId]/general/page.tsx @@ -41,6 +41,7 @@ import { } from "@app/components/InfoSection"; import CopyToClipboard from "@app/components/CopyToClipboard"; import IdpTypeBadge from "@app/components/IdpTypeBadge"; +import IdpIdentifierChangeDialog from "@app/components/IdpIdentifierChangeDialog"; import { useTranslations } from "next-intl"; export default function GeneralPage() { @@ -51,6 +52,12 @@ export default function GeneralPage() { const [loading, setLoading] = useState(false); const [initialLoading, setInitialLoading] = useState(true); const [variant, setVariant] = useState<"oidc" | "google" | "azure">("oidc"); + const [originalIdentifierPath, setOriginalIdentifierPath] = useState(""); + const [identifierConfirmOpen, setIdentifierConfirmOpen] = useState(false); + const [pendingPayload, setPendingPayload] = useState | null>(null); const redirectUrl = `${env.app.dashboardUrl}/auth/idp/${idpId}/oidc/callback`; const t = useTranslations(); @@ -141,6 +148,9 @@ export default function GeneralPage() { | "google" | "azure") || "oidc"; setVariant(idpVariant); + setOriginalIdentifierPath( + data.idpOidcConfig?.identifierPath ?? "sub" + ); let tenantId = ""; if (idpVariant === "azure" && data.idpOidcConfig?.authUrl) { @@ -258,15 +268,56 @@ export default function GeneralPage() { }; } - const res = await api.post(`/idp/${idpId}/oidc`, payload); + const nextIdentifierPath = + variant === "oidc" + ? (data as OidcFormValues).identifierPath + : undefined; - if (res.status === 200) { - toast({ - title: t("success"), - description: t("idpUpdatedDescription") - }); - router.refresh(); + if ( + typeof nextIdentifierPath === "string" && + nextIdentifierPath !== originalIdentifierPath + ) { + setPendingPayload(payload); + setIdentifierConfirmOpen(true); + return; } + + await persistIdp(payload); + } catch (e) { + toast({ + title: t("error"), + description: formatAxiosError(e), + variant: "destructive" + }); + } finally { + setLoading(false); + } + } + + async function persistIdp(payload: Record) { + const res = await api.post(`/idp/${idpId}/oidc`, payload); + + if (res.status === 200) { + if (typeof payload.identifierPath === "string") { + setOriginalIdentifierPath(payload.identifierPath); + } + toast({ + title: t("success"), + description: t("idpUpdatedDescription") + }); + router.refresh(); + } + } + + async function confirmIdentifierChange() { + if (!pendingPayload) { + return; + } + + setLoading(true); + try { + await persistIdp(pendingPayload); + setPendingPayload(null); } catch (e) { toast({ title: t("error"), @@ -284,6 +335,16 @@ export default function GeneralPage() { return ( <> + { + setIdentifierConfirmOpen(open); + if (!open) { + setPendingPayload(null); + } + }} + onConfirm={confirmIdentifierChange} + /> diff --git a/src/components/IdpIdentifierChangeDialog.tsx b/src/components/IdpIdentifierChangeDialog.tsx new file mode 100644 index 000000000..03ba7ee20 --- /dev/null +++ b/src/components/IdpIdentifierChangeDialog.tsx @@ -0,0 +1,35 @@ +"use client"; + +import ConfirmDeleteDialog from "@app/components/ConfirmDeleteDialog"; +import { useTranslations } from "next-intl"; + +type IdpIdentifierChangeDialogProps = { + open: boolean; + setOpen: (open: boolean) => void; + onConfirm: () => Promise; +}; + +export default function IdpIdentifierChangeDialog({ + open, + setOpen, + onConfirm +}: IdpIdentifierChangeDialogProps) { + const t = useTranslations(); + + return ( + +

{t("idpIdentifierChangeDescription")}

+ + } + buttonText={t("saveGeneralSettings")} + onConfirm={onConfirm} + string={t("idpIdentifierChangeConfirmMessage")} + title={t("idpIdentifierChangeTitle")} + warningText={t("idpIdentifierChangeWarningText")} + /> + ); +}