search

openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
985 stars 161 forks source link

Adding of id_token on TokenResponse class #42

Closed sbatezat closed 6 years ago

sbatezat commented 6 years ago

"id_token" is missing on TokenResponse class. This pull request fix that.

Implementation was based from https://tools.ietf.org/html/rfc6749#section-5.1 instead of https://openid.net/specs/openid-connect-core-1_0.html#TokenResponse

tikurahul commented 6 years ago

Sorry about the lack of updates here. I promise to take a look at this, this week. Also, please look at https://github.com/openid/AppAuth-JS/blob/master/CONTRIBUTING.md and make sure you sign a CLA. Thanks for your patience !

tikurahul commented 6 years ago

Hi @sbatezat. Sorry for the delay. Firstly, thanks for the P.R.

I would like to make id_token optional because it's only mandatory for OpenID-Connect compliant identity providers. For many vanilla OAuth2 providers, id_token will actually be null.

Can you please send me an updated P.R ?

sbatezat commented 6 years ago

Hi @tikurahul. No worries for the delay, I'm very busy too.

I was thinking this library was only targetting OpenID-Connect providers (the repository name confused me: openid/AppAuth-JS) - but you're right, as per the documentation it should work for both OAuth 2.0 and OpenId providers:

AppAuth for JavaScript is a client SDK for public clients for communicating with OAuth 2.0 and OpenID Connect providers [...]

I'll then update my PR with an optional id_token, probably by the end of the week. I also keep in mind that I'll have to sign a CLA.

markphillips100 commented 6 years ago

Any chance this pull request (with the "optional" change) can be merged soon? Really need the id_token so I can get at the user's claims.

tikurahul commented 6 years ago

I will do it. :smile:

tikurahul commented 6 years ago

This is now obsolete.