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 032bc6980518..e65fce238d74 100644 --- a/server/policies/user.test.ts +++ b/server/policies/user.test.ts @@ -1,4 +1,6 @@ +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, EmailDisplay, UserRole } from "@shared/types"; +import env from "@server/env"; import { buildUser, buildTeam, @@ -8,6 +10,51 @@ 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", () => { + let originalAuthType: string | undefined; + + beforeEach(() => { + originalAuthType = env.AUTH_TYPE; + env.AUTH_TYPE = AUTH_TYPE_SSO; + }); + + afterEach(() => { + env.AUTH_TYPE = originalAuthType; + }); + + 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 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({ + 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..0290836b3cf4 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -1,4 +1,6 @@ +import { AUTH_TYPE_SSO } from "@shared/constants"; import { TeamPreference, EmailDisplay } from "@shared/types"; +import env from "@server/env"; import { User, Team } from "@server/models"; import { allow } from "./cancan"; import { @@ -63,15 +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) ) - ) -); + ); +}); 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 f5759a0ad55f..a7f12877b644 100644 --- a/server/routes/api/users/users.test.ts +++ b/server/routes/api/users/users.test.ts @@ -1,5 +1,7 @@ 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"; import { TeamDomain } from "@server/models"; import { @@ -647,6 +649,71 @@ describe("#users.delete", () => { expect(res.status).toEqual(200); }); + describe("when AUTH_TYPE is SSO", () => { + let originalAuthType: string | undefined; + + beforeEach(() => { + originalAuthType = env.AUTH_TYPE; + env.AUTH_TYPE = AUTH_TYPE_SSO; + }); + + afterEach(() => { + env.AUTH_TYPE = originalAuthType; + }); + + 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 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({ + 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(); 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 = {