Skip to content

Commit d9d55c2

Browse files
committed
refactor(credentials): drop the principal abstraction, keep the identity fixes
#6201 introduced a ServiceAccountPrincipal union mirrored centrally into audit and blob metadata, and replaced 21 provider-named audit keys with uniform ones. Nothing reads any of it. It was built for an identity UI that was deliberately not shipped, and the audit-key rename is a breaking change for anything consuming those rows. The gap it was meant to close needed a fraction of that: Atlassian already resolved its accountId, it just was not recorded where every other provider records its identifier. Removes principal.ts, the required-nullable field on all three registry result types, and the central mirroring. Restores the per-provider audit keys, so the only breaking change in #6201 is undone. 14 validators, both registry server.ts files, errors.ts, Zoom, Zoho Desk and 12 test files are byte-identical to main again — each verified as a pure principal swap with no fix inside. Keeps every bug fix: atlassianAccountId + email, googleClientEmail/projectId and slackBotUserId now land in auditMetadata alongside the existing keys; Box and Salesforce log identity-lookup failures (neither file had a logger, so a degraded connect left no trace); Shopify no longer rejects a working credential on a partial-scope error; Google/Slack rotation still re-labels and records the new identity. Also drops providerFailureReason, which became dead code once the minters were rebuilt on main's shape.
1 parent 03333ce commit d9d55c2

43 files changed

Lines changed: 160 additions & 475 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ describe('mintBoxServiceAccountToken', () => {
6767
.mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 3600 }))
6868
.mockResolvedValueOnce(
6969
jsonResponse(200, {
70-
id: '33445566',
7170
name: 'Sim Automation',
7271
login: 'AutomationUser_123_abc@boxdevedition.com',
7372
})
@@ -80,21 +79,22 @@ describe('mintBoxServiceAccountToken', () => {
8079
expiresInSeconds: 3600,
8180
identity: {
8281
displayName: 'Sim Automation',
83-
principal: {
84-
kind: 'user',
85-
id: '33445566',
86-
label: 'AutomationUser_123_abc@boxdevedition.com',
82+
auditMetadata: {
83+
boxEnterpriseId: '1234567',
84+
boxServiceAccountLogin: 'AutomationUser_123_abc@boxdevedition.com',
85+
},
86+
storedMetadata: {
87+
enterpriseId: '1234567',
88+
serviceAccountLogin: 'AutomationUser_123_abc@boxdevedition.com',
8789
},
88-
auditMetadata: { boxEnterpriseId: '1234567' },
89-
storedMetadata: { enterpriseId: '1234567' },
9090
},
9191
})
9292
expect(mockFetch).toHaveBeenCalledTimes(2)
9393
expectMintCall()
9494
expectIdentityCall()
9595
})
9696

97-
it('marks the principal as lookup_failed when users/me fails', async () => {
97+
it('still succeeds with a fallback identity when users/me fails', async () => {
9898
mockFetch
9999
.mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 2400 }))
100100
.mockResolvedValueOnce(jsonResponse(500, { message: 'boom' }))
@@ -105,26 +105,8 @@ describe('mintBoxServiceAccountToken', () => {
105105
expect(result.expiresInSeconds).toBe(2400)
106106
expect(result.identity).toEqual({
107107
displayName: 'Box enterprise 1234567',
108-
principal: { kind: 'lookup_failed', reason: 'HTTP 500' },
109108
auditMetadata: { boxEnterpriseId: '1234567' },
110-
storedMetadata: { enterpriseId: '1234567' },
111-
})
112-
})
113-
114-
it('marks the principal as lookup_failed when users/me omits the user id', async () => {
115-
mockFetch
116-
.mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 3600 }))
117-
.mockResolvedValueOnce(jsonResponse(200, { name: 'Sim Automation' }))
118-
119-
const result = await mintBoxServiceAccountToken(FIELDS)
120-
121-
expect(result.identity?.principal).toEqual({
122-
kind: 'lookup_failed',
123-
reason: 'response missing user id',
124109
})
125-
// Only the principal degrades — a name that did come back still beats the
126-
// Enterprise-ID fallback, so the credential does not lose its label.
127-
expect(result.identity?.displayName).toBe('Sim Automation')
128110
})
129111

