Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions app/scenes/Settings/Security.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -306,19 +308,21 @@ function Security() {
onChange={handleViewersCanExportChange}
/>
</SettingRow>
<SettingRow
label={t("Users can delete account")}
name={TeamPreference.MembersCanDeleteAccount}
description={t(
"When enabled, users can delete their own account from the workspace"
)}
>
<Switch
id={TeamPreference.MembersCanDeleteAccount}
checked={team.getPreference(TeamPreference.MembersCanDeleteAccount)}
onChange={handleMembersCanDeleteAccountChange}
/>
</SettingRow>
{env.AUTH_TYPE !== AUTH_TYPE_SSO && (
<SettingRow
label={t("Users can delete account")}
name={TeamPreference.MembersCanDeleteAccount}
description={t(
"When enabled, users can delete their own account from the workspace"
)}
>
<Switch
id={TeamPreference.MembersCanDeleteAccount}
checked={team.getPreference(TeamPreference.MembersCanDeleteAccount)}
onChange={handleMembersCanDeleteAccountChange}
/>
</SettingRow>
)}
<SettingRow
label={t("Email address visibility")}
name={TeamPreference.EmailDisplay}
Expand Down
3 changes: 2 additions & 1 deletion app/stores/AuthStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import invariant from "invariant";
import isNil from "lodash/isNil";
import { observable, action, computed, autorun, runInAction } from "mobx";
import { getCookie, setCookie } from "tiny-cookie";
import { AUTH_TYPE_SSO } from "@shared/constants";
import type { CustomTheme } from "@shared/types";
import Storage from "@shared/utils/Storage";
import { getCookieDomain, parseDomain } from "@shared/utils/domains";
Expand Down Expand Up @@ -214,7 +215,7 @@ export default class AuthStore extends Store<Team> {
// 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"
);
Expand Down
5 changes: 3 additions & 2 deletions app/utils/ApiClient.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions server/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,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()
Expand Down
67 changes: 33 additions & 34 deletions server/middlewares/authentication.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -417,7 +418,7 @@ describe("Authentication middleware", () => {

describe("with ForwardAuth headers", () => {
beforeEach(() => {
env.AUTH_TYPE = "SSO";
env.AUTH_TYPE = AUTH_TYPE_SSO;
});

afterEach(() => {
Expand Down Expand Up @@ -510,9 +511,6 @@ describe("Authentication middleware", () => {
if (header === "x-auth-request-email") {
return newEmail;
}
if (header === "x-auth-request-user") {
return "New User";
}
return "";
}),
},
Expand All @@ -529,36 +527,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]
Expand Down Expand Up @@ -600,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 "@<DEFAULT_EMAIL_DOMAIN>" 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 });
Expand Down
19 changes: 12 additions & 7 deletions server/middlewares/authentication.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -372,9 +370,16 @@ async function validateAuthentication(
service = FORWARDAUTH_SERVICE;

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);
const { local: localPart, domain } = parseEmail(email);

// A malformed forwarded email with no local part (e.g. "@example.com")
// normalises to "@<DEFAULT_EMAIL_DOMAIN>" 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
Expand Down Expand Up @@ -446,7 +451,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.
Expand Down
47 changes: 47 additions & 0 deletions server/policies/user.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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();
Expand Down
14 changes: 10 additions & 4 deletions server/policies/user.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading