Skip to content

Commit b307d55

Browse files
feat(auth): Google-only sign-in; atomic invite redemption
Password and GitHub sign-in are switched off while a Workspace allow-list (GOOGLE_ALLOWED_DOMAIN) is configured; everyone allowed in has Google. - email sign-in, sign-up and password reset are refused server-side and hidden on the login and invite pages; ALLOW_PASSWORD_LOGIN=true is a break-glass that restores password sign-in for existing users only - password sign-up stays off even with the break-glass: it trusts a typed email, so whoever holds an invite link could create the invitee's account with their own password and Google would later link into it - GitHub is refused when starting sign-in and in the callback, since a GitHub email stays verified after someone leaves the company - installs without an allow-list (local dev, self-hosting) keep passwords - auth.providers tells the UI which methods to offer Invite redemption now runs in one transaction (claim invite, upsert membership, create grants). A failed grant insert previously left a project-limited invitee as a member with no grants, i.e. access to every project, and a retry treated them as already unrestricted. The invite is claimed first so concurrent redemptions cannot both grant. Verified on dev Postgres by forcing a foreign-key failure: nothing was left behind and the invite stayed redeemable.
1 parent 8cb7306 commit b307d55

8 files changed

Lines changed: 282 additions & 81 deletions

File tree

‎apps/api/src/controllers/oauth-callback.controller.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
github,
1111
google,
1212
isEmailDomainAllowed,
13+
isGithubLoginEnabled,
1314
parseGoogleIdentity,
1415
setSessionTokenCookie,
1516
} from '@openpanel/auth';
@@ -430,7 +431,8 @@ export async function githubCallback(req: FastifyRequest, reply: FastifyReply) {
430431
const inviteId = req.cookies.inviteId;
431432
const tokens = await github.validateAuthorizationCode(code);
432433
const githubUser = await fetchGithubUser(tokens.accessToken());
433-
if (!isEmailDomainAllowed(githubUser.email)) {
434+
// Belt and braces: signInOAuth already refuses GitHub with an allow-list
435+
if (!isGithubLoginEnabled() || !isEmailDomainAllowed(githubUser.email)) {
434436
throw new LogError(
435437
`Sign in with your ${describeAllowedDomains()} Google account`,
436438
);

‎apps/start/src/routes/_login.login.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { SignInEmailForm } from '@/components/auth/sign-in-email-form';
33
import { SignInGoogle } from '@/components/auth/sign-in-google';
44
import { LogoSquare } from '@/components/logo';
55
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
6+
import { useTRPC } from '@/integrations/trpc/react';
67
import { PAGE_TITLES, createTitle } from '@/utils/title';
8+
import { useQuery } from '@tanstack/react-query';
79
import { createFileRoute, redirect } from '@tanstack/react-router';
810
import { AlertCircle } from 'lucide-react';
911
import { z } from 'zod';
@@ -25,6 +27,8 @@ export const Route = createFileRoute('/_login/login')({
2527

2628
function LoginPage() {
2729
const { error, correlationId, inviteId } = Route.useSearch();
30+
const trpc = useTRPC();
31+
const { data: providers } = useQuery(trpc.auth.providers.queryOptions());
2832

2933
return (
3034
<div className="col gap-8 w-full text-left">
@@ -59,8 +63,12 @@ function LoginPage() {
5963
<div className="space-y-4">
6064
<SignInGoogle type="sign-in" inviteId={inviteId} />
6165
</div>
62-
<Or />
63-
<SignInEmailForm inviteId={inviteId} />
66+
{providers?.password && (
67+
<>
68+
<Or />
69+
<SignInEmailForm inviteId={inviteId} />
70+
</>
71+
)}
6472
</div>
6573
);
6674
}

‎apps/start/src/routes/_public.onboarding.tsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export const Route = createFileRoute('/_public/onboarding')({
4646
function Component() {
4747
const { inviteId } = Route.useSearch();
4848
const trpc = useTRPC();
49+
const { data: providers } = useQuery(trpc.auth.providers.queryOptions());
4950
const { data: invite } = useQuery(
5051
trpc.organization.getInvite.queryOptions(
5152
{
@@ -105,13 +106,17 @@ function Component() {
105106
<div className="space-y-6">
106107
<SignInGoogle type="sign-up" inviteId={inviteId} />
107108

108-
<Or className="my-6" />
109+
{providers?.passwordSignUp && (
110+
<>
111+
<Or className="my-6" />
109112

110-
<div className="flex items-center gap-2 font-semibold mb-4 text-lg">
111-
<MailIcon className="size-4" />
112-
Or create a password
113-
</div>
114-
<SignUpEmailForm inviteId={inviteId} />
113+
<div className="flex items-center gap-2 font-semibold mb-4 text-lg">
114+
<MailIcon className="size-4" />
115+
Or create a password
116+
</div>
117+
<SignUpEmailForm inviteId={inviteId} />
118+
</>
119+
)}
115120
</div>
116121
)}
117122
</div>

‎packages/auth/src/domain-policy.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import {
33
DomainPolicyError,
44
getEmailDomain,
55
isEmailDomainAllowed,
6+
isGithubLoginEnabled,
7+
isPasswordLoginEnabled,
8+
isPasswordSignUpEnabled,
69
parseAllowedDomains,
710
parseGoogleIdentity,
811
} from './domain-policy';
@@ -132,3 +135,64 @@ describe('parseGoogleIdentity', () => {
132135
).toBe('x@gmail.com');
133136
});
134137
});
138+
139+
describe('isPasswordLoginEnabled', () => {
140+
it('is off when a Workspace allow-list is configured', () => {
141+
expect(
142+
isPasswordLoginEnabled({ GOOGLE_ALLOWED_DOMAIN: 'dashverse.ai' }),
143+
).toBe(false);
144+
});
145+
146+
it('can be switched back on as a break-glass', () => {
147+
expect(
148+
isPasswordLoginEnabled({
149+
GOOGLE_ALLOWED_DOMAIN: 'dashverse.ai',
150+
ALLOW_PASSWORD_LOGIN: 'true',
151+
}),
152+
).toBe(true);
153+
});
154+
155+
it('stays on without an allow-list (local dev, self-hosting)', () => {
156+
expect(isPasswordLoginEnabled({})).toBe(true);
157+
});
158+
159+
it('can be switched off explicitly without an allow-list', () => {
160+
expect(isPasswordLoginEnabled({ ALLOW_PASSWORD_LOGIN: 'false' })).toBe(
161+
false,
162+
);
163+
});
164+
});
165+
166+
describe('isPasswordSignUpEnabled', () => {
167+
it('is off with an allow-list, even with the sign-in break-glass on', () => {
168+
expect(
169+
isPasswordSignUpEnabled({
170+
GOOGLE_ALLOWED_DOMAIN: 'dashverse.ai',
171+
ALLOW_PASSWORD_LOGIN: 'true',
172+
}),
173+
).toBe(false);
174+
});
175+
176+
it('follows password sign-in without an allow-list', () => {
177+
expect(isPasswordSignUpEnabled({})).toBe(true);
178+
expect(isPasswordSignUpEnabled({ ALLOW_PASSWORD_LOGIN: 'false' })).toBe(
179+
false,
180+
);
181+
});
182+
});
183+
184+
describe('isGithubLoginEnabled', () => {
185+
it('is off with an allow-list even when GitHub is configured', () => {
186+
expect(
187+
isGithubLoginEnabled({
188+
GOOGLE_ALLOWED_DOMAIN: 'dashverse.ai',
189+
GITHUB_CLIENT_ID: 'gh',
190+
}),
191+
).toBe(false);
192+
});
193+
194+
it('needs a GitHub client id without an allow-list', () => {
195+
expect(isGithubLoginEnabled({})).toBe(false);
196+
expect(isGithubLoginEnabled({ GITHUB_CLIENT_ID: 'gh' })).toBe(true);
197+
});
198+
});

‎packages/auth/src/domain-policy.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,47 @@ export function parseGoogleIdentity(
106106
lastName: typeof data.family_name === 'string' ? data.family_name : '',
107107
};
108108
}
109+
110+
/**
111+
* Whether email/password sign-in, sign-up and reset are available.
112+
*
113+
* With a Google Workspace allow-list configured everyone allowed in has a
114+
* Google account, so passwords are off: one less way in to guess at.
115+
* ALLOW_PASSWORD_LOGIN=true turns them back on (break-glass if Google sign-in
116+
* is unavailable); without an allow-list (local dev, self-hosting) they stay
117+
* on so an install without Google OAuth still works.
118+
*/
119+
export function isPasswordLoginEnabled(env: Env = process.env as Env): boolean {
120+
if (
121+
env.ALLOW_PASSWORD_LOGIN !== undefined &&
122+
env.ALLOW_PASSWORD_LOGIN !== ''
123+
) {
124+
return env.ALLOW_PASSWORD_LOGIN === 'true';
125+
}
126+
return getAllowedEmailDomains(env).length === 0;
127+
}
128+
129+
/**
130+
* Whether new accounts may be created with a password.
131+
*
132+
* Never with a Workspace allow-list: a password sign-up trusts a typed email,
133+
* so whoever holds an invite link could create the invitee's account with
134+
* their own password, and Google would later link into it. The
135+
* ALLOW_PASSWORD_LOGIN break-glass only restores sign-in for existing users.
136+
*/
137+
export function isPasswordSignUpEnabled(
138+
env: Env = process.env as Env,
139+
): boolean {
140+
return (
141+
getAllowedEmailDomains(env).length === 0 && isPasswordLoginEnabled(env)
142+
);
143+
}
144+
145+
/**
146+
* GitHub sign-in proves nothing about Workspace membership, and a GitHub
147+
* email stays verified after someone leaves the company, so it is unavailable
148+
* whenever a Workspace allow-list is configured.
149+
*/
150+
export function isGithubLoginEnabled(env: Env = process.env as Env): boolean {
151+
return getAllowedEmailDomains(env).length === 0 && !!env.GITHUB_CLIENT_ID;
152+
}

‎packages/db/src/services/organization.invite.test.ts‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,34 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
33
const mocks = vi.hoisted(() => ({
44
inviteFindUnique: vi.fn(),
55
inviteFindMany: vi.fn(),
6-
inviteDelete: vi.fn(),
6+
inviteDeleteMany: vi.fn(),
77
memberFindUnique: vi.fn(),
88
memberUpsert: vi.fn(),
99
projectAccessFindMany: vi.fn(),
10-
projectAccessCreate: vi.fn(),
10+
projectAccessCreateMany: vi.fn(),
11+
transaction: vi.fn(),
1112
}));
1213

1314
vi.mock('../prisma-client', () => {
1415
const client = {
1516
invite: {
1617
findUnique: mocks.inviteFindUnique,
1718
findMany: mocks.inviteFindMany,
18-
delete: mocks.inviteDelete,
19+
deleteMany: mocks.inviteDeleteMany,
1920
},
2021
member: { findUnique: mocks.memberFindUnique, upsert: mocks.memberUpsert },
2122
projectAccess: {
2223
findMany: mocks.projectAccessFindMany,
23-
create: mocks.projectAccessCreate,
24+
createMany: mocks.projectAccessCreateMany,
2425
},
2526
};
26-
return { db: { ...client, $primary: () => client } };
27+
// Interactive transaction: run the callback against the same client
28+
mocks.transaction.mockImplementation((fn: (tx: unknown) => unknown) =>
29+
fn(client),
30+
);
31+
return {
32+
db: { ...client, $primary: () => client, $transaction: mocks.transaction },
33+
};
2734
});
2835

2936
vi.mock('./access.service', () => ({
@@ -54,6 +61,8 @@ beforeEach(() => {
5461
mocks.memberFindUnique.mockResolvedValue(null);
5562
mocks.memberUpsert.mockResolvedValue({ id: 'm1' });
5663
mocks.projectAccessFindMany.mockResolvedValue([]);
64+
mocks.inviteDeleteMany.mockResolvedValue({ count: 1 });
65+
mocks.projectAccessCreateMany.mockResolvedValue({ count: 0 });
5766
});
5867

5968
describe('connectUserToOrganization', () => {
@@ -70,7 +79,10 @@ describe('connectUserToOrganization', () => {
7079
}),
7180
}),
7281
);
73-
expect(mocks.inviteDelete).toHaveBeenCalledWith({ where: { id: 'inv1' } });
82+
expect(mocks.inviteDeleteMany).toHaveBeenCalledWith({
83+
where: { id: 'inv1' },
84+
});
85+
expect(mocks.transaction).toHaveBeenCalledTimes(1);
7486
});
7587

7688
it('refuses an invite addressed to someone else', async () => {
@@ -81,7 +93,7 @@ describe('connectUserToOrganization', () => {
8193
connectUserToOrganization({ user, inviteId: 'inv1' }),
8294
).rejects.toThrow('different email address');
8395
expect(mocks.memberUpsert).not.toHaveBeenCalled();
84-
expect(mocks.inviteDelete).not.toHaveBeenCalled();
96+
expect(mocks.inviteDeleteMany).not.toHaveBeenCalled();
8597
});
8698

8799
it('refuses an expired invite', async () => {
@@ -98,21 +110,46 @@ describe('connectUserToOrganization', () => {
98110
invite({ projectAccess: ['frameo', 'studio'] }),
99111
);
100112
await connectUserToOrganization({ user, inviteId: 'inv1' });
101-
expect(mocks.projectAccessCreate).toHaveBeenCalledTimes(2);
102-
expect(mocks.projectAccessCreate).toHaveBeenCalledWith({
103-
data: expect.objectContaining({ projectId: 'frameo', level: 'write' }),
113+
expect(mocks.projectAccessCreateMany).toHaveBeenCalledWith({
114+
data: [
115+
expect.objectContaining({ projectId: 'frameo', level: 'write' }),
116+
expect.objectContaining({ projectId: 'studio', level: 'write' }),
117+
],
104118
});
105119
});
106120

121+
it('fails as a whole (inside the transaction) if a grant insert fails', async () => {
122+
mocks.inviteFindUnique.mockResolvedValue(
123+
invite({ projectAccess: ['frameo'] }),
124+
);
125+
mocks.projectAccessCreateMany.mockRejectedValue(new Error('db down'));
126+
await expect(
127+
connectUserToOrganization({ user, inviteId: 'inv1' }),
128+
).rejects.toThrow('db down');
129+
// Membership, invite claim and grants all ran in the one transaction, so
130+
// Postgres rolls back all of them and the invite can be redeemed again.
131+
expect(mocks.transaction).toHaveBeenCalledTimes(1);
132+
expect(mocks.memberUpsert).toHaveBeenCalledTimes(1);
133+
});
134+
135+
it('refuses an invite that was already used concurrently', async () => {
136+
mocks.inviteFindUnique.mockResolvedValue(invite());
137+
mocks.inviteDeleteMany.mockResolvedValue({ count: 0 });
138+
await expect(
139+
connectUserToOrganization({ user, inviteId: 'inv1' }),
140+
).rejects.toThrow('Invite already used');
141+
expect(mocks.memberUpsert).not.toHaveBeenCalled();
142+
});
143+
107144
it('never narrows an existing member who already sees every project', async () => {
108145
mocks.memberFindUnique.mockResolvedValue({ id: 'm1' });
109146
mocks.projectAccessFindMany.mockResolvedValue([]);
110147
mocks.inviteFindUnique.mockResolvedValue(
111148
invite({ projectAccess: ['frameo'] }),
112149
);
113150
await connectUserToOrganization({ user, inviteId: 'inv1' });
114-
expect(mocks.projectAccessCreate).not.toHaveBeenCalled();
115-
expect(mocks.inviteDelete).toHaveBeenCalled();
151+
expect(mocks.projectAccessCreateMany).not.toHaveBeenCalled();
152+
expect(mocks.inviteDeleteMany).toHaveBeenCalled();
116153
});
117154

118155
it('adds only missing grants to an existing restricted member', async () => {
@@ -122,9 +159,8 @@ describe('connectUserToOrganization', () => {
122159
invite({ projectAccess: ['frameo', 'studio'] }),
123160
);
124161
await connectUserToOrganization({ user, inviteId: 'inv1' });
125-
expect(mocks.projectAccessCreate).toHaveBeenCalledTimes(1);
126-
expect(mocks.projectAccessCreate).toHaveBeenCalledWith({
127-
data: expect.objectContaining({ projectId: 'studio' }),
162+
expect(mocks.projectAccessCreateMany).toHaveBeenCalledWith({
163+
data: [expect.objectContaining({ projectId: 'studio' })],
128164
});
129165
});
130166
});

0 commit comments

Comments
 (0)