starred/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-12 04:10:32 +01:00
Code Issues 98 Packages Projects Releases 10 Wiki Activity

[Bug] Inconsistent handling of OIDC claims #1050

New Issue
Closed
opened 2025-12-29 02:27:57 +01:00 by adam · 9 comments
adam commented 2025-12-29 02:27:57 +01:00
Owner
Copy Link

Originally created by @nblock on GitHub (Jun 24, 2025).

Is this a support request?

  • This is not a support request

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Recent Headscale versions fetch OIDC claims from the ID token and also query the userinfo endpoint to gather details about the user. This is nice as identity providers seem to reduce the size of tokens lately.

But user authorization happens right before the userinfo endpoint is queried which means that authorization only uses the claims from the ID token. Thus, the data used for authorization and storage might be different, affected config settings:

  • oidc.allowed_domains
  • oidc.allowed_users
  • oidc.allowed_groups

Also the handling of the email_verified is inconsistent:

  • Authorization: the email_verified claim is not checked. The check for oidc.allowed_domains and oidc.allowed_users only considers email
  • Storage: the e-mail address is only considered as valid if email_verified: true

Expected Behavior

Information retrieval from OIDC should be identical for all uses within the codebase.

Steps To Reproduce

  • Experiment with oidc.allowed_domains, oidc.allowed_users, oidc.allowed_groups

Environment

- OS: Debian 13
- Headscale version: 0.26.0
- Tailscale version: -

Runtime environment

  • Headscale is behind a (reverse) proxy
  • Headscale runs in a container

Additional notes

  • Just enabling email_verified for authorization checks would break existing installs as users without a verified e-mail address would no longer pass the check.
  • User are identified via OIDC's iss and sub claim and no longer via email. Maybe the email_verified: true claim is no longer that important?
Originally created by @nblock on GitHub (Jun 24, 2025). ### Is this a support request? - [x] This is not a support request ### Is there an existing issue for this? - [x] I have searched the existing issues ### Current Behavior Recent Headscale versions fetch OIDC claims from the ID token and also query the userinfo endpoint to gather details about the user. This is nice as identity providers seem to reduce the size of tokens lately. But user authorization [happens right before the userinfo endpoint is queried](https://github.com/juanfont/headscale/blob/081af2674b9d14f9db06528f4be896bb82752cba/hscontrol/oidc.go#L280) which means that authorization only uses the claims from the ID token. Thus, the data used for authorization and storage might be different, affected config settings: - `oidc.allowed_domains` - `oidc.allowed_users` - `oidc.allowed_groups` Also the handling of the `email_verified` is inconsistent: * Authorization: the `email_verified` claim is not checked. The check for `oidc.allowed_domains` and `oidc.allowed_users` only considers `email` * Storage: the e-mail address is only considered as valid if `email_verified: true` ### Expected Behavior Information retrieval from OIDC should be identical for all uses within the codebase. ### Steps To Reproduce - Experiment with `oidc.allowed_domains`, `oidc.allowed_users`, `oidc.allowed_groups` ### Environment ```markdown - OS: Debian 13 - Headscale version: 0.26.0 - Tailscale version: - ``` ### Runtime environment - [ ] Headscale is behind a (reverse) proxy - [ ] Headscale runs in a container ### Additional notes - Just enabling `email_verified` for authorization checks would break existing installs as users without a verified e-mail address would no longer pass the check. - User are identified via OIDC's `iss` and `sub` claim and no longer via `email`. Maybe the `email_verified: true` claim is no longer that important?
adam added the bugno-stale-botOIDC labels 2025-12-29 02:27:57 +01:00
adam closed this issue 2025-12-29 02:27:57 +01:00
adam commented 2025-12-29 02:27:57 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (Jun 24, 2025):

One possible solution to solve the email_verified difference is to make it configurable whether email_verified: true is required or not. For example:

  • oidc.require_email_verified: true: Process the email_verified claim and accept email only if email_verified: true
  • oidc.require_email_verified: false: Accept email regardless of email_verified
@nblock commented on GitHub (Jun 24, 2025): One possible solution to solve the `email_verified` difference is to make it configurable whether `email_verified: true` is required or not. For example: * `oidc.require_email_verified: true`: Process the `email_verified` claim and accept `email` only if `email_verified: true` * `oidc.require_email_verified: false`: Accept `email` regardless of `email_verified`
adam commented 2025-12-29 02:27:57 +01:00
Author
Owner
Copy Link

