Skip to content

Commit 07765ba

Browse files
authored
fix(auth): admin member management works without the admin's own membership (#881)
adminAddMember forwarded the caller's headers into Better Auth's addMember, which checks the caller's ORGANIZATION permissions — a platform admin who was not a member of the target organization (the whole point of an admin route) got FORBIDDEN. The mutation is now a direct write like the rest of AdminOrganizationsService: membership-limit policy enforced, duplicate memberships refused. Tests now cover adding a member to an organization the admin does not belong to — the case the old fixtures masked by creating every organization owned by the admin. - adminUpdateMemberRole refuses to demote an organization's only owner, matching adminRemoveMember's last-owner guard - getOrganizationForAdmin uses the standard requireAdmin middleware - crossSubDomainCookies gains config-time warnings for the two silent browser failures: a cookie domain that does not cover the baseUrl host, and localhost/loopback base URLs - README documents the full multi-app session recipe: shared cookie domain + shared BETTER_AUTH_SECRET + shared postgres:// AUTH_DATABASE_URL (ADR 0027)
1 parent 93838ca commit 07765ba

10 files changed

Lines changed: 273 additions & 41 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"everything-dev": patch
3+
---
4+
5+
auth: admin member management no longer depends on the admin's own organization membership (#876 follow-up)
6+
7+
`adminAddMember` forwarded the caller's headers into Better Auth's `addMember`, which checks the
8+
caller's **organization** permissions — so a platform admin who was not a member of the target
9+
organization (the whole point of an admin route) got FORBIDDEN. The mutation is now a direct write,
10+
like every other mutation in `AdminOrganizationsService`: it enforces the organization membership
11+
limit through the plugin's membership policy and refuses duplicate memberships. The integration
12+
suite now covers adding a member to an organization the admin does not belong to — the case the
13+
previous tests masked by creating every fixture organization owned by the admin.
14+
15+
Two smaller consistency fixes ride along:
16+
17+
- `adminUpdateMemberRole` refuses to demote an organization's **only owner** to a non-owner role,
18+
matching `adminRemoveMember`'s existing last-owner guard.
19+
- `getOrganizationForAdmin` guards with the standard `requireAdmin` middleware instead of a
20+
hand-rolled role check, matching the admin routes' pattern.
21+
22+
`crossSubDomainCookies` (#668) also gained config-time warnings for the two ways a shared cookie
23+
domain fails silently at the browser: a `domain` that does not cover the app's own `baseUrl` host
24+
(browsers drop the cookie outright, breaking sign-in) and localhost/loopback base URLs (browsers
25+
refuse `Domain` attributes there). The auth plugin README now documents the full multi-app recipe —
26+
shared cookie domain + shared `BETTER_AUTH_SECRET` + shared `postgres://` `AUTH_DATABASE_URL` —
27+
mirroring the ADR 0027 decomposition.

‎plugins/auth/README.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,15 @@ It runs inside the **everything-plugin** framework (oRPC + Effect) and is design
6565

6666
`onboard` hrefs must be app paths (`/…`) or `https://` URLs. `links.next`, `links.home` and `community.url` replace `{organizationId}` with the joined organization's ID. A failed or invalid `community.url` response shows nothing extra.
6767

68-
`crossSubDomainCookies` scopes the session cookie to a shared registrable domain so sibling subdomains (e.g. `app.example.com` and `api.example.com`) share one session. Every app on that domain must run the **same `BETTER_AUTH_SECRET`** — the cookie is sealed with it — and each app should list its siblings' origins in `trustedOrigins`. Absent or disabled config keeps cookies host-scoped.
68+
`crossSubDomainCookies` scopes the session cookie to a shared registrable domain so sibling subdomains (e.g. `app.example.com` and `api.example.com`) share one session. Absent or disabled config keeps cookies host-scoped.
69+
70+
**Multi-app session sharing** — a family of apps whose sessions roam a shared registrable domain needs all three legs together; the cookie alone is not enough:
71+
72+
1. **Shared cookie** — each app sets `crossSubDomainCookies: { enabled: true, domain: ".example.com" }` where `example.com` covers every app's host. A cookie whose `Domain` does not cover the app's own host is dropped by the browser, which breaks sign-in on that app (the boot warnings flag this, along with localhost hosts, which can never carry a `Domain` attribute).
73+
2. **Shared secret** — every app on the domain must run the **same `BETTER_AUTH_SECRET`**: the cookie is sealed with it, so a sibling that runs a different secret sees every incoming session as invalid and signs the user out.
74+
3. **Shared database** — every app's `AUTH_DATABASE_URL` must point at the **same `postgres://` database**. Auth tables live un-namespaced in that database (`namespace: false` — Better Auth's schema owns the public tables), so the shared cookie resolves the same session row everywhere. Without it, the cookie resolves to a session row the sibling cannot see and the request still reads as signed-out. The pglite default is per-identity and dev-only; cross-app session sharing is a `postgres://` setup.
75+
76+
Each app should also cross-list its siblings' origins in `trustedOrigins`. The everything.dev family recipe this configuration mirrors is documented in `docs/adr/0027-apps-decomposition-every-app.md`.
6977

