Skip to content

Commit e1039f2

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
refactor(oci): remove shared credential hardening
1 parent 25ad712 commit e1039f2

7 files changed

Lines changed: 166 additions & 311 deletions

File tree

‎apps/sim/lib/credentials/orchestration/credential-create.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { decryptSecret } from '@/lib/core/security/encryption'
1313
import { getCredentialActorContext, requireOrdinaryCredentialType } from '@/lib/credentials/access'
1414
import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account'
1515
import { getCredentialCreationWorkspaceContext } from '@/lib/credentials/environment'
16+
import { OciCredentialVerificationError } from '@/lib/credentials/oci-api-key-service-account.server'
1617
import type { CredentialOrchestrationErrorCode } from '@/lib/credentials/orchestration'
1718
import {
1819
ServiceAccountSecretError,
@@ -295,10 +296,7 @@ export async function createCredentialRecord(
295296
Object.assign(extraAuditMetadata, secret.auditMetadata)
296297
} catch (error) {
297298
if (error instanceof ServiceAccountSecretError) {
298-
return failure(error.message, 'validation', {
299-
providerErrorCode: error.providerErrorCode,
300-
providerUnavailable: isProviderOutageCode(error.providerErrorCode),
301-
})
299+
return failure(error.message, 'validation')
302300
}
303301
throw error
304302
}
@@ -540,6 +538,18 @@ export async function createCredentialRecord(
540538

541539
return { success: true, credential: created, created: true, auditMetadata: extraAuditMetadata }
542540
} catch (error: unknown) {
541+
if (error instanceof OciCredentialVerificationError) {
542+
const providerUnavailable = error.code !== 'invalid_credentials'
543+
const providerErrorCode = providerUnavailable ? 'provider_unavailable' : 'invalid_credentials'
544+
logger.warn(`OCI credential rejected: ${error.code}`)
545+
return failure(
546+
providerUnavailable
547+
? 'OCI is temporarily unavailable for credential verification'
548+
: 'OCI rejected the API-key credential',
549+
'validation',
550+
{ providerErrorCode, providerUnavailable }
551+
)
552+
}
543553
if (error instanceof AtlassianValidationError) {
544554
logger.warn(`Atlassian credential rejected: ${error.code}`, {
545555
code: error.code,

‎apps/sim/lib/credentials/orchestration/index.test.ts‎

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ vi.mock('@/lib/credentials/environment', () => ({
6767
vi.mock('@/lib/credentials/atlassian-service-account', () => ({
6868
AtlassianValidationError: class AtlassianValidationError extends Error {},
6969
}))
70+
vi.mock('@/lib/credentials/oci-api-key-service-account.server', () => ({
71+
OciCredentialVerificationError: class OciCredentialVerificationError extends Error {
72+
constructor(readonly code: string) {
73+
super(code)
74+
}
75+
},
76+
}))
7077
vi.mock('@/lib/credentials/token-service-accounts/errors', () => ({
7178
TokenServiceAccountValidationError: class TokenServiceAccountValidationError extends Error {},
7279
}))
@@ -176,6 +183,13 @@ describe('performUpdateCredential — service-account secret rotation', () => {
176183
principal: { kind: 'user', id: 'ocid1.user.oc1..replacement' },
177184
})
178185

186+
const { ServiceAccountSecretError } = await import('@/lib/credentials/service-account-secret')
187+
mockVerifyAndBuildServiceAccountSecret.mockRejectedValueOnce(
188+
new ServiceAccountSecretError(
189+
'tenancyOcid, userOcid, fingerprint, privateKey, and region are required for OCI API-key credentials'
190+
)
191+
)
192+
179193
const incomplete = await performUpdateCredential({
180194
credentialId: 'cred-1',
181195
userId: 'user-1',
@@ -184,9 +198,12 @@ describe('performUpdateCredential — service-account secret rotation', () => {
184198
expect(incomplete).toMatchObject({
185199
success: false,
186200
errorCode: 'validation',
187-
error: expect.stringContaining('complete signing tuple'),
201+
error: expect.stringContaining('tenancyOcid, userOcid, fingerprint, privateKey, and region'),
188202
})
189-
expect(mockVerifyAndBuildServiceAccountSecret).not.toHaveBeenCalled()
203+
expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenCalledWith(
204+
'oci-api-key-service-account',
205+
expect.objectContaining({ privateKey: 'replacement-key' })
206+
)
190207

191208
const complete = await performUpdateCredential({
192209
credentialId: 'cred-1',
@@ -198,7 +215,7 @@ describe('performUpdateCredential — service-account secret rotation', () => {
198215
region: 'us-ashburn-1',
199216
})
200217
expect(complete.success).toBe(true)
201-
expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenCalledWith(
218+
expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenLastCalledWith(
202219
'oci-api-key-service-account',
203220
expect.objectContaining({
204221
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
@@ -212,6 +229,49 @@ describe('performUpdateCredential — service-account secret rotation', () => {
212229
expect(updatePayload().encryptedServiceAccountKey).toBe('new-oci-cipher')
213230
})
214231

232+
it.each([
233+
['invalid_credentials', 'invalid_credentials', 'OCI rejected the API-key credential'],
234+
[
235+
'service_unavailable',
236+
'provider_unavailable',
237+
'OCI is temporarily unavailable for credential verification',
238+
],
239+
[
240+
'invalid_response',
241+
'provider_unavailable',
242+
'OCI is temporarily unavailable for credential verification',
243+
],
244+
] as const)(
245+
'maps OCI %s rotation failures without changing generic service-account errors',
246+
async (code, providerErrorCode, message) => {
247+
mockCredential({ providerId: 'oci-api-key-service-account' })
248+
const { OciCredentialVerificationError } = await import(
249+
'@/lib/credentials/oci-api-key-service-account.server'
250+
)
251+
mockVerifyAndBuildServiceAccountSecret.mockRejectedValue(
252+
new OciCredentialVerificationError(code)
253+
)
254+
255+
const result = await performUpdateCredential({
256+
credentialId: 'cred-1',
257+
userId: 'user-1',
258+
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
259+
userOcid: 'ocid1.user.oc1..replacement',
260+
fingerprint: '00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
261+
privateKey: 'replacement-key',
262+
region: 'us-ashburn-1',
263+
})
264+
265+
expect(result).toMatchObject({
266+
success: false,
267+
errorCode: 'validation',
268+
error: message,
269+
providerErrorCode,
270+
})
271+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
272+
}
273+
)
274+
215275
it('keeps a label the user typed instead of the derived identity', async () => {
216276
mockCredential({ displayName: 'Prod billing exporter' })
217277
mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL })
@@ -481,6 +541,7 @@ describe('performUpdateCredential — service-account secret rotation', () => {
481541
})
482542

483543
expect(result).toMatchObject({ success: false, errorCode: 'validation' })
544+
expect(result).not.toHaveProperty('providerErrorCode')
484545
expect(dbChainMockFns.update).not.toHaveBeenCalled()
485546
expect(mockRecordAudit).not.toHaveBeenCalled()
486547
})
@@ -662,6 +723,54 @@ describe('createServiceAccountCredential', () => {
662723
resetDbChainMock()
663724
})
664725

726+
it.each([
727+
['invalid_credentials', 'invalid_credentials', false, 'OCI rejected the API-key credential'],
728+
[
729+
'service_unavailable',
730+
'provider_unavailable',
731+
true,
732+
'OCI is temporarily unavailable for credential verification',
733+
],
734+
[
735+
'invalid_response',
736+
'provider_unavailable',
737+
true,
738+
'OCI is temporarily unavailable for credential verification',
739+
],
740+
] as const)(
741+
'maps OCI %s creation failures through the existing provider result contract',
742+
async (code, providerErrorCode, providerUnavailable, message) => {
743+
const { OciCredentialVerificationError } = await import(
744+
'@/lib/credentials/oci-api-key-service-account.server'
745+
)
746+
mockVerifyAndBuildServiceAccountSecret.mockRejectedValue(
747+
new OciCredentialVerificationError(code)
748+
)
749+
750+
const result = await createServiceAccountCredential({
751+
workspaceId: 'workspace-1',
752+
userId: 'user-1',
753+
providerId: 'oci-api-key-service-account',
754+
displayName: 'OCI signer',
755+
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
756+
userOcid: 'ocid1.user.oc1..principal',
757+
fingerprint: '00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
758+
privateKey: 'provider-secret-key',
759+
region: 'us-ashburn-1',
760+
})
761+
762+
expect(result).toMatchObject({
763+
success: false,
764+
errorCode: 'validation',
765+
error: message,
766+
providerErrorCode,
767+
providerUnavailable,
768+
})
769+
expect(JSON.stringify(result)).not.toContain('provider-secret-key')
770+
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
771+
}
772+
)
773+
665774
it('rejects an existing service-account source instead of discarding the submitted secret', async () => {
666775
mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({
667776
providerId: 'zoom-service-account',

‎apps/sim/lib/credentials/orchestration/index.ts‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
deletePersonalEnvCredentialForUser,
3636
deleteWorkspaceEnvCredentials,
3737
} from '@/lib/credentials/environment'
38+
import { OciCredentialVerificationError } from '@/lib/credentials/oci-api-key-service-account.server'
3839
import type { ServiceAccountFieldId } from '@/lib/credentials/service-account-fields'
3940
import {
4041
ServiceAccountSecretError,
@@ -44,7 +45,6 @@ import { TokenServiceAccountValidationError } from '@/lib/credentials/token-serv
4445
import { invalidateEffectiveDecryptedEnvCache } from '@/lib/environment/utils'
4546
import {
4647
GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID,
47-
OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID,
4848
SLACK_CUSTOM_BOT_PROVIDER_ID,
4949
SLACK_CUSTOM_BOT_SECRET_TYPE,
5050
} from '@/lib/oauth/types'
@@ -296,24 +296,6 @@ export async function updateCredentialRecord(
296296
if (hasRotationSecret) {
297297
const providerId = params.credential.providerId ?? ''
298298

299-
if (providerId === OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID) {
300-
const requiredOciFields = [
301-
'tenancyOcid',
302-
'userOcid',
303-
'fingerprint',
304-
'privateKey',
305-
'region',
306-
] as const
307-
const missingOciFields = requiredOciFields.filter((field) => params[field] === undefined)
308-
if (missingOciFields.length > 0) {
309-
return {
310-
success: false,
311-
error: `OCI credential rotation requires the complete signing tuple; missing ${missingOciFields.join(', ')}`,
312-
errorCode: 'validation',
313-
}
314-
}
315-
}
316-
317299
// A reconnect rebuilds the secret blob from the submitted fields only, and
318300
// the modal never prefills (secrets are never echoed back). For an actual
319301
// secret that is correct - the admin retypes it. But a non-secret selector
@@ -422,11 +404,17 @@ export async function updateCredentialRecord(
422404
}
423405
} catch (error) {
424406
if (error instanceof ServiceAccountSecretError) {
407+
return { success: false, error: error.message, errorCode: 'validation' }
408+
}
409+
if (error instanceof OciCredentialVerificationError) {
410+
const providerUnavailable = error.code !== 'invalid_credentials'
425411
return {
426412
success: false,
427-
error: error.message,
413+
error: providerUnavailable
414+
? 'OCI is temporarily unavailable for credential verification'
415+
: 'OCI rejected the API-key credential',
428416
errorCode: 'validation',
429-
providerErrorCode: error.providerErrorCode,
417+
providerErrorCode: providerUnavailable ? 'provider_unavailable' : 'invalid_credentials',
430418
}
431419
}
432420
if (error instanceof AtlassianValidationError) {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ describe('verifyAndBuildServiceAccountSecret', () => {
218218
})
219219

220220
it.each(['service_unavailable', 'invalid_response'] as const)(
221-
'classifies OCI %s failures as provider outages without exposing provider details',
221+
'preserves the dedicated OCI %s classification for orchestration',
222222
async (code) => {
223223
const { OciCredentialVerificationError } = await import(
224224
'@/lib/credentials/oci-api-key-service-account.server'
@@ -233,16 +233,16 @@ describe('verifyAndBuildServiceAccountSecret', () => {
233233
region: 'us-ashburn-1',
234234
}).catch((error: unknown) => error)
235235

236-
expect(failure).toBeInstanceOf(ServiceAccountSecretError)
236+
expect(failure).toBeInstanceOf(OciCredentialVerificationError)
237237
expect(failure).toMatchObject({
238-
message: 'OCI is temporarily unavailable for credential verification',
239-
providerErrorCode: 'provider_unavailable',
238+
message: code,
239+
code,
240240
})
241241
expect(JSON.stringify(failure)).not.toContain('provider-secret-key')
242242
}
243243
)
244244

245-
it('classifies local OCI field validation as rejected credentials', async () => {
245+
it('preserves the dedicated OCI invalid-credential classification for orchestration', async () => {
246246
const { OciCredentialVerificationError } = await import(
247247
'@/lib/credentials/oci-api-key-service-account.server'
248248
)
@@ -258,11 +258,7 @@ describe('verifyAndBuildServiceAccountSecret', () => {
258258
privateKey: 'invalid-key',
259259
region: 'us-ashburn-1',
260260
})
261-
).rejects.toMatchObject({
262-
name: 'ServiceAccountSecretError',
263-
message: 'OCI rejected the API-key credential',
264-
providerErrorCode: 'invalid_credentials',
265-
})
261+
).rejects.toMatchObject({ message: 'invalid_credentials', code: 'invalid_credentials' })
266262
})
267263

268264
it('does not misclassify an internal OCI credential failure as rejected credentials', async () => {

‎apps/sim/lib/credentials/service-account-secret.ts‎

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ import {
2020
getClientCredentialAccountMinter,
2121
} from '@/lib/credentials/client-credential-accounts/server'
2222
import { slackCustomBotDisplayName } from '@/lib/credentials/display-name'
23-
import {
24-
OciCredentialVerificationError,
25-
verifyAndEncryptOciApiKeyCredential,
26-
} from '@/lib/credentials/oci-api-key-service-account.server'
23+
import { verifyAndEncryptOciApiKeyCredential } from '@/lib/credentials/oci-api-key-service-account.server'
2724
import {
2825
type ServiceAccountPrincipal,
2926
serviceAccountPrincipalMetadata,
@@ -87,10 +84,7 @@ export interface ServiceAccountSecretResult {
8784

8885
/** Thrown when a service-account secret is missing or fails provider verification. */
8986
export class ServiceAccountSecretError extends Error {
90-
constructor(
91-
message: string,
92-
readonly providerErrorCode?: string
93-
) {
87+
constructor(message: string) {
9488
super(message)
9589
this.name = 'ServiceAccountSecretError'
9690
}
@@ -239,36 +233,22 @@ async function buildOciApiKeyServiceAccountSecret(
239233
'tenancyOcid, userOcid, fingerprint, privateKey, and region are required for OCI API-key credentials'
240234
)
241235
}
242-
try {
243-
const result = await verifyAndEncryptOciApiKeyCredential({
244-
tenancyOcid,
245-
userOcid,
246-
fingerprint,
247-
privateKey,
248-
...(privateKeyPassphrase !== undefined ? { privateKeyPassphrase } : {}),
249-
region,
250-
})
251-
const principal: ServiceAccountPrincipal = { kind: 'user', id: result.userOcid }
252-
const metadata = serviceAccountPrincipalMetadata(principal)
253-
return {
254-
providerId: OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID,
255-
encryptedServiceAccountKey: result.encryptedServiceAccountKey,
256-
displayName: result.userOcid,
257-
auditMetadata: metadata,
258-
principal,
259-
}
260-
} catch (error) {
261-
if (error instanceof OciCredentialVerificationError) {
262-
const providerUnavailable =
263-
error.code === 'service_unavailable' || error.code === 'invalid_response'
264-
throw new ServiceAccountSecretError(
265-
providerUnavailable
266-
? 'OCI is temporarily unavailable for credential verification'
267-
: 'OCI rejected the API-key credential',
268-
providerUnavailable ? 'provider_unavailable' : 'invalid_credentials'
269-
)
270-
}
271-
throw error
236+
const result = await verifyAndEncryptOciApiKeyCredential({
237+
tenancyOcid,
238+
userOcid,
239+
fingerprint,
240+
privateKey,
241+
...(privateKeyPassphrase !== undefined ? { privateKeyPassphrase } : {}),
242+
region,
243+
})
244+
const principal: ServiceAccountPrincipal = { kind: 'user', id: result.userOcid }
245+
const metadata = serviceAccountPrincipalMetadata(principal)
246+
return {
247+
providerId: OCI_API_KEY_SERVICE_ACCOUNT_PROVIDER_ID,
248+
encryptedServiceAccountKey: result.encryptedServiceAccountKey,
249+
displayName: result.userOcid,
250+
auditMetadata: metadata,
251+
principal,
272252
}
273253
}
274254

0 commit comments

Comments
 (0)