Skip to content

Commit 0a6f4e9

Browse files
waleedlatif1waleedlatif1
committed
fix(credentials): address review round 1
GitLab's integration page offers personal tokens and saved access tokens side by side, a malformed GitLab token lookup reports the provider as unavailable, and the keyless issuer refuses a query string or fragment.
1 parent 7aa8030 commit 0a6f4e9

5 files changed

Lines changed: 80 additions & 51 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/integrations/[block]/integration-block-detail.tsx‎

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -250,32 +250,40 @@ function IntegrationBlockDetailContent({ integration, workspaceId }: Integration
250250
setConnectMode,
251251
])
252252

253-
const connectOptions = oauthService
254-
? [
255-
...(oauthAvailable
256-
? [
257-
{
258-
value: CONNECT_MODE.oauth,
259-
label: 'Connect with OAuth',
260-
icon: oauthService.serviceIcon,
261-
},
262-
]
263-
: []),
264-
...(hasServiceAccount
265-
? [
266-
{
267-
value: CONNECT_MODE.serviceAccount,
268-
label: serviceAccountConnectLabel,
269-
icon: serviceAccountTarget?.serviceIcon ?? oauthService.serviceIcon,
270-
},
271-
]
272-
: []),
273-
]
274-
: []
253+
const connectOptions = [
254+
...(personalTokenAvailable
255+
? [
256+
{
257+
value: CONNECT_MODE.personalToken,
258+
label: 'Add personal token',
259+
icon: oauthService?.serviceIcon ?? Icon,
260+
},
261+
]
262+
: []),
263+
...(oauthService && oauthAvailable
264+
? [
265+
{
266+
value: CONNECT_MODE.oauth,
267+
label: 'Connect with OAuth',
268+
icon: oauthService.serviceIcon,
269+
},
270+
]
271+
: []),
272+
...(oauthService && hasServiceAccount
273+
? [
274+
{
275+
value: CONNECT_MODE.serviceAccount,
276+
label: serviceAccountConnectLabel,
277+
icon: serviceAccountTarget?.serviceIcon ?? oauthService.serviceIcon,
278+
},
279+
]
280+
: []),
281+
]
275282

276283
const handleSelectConnectOption = (value: string) => {
277284
if (value === CONNECT_MODE.oauth) setOAuthOpen(true)
278285
else if (value === CONNECT_MODE.serviceAccount) setServiceAccountOpen(true)
286+
else if (value === CONNECT_MODE.personalToken) setPersonalTokenOpen(true)
279287
}
280288