7078
### Plugin Secrets
7179

‎plugins/auth/src/handlers/admin-organizations.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import type {
88
} from "../middleware";
99
import { AdminOrganizationsService } from "../services/admin-organizations";
1010
import { mapDomainError } from "../services/domain-errors";
11-
import { createHeaders } from "../utils";
1211

1312
export function createAdminOrganizationHandlers(
1413
builder: Implementer<ContractType, AuthHandlerContext>,
@@ -59,9 +58,9 @@ export function createAdminOrganizationHandlers(
5958
adminAddMember: builder.adminAddMember
6059
.use(requireAuth)
6160
.use(requireAdmin)
62-
.effect(function* ({ input, context }) {
61+
.effect(function* ({ input }) {
6362
const admin = yield* AdminOrganizationsService;
64-
return yield* mapDomainError(admin.addMember(createHeaders(context.reqHeaders), input));
63+
return yield* mapDomainError(admin.addMember(input));
6564
}),
6665

6766
adminRemoveMember: builder.adminRemoveMember

‎plugins/auth/src/handlers/organizations.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import type { Implementer } from "@orpc/server";
2-
import { Effect } from "effect";
2+
import { createAuthMiddleware } from "everything-dev/api";
33
import type { ContractType } from "../contract";
4-
import type { AuthHandlerContext, RequireAuthMiddleware } from "../middleware";
4+
import type {
5+
AuthHandlerAuthedContext,
6+
AuthHandlerContext,
7+
RequireAuthMiddleware,
8+
} from "../middleware";
59
import { mapDomainError } from "../services/domain-errors";
610
import { OrganizationsService } from "../services/organizations";
711
import { createHeaders } from "../utils";
@@ -10,6 +14,7 @@ export function createOrganizationHandlers(
1014
builder: Implementer<ContractType, AuthHandlerContext>,
1115
requireAuth: RequireAuthMiddleware,
1216
) {
17+
const { requireAdmin } = createAuthMiddleware<AuthHandlerAuthedContext>(builder);
1318
return {
1419
listOrganizations: builder.listOrganizations.use(requireAuth).effect(function* ({ context }) {
1520
const organizations = yield* OrganizationsService;
@@ -35,19 +40,13 @@ export function createOrganizationHandlers(
3540
);
3641
}),
3742

38-
getOrganizationForAdmin: builder.getOrganizationForAdmin.use(requireAuth).effect(function* ({
39-
input,
40-
context,
41-
errors,
42-
}) {
43-
const organizations = yield* OrganizationsService;
44-
if (context.user.role !== "admin") {
45-
return yield* Effect.fail(
46-
errors.FORBIDDEN({ message: "Admin access required", data: { action: "admin-access" } }),
47-
);
48-
}
49-
return yield* mapDomainError(organizations.getOrganizationById(input.organizationId));
50-
}),
43+
getOrganizationForAdmin: builder.getOrganizationForAdmin
44+
.use(requireAuth)
45+
.use(requireAdmin)
46+
.effect(function* ({ input }) {
47+
const organizations = yield* OrganizationsService;
48+
return yield* mapDomainError(organizations.getOrganizationById(input.organizationId));
49+
}),
5150

5251
createOrganization: builder.createOrganization.use(requireAuth).effect(function* ({
5352
input,

‎plugins/auth/src/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { OrphanSweepLive } from "./orphan-sweep";
2929
import { AuthServicesTag } from "./service-types";
3030
import { buildAuthServicesLayer } from "./service-wiring";
3131
import { toError } from "./utils";
32-
import { socialProviderWarnings } from "./warnings";
32+
import { cookieDomainWarnings, socialProviderWarnings } from "./warnings";
3333

3434
export default createPlugin.withPlugins<PluginsClient>()({
3535
variables: authVariablesSchema,
@@ -44,7 +44,7 @@ export default createPlugin.withPlugins<PluginsClient>()({
4444

4545
servicesTag: AuthServicesTag,
4646

47-
warnings: socialProviderWarnings,
47+
warnings: (config) => [...socialProviderWarnings(config), ...cookieDomainWarnings(config)],
4848

4949
initialize: (config) =>
5050
Effect.gen(function* () {

‎plugins/auth/src/service-wiring.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export function buildAuthServicesLayer(services: PluginServices) {
8989
buildAdminOrganizationsService({
9090
auth: services.auth,
9191
db: services.db,
92+
membershipPolicy: services.membershipPolicy,
9293
registry: services.permissionRegistry,
9394
}),
9495
),

‎plugins/auth/src/services/admin-organizations.ts‎

Lines changed: 97 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Context, Effect } from "effect";
33
import { attemptAuth, attemptDb } from "../attempts";
44
import type { InferInput, InferOutput } from "../contract";
55
import * as schema from "../db/schema";
6+
import type { OrganizationMembershipPolicy } from "../organization-membership-policy";
67
import { toDate, toInvitation, toOrganizationInfo, toTeam } from "../serializers";
78
import type { PermissionRegistry, PluginServices } from "../service-types";
89
import { parseMemberRoles } from "../utils";
@@ -42,7 +43,6 @@ export class AdminOrganizationsService extends Context.Service<
4243
organizationId: string,
4344
) => Effect.Effect<{ success: boolean }, AuthFailure>;
4445
addMember: (
45-
headers: Headers,
4646
input: InferInput<"adminAddMember">,
4747
) => Effect.Effect<InferOutput<"adminAddMember">, AuthFailure>;
4848
removeMember: (
@@ -70,10 +70,12 @@ export class AdminOrganizationsService extends Context.Service<
7070
export const buildAdminOrganizationsService = ({
7171
auth,
7272
db,
73+
membershipPolicy,
7374
registry,
7475
}: {
7576
auth: Auth;
7677
db: Db;
78+
membershipPolicy: OrganizationMembershipPolicy;
7779
registry: PermissionRegistry;
7880
}) => {
7981
const notFound = (organizationId: string) =>
@@ -357,25 +359,82 @@ export const buildAdminOrganizationsService = ({
357359
return { success: true };
358360
}),
359361

360-
addMember: (headers, input) =>
361-
attemptAuth(() =>
362-
auth.api.addMember({
363-
headers,
364-
body: {
362+
addMember: Effect.fn("AdminOrganizationsService.addMember")(function* (
363+
input: InferInput<"adminAddMember">,
364+
) {
365+
const organization = yield* attemptDb(() =>
366+
db.query.organization.findFirst({
367+
where: eq(schema.organization.id, input.organizationId),
368+
}),
369+
);
370+
if (!organization) {
371+
return yield* notFound(input.organizationId);
372+
}
373+
374+
const user = yield* attemptDb(() =>
375+
db.query.user.findFirst({ where: eq(schema.user.id, input.userId) }),
376+
);
377+
if (!user) {
378+
return yield* new UserNotFound({
379+
message: `User not found: ${input.userId}`,
380+
userId: input.userId,
381+
});
382+
}
383+
384+
const existing = yield* attemptDb(() =>
385+
db.query.member.findFirst({
386+
where: and(
387+
eq(schema.member.organizationId, input.organizationId),
388+
eq(schema.member.userId, input.userId),
389+
),
390+
}),
391+
);
392+
if (existing) {
393+
return yield* new AuthApiError({
394+
code: "BAD_REQUEST",
395+
data: {},
396+
message: "User is already a member of this organization",
397+
});
398+
}
399+
400+
const [{ memberCount } = { memberCount: 0 }] = yield* attemptDb(() =>
401+
db
402+
.select({ memberCount: sql<number>`count(*)::int` })
403+
.from(schema.member)
404+
.where(eq(schema.member.organizationId, input.organizationId)),
405+
);
406+
if (!membershipPolicy.hasCapacity(memberCount)) {
407+
return yield* new AuthApiError({
408+
code: "BAD_REQUEST",
409+
data: {},
410+
message: "This organization is full, so no more members can join",
411+
});
412+
}
413+
414+
const now = new Date();
415+
const [member] = yield* attemptDb(() =>
416+
db
417+
.insert(schema.member)
418+
.values({
419+
id: crypto.randomUUID(),
420+
organizationId: input.organizationId,
365421
userId: input.userId,
366422
role: input.role,
367-
organizationId: input.organizationId,
368-
},
369-
}),
370-
).pipe(
371-
Effect.map((member: any) => ({
372-
id: member.id,
373-
userId: member.userId,
374-
organizationId: member.organizationId,
375-
role: member.role,
376-
createdAt: toDate(member.createdAt),
377-
})),
378-
),
423+
createdAt: now,
424+
})
425+
.returning(),
426+
);
427+
if (!member) {
428+
return yield* new MemberNotFound({ message: "Member creation failed" });
429+
}
430+
return {
431+
id: member.id,
432+
userId: member.userId,
433+
organizationId: member.organizationId,
434+
role: member.role,
435+
createdAt: toDate(member.createdAt),
436+
};
437+
}),
379438

380439
removeMember: Effect.fn("AdminOrganizationsService.removeMember")(function* (
381440
input: InferInput<"adminRemoveMember">,
@@ -478,6 +537,26 @@ export const buildAdminOrganizationsService = ({
478537
return yield* new MemberNotFound({ message: "Member not found" });
479538
}
480539

540+
const demotingOnlyOwner =
541+
parseMemberRoles(member.role).includes("owner") &&
542+
!parseMemberRoles(input.role).includes("owner");
543+
if (demotingOnlyOwner) {
544+
const members = yield* attemptDb(() =>
545+
db.query.member.findMany({
546+
where: eq(schema.member.organizationId, input.organizationId),
547+
columns: { role: true },
548+
}),
549+
);
550+
const owners = members.filter((row) => parseMemberRoles(row.role).includes("owner"));
551+
if (owners.length <= 1) {
552+
return yield* new AuthApiError({
553+
code: "BAD_REQUEST",
554+
data: {},
555+
message: "Cannot demote the only owner of an organization",
556+
});
557+
}
558+
}
559+
481560
const [updated] = yield* attemptDb(() =>
482561
db
483562
.update(schema.member)

‎plugins/auth/src/warnings.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,55 @@ export function socialProviderWarnings(config: {
5858
}),
5959
].filter((warning): warning is PluginWarningInfo => warning !== null);
6060
}
61+
62+
function authBaseUrlHost(baseUrl: string | undefined): string | null {
63+
if (!baseUrl) return null;
64+
const candidate = /^https?:\/\//i.test(baseUrl) ? baseUrl : `https://${baseUrl}`;
65+
try {
66+
return new URL(candidate).hostname.toLowerCase();
67+
} catch {
68+
return null;
69+
}
70+
}
71+
72+
function isLoopbackHost(host: string): boolean {
73+
return (
74+
host === "localhost" || host === "127.0.0.1" || host === "::1" || host.endsWith(".localhost")
75+
);
76+
}
77+
78+
/**
79+
* crossSubDomainCookies (#668) fails silently at the browser: a Set-Cookie
80+
* whose Domain does not cover the app's own host is dropped outright (sign-in
81+
* stops working), and localhost hosts can never carry a Domain attribute.
82+
* Both misconfigurations are caught at config time here instead.
83+
*/
84+
export function cookieDomainWarnings(config: {
85+
variables: AuthPluginVariables;
86+
}): PluginWarningInfo[] {
87+
const cookies = config.variables.crossSubDomainCookies;
88+
if (!cookies?.enabled) return [];
89+
const host = authBaseUrlHost(config.variables.baseUrl);
90+
if (!host) return [];
91+
const warnings: PluginWarningInfo[] = [];
92+
if (isLoopbackHost(host)) {
93+
warnings.push({
94+
pluginKey: "auth",
95+
message:
96+
"crossSubDomainCookies is enabled but the auth baseUrl is a localhost host — browsers refuse Domain cookies for local hosts, so sessions stay host-scoped in this environment",
97+
hint: "Exercise cross-domain session sharing on a staging deployment whose hosts share the configured registrable domain instead",
98+
});
99+
}
100+
if (cookies.domain) {
101+
const domain = cookies.domain.toLowerCase().replace(/^\./, "");
102+
const coversHost = host === domain || host.endsWith(`.${domain}`);
103+
if (!coversHost) {
104+
warnings.push({
105+
pluginKey: "auth",
106+
message: `crossSubDomainCookies.domain "${cookies.domain}" does not cover the auth baseUrl host "${host}" — browsers drop cookies scoped to a foreign domain, which breaks sign-in on this app`,
107+
hint: `Use a dot-prefixed registrable domain that is a suffix of "${host}" (e.g. ".${host.split(".").slice(-2).join(".")}")`,
108+
});
109+
}
110+
}
111+
return warnings;
112+
}

0 commit comments

Comments
 (0)