feat(auth): External OIDC authentication (SSO) and remove push-notification 2FA#335
Draft
runleveldev wants to merge 8 commits into
Draft
feat(auth): External OIDC authentication (SSO) and remove push-notification 2FA#335runleveldev wants to merge 8 commits into
runleveldev wants to merge 8 commits into
Conversation
Add openid-client@5 (CJS-compatible) for OIDC single sign-on and drop the now-unused qrcode dependency used by push-notification 2FA enrollment.
- utils/oidc.js: config-driven OIDC helper (enabled only when issuer, client id, and secret are all set) with PKCE/state/nonce, discovery, and callback handling. - Add oidcSubject (unique) and oidcIssuer columns via migration and model. - User.findOrProvisionFromOidc() matches by subject, then email, and optionally just-in-time provisions accounts with a random unusable password; add User.uniqueUid() helper.
- Add GET /auth/oidc/login and /auth/oidc/callback routes implementing the authorization-code flow with session-stored PKCE/state/nonce. - When OIDC is configured, password login and self-registration return 403 (oidc_enabled); remove the push-notification 2FA challenge logic. - Expose oidcEnabled via /health so the SPA can auto-redirect to the IdP; stop returning pushNotificationUrl from /session.
MFA is now delegated to the OIDC identity provider, so the bespoke push-approval 2FA is removed: - Drop the push_notification_* settings (GET/PUT/validation) and the twoFactorWarning invite path from user creation. - Delete utils/push-notification-invite.js and the dead push_notification_url lookup in the currentSite middleware. - Add a migration that removes the obsolete push_notification_* settings.
- LoginPage auto-redirects to /auth/oidc/login when oidcEnabled, shows a friendly SSO error/retry screen on ?oidc_error, and only renders the password form when OIDC is off. - Remove all push-notification 2FA UI: challenge polling, QR enrollment on the register pages, the MFA Admin sidebar link, the twoFactorWarning toast, and the push-notification settings section. - Update auth/types models: add oidcEnabled to ServerInfo; drop pushNotificationUrl, twoFactorWarning, and challenge types.
- example.env: document OIDC_ISSUER_URL, client credentials, redirect URI, scopes, JIT provisioning, and post-logout redirect. - openapi.v1.yaml: add /auth/oidc/login and /auth/oidc/callback, note the 403 on /auth/login when OIDC is enabled, and remove the 2FA challenge and 2fa-qr endpoints.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds external OpenID Connect (OIDC) single sign-on (SSO) to the app and removes the bespoke push-notification 2FA flow, delegating MFA to the configured identity provider.
Changes:
- Introduces OIDC utilities + API routes (login + callback) and exposes
oidcEnabledvia/healthfor SPA auto-redirect. - Extends the User model + migrations to store OIDC identity linkage and supports optional JIT provisioning.
- Removes push-notification 2FA backend, related settings, OpenAPI docs, and all client UI paths.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| create-a-container/utils/push-notification-invite.js | Removes obsolete push-notification 2FA invite utility. |
| create-a-container/utils/oidc.js | Adds OIDC client/discovery, PKCE auth request builder, and callback handler. |
| create-a-container/routers/api/v1/users.js | Removes push-2FA invite-on-activation behavior and response field. |
| create-a-container/routers/api/v1/settings.js | Removes push-notification settings fields and validation from API. |
| create-a-container/routers/api/v1/index.js | Adds oidcEnabled to /health and removes MFA-admin session payload. |
| create-a-container/routers/api/v1/auth.js | Adds OIDC login/callback routes; disables internal login/registration when OIDC is enabled; removes push-2FA flow. |
| create-a-container/package.json | Replaces qrcode with openid-client dependency. |
| create-a-container/openapi.v1.yaml | Updates Auth endpoints: removes 2FA polling + adds OIDC endpoints and new login behavior. |
| create-a-container/models/user.js | Adds OIDC fields and implements JIT provisioning + account linking logic. |
| create-a-container/migrations/20260604000001-add-oidc-fields-to-users.js | Adds oidcSubject/oidcIssuer columns. |
| create-a-container/migrations/20260604000002-remove-push-notification-settings.js | Deletes obsolete push-notification settings (with rollback recreation). |
| create-a-container/middlewares/currentSite.js | Removes push-notification URL exposure to views/layout. |
| create-a-container/example.env | Documents OIDC environment variables and enablement rules. |
| create-a-container/client/src/pages/users/UserFormPage.tsx | Removes 2FA invite warning toast handling. |
| create-a-container/client/src/pages/settings/SettingsPage.tsx | Removes push-notification settings UI and form schema fields. |
| create-a-container/client/src/pages/auth/RegisterSuccessPage.tsx | Removes push-2FA enrollment QR flow on registration success page. |
| create-a-container/client/src/pages/auth/RegisterPage.tsx | Removes registration response fields related to 2FA enrollment. |
| create-a-container/client/src/pages/auth/LoginPage.tsx | Adds OIDC auto-redirect + OIDC error UI and removes push-2FA polling UI. |
| create-a-container/client/src/lib/types.ts | Removes push-notification settings + warning fields from client types. |
| create-a-container/client/src/lib/auth.ts | Removes 2FA challenge API/types; adds oidcEnabled to server info type. |
| create-a-container/client/src/app/Sidebar.tsx | Removes “MFA Admin” external link behavior. |
Files not reviewed (1)
- create-a-container/package-lock.json: Language not supported
Comment on lines
10
to
13
| Button, | ||
| Input, | ||
| Spinner, | ||
| usePrefersReducedMotion, | ||
| } from '@mieweb/ui'; |
Comment on lines
+90
to
+112
| if (claims.sub) { | ||
| const linked = await User.findOne({ | ||
| where: { oidcSubject: claims.sub }, | ||
| ...includeGroups, | ||
| }); | ||
| if (linked) return { user: linked }; | ||
| } | ||
|
|
||
| if (claims.email) { | ||
| const byEmail = await User.findOne({ | ||
| where: { mail: claims.email }, | ||
| ...includeGroups, | ||
| }); | ||
| if (byEmail) { | ||
| // Link the OIDC identity to the existing local account. | ||
| if (!byEmail.oidcSubject && claims.sub) { | ||
| byEmail.oidcSubject = claims.sub; | ||
| byEmail.oidcIssuer = claims.issuer || null; | ||
| await byEmail.save(); | ||
| } | ||
| return { user: byEmail }; | ||
| } | ||
| } |
Comment on lines
+203
to
211
| oidcSubject: { | ||
| type: DataTypes.STRING(255), | ||
| allowNull: true, | ||
| unique: true | ||
| }, | ||
| oidcIssuer: { | ||
| type: DataTypes.STRING(255), | ||
| allowNull: true | ||
| } |
Comment on lines
+58
to
+71
| static async uniqueUid(base) { | ||
| const sanitized = (base || 'user') | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9._-]/g, '') | ||
| .replace(/^[._-]+/, '') || 'user'; | ||
| let candidate = sanitized; | ||
| let suffix = 1; | ||
| // eslint-disable-next-line no-await-in-loop | ||
| while (await User.findOne({ where: { uid: candidate } })) { | ||
| candidate = `${sanitized}${suffix}`; | ||
| suffix += 1; | ||
| } | ||
| return candidate; | ||
| } |
Comment on lines
+53
to
+69
| // Lazily discover the issuer and build a Client. Cached after first success. | ||
| let cachedClient = null; | ||
| async function getClient(redirectUri) { | ||
| if (!isOidcEnabled()) { | ||
| throw new Error('OIDC is not configured'); | ||
| } | ||
| if (!cachedClient) { | ||
| const issuer = await Issuer.discover(process.env.OIDC_ISSUER_URL); | ||
| cachedClient = new issuer.Client({ | ||
| client_id: process.env.OIDC_CLIENT_ID, | ||
| client_secret: process.env.OIDC_CLIENT_SECRET, | ||
| redirect_uris: redirectUri ? [redirectUri] : undefined, | ||
| response_types: ['code'], | ||
| }); | ||
| } | ||
| return cachedClient; | ||
| } |
Comment on lines
+164
to
+165
| const pending = req.session.oidc; | ||
| const fail = (code) => res.redirect(`/login?oidc_error=${encodeURIComponent(code)}`); |
Signing out only cleared the local Manager session, leaving the IdP session alive. The login page then auto-redirected to the IdP, which silently re-issued a login — so users could never actually sign out. - utils/oidc: add buildEndSessionUrl() to construct the IdP end-session URL (id_token_hint + post_logout_redirect_uri); capture the raw id_token from the callback for use as the hint. - auth router: store the id_token on the session at login; logout now returns a `logoutUrl` to the IdP end-session endpoint when OIDC is enabled, defaulting the post-logout target to /login?logged_out=1. Falls back to local-only logout if the IdP has no end-session endpoint. - client: redirect the browser to `logoutUrl` immediately after the logout POST resolves, before clearing the query cache, so the IdP navigation isn't beaten by the SPA's re-render -> /login -> SSO auto-redirect. Suppress that auto-redirect when ?logged_out=1 is present and show a "Signed out" confirmation. - example.env: document OIDC_POST_LOGOUT_REDIRECT_URI default/behavior.
- New admins/oidc.md: enabling OIDC, env-var reference, IdP client registration, user matching/JIT provisioning, RP-initiated sign-out, recovery, and troubleshooting. Includes an authentik-specific note on registering the post-logout redirect URI (Logout type + Strict/Regex matching) to avoid 400 invalid_post_logout_redirect_uri. - Add the guide to the Admins nav and overview index. - settings.md: drop the removed Push Notification 2FA section; point to OIDC for delegated auth/MFA. - ldap-servers.md: remove obsolete push-notification env vars (NOTIFICATION_URL, sql,notification) now that that backend is gone.
| if (sessionLoading || (session && !challengeId)) { | ||
| // Avoid flashing the form while we resolve the session / server info, or | ||
| // while we hand off to the identity provider. | ||
| if (sessionLoading || serverInfoLoading || (session && !sessionLoading) || shouldAutoRedirectToIdp) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds OIDC (OpenID Connect) single sign-on to the create-a-container manager and removes the bespoke push-notification 2FA in favor of delegating MFA to the identity provider.
Addresses the OIDC option in #323.
When an IdP is configured, the login page automatically redirects to it; internal password login and self-registration are disabled. When OIDC is not configured, behavior is unchanged (internal password auth remains the default).
How it works
OIDC_ISSUER_URL,OIDC_CLIENT_ID, andOIDC_CLIENT_SECRETare all set.state, andnoncestored in the session; callback at/api/v1/auth/oidc/callback.oidcSubject→ email → optional just-in-time provisioning (gated byOIDC_JIT_PROVISION=true)./healthexposesoidcEnabledso the SPA can auto-redirect; the login screen shows a friendly retry/error view on?oidc_error.POST /api/v1/auth/devstill works).Commits (one per major step)
build(deps)— replaceqrcodewithopenid-client@5feat(auth)— OIDC client module,oidcSubject/oidcIssuerfields + migration, JIT provisioningfeat(auth)— OIDC login/callback routes; disable internal login when enabled;oidcEnabledon/healthrefactor— remove push-notification 2FA backend (settings, invite util, cleanup migration)feat(client)— OIDC auto-redirect login + remove all 2FA UIdocs—example.envOIDC vars + OpenAPI spec updatesNew environment variables
OIDC_ISSUER_URLOIDC_CLIENT_IDOIDC_CLIENT_SECRETOIDC_REDIRECT_URIOIDC_SCOPESopenid profile emailOIDC_JIT_PROVISIONtrueto auto-create users on first loginOIDC_POST_LOGOUT_REDIRECT_URIVerification
npm run client:type-check✅npm run client:build✅node -con all changed backend files + both migrations ✅Notes / follow-ups
userPassword(keeps theNOT NULLcolumn). Such users cannot password-authenticate to LDAP/sssd container shells — container SSH auth is out of scope here and tracked separately under External Authentication #323 (LDAP/SSH-key/short-lived-cert options).npm run db:migrate. The cleanup migration removes the obsoletepush_notification_*settings.Draft / excluded
Opened as a draft pending an end-to-end test against a real IdP. Two unrelated local working-tree changes (
public/logo.png,client/package.jsonbuild:watchflag) were intentionally left out of this branch.