From 19ae3242751ce993d7e6213051e86bb3fc3a1ba8 Mon Sep 17 00:00:00 2001 From: jawad-khan Date: Wed, 20 May 2026 18:54:38 +0500 Subject: [PATCH 1/2] fix(auth): hide self-service account deletion when AUTH_TYPE is SSO --- server/policies/user.test.ts | 34 +++++++++++++++++++++++ server/policies/user.ts | 4 ++- server/routes/api/users/users.test.ts | 39 +++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/server/policies/user.test.ts b/server/policies/user.test.ts index 032bc6980518..3203d51608df 100644 --- a/server/policies/user.test.ts +++ b/server/policies/user.test.ts @@ -1,4 +1,5 @@ import { TeamPreference, EmailDisplay, UserRole } from "@shared/types"; +import env from "@server/env"; import { buildUser, buildTeam, @@ -8,6 +9,39 @@ import { import { serialize } from "./index"; describe("policies/user", () => { + describe("delete", () => { + it("should allow users to delete their own account by default", async () => { + const user = await buildUser(); + const abilities = serialize(user, user); + expect(abilities.delete).toBeTruthy(); + }); + + describe("when AUTH_TYPE is SSO", () => { + beforeEach(() => { + env.AUTH_TYPE = "SSO"; + }); + + afterEach(() => { + env.AUTH_TYPE = undefined; + }); + + it("should not allow users to delete their own account", async () => { + const user = await buildUser(); + const abilities = serialize(user, user); + expect(abilities.delete).toBeFalsy(); + }); + + it("should still allow admins to delete other users", async () => { + const admin = await buildAdmin(); + const user = await buildUser({ + teamId: admin.teamId, + }); + const abilities = serialize(admin, user); + expect(abilities.delete).toBeTruthy(); + }); + }); + }); + describe("readEmail", () => { it("should allow user to read their own email", async () => { const team = await buildTeam(); diff --git a/server/policies/user.ts b/server/policies/user.ts index 170507945505..d44463c398dc 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -1,4 +1,5 @@ import { TeamPreference, EmailDisplay } from "@shared/types"; +import env from "@server/env"; import { User, Team } from "@server/models"; import { allow } from "./cancan"; import { @@ -68,7 +69,8 @@ allow(User, "delete", User, (actor, user) => isTeamAdmin(actor, user), and( actor.id === user?.id, - !!actor.team.getPreference(TeamPreference.MembersCanDeleteAccount) + !!actor.team.getPreference(TeamPreference.MembersCanDeleteAccount), + env.AUTH_TYPE !== "SSO" ) ) ); diff --git a/server/routes/api/users/users.test.ts b/server/routes/api/users/users.test.ts index f5759a0ad55f..4b53bfb18163 100644 --- a/server/routes/api/users/users.test.ts +++ b/server/routes/api/users/users.test.ts @@ -1,5 +1,6 @@ import { faker } from "@faker-js/faker"; import { TeamPreference, UserRole } from "@shared/types"; +import env from "@server/env"; import ConfirmUpdateEmail from "@server/emails/templates/ConfirmUpdateEmail"; import { TeamDomain } from "@server/models"; import { @@ -647,6 +648,44 @@ describe("#users.delete", () => { expect(res.status).toEqual(200); }); + describe("when AUTH_TYPE is SSO", () => { + beforeEach(() => { + env.AUTH_TYPE = "SSO"; + }); + + afterEach(() => { + env.AUTH_TYPE = undefined; + }); + + it("should not allow users to delete their own account", async () => { + const user = await buildUser(); + await buildUser({ + teamId: user.teamId, + }); + const res = await server.post("/api/users.delete", { + body: { + code: user.deleteConfirmationCode, + token: user.getJwtToken(), + }, + }); + expect(res.status).toEqual(403); + }); + + it("should still allow admins to delete other users", async () => { + const admin = await buildAdmin(); + const user = await buildUser({ + teamId: admin.teamId, + }); + const res = await server.post("/api/users.delete", { + body: { + id: user.id, + token: admin.getJwtToken(), + }, + }); + expect(res.status).toEqual(200); + }); + }); + it("should require authentication", async () => { const res = await server.post("/api/users.delete"); const body = await res.json(); From 5adfc01ae4bd4a7bd37892609a5eaa02c1a29001 Mon Sep 17 00:00:00 2001 From: jawad-khan Date: Wed, 20 May 2026 19:24:13 +0500 Subject: [PATCH 2/2] fix: fixed review points --- app/scenes/Settings/Security.tsx | 30 ++++++++++++--------- app/stores/AuthStore.ts | 3 ++- app/utils/ApiClient.ts | 5 ++-- server/middlewares/authentication.test.ts | 3 ++- server/middlewares/authentication.ts | 4 +-- server/policies/user.test.ts | 17 ++++++++++-- server/policies/user.ts | 16 +++++++----- server/routes/api/users/users.test.ts | 32 +++++++++++++++++++++-- shared/constants.ts | 3 +++ 9 files changed, 83 insertions(+), 30 deletions(-) diff --git a/app/scenes/Settings/Security.tsx b/app/scenes/Settings/Security.tsx index 93445473b46f..936b574519d4 100644 --- a/app/scenes/Settings/Security.tsx +++ b/app/scenes/Settings/Security.tsx @@ -5,7 +5,9 @@ import { useState } from "react"; import * as React from "react"; import { useTranslation, Trans } from "react-i18next"; import { toast } from "sonner"; +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, EmailDisplay } from "@shared/types"; +import env from "~/env"; import ConfirmationDialog from "~/components/ConfirmationDialog"; import Heading from "~/components/Heading"; import type { Option } from "~/components/InputSelect"; @@ -306,19 +308,21 @@ function Security() { onChange={handleViewersCanExportChange} /> - - - + {env.AUTH_TYPE !== AUTH_TYPE_SSO && ( + + + + )} { // throws into the ErrorBoundary. This is belt-and-suspenders on // top of ApiClient.fetch's primary detection; either path lands // us on the same wipeAndReload helper which is idempotent. - if (env.AUTH_TYPE === "SSO" && !res?.data?.user) { + if (env.AUTH_TYPE === AUTH_TYPE_SSO && !res?.data?.user) { Logger.warn( "/auth.info returned no user payload — assuming stale session" ); diff --git a/app/utils/ApiClient.ts b/app/utils/ApiClient.ts index d0e9ee5c6697..270c13fc6c7f 100644 --- a/app/utils/ApiClient.ts +++ b/app/utils/ApiClient.ts @@ -1,6 +1,7 @@ import retry from "fetch-retry"; import trim from "lodash/trim"; import queryString from "query-string"; +import { AUTH_TYPE_SSO } from "@shared/constants"; import EDITOR_VERSION from "@shared/editor/version"; import type { JSONObject } from "@shared/types"; import { Scope } from "@shared/types"; @@ -203,7 +204,7 @@ class ApiClient { // Either signal is sufficient. Both can be true together but only // the first qualifying detection matters since wipeAndReload is // idempotent. - if (env.AUTH_TYPE === "SSO") { + if (env.AUTH_TYPE === AUTH_TYPE_SSO) { const contentType = response.headers.get("content-type") || ""; const finalUrlOffApi = !response.url.includes("/api/"); const wasRedirected = response.redirected && finalUrlOffApi; @@ -244,7 +245,7 @@ class ApiClient { // Handle 401, log out user if (response.status === 401) { if (!this.shareId) { - if (env.AUTH_TYPE === "SSO") { + if (env.AUTH_TYPE === AUTH_TYPE_SSO) { // In ForwardAuth mode, the stale JWT cookie has been cleared by the // server. Navigate to the current URL so the browser makes a fresh // HTTP request — the proxy will inject new identity headers and a new diff --git a/server/middlewares/authentication.test.ts b/server/middlewares/authentication.test.ts index 9365f6742bd5..4de2ef760ec6 100644 --- a/server/middlewares/authentication.test.ts +++ b/server/middlewares/authentication.test.ts @@ -1,4 +1,5 @@ import type { DefaultState } from "koa"; +import { AUTH_TYPE_SSO } from "@shared/constants"; import { randomString } from "@shared/random"; import { Scope } from "@shared/types"; import env from "@server/env"; @@ -417,7 +418,7 @@ describe("Authentication middleware", () => { describe("with ForwardAuth headers", () => { beforeEach(() => { - env.AUTH_TYPE = "SSO"; + env.AUTH_TYPE = AUTH_TYPE_SSO; }); afterEach(() => { diff --git a/server/middlewares/authentication.ts b/server/middlewares/authentication.ts index 4452bca17df1..2e9db16aac92 100644 --- a/server/middlewares/authentication.ts +++ b/server/middlewares/authentication.ts @@ -1,6 +1,7 @@ import { addDays } from "date-fns"; import type { Next } from "koa"; import capitalize from "lodash/capitalize"; +import { AUTH_TYPE_SSO } from "@shared/constants"; import { UserRole } from "@shared/types"; import { slugifyDomain } from "@shared/utils/domains"; import { parseEmail } from "@shared/utils/email"; @@ -29,9 +30,6 @@ import { /** Service identifier used by the ForwardAuth authentication flow. */ export const FORWARDAUTH_SERVICE = "forwardauth"; -/** The {@link env.AUTH_TYPE} value that activates ForwardAuth/SSO mode. */ -const AUTH_TYPE_SSO = "SSO"; - type AuthenticationOptions = { /** Role required to access the route. */ role?: UserRole; diff --git a/server/policies/user.test.ts b/server/policies/user.test.ts index 3203d51608df..e65fce238d74 100644 --- a/server/policies/user.test.ts +++ b/server/policies/user.test.ts @@ -1,3 +1,4 @@ +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, EmailDisplay, UserRole } from "@shared/types"; import env from "@server/env"; import { @@ -17,12 +18,15 @@ describe("policies/user", () => { }); describe("when AUTH_TYPE is SSO", () => { + let originalAuthType: string | undefined; + beforeEach(() => { - env.AUTH_TYPE = "SSO"; + originalAuthType = env.AUTH_TYPE; + env.AUTH_TYPE = AUTH_TYPE_SSO; }); afterEach(() => { - env.AUTH_TYPE = undefined; + env.AUTH_TYPE = originalAuthType; }); it("should not allow users to delete their own account", async () => { @@ -31,6 +35,15 @@ describe("policies/user", () => { expect(abilities.delete).toBeFalsy(); }); + it("should not allow admins to delete their own account", async () => { + const admin = await buildAdmin(); + await buildUser({ + teamId: admin.teamId, + }); + const abilities = serialize(admin, admin); + expect(abilities.delete).toBeFalsy(); + }); + it("should still allow admins to delete other users", async () => { const admin = await buildAdmin(); const user = await buildUser({ diff --git a/server/policies/user.ts b/server/policies/user.ts index d44463c398dc..0290836b3cf4 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -1,3 +1,4 @@ +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, EmailDisplay } from "@shared/types"; import env from "@server/env"; import { User, Team } from "@server/models"; @@ -64,16 +65,19 @@ allow(User, "readEmail", User, (actor, user) => { ); }); -allow(User, "delete", User, (actor, user) => - or( +allow(User, "delete", User, (actor, user) => { + if (env.AUTH_TYPE === AUTH_TYPE_SSO && actor.id === user?.id) { + return false; + } + + return or( isTeamAdmin(actor, user), and( actor.id === user?.id, - !!actor.team.getPreference(TeamPreference.MembersCanDeleteAccount), - env.AUTH_TYPE !== "SSO" + !!actor.team.getPreference(TeamPreference.MembersCanDeleteAccount) ) - ) -); + ); +}); allow(User, ["activate", "suspend"], User, (actor, user) => and(isTeamAdmin(actor, user), user?.id !== actor.id) diff --git a/server/routes/api/users/users.test.ts b/server/routes/api/users/users.test.ts index 4b53bfb18163..a7f12877b644 100644 --- a/server/routes/api/users/users.test.ts +++ b/server/routes/api/users/users.test.ts @@ -1,4 +1,5 @@ import { faker } from "@faker-js/faker"; +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, UserRole } from "@shared/types"; import env from "@server/env"; import ConfirmUpdateEmail from "@server/emails/templates/ConfirmUpdateEmail"; @@ -649,12 +650,15 @@ describe("#users.delete", () => { }); describe("when AUTH_TYPE is SSO", () => { + let originalAuthType: string | undefined; + beforeEach(() => { - env.AUTH_TYPE = "SSO"; + originalAuthType = env.AUTH_TYPE; + env.AUTH_TYPE = AUTH_TYPE_SSO; }); afterEach(() => { - env.AUTH_TYPE = undefined; + env.AUTH_TYPE = originalAuthType; }); it("should not allow users to delete their own account", async () => { @@ -671,6 +675,30 @@ describe("#users.delete", () => { expect(res.status).toEqual(403); }); + it("should not allow admins to delete their own account", async () => { + const admin = await buildAdmin(); + await buildUser({ + teamId: admin.teamId, + }); + const res = await server.post("/api/users.delete", { + body: { + code: admin.deleteConfirmationCode, + token: admin.getJwtToken(), + }, + }); + expect(res.status).toEqual(403); + }); + + it("should not allow users to request their own deletion", async () => { + const user = await buildUser(); + const res = await server.post("/api/users.requestDelete", { + body: { + token: user.getJwtToken(), + }, + }); + expect(res.status).toEqual(403); + }); + it("should still allow admins to delete other users", async () => { const admin = await buildAdmin(); const user = await buildUser({ diff --git a/shared/constants.ts b/shared/constants.ts index 6439bf0be389..3fc92962380b 100644 --- a/shared/constants.ts +++ b/shared/constants.ts @@ -7,6 +7,9 @@ import { NotificationBadgeType, } from "./types"; +/** The {@link PublicEnv.AUTH_TYPE} value that activates ForwardAuth/SSO mode. */ +export const AUTH_TYPE_SSO = "SSO"; + export const MAX_AVATAR_DISPLAY = 6; export const Pagination = {