Skip to content

Commit 981f757

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(credentials): centralize application authorization
1 parent 22d845a commit 981f757

52 files changed

Lines changed: 3118 additions & 2666 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/sim/app/api/auth/accounts/route.ts‎

Lines changed: 22 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,23 @@
1-
import { db } from '@sim/db'
2-
import { account, credential } from '@sim/db/schema'
3-
import { createLogger } from '@sim/logger'
4-
import { and, desc, eq } from 'drizzle-orm'
5-
import { type NextRequest, NextResponse } from 'next/server'
6-
import { connectedAccountsQuerySchema } from '@/lib/api/contracts/oauth-connections'
7-
import { getSession } from '@/lib/auth'
8-
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
9-
10-
const logger = createLogger('AuthAccountsAPI')
11-
12-
export const GET = withRouteHandler(async (request: NextRequest) => {
13-
try {
14-
const session = await getSession()
15-
if (!session?.user?.id) {
16-
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
17-
}
18-
19-
const { searchParams } = new URL(request.url)
20-
const { provider } = connectedAccountsQuerySchema.parse({
21-
provider: searchParams.get('provider') || undefined,
22-
})
23-
24-
const whereConditions = [eq(account.userId, session.user.id)]
25-
26-
if (provider) {
27-
whereConditions.push(eq(account.providerId, provider))
28-
}
29-
30-
const accounts = await db
31-
.select({
32-
id: account.id,
33-
accountId: account.accountId,
34-
providerId: account.providerId,
35-
credentialDisplayName: credential.displayName,
36-
})
37-
.from(account)
38-
.leftJoin(credential, eq(credential.accountId, account.id))
39-
.where(and(...whereConditions))
40-
.orderBy(desc(account.updatedAt))
41-
42-
const seen = new Map<string, (typeof accounts)[number]>()
43-
for (const acc of accounts) {
44-
if (!seen.has(acc.id)) {
45-
seen.set(acc.id, acc)
46-
}
47-
}
48-
49-
const accountsWithDisplayName = Array.from(seen.values()).map((acc) => ({
50-
id: acc.id,
51-
accountId: acc.accountId,
52-
providerId: acc.providerId,
53-
displayName: acc.credentialDisplayName || acc.accountId || acc.providerId,
54-
}))
55-
56-
return NextResponse.json({ accounts: accountsWithDisplayName })
57-
} catch (error) {
58-
logger.error('Failed to fetch accounts', { error })
59-
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
60-
}
1+
import { listConnectedAccountsContract } from '@/lib/api/contracts/oauth-connections'
2+
import {
3+
defineInternalJsonRoute,
4+
internalRateLimits,
5+
internalSessionAuth,
6+
} from '@/lib/api/server/routes'
7+
import {
8+
credentialValidationParseOptions,
9+
internalCredentialErrorPolicy,
10+
} from '@/lib/credentials/api/route-policies'
11+
import { listConnectedAccountsUseCase } from '@/lib/credentials/application/oauth-accounts'
12+
import { credentialUserOperations } from '@/lib/credentials/application/operations'
13+
14+
export const GET = defineInternalJsonRoute({
15+
contract: listConnectedAccountsContract,
16+
auth: internalSessionAuth,
17+
operation: credentialUserOperations.listConnectedAccounts,
18+
rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal behavior' }),
19+
errorPolicy: internalCredentialErrorPolicy,
20+
parseOptions: credentialValidationParseOptions,
21+
mapInput: ({ query }) => query,
22+
useCase: listConnectedAccountsUseCase,
6123
})

‎apps/sim/app/api/auth/oauth/connections/route.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ describe('OAuth Connections API Route', () => {
4949
it('should return connections successfully', async () => {
5050
authMockFns.mockGetSession.mockResolvedValueOnce({
5151
user: { id: 'user-123' },
52+
session: { id: 'session-1' },
5253
})
5354

5455
const mockAccounts = [
@@ -105,12 +106,13 @@ describe('OAuth Connections API Route', () => {
105106
const data = await response.json()
106107

107108
expect(response.status).toBe(401)
108-
expect(data.error).toBe('User not authenticated')
109+
expect(data.error).toBe('Unauthorized')
109110
})
110111

111112
it('should handle user with no connections', async () => {
112113
authMockFns.mockGetSession.mockResolvedValueOnce({
113114
user: { id: 'user-123' },
115+
session: { id: 'session-1' },
114116
})
115117

116118
dbChainMockFns.where.mockResolvedValueOnce([])
@@ -128,6 +130,7 @@ describe('OAuth Connections API Route', () => {
128130
it('should handle database error', async () => {
129131
authMockFns.mockGetSession.mockResolvedValueOnce({
130132
user: { id: 'user-123' },
133+
session: { id: 'session-1' },
131134
})
132135

133136
dbChainMockFns.where.mockRejectedValueOnce(new Error('Database error'))
@@ -144,6 +147,7 @@ describe('OAuth Connections API Route', () => {
144147
it('should decode ID token for display name', async () => {
145148
authMockFns.mockGetSession.mockResolvedValueOnce({
146149
user: { id: 'user-123' },
150+
session: { id: 'session-1' },
147151
})
148152

149153
const mockAccounts = [

‎apps/sim/app/api/auth/oauth/connections/route.ts‎

Lines changed: 18 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -1,139 +1,19 @@
1-
import { account, db, user } from '@sim/db'
2-
import { createLogger } from '@sim/logger'
3-
import { eq } from 'drizzle-orm'
4-
import { decodeJwt } from 'jose'
5-
import { type NextRequest, NextResponse } from 'next/server'
6-
import type { OAuthConnection } from '@/lib/api/contracts/oauth-connections'
7-
import { getSession } from '@/lib/auth'
8-
import { generateRequestId } from '@/lib/core/utils/request'
9-
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
10-
import type { OAuthProvider } from '@/lib/oauth'
11-
import { parseProvider } from '@/lib/oauth'
12-
13-
const logger = createLogger('OAuthConnectionsAPI')
14-
15-
interface GoogleIdToken {
16-
email?: string
17-
sub?: string
18-
name?: string
19-
}
20-
21-
/**
22-
* Get all OAuth connections for the current user
23-
*/
24-
export const GET = withRouteHandler(async (request: NextRequest) => {
25-
const requestId = generateRequestId()
26-
27-
try {
28-
// Get the session
29-
const session = await getSession()
30-
31-
// Check if the user is authenticated
32-
if (!session?.user?.id) {
33-
logger.warn(`[${requestId}] Unauthenticated request rejected`)
34-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
35-
}
36-
37-
// Get all accounts for this user
38-
const accounts = await db.select().from(account).where(eq(account.userId, session.user.id))
39-
40-
// Get the user's email for fallback
41-
const userRecord = await db
42-
.select({ email: user.email })
43-
.from(user)
44-
.where(eq(user.id, session.user.id))
45-
.limit(1)
46-
47-
const userEmail = userRecord.length > 0 ? userRecord[0]?.email : null
48-
49-
// Process accounts to determine connections
50-
const connections: OAuthConnection[] = []
51-
52-
for (const acc of accounts) {
53-
const { baseProvider, featureType } = parseProvider(acc.providerId as OAuthProvider)
54-
const scopes = acc.scope ? acc.scope.split(/\s+/).filter(Boolean) : []
55-
56-
if (baseProvider) {
57-
// Try multiple methods to get a user-friendly display name
58-
let displayName = ''
59-
60-
// Method 1: Try to extract email from ID token (works for Google, etc.)
61-
if (acc.idToken) {
62-
try {
63-
const decoded = decodeJwt<GoogleIdToken>(acc.idToken)
64-
if (decoded.email) {
65-
displayName = decoded.email
66-
} else if (decoded.name) {
67-
displayName = decoded.name
68-
}
69-
} catch (_error) {
70-
logger.warn(`[${requestId}] Error decoding ID token`, {
71-
accountId: acc.id,
72-
})
73-
}
74-
}
75-
76-
// Method 2: For GitHub, the accountId might be the username
77-
if (!displayName && baseProvider === 'github') {
78-
displayName = `${acc.accountId} (GitHub)`
79-
}
80-
81-
// Method 3: Use the user's email from our database
82-
if (!displayName && userEmail) {
83-
displayName = userEmail
84-
}
85-
86-
// Fallback: Use accountId with provider type as context
87-
if (!displayName) {
88-
displayName = `${acc.accountId} (${baseProvider})`
89-
}
90-
91-
// Create a unique connection key that includes the full provider ID
92-
const connectionKey = acc.providerId
93-
94-
// Find existing connection for this specific provider ID
95-
const existingConnection = connections.find((conn) => conn.provider === connectionKey)
96-
97-
const accountSummary = {
98-
id: acc.id,
99-
name: displayName,
100-
}
101-
102-
if (existingConnection) {
103-
// Add account to existing connection
104-
existingConnection.accounts = existingConnection.accounts || []
105-
existingConnection.accounts.push(accountSummary)
106-
107-
existingConnection.scopes = Array.from(
108-
new Set([...(existingConnection.scopes || []), ...scopes])
109-
)
110-
111-
const existingTimestamp = existingConnection.lastConnected
112-
? new Date(existingConnection.lastConnected).getTime()
113-
: 0
114-
const candidateTimestamp = acc.updatedAt.getTime()
115-
116-
if (candidateTimestamp > existingTimestamp) {
117-
existingConnection.lastConnected = acc.updatedAt.toISOString()
118-
}
119-
} else {
120-
// Create new connection
121-
connections.push({
122-
provider: connectionKey,
123-
baseProvider,
124-
featureType,
125-
isConnected: true,
126-
scopes,
127-
lastConnected: acc.updatedAt.toISOString(),
128-
accounts: [accountSummary],
129-
})
130-
}
131-
}
132-
}
133-
134-
return NextResponse.json({ connections }, { status: 200 })
135-
} catch (error) {
136-
logger.error(`[${requestId}] Error fetching OAuth connections`, error)
137-
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
138-
}
1+
import { listOAuthConnectionsContract } from '@/lib/api/contracts/oauth-connections'
2+
import {
3+
defineInternalJsonRoute,
4+
internalRateLimits,
5+
internalSessionAuth,
6+
} from '@/lib/api/server/routes'
7+
import { internalCredentialErrorPolicy } from '@/lib/credentials/api/route-policies'
8+
import { listOAuthConnectionsUseCase } from '@/lib/credentials/application/oauth-accounts'
9+
import { credentialUserOperations } from '@/lib/credentials/application/operations'
10+
11+
export const GET = defineInternalJsonRoute({
12+
contract: listOAuthConnectionsContract,
13+
auth: internalSessionAuth,
14+
operation: credentialUserOperations.listOAuthConnections,
15+
rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal behavior' }),
16+
errorPolicy: internalCredentialErrorPolicy,
17+
mapInput: () => ({}),
18+
useCase: listOAuthConnectionsUseCase,
13919
})

‎apps/sim/app/api/auth/oauth/disconnect/route.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('OAuth Disconnect API Route', () => {
2626
it('should disconnect provider successfully', async () => {
2727
authMockFns.mockGetSession.mockResolvedValueOnce({
2828
user: { id: 'user-123' },
29+
session: { id: 'session-1' },
2930
})
3031

3132
const req = createMockRequest('POST', {
@@ -42,6 +43,7 @@ describe('OAuth Disconnect API Route', () => {
4243
it('should disconnect specific provider ID successfully', async () => {
4344
authMockFns.mockGetSession.mockResolvedValueOnce({
4445
user: { id: 'user-123' },
46+
session: { id: 'session-1' },
4547
})
4648

4749
const req = createMockRequest('POST', {
@@ -67,12 +69,13 @@ describe('OAuth Disconnect API Route', () => {
6769
const data = await response.json()
6870

6971
expect(response.status).toBe(401)
70-
expect(data.error).toBe('User not authenticated')
72+
expect(data.error).toBe('Unauthorized')
7173
})
7274

7375
it('should handle missing provider', async () => {
7476
authMockFns.mockGetSession.mockResolvedValueOnce({
7577
user: { id: 'user-123' },
78+
session: { id: 'session-1' },
7679
})
7780

7881
const req = createMockRequest('POST', {})
@@ -87,6 +90,7 @@ describe('OAuth Disconnect API Route', () => {
8790
it('should handle database error', async () => {
8891
authMockFns.mockGetSession.mockResolvedValueOnce({
8992
user: { id: 'user-123' },
93+
session: { id: 'session-1' },
9094
})
9195

9296
dbChainMockFns.where.mockRejectedValueOnce(new Error('Database error'))

0 commit comments

Comments
 (0)