Skip to content

Commit 080818d

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(monday): address OAuth review feedback
1 parent 35c6883 commit 080818d

6 files changed

Lines changed: 147 additions & 157 deletions

File tree

apps/sim/app/api/auth/oauth/utils.test.ts

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -142,42 +142,19 @@ describe('OAuth Utils', () => {
142142
refreshToken: 'new-refresh-token',
143143
})
144144

145-
mockUpdateChain()
145+
const { mockSet } = mockUpdateChain()
146146

147147
const result = await refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
148148

149149
expect(mockRefreshOAuthToken).toHaveBeenCalledWith('google', 'refresh-token')
150-
expect(mockDb.update).toHaveBeenCalled()
151-
expect(result).toEqual({ accessToken: 'new-token', refreshed: true })
152-
})
153-
154-
it('persists a rotated Monday refresh token with the refreshed access token', async () => {
155-
const credential = {
156-
id: 'monday-credential-id',
157-
accessToken: 'expired-monday-token',
158-
refreshToken: 'old-monday-refresh-token',
159-
accessTokenExpiresAt: new Date(Date.now() - 60_000),
160-
providerId: 'monday',
161-
}
162-
mockRefreshOAuthToken.mockResolvedValueOnce({
163-
ok: true,
164-
accessToken: 'new-monday-token',
165-
expiresIn: 3600,
166-
refreshToken: 'rotated-monday-refresh-token',
167-
})
168-
const { mockSet } = mockUpdateChain()
169-
170-
const result = await refreshTokenIfNeeded('request-id', credential, credential.id)
171-
172-
expect(mockRefreshOAuthToken).toHaveBeenCalledWith('monday', 'old-monday-refresh-token')
173150
expect(mockSet).toHaveBeenCalledWith(
174151
expect.objectContaining({
175-
accessToken: 'new-monday-token',
176-
refreshToken: 'rotated-monday-refresh-token',
152+
accessToken: 'new-token',
153+
refreshToken: 'new-refresh-token',
177154
accessTokenExpiresAt: expect.any(Date),
178155
})
179156
)
180-
expect(result).toEqual({ accessToken: 'new-monday-token', refreshed: true })
157+
expect(result).toEqual({ accessToken: 'new-token', refreshed: true })
181158
})
182159

