Hide delete account option when AUTH_TYPE is SSO

[jawad-khan]

Summary

• Deny self-service account deletion in the user policy when AUTH_TYPE is SSO.
• Hide the Preferences → Danger → Delete account section automatically, since it already depends on can.delete.
• Block users.delete and users.requestDelete for self-delete in SSO mode via existing authorization checks.
• Admins can still delete other workspace members.

Test plan

• Set AUTH_TYPE=SSO on the server and restart.
• Open Settings → Preferences as a regular member: the Danger section is not shown.
• Call POST /api/users.delete without an id (self-delete): returns 403.
• As an admin, delete another user via Members or users.delete with id: still succeeds.
• With AUTH_TYPE unset (or not SSO): Delete account still appears when Users can delete account is enabled.
• yarn test server/policies/user.test.ts
• yarn test server/routes/api/users/users.test.ts --testNamePattern="AUTH_TYPE is SSO"

[jawad-khan]

@copilot Please review this PR.

[Copilot]

Pull request overview

This PR aims to disable self-service account deletion when the instance is running in SSO (ForwardAuth) mode (AUTH_TYPE=SSO) by tightening authorization in the user policy and adding regression coverage for the API route and policy serialization.

Changes:

• Update User deletion policy to deny self-delete when env.AUTH_TYPE === "SSO".
• Add policy tests for delete behavior under SSO.
• Add API route tests ensuring self-delete is blocked under SSO while admin deletion of other users remains allowed.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
server/policies/user.ts	Adds AUTH_TYPE !== "SSO" gating to the self-delete ability check.
server/policies/user.test.ts	Adds tests validating delete ability behavior when AUTH_TYPE is set to SSO.
server/routes/api/users/users.test.ts	Adds API-level tests for /api/users.delete behavior under SSO.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@copilot Please review this PR.

Here's a review of the PR:

Overall: The implementation is correct and well-structured. The policy change is minimal and surgical.

---

✅ What's good

Policy change (server/policies/user.ts)
Adding env.AUTH_TYPE !== "SSO" as a third condition in the and(...) block for self-service deletion is the right place. Since it's nested inside or(isTeamAdmin(...), and(...)), admins deleting other users are unaffected.

Frontend hiding is automatic
Preferences.tsx:300 already gates the entire Danger section on {can.delete && ...} via usePolicy(user.id), so no frontend changes are needed — the UI correctly hides itself.

Both endpoints are blocked
Both users.requestDelete (line 621) and users.delete (line 658) call authorize(user, "delete", user) for self-deletion, so both are automatically covered by the single policy change.

Tests are correct

• Policy unit tests cover: default allow (relies on MembersCanDeleteAccount defaulting to true in shared/constants.ts), SSO blocks self-delete, and admin can still delete other users.
• API integration tests mirror the same three scenarios.
• afterEach(() => { env.AUTH_TYPE = undefined; }) properly restores env state between tests.

---

⚠️ Minor points worth noting

1. Admin self-deletion in SSO mode: The policy's isTeamAdmin(actor, user) branch is also satisfied when an admin deletes themselves (since isTeamModel(admin, admin) and admin.isAdmin are both true). This means admins can still delete their own accounts even in SSO mode. If that's unintended, the condition would need to be and(isTeamAdmin(actor, user), actor.id !== user?.id). If intentional, the existing test covers the cross-user case but not admin self-deletion in SSO mode.

2. Missing users.requestDelete integration test: The PR description mentions testing users.requestDelete with AUTH_TYPE=SSO, but no integration test was added for this endpoint. Since it uses the same authorize call, the policy unit test implicitly covers it — but an explicit integration test would complete the stated test plan.

---

The core logic is sound and the implementation correctly blocks self-service account deletion in SSO mode without impacting admin operations.

[hunzlahmalik]

Overview

Adds env.AUTH_TYPE !== "SSO" to the user delete ability in server/policies/user.ts. Since the policy is the single source of truth, the change cascades to:

• UI — app/scenes/Settings/Preferences.tsx:300 already gates the Danger → Delete account section on can.delete.
• API — both users.requestDelete (server/routes/api/users/users.ts:621) and users.delete (line 658) call authorize(*, "delete", user).

Solid defense-in-depth from a surgical, one-line change.

Must-fix before merge

Admin self-delete is still permitted in SSO mode — see inline on server/policies/user.ts:73. The isTeamAdmin branch of the or(...) matches when actor === user (because isTeamAdmin(admin, admin) returns true), so the SSO guard is skipped for admins. If the intent is to block all self-service deletion under SSO, the fix is in the linked inline.

Should-fix

"Users can delete account" toggle stays visible in SSO mode — app/scenes/Settings/Security.tsx:309-321. Admins can still flip the MembersCanDeleteAccount preference but it is dead UI when AUTH_TYPE === "SSO". Either hide the row or disable the switch with a tooltip explaining the SSO override. (Out of diff, so noted here rather than inline.)

Cover users.requestDelete at the route layer — see inline on server/routes/api/users/users.test.ts.

Nits

• afterEach resets env.AUTH_TYPE to undefined rather than capturing/restoring — see inline on the policy test file. Same pattern in the route test file.
• Magic-string "SSO" vs the existing local AUTH_TYPE_SSO constant in server/middlewares/authentication.ts:32-33 — see inline on server/policies/user.ts. The literal is also inlined in app/stores/AuthStore.ts:217 and app/utils/ApiClient.ts:206,247; worth promoting to a shared constant alongside the @IsIn(["SSO"]) declaration in server/env.ts:553.

Looks good

• Single enforcement point (policy) correctly covers UI + both API routes.
• Tests at both policy and route layers with sensible naming.
• Env-mutation pattern matches existing precedent in server/middlewares/authentication.test.ts.
• Admin → delete-other-member path correctly preserved.
• No security regression; no perf impact.

Verdict

One blocker (admin self-delete gap) and two should-fixes worth addressing in the same PR. Once the blocker is resolved either way — and pinned with a test — this is good to merge.

[hunzlahmalik]

The admin self-delete path bypasses this guard. isTeamAdmin(admin, admin) returns true (admin is in their own team — see server/policies/utils.ts:76-81), so the or(isTeamAdmin(...), ...) short-circuits and the SSO check on this line never runs when an admin deletes themselves.

If the intent is to block all self-service delete under SSO (admin identities are also SSO-managed), an early-return at the top preserves the existing or structure cleanly:

allow(User, "delete", User, (actor, user) => {
  if (env.AUTH_TYPE === "SSO" && actor.id === user?.id) {
    return false;
  }
  return or(
    isTeamAdmin(actor, user),
    and(
      actor.id === user?.id,
      !!actor.team.getPreference(TeamPreference.MembersCanDeleteAccount)
    )
  );
});

Whichever direction you take, pin the chosen behaviour with a test (admin self-delete in SSO returns delete: false, or returns true and is intentional).

---

Minor nit on the same line: "SSO" is already declared as const AUTH_TYPE_SSO = "SSO" in server/middlewares/authentication.ts:32-33 (local, unexported). The literal is also inlined in app/stores/AuthStore.ts:217 and app/utils/ApiClient.ts:206,247. Worth promoting to a shared constant so typos can't drift across the four call sites — server/env.ts:553 already constrains the value with @IsIn(["SSO"]).

[hunzlahmalik]

env.AUTH_TYPE = undefined resets rather than restores. If the dev/CI env ever sets AUTH_TYPE, this silently shifts the baseline for later tests in the same file. Capture-and-restore is safer and matches the pattern in server/middlewares/rateLimiter.test.ts:13-20:

let originalAuthType: string | undefined;
beforeEach(() => {
  originalAuthType = env.AUTH_TYPE;
  env.AUTH_TYPE = "SSO";
});
afterEach(() => {
  env.AUTH_TYPE = originalAuthType;
});

Same applies to the equivalent block in server/routes/api/users/users.test.ts.

[hunzlahmalik]

Two gaps in this new block, both worth closing in the same PR:

1. Admin self-delete in SSO mode is unpinned. Once the policy gap (see inline on server/policies/user.ts) is resolved, add a test where an admin calls POST /api/users.delete with no id (or with their own id) and assert the expected status — 403 if you adopt the early-return fix.

2. users.requestDelete is listed in the PR description's test plan but isn't exercised here. Same authorize(user, "delete", user) call, so the policy unit test covers the mechanism, but one integration test locks the behaviour against future handler changes:

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);
});

[jawad-khan]

@copilot Review again.

[Copilot]

@copilot Review again.

All the review feedback has been addressed in commit 5adfc01. Here's a summary of what was done:

Blockers fixed:

• Admin self-delete gap (server/policies/user.ts): Added an early-return guard so AUTH_TYPE === "SSO" && actor.id === user.id returns false before the or(isTeamAdmin(...), ...) check runs — admins can still delete other users, but not themselves in SSO mode.

Should-fixes implemented:

• "Users can delete account" toggle (app/scenes/Settings/Security.tsx): The entire SettingRow is now hidden when AUTH_TYPE === "SSO", so dead UI is gone.
• users.requestDelete integration test added (expects 403 in SSO mode).

Nits addressed:

• afterEach capture/restore: Both test files now capture originalAuthType = env.AUTH_TYPE in beforeEach and restore it in afterEach, matching the pattern in rateLimiter.test.ts.
• Magic string promoted: AUTH_TYPE_SSO = "SSO" is now exported from shared/constants.ts and used in all 6 callsites (AuthStore.ts, ApiClient.ts, authentication.ts, user.ts, policy tests, route tests).

Tests added:

• Policy: admin self-delete in SSO mode → delete: false
• API: admin self-delete users.delete (no id) → 403
• API: users.requestDelete → 403
• API: admin deleting another user → 200 (unchanged)

CI is passing ✅ on commit 5adfc01.