improvement(credentials-security): use clear credentials sharing helper, fix google sheets block url split bug

apps/sim/app/api/auth/oauth/token/route.test.ts

@@ -10,6 +10,8 @@ describe('OAuth Token API Routes', () => {
   const mockGetUserId = vi.fn()
   const mockGetCredential = vi.fn()
   const mockRefreshTokenIfNeeded = vi.fn()
+  const mockAuthorizeCredentialUse = vi.fn()
+  const mockCheckHybridAuth = vi.fn()
 
   const mockLogger = {
     info: vi.fn(),
@@ -37,6 +39,14 @@ describe('OAuth Token API Routes', () => {
     vi.doMock('@/lib/logs/console/logger', () => ({
       createLogger: vi.fn().mockReturnValue(mockLogger),
     }))
+
+    vi.doMock('@/lib/auth/credential-access', () => ({
+      authorizeCredentialUse: mockAuthorizeCredentialUse,
+    }))
+
+    vi.doMock('@/lib/auth/hybrid', () => ({
+      checkHybridAuth: mockCheckHybridAuth,
+    }))
   })
 
   afterEach(() => {
@@ -48,7 +58,12 @@ describe('OAuth Token API Routes', () => {
    */
   describe('POST handler', () => {
     it('should return access token successfully', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: true,
+        authType: 'session',
+        requesterUserId: 'test-user-id',
+        credentialOwnerUserId: 'owner-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: 'test-token',
@@ -78,14 +93,18 @@ describe('OAuth Token API Routes', () => {
       expect(data).toHaveProperty('accessToken', 'fresh-token')
 
       // Verify mocks were called correctly
-      // POST no longer calls getUserId; token resolution uses credential owner.
-      expect(mockGetUserId).not.toHaveBeenCalled()
+      expect(mockAuthorizeCredentialUse).toHaveBeenCalled()
       expect(mockGetCredential).toHaveBeenCalled()
       expect(mockRefreshTokenIfNeeded).toHaveBeenCalled()
     })
 
     it('should handle workflowId for server-side authentication', async () => {
-      mockGetUserId.mockResolvedValueOnce('workflow-owner-id')
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: true,
+        authType: 'internal_jwt',
+        requesterUserId: 'workflow-owner-id',
+        credentialOwnerUserId: 'workflow-owner-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: 'test-token',
@@ -111,8 +130,7 @@ describe('OAuth Token API Routes', () => {
       expect(response.status).toBe(200)
       expect(data).toHaveProperty('accessToken', 'fresh-token')
 
-      // POST no longer calls getUserId; still refreshes successfully
-      expect(mockGetUserId).not.toHaveBeenCalled()
+      expect(mockAuthorizeCredentialUse).toHaveBeenCalled()
       expect(mockGetCredential).toHaveBeenCalled()
     })
 
@@ -130,8 +148,10 @@ describe('OAuth Token API Routes', () => {
     })
 
     it('should handle authentication failure', async () => {
-      // Authentication failure no longer applies to POST path; treat as refresh failure via missing owner
-      mockGetUserId.mockResolvedValueOnce(undefined)
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: false,
+        error: 'Authentication required',
+      })
 
       const req = createMockRequest('POST', {
         credentialId: 'credential-id',
@@ -142,12 +162,12 @@ describe('OAuth Token API Routes', () => {
       const response = await POST(req)
       const data = await response.json()
 
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(403)
       expect(data).toHaveProperty('error')
     })
 
     it('should handle workflow not found', async () => {
-      mockGetUserId.mockResolvedValueOnce(undefined)
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({ ok: false, error: 'Workflow not found' })
 
       const req = createMockRequest('POST', {
         credentialId: 'credential-id',
@@ -159,13 +179,16 @@ describe('OAuth Token API Routes', () => {
       const response = await POST(req)
       const data = await response.json()
 
-      // With owner-based resolution, missing workflowId no longer matters.
-      // If credential not found via owner lookup, returns 404 accordingly
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(403)
     })
 
     it('should handle credential not found', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: true,
+        authType: 'session',
+        requesterUserId: 'test-user-id',
+        credentialOwnerUserId: 'owner-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce(undefined)
 
       const req = createMockRequest('POST', {
@@ -177,12 +200,17 @@ describe('OAuth Token API Routes', () => {
       const response = await POST(req)
       const data = await response.json()
 
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(401)
       expect(data).toHaveProperty('error')
     })
 
     it('should handle token refresh failure', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: true,
+        authType: 'session',
+        requesterUserId: 'test-user-id',
+        credentialOwnerUserId: 'owner-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: 'test-token',
@@ -211,7 +239,11 @@ describe('OAuth Token API Routes', () => {
    */
   describe('GET handler', () => {
     it('should return access token successfully', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockCheckHybridAuth.mockResolvedValueOnce({
+        success: true,
+        authType: 'session',
+        userId: 'test-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: 'test-token',
@@ -236,7 +268,7 @@ describe('OAuth Token API Routes', () => {
       expect(response.status).toBe(200)
       expect(data).toHaveProperty('accessToken', 'fresh-token')
 
-      expect(mockGetUserId).toHaveBeenCalledWith(mockRequestId)
+      expect(mockCheckHybridAuth).toHaveBeenCalled()
       expect(mockGetCredential).toHaveBeenCalledWith(mockRequestId, 'credential-id', 'test-user-id')
       expect(mockRefreshTokenIfNeeded).toHaveBeenCalled()
     })
@@ -255,7 +287,10 @@ describe('OAuth Token API Routes', () => {
     })
 
     it('should handle authentication failure', async () => {
-      mockGetUserId.mockResolvedValueOnce(undefined)
+      mockCheckHybridAuth.mockResolvedValueOnce({
+        success: false,
+        error: 'Authentication required',
+      })
 
       const req = new Request(
         'http://localhost:3000/api/auth/oauth/token?credentialId=credential-id'
@@ -266,12 +301,16 @@ describe('OAuth Token API Routes', () => {
       const response = await GET(req as any)
       const data = await response.json()
 
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(401)
       expect(data).toHaveProperty('error')
     })
 
     it('should handle credential not found', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockCheckHybridAuth.mockResolvedValueOnce({
+        success: true,
+        authType: 'session',
+        userId: 'test-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce(undefined)
 
       const req = new Request(
@@ -283,12 +322,16 @@ describe('OAuth Token API Routes', () => {
       const response = await GET(req as any)
       const data = await response.json()
 
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(404)
       expect(data).toHaveProperty('error')
     })
 
     it('should handle missing access token', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockCheckHybridAuth.mockResolvedValueOnce({
+        success: true,
+        authType: 'session',
+        userId: 'test-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: null,
@@ -305,12 +348,16 @@ describe('OAuth Token API Routes', () => {
       const response = await GET(req as any)
       const data = await response.json()
 
-      expect([400, 401]).toContain(response.status)
+      expect(response.status).toBe(400)
       expect(data).toHaveProperty('error')
     })
 
     it('should handle token refresh failure', async () => {
-      mockGetUserId.mockResolvedValueOnce('test-user-id')
+      mockCheckHybridAuth.mockResolvedValueOnce({
+        success: true,
+        authType: 'session',
+        userId: 'test-user-id',
+      })
       mockGetCredential.mockResolvedValueOnce({
         id: 'credential-id',
         accessToken: 'test-token',
@@ -329,7 +376,7 @@ describe('OAuth Token API Routes', () => {
       const response = await GET(req as any)
       const data = await response.json()
 
-      expect([401, 404]).toContain(response.status)
+      expect(response.status).toBe(401)
       expect(data).toHaveProperty('error')
     })
   })

apps/sim/app/api/auth/oauth/token/route.ts

@@ -1,9 +1,8 @@
-import { eq } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
+import { authorizeCredentialUse } from '@/lib/auth/credential-access'
+import { checkHybridAuth } from '@/lib/auth/hybrid'
 import { createLogger } from '@/lib/logs/console/logger'
-import { getCredential, getUserId, refreshTokenIfNeeded } from '@/app/api/auth/oauth/utils'
-import { db } from '@/db'
-import { account } from '@/db/schema'
+import { getCredential, refreshTokenIfNeeded } from '@/app/api/auth/oauth/utils'
 
 export const dynamic = 'force-dynamic'
 
@@ -29,18 +28,13 @@ export async function POST(request: NextRequest) {
       return NextResponse.json({ error: 'Credential ID is required' }, { status: 400 })
     }
 
-    // Resolve the credential owner directly by id. This lets API/UI/webhooks run under
-    // whichever user owns the persisted credential, not necessarily the session user
-    // or workflow owner.
-    const creds = await db.select().from(account).where(eq(account.id, credentialId)).limit(1)
-    if (!creds.length) {
-      logger.error(`[${requestId}] Credential not found: ${credentialId}`)
-      return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
+    const authz = await authorizeCredentialUse(request, { credentialId, workflowId })
+    if (!authz.ok || !authz.credentialOwnerUserId) {
+      return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
     }
-    const credentialOwnerUserId = creds[0].userId
 
-    // Fetch the credential verifying it belongs to the resolved owner
-    const credential = await getCredential(requestId, credentialId, credentialOwnerUserId)
+    // Fetch the credential as the owner to enforce ownership scoping
+    const credential = await getCredential(requestId, credentialId, authz.credentialOwnerUserId)
 
     try {
       // Refresh the token if needed
@@ -73,14 +67,13 @@ export async function GET(request: NextRequest) {
     }
 
     // For GET requests, we only support session-based authentication
-    const userId = await getUserId(requestId)
-
-    if (!userId) {
+    const auth = await checkHybridAuth(request, { requireWorkflowId: false })
+    if (!auth.success || auth.authType !== 'session' || !auth.userId) {
       return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
     }
 
     // Get the credential from the database
-    const credential = await getCredential(requestId, credentialId, userId)
+    const credential = await getCredential(requestId, credentialId, auth.userId)
 
     if (!credential) {
       return NextResponse.json({ error: 'Credential not found' }, { status: 404 })

apps/sim/app/api/tools/drive/file/route.ts

@@ -1,10 +1,7 @@
-import { eq } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
-import { getSession } from '@/lib/auth'
+import { authorizeCredentialUse } from '@/lib/auth/credential-access'
 import { createLogger } from '@/lib/logs/console/logger'
 import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
-import { db } from '@/db'
-import { account } from '@/db/schema'
 
 export const dynamic = 'force-dynamic'
 
@@ -18,46 +15,28 @@ export async function GET(request: NextRequest) {
   logger.info(`[${requestId}] Google Drive file request received`)
 
   try {
-    // Get the session
-    const session = await getSession()
-
-    // Check if the user is authenticated
-    if (!session?.user?.id) {
-      logger.warn(`[${requestId}] Unauthenticated request rejected`)
-      return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
-    }
-
     // Get the credential ID and file ID from the query params
     const { searchParams } = new URL(request.url)
     const credentialId = searchParams.get('credentialId')
     const fileId = searchParams.get('fileId')
-    const workflowId = searchParams.get('workflowId')
+    const workflowId = searchParams.get('workflowId') || undefined
 
     if (!credentialId || !fileId) {
       logger.warn(`[${requestId}] Missing required parameters`)
       return NextResponse.json({ error: 'Credential ID and File ID are required' }, { status: 400 })
     }
 
-    // Get the credential from the database
-    const credentials = await db.select().from(account).where(eq(account.id, credentialId)).limit(1)
-
-    if (!credentials.length) {
-      logger.warn(`[${requestId}] Credential not found`, { credentialId })
-      return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
-    }
-
-    const credential = credentials[0]
-
-    // Credential ownership:
-    // - If session user owns the credential: allow
-    // - If not, allow read-only resolution when a workflowId is present (collaboration case)
-    if (credential.userId !== session.user.id && !workflowId) {
-      return NextResponse.json({ error: 'Unauthorized' }, { status: 403 })
+    const authz = await authorizeCredentialUse(request, { credentialId: credentialId, workflowId })
+    if (!authz.ok || !authz.credentialOwnerUserId) {
+      return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
     }
 
     // Refresh access token if needed using the utility function
-    const ownerUserId = credential.userId
-    const accessToken = await refreshAccessTokenIfNeeded(credentialId, ownerUserId, requestId)
+    const accessToken = await refreshAccessTokenIfNeeded(
+      credentialId,
+      authz.credentialOwnerUserId,
+      requestId
+    )
 
     if (!accessToken) {
       return NextResponse.json({ error: 'Failed to obtain valid access token' }, { status: 401 })