Allow emails in usernames which facilitates SSO auth - #987
Merged
Conversation
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.
Email usernames for SSO, and opt-in linking of existing local accounts
Two related changes to how federated identities become osctrl accounts.
1. Usernames may now be email addresses
AdminUser.Usernamewas constrained to^[a-zA-Z0-9_-]{1,64}$, which rejects the value most IdPs actually identify people by. Entra ID, Okta and Google Workspace all emit a mailbox aspreferred_username/ NameID, so operators had to point the config at a short handle instead — andoidc.usernameClaim = emailwas self-defeating:pickUsernameselected the email, then the sanitizer rejected it.A username may now be either the original plain shape or an email address.
One validator, not three.
pkg/auth/oidcandpkg/auth/samleach carried a byte-identical copy of the regex andsanitizeUsername. Both now delegate topkg/utils.SanitizeUsername, so the accepted shape is defined once (and SCIM provisioning can reuse it).The email pattern is deliberately narrower than RFC 5322. The RFC permits quotes, slashes and backticks — precisely what the existing T23/T26 threat model exists to keep out, and nothing real emits them. The dot rules are encoded structurally rather than checked separately, so a leading dot, a trailing dot and
..are all unmatchable;..in a username would otherwise be a path-traversal primitive once interpolated into/api/v1/users/{username}.Email usernames are stored lowercased.
Get()does a rawusername = ?, so without canonicalizationJane@corp.comandjane@corp.comare two accounts on PostgreSQL but collide on MySQL's default collation — identity semantics differing by database backend. Plain handles are deliberately not folded: that would stop existing mixed-case accounts from matching their row on the next login.Removed
LegacyPermissiveUsername. It bypassed validation entirely — raw IdP value, newlines and NULs included — and existed only for the deletedcmd/admin's email usernames. Nothing set it true anymore, and proper email support is what made it obsolete. Deleting it closes a one-config-field bypass of the boundary this PR hardens.The
email_verifiedgate on theemailclaim is unchanged and now load-bearing: an unverified address falls back tosub, so a user who hasn't proven control of a mailbox can't claim the account belonging to whoever owns it.2. Federated login can link an existing local account
A federated login whose username matched an existing local password account was refused outright. With emails now usable as usernames, this is easy to trip over: an admin pre-creates
jane@corp.com, and her SSO login is then blocked by the account she was given.Linking is now available, opt-in per provider, default off:
(
OIDC_LINK_LOCAL_ACCOUNTS,SAML_LINK_LOCAL_ACCOUNTS,--oidc-link-local-accounts,--saml-link-local-accounts.)Why not automatic. Matching the IdP's email against the local account's email was the obvious candidate and doesn't hold up:
ResolvedIdentity.Emailis documented as untrusted (mutable, spoofable — threat T24), carries noemail_verified, and SAML has no equivalent signal. Auto-linking on same-name match is exactly the takeover vector the original block existed to stop — make the IdP assertadmin, inherit the local admin row. The authorization for linking is therefore operator intent, expressed once in config.Behavior:
adminflag, and written to the audit log with the client IP. The stored password is untouched, and linking grants nothing: a non-admin stays a non-admin, environment permissions unchanged.Rows already carrying an auth source were created by federated login in the first place, so cross-protocol re-matching (OIDC↔SAML, same IdP) stays unconditional as before.
Refactor
resolveFederatedUser(identity, jitProvision bool, authSource string)became(identity, federatedPolicy, clientIP). The call was about to grow to(identity, true, false, "oidc"), where transposing two adjacent bools would silently disable a security control.Changes
pkg/utils/username.go(new) —SanitizeUsername,IsEmailUsernamepkg/auth/{oidc,saml}— delegate to it;LegacyPermissiveUsernameremovedcmd/api/handlers/auth_resolve.go—federatedPolicy, linking branch, audit trailpkg/config—LinkLocalAccountson both providers, flags and env varsfrontend/src/features/users/UsersPage.tsx— the create-user form enforced the old regex; now mirrors the backend, lowercasing included, so a locally created account matches what SSO resolves toUsername rulesrewritten, newLinking existing local accountssection, both env-var tables, sample YAML, service-config field helpTesting
pkg/utilssuite: accept cases, lowercase canonicalization, and ~25 rejects including disguised-email attacks (alice@example.com\nadmin,alice@example.com/../root,al..ice@example.com,alice'@example.com).Alice@Example.comsurvives the callback asalice@example.com.go build/go vet/go test,make openapi-check, frontend 255 tests,tsc.Upgrade notes
No migration. Existing accounts and IdP configurations validate exactly as before — the plain shape is unchanged and plain handles are not case-folded. Anyone who was setting
LegacyPermissiveUsernamein Go code (nothing in-tree did) loses that field.Changing
usernameClaimon a live deployment creates new accounts rather than renaming existing ones — the username is the identity, soaliceandalice@example.comare two users with separate permissions. Documented in the guide.