Skip to content

Commit 8f1b5ac

Browse files
brendan-kellambrendan-kellam
committed
feedback
1 parent 97666d0 commit 8f1b5ac

7 files changed

Lines changed: 137 additions & 177 deletions

File tree

‎packages/backend/src/ee/accountPermissionSyncer.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as Sentry from "@sentry/node";
22
import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSource} from "@sourcebot/db";
3-
import { env, createLogger, loadConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared";
3+
import { env, createLogger, getIdentityProviderConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared";
44
import { hasEntitlement } from "../entitlements.js";
55
import { ensureFreshAccountToken } from "./tokenRefresh.js";
66
import { Job, Queue, Worker } from "bullmq";
@@ -218,8 +218,6 @@ export class AccountPermissionSyncer {
218218
account: Account & { user: { email: string | null } },
219219
logger: ReturnType<typeof createJobLogger>,
220220
) {
221-
const config = await loadConfig(env.CONFIG_PATH);
222-
223221
logger.debug(`Syncing permissions for ${account.providerId} account (id: ${account.id}) for user ${account.user.email}...`);
224222

225223
// Ensure the OAuth token is fresh, refreshing it if it is expired or near expiry.
@@ -242,9 +240,7 @@ export class AccountPermissionSyncer {
242240
const repoIds = await (async () => {
243241
const aggregatedRepoIds: Set<number> = new Set();
244242

245-
const idpConfig = config.identityProviders ?
246-
config.identityProviders[account.providerId] :
247-
undefined;
243+
const idpConfig = await getIdentityProviderConfig(account.providerId);
248244

249245
if (!idpConfig) {
250246
throw new Error(`Unable to find IDP config in config.json.`);

‎packages/backend/src/ee/tokenRefresh.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import {
1010
decryptOAuthToken,
1111
encryptOAuthToken,
1212
env,
13+
getIdentityProviderConfig,
1314
getTokenFromConfig,
1415
IdentityProviderType,
15-
loadConfig,
1616
} from '@sourcebot/shared';
1717
import { z } from 'zod';
1818

@@ -157,10 +157,7 @@ const refreshOAuthToken = async (
157157
refreshToken: string,
158158
): Promise<OAuthTokenResponse | null> => {
159159
try {
160-
const config = await loadConfig(env.CONFIG_PATH);
161-
const idpConfig = config.identityProviders ?
162-
config.identityProviders[providerId] :
163-
undefined;
160+
const idpConfig = await getIdentityProviderConfig(providerId);
164161

165162
if (!idpConfig) {
166163
logger.error(`No provider config found for: ${providerId}`);

‎packages/shared/src/env.server.ts‎

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@ import stripJsonComments from "strip-json-comments";
77
import { z } from "zod";
88
import { getTokenFromConfig } from "./crypto.js";
99

10-
export type NormalizedIdentityProviders = Record<string, IdentityProviderConfig>;
11-
export type NormalizedSourcebotConfig = Omit<SourcebotConfig, "identityProviders"> & {
12-
identityProviders?: NormalizedIdentityProviders;
13-
};
14-
1510
// Booleans are specified as 'true' or 'false' strings.
1611
const booleanSchema = z.enum(["true", "false"]);
1712

@@ -25,7 +20,7 @@ const ajv = new Ajv({
2520
validateFormats: false,
2621
});
2722

28-
export const resolveEnvironmentVariableOverridesFromConfig = async (config: NormalizedSourcebotConfig): Promise<Record<string, string>> => {
23+
export const resolveEnvironmentVariableOverridesFromConfig = async (config: SourcebotConfig): Promise<Record<string, string>> => {
2924
if (!config.environmentOverrides) {
3025
return {};
3126
}
@@ -61,39 +56,8 @@ export const isRemotePath = (path: string) => {
6156
return path.startsWith('https://') || path.startsWith('http://');
6257
}
6358

64-
/**
65-
* Collapses the dual-form `identityProviders` field into the canonical object
66-
* form keyed by id. The array form is deprecated and only supports a single
67-
* instance per provider type - its synthesized id is `entry.provider`, which
68-
* matches the value historically stored in `Account.provider` for those users,
69-
* so existing single-instance deployments don't need a data migration.
70-
*/
71-
const normalizeIdentityProviders = (
72-
raw: SourcebotConfig["identityProviders"],
73-
): NormalizedIdentityProviders | undefined => {
74-
if (!raw) {
75-
return undefined;
76-
}
77-
if (!Array.isArray(raw)) {
78-
return raw;
79-
}
80-
81-
const result: NormalizedIdentityProviders = {};
82-
for (const entry of raw) {
83-
const id = entry.provider;
84-
if (result[id]) {
85-
throw new Error(
86-
`Duplicate identity provider id "${id}" in array-form \`identityProviders\`. ` +
87-
`The array form is deprecated and only supports one instance per provider type. ` +
88-
`Migrate to the object form (keyed by id) to configure multiple instances.`,
89-
);
90-
}
91-
result[id] = entry;
92-
}
93-
return result;
94-
};
9559

96-
export const loadConfig = async (configPath?: string): Promise<NormalizedSourcebotConfig> => {
60+
export const loadConfig = async (configPath?: string): Promise<SourcebotConfig> => {
9761
if (!configPath) {
9862
throw new Error('CONFIG_PATH is required but not provided');
9963
}
@@ -147,10 +111,45 @@ export const loadConfig = async (configPath?: string): Promise<NormalizedSourceb
147111
throw new Error(`Config file '${configPath}' is invalid: ${ajv.errorsText(ajv.errors)}`);
148112
}
149113

150-
return {
151-
...config,
152-
identityProviders: normalizeIdentityProviders(config.identityProviders),
153-
};
114+
return config;
115+
}
116+
117+
118+
export const getIdentityProviderConfigs = async (): Promise<Record<string, IdentityProviderConfig>> => {
119+
const config = await loadConfig(env.CONFIG_PATH);
120+
121+
// Collapses the dual-form `identityProviders` field into the canonical object
122+
// form keyed by id.
123+
const idpConfigs = (() => {
124+
if (!config.identityProviders) {
125+
return undefined;
126+
}
127+
if (!Array.isArray(config.identityProviders)) {
128+
return config.identityProviders;
129+
}
130+
131+
const result: Record<string, IdentityProviderConfig> = {};
132+
for (const entry of config.identityProviders) {
133+
const id = entry.provider;
134+
if (result[id]) {
135+
throw new Error(
136+
`Duplicate identity provider id "${id}" in array-form \`identityProviders\`. ` +
137+
`The array form is deprecated and only supports one instance per provider type. ` +
138+
`Migrate to the object form (keyed by id) to configure multiple instances.`,
139+
);
140+
}
141+
result[id] = entry;
142+
}
143+
return result;
144+
})();
145+
146+
return idpConfigs ?? {};
147+
}
148+
149+
export const getIdentityProviderConfig = async (id: string): Promise<IdentityProviderConfig | undefined> => {
150+
const idps = await getIdentityProviderConfigs();
151+
const idp = idps[id] as IdentityProviderConfig | undefined;
152+
return idp;
154153
}
155154

156155
// Merge process.env with environment variables resolved from config.json

‎packages/shared/src/index.server.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,10 @@ export * from "./constants.js";
3636
export {
3737
resolveEnvironmentVariableOverridesFromConfig,
3838
loadConfig,
39+
getIdentityProviderConfigs,
40+
getIdentityProviderConfig,
3941
isRemotePath,
4042
} from "./env.server.js";
41-
export type {
42-
NormalizedSourcebotConfig,
43-
} from "./env.server.js";
4443
export { env } from "./env.server.js"
4544
export {
4645
createLogger,

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { withAuth } from "@/middleware/withAuth";
66
import { withMinimumOrgRole } from "@/middleware/withMinimumOrgRole";
77
import { OrgRole } from "@sourcebot/db";
88
import { hasEntitlement } from "@/lib/entitlements";
9-
import { createLogger, doesIdpSupportPermissionSyncing, env, loadConfig } from "@sourcebot/shared";
9+
import { createLogger, doesIdpSupportPermissionSyncing, env, getIdentityProviderConfig, getIdentityProviderConfigs } from "@sourcebot/shared";
1010
import { cookies } from "next/headers";
1111

1212
const logger = createLogger('web-ee-sso-actions');
@@ -44,8 +44,6 @@ export const getLinkedAccounts = async () => sew(() =>
4444
},
4545
});
4646

47-
const config = await loadConfig(env.CONFIG_PATH);
48-
4947
const permissionSyncEnabled =
5048
env.PERMISSION_SYNC_ENABLED === 'true' &&
5149
await hasEntitlement('permission-syncing');
@@ -54,9 +52,7 @@ export const getLinkedAccounts = async () => sew(() =>
5452

5553
// All connected accounts (from DB), enriched with config data where available
5654
for (const account of accounts) {
57-
const providerConfig = config.identityProviders ?
58-
config.identityProviders[account.providerId] :
59-
undefined;
55+
const providerConfig = await getIdentityProviderConfig(account.providerId);
6056
const isAccountLinking = providerConfig?.purpose === 'account_linking';
6157

6258
result.push({
@@ -74,7 +70,8 @@ export const getLinkedAccounts = async () => sew(() =>
7470
}
7571

7672
// Unlinked account_linking providers from config (not yet connected)
77-
for (const [id, providerConfig] of Object.entries(config.identityProviders ?? {})) {
73+
const identityProviders = await getIdentityProviderConfigs();
74+
for (const [id, providerConfig] of Object.entries(identityProviders)) {
7875
const account = accounts.find((account) => account.providerId === id);
7976
if (!account) {
8077
result.push({

0 commit comments

Comments
 (0)