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

verification of at_hash == access_token #566

Open
stoivo opened this issue Jul 12, 2023 · 4 comments
Open

verification of at_hash == access_token #566

stoivo opened this issue Jul 12, 2023 · 4 comments

Comments

@stoivo
Copy link

stoivo commented Jul 12, 2023

I am working on a connection to IdPorten in Norway. We use JWTs here. I am porting some example scripts over to ruby and it seems like the python package python-jose includes verification of at_hash. https://github.com/mpdavis/python-jose/blob/4b0701b46a8d00988afcc5168c2b3a1fd60d15d8/jose/jwt.py#L426-L458

I haven't tried to look in the spec to see what the standards say. Would you be open to a merge request with this verification? Where is the official spec for this?

Also, I find it quite strange that when you call decode with aud but skip verify_aud it doesn't verify. Why would I pass the aud if I don't want it to verify? Also, the third argument is called verify but seems to work as a kill switch for all verifications.

JWT.decode(
  id_token, nil, true, algorithms: ["RS256"],
  aud: @client_id)

JWT.decode(
  id_token, nil, true, algorithms: ["RS256"],
  aud: @client_id, verify_aud: true)

I would like to switch the default value verify_* to true

@stoivo stoivo changed the title Verofocatopm of at_hash == access_token verification of at_hash == access_token Jul 13, 2023
@anakinj
Copy link
Member

anakinj commented Jul 17, 2023

Hi. Now when pointed out the verify_aud flag seems a bit off and I totally agree with you on that. I've been brewing a 3.0 version of this gem and I think this kind of change in defaults would fit pretty well in there.

About the original question. Im guessing this at_hash is something related to the openid spec and not totally a fan of including that into the realms of this gem that only does JOSE related things.

The need to do custom things with JWT tokens is growing all the time and when designing the new 3.0 interfaces I think we can support these kind of usecases a lot easier, now it would just be yet another if-else....

@stoivo
Copy link
Author

stoivo commented Jul 22, 2023

Ah, I think we will wait until version 3 then. Also the at_hash think can wait until version 3 and I could help implement that. I will probably try to implement it myself in my operation.

I can test version 3 when it is ready if you want to. Something you want help with with version 3?

@anakinj
Copy link
Member

anakinj commented Jul 25, 2023

There is version-3.0 branch on this repo. Been slowly sketching out things on that one trying to keep somewhat backwards compatibility. Currently trying to unravel the claim validations from the current form into separate classes...

@anakinj
Copy link
Member

anakinj commented Jul 25, 2023

If you're interested you could maybe look into continuing moving the verify_* methods into their own classes (and specs).

A few examples on what I've been trying to to:

2e79d25
3192bf5

Also maybe testing the concept with the at_hash validation.

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

2 participants