feat: Keycloak SSO via better-auth genericOAuth#135
feat: Keycloak SSO via better-auth genericOAuth#135escooterclinic wants to merge 2 commits intoreqcore-inc:mainfrom
Conversation
- Add genericOAuth plugin for Keycloak SSO (server + client) - Add KC_CLIENT_ID/SECRET/DISCOVERY_URL env vars - Add SSO button to sign-in page - Fix MinIO console port conflict with Portainer (9001→9091) - Fix create-org auto-switch redirecting when user already has active org
📝 WalkthroughWalkthroughThe PR introduces Keycloak SSO authentication using the better-auth genericOAuth plugin, adding server-side OAuth configuration with environment variables and client-side sign-in UI. It also refines the onboarding auto-switch logic to check session state, adjusts MinIO's web console port mapping, and introduces an update script for deployment automation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignIn as Sign-in Page
participant AuthClient as Auth Client<br/>(genericOAuth)
participant Server as Auth Server
participant Keycloak as Keycloak/OIDC<br/>Provider
User->>SignIn: Click "Sign in with SSO"
SignIn->>SignIn: handleKeycloakSignIn()<br/>(set isLoading=true)
SignIn->>AuthClient: authClient.signIn.oauth2<br/>(providerId: 'keycloak')
AuthClient->>Server: OAuth2 authorization request
Server->>Keycloak: Redirect to Keycloak login
Keycloak->>User: Display login form
User->>Keycloak: Enter credentials & authorize
Keycloak->>Server: Return authorization code
Server->>Keycloak: Exchange code for tokens
Keycloak->>Server: Return ID/access tokens
Server->>Server: Extract user profile<br/>(name, email, picture)
Server->>SignIn: Redirect with session
SignIn->>SignIn: set isLoading=false
SignIn->>User: Navigate to dashboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 3
🧹 Nitpick comments (3)
server/utils/auth.ts (1)
113-117: Consider defensive handling for missing profile claims.If the OIDC provider doesn't return
name,given_name, orfamily_name(e.g., misconfigured or minimal claims), thenamefield becomes an empty string. Consider a fallback to the email prefix or a placeholder.Optional defensive fallback
mapProfileToUser: (profile) => ({ - name: profile.name || `${profile.given_name || ''} ${profile.family_name || ''}`.trim(), + name: profile.name + || `${profile.given_name || ''} ${profile.family_name || ''}`.trim() + || profile.email?.split('@')[0] + || 'SSO User', email: profile.email, image: profile.picture, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/auth.ts` around lines 113 - 117, The mapProfileToUser mapping can produce an empty name when profile lacks name/given_name/family_name; update mapProfileToUser to defensively compute name by first using profile.name, then joining given_name and family_name if present, and if still empty fallback to the local-part of profile.email (substring before '@') or a constant placeholder like "User" so name is never an empty string; ensure you reference mapProfileToUser and use profile.email only if present to avoid runtime errors.server/utils/env.ts (1)
91-96: Consider adding all-or-nothing validation for Keycloak config.Currently, if an operator sets only one or two of the
KC_*variables, SSO is silently disabled. Adding asuperRefinecheck to warn when the configuration is partial could improve operator experience during deployment.Example validation addition
}) .superRefine((data, ctx) => { + // Warn if Keycloak SSO is partially configured + const kcVars = [data.KC_CLIENT_ID, data.KC_CLIENT_SECRET, data.KC_DISCOVERY_URL] + const kcSet = kcVars.filter(Boolean).length + if (kcSet > 0 && kcSet < 3) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['KC_CLIENT_ID'], + message: 'Keycloak SSO requires all three: KC_CLIENT_ID, KC_CLIENT_SECRET, and KC_DISCOVERY_URL', + }) + } + // BETTER_AUTH_URL can be derived at runtime from RAILWAY_PUBLIC_DOMAIN,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/env.ts` around lines 91 - 96, Add an all-or-nothing validation to the environment zod schema so Keycloak config is either fully provided or fully absent: update the schema that currently defines KC_CLIENT_ID, KC_CLIENT_SECRET, and KC_DISCOVERY_URL (which use emptyToUndefined.pipe(z.string().min(1)).optional() / .url().optional()) to include a superRefine on the parent object (e.g., env schema) that checks if any of these KC_* fields are set then all must be set (otherwise add a clear ZodIssue with path(s) and message), and if none are set allow it; ensure you reference KC_CLIENT_ID, KC_CLIENT_SECRET, and KC_DISCOVERY_URL in the check.app/pages/auth/sign-in.vue (1)
90-98: SSO button is always visible, even when Keycloak is not configured.The button will be shown to all users regardless of whether the server has Keycloak enabled (via
KC_CLIENT_ID,KC_CLIENT_SECRET,KC_DISCOVERY_URL). While clicking it when unavailable triggers an error that is displayed (improving upon silent failure), the UX could be better by hiding the button entirely when SSO is disabled. Expose Keycloak availability via aNUXT_PUBLIC_variable innuxt.config.tsand conditionally render the button, or create a server endpoint that returns enabled providers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/auth/sign-in.vue` around lines 90 - 98, The SSO button is always shown even when Keycloak is not configured; fix by exposing Keycloak availability to the client and conditionally rendering the button. Add a boolean public flag (e.g. NUXT_PUBLIC_KEYCLOAK_ENABLED or NUXT_PUBLIC_KEYCLOAK_AVAILABLE) in nuxt.config.ts derived from KC_CLIENT_ID/ KC_CLIENT_SECRET/ KC_DISCOVERY_URL (or create a server endpoint that returns provider availability), then update sign-in.vue to check that flag before rendering the button and keep the existing click handler handleKeycloakSignIn unchanged so the button only appears when Keycloak is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/auth/sign-in.vue`:
- Around line 71-83: handleKeycloakSignIn currently hardcodes callbackURL to
localePath('/dashboard') and loses route.query.invitation; update
handleKeycloakSignIn to read the invitation from route.query (e.g., const
invitation = route.query.invitation) and preserve it when calling
authClient.signIn.oauth2 by including the invitation in the callback (or oauth
state) so the app can restore context after SSO. Modify the callbackURL passed
to authClient.signIn.oauth2 (or add a state param) to include the encoded
invitation value and keep the existing error/loading handling in
handleKeycloakSignIn.
In `@docker-compose.yml`:
- Line 26: Update the stale MinIO console URL comment for the port mapping "-
\"127.0.0.1:9091:9001\"" so it reflects the host port 9091 (e.g., change "MinIO
Console → http://localhost:9001" to "MinIO Console → http://localhost:9091") to
avoid operator confusion.
In `@update.sh`:
- Around line 33-38: Replace the brittle sleep+exec approach: stop using the
fixed sleep and instead run docker compose up with the --wait option (e.g., keep
detached if desired by including -d) to wait for service start, and when running
the migration use docker compose exec -T app npm run db:migrate so no TTY is
allocated in non-interactive CI; update the lines that currently call "docker
compose up -d", "sleep 5", and "docker compose exec app npm run db:migrate" to
use the --wait flag and add -T to the exec invocation.
---
Nitpick comments:
In `@app/pages/auth/sign-in.vue`:
- Around line 90-98: The SSO button is always shown even when Keycloak is not
configured; fix by exposing Keycloak availability to the client and
conditionally rendering the button. Add a boolean public flag (e.g.
NUXT_PUBLIC_KEYCLOAK_ENABLED or NUXT_PUBLIC_KEYCLOAK_AVAILABLE) in
nuxt.config.ts derived from KC_CLIENT_ID/ KC_CLIENT_SECRET/ KC_DISCOVERY_URL (or
create a server endpoint that returns provider availability), then update
sign-in.vue to check that flag before rendering the button and keep the existing
click handler handleKeycloakSignIn unchanged so the button only appears when
Keycloak is enabled.
In `@server/utils/auth.ts`:
- Around line 113-117: The mapProfileToUser mapping can produce an empty name
when profile lacks name/given_name/family_name; update mapProfileToUser to
defensively compute name by first using profile.name, then joining given_name
and family_name if present, and if still empty fallback to the local-part of
profile.email (substring before '@') or a constant placeholder like "User" so
name is never an empty string; ensure you reference mapProfileToUser and use
profile.email only if present to avoid runtime errors.
In `@server/utils/env.ts`:
- Around line 91-96: Add an all-or-nothing validation to the environment zod
schema so Keycloak config is either fully provided or fully absent: update the
schema that currently defines KC_CLIENT_ID, KC_CLIENT_SECRET, and
KC_DISCOVERY_URL (which use emptyToUndefined.pipe(z.string().min(1)).optional()
/ .url().optional()) to include a superRefine on the parent object (e.g., env
schema) that checks if any of these KC_* fields are set then all must be set
(otherwise add a clear ZodIssue with path(s) and message), and if none are set
allow it; ensure you reference KC_CLIENT_ID, KC_CLIENT_SECRET, and
KC_DISCOVERY_URL in the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 598285c3-d8e7-4585-bda7-156737614d1b
📒 Files selected for processing (7)
app/pages/auth/sign-in.vueapp/pages/onboarding/create-org.vueapp/utils/auth-client.tsdocker-compose.ymlserver/utils/auth.tsserver/utils/env.tsupdate.sh
| async function handleKeycloakSignIn() { | ||
| isLoading.value = true | ||
| error.value = '' | ||
| try { | ||
| await authClient.signIn.oauth2({ | ||
| providerId: 'keycloak', | ||
| callbackURL: localePath('/dashboard'), | ||
| }) | ||
| } catch (e: unknown) { | ||
| error.value = e instanceof Error ? e.message : 'SSO sign-in failed' | ||
| isLoading.value = false | ||
| } | ||
| } |
There was a problem hiding this comment.
SSO callback doesn't preserve pending invitation context.
The email sign-in flow checks route.query.invitation and redirects to the invitation acceptance page after authentication. The SSO flow hardcodes callbackURL: localePath('/dashboard'), so users accepting invitations via SSO will lose the invitation context.
Proposed fix to preserve invitation context
async function handleKeycloakSignIn() {
isLoading.value = true
error.value = ''
try {
+ const pendingInvitation = route.query.invitation as string | undefined
+ const callbackURL = pendingInvitation
+ ? localePath(`/auth/accept-invitation/${pendingInvitation}`)
+ : localePath('/dashboard')
await authClient.signIn.oauth2({
providerId: 'keycloak',
- callbackURL: localePath('/dashboard'),
+ callbackURL,
})
} catch (e: unknown) {
error.value = e instanceof Error ? e.message : 'SSO sign-in failed'
isLoading.value = false
}
}📝 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.
| async function handleKeycloakSignIn() { | |
| isLoading.value = true | |
| error.value = '' | |
| try { | |
| await authClient.signIn.oauth2({ | |
| providerId: 'keycloak', | |
| callbackURL: localePath('/dashboard'), | |
| }) | |
| } catch (e: unknown) { | |
| error.value = e instanceof Error ? e.message : 'SSO sign-in failed' | |
| isLoading.value = false | |
| } | |
| } | |
| async function handleKeycloakSignIn() { | |
| isLoading.value = true | |
| error.value = '' | |
| try { | |
| const pendingInvitation = route.query.invitation as string | undefined | |
| const callbackURL = pendingInvitation | |
| ? localePath(`/auth/accept-invitation/${pendingInvitation}`) | |
| : localePath('/dashboard') | |
| await authClient.signIn.oauth2({ | |
| providerId: 'keycloak', | |
| callbackURL, | |
| }) | |
| } catch (e: unknown) { | |
| error.value = e instanceof Error ? e.message : 'SSO sign-in failed' | |
| isLoading.value = false | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/auth/sign-in.vue` around lines 71 - 83, handleKeycloakSignIn
currently hardcodes callbackURL to localePath('/dashboard') and loses
route.query.invitation; update handleKeycloakSignIn to read the invitation from
route.query (e.g., const invitation = route.query.invitation) and preserve it
when calling authClient.signIn.oauth2 by including the invitation in the
callback (or oauth state) so the app can restore context after SSO. Modify the
callbackURL passed to authClient.signIn.oauth2 (or add a state param) to include
the encoded invitation value and keep the existing error/loading handling in
handleKeycloakSignIn.
| ports: | ||
| - "127.0.0.1:9000:9000" # S3 API — localhost only, never expose publicly | ||
| - "127.0.0.1:9001:9001" # MinIO Console → http://localhost:9001 | ||
| - "127.0.0.1:9091:9001" # MinIO Console → http://localhost:9001 |
There was a problem hiding this comment.
Fix stale MinIO console URL comment.
Line 26 maps host port 9091 to container port 9001, but the comment still points to http://localhost:9001. Please update it to avoid operator confusion.
✏️ Proposed fix
- - "127.0.0.1:9091:9001" # MinIO Console → http://localhost:9001
+ - "127.0.0.1:9091:9001" # MinIO Console → http://localhost:9091📝 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.
| - "127.0.0.1:9091:9001" # MinIO Console → http://localhost:9001 | |
| - "127.0.0.1:9091:9001" # MinIO Console → http://localhost:9091 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` at line 26, Update the stale MinIO console URL comment
for the port mapping "- \"127.0.0.1:9091:9001\"" so it reflects the host port
9091 (e.g., change "MinIO Console → http://localhost:9001" to "MinIO Console →
http://localhost:9091") to avoid operator confusion.
| echo "==> Restarting stack..." | ||
| docker compose up -d | ||
|
|
||
| echo "==> Running migrations..." | ||
| sleep 5 | ||
| docker compose exec app npm run db:migrate |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does docker compose execallocate a TTY by default (requiring-Tin non-interactive scripts), and what minimum Docker Compose version supportsdocker compose up --wait?
💡 Result:
- Yes.
docker compose execallocates a TTY by default, so in non-interactive/scripted usage you typically need-T/--no-tty(or--interactive=falsewhere appropriate) to avoid “not a TTY” issues. [1] - Minimum Docker Compose version for
docker compose up --wait:v2.7.0(it’s referenced as an existing behavior in the v2.7.0 release notes). [3]- The
docker compose upCLI docs also document--wait/--wait-timeout. [2]
- The
Sources: [1] (docs.docker.com) [2] (docs.docker.com) [3] (github.com)
Citations:
- 1: https://docs.docker.com/reference/cli/docker/compose/exec/
- 2: https://docs.docker.com/reference/cli/docker/compose/up/
- 3: https://github.com/docker/compose/releases/tag/v2.7.0
Harden migration execution for unattended runs.
Line 37 uses a fixed sleep and Line 38 uses docker compose exec without -T; this is brittle in automation. The exec command allocates a TTY by default, which causes failures in non-interactive contexts. Replace the sleep with proper health checking via docker compose up --wait (requires Docker Compose v2.7.0+) and add -T to prevent TTY allocation.
🔧 Proposed fix
echo "==> Restarting stack..."
-docker compose up -d
+docker compose up -d --wait
echo "==> Running migrations..."
-sleep 5
-docker compose exec app npm run db:migrate
+docker compose exec -T app npm run db:migrate📝 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.
| echo "==> Restarting stack..." | |
| docker compose up -d | |
| echo "==> Running migrations..." | |
| sleep 5 | |
| docker compose exec app npm run db:migrate | |
| echo "==> Restarting stack..." | |
| docker compose up -d --wait | |
| echo "==> Running migrations..." | |
| docker compose exec -T app npm run db:migrate |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@update.sh` around lines 33 - 38, Replace the brittle sleep+exec approach:
stop using the fixed sleep and instead run docker compose up with the --wait
option (e.g., keep detached if desired by including -d) to wait for service
start, and when running the migration use docker compose exec -T app npm run
db:migrate so no TTY is allocated in non-interactive CI; update the lines that
currently call "docker compose up -d", "sleep 5", and "docker compose exec app
npm run db:migrate" to use the --wait flag and add -T to the exec invocation.
|
Thank you, this is a really clean PR and exactly the right direction. The genericOAuth approach, the PKCE toggle, the conditional plugin registration, and the create-org auto-switch fix are all spot-on. While this was in open, I'd been building out a broader SSO implementation on feat/sso (now #136) that ended up covering this same ground plus a second tier (per-org enterprise SSO via Better Auth's sso plugin). Since the two overlap almost entirely, I'm going to close this in favour of #136 rather than try to merge both. A few things from your PR that directly influenced the final implementation: Really appreciate you taking the time on this. If you find any bugs with the SSO implementation, just let me know : ) |
2 participants
Summary
genericOAuthpluginKC_CLIENT_ID,KC_CLIENT_SECRET,KC_DISCOVERY_URL)Changes
server/utils/env.ts— 3 new optional env vars with Zod validationserver/utils/auth.ts— conditionalgenericOAuthplugin registration with profile mappingapp/utils/auth-client.ts—genericOAuthClient()plugin addedapp/pages/auth/sign-in.vue— SSO button +handleKeycloakSignIn()handlerapp/pages/onboarding/create-org.vue— guard against auto-switch when session already has active orgHow it works
When all 3
KC_*env vars are set, the genericOAuth plugin registers akeycloakprovider. The sign-in page shows an "Sign in with SSO" button that triggers the OIDC flow. User profile (name, email, avatar) is mapped from standard OIDC claims.Since this uses
genericOAuth(not a Keycloak-specific plugin), it works with any OIDC provider — Authentik, Authelia, Okta, Azure AD — just by changing the discovery URL.Keycloak setup
https://your-domain.com/api/auth/callback/keycloakhttps://keycloak.example.com/realms/REALM/.well-known/openid-configurationTest plan
Related: #134
Summary by CodeRabbit
New Features
Improvements
Chores