Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for setting claims in clientcredential access tokens #1383

Closed
jacksontj opened this issue Apr 24, 2019 · 11 comments
Closed

Add support for setting claims in clientcredential access tokens #1383

jacksontj opened this issue Apr 24, 2019 · 11 comments

Comments

@jacksontj
Copy link

Is your feature request related to a problem? Please describe.
I have a variety of internal applications which have various metadata they require when a token shows up. Specifically this is problematic for "bot" users. I have bot or service-accounts that are created by my oauth clients on behalf of a user. These "bot" tokens need to have some metadata (who created it, when it was created, etc.). Today this would require me to maintain a map of token -> metadata which means (1) I have duplication of the token and (2) I'm having to store it in a few places.

Describe the solution you'd like
Simply I'd like to be able to define claims for an access token minted through the clientcredential flow. This would enable my use-case as well as others (e.g. #1221).

Describe alternatives you've considered
As it stands today I don't use hydra/oauth for this, as it would require me to effectively duplicate al the token data.

@aeneasr
Copy link
Member

aeneasr commented Apr 25, 2019

It would be quite difficult to achieve that with the feature set right now because claims should only be set by the Authorization Server (or one of it's sources, e.g. the IDP) and never by a client. Therefore:

  1. setting claims during client registration is tricky as we effectively support OpenID Connect Dynamic Client Registration which would allow third parties, if configured incorrectly, to register their own claims (e.g. roles: [admin])
  2. setting claims during client_credentials is even more dangerous because again, the relying party can simply say (e.g. roles: [admin])

It would therefore require some other means of setting this data.

@jimmytheneutrino
Copy link

This is a very useful feature that comes up sooner or later in almost every real-life application using OIDC. Apps very often need service accounts. Many OIDC providers currently allow them via client_credentials flow (including KeyCloak, Connect2Id, Google's OIDC and others).

1....if configured incorrectly....
Why is this an issue? Many things in a security system could be dangerous if configured incorrectly. In a situation with service accounts, it is probably a good idea to disallow dynamic client registration for untrusted parties anyway.

That being said, of course, it could be more convenient if Hydra could ask some external app for client's claims similarly to what it currently does with login and consent apps. But this probably needs much more intervention into the code (and sways further away from the main purpose of Hydra, i.e., implementing protocol) than simply allowing to specify client's claims on client creation.

@svrakitin
Copy link
Contributor

svrakitin commented Apr 26, 2021

@aeneasr Is it possible to solve this by introducing configurable webhook which can be called when there is a client_credentials or any other machine-to-machine flow? This could be a global setting like urls.client_credentials_hook.

If it is configured Hydra could POST to this webhook with a request context and expect implementation to respond with a payload similar to one for accepting a consent, which contains extra claims and various token customizations. If it is not configured then just keep doing what it was doing before.

Such approach seems to be consistent with what Hydra already does.

@aeneasr
Copy link
Member

aeneasr commented May 2, 2021

@svrakitin have you seen #1748 ? I think it makes a tad more sense as it kinda decouples the two components. We might still be able to add some function which does a HTTP request to the jsonnet though!

@jimmytheneutrino
Copy link

jimmytheneutrino commented May 3, 2021

@aeneasr
It looks like the (first-step) solution proposed by you in #1748 is exactly the original intention of the current ticket:
Allow to define claims for a client to be issued in tokens for client_credentials_flow on client registration (but of course not via public registration).

As for the second step: the solution via a hook aka "client_credentials_claims-app" proposed here in my opinion still looks much more flexible than jsonnet and more in line with Hydra's approach. (Of course, again it is a good idea to disallow defining such a hook on public client registration.)

The only real benefit of jsonnet seems to be the speed of token issuing. But token issuing is rarely a bottleneck, instead token interpolation/userinfo requests usually are.

@aeneasr
Copy link
Member

aeneasr commented May 8, 2021

Hm I think it would be better to have a dedicated endpoint where admin access allows to specify client claims. You can then update the client when changes are needed, instead of Hydra requesting info on every client request. So push instead of pull :)

@github-actions
Copy link

github-actions bot commented May 9, 2022

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan on resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneous you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label May 9, 2022
@aeneasr aeneasr removed the stale Feedback from one or more authors is required to proceed. label May 9, 2022
@github-actions
Copy link

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas on how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label May 10, 2023
@mih-kopylov
Copy link
Contributor

Unstale please

@github-actions github-actions bot removed the stale Feedback from one or more authors is required to proceed. label May 11, 2023
@mrwormhole
Copy link

mrwormhole commented Jan 25, 2024

hello for those who need this feature(injecting client's metadata to JWT access tokens), I have created this in my own fork so feel free to use

also deliberately would love if people used less interfaces according to core Go philosophy :/

https://github.com/mrwormhole/hydra

@alnr
Copy link
Contributor

alnr commented Apr 12, 2024

This is implemented via token webhooks.

@alnr alnr closed this as completed Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants