From 415b05fd29d302f22ea6d3aae046c23cef28d721 Mon Sep 17 00:00:00 2001 From: _ <50262751+hunzlahmalik@users.noreply.github.com> Date: Mon, 18 May 2026 14:06:21 +0500 Subject: [PATCH 1/4] fix(auth): derive SSO display name from email local-part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oauth2-proxy was putting the Cognito sub UUID into x-auth-request-user instead of a human-readable username, so newly-provisioned SSO users landed with a UUID as their User.name. Drop the header read in the ForwardAuth middleware and use the email local-part instead — the same value both apps already fell back to when the header was absent; we're promoting that fallback to the only source. Existing users with UUID names are not auto-corrected: the middleware does not re-sync the name field on subsequent logins. Co-Authored-By: Claude Opus 4.7 --- server/env.ts | 9 ++++--- server/middlewares/authentication.test.ts | 33 ----------------------- server/middlewares/authentication.ts | 3 +-- 3 files changed, 6 insertions(+), 39 deletions(-) diff --git a/server/env.ts b/server/env.ts index bba2e35bdab0..36dcaa34c91a 100644 --- a/server/env.ts +++ b/server/env.ts @@ -520,10 +520,11 @@ export class Environment { /** * The authentication type to use. When set to "SSO", the server will trust - * X-Auth-Request-Email and X-Auth-Request-User headers injected by a reverse - * proxy (e.g. oauth2-proxy, Authelia) for authentication and automatic user - * provisioning. Only enable this when Outline is deployed behind a trusted - * authenticating proxy on a self-hosted instance. + * the X-Auth-Request-Email header injected by a reverse proxy + * (e.g. oauth2-proxy, Authelia) for authentication and automatic user + * provisioning. The display name is derived from the email local-part. + * Only enable this when Outline is deployed behind a trusted authenticating + * proxy on a self-hosted instance. */ @Public @IsOptional() diff --git a/server/middlewares/authentication.test.ts b/server/middlewares/authentication.test.ts index f26eba9551c9..7b4931cb7298 100644 --- a/server/middlewares/authentication.test.ts +++ b/server/middlewares/authentication.test.ts @@ -436,9 +436,6 @@ describe("Authentication middleware", () => { if (header === "x-auth-request-email") { return newEmail; } - if (header === "x-auth-request-user") { - return "New User"; - } return ""; }), }, @@ -455,36 +452,6 @@ describe("Authentication middleware", () => { where: { email: newEmail.toLowerCase() }, }); expect(provisioned).not.toBeNull(); - expect(state.auth.user.email).toEqual(newEmail.toLowerCase()); - expect(state.auth.user.name).toEqual("New User"); - }); - - it("should use email prefix as name when X-Auth-Request-User is absent", async () => { - await buildTeam(); - const state = {} as DefaultState; - const authMiddleware = auth(); - const newEmail = `prefix-${randomString(6)}@example.com`; - - await authMiddleware( - { - // @ts-expect-error mock request - request: { - get: jest.fn((header: string) => { - if (header === "x-auth-request-email") { - return newEmail; - } - return ""; - }), - }, - // @ts-expect-error mock cookies - cookies: { get: jest.fn(() => undefined), set: jest.fn() }, - state, - ip: "127.0.0.1", - cache: {}, - }, - jest.fn() - ); - expect(state.auth.user.email).toEqual(newEmail.toLowerCase()); expect(state.auth.user.name).toEqual( newEmail.toLowerCase().split("@")[0] diff --git a/server/middlewares/authentication.ts b/server/middlewares/authentication.ts index 9fc103f79b36..7dd08083f3b9 100644 --- a/server/middlewares/authentication.ts +++ b/server/middlewares/authentication.ts @@ -375,7 +375,6 @@ async function validateAuthentication( const email = normalizeProxyEmail(token.slice(4)); const localPart = email.split("@")[0]; - const displayName = ctx.request.get("x-auth-request-user") || localPart; const { domain } = parseEmail(email); // Concurrent-creation race guard. The SPA on first-ever login fires @@ -448,7 +447,7 @@ async function validateAuthentication( }); const created = await User.create( { - name: displayName, + name: localPart, email, teamId: team.id, // First user into a brand-new team becomes admin. From 19ae3242751ce993d7e6213051e86bb3fc3a1ba8 Mon Sep 17 00:00:00 2001 From: jawad-khan Date: Wed, 20 May 2026 18:54:38 +0500 Subject: [PATCH 2/4] 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 3/4] 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 = { From ba63618759b6cd63367103c8d75128041b74ae76 Mon Sep 17 00:00:00 2001 From: _ <50262751+hunzlahmalik@users.noreply.github.com> Date: Tue, 2 Jun 2026 15:33:54 +0500 Subject: [PATCH 4/4] fix(auth): reject forwarded email with empty local-part A malformed X-Auth-Request-Email with no local part (e.g. "@example.com") normalises to "@", yielding an empty localPart. Since User.name enforces a min length of 1, provisioning failed with an opaque validation error. Source localPart from parseEmail and reject empty local parts up front with an AuthenticationError for a deterministic failure mode. Co-Authored-By: Claude Opus 4.8 --- server/middlewares/authentication.test.ts | 31 +++++++++++++++++++++++ server/middlewares/authentication.ts | 12 +++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/server/middlewares/authentication.test.ts b/server/middlewares/authentication.test.ts index bcb290a33339..802485fa281b 100644 --- a/server/middlewares/authentication.test.ts +++ b/server/middlewares/authentication.test.ts @@ -568,6 +568,37 @@ describe("Authentication middleware", () => { } }); + it("should reject a forwarded email with no local part", async () => { + await buildTeam(); + const state = {} as DefaultState; + const authMiddleware = auth(); + + // "@example.com" normalises to "@" with an empty + // local part. User.name enforces min length 1, so we reject up front + // rather than letting provisioning fail with an opaque validation error. + await expect( + authMiddleware( + { + // @ts-expect-error mock request + request: { + get: jest.fn((header: string) => { + if (header === "x-auth-request-email") { + return "@example.com"; + } + return ""; + }), + }, + // @ts-expect-error mock cookies + cookies: { get: jest.fn(() => undefined), set: jest.fn() }, + state, + ip: "127.0.0.1", + cache: {}, + }, + jest.fn() + ) + ).rejects.toThrow("Invalid forwarded email: missing local part"); + }); + it("should not match existing users via SQL LIKE wildcard characters", async () => { const team = await buildTeam(); const existingUser = await buildUser({ teamId: team.id }); diff --git a/server/middlewares/authentication.ts b/server/middlewares/authentication.ts index 8c707d4f4760..9e68c502c82b 100644 --- a/server/middlewares/authentication.ts +++ b/server/middlewares/authentication.ts @@ -370,8 +370,16 @@ async function validateAuthentication( service = FORWARDAUTH_SERVICE; const email = normalizeProxyEmail(token.slice(4)); - const localPart = email.split("@")[0]; - const { domain } = parseEmail(email); + const { local: localPart, domain } = parseEmail(email); + + // A malformed forwarded email with no local part (e.g. "@example.com") + // normalises to "@" and yields an empty localPart. + // User.name enforces a min length of 1, so provisioning would otherwise + // fail with an opaque validation error — reject explicitly so the failure + // mode is deterministic. + if (!localPart) { + throw AuthenticationError("Invalid forwarded email: missing local part"); + } // Concurrent-creation race guard. The SPA on first-ever login fires // multiple parallel API requests (docs, team, access tokens, …) with