search

damienbod / angular-auth-oidc-client

npm package for OpenID Connect, OAuth Code Flow with PKCE, Refresh tokens, Implicit Flow
https://www.npmjs.com/package/angular-auth-oidc-client
MIT License
1.15k stars 434 forks source link

Issue verifying hash [RSA256] Code Flow optional #376

Closed Davidmesm closed 5 years ago

Davidmesm commented 5 years ago

In oidc-security-state-validation.service.ts, in the function validateState in line 108 the attribute toReturn.decoded_id_token.at_hash is incorrect. This will always return undifiened causing the verification to fail. For more information please let me know.

damienbod commented 5 years ago

I need some more info, but this happens if the tokens are incorrect, or you are using old tokens

Davidmesm commented 5 years ago

What other information I can send you? I tested recreating the code, and the function at_hash always returns me an undefined.

toReturn.decoded_id_token = this.tokenHelperService.getPayloadFromToken(toReturn.id_token, false); will return a JSON with the user information to toReturn.decoded_id_token, afterwards it calls the at_hash which is the one that is failing, what characteristics should the JSON have so that the .at_hash doesn't returns undifiened.

damienbod commented 5 years ago

@Davidmesm What token service are you using, maybe the token service is created this at_hash value incorrectly. If you sent a token I could validate this. Or see the OpenID connect specification for this, to see if the at_hash is being created correctly.

Or maybe something in between is tampering with your token

Davidmesm commented 5 years ago

I've been investigating myself, and as you described my token doesn't have an at_hash as part of the payload. The token is being created by pingFederate and what I've read on OpenID specifications is that only on an implicit flow the at_hash is a REQUIREMENT, when using a code flow the at_hash is OPTIONAL. That is the reason that is not being included.

damienbod commented 5 years ago

@Davidmesm You are correct, this is optional, but I think it's a bad idea not to do this. The access token should be validated in this way. I think pingFederate should update this.

BUT, in the specs it is optional for the code flow, so I will also make it optional for the code flow. I will make a fix for this and create a new npm as soon as possible

Thanks for reporting.

Greetings Damien

damienbod commented 5 years ago

will be released in version 9.0.2, hopefully Friday

Davidmesm commented 5 years ago

Thank you! I really appreciate it.

damienbod commented 5 years ago

released, thanks for reporting