Configurable email_verified OIDC Claim Requirement #2860

ImpostorKeanu · 2025-11-03T15:39:27Z

have read the CONTRIBUTING.md file
raised a GitHub issue or discussed it on the projects chat beforehand
added unit tests
added integration tests
updated documentation if needed
updated CHANGELOG.md

When configured to use OIDC, email_verified=true is required to appear in ID token claims or user info before the email will be saved in a user's information. There are some scenarios where the email_verified claim cannot be sent/configured, such as when using Cloudflare One-time pin -- which doesn't support adding/modifying claims. This can make writing ACLs difficult or impossible.

This PR adds a configuration that relaxes the email_verified=true requirement by:

adding a configuration field: oidc.use_unverified_email
considering the setting when:
- applying oidc.allowed_domains during authorization
- user records are initialized via User.FromClaim

Related: #2655

ImpostorKeanu · 2025-11-10T14:39:17Z

Just checking in on this. Please let me know if I've gone about the process wrong.

Think it's something we're interested in accepting?

Really appreciate everything.

nblock · 2025-11-10T14:42:56Z

Really appreciate everything.

Thx for your PR! I'll do some testing once 0.27.1 is out and we'll come back to this afterwards.

ImpostorKeanu · 2025-11-10T14:53:53Z

No rush at all! Thanks a ton!

nblock

Thanks for your PR!

The following behavior makes authorization and profile update consistent:

For oidc.email_verified_required: true (default), require the e-mail to be verified to do any processing on it (email_verified: true). This gives as two cases:

email_verified: true:
- accept if the email matches the oidc.allowed_users filter
- accept if the domain matches the oidc.allowed_domains filter
- update the email in the user's profile
email_verified: false (or not provided):
- reject even if the email would match the oidc.allowed_users filter (breaking)
- reject even if the domain would match the oidc.allowed_domains filter (breaking)
- don't update the email in the user's profile

For oidc.email_verified_required: false, ignore the email_verified flag during processing. This results in:

accept if the email matches the oidc.allowed_users filter
accept if the domain matches the oidcs.allowed_groups filter
update the email in the user's profile

config-example.yaml

hscontrol/types/config.go

hscontrol/oidc.go

ImpostorKeanu · 2025-12-03T15:04:26Z

So sorry for the delay on this! Got wrapped up in work and the holiday.

I'll try to knock it out later today.

Thanks!

ImpostorKeanu · 2025-12-08T16:31:03Z

I fumbled the tests for a few minutes but everything should be in order now.

I'm not sure why the test workflow says it's waiting for a "status to be reported" for the tests. Weird.

If I've borked stuff, let me know and I'll fix it.

nblock · 2025-12-10T06:20:12Z

Thanks for following up and implementing most of #2860 (review). It seems that the following cases are not implemented?

email_verified_required: true, email_verified: false (or not provided):
- reject even if the email would match the oidc.allowed_users filter (breaking)
- reject even if the domain would match the oidc.allowed_domains filter (breaking)
- don't update the email in the user's profile

I tested it both cases ( oidc.allowed_users, oidc.allowed_domains) and noticed that a user with an unverified email address was able to authenticate.

Could you have a look at this?

ImpostorKeanu · 2025-12-10T17:46:45Z

Thanks for following up and implementing most of #2860 (review). It seems that the following cases are not implemented?
* `email_verified_required: true`, `email_verified: false` (or not provided):
  
  * reject even if the email would match the `oidc.allowed_users` filter (breaking)
  * reject even if the domain would match the `oidc.allowed_domains` filter (breaking)
  * don't update the email in the user's profile
I tested it both cases ( oidc.allowed_users, oidc.allowed_domains) and noticed that a user with an unverified email address was able to authenticate.

Could you have a look at this?

Checking into it but I have a question:

As of now, OIDC authorization checks are AND'd. Shouldn't they be OR'd?

Example:

If both AllowedDomains and AllowedGroups are configured and claims.Email doesn't have an allowed domain, AllowedGroups isn't evaluated.

ImpostorKeanu · 2025-12-10T21:52:03Z

I refactored to OR authorization checks, cleaned stuff up, and added standalone tests for OIDC authorization. Some of the decisions are questionable. All tests are succeeding on my side. Still waiting to see if GitHub actions wreck me.

I'm open to changes/rejection. Feel free to share your thoughts, and sorry if I'm making this more complicated than it has to be.

nblock · 2025-12-11T11:56:07Z

As of now, OIDC authorization checks are AND'd. Shouldn't they be OR'd?

This could be discussed as a possible follow-up to this PR, but let's don't change the current behavior in this PR and keep it focused (docs).

(Feel free to rebase and squash as single commit.)

ImpostorKeanu · 2025-12-12T17:08:24Z

As of now, OIDC authorization checks are AND'd. Shouldn't they be OR'd?

This could be discussed as a possible follow-up to this PR, but let's don't change the current behavior in this PR and keep it focused (docs).

(Feel free to rebase and squash as single commit.)

Ok, I reset to an earlier commit and saved those changes to another branch. I'll open another PR.

Latest push still moves authorization to a distinct function for testing while preserving the authorization flow.

need to test it

nblock · 2025-12-16T09:56:59Z

Thx, will have a look soonish

nblock · 2025-12-17T08:05:40Z

hscontrol/oidc.go

