Skip to content

Commit a07e652

Browse files
committed
fix(security): enforce workspace scope on workflow middleware and validate shopify shop domain
- validateWorkflowAccess now rejects workspace-scoped API keys whose workspaceId doesn't match the workflow's workspace, closing a boundary leak across /api/workflows/[id]/{log,paused,status} and /api/resume/[workflowId]/[executionId]/[contextId] - shopify authorize route now validates the resolved shop domain against shopifyShopDomainSchema before proceeding - adds middleware tests covering workspace/personal/session auth paths
1 parent 773720e commit a07e652

3 files changed

Lines changed: 148 additions & 1 deletion

File tree

apps/sim/app/api/auth/shopify/authorize/route.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { createLogger } from '@sim/logger'
22
import { generateId } from '@sim/utils/id'
33
import { type NextRequest, NextResponse } from 'next/server'
4-
import { shopifyAuthorizeQuerySchema } from '@/lib/api/contracts/oauth-connections'
4+
import {
5+
shopifyAuthorizeQuerySchema,
6+
shopifyShopDomainSchema,
7+
} from '@/lib/api/contracts/oauth-connections'
58
import { getSession } from '@/lib/auth'
69
import { env } from '@/lib/core/config/env'
710
import { getBaseUrl } from '@/lib/core/utils/urls'
@@ -161,6 +164,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
161164
cleanShop = `${cleanShop.replace('.myshopify.com', '')}.myshopify.com`
162165
}
163166

167+
if (!shopifyShopDomainSchema.safeParse(cleanShop).success) {
168+
logger.warn('Rejected invalid Shopify shop domain', { shop: shopDomain })
169+
return NextResponse.json({ error: 'Invalid Shopify shop domain' }, { status: 400 })
170+
}
171+
164172
const baseUrl = getBaseUrl()
165173
const redirectUri = `${baseUrl}/api/auth/oauth2/callback/shopify`
166174

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* Tests for workflow access middleware — focused on the workspace-scoped
3+
* API key boundary check in the `requireDeployment=false` branch.
4+
*
5+
* @vitest-environment node
6+
*/
7+
8+
import {
9+
hybridAuthMockFns,
10+
workflowAuthzMock,
11+
workflowAuthzMockFns,
12+
workflowsUtilsMock,
13+
workflowsUtilsMockFns,
14+
} from '@sim/testing'
15+
import { NextRequest } from 'next/server'
16+
import { beforeEach, describe, expect, it, vi } from 'vitest'
17+
18+
vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
19+
vi.mock('@sim/workflow-authz', () => workflowAuthzMock)
20+
vi.mock('@/lib/api-key/service', () => ({
21+
authenticateApiKeyFromHeader: vi.fn(),
22+
updateApiKeyLastUsed: vi.fn(),
23+
}))
24+
25+
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
26+
27+
function makeRequest() {
28+
return new NextRequest(new URL('https://example.com/api/workflows/wf-1/log'))
29+
}
30+
31+
describe('validateWorkflowAccess (requireDeployment=false)', () => {
32+
beforeEach(() => {
33+
vi.clearAllMocks()
34+
workflowsUtilsMockFns.mockGetWorkflowById.mockResolvedValue({
35+
id: 'wf-1',
36+
workspaceId: 'ws-A',
37+
isDeployed: true,
38+
})
39+
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
40+
allowed: true,
41+
status: 200,
42+
workflow: { id: 'wf-1', workspaceId: 'ws-A' },
43+
})
44+
})
45+
46+
it('rejects a workspace-scoped API key issued for a different workspace', async () => {
47+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
48+
success: true,
49+
userId: 'user-1',
50+
authType: 'api_key',
51+
apiKeyType: 'workspace',
52+
workspaceId: 'ws-B',
53+
})
54+
55+
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
56+
57+
expect(result.error).toEqual({
58+
message: 'API key is not authorized for this workspace',
59+
status: 403,
60+
})
61+
expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
62+
})
63+
64+
it('allows a workspace-scoped API key issued for the matching workspace', async () => {
65+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
66+
success: true,
67+
userId: 'user-1',
68+
authType: 'api_key',
69+
apiKeyType: 'workspace',
70+
workspaceId: 'ws-A',
71+
})
72+
73+
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
74+
75+
expect(result.error).toBeUndefined()
76+
expect(result.workflow).toBeDefined()
77+
expect(result.auth?.workspaceId).toBe('ws-A')
78+
expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({
79+
workflowId: 'wf-1',
80+
userId: 'user-1',
81+
action: 'read',
82+
})
83+
})
84+
85+
it('allows a personal API key regardless of workspaceId on the auth result', async () => {
86+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
87+
success: true,
88+
userId: 'user-1',
89+
authType: 'api_key',
90+
apiKeyType: 'personal',
91+
workspaceId: 'ws-B',
92+
})
93+
94+
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
95+
96+
expect(result.error).toBeUndefined()
97+
expect(result.workflow).toBeDefined()
98+
})
99+
100+
it('allows session auth (no apiKeyType) when workspace permission grants access', async () => {
101+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
102+
success: true,
103+
userId: 'user-1',
104+
authType: 'session',
105+
})
106+
107+
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
108+
109+
expect(result.error).toBeUndefined()
110+
expect(result.workflow).toBeDefined()
111+
})
112+
113+
it('still enforces workspace-permission rejection for personal keys', async () => {
114+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
115+
success: true,
116+
userId: 'user-1',
117+
authType: 'api_key',
118+
apiKeyType: 'personal',
119+
})
120+
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
121+
allowed: false,
122+
status: 403,
123+
message: 'Access denied',
124+
})
125+
126+
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
127+
128+
expect(result.error).toEqual({ message: 'Access denied', status: 403 })
129+
})
130+
})

apps/sim/app/api/workflows/middleware.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,15 @@ export async function validateWorkflowAccess(
5454
}
5555
}
5656

57+
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflow.workspaceId) {
58+
return {
59+
error: {
60+
message: 'API key is not authorized for this workspace',
61+
status: 403,
62+
},
63+
}
64+
}
65+
5766
const authorization = await authorizeWorkflowByWorkspacePermission({
5867
workflowId,
5968
userId: auth.userId,

0 commit comments

Comments
 (0)