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.

Sign up for GitHub

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

Jump to bottom

If JWT custom claim field is string, parse as JSON #5513

Closed
wants to merge 1 commit into from
Closed

If JWT custom claim field is string, parse as JSON #5513

wants to merge 1 commit into from

Conversation

dpeek
Copy link
Contributor

@dpeek dpeek commented May 25, 2020 •
edited by manishrjain
Loading

Amazon Cognito does not support objects as JWT custom claim values. If custom claim is string, parse as JSON.

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented May 25, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@pawanrawal pawanrawal requested a review from arijitAD May 28, 2020 10:08
arijitAD
arijitAD approved these changes May 28, 2020
View reviewed changes
Copy link

@arijitAD arijitAD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MichaelJCompton and @pawanrawal)

@arijitAD
Copy link

@dpeek Thank you for the PR! I will add a test to this and get it merged to master. Also, Can you provide me a JWT with a custom claim generated by Amazon Cognito?

@dpeek
Copy link
Contributor Author

dpeek commented Jun 2, 2020

Hi @arijitAD, apologies for the delay, here is an example (expired) token:

eyJraWQiOiIyRWplN2tIRklLZVwvdjBUVU9yUWJVSVlicUljTVB2dkxQYzN0UkNuRHJQQT0iLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6XC9cL2NvZ25pdG8taWRwLmFwLXNvdXRoZWFzdC0yLmFtYXpvbmF3cy5jb21cL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczpcL1wvd3d3LnR5cGVqb2luLmNvbVwvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6IjYzZG8wcTE2bjZlYmpna3VtdTA1a2tlaWFuIiwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjE1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.Fl1zQeTwEFPSHRA-EJ5BzAzRDqVnsmiSvSd1xEkYNQt1lxNEuOwMPMSa-i9fu-AKFR9YJlb653jzQVitha3hy8UjyrNF7nNmuDrUZ-hfroxo4QbQ49fO-dvwUXXywl1m9huE_LPp80yk2V2KB27mRC3wayjGLNx6OGtFknmK5rHfiRD867D2jwtwIWNm8yxhiPpHyOrDYF-xHEVvaJsyu8VRCsMJfOK_Eh-3rLBvetb_RnafWOq3BRPj3cuDHslw_we7RtiCaEkGjLRfwGdMegzXdePBHX8PxNuPQ1GuUaoW94LouZhFtJ2LIEaKeAwFEQi7ExhXF64R5xuU0EkFuw

@dpeek
Copy link
Contributor Author

dpeek commented Jun 10, 2020

Anything I can do to help get this merged? I've run out of other items on my todo so it's back to getting auth working :)

@MichaelJCompton MichaelJCompton added the area/graphql Issues related to GraphQL support on Dgraph. label Jun 17, 2020
@MichaelJCompton
Copy link
Contributor

@arijitAD can you get this through please.

@arijitAD arijitAD mentioned this pull request Jun 17, 2020
Merged
@arijitAD
Copy link

arijitAD commented Jun 18, 2020 •
edited
Loading

@dpeek I have added the unit test and merged the PR to master. #5668 Appreciate your help.

@arijitAD arijitAD closed this Jun 18, 2020
@dpeek
Copy link
Contributor Author

dpeek commented Jun 18, 2020

Thanks @arijitAD!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arijitAD arijitAD arijitAD approved these changes

@MichaelJCompton MichaelJCompton Awaiting requested review from MichaelJCompton

@pawanrawal pawanrawal Awaiting requested review from pawanrawal

Assignees

@arijitAD arijitAD

Labels
area/graphql Issues related to GraphQL support on Dgraph.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dpeek @CLAassistant @arijitAD @MichaelJCompton