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

Separate verification into its own function #396

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quantumsheep
Copy link

Hello!

I have this use-case where I want to parse the JWT, fetch the secret elsewhere and then verify the JWT.

The current available functions forces me to parse the JWT another time. I want to validate the token, not parse it again.

Thanks,

@oxisto
Copy link
Collaborator

oxisto commented Jul 4, 2024

Maybe a trivial question, but can't you do this logic inside the keyfunc? You have access to the *Token which has been parsed (but not validated) up to this point. You can then fetch the secret and then return it. This is basically what libraries such as https://github.com/MicahParks/keyfunc do.

@quantumsheep
Copy link
Author

I sometimes only parse the JWT to speedup the process when it comes from a fully trusted source (from internal code). I could duplicate some code to make it work but separating the functions costs nothing and fits my use-case.

@oxisto
Copy link
Collaborator

oxisto commented Jul 4, 2024

I sometimes only parse the JWT to speedup the process when it comes from a fully trusted source (from internal code). I could duplicate some code to make it work but separating the functions costs nothing and fits my use-case.

We are extremely careful about introducing new public functions because we need to maintain them in a way that we cannot break their function signature for quite a long time (since we tend to stick with major versions for quite a while). So yes, separating these functions actually does costs something: the time of a maintainer ;)

We intentionally did not expose any of these functions to not confuse people who might not be as experienced as you and might be confused, whether a simple Parse is enough or if VerifyToken is also needed; probably further complicated through the fact that we also have now a "validator".

As a bare minimum this function needs a godoc string and we probably would need to have an additional though about its function signature, because as I said before we need to stick with it for quite a while.

@oxisto
Copy link
Collaborator

oxisto commented Jul 23, 2024

any though on this @mfridman ?

@mfridman
Copy link
Member

mfridman commented Sep 6, 2024

Yeah I'm okay with this. Needs a godoc comment though.

return p.VerifyToken(token, parts, claims, keyFunc)
}

func (p *Parser) VerifyToken(token *Token, parts []string, claims Claims, keyFunc Keyfunc) (*Token, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@quantumsheep Can you supply a good for this function? Then we can accept the merge.

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