+	trustEmail := !cfg.EmailVerifiedRequired || bool(claims.EmailVerified)
+	hasEmailTests := len(cfg.AllowedDomains) > 0 || len(cfg.AllowedUsers) > 0
+	if !trustEmail && hasEmailTests {
+		return errOIDCUnverifiedEmail
+	}


An "internal server error" is raised when:

oidc.allowed_users is configured

oidc.email_verified_enabled is enabled

A user with an unverified e-mail address tries to authenticate.

Logs: ERR http internal server error error="authenticated principal has an unverified email" code=500

Browser: internal server error

This should raise a http/401 instead, similar to:

headscale/hscontrol/oidc.go

Line 422 in 21ba197

return NewHTTPError(http.StatusUnauthorized, "unauthorised domain", errOIDCAllowedDomains)

headscale/hscontrol/oidc.go

Line 444 in 21ba197

return NewHTTPError(http.StatusUnauthorized, "unauthorised group", errOIDCAllowedGroups)

headscale/hscontrol/oidc.go

Line 458 in 21ba197

return NewHTTPError(http.StatusUnauthorized, "unauthorised user", errOIDCAllowedUsers)

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

nblock

Thx, did a quick rundown with the following testcases, looks good:

@kradalby will have another look at the code.

Add test cases covering: - unverified email with allowed user filter - no filters configured with unverified email

Add test case that verifies a user in the second allowed group should be authorized. This test currently fails due to a bug where validateOIDCAllowedGroups returns error on first non-match.

Move error return outside the for loop so all allowed groups are checked before returning unauthorized. Previously, it would return error on first non-matching group instead of checking if any subsequent group matches.

Signed-off-by: Kristoffer Dalby <[email protected]>

ImpostorKeanu requested review from juanfont, kradalby, nblock and ohdearaugustin as code owners November 3, 2025 15:39

nblock requested changes Nov 16, 2025

View reviewed changes

config-example.yaml Outdated Show resolved Hide resolved

hscontrol/types/config.go Outdated Show resolved Hide resolved

hscontrol/oidc.go Outdated Show resolved Hide resolved

hscontrol/oidc.go Outdated Show resolved Hide resolved

ImpostorKeanu force-pushed the use-unverified-email branch from 131b92b to 4e01aed Compare December 8, 2025 14:00

ImpostorKeanu force-pushed the use-unverified-email branch from e0ced73 to 7a78fcd Compare December 12, 2025 16:53

nblock previously approved these changes Dec 16, 2025

View reviewed changes

kradalby added this to the v0.28.0 milestone Dec 17, 2025

nblock requested changes Dec 17, 2025

View reviewed changes

kradalby requested a review from Copilot December 17, 2025 08:07

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Copilot started reviewing on behalf of kradalby December 17, 2025 08:32 View session

kradalby modified the milestones: v0.28.0, v0.29.0 Dec 17, 2025

ImpostorKeanu force-pushed the use-unverified-email branch from 7a78fcd to c573dfd Compare December 17, 2025 18:53

nblock approved these changes Dec 17, 2025

View reviewed changes

squash latest commits

1a1faac

kradalby added 4 commits December 18, 2025 10:48

oidc: add tests for unverified email scenarios

34a606d

Add test cases covering: - unverified email with allowed user filter - no filters configured with unverified email

oidc: add test for multiple allowed groups

d9f9149

Add test case that verifies a user in the second allowed group should be authorized. This test currently fails due to a bug where validateOIDCAllowedGroups returns error on first non-match.

oidc: fix validateOIDCAllowedGroups checking all groups

ed06e1d

Move error return outside the for loop so all allowed groups are checked before returning unauthorized. Previously, it would return error on first non-matching group instead of checking if any subsequent group matches.

docs: add changelog for email_verified_required option

d95e291

kradalby force-pushed the use-unverified-email branch from beabaed to d95e291 Compare December 18, 2025 09:52

kradalby added 2 commits December 18, 2025 11:46

lint

80bc5d6

Signed-off-by: Kristoffer Dalby <[email protected]>

fix nil panic on batcher close

8a1e3f6

Signed-off-by: Kristoffer Dalby <[email protected]>

kradalby enabled auto-merge (rebase) December 18, 2025 11:39

kradalby disabled auto-merge December 18, 2025 11:40

kradalby enabled auto-merge (squash) December 18, 2025 11:40

kradalby merged commit 7be2091 into juanfont:main Dec 18, 2025
14 checks passed

Uh oh!

Configurable email_verified OIDC Claim Requirement #2860

Configurable email_verified OIDC Claim Requirement #2860

Uh oh!

Conversation

ImpostorKeanu commented Nov 3, 2025

Uh oh!

ImpostorKeanu commented Nov 10, 2025

Uh oh!

nblock commented Nov 10, 2025

Uh oh!

ImpostorKeanu commented Nov 10, 2025

Uh oh!

nblock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ImpostorKeanu commented Dec 3, 2025

Uh oh!

ImpostorKeanu commented Dec 8, 2025

Uh oh!

nblock commented Dec 10, 2025

Uh oh!

ImpostorKeanu commented Dec 10, 2025

Uh oh!

ImpostorKeanu commented Dec 10, 2025

Uh oh!

nblock commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ImpostorKeanu commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nblock commented Dec 16, 2025

Uh oh!

nblock Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

nblock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nblock commented Dec 11, 2025 •

edited

Loading

ImpostorKeanu commented Dec 12, 2025 •

edited

Loading