feat(auth)!: multi provider authentication system#183
Conversation
Replaces single provider OIDC/LDAP with a flexible multi-provider architecture supporting multiple simultaneous instances per type and adding OAuth 2.0 Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Move cookie string literals to AUTH_COOKIES constant. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
| /> | ||
| </div> | ||
| <div class="flex items-center gap-4"> | ||
| <label class="w-48 text-left text-sm font-medium">Callback URL</label> |
There was a problem hiding this comment.
you should consider adding a help tooltip to clarify that this default value is likely good enough and that you will need to provide this callback url to the auth provider
There was a problem hiding this comment.
Addressed in 847dc40. Added callback URL help text for OIDC and OAuth.
| @@ -1,20 +1,52 @@ | |||
| <template> | |||
| <dl class="mb-6 space-y-6 divide-y divide-gray-100 border-b border-t border-gray-200 pb-6 pt-6 text-sm leading-6"> | |||
| <!-- Default Login Tab --> | |||
There was a problem hiding this comment.
have a tooltip that explains that the 'default tab' is for when you have to type in your username/password, and that the remainder of the SSO options will show up on the list below
There was a problem hiding this comment.
I can definitely summarize more but it is getting a bit wordy. Maybe stop at buttons for SSO. @Amndeep7
There was a problem hiding this comment.
"
Default Login Tab
Choose the tab shown when users open the login page. Only Local and LDAP providers appear as tabs; OAuth and OIDC providers are always visible as buttons so they cannot be set as the default.
"
As a test, I sent this pic and a screencap of the login page of TIR to the AI and it recommended the above (slightly tweaked by me) language as a more concise version.
I think this new text is better cause a) it avoids unnecessary details like why local/ldap providers are tabs - users won't care so why bother telling them, b) removes stuff like where things are positioned (ex. 'below that form') which is just tech debt for the next style update, and c) makes it a verb at the beginning "Choose to do X" instead of passive voice.
Feel free to use the above or come up with alternative text you like more.
There was a problem hiding this comment.
Done in 897e9e8 - went with your suggested wording for the Default Login Tab description. Thanks!
| </div> | ||
| </dd> | ||
|
|
||
| <!-- Local Auth --> |
There was a problem hiding this comment.
the default password requirements should probably match the AS&D (i.e. 15 chars, 1 of each character class)
There was a problem hiding this comment.
Addressed in 1ef555c. Local password defaults are now 15 chars with one of each character class.
| @@ -1,20 +1,52 @@ | |||
| <template> | |||
| <dl class="mb-6 space-y-6 divide-y divide-gray-100 border-b border-t border-gray-200 pb-6 pt-6 text-sm leading-6"> | |||
There was a problem hiding this comment.
it feels weird that the divider between the different auth options in their list is the same as the divider between all the other UI bits that appear vertically
note the horizontal line with same styling + size for everything
There was a problem hiding this comment.
Tracked in #194, folded into a broader provider layout rework.
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Groups are read from a configurable LDAP attribute (default: memberOf), CNs are extracted from DNs, and mapped to roles using the same groupName:roleId format as OIDC/OAuth. Access is denied when mappings are configured but the user matches no group. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Removes CN extraction so groups with identical CNs in different OUs are unambiguous. Pipe delimiter avoids conflicts with commas inside DNs. OIDC/OAuth mapping parsing is unchanged (still comma-delimited). Signed-off-by: Mark Rivera <mcrivera@gmail.com>
|
@markcrivera is this good to start reviewing yet? |
@Amndeep7 We are getting close. I decided to make the test buttons more robust instead of removing them. We're also doing our final round of integration testing for the auth methods. |
OIDC and OAuth use a popup window with postMessage to run the full auth flow without affecting the current admin session. LDAP uses an inline credential form with a dedicated test endpoint that validates credentials and resolves group mappings without creating a session or user record. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
When no group mappings are set, userRoleId is null but the real login defaults to role 2 (User). Display now uses denied flag as the signal for None rather than checking for an explicit roleId of 2. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
…onent Signed-off-by: Mark Rivera <mcrivera@gmail.com>
…ponents Reduces auth.vue from over 1000 lines. Each provider component manages its own test state and functions. OIDC and OAuth components properly clean up their postMessage listeners via onUnmounted. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Replace the per-provider .input-field styles with ring-based AppInput, AppSelect, and AppTextarea wrappers that match the rest of the app, and extract Local Auth into its own AuthLocalProvider component. The wrappers expose a size prop (xs/sm), and AppInput honors the number model modifier so v-model.number keeps working on the password-policy fields. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
…nput sizing Add dirty tracking via JSON snapshot comparison, saving state, and discard handler. Restore AppInput/AppSelect/AppTextarea to original border/padding style (py-1, border) and fix LocalProvider pt-6 regression from extraction. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
markcrivera
force-pushed
the
feat/multi-provider-auth
branch
from
937e495 to
f56dc86
Compare
Add for/id associations to the auth provider config labels so screen readers announce each control's name, resolving the SonarCloud accessibility findings (Web:S6853). UISlideSwitch gains an optional id prop forwarded to the underlying switch so toggles can be targeted by their labels. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Replace global parseInt/parseFloat/isNaN with their Number.* equivalents and mark never-reassigned class members as readonly, resolving the SonarCloud findings S7773 and S2933. No behavior change. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Use globalThis.window/globalThis.location instead of window for the location and SSR-guard accesses, resolving SonarCloud finding S7764. Window-specific APIs (addEventListener, open) are left unchanged. No behavior change. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Extract nested ternary operations into if/else blocks and guard clauses for role selection, TLS options, request body typing, and JWKS pluralization, resolving SonarCloud finding S3358. No behavior change. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Add the 'RoleId 1=Admin, 2=User' legend to the OAuth provider's group mappings helper text so it matches the OIDC and LDAP providers, per PR review feedback. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Default the local password policy to 15 characters with at least one upper, lower, number, and special character, per PR review feedback. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Add helper text under the OIDC and OAuth callback URL fields noting the default is usually fine and must be registered as an allowed callback/ redirect URL with the identity provider. Addresses PR review feedback. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
There was a problem hiding this comment.
Question: What happens when you do not have local auth nor LDAP (ex. github auth only)?
Like what happens to that complete ui section for the horizontal tabs + all the username/password fields?
There was a problem hiding this comment.
@markcrivera see this comment from late yesterday
There was a problem hiding this comment.
@Amndeep7 I did forget this.
Two issues are arising.
- When disabling username/password auth, it is not being removed from the selectable items.
- Username/password is still presented on the login screen with no username/password auth providers selected.
Working a change now.
There was a problem hiding this comment.
Fixed in db880d5 - when no Local or LDAP provider is enabled, the login page no longer renders the tab bar or the username/password form. Only the SSO buttons show, under a "Sign in with" header (instead of "or sign in with"). The consent control stays visible since the SSO buttons gate on it.
There was a problem hiding this comment.
Related follow-up in 82b072c: disabled Local/LDAP providers can no longer be set as the default. Their radios are disabled and greyed, and if the current default provider is later disabled or removed, the default reassigns to the first enabled provider.
|
@markcrivera review is likely going to be started next week - have a dr's apt tomorrow that'll take up most of the day |
Reword the Default Login Tab description per review: explain that only Local and LDAP appear as tabs while OAuth and OIDC are always shown as buttons and cannot be the default. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
When no Local or LDAP provider is enabled, the login page rendered an empty tab bar, a non-functional username/password form, and an 'or sign in with' divider. Hide the tab selector and credential fields when there is no credential provider, keep the consent control visible (the SSO buttons gate on it), and label the SSO section 'Sign in with' instead of 'or sign in with'. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Disable and grey the Default Login Tab radios for Local/LDAP providers that are not enabled so they cannot be selected. When the stored default points at a provider that is disabled or removed, reassign it to the first enabled provider. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Replace typeof globalThis.window === "undefined" with a direct === undefined check in the callbackUrl helpers (SonarCloud S7741). The property access cannot throw, so typeof is unnecessary. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Drop a redundant `as Exclude<OAuthProviderType, "custom">` cast (the else branch already narrows providerType, matching the unasserted lookup elsewhere) and an `as any` on an index into an any-typed options object (SonarCloud S4325). Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Replace !email || !email.includes("@") with !email?.includes("@") in the
LDAP/AD email fallback (SonarCloud S6582); nullish email short-circuits the
optional call to undefined, preserving the original behavior.
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Deduplicate escapeFilter, domainFromBaseDn, firstAttr, allAttrs, and the UAC constant across ldapAuthProvider and the ldap-login test route. Rewrite filter escaping to replaceAll/String.raw, clearing SonarCloud S7780/S7781. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Invert if/else and ternary tests so the positive branch leads, removing six "unexpected negated condition" smells across the OIDC/LDAP test routes, groupClaimExtractor, and authConfig. Behavior preserved. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Merge two consecutive checks.push() calls into one (S7778) in the OAuth test route, and lift the inner template literal in resolveRole out into a mappingSummary variable so the debug message is no longer a nested template (S4624). Behavior unchanged. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Replace the innermost forEach callback in insecureFetch with a for...of loop so header collection is no longer a function nested five levels deep. Behavior unchanged. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
Extract the three near-identical ldap/oidc/oauth provider-loading loops into a typed loadProviderGroup helper, and flatten the local fallback by naming the noProvidersEnabled guard instead of nesting an if. The helper's secretField is constrained to "password" | "secret". Attempts to address SonarCloud S3776. Signed-off-by: Mark Rivera <mcrivera@gmail.com>
|
markcrivera
changed the title
3 participants
Summary
Replaces the single-provider OIDC/LDAP setup with a flexible multi-provider authentication architecture: multiple simultaneous instances per provider type, plus a new OAuth 2.0 provider.
Changes
Provider architecture
API
UI
Data
Testing
Validated against real providers:
Related
closes #3
closes #171
BREAKING CHANGE: The AuthConfigKeyFormat migration changes how auth configuration is stored and is not backward compatible. Existing deployments must run the migration; rolling back to a pre-merge version after migrating is not supported.