@fredrikekre commented on GitHub (Jun 26, 2025):

Since the userinfo endpoint is queried unconditionally anyway, couldn't that query simply be moved up to before the validateOIDCAllowed* calls?

Maybe the email_verified: true claim is no longer that important?

I think it still is, at least if you are filtering users based on email or domain?

@fredrikekre commented on GitHub (Jun 26, 2025): Since the userinfo endpoint is queried unconditionally anyway, couldn't that query simply be moved up to before the `validateOIDCAllowed*` calls? > Maybe the email_verified: true claim is no longer that important? I think it still is, at least if you are filtering users based on email or domain?
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@james-d-elliott commented on GitHub (Jun 26, 2025):

Please be aware I'm not aware of the particular implementation at play here so some of these comments may not be applicable.

It should be noted that the scope parameter, and consequently the granted scopes; has no spec defined effect on an ID Token with the exception of the Implicit Flow which only results in an ID Token. For these reasons the only correct location to obtain claims outside of the ID Token Claims by default is the UserInfo Endpoint.

While it's technically permissible for a provider to choose to include additional claims for any reason, it is strongly discouraged by several areas of the spec; and the client should never error if the claims do not exist in the ID Token as long as they are present at the UserInfo endpoint as this is the normative behavior expected of providers for all of the Scope Claims, and is fundamentally a minimum expectation of clients.

It should also be noted that it's critical that a client uses the iss and sub parameters to identify the user. The email and preferred_username claims are not safe to use for this purpose. These claims are useful only for registration and updating client accounts. Clients should store the iss and sub claims in a way that identifies the user locally.

Let me know if you would prefer I open separate issues for any of this. I understand while the OP's bug is somewhat related to these points I would understand if this was considered a separate bug as well.

Also feel free to let me know if you're interested in additional information surrounding this. However:

  1. Section 5.4 Requesting Claims using Scope Values which was the link I provided for Scope Claims, very clearly explains the aspects surrounding Claims in the ID Token vs UserInfo.
  2. Section 5.7 Claim Stability and Uniqueness explains the concept surrounding the iss and sub claims. If this is not currently implemented then it might be worth prioritizing this.
@james-d-elliott commented on GitHub (Jun 26, 2025): Please be aware I'm not aware of the particular implementation at play here so some of these comments may not be applicable. It should be noted that the `scope` parameter, and consequently the granted scopes; has no spec defined effect on an ID Token with the exception of the Implicit Flow which only results in an ID Token. For these reasons the only correct location to obtain claims outside of the [ID Token Claims](https://openid.net/specs/openid-connect-core-1_0.html#IDToken) by default is the UserInfo Endpoint. While it's **_technically permissible_** for a provider to choose to include additional claims for any reason, it is strongly discouraged by several areas of the spec; and the client should never error if the claims do not exist in the ID Token as long as they are present at the UserInfo endpoint as this is the normative behavior expected of providers for all of the [Scope Claims](https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims), and is fundamentally a minimum expectation of clients. It should also be noted that it's critical that a client uses the `iss` and `sub` parameters to identify the user. The `email` and `preferred_username` claims are not safe to use for this purpose. These claims are useful only for registration and updating client accounts. Clients should store the `iss` and `sub` claims in a way that identifies the user locally. Let me know if you would prefer I open separate issues for any of this. I understand while the OP's bug is somewhat related to these points I would understand if this was considered a separate bug as well. Also feel free to let me know if you're interested in additional information surrounding this. However: 1. Section [5.4 Requesting Claims using Scope Values](https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) which was the link I provided for Scope Claims, very clearly explains the aspects surrounding Claims in the ID Token vs UserInfo. 2. Section [5.7 Claim Stability and Uniqueness](https://openid.net/specs/openid-connect-core-1_0.html#ClaimStability) explains the concept surrounding the `iss` and `sub` claims. If this is not currently implemented then it might be worth prioritizing this.
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@tecosaur commented on GitHub (Oct 8, 2025):

@fredrikekre do you plan on PRing 8c7518c989d52bddbe15764818776529e3ec1a42 at any point?

