OIDC Intergration - #1847
Open
Eveeifyeve wants to merge 8 commits into
Open
Conversation
Eveeifyeve
force-pushed
the
kjtsanaktsidis/oidc
branch
4 times, most recently
from
August 3, 2026 15:56
457212c to
bf3ba53
Compare
Member
|
@Eveeifyeve you can also squash those fixups if you are okay with the changes they do and you have tested the final result. I mainly kept them to make it more transparent why I did those fixes. |
Eveeifyeve
force-pushed
the
kjtsanaktsidis/oidc
branch
from
August 5, 2026 08:17
bf3ba53 to
5eb9452
Compare
This will be of use when developing & testing OIDC Bind Kanidm to 127.0.0.1 instead of [::1]. The Nix build sandbox only provides IPv4 loopback, so binding to ::1 made the server unreachable in CI (the health check timed out even though kanidm logged 'ready to rock'). Also dump the log tail on timeout since the sandbox file is gone by the time you'd want to read it.
The OIDC work needs to cache some information (at least, the JWKS keys) so we don't ask for them over and over again needlessly. An in-process cache is in theory fine, but using Plugin::Cache nicely gives us expiry support which we need too (to catch key revocations).
The login code in doEmailLogin is treating email address & username as the same concept, but it shouldn't be. Allow passing in a username field explicitly to doEmailLogin to set as the user's username column. The templates have also been changed to call a new usernameForDisplay method which will be able to customise the display of the username in circumstances where the username would look stupid (e.g. in the forthcoming OIDC support)
OIDC providers are configured in the config file under <oidc> <provider foobar>. There can be multiple providers, each one will get a menu item in the dropdown to log in with. After login is successful, we validate all the claims and then perform doEmailLogin, creating the user if it did not exist. fixup! Implement OIDC login Sanitize 'after' parameter to prevent open redirect: //evil.com would be interpreted as a protocol-relative URL by browsers. fixup! Implement OIDC login Use constant-time comparison for state and nonce to avoid timing side-channels. fixup! Implement OIDC login - Reject token endpoint responses with HTTP error but no 'error' field - Validate aud claim exists before dereferencing fixup! Implement OIDC login Don't let an unreachable IdP prevent Hydra from starting. Log the error and disable OIDC login instead of dying in setup_finalize. fixup! Implement OIDC login Remove unused imports (Data::Dumper debug leftover, sha256_hex). fixup! Implement OIDC login Drop redundant base64URLEncode wrapper: MIME::Base64::encode_base64url already strips '=' padding per RFC 4648 §5, so the s/=+$//r was a no-op. fixup! Implement OIDC login Collapse 4x repeated config validation into a loop. Bonus: the error now names which field is missing instead of listing all four. fixup! Implement OIDC login Let Crypt::JWT verify iss/aud/exp/nbf instead of reimplementing the OIDC §3.1.3.7 checklist by hand. The library already handles the array-or-scalar aud case and rejects alg=none. Keep only the OIDC-specific nonce check and sub presence check. fixup! Implement OIDC login Drop Crypt::URandom::Token dependency (and its flake overlay). base64url-encoding raw urandom bytes satisfies both RFC 6749 state/nonce and RFC 7636 PKCE verifier requirements without needing a custom token-from-charset generator. fixup! Implement OIDC login Modernize with Perl 5.36+ features: - Subroutine signatures replace 'my ($self, ...) = @_' boilerplate - try/catch replaces eval/$@ (avoids the $@-clobbered-by-destructor footgun and gives a lexically-scoped error variable) - Simplify scope construction with join+grep fixup! Implement OIDC login Make OIDC discovery lazy instead of blocking startup. Previously resolveOIDCConfig did an HTTP GET to the discovery_url in setup_finalize, meaning every Hydra start blocked on the IdP for up to 10s and an IdP that was down at boot left OIDC broken until restart. Now resolveOIDCConfig only loads secrets from disk and validates static config (no network). Discovery happens in _resolvedConf on the first login attempt, cached for 1h via Plugin::Cache and also written back into the in-process config so subsequent calls short-circuit. An IdP outage now only affects users trying to log in (503), and recovers automatically without a restart.
If presented with a "hydra_roles" claim, we will set the user roles to that claim. fixup! Add mapping of OIDC claims to roles Wrap setRoles INSERT+DELETE in a transaction so role updates are atomic.
fixup! Add test for OIDC implementation Move test-only Perl modules (TestWWWMechanize, HTTPCookieJar, ...) from the main hydra package to hydra-tests where they belong.
Without RP-Initiated Logout, users stay signed in at the IdP after logging out of Hydra, so clicking "Sign in" immediately re-authenticates without a password prompt. The endpoint is read from the discovery document or can be set manually. Adapted from https://git.lix.systems/lix-project/hydra/pulls/73 Co-authored-by: Jörg Thalheim <joerg@thalheim.io> fixup! hydra-server: redirect to OIDC end_session_endpoint on logout Add CSRF protection to GET /logout: require a token derived from the session ID (sha256(sessionid + ':logout')) so cross-site requests cannot log the user out. The token is exposed to templates via the stash and embedded in the Sign out link.
Eveeifyeve
force-pushed
the
kjtsanaktsidis/oidc
branch
from
August 5, 2026 08:20
5eb9452 to
3b442e2
Compare
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
4 participants
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.
Rebased #1568 pr, due to lack of activity as seen (see pr for descriptions of what this pr does)