OIDC login #73

ma27 · 2025-12-24T10:32:21Z

ma27 commented

2025-12-24 10:32:21 +00:00

Closes #38

cc @benaryorg @raito

Based on https://github.com/NixOS/hydra/pull/1298

Additional features:

actually validate the id token
use well-known endpoint rather than having to configure dozens of endpoints by hand
use oidc logout method
removes google/github login

Todo

Deploy to a live instance
Migration code for google/github users
Docs

Closes #38 cc @benaryorg @raito Based on https://github.com/NixOS/hydra/pull/1298 Additional features: * actually validate the id token * use well-known endpoint rather than having to configure dozens of endpoints by hand * use oidc logout method * removes google/github login Todo * [x] Deploy to a live instance * [x] Migration code for google/github users * [x] Docs

ma27 added 10 commits

2025-12-24 10:32:22 +00:00

Implement generic OIDC-based authentication eb6541abc3

Co-Authored-By: Maximilian Bosch <maximilian@mbosch.me>
(cherry picked from commit f963dba1552d85f299859c8a78b1d9ef71f69e76)

Set convenient variables for db access in shellHook c430d964f7

(cherry picked from commit 039165e1dbad90bcf755f441c22d81ed5233f96b)

Make OIDC name configurable 4a67ec1fd5

(cherry picked from commit 9d8b420281fd0f9bc7443474747de32e341e9748)

Make Hydra login optional 4380bf119e

(cherry picked from commit 8d220903696a0be5459777d32ad7ef05c6ee7970)

Use session instead of cookie for OIDC 466c4496f1

(cherry picked from commit f9072362dd77fccb659913ed3c35bfb42a4d3a29)

Add OIDC role mapping c688aef68b

(cherry picked from commit a9c16a19518a238d74fce789e27dd166ef7058b1)

hydra-server: improve oidc configuration a73134f8dc

* Struct with all parameters inside
* Normalize / process all parameters initially and fail early
* Use well-known URL and derive all other URLs from that.

hydra-server: validate ID token and sub before completing auth 848994835f

hydra-server: remove google/github login 254262430f

We have generic OIDC now, no need for custom implementations!

hydra-server: logout redirects to oidc logout if user type is oidc ccd50ec7c5

ma27 force-pushed oidc from ccd50ec7c5 to 19e57225ba

2025-12-30 13:13:51 +00:00

Compare

ma27 changed title from ~~Draft: OIDC login~~ to OIDC login

2025-12-30 13:16:49 +00:00

lheckemann reviewed

2026-01-29 09:36:57 +00:00

lheckemann left a comment

A couple of typos, some practical concerns that I'm uncertain about.

Sorry for taking so long!

A couple of typos, some practical concerns that I'm uncertain about. Sorry for taking so long!

doc/manual/src/configuration.md Outdated

					
				@ -261,6 +261,72 @@ default role mapping:

				Note that configuring both the LDAP parameters in the hydra.conf and via

				the environment variable is a fatal error.

				OIDC Authentijcation backend

Typo: Authentication

ma27 marked this conversation as resolved