183160
it('should handle refresh token error', async () => {

apps/sim/lib/credential-groups/standard-oauth-provider.test.ts

Lines changed: 0 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -44,32 +44,6 @@ vi.mock('@/lib/auth/connectors/managed-oauth', () => ({
4444
},
4545
}
4646
}
47-
if (providerId === 'monday') {
48-
return {
49-
providerId,
50-
clientId: 'monday-client-1',
51-
clientSecret: 'monday-secret-1',
52-
authorizationUrl: 'https://auth.monday.com/oauth2/authorize',
53-
tokenUrl: 'https://auth.monday.com/oauth_ms/oauth/token',
54-
redirectURI: 'https://sim.example.com/api/auth/oauth2/callback/monday',
55-
scopes: ['boards:read', 'me:read'],
56-
responseType: 'code',
57-
authentication: 'post',
58-
getToken: mockGetToken,
59-
managedOAuth: {
60-
additionalScopes: [],
61-
requiresRefreshToken: true,
62-
pkce: true,
63-
nonceVerification: 'state_only',
64-
includeLoginHint: false,
65-
getAuthorizationAppId: (clientId: string) => `monday:${clientId}`,
66-
verifyIdentity: mockVerifyIdentity,
67-
hasRequiredScopes: (granted: string[], required: string[]) =>
68-
required.every((scope) => granted.includes(scope)),
69-
isTerminalRefreshError: (errorCode: string | undefined) => errorCode === 'invalid_grant',
70-
},
71-
}
72-
}
7347
if (providerId === 'jira') {
7448
return {
7549
providerId,
@@ -108,7 +82,6 @@ import { createStandardOAuthCredentialGroupProviderAdapter } from '@/lib/credent
10882

10983
const adapter = createStandardOAuthCredentialGroupProviderAdapter('google-calendar')
11084
const jiraAdapter = createStandardOAuthCredentialGroupProviderAdapter('jira')
111-
const mondayAdapter = createStandardOAuthCredentialGroupProviderAdapter('monday')
11285

11386
function buildContext(): CredentialGroupOAuthContext {
11487
return {
@@ -237,82 +210,6 @@ describe('standard OAuth Credential Group provider', () => {
237210
})
238211
})
239212

240-
it('uses PKCE and persists expiring rotating credentials for managed Monday OAuth', async () => {
241-
const requiredScopes = ['boards:read', 'me:read']
242-
const context: CredentialGroupOAuthContext = {
243-
...buildContext(),
244-
option: {
245-
...buildContext().option,
246-
provider: 'monday',
247-
label: 'Monday.com',
248-
authorizationAppId: 'monday:monday-client-1',
249-
requiredScopes,
250-
},
251-
}
252-
const policy = await mondayAdapter.getPolicy(context.option, {
253-
workspaceId: context.workspaceId,
254-
credentialGroupId: context.credentialGroupId,
255-
})
256-
const prepared = await mondayAdapter.prepareAuthorization(context, policy)
257-
const authorizationUrl = new URL(
258-
await prepared.buildAuthorizationUrl({ state: 'monday-state-1', nonce: 'nonce-ignored' })
259-
)
260-
261-
expect(mondayAdapter.requiresRefreshToken).toBe(true)
262-
expect(prepared.codeVerifier).toHaveLength(86)
263-
expect(authorizationUrl.searchParams.get('code_challenge_method')).toBe('S256')
264-
expect(authorizationUrl.searchParams.get('code_challenge')).toBeTruthy()
265-
266-
const accessTokenExpiresAt = new Date('2026-08-14T01:00:00Z')
267-
mockGetToken.mockResolvedValueOnce({
268-
tokenType: 'Bearer',
269-
accessToken: 'monday-access-1',
270-
refreshToken: 'monday-refresh-1',
271-
accessTokenExpiresAt,
272-
scopes: requiredScopes,
273-
})
274-
mockVerifyIdentity.mockResolvedValueOnce({
275-
providerSubjectId: 'monday-user-1',
276-
providerTenantId: null,
277-
email: 'person@example.com',
278-
emailVerified: true,
279-
grantedScopes: requiredScopes,
280-
})
281-
282-
const grant = await mondayAdapter.exchangeAndVerify({
283-
context,
284-
attempt: {
285-
state: 'monday-state-1',
286-
provider: 'monday',
287-
nonceHash: 'unused-for-state-bound-provider',
288-
enrollmentId: context.enrollmentId,
289-
credentialGroupId: context.credentialGroupId,
290-
optionId: context.option.id,
291-
authorizationAppId: policy.authorizationAppId,
292-
scopeVersion: policy.scopeVersion,
293-
requiredScopes,
294-
redirectUri: prepared.redirectUri,
295-
codeVerifier: prepared.codeVerifier,
296-
invitationToken: 'invitation-1',
297-
createdAt: Date.now(),
298-
},
299-
code: 'monday-code-1',
300-
policy,
301-
})
302-
303-
expect(mockGetToken).toHaveBeenLastCalledWith({
304-
code: 'monday-code-1',
305-
redirectURI: 'https://sim.example.com/api/auth/oauth2/callback/monday',
306-
codeVerifier: prepared.codeVerifier,
307-
})
308-
expect(grant).toMatchObject({
309-
accessToken: 'monday-access-1',
310-
refreshToken: 'monday-refresh-1',
311-
accessTokenExpiresAt,
312-
grantedScopes: requiredScopes,
313-
})
314-
})
315-
316213
it('rejects a different invited email', async () => {
317214
mockVerifyIdentity.mockResolvedValueOnce({
318215
providerSubjectId: 'google-sub-2',

apps/sim/lib/credentials/managed-oauth.test.ts

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22
* @vitest-environment node
33
*/
44
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

77
const mocks = vi.hoisted(() => ({
88
getBilling: vi.fn(),
99
isAvailable: vi.fn(),
1010
getAdapter: vi.fn(),
1111
decryptSecret: vi.fn(),
12+
encryptSecret: vi.fn(),
1213
}))
1314

1415
vi.mock('@/lib/billing/core/workspace-access', () => ({
@@ -25,15 +26,44 @@ vi.mock('@/lib/credential-groups/provider-registry', () => ({
2526

2627
vi.mock('@/lib/core/security/encryption', () => ({
2728
decryptSecret: mocks.decryptSecret,
28-
encryptSecret: vi.fn(),
29+
encryptSecret: mocks.encryptSecret,
2930
}))
3031

3132
import { resolveManagedOAuthToken } from '@/lib/credentials/managed-oauth'
3233

34+
function mondayCredentialRow() {
35+
return {
36+
id: 'credential-1',
37+
workspaceId: 'workspace-1',
38+
type: 'managed_oauth',
39+
providerId: 'monday',
40+
authorizationAppId: 'monday:monday-client-1',
41+
managedOauthScopeVersion: 1,
42+
managedOauthStatus: 'active',
43+
grantedScopes: ['boards:read', 'me:read'],
44+
encryptedOauthTokenSet: 'encrypted-token-set',
45+
accessTokenExpiresAt: new Date('2026-09-01T11:00:00.000Z'),
46+
refreshTokenExpiresAt: null,
47+
credentialGroupId: 'group-1',
48+
credentialGroupEnrollmentId: 'enrollment-1',
49+
}
50+
}
51+
52+
function mondayTokenResolutionParams() {
53+
return {
54+
credentialId: 'credential-1',
55+
workspaceId: 'workspace-1',
56+
expectedProviderId: 'monday',
57+
requiredScopes: ['boards:read', 'me:read'],
58+
}
59+
}
60+
3361
describe('managed OAuth token resolution', () => {
3462
beforeEach(() => {
3563
vi.clearAllMocks()
3664
resetDbChainMock()
65+
vi.useFakeTimers()
66+
vi.setSystemTime(new Date('2026-09-01T12:00:00.000Z'))
3767
mocks.getBilling.mockResolvedValue({ plan: 'enterprise' })
3868
mocks.isAvailable.mockResolvedValue(true)
3969
mocks.decryptSecret.mockResolvedValue({
@@ -53,6 +83,10 @@ describe('managed OAuth token resolution', () => {
5383
})
5484
})
5585

86+
afterEach(() => {
87+
vi.useRealTimers()
88+
})
89+
5690
it('uses a non-expiring Slack access token without entering refresh', async () => {
5791
dbChainMockFns.limit.mockResolvedValueOnce([
5892
{
@@ -80,4 +114,98 @@ describe('managed OAuth token resolution', () => {
80114
).resolves.toEqual({ accessToken: 'xoxp-slack-token', refreshed: false })
81115
expect(dbChainMockFns.transaction).not.toHaveBeenCalled()
82116
})
117+
118+
it('refreshes an expired Monday credential and persists its rotated token set', async () => {
119+
const row = mondayCredentialRow()
120+
dbChainMockFns.limit.mockResolvedValueOnce([row]).mockResolvedValueOnce([row])
121+
dbChainMockFns.returning.mockResolvedValueOnce([{ id: row.id }])
122+
mocks.decryptSecret.mockResolvedValue({
123+
decrypted: JSON.stringify({
124+
type: 'managed-oauth-token-set',
125+
version: 1,
126+
tokenType: 'Bearer',
127+
accessToken: 'expired-access-token',
128+
refreshToken: 'old-refresh-token',
129+
}),
130+
})
131+
mocks.encryptSecret.mockResolvedValue({ encrypted: 'encrypted-rotated-token-set' })
132+
const refreshToken = vi.fn().mockResolvedValue({
133+
ok: true,
134+
accessToken: 'new-access-token',
135+
refreshToken: 'rotated-refresh-token',
136+
expiresIn: 3600,
137+
})
138+
mocks.getAdapter.mockReturnValue({
139+
getPolicy: vi.fn().mockResolvedValue({
140+
authorizationAppId: row.authorizationAppId,
141+
scopeVersion: 1,
142+
}),
143+
hasRequiredScopes: vi.fn().mockReturnValue(true),
144+
refreshToken,
145+
isTerminalRefreshError: vi.fn().mockReturnValue(false),
146+
})
147+
148+
await expect(resolveManagedOAuthToken(mondayTokenResolutionParams())).resolves.toEqual({
149+
accessToken: 'new-access-token',
150+
refreshed: true,
151+
})
152+
153+
expect(refreshToken).toHaveBeenCalledWith('old-refresh-token')
154+
const [serializedTokenSet] = mocks.encryptSecret.mock.calls[0] as [string]
155+
expect(JSON.parse(serializedTokenSet)).toEqual({
156+
type: 'managed-oauth-token-set',
157+
version: 1,
158+
tokenType: 'Bearer',
159+
accessToken: 'new-access-token',
160+
refreshToken: 'rotated-refresh-token',
161+
})
162+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
163+
expect.objectContaining({
164+
encryptedOauthTokenSet: 'encrypted-rotated-token-set',
165+
accessTokenExpiresAt: new Date('2026-09-01T13:00:00.000Z'),
166+
lastRefreshedAt: new Date('2026-09-01T12:00:00.000Z'),
167+
})
168+
)
169+
})
170+
171+
it('marks an expired Monday credential for reauthorization after a terminal refresh error', async () => {
172+
const row = mondayCredentialRow()
173+
dbChainMockFns.limit.mockResolvedValueOnce([row]).mockResolvedValueOnce([row])
174+
mocks.decryptSecret.mockResolvedValue({
175+
decrypted: JSON.stringify({
176+
type: 'managed-oauth-token-set',
177+
version: 1,
178+
tokenType: 'Bearer',
179+
accessToken: 'expired-access-token',
180+
refreshToken: 'old-refresh-token',
181+
}),
182+
})
183+
const refreshToken = vi.fn().mockResolvedValue({
184+
ok: false,
185+
errorCode: 'invalid_grant',
186+
message: 'Refresh token rejected',
187+
})
188+
const isTerminalRefreshError = vi.fn().mockReturnValue(true)
189+
mocks.getAdapter.mockReturnValue({
190+
getPolicy: vi.fn().mockResolvedValue({
191+
authorizationAppId: row.authorizationAppId,
192+
scopeVersion: 1,
193+
}),
194+
hasRequiredScopes: vi.fn().mockReturnValue(true),
195+
refreshToken,
196+
isTerminalRefreshError,
197+
})
198+
199+
await expect(resolveManagedOAuthToken(mondayTokenResolutionParams())).rejects.toMatchObject({
200+
code: 'MANAGED_CREDENTIAL_NEEDS_REAUTH',
201+
statusCode: 401,
202+
})
203+
204+
expect(refreshToken).toHaveBeenCalledWith('old-refresh-token')
205+
expect(isTerminalRefreshError).toHaveBeenCalledWith('invalid_grant')
206+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
207+
expect.objectContaining({ managedOauthStatus: 'needs_reauth' })
208+
)
209+
expect(mocks.encryptSecret).not.toHaveBeenCalled()
210+
})
83211
})

apps/sim/lib/oauth/monday.test.ts

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* @vitest-environment node
33
*/
44
import { afterEach, describe, expect, it, vi } from 'vitest'
5-
import { DEFAULT_MAX_ERROR_BODY_BYTES } from '@/lib/core/utils/stream-limits'
65
import {
76
exchangeMondayAuthorizationCode,
87
MONDAY_OAUTH_TOKEN_URL,
@@ -94,6 +93,15 @@ describe('Monday OAuth 2.1', () => {
9493
expect(expiresAt).toEqual(new Date(jwtExpirySeconds * 1000))
9594
})
9695

96+
it('preserves an expired JWT expiration so the credential refreshes immediately', () => {
97+
const now = new Date('2026-09-01T12:00:00.000Z')
98+
const jwtExpirySeconds = Math.floor(now.getTime() / 1000) - 60
99+
100+
expect(
101+
resolveMondayAccessTokenExpiresAt(unsignedJwt({ exp: jwtExpirySeconds }), 3600, now)
102+
).toEqual(new Date(jwtExpirySeconds * 1000))
103+
})
104+
97105
it('falls back to expires_in and then one hour for an opaque access token', () => {
98106
const now = new Date('2026-09-01T12:00:00.000Z')
99107

@@ -147,21 +155,4 @@ describe('Monday OAuth 2.1', () => {
147155
expect((error as Error).message).toBe('Monday OAuth token exchange failed with HTTP 400')
148156
expect((error as Error).message).not.toContain(providerSecret)
149157
})
150-
151-
it('bounds the token endpoint response', async () => {
152-
vi.stubGlobal(
153-
'fetch',
154-
vi.fn().mockResolvedValue(new Response('x'.repeat(DEFAULT_MAX_ERROR_BODY_BYTES + 1)))
155-
)
156-
157-
await expect(
158-
exchangeMondayAuthorizationCode({
159-
clientId: 'client-id',
160-
clientSecret: 'client-secret',
161-
code: 'authorization-code',
162-
codeVerifier: 'pkce-verifier',
163-
redirectUri: 'https://www.sim.ai/api/auth/oauth2/callback/monday',
164-
})
165-
).rejects.toThrow('exceeds maximum size')
166-
})
167158
})

apps/sim/lib/oauth/monday.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export function resolveMondayAccessTokenExpiresAt(
5353
): Date {
5454
try {
5555
const { exp } = decodeJwt(accessToken)
56-
if (typeof exp === 'number' && Number.isFinite(exp) && exp * 1000 > now.getTime()) {
56+
if (typeof exp === 'number' && Number.isFinite(exp)) {
5757
const expiresAt = new Date(exp * 1000)
5858
if (!Number.isNaN(expiresAt.getTime())) return expiresAt
5959
}

0 commit comments

Comments
 (0)