Add support for OpenID Connect authentication#719
Conversation
|
@acvigue is attempting to deploy a commit to the goldflag's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces OpenID Connect (OIDC) / OAuth2 authentication support as a complement to existing social providers. It makes authentication configuration dynamic by reading enabled providers from environment variables at runtime and updates login flows to support SSO login with conditional authentication methods based on server configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/lib/auth.ts (1)
42-92: Confirm the impact of disabling emailOTP when internal authentication is disabled.When
INTERNAL_AUTHENTICATION_ENABLEDis false, the entireemailOTPplugin is excluded, which disables:
- Sign-in via email OTP
- Email verification via OTP
- Password reset via OTP (forget-password flow)
This creates a dependency where users relying on OIDC/social providers cannot use OTP-based flows at all. If your application expects to support password resets or email verification for users created before OIDC was enabled, this could break those flows.
Additionally, the
sendEmailfunction (fromserver/src/lib/email/email.tslines 14-32) returns early without throwing if Resend is not initialized, which could cause OTP emails to silently fail in self-hosted instances without proper email configuration.Consider:
- Whether email verification or password reset should remain available even when internal auth is disabled
- Adding error handling or user feedback when email sending fails silently
🧹 Nitpick comments (3)
client/src/components/auth/SocialButtons.tsx (2)
15-16: Consider handling loading and empty provider states.The component doesn't check
isLoadingfromuseConfigs(), and ifconfigsis undefined or all provider arrays are empty, the UI will display "Or continue with" without any buttons, which could confuse users.Consider adding early returns or conditional rendering:
export function SocialButtons({ onError, callbackURL, mode = "signin", className = "" }: SocialButtonsProps) { - const { configs } = useConfigs(); + const { configs, isLoading } = useConfigs(); + + if (isLoading || !configs) { + return null; + } + + const hasProviders = (configs.enabledOIDCProviders?.length ?? 0) > 0 || + (configs.enabledSocialProviders?.length ?? 0) > 0; + + if (!hasProviders) { + return null; + }
18-29: Improve error handling in OIDC authentication.Converting errors to strings with
String(error)may lose valuable error context. Consider preserving error messages or using a more robust error extraction pattern.Apply this diff:
const handleOIDCAuth = async (providerId: string) => { try { await authClient.signIn.oauth2({ providerId, ...(callbackURL && mode !== "signup" ? { callbackURL } : {}), ...(mode === "signup" && callbackURL ? { newUserCallbackURL: callbackURL } : {}), }); } catch (error) { - onError(String(error)); + onError(error instanceof Error ? error.message : String(error)); } }server/src/lib/const.ts (1)
376-426: Based on verification, I can now provide the rewritten review comment:Improve OIDC provider configuration with URL validation and better default naming.
The implementation correctly scans environment variables and builds OIDC provider configs. The suggested improvements are valid:
URL validation: The
discoveryUrlshould be validated. This aligns with validation patterns used elsewhere in the codebase and prevents runtime errors during initialization.Default provider name: The fallback to
'OpenID Connect'is too generic when multiple providers are configured without explicit names. UseSSO (${providerId.toLowerCase()})for better clarity.Provider ID format: The regex correctly enforces uppercase-only IDs in environment variables (e.g.,
OIDC_OKTA_CLIENT_ID), normalized to lowercase. Document this requirement in configuration comments.Hardcoded scopes: The scopes
["openid", "profile", "email"]are fixed per implementation. Note that better-auth's genericOAuth plugin supports custom scopes per provider, so consider addingOIDC_${providerId}_SCOPESenvironment variable support for future flexibility.Recommended changes:
for (const providerId of providerIds) { const clientId = process.env[`OIDC_${providerId}_CLIENT_ID`]; const clientSecret = process.env[`OIDC_${providerId}_CLIENT_SECRET`]; const discoveryUrl = process.env[`OIDC_${providerId}_DISCOVERY_URL`]; - const name = process.env[`OIDC_${providerId}_NAME`] || 'OpenID Connect'; + const name = process.env[`OIDC_${providerId}_NAME`] || `SSO (${providerId.toLowerCase()})`; if (clientId && clientSecret && discoveryUrl) { + // Validate discovery URL format + try { + new URL(discoveryUrl); + } catch { + console.error(`Invalid OIDC discovery URL for provider ${providerId}: ${discoveryUrl}`); + continue; + } + providers.push({ providerId: providerId.toLowerCase(), clientId, clientSecret, discoveryUrl, scopes: ["openid", "profile", "email"], name }); } }Update docker-compose documentation to clarify provider ID naming:
# Optional for OpenID Connect. Can have multiple OIDC providers with different names # Note: PROVIDER must be uppercase letters, numbers, and underscores only (e.g., OKTA, AZURE_AD)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
client/package-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.md(1 hunks)client/src/components/auth/SocialButtons.tsx(3 hunks)client/src/lib/auth.ts(1 hunks)client/src/lib/configs.ts(1 hunks)docker-compose.cloud.yml(1 hunks)docker-compose.yml(1 hunks)matomo_import(1 hunks)server/src/api/getConfig.ts(1 hunks)server/src/lib/auth.ts(7 hunks)server/src/lib/const.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-13T04:45:39.436Z
Learnt from: nktnet1
Repo: rybbit-io/rybbit PR: 270
File: server/src/db/clickhouse/clickhouse.ts:7-7
Timestamp: 2025-09-13T04:45:39.436Z
Learning: In the rybbit codebase, environment variables for ClickHouse configuration (CLICKHOUSE_HOST, CLICKHOUSE_DB, CLICKHOUSE_USER, CLICKHOUSE_PASSWORD) are used directly from process.env without default values to maintain consistency across the configuration pattern.
Applied to files:
docker-compose.ymldocker-compose.cloud.yml
🧬 Code graph analysis (4)
server/src/api/getConfig.ts (1)
server/src/lib/const.ts (4)
DISABLE_SIGNUP(7-7)MAPBOX_TOKEN(12-12)getOIDCProviders(376-426)getSocialProviders(429-444)
client/src/components/auth/SocialButtons.tsx (2)
client/src/lib/configs.ts (1)
useConfigs(14-25)client/src/lib/auth.ts (1)
authClient(5-12)
server/src/lib/const.ts (1)
client/src/lib/const.ts (1)
IS_CLOUD(5-5)
server/src/lib/auth.ts (2)
server/src/lib/const.ts (5)
getOIDCProviders(376-426)INTERNAL_AUTHENTICATION_ENABLED(9-9)IS_CLOUD(6-6)DISABLE_SIGNUP(7-7)getSocialProviders(429-444)server/src/lib/email/email.ts (1)
sendEmail(15-33)
🔇 Additional comments (15)
matomo_import (1)
1-1: Clarify the rationale for the submodule update.Line 1 updates the
matomo_importsubmodule reference to commit61e17c1c1e37d311cf2c23e5914b53b24537cd1b. While the enriched summary indicates no code changes to the submodule itself, the PR objectives focus on OIDC authentication support. It's unclear why an analytics/tracking submodule needs to be updated as part of an authentication feature addition.Please clarify:
- Is this submodule update intentional and part of the OIDC authentication changes, or was it accidentally included?
- What changes are present in the commit
61e17c1c1e37d311cf2c23e5914b53b24537cd1bthat relate to the OIDC feature?- If the update is unrelated to OIDC support, consider moving it to a separate PR to keep concerns isolated.
If this is an intentional, minimal housekeeping update unrelated to the OIDC feature, you may safely proceed, but the rationale should be documented in the PR description.
README.md (1)
81-81: LGTM!The feature comparison table accurately reflects the new SSO/OIDC capability being added in this PR.
docker-compose.yml (1)
89-94: LGTM!The OIDC configuration documentation is clear and aligns with the server-side implementation pattern.
docker-compose.cloud.yml (1)
133-137: LGTM!The OIDC configuration documentation is consistent with the base docker-compose.yml file.
client/src/lib/configs.ts (1)
7-11: LGTM!The type definitions correctly match the server-side response structure and support the dynamic provider rendering.
server/src/api/getConfig.ts (1)
2-15: LGTM!The implementation correctly exposes provider metadata to the client without leaking sensitive credentials. The response structure matches the client-side type definitions.
client/src/components/auth/SocialButtons.tsx (1)
51-70: LGTM - Dynamic provider rendering looks correct!The component properly renders OIDC and social providers based on server configuration. The use of optional chaining and conditional rendering ensures safe access to provider arrays.
server/src/lib/const.ts (2)
9-9: LGTM - Good backward compatibility default!
INTERNAL_AUTHENTICATION_ENABLEDdefaulting to enabled (when not explicitly set to"false") preserves existing behavior for deployments that don't set this variable.
429-444: LGTM - Clean social provider discovery!The conditional spread syntax elegantly builds the social providers object based on available environment variables. Only Google and GitHub are supported, which aligns with the client-side changes.
client/src/lib/auth.ts (1)
4-11: I need to gather more information to verify whether this mismatch causes actual authentication issues.Let me search more specifically about the client-side socialProviders parameter:
Now let me search for the
getSocialProviders()function in the codebase:I need to find the
getSocialProviders()implementation to understand the full picture. Let me run a final script:Based on my verification through better-auth documentation and type definitions:
socialProvidersis exclusively a server-side configuration option. To use a social provider, you need to configure the ones you need in the socialProviders option on your auth object. The client-sidesocialProvidersparameter in the code does not appear in better-auth's documented client configuration and is not part of thecreateAuthClient()type definition.The actual OAuth provider availability is controlled server-side via
getSocialProviders(), and the client-side code just passes the provider string toauthClient.signIn.social({ provider: 'google' }). The social provider ID like "github", "google", or "apple" should be set on the provider property.While the static client-side array is non-standard and likely ignored by better-auth, it does not cause authentication failures—the server configuration always takes precedence and will validate requested providers at the API level. The review concern about inconsistency is valid from a code quality perspective, though not a functional breaking issue.
Remove the static
socialProvidersarray from the client config since it's non-standard, undocumented, and doesn't affect OAuth flow control (which is server-driven). Either delete the line or update the TODO to explain why the empty array approach is preferable if hooks cannot be used.server/src/lib/auth.ts (5)
2-2: LGTM!The
genericOAuthimport is necessary for the OIDC provider support added below.
10-10: LGTM!All imported constants and functions are properly utilized in the authentication configuration.
114-114: LGTM!Using
INTERNAL_AUTHENTICATION_ENABLEDto control email/password authentication aligns with the PR objectives. The default value preserves existing behavior when the environment variable is unset.
119-119: LGTM!Replacing the static social providers configuration with
getSocialProviders()enables dynamic provider configuration via environment variables, consistent with the OIDC approach and the PR objectives.
35-41: The genericOAuth configuration is compatible with the better-auth plugin.The
getOIDCProviders()return format aligns with the genericOAuth plugin's expected configuration. The plugin expects a config array of provider objects with providerId, clientId, clientSecret, plus either OIDC discoveryUrl or explicit endpoints. Your configuration provides all required fields (providerId, clientId, clientSecret, discoveryUrl, scopes), so no changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
client/src/components/auth/SocialButtons.tsx (3)
16-26: Consider a loading skeleton to prevent layout shift.Returning
nullwhile configs load may cause the UI to shift when provider buttons appear. A skeleton loader would provide a smoother user experience.Example approach:
if (isLoading || !configs) { - return null; + return ( + <div className="flex flex-col gap-2"> + <div className="h-10 bg-muted animate-pulse rounded" /> + <div className="h-10 bg-muted animate-pulse rounded" /> + </div> + ); }
61-61: Remove redundant optional chaining.The optional chaining
configs?.enabledOIDCProviderson line 61 is unnecessary since you already verified!configson line 18. The component returns early if configs is null or undefined.- {configs?.enabledOIDCProviders.map((provider) => ( + {configs.enabledOIDCProviders.map((provider) => (
61-80: Consider consistent rendering pattern for all providers.OIDC providers are rendered dynamically using
.map(), while social providers (Google, GitHub) use individual hardcoded conditional blocks. For better maintainability and to reduce code duplication, consider unifying the approach.Example refactor:
const socialProviderConfig = [ { id: 'google', name: 'Google', icon: SiGoogle }, { id: 'github', name: 'GitHub', icon: SiGithub }, ]; // Then render: {socialProviderConfig .filter(p => configs.enabledSocialProviders.includes(p.id)) .map((provider) => { const Icon = provider.icon; return ( <Button key={provider.id} type="button" onClick={() => handleSocialAuth(provider.id)}> <Icon /> {provider.name} </Button> ); }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/components/auth/SocialButtons.tsx(3 hunks)server/src/lib/const.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/lib/const.ts
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/components/auth/SocialButtons.tsx (2)
client/src/lib/configs.ts (1)
useConfigs(14-25)client/src/lib/auth.ts (1)
authClient(5-12)
🔇 Additional comments (1)
client/src/components/auth/SocialButtons.tsx (1)
4-6: LGTM: Import additions are appropriate.The new imports for
SiOpenidicon anduseConfigshook are necessary for the dynamic OIDC provider functionality.
| const handleOIDCAuth = async (providerId: string) => { | ||
| try { | ||
| await authClient.signIn.oauth2({ | ||
| providerId, | ||
| ...(callbackURL && mode !== "signup" ? { callbackURL } : {}), | ||
| // For signup flow, new users should be redirected to the same callbackURL | ||
| ...(mode === "signup" && callbackURL ? { newUserCallbackURL: callbackURL } : {}), | ||
| }); | ||
| } catch (error) { | ||
| onError(String(error)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify callback URL logic and improve error handling.
Two observations:
-
The callback URL logic distinguishes between
signinandsignupmodes, usingcallbackURLfor signin andnewUserCallbackURLfor signup. Please verify this behavior matches the auth library's expectations. -
Converting the error to a string with
String(error)may lose important error details like stack traces or error codes. Consider usingerror.messageor logging the full error object.
Suggested improvement for error handling:
} catch (error) {
- onError(String(error));
+ onError(error instanceof Error ? error.message : String(error));
+ console.error('OIDC authentication failed:', error);
}🏁 Script executed:
#!/bin/bash
# Find where authClient is imported or defined in SocialButtons.tsx
rg -n "authClient" client/src/components/auth/SocialButtons.tsx -A2 -B2Length of output: 649
🏁 Script executed:
#!/bin/bash
# Find authClient imports across the codebase
rg -n "import.*authClient|from.*authClient" --type=ts --type=tsx -g '!**/node_modules/**'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find other oauth2 method calls to verify callback URL pattern
rg -n "oauth2\s*\(" --type=ts --type=tsx -C3 -g '!**/node_modules/**'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find other error handling patterns with onError callback
rg -n "onError\s*\(" --type=ts --type=tsx -C2 -g '!**/node_modules/**' | head -50Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check package.json for auth library dependencies
cat package.json | grep -A5 -B5 "auth"Length of output: 44
🏁 Script executed:
#!/bin/bash
# Find the auth client library definition
fd -n "auth\." client/src/lib/ 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find package.json
fd -n "package.json" -g "package.json" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the auth library definition
find . -path "*/lib/auth*" -type f 2>/dev/null | head -10Length of output: 111
🏁 Script executed:
#!/bin/bash
# Search for onError patterns without type restriction
rg -n "onError" client/src/components/auth/SocialButtons.tsx -B5 -A3Length of output: 1232
🏁 Script executed:
#!/bin/bash
# Check for error handling patterns in the entire auth component directory
rg -n "catch\s*\(" client/src/components/auth/ -A2Length of output: 406
🏁 Script executed:
#!/bin/bash
# Find all social/oauth related method calls in the file
cat client/src/components/auth/SocialButtons.tsxLength of output: 2575
🏁 Script executed:
#!/bin/bash
# Examine the auth client library definition
cat client/src/lib/auth.ts | head -100Length of output: 548
🏁 Script executed:
#!/bin/bash
# Search for oauth2 and social method definitions in auth.ts
rg -n "oauth2|social" client/src/lib/auth.ts -B2 -A5Length of output: 445
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions or interfaces for these methods
rg -n "interface.*signIn|type.*signIn|oauth2|newUserCallbackURL" client/src/lib/auth.ts -B2 -A8Length of output: 0
🌐 Web query:
better-auth oauth2 method signature callback parameters
💡 Result:
Here are the relevant Better Auth OAuth2 method signatures and the callback parameters you can access.
- Client sign-in call
- Signature (client): authClient.signIn.oauth2({ providerId: string, callbackURL?: string, errorCallbackURL?: string, newUserCallbackURL?: string, disableRedirect?: boolean, scopes?: string[], requestSignUp?: boolean }). [1]
- Linking OAuth accounts
- Signature (client): authClient.oauth2.link({ providerId: string, callbackURL: string }). [1]
- OAuth callback route
- The plugin mounts /oauth2/callback/:providerId (default baseURL/api/auth/oauth2/callback/:providerId). The provider will POST/GET back standard OAuth params (code, state, error) to that route; Better Auth handles the exchange and then redirects to your callbackURL. [1][6]
- Data available during the callback (via getOAuthState)
- Default OAuth state fields you can read in hooks or DB hooks with getOAuthState():
- callbackURL
- codeVerifier
- errorURL
- newUserURL
- link (email + user id for linking flows)
- requestSignUp
- expiresAt
- any additionalData you passed when initiating the flow (custom keys). [3]
Sources
- Better Auth Generic OAuth docs (method signatures, callback route). [1]
- Better Auth OAuth concept page (getOAuthState / available state fields). [3]
- GitHub issue confirming callback route behavior. [6]
References
[1] https://www.better-auth.com/docs/plugins/generic-oauth
[3] https://www.better-auth.com/docs/concepts/oauth
[6] better-auth/better-auth#4327
🌐 Web query:
better-auth social signin method newUserCallbackURL parameter
💡 Result:
Yes — Better Auth's social sign-in methods accept a newUserCallbackURL parameter. Pass newUserCallbackURL to authClient.signIn.social (or signIn.oauth2) to redirect newly created users to a different URL than returning users. Example:
await authClient.signIn.social({
provider: "google",
callbackURL: "/dashboard", // returning users
newUserCallbackURL: "/onboarding", // first-time users
});
Documentation: Better Auth social sign-in and Generic OAuth docs, and the Magic Link docs showing the same newUserCallbackURL option. [1][2][3]
Improve error handling in catch blocks.
The callback URL logic for distinguishing between signin and signup flows is correct and aligns with better-auth's newUserCallbackURL parameter design.
However, the error handling should be improved: Converting errors to string with String(error) results in "[object Object]" for Error objects, losing meaningful error information. Since the onError callback expects (error: string), extract the error message instead:
} catch (error) {
- onError(String(error));
+ onError(error instanceof Error ? error.message : String(error));
}Apply this fix to both handleOIDCAuth (line 37) and handleSocialAuth (line 50).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleOIDCAuth = async (providerId: string) => { | |
| try { | |
| await authClient.signIn.oauth2({ | |
| providerId, | |
| ...(callbackURL && mode !== "signup" ? { callbackURL } : {}), | |
| // For signup flow, new users should be redirected to the same callbackURL | |
| ...(mode === "signup" && callbackURL ? { newUserCallbackURL: callbackURL } : {}), | |
| }); | |
| } catch (error) { | |
| onError(String(error)); | |
| } | |
| } | |
| const handleOIDCAuth = async (providerId: string) => { | |
| try { | |
| await authClient.signIn.oauth2({ | |
| providerId, | |
| ...(callbackURL && mode !== "signup" ? { callbackURL } : {}), | |
| // For signup flow, new users should be redirected to the same callbackURL | |
| ...(mode === "signup" && callbackURL ? { newUserCallbackURL: callbackURL } : {}), | |
| }); | |
| } catch (error) { | |
| onError(error instanceof Error ? error.message : String(error)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In client/src/components/auth/SocialButtons.tsx around lines 28 to 39, the catch
block uses onError(String(error)) which yields "[object Object]" for Error
objects; change the call to pass a real message by extracting error.message or
falling back to a stringified value (e.g., onError(typeof error === "string" ?
error : error?.message ?? JSON.stringify(error))). Apply the same change to the
other catch in this file (handleSocialAuth around line ~50) so both handlers
send meaningful error text to onError.
| {configs?.enabledSocialProviders.includes("google") && ( | ||
| <Button type="button" onClick={() => handleSocialAuth("google")}> | ||
| <SiGoogle /> | ||
| </Button> | ||
| )} | ||
|
|
||
| {configs?.enabledSocialProviders.includes("github") && ( | ||
| <Button type="button" onClick={() => handleSocialAuth("github")}> | ||
| <SiGithub /> | ||
| GitHub | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify synchronization between client auth config and server-enabled providers.
The UI renders buttons based on configs.enabledSocialProviders from the server, but the auth client in client/src/lib/auth.ts has a hardcoded socialProviders: ['google', 'github'] array. This creates a potential synchronization issue:
- If the server enables a social provider not listed in the auth client's
socialProvidersarray, the button will render but authentication may fail. - Adding new social providers would require updating both the server configuration AND the client auth initialization.
Please verify whether the auth client can handle social providers dynamically or if the socialProviders array needs to be kept in sync with server configuration.
Run this script to check how social providers are used throughout the codebase:
🏁 Script executed:
#!/bin/bash
# Check auth client configuration and social provider usage
# Find auth client configurations
echo "=== Auth client configurations ==="
rg -n "socialProviders" --type=ts --type=tsx -C3
# Find where social auth is called
echo -e "\n=== Social auth sign-in calls ==="
rg -n "signIn\.social" --type=ts --type=tsx -C3
# Check if there are other references to social provider configuration
echo -e "\n=== Server-side social provider configs ==="
rg -n "enabledSocialProviders" --type=ts --type=tsx -C3Length of output: 198
🏁 Script executed:
#!/bin/bash
# Check auth client configuration and social provider usage
# Find auth client configurations
echo "=== Auth client configurations ==="
rg -n "socialProviders" -C3
# Find where social auth is called
echo -e "\n=== Social auth sign-in calls ==="
rg -n "signIn" -C3 | head -50
# Check if there are other references to social provider configuration
echo -e "\n=== Server-side social provider configs ==="
rg -n "enabledSocialProviders" -C3
# Check auth.ts file directly
echo -e "\n=== Content of auth.ts ==="
fd -t f "auth.ts" -x cat {}Length of output: 15275
Address the synchronization issue between client auth configuration and server-enabled social providers.
The review comment correctly identifies a real synchronization problem. Code inspection confirms:
- Server side (
server/src/lib/auth.ts:119): Social providers are dynamically configured viagetSocialProviders()function - Server config API (
server/src/api/getConfig.ts:14): ExposesenabledSocialProvidersto the client dynamically - Client side (
client/src/lib/auth.ts): The auth client has hardcodedsocialProviders: ['google', 'github']with an explicit TODO comment: "Load socialProviders from configs, but we can't use hooks here" - Client UI (
client/src/components/auth/SocialButtons.tsx:68-80): Correctly renders buttons based on the dynamicconfigs?.enabledSocialProviders, but callsauthClient.signIn.social({ provider })which relies on the hardcoded list
The issue: If the server enables an additional provider (e.g., 'discord'), the UI button will render, but authentication will fail because the client auth library doesn't recognize it.
To fix: Refactor the auth client initialization to load social providers dynamically. The blocker ("can't use hooks here") requires either using a different approach at module initialization or restructuring how the auth client is created/configured.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/package.json`:
- Line 68: The Next.js dependency entry "next": "^15.5.8" is unsafe; replace it
with a pinned, patched release (e.g. set "next" to the exact patched version
recommended by the Next.js security advisory) and remove the caret to avoid
accidental resolution to 15.5.8; update package.json accordingly, regenerate the
lockfile (npm/yarn/pnpm) and run npm install and npm audit/fix, then run your
test suite to verify compatibility (look for the "next" key in package.json to
locate the change).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
client/package.json
…invitation pages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/invitation/components/login.tsx`:
- Around line 25-33: The SSO flow always uses the first entry in
configs.enabledOIDCProviders, causing incorrect IdP selection for multi-provider
setups; update the UI and logic (e.g., handleSSOLogin and wherever the “Login
with SSO” button is rendered) to allow selecting the correct provider: present a
provider chooser when configs.enabledOIDCProviders.length > 1 (dropdown, menu,
or per-provider buttons), pass the chosen provider.providerId into
authClient.signIn.oauth2(callbackURL) instead of using enabledOIDCProviders[0],
and ensure any other SSO trigger (the code around the second occurrence
described) uses the same chosen provider value.
- Around line 72-73: The conditional rendering currently uses
configs?.internalAuthEnabled which hides the internal credential login when the
config is missing; change the check to treat undefined as enabled (e.g., use the
nullish-coalescing or explicit !== false pattern) so the block that renders the
credential login (the JSX guarded by configs?.internalAuthEnabled) shows by
default; update the condition around that fragment in the login.tsx component to
use configs?.internalAuthEnabled ?? true (or configs?.internalAuthEnabled !==
false) so only an explicit false disables it.
In `@client/src/app/login/page.tsx`:
- Around line 103-153: AuthError is currently rendered only when
configs?.internalAuthEnabled is true, so SSO/social login errors are hidden when
internal auth is disabled; move the <AuthError error={error} title={t("Error
Logging In")} /> component out of the internal-auth-only conditional (render it
alongside or below the social/SSO login UI) so it always displays regardless of
configs?.internalAuthEnabled, ensuring it still receives the same error and
title props and that any state variables (error) used by handleSubmit and SSO
handlers remain in scope.
- Around line 78-79: The UI currently treats an unset
configs.internalAuthEnabled as false due to truthy checks, hiding email/password
auth; update both conditionals that reference configs.internalAuthEnabled to
explicitly default to true when undefined (use the nullish coalescing operator).
For example, replace occurrences like !configs.internalAuthEnabled with
!(configs.internalAuthEnabled ?? true) and replace configs?.internalAuthEnabled
with configs?.internalAuthEnabled ?? true so the Login page logic (the
conditional around enabledOIDCProviders and the later redirect/display logic)
preserves default-to-enabled behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdclient/src/app/invitation/components/login.tsxclient/src/app/login/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
| const handleSSOLogin = async () => { | ||
| if (!configs?.enabledOIDCProviders.length) return; | ||
|
|
||
| const provider = configs.enabledOIDCProviders[0]; | ||
| try { | ||
| await authClient.signIn.oauth2({ | ||
| providerId: provider.providerId, | ||
| callbackURL, | ||
| }); |
There was a problem hiding this comment.
SSO button always picks the first provider, which is unsafe for multi-provider configs.
At Line 28-32 and Line 97-105, users get a generic “Login with SSO” button, but the implementation always authenticates with enabledOIDCProviders[0]. In multi-provider setups, this can direct users to the wrong IdP.
Suggested fix
- const handleSSOLogin = async () => {
- if (!configs?.enabledOIDCProviders.length) return;
-
- const provider = configs.enabledOIDCProviders[0];
+ const oidcProviders = configs?.enabledOIDCProviders ?? [];
+ const handleSSOLogin = async (providerId: string) => {
try {
await authClient.signIn.oauth2({
- providerId: provider.providerId,
+ providerId,
callbackURL,
});
} catch (err) {
setError(String(err));
}
};
@@
- {configs?.enabledOIDCProviders.length ? (
- <AuthButton
- isLoading={false}
- type="button"
- variant="default"
- onClick={handleSSOLogin}
- >
- Login with SSO
- </AuthButton>
- ) : null}
+ {oidcProviders.map(provider => (
+ <AuthButton
+ key={provider.providerId}
+ isLoading={false}
+ type="button"
+ variant="default"
+ onClick={() => handleSSOLogin(provider.providerId)}
+ >
+ {provider.name}
+ </AuthButton>
+ ))}Also applies to: 97-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/invitation/components/login.tsx` around lines 25 - 33, The SSO
flow always uses the first entry in configs.enabledOIDCProviders, causing
incorrect IdP selection for multi-provider setups; update the UI and logic
(e.g., handleSSOLogin and wherever the “Login with SSO” button is rendered) to
allow selecting the correct provider: present a provider chooser when
configs.enabledOIDCProviders.length > 1 (dropdown, menu, or per-provider
buttons), pass the chosen provider.providerId into
authClient.signIn.oauth2(callbackURL) instead of using enabledOIDCProviders[0],
and ensure any other SSO trigger (the code around the second occurrence
described) uses the same chosen provider value.
| {configs?.internalAuthEnabled && ( | ||
| <> |
There was a problem hiding this comment.
Internal auth should default to enabled when config is unset.
At Line 72, gating on configs?.internalAuthEnabled can hide credential login unless config is explicitly loaded with true. This weakens the intended default behavior.
Suggested fix
export function Login({ callbackURL }: LoginProps) {
@@
const { configs } = useConfigs();
+ const internalAuthEnabled = configs?.internalAuthEnabled ?? true;
@@
- {configs?.internalAuthEnabled && (
+ {internalAuthEnabled && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {configs?.internalAuthEnabled && ( | |
| <> | |
| export function Login({ callbackURL }: LoginProps) { | |
| const { configs } = useConfigs(); | |
| const internalAuthEnabled = configs?.internalAuthEnabled ?? true; | |
| // ... other code ... | |
| {internalAuthEnabled && ( | |
| <> | |
| {/* internal auth form content */} | |
| </> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/invitation/components/login.tsx` around lines 72 - 73, The
conditional rendering currently uses configs?.internalAuthEnabled which hides
the internal credential login when the config is missing; change the check to
treat undefined as enabled (e.g., use the nullish-coalescing or explicit !==
false pattern) so the block that renders the credential login (the JSX guarded
by configs?.internalAuthEnabled) shows by default; update the condition around
that fragment in the login.tsx component to use configs?.internalAuthEnabled ??
true (or configs?.internalAuthEnabled !== false) so only an explicit false
disables it.
| if (!isLoadingConfigs && configs && !configs.internalAuthEnabled && configs.enabledOIDCProviders.length === 1) { | ||
| const provider = configs.enabledOIDCProviders[0]; |
There was a problem hiding this comment.
Default-to-enabled auth behavior is not preserved in the UI.
At Line 78 and Line 103, the checks rely on truthiness (!configs.internalAuthEnabled / configs?.internalAuthEnabled) instead of explicit defaulting. If internalAuthEnabled is unset, this can hide email/password auth and alter redirect behavior.
Suggested fix
export default function Page() {
const { configs, isLoading: isLoadingConfigs } = useConfigs();
+ const internalAuthEnabled = configs?.internalAuthEnabled ?? true;
+ const enabledOidcProviders = configs?.enabledOIDCProviders ?? [];
@@
- useEffect(() => {
- if (!isLoadingConfigs && configs && !configs.internalAuthEnabled && configs.enabledOIDCProviders.length === 1) {
- const provider = configs.enabledOIDCProviders[0];
+ useEffect(() => {
+ if (!isLoadingConfigs && configs && internalAuthEnabled === false && enabledOidcProviders.length === 1) {
+ const provider = enabledOidcProviders[0];
authClient.signIn.oauth2({
providerId: provider.providerId,
callbackURL: "/",
}).catch(err => {
setError(String(err));
});
}
- }, [configs, isLoadingConfigs]);
+ }, [configs, isLoadingConfigs, internalAuthEnabled, enabledOidcProviders]);
@@
- {configs?.internalAuthEnabled && (
+ {internalAuthEnabled && (Also applies to: 103-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/login/page.tsx` around lines 78 - 79, The UI currently treats
an unset configs.internalAuthEnabled as false due to truthy checks, hiding
email/password auth; update both conditionals that reference
configs.internalAuthEnabled to explicitly default to true when undefined (use
the nullish coalescing operator). For example, replace occurrences like
!configs.internalAuthEnabled with !(configs.internalAuthEnabled ?? true) and
replace configs?.internalAuthEnabled with configs?.internalAuthEnabled ?? true
so the Login page logic (the conditional around enabledOIDCProviders and the
later redirect/display logic) preserves default-to-enabled behavior.
| {configs?.internalAuthEnabled && ( | ||
| <form onSubmit={handleSubmit}> | ||
| <div className="flex flex-col gap-4"> | ||
| <AuthInput | ||
| id="email" | ||
| label={t("Email")} | ||
| type="email" | ||
| placeholder="example@email.com" | ||
| required | ||
| value={email} | ||
| onChange={e => setEmail(e.target.value)} | ||
| /> | ||
| )} | ||
|
|
||
| <AuthButton | ||
| isLoading={isLoading} | ||
| loadingText={t("Logging in...")} | ||
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | ||
| > | ||
| {t("Login")} | ||
| </AuthButton> | ||
| <AuthInput | ||
| id="password" | ||
| label={t("Password")} | ||
| type="password" | ||
| placeholder="••••••••" | ||
| required | ||
| value={password} | ||
| onChange={e => setPassword(e.target.value)} | ||
| rightElement={ | ||
| IS_CLOUD && ( | ||
| <Link href="/reset-password" className="text-xs text-muted-foreground hover:text-primary"> | ||
| {t("Forgot password?")} | ||
| </Link> | ||
| ) | ||
| } | ||
| /> | ||
|
|
||
| <AuthError error={error} title={t("Error Logging In")} /> | ||
| </div> | ||
| </form> | ||
| {turnstileEnabled && ( | ||
| <Turnstile | ||
| onSuccess={token => setTurnstileToken(token)} | ||
| onError={() => setTurnstileToken("")} | ||
| onExpire={() => setTurnstileToken("")} | ||
| className="flex justify-center" | ||
| /> | ||
| )} | ||
|
|
||
| <AuthButton | ||
| isLoading={isLoading} | ||
| loadingText={t("Logging in...")} | ||
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | ||
| > | ||
| {t("Login")} | ||
| </AuthButton> | ||
|
|
||
| <AuthError error={error} title={t("Error Logging In")} /> | ||
| </div> | ||
| </form> | ||
| )} |
There was a problem hiding this comment.
Auth errors are hidden when internal auth is disabled.
AuthError (Line 150) is inside the internal-auth-only block. If SSO/social login fails while internal auth is off, users won’t see the error message.
Suggested fix
- {configs?.internalAuthEnabled && (
+ {configs?.internalAuthEnabled && (
<form onSubmit={handleSubmit}>
<div className="flex flex-col gap-4">
@@
- <AuthError error={error} title={t("Error Logging In")} />
</div>
</form>
)}
+ <AuthError error={error} title={t("Error Logging In")} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {configs?.internalAuthEnabled && ( | |
| <form onSubmit={handleSubmit}> | |
| <div className="flex flex-col gap-4"> | |
| <AuthInput | |
| id="email" | |
| label={t("Email")} | |
| type="email" | |
| placeholder="example@email.com" | |
| required | |
| value={email} | |
| onChange={e => setEmail(e.target.value)} | |
| /> | |
| )} | |
| <AuthButton | |
| isLoading={isLoading} | |
| loadingText={t("Logging in...")} | |
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | |
| > | |
| {t("Login")} | |
| </AuthButton> | |
| <AuthInput | |
| id="password" | |
| label={t("Password")} | |
| type="password" | |
| placeholder="••••••••" | |
| required | |
| value={password} | |
| onChange={e => setPassword(e.target.value)} | |
| rightElement={ | |
| IS_CLOUD && ( | |
| <Link href="/reset-password" className="text-xs text-muted-foreground hover:text-primary"> | |
| {t("Forgot password?")} | |
| </Link> | |
| ) | |
| } | |
| /> | |
| <AuthError error={error} title={t("Error Logging In")} /> | |
| </div> | |
| </form> | |
| {turnstileEnabled && ( | |
| <Turnstile | |
| onSuccess={token => setTurnstileToken(token)} | |
| onError={() => setTurnstileToken("")} | |
| onExpire={() => setTurnstileToken("")} | |
| className="flex justify-center" | |
| /> | |
| )} | |
| <AuthButton | |
| isLoading={isLoading} | |
| loadingText={t("Logging in...")} | |
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | |
| > | |
| {t("Login")} | |
| </AuthButton> | |
| <AuthError error={error} title={t("Error Logging In")} /> | |
| </div> | |
| </form> | |
| )} | |
| {configs?.internalAuthEnabled && ( | |
| <form onSubmit={handleSubmit}> | |
| <div className="flex flex-col gap-4"> | |
| <AuthInput | |
| id="email" | |
| label={t("Email")} | |
| type="email" | |
| placeholder="example@email.com" | |
| required | |
| value={email} | |
| onChange={e => setEmail(e.target.value)} | |
| /> | |
| <AuthInput | |
| id="password" | |
| label={t("Password")} | |
| type="password" | |
| placeholder="••••••••" | |
| required | |
| value={password} | |
| onChange={e => setPassword(e.target.value)} | |
| rightElement={ | |
| IS_CLOUD && ( | |
| <Link href="/reset-password" className="text-xs text-muted-foreground hover:text-primary"> | |
| {t("Forgot password?")} | |
| </Link> | |
| ) | |
| } | |
| /> | |
| {turnstileEnabled && ( | |
| <Turnstile | |
| onSuccess={token => setTurnstileToken(token)} | |
| onError={() => setTurnstileToken("")} | |
| onExpire={() => setTurnstileToken("")} | |
| className="flex justify-center" | |
| /> | |
| )} | |
| <AuthButton | |
| isLoading={isLoading} | |
| loadingText={t("Logging in...")} | |
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | |
| > | |
| {t("Login")} | |
| </AuthButton> | |
| </div> | |
| </form> | |
| )} | |
| <AuthError error={error} title={t("Error Logging In")} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/login/page.tsx` around lines 103 - 153, AuthError is currently
rendered only when configs?.internalAuthEnabled is true, so SSO/social login
errors are hidden when internal auth is disabled; move the <AuthError
error={error} title={t("Error Logging In")} /> component out of the
internal-auth-only conditional (render it alongside or below the social/SSO
login UI) so it always displays regardless of configs?.internalAuthEnabled,
ensuring it still receives the same error and title props and that any state
variables (error) used by handleSubmit and SSO handlers remain in scope.
1 participant
This pull request adds support for Single Sign-On (SSO) via OpenID Connect (OIDC) providers, allowing users to sign in using external identity providers (in addition to existing social login options in case of cloud.) Internal email/password authentication can now be disabled (if unset, it defaults to enabled to keep w/ current behavior)
It introduces configuration and UI changes to enable and display available OIDC and social providers dynamically, and updates both frontend and backend to support these features. Also, cleaned up the display logic of the
<SocialButtons/>component to fetch both enabled OIDC and social providers from the server.OIDC providers can be added through the use of four environment variables, OIDC_{PROVIDER}_NAME, _CLIENT_ID, _CLIENT_SECRET, and _DISCOVERY_URL (currently does not support setting the specific token endpoint, auth endpoint, etc. but most / if not all SSO solutions provide the discovery url)
Also extended
docker-compose.ymlanddocker-compose.cloud.ymlwith example environment variables for configuring OIDC providers. [1] [2]README.mdto indicate SSO/OIDC support across solutions.Summary by CodeRabbit
New Features
Documentation