doc/manual/src/configuration.md Outdated

					
				@ -264,0 +270,4 @@

				```

				<oidc>

				  enable = 0    # 1 to enab le

typo: enable

ma27 marked this conversation as resolved

src/lib/Hydra/Config.pm Outdated

					
				@ -155,1 +159,4 @@

				my $oidcCfgCache;

				sub downloadSettings {

I think I preferred the previous name downloadOIDCSettings, there isn't really that much context that makes it obvious that this is about OIDC without reading the implementation or call site?

I think I preferred the previous name `downloadOIDCSettings`, there isn't really that much context that makes it obvious that this is about OIDC without reading the implementation or call site?

ma27 marked this conversation as resolved

src/lib/Hydra/Controller/User.pm Outdated

					
				@ -166,0 +230,4 @@

				    doEmailLogin($self, $c, "oidc", $claims->{email}, $claims->{name} // undef);

				    if ($oidcConfig->{groups_to_roles}) {

				        $c->user->userroles->delete;

This could lock an admin out if they've been using stateful role membership up to now? Seems potentially annoying. I think this should probably be mentioned in the docs at the very least.

I mean there are several relatively simple ways to recover:

hydra-create-user
Do the group mapping in the IdP.

Especially considering that this needs a flag to be enabled to work at all.

I mean there are several relatively simple ways to recover: * `hydra-create-user` * Do the group mapping in the IdP. Especially considering that this needs a flag to be enabled to work at all.

👍 1

lheckemann marked this conversation as resolved

src/lib/Hydra/Controller/User.pm Outdated

					
				@ -166,0 +237,4 @@

				            my @mapping = @{$oidcConfig->{groups_to_roles}->{$group}};

				            for my $mapped_role (@mapping) {

				                next unless $mapped_role;

				                $c->user->userroles->create({ role => $mapped_role });

I'm not confident enough in my knowledge of the system as a whole to be sure of any of what I'm saying here, but...

This empties and recreates the users' roles in the DB on every login? Probably not an issue realistically, but it does cause some churn.

"Caching" the roles like this could also cause confusion when looking at users in the admin UI:

Stateful changes are still allowed, but will be overridden by the login?
They won't be updated if the group memberships change on the OIDC side, at least until the user logs in again. The stale info may be confusing.

Maybe the roles should be stored in the session instead, and the admin UI should make sure to show that the roles are unknown due to coming from OIDC mapping?

I'm not confident enough in my knowledge of the system as a whole to be sure of any of what I'm saying here, but... This empties and recreates the users' roles in the DB on every login? Probably not an issue realistically, but it does cause some churn. "Caching" the roles like this could also cause confusion when looking at users in the admin UI: - Stateful changes are still allowed, but will be overridden by the login? - They won't be updated if the group memberships change on the OIDC side, at least until the user logs in again. The stale info may be confusing. Maybe the roles should be stored in the session instead, and the admin UI should make sure to show that the roles are unknown due to coming from OIDC mapping?

Wouldn't it be way easier to just turn off that part of the UI when the user type is OIDC rather than having two places where roles can be stored?

Sure, that would be fine by me too.

ma27 marked this conversation as resolved

src/lib/Hydra/Controller/User.pm Outdated

					
				@ -166,0 +196,4 @@

				            accepted_alg => [$alg],

				            verify_sub => sub { return 1; },

				            verify_aud => sub { return 1; },

				            verify_iat => sub { return 1; }

We should probably validate the issued-at time in some way?

This is already happening in decode_jwt: https://metacpan.org/dist/Crypt-JWT/source/lib/Crypt/JWT.pm#L239

I wonder if we should prohibit stuff that's too far in the past though?

This is already happening in decode_jwt: https://metacpan.org/dist/Crypt-JWT/source/lib/Crypt/JWT.pm#L239 I wonder if we should prohibit stuff that's too far in the past though?

ma27 marked this conversation as resolved

src/lib/Hydra/Controller/User.pm Outdated

					
				@ -166,0 +204,4 @@

				    }

				    unless ($decoded_token->{aud} eq $oidcConfig->{client_id}) {

				        error($c, "Token audience/subject don't match ID token", 401);

Shouldn't this be in verify_aud function passed to decode_jwt above?

ma27 marked this conversation as resolved

ma27 force-pushed oidc from 19e57225ba to 6085ed2bad

2026-02-06 15:38:21 +00:00

Compare

ma27 force-pushed oidc from 6085ed2bad to 3ca2e2fd7a

2026-02-06 15:44:56 +00:00

Compare

ma27 force-pushed oidc from 3ca2e2fd7a to 2b4d04e173

2026-02-06 16:00:21 +00:00

Compare

ma27 force-pushed oidc from 2b4d04e173 to cccec0822a

2026-02-07 17:59:21 +00:00

Compare

ma27 commented

2026-03-06 10:12:09 +00:00

Good to go IMHO.
cc @lheckemann @raito @delroth if you want to give it a final review.

Good to go IMHO. cc @lheckemann @raito @delroth if you want to give it a final review.

raito approved these changes

2026-03-06 12:46:57 +00:00

raito left a comment

Not a Perl person, but this looks good to me.

ma27 force-pushed oidc from cccec0822a to a6c840d290

2026-03-06 15:54:13 +00:00

Compare

ma27 merged commit 48ba16a317 into main

2026-03-06 18:34:17 +00:00

ma27 deleted branch oidc

2026-03-06 18:34:17 +00:00

ma27 referenced this pull request from a commit

2026-03-06 18:34:17 +00:00

Merge pull request 'OIDC login' (#73) from oidc into main

benaryorg commented

2026-03-11 05:35:00 +00:00

I realize I'm significantly late to the party, but I still want to confirm that this works.
What had me confused is that the OIDC code uses groups for determining the permissions, rather than the otherwise ubiquitous roles.
Not a big deal for me since I use authentik anyway so adding another property mapping is simple.
So other than some minor confusion on my behalf, everything's working splendid.

Thank you very much!

I realize I'm significantly late to the party, but I still want to confirm that this works. What had me confused is that [the OIDC code uses *groups* for determining the permissions](https://git.lix.systems/lix-project/hydra/src/commit/a6c840d2909fcb418c6f5a1902896a449871f2fa/src/lib/Hydra/Controller/User.pm#L240), rather than the otherwise ubiquitous *roles*. Not a big deal for me since I use *authentik* anyway so adding another property mapping is simple. So other than some minor confusion on my behalf, everything's working splendid. Thank you very much!

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

4 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lix-project/hydra!73

Rows
Columns