Skip to content

Commit 84b1cf4

Browse files
To revert: in this commit, I added a pretty elaborate system of automatically signing out accounts when oauth scopes change. The system is pretty fragile, so I'm going to take the safer approach and just add document this as a known issue.
1 parent 9a3ac61 commit 84b1cf4

6 files changed

Lines changed: 114 additions & 31 deletions

File tree

packages/backend/src/gitlab.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@ export const getProjectsForAuthenticatedUser = async (visibility: 'private' | 'i
324324
export const getOAuthScopesForAuthenticatedUser = async (api: InstanceType<typeof Gitlab>) => {
325325
try {
326326
const response = await api.requester.get('/oauth/token/info');
327-
console.log('response', response);
328327
if (
329328
response &&
330329
typeof response.body === 'object' &&

packages/web/src/app/components/authMethodSelector.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ export const AuthMethodSelector = ({
2929
// Call the optional analytics callback first
3030
onProviderClick?.(provider);
3131

32-
// @nocheckin
3332
signIn(
3433
provider,
3534
{
@@ -40,7 +39,6 @@ export const AuthMethodSelector = ({
4039
// @see: https://next-auth.js.org/getting-started/client#additional-parameters
4140
{
4241
prompt: 'consent',
43-
scope: 'read:user user:email repo'
4442
}
4543
);
4644
}, [callbackUrl, onProviderClick]);

packages/web/src/auth.ts

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { onCreateUser } from '@/lib/authUtils';
1919
import { getAuditService } from '@/ee/features/audit/factory';
2020
import { SINGLE_TENANT_ORG_ID } from './lib/constants';
2121
import { refreshLinkedAccountTokens } from '@/ee/features/permissionSyncing/tokenRefresh';
22+
import { getRequiredScopes, normalizeScopes } from './lib/oauthScopes';
2223

2324
const auditService = getAuditService();
2425
const eeIdentityProviders = hasEntitlement("sso") ? await getEEIdentityProviders() : [];
@@ -52,6 +53,8 @@ declare module 'next-auth' {
5253
declare module 'next-auth/jwt' {
5354
interface JWT {
5455
userId: string;
56+
providerAccountId: string;
57+
provider: string;
5558
linkedAccountTokens?: LinkedAccountTokensMap;
5659
}
5760
}
@@ -164,7 +167,7 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
164167
// Explicitly update the Account record with the OAuth token details.
165168
// This is necessary to update the access token when the user
166169
// re-authenticates.
167-
if (account && account.provider && account.providerAccountId) {
170+
if (account && account.provider && account.provider !== 'credentials' && account.providerAccountId) {
168171
await prisma.account.update({
169172
where: {
170173
provider_providerAccountId: {
@@ -217,14 +220,60 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
217220
}
218221
},
219222
callbacks: {
220-
async jwt({ token, user: _user, account }) {
221-
const user = _user as User | undefined;
223+
async jwt({ token, user: _user, account: _account }) {
222224
// @note: `user` will be available on signUp or signIn triggers.
223225
// Cache the userId in the JWT for later use.
224-
if (user) {
226+
if (_user) {
227+
const user = _user as User;
225228
token.userId = user.id;
226229
}
227230

231+
if (_account) {
232+
token.providerAccountId = _account.providerAccountId;
233+
token.provider = _account.provider;
234+
}
235+
236+
const account = await prisma.account.findUnique({
237+
where: {
238+
provider_providerAccountId: {
239+
provider: token.provider,
240+
providerAccountId: token.providerAccountId,
241+
},
242+
},
243+
});
244+
245+
if (!account) {
246+
return null;
247+
}
248+
249+
const currentScopes = normalizeScopes(account.scope);
250+
const requiredScopes = getRequiredScopes(account.provider);
251+
252+
if (currentScopes !== requiredScopes) {
253+
const doesAccountExist = await prisma.account.findUnique({
254+
where: {
255+
provider_providerAccountId: {
256+
provider: account.provider,
257+
providerAccountId: account.providerAccountId,
258+
},
259+
},
260+
});
261+
262+
if (doesAccountExist) {
263+
await prisma.account.delete({
264+
where: {
265+
provider_providerAccountId: {
266+
provider: account.provider,
267+
providerAccountId: account.providerAccountId,
268+
},
269+
},
270+
});
271+
console.log(`Deleted account ${account.providerAccountId} for provider ${account.provider} because it did not have the required scopes.`);
272+
}
273+
274+
return null;
275+
}
276+
228277
if (hasEntitlement('permission-syncing')) {
229278
if (account && account.access_token && account.refresh_token && account.expires_at) {
230279
token.linkedAccountTokens = token.linkedAccountTokens || {};
@@ -273,4 +322,4 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
273322
// We set redirect to false in signInOptions so we can pass the email is as a param
274323
// verifyRequest: "/login/verify",
275324
}
276-
});
325+
});

packages/web/src/ee/features/permissionSyncing/components/linkButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ interface LinkButtonProps {
1212
export const LinkButton = ({ provider, callbackUrl }: LinkButtonProps) => {
1313
const handleLink = () => {
1414
signIn(provider, {
15-
redirectTo: callbackUrl
15+
redirectTo: callbackUrl,
1616
});
1717
};
1818

packages/web/src/ee/features/sso/sso.ts

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import type { IdentityProvider } from "@/auth";
22
import { onCreateUser } from "@/lib/authUtils";
3+
import { getRequiredScopes } from "@/lib/oauthScopes";
34
import { prisma } from "@/prisma";
45
import { AuthentikIdentityProviderConfig, GCPIAPIdentityProviderConfig, GitHubIdentityProviderConfig, GitLabIdentityProviderConfig, GoogleIdentityProviderConfig, KeycloakIdentityProviderConfig, MicrosoftEntraIDIdentityProviderConfig, OktaIdentityProviderConfig } from "@sourcebot/schemas/v3/index.type";
5-
import { createLogger, env, getTokenFromConfig, hasEntitlement, loadConfig } from "@sourcebot/shared";
6+
import { createLogger, env, getTokenFromConfig, loadConfig } from "@sourcebot/shared";
67
import { OAuth2Client } from "google-auth-library";
78
import type { User as AuthJsUser } from "next-auth";
89
import type { Provider } from "next-auth/providers";
@@ -126,19 +127,10 @@ const createGitHubProvider = (clientId: string, clientSecret: string, baseUrl?:
126127
...(hostname === GITHUB_CLOUD_HOSTNAME ? { enterprise: { baseUrl: baseUrl } } : {}), // if this is set the provider expects GHE so we need this check
127128
authorization: {
128129
params: {
129-
scope: [
130-
'read:user',
131-
'user:email',
132-
// Permission syncing requires the `repo` scope in order to fetch repositories
133-
// for the authenticated user.
134-
// @see: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repositories-for-the-authenticated-user
135-
...(env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing') ?
136-
['repo'] :
137-
[]
138-
),
139-
].join(' '),
130+
scope: getRequiredScopes('github'),
140131
},
141132
},
133+
allowDangerousEmailAccountLinking: true,
142134
});
143135
}
144136

@@ -150,16 +142,7 @@ const createGitLabProvider = (clientId: string, clientSecret: string, baseUrl?:
150142
authorization: {
151143
url: `${url}/oauth/authorize`,
152144
params: {
153-
scope: [
154-
"read_user",
155-
// Permission syncing requires the `read_api` scope in order to fetch projects
156-
// for the authenticated user and project members.
157-
// @see: https://docs.gitlab.com/ee/api/projects.html#list-all-projects
158-
...(env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing') ?
159-
['read_api'] :
160-
[]
161-
),
162-
].join(' '),
145+
scope: getRequiredScopes('gitlab'),
163146
},
164147
},
165148
token: {
@@ -168,13 +151,15 @@ const createGitLabProvider = (clientId: string, clientSecret: string, baseUrl?:
168151
userinfo: {
169152
url: `${url}/api/v4/user`,
170153
},
154+
allowDangerousEmailAccountLinking: true,
171155
});
172156
}
173157

174158
const createGoogleProvider = (clientId: string, clientSecret: string): Provider => {
175159
return Google({
176160
clientId: clientId,
177161
clientSecret: clientSecret,
162+
allowDangerousEmailAccountLinking: true,
178163
});
179164
}
180165

@@ -183,6 +168,7 @@ const createOktaProvider = (clientId: string, clientSecret: string, issuer: stri
183168
clientId: clientId,
184169
clientSecret: clientSecret,
185170
issuer: issuer,
171+
allowDangerousEmailAccountLinking: true,
186172
});
187173
}
188174

@@ -191,6 +177,7 @@ const createKeycloakProvider = (clientId: string, clientSecret: string, issuer:
191177
clientId: clientId,
192178
clientSecret: clientSecret,
193179
issuer: issuer,
180+
allowDangerousEmailAccountLinking: true,
194181
});
195182
}
196183

@@ -199,6 +186,7 @@ const createMicrosoftEntraIDProvider = (clientId: string, clientSecret: string,
199186
clientId: clientId,
200187
clientSecret: clientSecret,
201188
issuer: issuer,
189+
allowDangerousEmailAccountLinking: true,
202190
});
203191
}
204192

@@ -283,5 +271,6 @@ export const createAuthentikProvider = (clientId: string, clientSecret: string,
283271
clientId: clientId,
284272
clientSecret: clientSecret,
285273
issuer: issuer,
274+
allowDangerousEmailAccountLinking: true,
286275
});
287276
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { env, hasEntitlement } from "@sourcebot/shared";
2+
3+
/**
4+
* Normalize scope strings for consistent comparison.
5+
* Handles different delimiters and ordering.
6+
*/
7+
export function normalizeScopes(scopeString: string | null | undefined): string {
8+
if (!scopeString) return '';
9+
return scopeString.split(/[\s,]+/).filter(Boolean).sort().join(' ');
10+
}
11+
12+
/**
13+
* Calculate the required OAuth scopes for a given provider based on current configuration.
14+
* Returns a normalized, sorted scope string.
15+
*/
16+
export function getRequiredScopes(provider: string): string {
17+
const scopes: string[] = [];
18+
19+
switch (provider) {
20+
case 'github':
21+
scopes.push('read:user', 'user:email');
22+
// Permission syncing requires the `repo` scope in order to fetch repositories
23+
// for the authenticated user.
24+
// @see: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repositories-for-the-authenticated-user
25+
if (env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing')) {
26+
scopes.push('repo');
27+
}
28+
29+
break;
30+
31+
case 'gitlab':
32+
scopes.push('read_user');
33+
// Permission syncing requires the `read_api` scope in order to fetch projects
34+
// for the authenticated user and project members.
35+
// @see: https://docs.gitlab.com/ee/api/projects.html#list-all-projects
36+
if (env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing')) {
37+
scopes.push('read_api');
38+
}
39+
break;
40+
41+
default:
42+
// Other providers (Google, Okta, etc.) don't have dynamic scope requirements
43+
return '';
44+
}
45+
46+
return normalizeScopes(scopes.join(' '));
47+
}
48+

0 commit comments

Comments
 (0)