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

Custom claims in access token jwt #1974

Closed
petertriho opened this issue Aug 5, 2020 · 9 comments
Closed

Custom claims in access token jwt #1974

petertriho opened this issue Aug 5, 2020 · 9 comments
Labels
feat New feature or request.

Comments

@petertriho
Copy link

Is your feature request related to a problem? Please describe.

At the moment trying to add custom claims at the root of the access token but it ends up being under "ext" e.g.

hydraAdmin
  .acceptConsentRequest(challenge, {
    session: {
      access_token: {
        foo: 'bar'
      },
    },
  })

gives

{
  "ext": {
    "foo": "bar"
  }
  ...
}

I would like

{
  "foo": "bar"
  ...
}

E.g. in hasura the format for an access token jwt should be

{
  "sub": "1234567890",
  "name": "John Doe",
  "admin": true,
  "iat": 1516239022,
  "https://hasura.io/jwt/claims": {
    "x-hasura-allowed-roles": ["editor","user", "mod"],
    "x-hasura-default-role": "user",
    "x-hasura-user-id": "1234567890",
    "x-hasura-org-id": "123",
    "x-hasura-custom": "custom-value"
  }
}

Describe the solution you'd like

Would like the possibility of adding custom claims to the access token jwt.

Describe alternatives you've considered

Current behaviour is ok, I can configure the claims_namespace_path to be $.ext.hasura or seems like I should consider using webhooks instead

@aeneasr
Copy link
Member

aeneasr commented Aug 6, 2020

Yeah this should be possible. We obviously would not want to override any claims which are set by Hydra though. Fixing this would mean quite a big breaking change so we would need to think about how to make this backwards compatible.

@petertriho
Copy link
Author

petertriho commented Aug 6, 2020

I think keeping the existing behaviour would be best (defaults to be being added to ext) and would allow it to be backwards compatible. In the configuration yaml and/or flag of something like allowed_access_token_claims (list of strings in yaml, comma separated string for flag) for any additional claims that wants to be added. This list of additional claims could then be checked to prevent overriding any claims set by hydra.

e.g.

allowed_access_token_claims:
  - https://hasura.io/jwt/claims
  - hasura
  - my-custom-claim

@aeneasr
Copy link
Member

aeneasr commented Aug 7, 2020

We could probably also just mirror the keys for a transition period:

{
  "my": "value"
  "ext": { "my": "value" }
}

@aeneasr
Copy link
Member

aeneasr commented Aug 10, 2020

Would you be open to contribute this change @petertriho?

@aeneasr aeneasr added the feat New feature or request. label Aug 10, 2020
@petertriho
Copy link
Author

I'm happy to try if you let me know where I should be looking to make the changes. I've never programmed in Go before so I may need some time.

@vinckr
Copy link
Member

vinckr commented Aug 31, 2020

Hey @petertriho, I am sure you are quite busy, are you still up for contributing on this?
Feel free to bounce this back if you don't have time to look at it, though. (And let me know if you'd prefer not to receive such requests in the future.)

@petertriho
Copy link
Author

petertriho commented Aug 31, 2020

Hi @vinckr,
Sorry, I've been busy and haven't had a chance to look at this yet (still need to learn Golang + understand hydra's codebase). I'm definitely still up to contributing on this, but I don't have a timeframe on when I can do this yet so if it's a priority to get this done as soon as possible, it might be best for someone else to take up on it.

@medexton
Copy link

I was able to achieve the desired behaviour by making the following change. A bit lazy as it doesn't validate much but got the job done.

https://github.com/ory/hydra/blob/40b09cdf69ad5cdb27c35493e345fbf9b97a7b1b/oauth2/session.go line: 57

from: Extra: map[string]interface{}{"ext": s.Extra},
to: Extra: s.Extra,

@svengerlach
Copy link

Thank you for hydra in general and that change specifically! Being in the middle of the migration from a legacy authentication manager towards hydra and being required to remain compatible to various client integrations, the chance to include private/custom claims at the top level eases my life a lot.

Is there a due date when this change will be included in a tagged release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request.
Projects
None yet
Development

No branches or pull requests

5 participants