Skip to content

Commit 7bd639b

Browse files
committed
improvement(credentials): report Databricks token failures in the shared service-account codes
1 parent df19b34 commit 7bd639b

4 files changed

Lines changed: 93 additions & 14 deletions

File tree

‎apps/sim/lib/credentials/databricks-service-account.server.test.ts‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@
22
* @vitest-environment node
33
*/
44

5+
import { queueTableRows, resetDbChainMock } from '@sim/testing'
56
import { jsonResponse } from '@sim/testing/helpers/http'
67
import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock'
78
import { beforeEach, describe, expect, it, vi } from 'vitest'
89

910
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
1011

11-
import { verifyAndEncryptDatabricksCredential } from '@/lib/credentials/databricks-service-account.server'
12+
import { credential } from '@sim/db/schema'
13+
import {
14+
resolveDatabricksCredentialToken,
15+
verifyAndEncryptDatabricksCredential,
16+
} from '@/lib/credentials/databricks-service-account.server'
17+
import { DATABRICKS_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
1218

1319
const owner = { workspaceId: 'ws-1', actorUserId: 'user-1' }
1420

@@ -49,3 +55,49 @@ describe('saving a Databricks connection', () => {
4955
expect(encryptionMockFns.mockEncryptSecret).not.toHaveBeenCalled()
5056
})
5157
})
58+
59+
describe('getting a Databricks token for a run', () => {
60+
const fetch = vi.fn()
61+
62+
beforeEach(() => {
63+
resetDbChainMock()
64+
queueTableRows(credential, [
65+
{
66+
workspaceId: 'ws-1',
67+
organizationId: null,
68+
type: 'service_account',
69+
providerId: DATABRICKS_SERVICE_ACCOUNT_PROVIDER_ID,
70+
revokedAt: null,
71+
encryptedServiceAccountKey: 'ciphertext',
72+
},
73+
])
74+
encryptionMockFns.mockDecryptSecret.mockResolvedValueOnce({
75+
decrypted: JSON.stringify({
76+
type: 'databricks_credential_v1',
77+
config: {
78+
authType: 'service_principal',
79+
host: 'dbc-1234abcd-5678.cloud.databricks.com',
80+
clientId: '00000000-0000-0000-0000-000000000000',
81+
clientSecret: 'dose-secret',
82+
},
83+
}),
84+
})
85+
vi.stubGlobal('fetch', fetch)
86+
})
87+
88+
it('reports a refused client secret as a credential to reconnect', async () => {
89+
fetch.mockResolvedValue(jsonResponse({ error: 'invalid_client' }, 401))
90+
91+
await expect(resolveDatabricksCredentialToken('cred-1')).rejects.toMatchObject({
92+
code: 'invalid_credentials',
93+
})
94+
})
95+
96+
it('reports a Databricks outage as the provider being unavailable, not a bad credential', async () => {
97+
fetch.mockResolvedValue(jsonResponse({ error: 'temporarily_unavailable' }, 503))
98+
99+
await expect(resolveDatabricksCredentialToken('cred-1')).rejects.toMatchObject({
100+
code: 'provider_unavailable',
101+
})
102+
})
103+
})

