Skip to content

Commit f843739

Browse files
committed
fix(credentials): close wave 2b review findings on DocuSign JWT Grant
- The DocuSign knowledge connector now honours a JWT Grant credential's bound account. The resolved token's instanceUrl reaches the connector as credentialInstanceUrl in the sync context (seeded on every path through syncContextForToken). The connector re-validates it with the same parser as DocuSignClient, lists and fetches from that account in the credential's environment, links envelopes to that environment's web app, and validates without asking the deployment host. A seeded value of the wrong shape fails closed. OAuth syncs are unchanged. - RSA keys under 2048 bits are refused as invalid_credentials with copy that names the size, instead of jose's raw TypeError. - A short expires_in no longer makes every call re-mint: the cached lifetime floors at six minutes while staying a minute under the real expiry. - The connect modal shows the consent URL as a link with a copy button, and account_not_found says "pinned account" only when one was entered.
1 parent a7276db commit f843739

10 files changed

Lines changed: 327 additions & 37 deletions

File tree

‎apps/docs/content/docs/integrations/docusign-service-account.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ You only need to consent once per user and integration key, unless the user revo
7575

7676
Add a DocuSign block and select the saved credential in the account dropdown. Every DocuSign operation works with it the same way as with a connected account. Sim calls the eSignature REST API for the account it found when it last requested a token, in the environment stored on the credential.
7777

78-
Knowledge base syncs from DocuSign look up the account through your deployment's DocuSign OAuth host and always use the user's default account. Use a JWT Grant credential there only when its environment matches the deployment's, and connect a personal account when you need a non-default account.
78+
Knowledge base syncs from DocuSign can use the credential too. They read the same account in the same environment, and envelope links open in that environment's DocuSign web app.
7979

8080
## Fix connection errors
8181

‎apps/sim/app/workspace/[workspaceId]/integrations/components/connect-service-account-modal/client-credential-account-modal.tsx‎

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
ChipModalFooter,
1010
ChipModalHeader,
1111
ChipTextarea,
12+
CopyCodeButton,
1213
SecretInput,
1314
} from '@sim/emcn'
1415
import { createLogger } from '@sim/logger'
@@ -37,6 +38,12 @@ const logger = createLogger('ClientCredentialAccountModal')
3738

3839
type FieldValues = Partial<Record<ClientCredentialAccountFieldId, string>>
3940

41+
/** Submit-error copy, with an optional provider link the user must open to recover. */
42+
interface ClientCredentialErrorCopy {
43+
message: string
44+
link?: { href: string; label: string }
45+
}
46+
4047
const FALLBACK_ERROR_MESSAGE = "We couldn't save this credential. Try again in a moment."
4148

