Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • upgraded all generic oauth plugin providers to use unqiue account ids

Type of Change

  • improvement

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 17, 2026 8:43pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 17, 2026

Greptile Summary

Added getUserInfo implementations to 16 OAuth providers (10 Google service variants and 6 Microsoft service variants) to generate unique account identifiers by concatenating the provider's user ID with crypto.randomUUID().

Key Changes:

  • Each provider now explicitly fetches user profile information and constructs account data
  • Google providers use profile.sub from OpenID Connect endpoint
  • Microsoft providers use profile.id from Microsoft Graph API
  • Account IDs are generated as ${providerId}-${crypto.randomUUID()} for uniqueness
  • All implementations include error handling with structured logging

Context:
This builds on previous work (commit f6b7c15) that changed the database unique constraint from (accountId, providerId, userId) to just (userId, providerId). The account.create.before hook (auth.ts:150-202) ensures that when users re-authenticate, the existing account is updated rather than creating duplicates. The accountId field is not used for lookups in the codebase - all queries use the primary key account.id instead.

Suggestions:

  • The 16 nearly-identical getUserInfo implementations create significant code duplication (400+ lines). Consider extracting shared helper functions for Google and Microsoft providers.
  • The random UUID suffix in accountId causes the field to change on every re-authentication (via the update on line 193). Since this field isn't used for lookups, consider whether the UUID suffix is necessary or if just using profile.sub/profile.id would be cleaner for debugging.

Confidence Score: 3/5

  • This PR is functionally safe to merge but has code quality concerns
  • The implementation follows an established pattern (GitHub provider already uses this approach) and the account.create.before hook ensures no duplicate accounts are created. However, the score is reduced due to: (1) significant code duplication across 16 nearly-identical implementations adding 400+ lines, (2) the accountId field changes on every re-auth despite being unused for lookups, creating unnecessary database churn, and (3) lack of test coverage mentioned in the PR checklist. The functionality should work correctly but maintainability and efficiency could be improved.
  • No files require special attention - the changes are repetitive and follow the same pattern throughout

Important Files Changed

Filename Overview
apps/sim/lib/auth/auth.ts Added getUserInfo implementations to 16 OAuth providers (Google and Microsoft variants) to generate unique account IDs using profile.sub/id + crypto.randomUUID() pattern

Sequence Diagram

sequenceDiagram
    participant User
    participant App
    participant BetterAuth
    participant OAuthProvider
    participant Database
    
    User->>App: Initiate OAuth flow
    App->>BetterAuth: Start authentication
    BetterAuth->>OAuthProvider: Redirect to authorization
    OAuthProvider->>User: Request consent
    User->>OAuthProvider: Grant permissions
    OAuthProvider->>BetterAuth: Return authorization code
    BetterAuth->>OAuthProvider: Exchange code for response
    OAuthProvider->>BetterAuth: Provide response data
    BetterAuth->>BetterAuth: Call getUserInfo handler
    BetterAuth->>OAuthProvider: Fetch user profile
    OAuthProvider->>BetterAuth: Return profile information
    BetterAuth->>BetterAuth: Concat profile.sub with UUID
    BetterAuth->>Database: Execute account.create.before hook
    Database->>Database: SELECT existing (userId, providerId)
    alt Existing Account Found
        Database->>Database: UPDATE account with new accountId
        Database->>BetterAuth: Return existing account
    else No Existing Account
        Database->>Database: INSERT new account
        Database->>BetterAuth: Return new account
    end
    BetterAuth->>App: Return auth session
    App->>User: Redirect to application
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 4b8534e into staging Jan 17, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/oauth branch January 17, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants