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

feat(idtoken): add ParsePayload returning unvalidated token payload #2136

Merged
merged 8 commits into from
Sep 14, 2023

Conversation

eytankidron
Copy link
Contributor

@eytankidron eytankidron commented Sep 7, 2023

GetPayload attempts to parse the token and return the parsed payload from therein. It does not attempt to Validate the token semantically or otherwise. This is especially useful for the case where Validate fails and the user wants to inspect the payload directly (Validate doesn't return the payload on failure currently).

Also, improve the error message for token expiration by including the expiration time and the evaluated time.

Fixes #2137

…rification.

Also make the error message for token expiration slightly more informative.
Both of these changes are made in order to improve debugability in cases where verification fails.
@eytankidron eytankidron requested a review from a team as a code owner September 7, 2023 19:33
@conventional-commit-lint-gcf
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@eytankidron eytankidron changed the title Add GetPayload() that only returns the payload, skipping the token ve… Add GetPayload() that only returns the payload, skipping the token verification Sep 7, 2023
@codyoss codyoss added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 7, 2023
@codyoss
Copy link
Member

codyoss commented Sep 7, 2023

Please open an issue for discussion before making changes detailing the how/why for the feature request.

@eytankidron
Copy link
Contributor Author

Created #2137

Copy link
Contributor

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

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

Please add some tests as well, thanks.

idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
@eytankidron
Copy link
Contributor Author

Thanks Noah for the comment improvements. I will also add unit tests shortly.

@codyoss codyoss removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 13, 2023
@noahdietz noahdietz changed the title Add GetPayload() that only returns the payload, skipping the token verification feat(idtoken): add GetPayload returning unvalidated token payload Sep 13, 2023
@eytankidron
Copy link
Contributor Author

Added unit tests

idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
idtoken/validate_test.go Outdated Show resolved Hide resolved
idtoken/validate_test.go Outdated Show resolved Hide resolved
idtoken/validate.go Outdated Show resolved Hide resolved
@noahdietz noahdietz requested a review from codyoss September 14, 2023 15:38
@noahdietz
Copy link
Contributor

LGTM @codyoss please stamp and I will get this into the next release (hopefully today!)

@codyoss codyoss changed the title feat(idtoken): add GetPayload returning unvalidated token payload feat(idtoken): add ParsePayload returning unvalidated token payload Sep 14, 2023
@noahdietz noahdietz enabled auto-merge (squash) September 14, 2023 18:00
@noahdietz noahdietz added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 14, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 14, 2023
@noahdietz noahdietz merged commit d541d8e into googleapis:main Sep 14, 2023
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Sep 14, 2023
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.

Add a function idtoken.GetPayload()
4 participants