4249
/**
@@ -50,44 +57,65 @@ function messageForClientCredentialError(
5057
requiredFields: ClientCredentialAccountField[],
5158
values: FieldValues,
5259
fallbackMessage = FALLBACK_ERROR_MESSAGE
53-
): string {
60+
): ClientCredentialErrorCopy {
5461
if (isApiClientError(err) && err.code) {
5562
// Names the fields the *selected* auth method needed, so a JWT failure
5663
// doesn't tell the user to check a consumer secret they never entered.
5764
const fieldLabels = requiredFields.map((field) => field.label).join(', ')
5865
switch (err.code) {
5966
case 'invalid_credentials':
60-
return `We couldn't authenticate with those credentials. Check that the ${fieldLabels} all belong to the same ${descriptor.serviceLabel} app and that the app is authorized.`
67+
return {
68+
message: `We couldn't authenticate with those credentials. Check that the ${fieldLabels} all belong to the same ${descriptor.serviceLabel} app and that the app is authorized.`,
69+
}
6170
case 'permission_conflict':
62-
return 'Use the same permissions for connections to this application, or create a separate application for different permissions.'
71+
return {
72+
message:
73+
'Use the same permissions for connections to this application, or create a separate application for different permissions.',
74+
}
6375
case 'site_not_found': {
6476
// "host field" named a label no provider renders — Salesforce calls it
6577
// My Domain host, and Zoom/Box/Zoho Desk have no host field at all.
6678
const hostFieldLabel =
6779
descriptor.fields.find((field) => field.id === 'orgId')?.label ?? 'host'
68-
return `We couldn't find a ${descriptor.serviceLabel} account at that host. Check the ${hostFieldLabel} field and try again.`
80+
return {
81+
message: `We couldn't find a ${descriptor.serviceLabel} account at that host. Check the ${hostFieldLabel} field and try again.`,
82+
}
6983
}
7084
case 'provider_unavailable':
71-
return `We couldn't reach ${descriptor.serviceLabel} to verify these credentials. Try again in a moment.`
85+
return {
86+
message: `We couldn't reach ${descriptor.serviceLabel} to verify these credentials. Try again in a moment.`,
87+
}
7288
case 'consent_required': {
7389
const redirectUri = window.location.origin
7490
const consentUrl = descriptor.consentUrl?.(values, redirectUri)
7591
return consentUrl
76-
? `${descriptor.serviceLabel} needs the impersonated user's consent first. Add ${redirectUri} as a redirect URI on the integration key, open this link signed in as that user, accept, then try again: ${consentUrl}`
77-
: `${descriptor.serviceLabel} needs the impersonated user's consent first. See the setup guide.`
92+
? {
93+
message: `${descriptor.serviceLabel} needs the impersonated user's consent first. Add ${redirectUri} as a redirect URI on the integration key, then open the consent page signed in as that user, accept, and try again.`,
94+
link: { href: consentUrl, label: 'Open consent page' },
95+
}
96+
: {
97+
message: `${descriptor.serviceLabel} needs the impersonated user's consent first. See the setup guide.`,
98+
}
7899
}
79100
case 'account_not_found': {
101+
if (!values.orgId?.trim()) {
102+
return {
103+
message: `That user has no ${descriptor.serviceLabel} account in this environment. Check the environment and user ID.`,
104+
}
105+
}
80106
const accountFieldLabel =
81107
descriptor.fields.find((field) => field.id === 'orgId')?.label ?? 'account'
82-
return `That user isn't a member of the ${accountFieldLabel} you entered. Check it, or leave it blank to use the user's default account.`
108+
return {
109+
message: `That user isn't a member of the ${accountFieldLabel} you entered. Check it, or leave it blank to use the user's default account.`,
110+
}
83111
}
84112
case 'duplicate_display_name':
85-
return 'A credential with that name already exists in this workspace.'
113+
return { message: 'A credential with that name already exists in this workspace.' }
86114
default:
87-
return fallbackMessage
115+
return { message: fallbackMessage }
88116
}
89117
}
90-
return fallbackMessage
118+
return { message: fallbackMessage }
91119
}
92120

93121
function openDocs(url: string): void {
@@ -155,7 +183,7 @@ function ClientCredentialAccountModalForm({
155183
const [values, setValues] = useState<FieldValues>({})
156184
const [displayName, setDisplayName] = useState(initialDisplayName ?? '')
157185
const [description, setDescription] = useState(initialDescription ?? '')
158-
const [error, setError] = useState<string | null>(null)
186+
const [error, setError] = useState<ClientCredentialErrorCopy | null>(null)
159187

160188
const createCredential = useCreateScopedCredential()
161189
const updateCredential = useUpdateScopedCredential()
@@ -397,7 +425,26 @@ function ClientCredentialAccountModalForm({
397425
minHeight={80}
398426
/>
399427

400-
<ChipModalError>{error}</ChipModalError>
428+
<ChipModalError>
429+
{error && (
430+
<>
431+
{error.message}
432+
{error.link && (
433+
<span className='mt-1 flex items-center gap-1'>
434+
<a
435+
href={error.link.href}
436+
target='_blank'
437+
rel='noopener noreferrer'
438+
className='text-[var(--brand-secondary)] underline underline-offset-2'
439+
>
440+
{error.link.label}
441+
</a>
442+
<CopyCodeButton code={error.link.href} />
443+
</span>
444+
)}
445+
</>
446+
)}
447+
</ChipModalError>
401448
</ChipModalBody>
402449
<ChipModalFooter
403450
onCancel={() => onOpenChange(false)}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { knowledgeDocumentsUtilsMock } from '@sim/testing/mocks/knowledge-documents-utils.mock'
2+
import {
3+
knowledgeSecureFetchMock,
4+
knowledgeSecureFetchMockFns,
5+
} from '@sim/testing/mocks/knowledge-secure-fetch.mock'
6+
import { describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('@/lib/knowledge/documents/utils', () => knowledgeDocumentsUtilsMock)
9+
vi.mock('@/lib/knowledge/documents/secure-fetch.server', () => knowledgeSecureFetchMock)
10+
11+
import { docusignConnector } from '@/connectors/docusign/docusign'
12+
13+
const mockFetchWithRetry = knowledgeSecureFetchMockFns.mockFetchWithRetry
14+
15+
const DEFAULT_ACCOUNT = '99999999-0000-0000-0000-000000000001'
16+
const PINNED_ACCOUNT = '99999999-0000-0000-0000-000000000002'
17+
/** A production account, pinned, while the deployment's OAuth host stays on demo. */
18+
const BOUND_BASE = `https://na3.docusign.net/restapi/v2.1/accounts/${PINNED_ACCOUNT}`
19+
20+
function reply(body: unknown, status = 200): Response {
21+
return {
22+
ok: status >= 200 && status < 300,
23+
status,
24+
json: async () => body,
25+
text: async () => JSON.stringify(body),
26+
} as unknown as Response
27+
}
28+
29+
/** Answers userinfo with a demo default account and every envelope listing with one envelope. */
30+
function mockDocuSign() {
31+
mockFetchWithRetry.mockImplementation(async (url: string) => {
32+
if (url.endsWith('/oauth/userinfo')) {
33+
return reply({
34+
accounts: [
35+
{ account_id: DEFAULT_ACCOUNT, is_default: true, base_uri: 'https://demo.docusign.net' },
36+
],
37+
})
38+
}
39+
return reply({
40+
envelopes: [{ envelopeId: 'env-1', emailSubject: 'Offer', status: 'sent' }],
41+
totalSetSize: '1',
42+
})
43+
})
44+
}
45+
46+
describe('docusign connector account binding', () => {
47+
it("lists the JWT credential's pinned account in its own environment, never the deployment default", async () => {
48+
mockDocuSign()
49+
50+
const result = await docusignConnector.listDocuments('jwt-token', {}, undefined, {
51+
credentialInstanceUrl: BOUND_BASE,
52+
})
53+
54+
const urls = mockFetchWithRetry.mock.calls.map(([url]) => String(url))
55+
expect(urls.some((url) => url.endsWith('/oauth/userinfo'))).toBe(false)
56+
expect(urls[0].startsWith(`${BOUND_BASE}/envelopes?`)).toBe(true)
57+
expect(result.documents[0]?.sourceUrl).toBe('https://app.docusign.com/documents/details/env-1')
58+
})
59+
60+
it('resolves an OAuth token through userinfo on the deployment host as before', async () => {
61+
mockDocuSign()
62+
63+
const result = await docusignConnector.listDocuments('oauth-token', {}, undefined, {})
64+
65+
const urls = mockFetchWithRetry.mock.calls.map(([url]) => String(url))
66+
expect(urls[0]).toBe('https://account-d.docusign.com/oauth/userinfo')
67+
expect(
68+
urls[1].startsWith(`https://demo.docusign.net/restapi/v2.1/accounts/${DEFAULT_ACCOUNT}/`)
69+
).toBe(true)
70+
expect(result.documents[0]?.sourceUrl).toBe(
71+
'https://appdemo.docusign.com/documents/details/env-1'
72+
)
73+
})
74+
75+
it('validates a JWT credential without asking the deployment host', async () => {
76+
mockDocuSign()
77+
78+
await expect(
79+
docusignConnector.validateConfig('jwt-token', {}, { credentialInstanceUrl: BOUND_BASE })
80+
).resolves.toEqual({ valid: true })
81+
expect(mockFetchWithRetry).not.toHaveBeenCalled()
82+
})
83+
84+
it.each([
85+
'https://evil.example.com/restapi/v2.1/accounts/x',
86+
`http://na3.docusign.net/restapi/v2.1/accounts/${PINNED_ACCOUNT}`,
87+
])('fails closed on a seeded account base of the wrong shape: %s', async (bound) => {
88+
mockDocuSign()
89+
90+
await expect(
91+
docusignConnector.listDocuments('jwt-token', {}, undefined, { credentialInstanceUrl: bound })
92+
).rejects.toThrow('DocuSign credential account URL is invalid')
93+
await expect(
94+
docusignConnector.validateConfig('jwt-token', {}, { credentialInstanceUrl: bound })
95+
).resolves.toMatchObject({ valid: false })
96+
expect(mockFetchWithRetry).not.toHaveBeenCalled()
97+
})
98+
})

