Skip to content

Conversation

@CodeMan62
Copy link

@CodeMan62 CodeMan62 commented Nov 21, 2025

Summary

Added generic OAuth support in sim helm

Fixes #2080

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

let me know if we have to add unit tests?

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 Nov 21, 2025

@CodeMan62 is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 21, 2025

Greptile Overview

Greptile Summary

Added generic OAuth provider support to enable Auth0, Okta, Keycloak, and other OIDC providers through environment variables and Helm configuration.

Key Changes:

  • Added 7 new environment variables (OAUTH_CLIENT_ID, OAUTH_CLIENT_SECRET, OAUTH_AUTHORIZATION_URL, OAUTH_TOKEN_URL, OAUTH_USERINFO_URL, OAUTH_SCOPES, OAUTH_PROVIDER_ID) in env.ts, values.yaml, and values.schema.json
  • Added generic OAuth provider to Better Auth's genericOAuth plugin configuration in auth.ts
  • Added provider ID to trusted providers list for cross-site authentication

Critical Issue:
The OAuth provider configuration in apps/sim/lib/auth.ts:1597-1608 is missing the required getUserInfo function. All other genericOAuth providers in the codebase (github-repo, salesforce, x, webflow, etc.) implement this function to fetch and map user profile data from the OAuth provider. Without this function, authentication will fail when users attempt to log in with the generic OAuth provider.

Additional Improvement:
The OAuth URL fields in env.ts should use .url() validation like other URL fields in the codebase (e.g., DATABASE_URL, OLLAMA_URL) to prevent configuration errors.

Confidence Score: 2/5

  • This PR cannot be safely merged without the missing getUserInfo function, which will cause authentication failures
  • Score reflects a critical missing implementation (getUserInfo function) that will cause runtime errors when users attempt OAuth authentication. The Helm configuration and environment variable definitions are well-structured, but the core authentication logic is incomplete. The missing URL validation is a minor issue compared to the blocking authentication bug.
  • Pay close attention to apps/sim/lib/auth.ts - the generic OAuth configuration must include a getUserInfo function before this can be merged

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/auth.ts 2/5 Added generic OAuth provider configuration but missing critical getUserInfo function required by Better Auth's genericOAuth plugin, which will cause runtime errors during authentication
apps/sim/lib/env.ts 3/5 Added OAuth environment variables but missing URL validation (.url()) for authorization, token, and userinfo URLs, unlike other URL fields in the codebase

Sequence Diagram

sequenceDiagram
    participant U as User
    participant S as Sim
    participant B as Better Auth
    participant P as Provider
    
    U->>S: Click login
    S->>B: Start flow
    B->>P: Authorize
    P->>U: Show form
    U->>P: Submit
    P->>B: Callback
    B->>P: Get token
    P->>B: Token
    Note over B: Missing getUserInfo<br/>Cannot fetch profile
    B->>P: Fetch profile
    P->>B: Profile data
    B->>S: Session
    S->>U: Complete
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.

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +1597 to +1608
{
providerId: env.OAUTH_PROVIDER_ID,
clientId: env.OAUTH_CLIENT_ID,
clientSecret: env.OAUTH_CLIENT_SECRET,
authorizationUrl: env.OAUTH_AUTHORIZATION_URL,
tokenUrl: env.OAUTH_TOKEN_URL,
userInfoUrl: env.OAUTH_USERINFO_URL,
scopes: env.OAUTH_SCOPES
? env.OAUTH_SCOPES.split(' ').filter(Boolean)
: ['openid', 'profile', 'email'],
redirectURI: `${getBaseUrl()}/api/auth/oauth2/callback/${env.OAUTH_PROVIDER_ID}`,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: missing getUserInfo function - this will cause authentication to fail

All other genericOAuth providers in this file (github-repo, wealthbox, pipedrive, hubspot, salesforce, supabase, x, webflow) include a getUserInfo function that fetches user data from the provider and returns it in Better Auth's expected format.

The generic OAuth configuration needs a getUserInfo async function that:

  1. Fetches user data from OAUTH_USERINFO_URL using the access token
  2. Maps the response to Better Auth's user schema (id, name, email, emailVerified, image, createdAt, updatedAt)
  3. Handles errors and returns null on failure

See apps/sim/lib/auth.ts:833-868 (Salesforce) for a standard OIDC implementation pattern.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/auth.ts
Line: 1597:1608

Comment:
**logic:** missing `getUserInfo` function - this will cause authentication to fail

All other genericOAuth providers in this file (github-repo, wealthbox, pipedrive, hubspot, salesforce, supabase, x, webflow) include a `getUserInfo` function that fetches user data from the provider and returns it in Better Auth's expected format.

The generic OAuth configuration needs a `getUserInfo` async function that:
1. Fetches user data from `OAUTH_USERINFO_URL` using the access token
2. Maps the response to Better Auth's user schema (id, name, email, emailVerified, image, createdAt, updatedAt)
3. Handles errors and returns null on failure

See `apps/sim/lib/auth.ts:833-868` (Salesforce) for a standard OIDC implementation pattern.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +179 to +181
OAUTH_AUTHORIZATION_URL: z.string().optional(), // OAuth authorization URL
OAUTH_TOKEN_URL: z.string().optional(), // OAuth token URL
OAUTH_USERINFO_URL: z.string().optional(), // OAuth userinfo URL
Copy link
Contributor

Choose a reason for hiding this comment

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

style: missing URL validation - use .url() validator like other URL fields

Other URL fields in this file use .url() validation (e.g., DATABASE_URL: z.string().url() on line 19, OLLAMA_URL: z.string().url().optional() on line 79). This validates URL format and prevents configuration errors.

Suggested change
OAUTH_AUTHORIZATION_URL: z.string().optional(), // OAuth authorization URL
OAUTH_TOKEN_URL: z.string().optional(), // OAuth token URL
OAUTH_USERINFO_URL: z.string().optional(), // OAuth userinfo URL
OAUTH_AUTHORIZATION_URL: z.string().url().optional(), // OAuth authorization URL
OAUTH_TOKEN_URL: z.string().url().optional(), // OAuth token URL
OAUTH_USERINFO_URL: z.string().url().optional(), // OAuth userinfo URL
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/env.ts
Line: 179:181

Comment:
**style:** missing URL validation - use `.url()` validator like other URL fields

Other URL fields in this file use `.url()` validation (e.g., `DATABASE_URL: z.string().url()` on line 19, `OLLAMA_URL: z.string().url().optional()` on line 79). This validates URL format and prevents configuration errors.

```suggestion
    OAUTH_AUTHORIZATION_URL:               z.string().url().optional(),            // OAuth authorization URL
    OAUTH_TOKEN_URL:                       z.string().url().optional(),            // OAuth token URL
    OAUTH_USERINFO_URL:                    z.string().url().optional(),            // OAuth userinfo URL
```

How can I resolve this? If you propose a fix, please make it concise.

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.

[REQUEST] Generic OAUTH support in app/helm/deployment

1 participant