281289
const handleAddInChat = () => {
@@ -290,22 +298,22 @@ function IntegrationBlockDetailContent({ integration, workspaceId }: Integration
290298
Integrations
291299
</ChipLink>
292300
<div className={cn('ml-auto', HEADER_ACTION_CLUSTER)}>
293-
{personalTokenAvailable ? (
301+
{connectOptions.length > 1 ? (
302+
<ChipDropdown
303+
variant='primary'
304+
leftIcon={Plus}
305+
placeholder='Add to Sim'
306+
showSelectedCheck={false}
307+
options={connectOptions}
308+
onChange={handleSelectConnectOption}
309+
matchTriggerWidth={false}
310+
/>
311+
) : personalTokenAvailable ? (
294312
<Chip variant='primary' leftIcon={Plus} onClick={() => setPersonalTokenOpen(true)}>
295313
Add personal token
296314
</Chip>
297315
) : oauthService ? (
298-
connectOptions.length > 1 ? (
299-
<ChipDropdown
300-
variant='primary'
301-
leftIcon={Plus}
302-
placeholder='Add to Sim'
303-
showSelectedCheck={false}
304-
options={connectOptions}
305-
onChange={handleSelectConnectOption}
306-
matchTriggerWidth={false}
307-
/>
308-
) : oauthAvailable ? (
316+
oauthAvailable ? (
309317
<Chip variant='primary' leftIcon={Plus} onClick={() => setOAuthOpen(true)}>
310318
Add to Sim
311319
</Chip>

‎apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ describe('GitLab saved token verification', () => {
5151
).rejects.toMatchObject({ code: 'invalid_credentials' })
5252
})
5353

54+
it('reports a malformed token lookup as GitLab misbehaving, not as a bad token', async () => {
55+
fetch.mockResolvedValue(new Response('<html>', { status: 200 }))
56+
57+
await expect(
58+
validateGitLabServiceAccount({ apiToken: 'glpat-x', domain: 'gitlab.com' })
59+
).rejects.toMatchObject({ code: 'provider_unavailable' })
60+
})
61+
5462
it('reports an unreachable GitLab instance as unavailable, not as a bad token', async () => {
5563
fetch.mockResolvedValue(jsonResponse({ message: 'Bad Gateway' }, 502))
5664

‎apps/sim/lib/credentials/token-service-accounts/validators/gitlab.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,16 @@ export async function validateGitLabServiceAccount(
6262
{ step: 'verify' }
6363
)
6464
}
65-
const token = toRecord(await response.json().catch(() => null))
65+
let body: unknown
66+
try {
67+
body = await response.json()
68+
} catch {
69+
throw new TokenServiceAccountValidationError('provider_unavailable', 502, {
70+
step: 'verify',
71+
reason: 'GitLab returned a malformed token lookup',
72+
})
73+
}
74+
const token = toRecord(body)
6675
const scopes = toArray(token.scopes)
6776
if (
6877
token.active !== true ||

‎apps/sim/lib/credentials/workload-identity/issuer.test.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,20 @@ describe('workload identity issuer', () => {
102102
expect((await workloadIdentityJwksResponse()).status).toBe(404)
103103
})
104104

105-
it.each(['http://sim.example.com/oidc/workload', 'https://sim.example.com:3000/oidc/workload'])(
106-
'stays off for an issuer no cloud can fetch (%s)',
107-
async (url) => {
108-
setEnv('WORKLOAD_IDENTITY_ISSUER', url)
109-
const issuer = await loadIssuer()
110-
const { workloadIdentityDiscoveryResponse } = await import(
111-
'@/lib/credentials/workload-identity/responses'
112-
)
113-
114-
expect(await issuer.isWorkloadIdentityConfigured()).toBe(false)
115-
expect((await workloadIdentityDiscoveryResponse()).status).toBe(404)
116-
}
117-
)
105+
it.each([
106+
'http://sim.example.com/oidc/workload',
107+
'https://sim.example.com:3000/oidc/workload',
108+
'https://sim.example.com/oidc/workload?tenant=1',
109+
])('stays off for an issuer no cloud can fetch (%s)', async (url) => {
110+
setEnv('WORKLOAD_IDENTITY_ISSUER', url)
111+
const issuer = await loadIssuer()
112+
const { workloadIdentityDiscoveryResponse } = await import(
113+
'@/lib/credentials/workload-identity/responses'
114+
)
115+
116+
expect(await issuer.isWorkloadIdentityConfigured()).toBe(false)
117+
expect((await workloadIdentityDiscoveryResponse()).status).toBe(404)
118+
})
118119

119120
it('keeps signing with the current key when the retiring key is malformed', async () => {
120121
setEnv('WORKLOAD_IDENTITY_PREVIOUS_KEY', 'not a PEM key')

‎apps/sim/lib/credentials/workload-identity/issuer.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,16 @@ async function loadPreviousPublicJwk(): Promise<JWK | null> {
7272

7373
/**
7474
* AWS and Databricks fetch discovery and keys from the issuer over https on the
75-
* default port, so any other issuer can never be trusted; keyless stays off
75+
* default port, and discovery appends paths to it, so any other issuer (or one
76+
* with a query or fragment) can never be trusted; keyless stays off
7677
* rather than advertising an issuer no relying party accepts.
7778
*/
7879
function assertPublicIssuer(issuer: string): void {
7980
const url = new URL(issuer)
80-
if (url.protocol !== 'https:' || url.port) {
81-
throw new Error('Workload identity issuer must be an https URL without a port')
81+
if (url.protocol !== 'https:' || url.port || url.search || url.hash) {
82+
throw new Error(
83+
'Workload identity issuer must be an https URL without a port, query, or fragment'
84+
)
8285
}
8386
if (!env.WORKLOAD_IDENTITY_ISSUER?.trim()) {
8487
logger.warn(

0 commit comments

Comments
 (0)