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

no id_token in claims object #850

Closed
ApostolisApo opened this issue Dec 5, 2019 · 14 comments
Closed

no id_token in claims object #850

ApostolisApo opened this issue Dec 5, 2019 · 14 comments
Assignees
Labels
Enhancement This is a feature request to add functionality that is not currently supported Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue

Comments

@ApostolisApo
Copy link

ApostolisApo commented Dec 5, 2019

Describe the bug
Have the same problem both in my app and in msalandroidkotlinapp example app. I am filling in my Azure details and when handling AuthenticationCallback in both:

  • the single fragment (mSingleAccountApp!!.signIn() )
  • multiple account fragment (mMultipleAccountApp!!.acquireToken() )

the claims object does not contain id_token. The access token and everything else is acquired properly

Smartphone

  • Device: Samsung S8
  • Android Version: Android 8
  • Browser Chrome
  • MSAL 1.0

To Reproduce
Replace the details in .json file with your Azure App details and run the msalandroidkotlinapp example app. Choose to sign in with in the single application fragment. If you inspect the url of the login page you can see that in the URL parameter there is this: response_type=code. Should it be like this? Shouldn't it be response_type=token id_token ?

Expected behavior
An id_token should be present in the claims object

Actual Behavior
The id_token is missing from the claims object
The claims object is missing id_token even if I call IMultipleAccountPublicClientApplication.getAccount(id)

@ApostolisApo ApostolisApo changed the title id_token (in claims) is null when acquiring token no id_token in claims object Dec 6, 2019
@iambmelt iambmelt self-assigned this Dec 9, 2019
@iambmelt
Copy link
Member

iambmelt commented Dec 9, 2019

Hi @ApostolisApo

The claims object is the Id Token -- it is a Map of the parsed JWT claims.

Is there a specific field within the IdToken you are trying to access? Please note, the raw token string is not currently exposed externally; if you have a good use case for needing the unparsed token we can evaluate adding this piece of API surface

@ApostolisApo
Copy link
Author

@iambmelt Yes actually the service I am building against is using the bare token, I cannot use the broken down claims. To reconstruct them to a string is quite hard right?
Anyway, in your example app you do exactly this, you access the bare string value and your app crashes because of that. And that was the reason I tried so much in vain to access it in mine.
It would be awesome if you exposed it because currently I resorted to another, a little more low-level library :(

@iambmelt
Copy link
Member

@ApostolisApo No, unfortunately you cannot reconstitute the token from the claims as the jwt signature won't match unless it is signed with the keys from STS.

I am tagging this issue as a Feature Request and will follow up with the team re: offering an API to access the raw IdToken in a future release.

@TaugyStyle
Copy link

Hello :)
Having the same request here , need the id_token string to validate signature against a back-end service. Since the ios MSAL library returns a json containing the string id_token and access_token it would be good to have more or less the same behavior.
Thanks in advance

@Tweener
Copy link

Tweener commented Dec 17, 2019

Hi @iambmelt,
I think you remember from another ticket already :)

I tested the latest release (1.0.1, which solved this issue, thanks for that!).

As @TaugyStyle said, I also realized the library does not expose the ID Token, whereas it does on the iOS library. We need this ID Token for the Open ID Connect (for the OAuth2).
This is again a big blocker for us as we still can't release the Microsoft SSO on Android, and our customers start to be impatient.

I see you tagged it as an Enhancement, do you any idea when this can be implemented?

Thanks in advance!
Cheers

@iambmelt
Copy link
Member

@TaugyStyle @Tweener I will work on an implementation for this today using MSAL; please note however, that the id token which will be exposed will be a v2 id token (more work/testing would be required to surface a v1 token).

I don't have an ETA for a release on this yet; I would like to treat it as a hotfix to avoid having to run a full test-pass for what is otherwise a relatively small PR.

I will post more details as I have them

/cc @shoatman @hamiltonha @Om83 FYI ^^

iambmelt added a commit that referenced this issue Dec 17, 2019
iambmelt added a commit that referenced this issue Dec 17, 2019
* Re #850

* Rev version number -> 1.1.0

* Update changelog for 1.1.0

* Improving javadoc
iambmelt added a commit that referenced this issue Dec 18, 2019
* Re #850

* Rev version number -> 1.1.0

* Update changelog for 1.1.0

* Improving javadoc
@iambmelt
Copy link
Member

@TaugyStyle @Tweener I have published a v1.1.0 implementing support for the v2 id_token on the AuthenticationResult.

Link to release:
https://github.com/AzureAD/microsoft-authentication-library-for-android/releases/tag/v1.1.0

Note: After a sync with the team, there was some concern raised about potentially insecure usage scenarios we may be inviting with this change; specifically: that any API which accepts the id token as a user credential has no way to guarantee that the token has not been leaked from a device or maliciously lifted from a user other than the current one.

Please be aware there is no proof-of-possession protection for these tokens. Keep id_tokens secure in your app: treat them like credentials. And be cognizant that any API/middleware design you implement which accepts these tokens needs to be aware that it is effectively subject to the same attack vectors of a Bearer flow. All this to say, take precautions to protect this artifact.

@iambmelt
Copy link
Member

@TaugyStyle @Tweener I have published a v1.1.0 implementing support for the v2 id_token on the AuthenticationResult.

Link to release:
https://github.com/AzureAD/microsoft-authentication-library-for-android/releases/tag/v1.1.0

Note: After a sync with the team, there was some concern raised about potentially insecure usage scenarios we may be inviting with this change; specifically: that any API which accepts the id token as a user credential has no way to guarantee that the token has not been leaked from a device or maliciously lifted from a user other than the current one.Please be aware there is no proof-of-possession protection for these tokens. Keep id_tokens secure in your app: treat them like credentials. And be cognizant that any API/middleware design you implement which accepts these tokens needs to be aware that it is effectively subject to the same attack vectors of a Bearer flow. All this to say, take precautions to protect this artifact.

@hamiltonha ^^ We need to prepare formal documentation for publication outlining best practices/patterns for handling, using, and validating id_tokens

@iambmelt
Copy link
Member

@ApostolisApo ^^

@iambmelt
Copy link
Member

Regarding usage, the id_token is available on the IAccount

For non-guest scenarios:

authenticationResult.getAccount().getIdToken();

For tenant-guest scenarios:

((IMultiTenantAccount) authenticationResult.getAccount()).getTenantProfiles().get("<tenant_id>").getIdToken();

@Tweener
Copy link

Tweener commented Dec 18, 2019

Thanks a lot @iambmelt for your reactivity! I'm gonna try that right away and let you know how it goes.

@TaugyStyle
Copy link

Incredible thanks ! Will try soon :)

@Tweener
Copy link

Tweener commented Dec 18, 2019

Works like a charm! Thanks a lot @iambmelt :)

@iambmelt
Copy link
Member

Thanks for verifying @Tweener 👍

@iambmelt iambmelt added the Enhancement This is a feature request to add functionality that is not currently supported label Dec 18, 2019
@iambmelt iambmelt added the Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue label Dec 18, 2019
iambmelt added a commit that referenced this issue Dec 19, 2019
* Re #850

* Rev version number -> 1.1.0

* Update changelog for 1.1.0

* Improving javadoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a feature request to add functionality that is not currently supported Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue
Projects
None yet
Development

No branches or pull requests

5 participants