‎apps/sim/lib/credentials/databricks-service-account.server.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import { z } from 'zod'
66
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
77
import { validateDatabricksWorkspaceHost } from '@/lib/core/security/input-validation'
88
import { parseServiceAccountConfig } from '@/lib/credentials/service-account-config'
9+
import {
10+
isTransientProviderStatus,
11+
TokenServiceAccountValidationError,
12+
} from '@/lib/credentials/token-service-accounts/errors'
913
import { WORKLOAD_IDENTITY_AUDIENCES } from '@/lib/credentials/workload-identity/audiences'
1014
import {
1115
mintWorkloadIdentityToken,
@@ -112,7 +116,11 @@ const storedSecretSchema = z
112116
.strict()
113117

114118
export class DatabricksCredentialError extends Error {
115-
constructor(message: string) {
119+
/** @param status Databricks' HTTP status, when Databricks itself refused the request. */
120+
constructor(
121+
message: string,
122+
readonly status?: number
123+
) {
116124
super(message)
117125
this.name = 'DatabricksCredentialError'
118126
}
@@ -145,7 +153,10 @@ async function requestWorkspaceToken(
145153
: typeof payload.error === 'string'
146154
? payload.error
147155
: `HTTP ${response.status}`
148-
throw new DatabricksCredentialError(`Databricks rejected the token request: ${reason}`)
156+
throw new DatabricksCredentialError(
157+
`Databricks rejected the token request: ${reason}`,
158+
response.status
159+
)
149160
}
150161
return payload.access_token
151162
}
@@ -299,8 +310,20 @@ export async function resolveDatabricksCredentialToken(
299310
const owner: WorkloadIdentityOwner = row.workspaceId
300311
? { workspaceId: row.workspaceId }
301312
: { organizationId: row.organizationId ?? undefined }
302-
return {
303-
accessToken: await resolveDatabricksAccessToken(config, owner, signal),
304-
domain: config.host,
313+
try {
314+
return {
315+
accessToken: await resolveDatabricksAccessToken(config, owner, signal),
316+
domain: config.host,
317+
}
318+
} catch (error) {
319+
if (!(error instanceof DatabricksCredentialError)) throw error
320+
// Report in the shared service-account codes: an outage is not a bad credential.
321+
const unavailable =
322+
error.status !== undefined && (error.status >= 500 || isTransientProviderStatus(error.status))
323+
throw new TokenServiceAccountValidationError(
324+
unavailable ? 'provider_unavailable' : 'invalid_credentials',
325+
error.status ?? 400,
326+
{ step: 'token', reason: error.message }
327+
)
305328
}
306329
}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,8 +667,17 @@ export const validateYouComServiceAccount = tokenCheckValidator({
667667
}),
668668
})
669669

670-
/** Google's 403 reasons that a valid key gets when it is only out of quota for now. */
671-
const YOUTUBE_QUOTA_REASONS = new Set(['quotaExceeded', 'dailyLimitExceeded', 'rateLimitExceeded'])
670+
/**
671+
* Google's 403 reasons for a valid key that is only out of quota for now. Mirrors
672+
* `isGoogleQuotaReason` in the Google connectors, whose module is too heavy for this graph.
673+
*/
674+
const YOUTUBE_QUOTA_REASONS = new Set([
675+
'quotaExceeded',
676+
'dailyLimitExceeded',
677+
'rateLimitExceeded',
678+
'userRateLimitExceeded',
679+
'RATE_LIMIT_EXCEEDED',
680+
])
672681

673682
/**
674683
* Listing video categories costs one quota unit. Google answers 400 for an invalid key and 403
@@ -684,7 +693,7 @@ export const validateYouTubeServiceAccount = tokenCheckValidator({
684693
forbiddenMeansScoped: true,
685694
rejectsKey: (errorBody) => {
686695
const error = toRecord(toRecord(errorBody).error)
687-
if (error.code !== 400 && error.code !== 403) return false
696+
if (error.code !== 403) return false
688697
return !toArray(error.errors).some((entry) =>
689698
YOUTUBE_QUOTA_REASONS.has(String(toRecord(entry).reason))
690699
)

‎apps/sim/lib/oauth/token-resolution.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -435,11 +435,6 @@ export async function resolveCredentialToken(
435435
}
436436
}
437437
}
438-
// Its messages name the trust, secret, or host problem and carry no secrets. Matched by
439-
// name because the Databricks module is loaded lazily, off this module's graph.
440-
if (error instanceof Error && error.name === 'DatabricksCredentialError') {
441-
return { ok: false, status: 401, error: error.message }
442-
}
443438
return { ok: false, status: 401, error: 'Failed to get service account token' }
444439
}
445440
}

0 commit comments

Comments
 (0)