Skip to content

Commit e085ddc

Browse files
Revert "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."
This reverts commit 84b1cf4.
1 parent 84b1cf4 commit e085ddc

6 files changed

Lines changed: 31 additions & 114 deletions

File tree

packages/backend/src/gitlab.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ 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);
327328
if (
328329
response &&
329330
typeof response.body === 'object' &&

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

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

32+
// @nocheckin
3233
signIn(
3334
provider,
3435
{
@@ -39,6 +40,7 @@ export const AuthMethodSelector = ({
3940
// @see: https://next-auth.js.org/getting-started/client#additional-parameters
4041
{
4142
prompt: 'consent',
43+
scope: 'read:user user:email repo'
4244
}
4345
);
4446
}, [callbackUrl, onProviderClick]);

packages/web/src/auth.ts

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ 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';
2322

2423
const auditService = getAuditService();
2524
const eeIdentityProviders = hasEntitlement("sso") ? await getEEIdentityProviders() : [];
@@ -53,8 +52,6 @@ declare module 'next-auth' {
5352
declare module 'next-auth/jwt' {
5453
interface JWT {
5554
userId: string;
56-
providerAccountId: string;
57-
provider: string;
5855
linkedAccountTokens?: LinkedAccountTokensMap;
5956
}
6057
}
@@ -167,7 +164,7 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
167164
// Explicitly update the Account record with the OAuth token details.
168165
// This is necessary to update the access token when the user
169166
// re-authenticates.
170-
if (account && account.provider && account.provider !== 'credentials' && account.providerAccountId) {
167+
if (account && account.provider && account.providerAccountId) {
171168
await prisma.account.update({
172169
where: {
173170
provider_providerAccountId: {
@@ -220,60 +217,14 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
220217
}
221218
},
222219
callbacks: {
223-
async jwt({ token, user: _user, account: _account }) {
220+
async jwt({ token, user: _user, account }) {
221+
const user = _user as User | undefined;
224222
// @note: `user` will be available on signUp or signIn triggers.
225223
// Cache the userId in the JWT for later use.
226-
if (_user) {
227-
const user = _user as User;
224+
if (user) {
228225
token.userId = user.id;
229226
}
230227

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-
277228
if (hasEntitlement('permission-syncing')) {
278229
if (account && account.access_token && account.refresh_token && account.expires_at) {
279230
token.linkedAccountTokens = token.linkedAccountTokens || {};
@@ -322,4 +273,4 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
322273
// We set redirect to false in signInOptions so we can pass the email is as a param
323274
// verifyRequest: "/login/verify",
324275
}
325-
});
276+
});

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: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import type { IdentityProvider } from "@/auth";
22
import { onCreateUser } from "@/lib/authUtils";
3-
import { getRequiredScopes } from "@/lib/oauthScopes";
43
import { prisma } from "@/prisma";
54
import { AuthentikIdentityProviderConfig, GCPIAPIdentityProviderConfig, GitHubIdentityProviderConfig, GitLabIdentityProviderConfig, GoogleIdentityProviderConfig, KeycloakIdentityProviderConfig, MicrosoftEntraIDIdentityProviderConfig, OktaIdentityProviderConfig } from "@sourcebot/schemas/v3/index.type";
6-
import { createLogger, env, getTokenFromConfig, loadConfig } from "@sourcebot/shared";
5+
import { createLogger, env, getTokenFromConfig, hasEntitlement, loadConfig } from "@sourcebot/shared";
76
import { OAuth2Client } from "google-auth-library";
87
import type { User as AuthJsUser } from "next-auth";
98
import type { Provider } from "next-auth/providers";
@@ -127,10 +126,19 @@ const createGitHubProvider = (clientId: string, clientSecret: string, baseUrl?:
127126
...(hostname === GITHUB_CLOUD_HOSTNAME ? { enterprise: { baseUrl: baseUrl } } : {}), // if this is set the provider expects GHE so we need this check
128127
authorization: {
129128
params: {
130-
scope: getRequiredScopes('github'),
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(' '),
131140
},
132141
},
133-
allowDangerousEmailAccountLinking: true,
134142
});
135143
}
136144

@@ -142,7 +150,16 @@ const createGitLabProvider = (clientId: string, clientSecret: string, baseUrl?:
142150
authorization: {
143151
url: `${url}/oauth/authorize`,
144152
params: {
145-
scope: getRequiredScopes('gitlab'),
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(' '),
146163
},
147164
},
148165
token: {
@@ -151,15 +168,13 @@ const createGitLabProvider = (clientId: string, clientSecret: string, baseUrl?:
151168
userinfo: {
152169
url: `${url}/api/v4/user`,
153170
},
154-
allowDangerousEmailAccountLinking: true,
155171
});
156172
}
157173

158174
const createGoogleProvider = (clientId: string, clientSecret: string): Provider => {
159175
return Google({
160176
clientId: clientId,
161177
clientSecret: clientSecret,
162-
allowDangerousEmailAccountLinking: true,
163178
});
164179
}
165180

@@ -168,7 +183,6 @@ const createOktaProvider = (clientId: string, clientSecret: string, issuer: stri
168183
clientId: clientId,
169184
clientSecret: clientSecret,
170185
issuer: issuer,
171-
allowDangerousEmailAccountLinking: true,
172186
});
173187
}
174188

@@ -177,7 +191,6 @@ const createKeycloakProvider = (clientId: string, clientSecret: string, issuer:
177191
clientId: clientId,
178192
clientSecret: clientSecret,
179193
issuer: issuer,
180-
allowDangerousEmailAccountLinking: true,
181194
});
182195
}
183196

@@ -186,7 +199,6 @@ const createMicrosoftEntraIDProvider = (clientId: string, clientSecret: string,
186199
clientId: clientId,
187200
clientSecret: clientSecret,
188201
issuer: issuer,
189-
allowDangerousEmailAccountLinking: true,
190202
});
191203
}
192204

@@ -271,6 +283,5 @@ export const createAuthentikProvider = (clientId: string, clientSecret: string,
271283
clientId: clientId,
272284
clientSecret: clientSecret,
273285
issuer: issuer,
274-
allowDangerousEmailAccountLinking: true,
275286
});
276287
}

packages/web/src/lib/oauthScopes.ts

Lines changed: 0 additions & 48 deletions
This file was deleted.

0 commit comments

Comments
 (0)