130112
it('still succeeds when the identity request itself throws', async () => {
@@ -136,10 +118,6 @@ describe('mintBoxServiceAccountToken', () => {
136118

137119
expect(result.accessToken).toBe('box-access')
138120
expect(result.identity?.displayName).toBe('Box enterprise 1234567')
139-
expect(result.identity?.principal).toEqual({
140-
kind: 'lookup_failed',
141-
reason: 'provider_unavailable (HTTP 502)',
142-
})
143121
})
144122

145123
it('throws invalid_credentials on 400 invalid_client', async () => {

apps/sim/lib/credentials/client-credential-accounts/minters/box.ts

Lines changed: 20 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,12 @@ import {
1010
fetchProvider,
1111
isTransientProviderStatus,
1212
parseProviderJson,
13-
providerFailureReason,
1413
readProviderErrorSnippet,
1514
TokenServiceAccountValidationError,
1615
} from '@/lib/credentials/token-service-accounts/errors'
1716

1817
const logger = createLogger('BoxServiceAccountMinter')
1918

20-
const IDENTITY_STEP = 'box_identity'
21-
2219
const BOX_TOKEN_URL = 'https://api.box.com/oauth2/token'
2320
const BOX_CURRENT_USER_URL = 'https://api.box.com/2.0/users/me'
2421

@@ -27,14 +24,7 @@ interface BoxTokenResponse {
2724
expires_in?: number
2825
}
2926

30-
/**
31-
* `id`, `name`, and `login` are all in the standard field set `GET /2.0/users/me`
32-
* returns without a `fields` parameter, so capturing the Service Account's user
33-
* id costs no extra request.
34-
* @see https://developer.box.com/reference/get-users-me/
35-
*/
3627
interface BoxCurrentUserResponse {
37-
id?: string
3828
name?: string
3929
login?: string
4030
}
@@ -67,67 +57,52 @@ function boxErrorHint(body: string): string | undefined {
6757

6858
/**
6959
* Best-effort identity lookup for the app's Service Account user. A failure
70-
* never fails the mint — the credential degrades to an Enterprise-ID-derived
71-
* display name with a `lookup_failed` principal, so the audit record shows the
72-
* identity was not captured rather than implying none exists.
60+
* never fails the mint — the caller falls back to an Enterprise-ID-derived
61+
* display name.
7362
*/
7463
async function fetchBoxServiceAccountIdentity(
7564
accessToken: string,
7665
orgId: string
7766
): Promise<ClientCredentialAccountIdentity> {
78-
/**
79-
* `label` keeps whatever human name the lookup did return. A response can
80-
* carry `name`/`login` but no `id` — the principal is then unusable, but the
81-
* label still beats the Enterprise-ID fallback, so only the principal
82-
* degrades and the credential does not silently lose its name.
83-
*/
84-
const degraded = (reason: string, label?: string): ClientCredentialAccountIdentity => ({
85-
displayName: label ?? `Box enterprise ${orgId}`,
86-
principal: { kind: 'lookup_failed', reason },
67+
const fallback: ClientCredentialAccountIdentity = {
68+
displayName: `Box enterprise ${orgId}`,
8769
auditMetadata: { boxEnterpriseId: orgId },
88-
storedMetadata: { enterpriseId: orgId },
89-
})
70+
}
9071
try {
9172
const res = await fetchProvider(
9273
BOX_CURRENT_USER_URL,
9374
{ headers: { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' } },
94-
IDENTITY_STEP
75+
'box_identity'
9576
)
9677
if (!res.ok) {
9778
logger.warn('Box service-account identity lookup failed', {
98-
step: IDENTITY_STEP,
79+
step: 'box_identity',
9980
status: res.status,
10081
enterpriseId: orgId,
10182
})
102-
return degraded(`HTTP ${res.status}`)
83+
return fallback
10384
}
104-
const user = await parseProviderJson<BoxCurrentUserResponse>(res, IDENTITY_STEP)
105-
const id = typeof user.id === 'string' && user.id ? user.id : undefined
85+
const user = await parseProviderJson<BoxCurrentUserResponse>(res, 'box_identity')
10686
const login = typeof user.login === 'string' && user.login ? user.login : undefined
10787
const name = typeof user.name === 'string' && user.name ? user.name : undefined
108-
if (!id) {
109-
logger.warn('Box service-account identity response carried no user id', {
110-
step: IDENTITY_STEP,
111-
status: res.status,
112-
enterpriseId: orgId,
113-
})
114-
return degraded('response missing user id', name ?? login)
115-
}
11688
return {
117-
displayName: name ?? login ?? `Box enterprise ${orgId}`,
118-
// The Service Account is a real Box user; `enterpriseId` is shared by
119-
// every app in the enterprise and so is kept as separate context.
120-
principal: { kind: 'user', id, ...(login ? { label: login } : {}) },
121-
auditMetadata: { boxEnterpriseId: orgId },
122-
storedMetadata: { enterpriseId: orgId },
89+
displayName: name ?? login ?? fallback.displayName,
90+
auditMetadata: {
91+
boxEnterpriseId: orgId,
92+
...(login ? { boxServiceAccountLogin: login } : {}),
93+
},
94+
storedMetadata: {
95+
enterpriseId: orgId,
96+
...(login ? { serviceAccountLogin: login } : {}),
97+
},
12398
}
12499
} catch (error) {
125100
logger.warn('Box service-account identity lookup threw', {
126-
step: IDENTITY_STEP,
101+
step: 'box_identity',
127102
enterpriseId: orgId,
128103
error: getErrorMessage(error),
129104
})
130-
return degraded(providerFailureReason(error))
105+
return fallback
131106
}
132107
}
133108

apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ describe('mintSalesforceServiceAccountToken', () => {
7878
name: 'Integration User',
7979
preferred_username: 'integration@yourorg.com',
8080
organization_id: '00Dxx0000000001EAA',
81-
user_id: '005xx000001Sv6DAAS',
8281
})
8382
)
8483

@@ -91,19 +90,16 @@ describe('mintSalesforceServiceAccountToken', () => {
9190
grantedScopes: ['api'],
9291
identity: {
9392
displayName: 'Integration User',
94-
principal: {
95-
kind: 'user',
96-
id: '005xx000001Sv6DAAS',
97-
label: 'integration@yourorg.com',
98-
},
9993
auditMetadata: {
10094
salesforceMyDomainHost: HOST,
10195
salesforceOrgId: '00Dxx0000000001EAA',
96+
salesforceRunAsUsername: 'integration@yourorg.com',
10297
},
10398
storedMetadata: {
10499
myDomainHost: HOST,
105100
instanceUrl: INSTANCE_URL,
106101
orgId: '00Dxx0000000001EAA',
102+
runAsUsername: 'integration@yourorg.com',
107103
grantedScopes: 'api',
108104
},
109105
},
@@ -263,7 +259,7 @@ describe('mintSalesforceServiceAccountToken', () => {
263259
})
264260
})
265261

266-
it('marks the principal as lookup_failed when the userinfo call throws', async () => {
262+
it('falls back to a host-derived identity when the userinfo call fails', async () => {
267263
mockFetch
268264
.mockResolvedValueOnce(
269265
jsonResponse(200, { access_token: 'sf-access', instance_url: INSTANCE_URL })
@@ -275,30 +271,11 @@ describe('mintSalesforceServiceAccountToken', () => {
275271
expect(result.accessToken).toBe('sf-access')
276272
expect(result.identity).toEqual({
277273
displayName: `Salesforce ${HOST}`,
278-
principal: { kind: 'lookup_failed', reason: 'provider_unavailable (HTTP 502)' },
279274
auditMetadata: { salesforceMyDomainHost: HOST },
280275
storedMetadata: { myDomainHost: HOST, instanceUrl: INSTANCE_URL },
281276
})
282277
})
283278

284-
it('marks the principal as lookup_failed when userinfo omits user_id', async () => {
285-
mockFetch
286-
.mockResolvedValueOnce(
287-
jsonResponse(200, { access_token: 'sf-access', instance_url: INSTANCE_URL })
288-
)
289-
.mockResolvedValueOnce(jsonResponse(200, { name: 'Integration User' }))
290-
291-
const result = await mintSalesforceServiceAccountToken(FIELDS)
292-
293-
expect(result.identity?.principal).toEqual({
294-
kind: 'lookup_failed',
295-
reason: 'response missing user_id',
296-
})
297-
// Only the principal degrades — a name that did come back still beats the
298-
// host fallback, so the credential does not lose its label.
299-
expect(result.identity?.displayName).toBe('Integration User')
300-
})
301-
302279
it('ignores a non-Salesforce instance_url and falls back to the validated host', async () => {
303280
mockFetch
304281
.mockResolvedValueOnce(

0 commit comments

Comments
 (0)