‎apps/sim/connectors/docusign/docusign.ts‎

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { fetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
44
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
5-
import { getDocusignOAuthUrl, getDocusignWebBase } from '@/lib/oauth/docusign'
5+
import {
6+
getDocusignOAuthUrl,
7+
getDocusignWebBase,
8+
parseDocuSignAccountApiBase,
9+
} from '@/lib/oauth/docusign'
610
import { docusignConnectorMeta } from '@/connectors/docusign/meta'
711
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
812
import { parseTagDate } from '@/connectors/utils'
@@ -34,6 +38,8 @@ interface DocuSignUserInfo {
3438
interface ResolvedAccount {
3539
accountId: string
3640
baseUri: string
41+
/** Web app origin for envelope links; set only for a credential-bound account. */
42+
webBase?: string
3743
}
3844

3945
interface DocuSignSigner {
@@ -138,6 +144,22 @@ function computeLookbackDays(
138144
return Math.min(incremental, baseline)
139145
}
140146

147+
/**
148+
* The account a JWT Grant credential is bound to, seeded into the sync context
149+
* from its token. Its environment and pinned account come from the credential,
150+
* so the deployment's OAuth host is never asked. A seeded value of the wrong
151+
* shape fails the run rather than falling back to the user's default account.
152+
*/
153+
function credentialBoundAccount(
154+
syncContext: Record<string, unknown> | undefined
155+
): ResolvedAccount | undefined {
156+
const bound = syncContext?.credentialInstanceUrl
157+
if (bound === undefined) return undefined
158+
const account = parseDocuSignAccountApiBase(bound)
159+
if (!account) throw new Error('DocuSign credential account URL is invalid')
160+
return account
161+
}
162+
141163
/**
142164
* Resolves and caches the user's DocuSign account ID and base URI in the sync context.
143165
* The userinfo lookup is expensive and identical across every page of a sync run, so it is
@@ -151,6 +173,11 @@ async function resolveAccount(
151173
const cacheKey = '_docusignAccount'
152174
const cached = syncContext?.[cacheKey] as ResolvedAccount | undefined
153175
if (cached) return cached
176+
const bound = credentialBoundAccount(syncContext)
177+
if (bound) {
178+
if (syncContext) syncContext[cacheKey] = bound
179+
return bound
180+
}
154181

155182
const response = await fetchWithRetry(
156183
getDocusignOAuthUrl('/oauth/userinfo'),
@@ -211,9 +238,12 @@ function buildContentHash(envelope: DocuSignEnvelope): string {
211238
/**
212239
* Builds a DocuSign web console link to the envelope, when an envelope ID is present.
213240
*/
214-
function buildSourceUrl(envelopeId: string | undefined): string | undefined {
241+
function buildSourceUrl(
242+
envelopeId: string | undefined,
243+
account: ResolvedAccount
244+
): string | undefined {
215245
if (!envelopeId) return undefined
216-
return `${getDocusignWebBase()}/documents/details/${envelopeId}`
246+
return `${account.webBase ?? getDocusignWebBase()}/documents/details/${envelopeId}`
217247
}
218248

219249
/**
@@ -286,14 +316,14 @@ function buildMetadata(envelope: DocuSignEnvelope): Record<string, unknown> {
286316
* Creates a lightweight document stub from an envelope list entry. No per-envelope API
287317
* calls are made — content is fetched lazily by getDocument for new/changed envelopes only.
288318
*/
289-
function envelopeToStub(envelope: DocuSignEnvelope): ExternalDocument {
319+
function envelopeToStub(envelope: DocuSignEnvelope, account: ResolvedAccount): ExternalDocument {
290320
return {
291321
externalId: envelope.envelopeId ?? '',
292322
title: envelope.emailSubject?.trim() || 'Untitled DocuSign Envelope',
293323
content: '',
294324
contentDeferred: true,
295325
mimeType: 'text/plain',
296-
sourceUrl: buildSourceUrl(envelope.envelopeId),
326+
sourceUrl: buildSourceUrl(envelope.envelopeId, account),
297327
contentHash: buildContentHash(envelope),
298328
metadata: buildMetadata(envelope),
299329
}
@@ -505,7 +535,7 @@ export const docusignConnector: ConnectorConfig = {
505535
*/
506536
let capped = envelopes.length < rawEnvelopes.length
507537

508-
let documents = envelopes.map(envelopeToStub)
538+
let documents = envelopes.map((envelope) => envelopeToStub(envelope, account))
509539
if (documents.length > remaining) {
510540
documents = documents.slice(0, remaining)
511541
capped = true
@@ -597,7 +627,7 @@ export const docusignConnector: ConnectorConfig = {
597627
content,
598628
contentDeferred: false,
599629
mimeType: 'text/plain',
600-
sourceUrl: buildSourceUrl(envelope.envelopeId),
630+
sourceUrl: buildSourceUrl(envelope.envelopeId, account),
601631
contentHash: buildContentHash(envelope),
602632
metadata: buildMetadata(envelope),
603633
}
@@ -618,15 +648,18 @@ export const docusignConnector: ConnectorConfig = {
618648

619649
validateConfig: async (
620650
accessToken: string,
621-
sourceConfig: Record<string, unknown>
651+
sourceConfig: Record<string, unknown>,
652+
syncContext?: Record<string, unknown>
622653
): Promise<{ valid: boolean; error?: string }> => {
623654
const maxEnvelopes = sourceConfig.maxEnvelopes as string | undefined
624655
if (maxEnvelopes && (Number.isNaN(Number(maxEnvelopes)) || Number(maxEnvelopes) < 0)) {
625656
return { valid: false, error: 'Max envelopes must be a non-negative number' }
626657
}
627658

628659
try {
629-
await resolveAccount(accessToken, undefined, VALIDATE_RETRY_OPTIONS)
660+
if (!credentialBoundAccount(syncContext)) {
661+
await resolveAccount(accessToken, undefined, VALIDATE_RETRY_OPTIONS)
662+
}
630663
return { valid: true }
631664
} catch (error) {
632665
const message = getErrorMessage(error, 'Failed to validate configuration')

0 commit comments

Comments
 (0)