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

Support JWT custom claims #138

Closed
Morishiri opened this issue Mar 29, 2021 · 6 comments
Closed

Support JWT custom claims #138

Morishiri opened this issue Mar 29, 2021 · 6 comments
Assignees
Labels
enhancement Enhancement of existing feature

Comments

@Morishiri
Copy link
Contributor

Morishiri commented Mar 29, 2021

Proposal

I propose to support custom claims in the generated JWT token.

The user fields to be included as claims could be specified by adding auth:"jwtInclude" (to follow current auth prefix).

This would allow people putting custom information in the token, e.g. when thinking about username, some additional name fields or permissions (I saw you're planning support in the future).

Possible drawbacks

By default - none, if someone uses too many fields then the token will be heavy, but that's up to user of the framework.

Additional information

I think I can handle this contribution if you agree.

@System-Glitch System-Glitch added enhancement Enhancement of existing feature feature request Request for new feature implementation labels Mar 29, 2021
@System-Glitch
Copy link
Member

Hello and thank you for your proposal! I was planning on adding a auth.GenerateTokenWithClaims() function in the near future, which would let you add custom claims.

Can you elaborate on how you planned on implementing this feature? I think this would be a great addition.

Do you need your JWT in other applications? You probably don't need to add anything else than the ID of the user since the authenticator will fetch the user for you in the database, so I'm not sure a struct tag auth:"jwtInclude" would be really useful (and it would require some reflection).

@Morishiri
Copy link
Contributor Author

I was planning on adding a auth.GenerateTokenWithClaims() function in the near future, which would let you add custom claims.

Can you elaborate on how you planned on implementing this feature? I think this would be a great addition.

Adding auth.GenerateTokenWithClaims() would help, my plan was simply not to introduce additional functions, but it actually may be better to do so to not use reflection too much. The idea was to add auth:"jwtInclude" annotation to the fields from the User struct which should be added to the token, then get this values from the user object and simply put them as claims, but as you already said, this would require reflection to get the fields annotations and may slow things down.

Do you need your JWT in other applications? You probably don't need to add anything else than the ID of the user since the authenticator will fetch the user for you in the database, so I'm not sure a struct tag auth:"jwtInclude" would be really useful (and it would require some reflection).

It can be used for e.g. to put user first and last name to the token, so when the user is logged in I can simply look into the token and display his name in the header menu (e.g. avatar button on github, there is Signed in as <nickname> text) without the need to execute a call to /users/me (or some similar route). But it's not critical. The authenticator fetching the user object is really great feature and implementing such /me endpoint is not a big hassle with that.

What I proposed was mainly front-end related to just read the token and not call APIs, may be improvement for some people, but I agree that in this case separate function may be better approach.

@System-Glitch
Copy link
Member

I understand your use-case. The best approach would be to implement your own Login handler and use this new GenerateTokenWithClaims() function to add everything you need, instead of using the built-in JWTController. But re-implementing that is quite a bit of work and produces duplicate code.

Maybe we could add some features to the JWTController to make it more flexible. What would you think about something like this?

jwtController := auth.NewJWTController()
jwtController.TokenFunc = func(r *goyave.Request, user interface{}) (string, error) {
    return auth.GenerateTokenWithClaims(jwt.MapClaims{
        "name": user.(*model.User).Name
    })
}

@Morishiri
Copy link
Contributor Author

Yeah, I think it's good idea, while leaving the default as it is currently. Then we don't have any breaking changes.

@System-Glitch
Copy link
Member

If you are still interested in the implementation of this, please let me know! Any contribution is very much welcome!

@Morishiri
Copy link
Contributor Author

Yes, I'm interested ;)

@System-Glitch System-Glitch removed the feature request Request for new feature implementation label Mar 29, 2021
@System-Glitch System-Glitch mentioned this issue Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing feature
Projects
None yet
Development

No branches or pull requests

2 participants