@tecosaur commented on GitHub (Oct 8, 2025): @fredrikekre do you plan on PRing 8c7518c989d52bddbe15764818776529e3ec1a42 at any point?
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@fredrikekre commented on GitHub (Oct 8, 2025):

Merged in 5d8a2c25ea

@fredrikekre commented on GitHub (Oct 8, 2025): Merged in https://github.com/juanfont/headscale/commit/5d8a2c25ea97e47b183dfbe96a87d73f72f89ac6
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@tecosaur commented on GitHub (Oct 8, 2025):

Ah nice, thanks for that. Seems like this issue could be closed now?

@tecosaur commented on GitHub (Oct 8, 2025): Ah nice, thanks for that. Seems like this issue could be closed now?
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (Oct 8, 2025):

Ah nice, thanks for that. Seems like this issue could be closed now?

Its partially solved, email_verified is handled differently during authorization and storage.

@nblock commented on GitHub (Oct 8, 2025): > Ah nice, thanks for that. Seems like this issue could be closed now? Its partially solved, `email_verified` is handled differently during authorization and storage.
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@ImpostorKeanu commented on GitHub (Oct 30, 2025):

One possible solution to solve the email_verified difference is to make it configurable whether email_verified: true is required or not. For example:

* `oidc.require_email_verified: true`: Process the `email_verified` claim and accept `email` only if `email_verified: true`

* `oidc.require_email_verified: false`: Accept `email` regardless of `email_verified`

I like this idea. Best of both worlds.

I'd like to authenticate users via Cloudflare One-time Pin, but it doesn't automatically include email_verified and it doesn't allow adding claims, which means I'll need implement a broker. Making the field configurable solves this problem and it works fine in cases like mine where the email has been verified but can't be signaled using a claim.

It'd create complexity, but I've also been considering how to make the implementation flexible.

Consider this invalid config snippet:

oidc:
  trust_claims:

    # can we trust the email claim?
    # (this is also the default)
    email:
      # access denied when checks fail
      require: true
      # tests are and'd
      tests:
        - claim: email_verified
          op: equals
          value: true

    # can we trust the username claim?
    username:
      # optional; fall back to email
      require: false
      tests:
        - claim: username
          op: regex
          value: '^[a-z0-9]{5,25}$'

Pretty much establish if we can trust claims by performing basic tests. Obviously more complicated, especially if the claim has a dictionary or array.

@ImpostorKeanu commented on GitHub (Oct 30, 2025): > One possible solution to solve the `email_verified` difference is to make it configurable whether `email_verified: true` is required or not. For example: > > * `oidc.require_email_verified: true`: Process the `email_verified` claim and accept `email` only if `email_verified: true` > > * `oidc.require_email_verified: false`: Accept `email` regardless of `email_verified` I like this idea. Best of both worlds. I'd like to authenticate users via [Cloudflare One-time Pin][onetime-pin], but it doesn't automatically include `email_verified` and it doesn't allow adding claims, which means I'll need implement a broker. Making the field configurable solves this problem and it works fine in cases like mine where the email has been verified but can't be signaled using a claim. It'd create complexity, but I've also been considering how to make the implementation flexible. Consider this **invalid** config snippet: ```yaml oidc: trust_claims: # can we trust the email claim? # (this is also the default) email: # access denied when checks fail require: true # tests are and'd tests: - claim: email_verified op: equals value: true # can we trust the username claim? username: # optional; fall back to email require: false tests: - claim: username op: regex value: '^[a-z0-9]{5,25}$' ``` Pretty much establish if we can trust claims by performing basic tests. Obviously more complicated, especially if the claim has a dictionary or array. [onetime-pin]: https://developers.cloudflare.com/cloudflare-one/integrations/identity-providers/one-time-pin/
adam commented 2025-12-29 02:27:58 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (Dec 18, 2025):

Fixed via 7be2091

@nblock commented on GitHub (Dec 18, 2025): Fixed via 7be2091
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
CLI
DERP
DNS
Nix
OIDC
SSH
bug
database
documentation
duplicate
enhancement
faq
good first issue
grants
help wanted
might-come
needs design doc
needs investigation
no-stale-bot
out of scope
performance
policy 📝
pull-request
Mirrored from GitHub Pull Request
 question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
No Label OIDC bug no-stale-bot
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#1050
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?