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

Fix occasional error while unmarshalling Apple id_token #538

Merged
merged 3 commits into from
Feb 16, 2024

Conversation

yaronius
Copy link
Contributor

@yaronius yaronius commented Feb 7, 2024

Hi! According to Apple documentation on the ID token it turns out that some claims could be either boolean or string representing boolean. In this library we always parse one of these claims (is_private_email) as a string (notice the tags on this line) and sometimes (when Apple actually sends boolean value) it results in the following error: json: invalid use of ,string struct tag, trying to unmarshal unquoted value into bool. I recently noticed an increase in the amount of such errors, probably now Apple uses boolean more frequently. This PR fixes that by detecting the value from token and handling it appropriately.

@yaronius yaronius changed the title Fix occasional error while unmarshalling id_token Fix occasional error while unmarshalling Apple id_token Feb 7, 2024
@yaronius
Copy link
Contributor Author

yaronius commented Feb 7, 2024

Not sure why tests are failing 🤔

@yaronius
Copy link
Contributor Author

yaronius commented Feb 9, 2024

Hi @techknowlogick, sorry for direct ping, haven't seen you here for a while, and the mentioned issue seems to be affecting more Apple users recently. If I can do anything to help you, just let me know. Thank you!

@yaronius yaronius mentioned this pull request Feb 14, 2024
@techknowlogick
Copy link
Collaborator

techknowlogick commented Feb 15, 2024

thanks @yaronius, could you merge in the recent push to master, as I've updated stuff around tests/min go version

@yaronius
Copy link
Contributor Author

Thank you @techknowlogick, done, now all tests pass 🎉

@techknowlogick
Copy link
Collaborator

Thanks!

@techknowlogick techknowlogick merged commit e9e5240 into markbates:master Feb 16, 2024
15 checks passed
@techknowlogick
Copy link
Collaborator

I'll (hopefully) cut a release soon.

@dstapleton92
Copy link
Contributor

This has been driving us crazy! Thanks so much for the fix. Also looking forward to a release soon!

@techknowlogick
Copy link
Collaborator

@dstapleton92 thanks for the reminder. I just cut the release.

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

Successfully merging this pull request may close these issues.

3 participants