oauth2-server
oauth2-server copied to clipboard
thephpleague
Ability to extend the JWT token with information
Right now the JWT token has one custom claim called scopes. But it would be a nice feature to add some more claims to the JWT when required.
Like a username, email, user-group, etc.
If/when this is available, implementors should be extra cautious about what they store in the JWT - especially if it's not encrypted - but even if it is, maybe things like email address aren't the best things to be putting inside a token. Use OpenID Connect instead.
Also, consider the extra weight that adding new claims will give to your headers - in some cases, larger JWTs could cause unexpected errors.
Personally, I would err on the side of keeping tokens slimmer, choosing to add an extra request to your process for less headaches.
+1, I implemented this now (for adding a group ID to the token) by extending all the grants, which isn't the nicest for maintainability.
@simonhamp Your concern is genuine, but I think it goes against spirit of this library of allowing flexibility in terms of "engine and tires". A word of warning will suffice for those who want to extends it and if someone extends it without knowing what they are doing, let them shoot their feet!
I suggest some sort of freedom we have in other things get extended there too!
@mtangoo I appreciate your point, but I have to disagree. I don't feel that what you suggest is the correct approach. Letting others 'shoot their feet' is one of the factors involved in why there are leaks of billions of people's personal details... we need to help each other improve security and an opinionated stance on the part of widely-used, standards-compliant libraries such as this can make a huge difference on that front.
Flexibility where flexibility is safe and useful, not just for flexibility's sake.
we need to help each other improve security and an opinionated stance on the part of widely-used.....Flexibility where flexibility is safe and useful, not just for flexibility's sake.
Thanks for genuine concerns. My point was, putting ability to extend as completely optional and add even a warning that only people who know what they are doing should even venture there. If someone still feels brave to transgress that, then let them shoot their feets for sure.
Am not against helping people to be compliant. But I really get worried when that ends up crippling flexibility.
Cheers!
@christiaangoossens do you mind sharing how you did the adding of (in your case) the group ID to the JWT token? I am needing this as well because users have access to multiple accounts. You mentioned extending the Grant types but I'm unsure how.
@nealoke all you need to do is reimplement AccessTokenEntity::convertToJWT() Copy it from AccessTokenTrait and adjust as needed No grant extension needed
It would be nice to be able to do this without having to re-implement/copy+paste the guts of AccessTokenTrait::convertToJWT() completely.
How about if we added the following method to AccessTokenTrait:
protected function setCustomScopes(\Lcobucci\JWT\Builder $builder): \Lcobucci\JWT\Builder
{
return $builder;
}
That gives any descendants the opportunity to simply override the setCustomScopes() method, and then in AccessTokenTrait::convertToJWT() we can run the builder through that method before returning the token.
Agree with @uphlewis, even if convertToJWT() returned an instance of Builder and not Token we could extend the trait and modify the token returned in __toString()
Even once you have managed to add custom claims to the JWT, how do you get them back out of the validated request?
Currently BearerTokenValidator is hard-coded to return only 4 specific claims:
// Return the request with additional attributes
return $request
->withAttribute('oauth_access_token_id', $token->getClaim('jti'))
->withAttribute('oauth_client_id', $token->getClaim('aud'))
->withAttribute('oauth_user_id', $token->getClaim('sub'))
->withAttribute('oauth_scopes', $token->getClaim('scopes'))
Since the 8.x release, you cannot override convertToJWT anymore, since it's marked private instead of public. @Sephster, is there any specific reason to not make it protected instead? I recognize the intention to keep things safe and prevent users from shooting their own foot, but OTOH I know what I'm doing and need a few custom claims in my tokens, which will be read by other applications.
It'd be nice to have a way to extend tokens without having to reimplement the whole library :(
Edit: Sorry, my bad. By overriding __toString, we can call a custom implementation instead. This still feels awkward, though, and doesn't invalidate the problem highlighted by @ol1s.
What is the status of this? I also need to add data to the generated JWT. In my case, I need to set a "KID" parameter in the header of the token.
This is an old thread, almost 4 years old.. Did someone find a workaround??
@zploited the only way is by extending the class(es) (and binding your implementation(s) in your dependency container if you use one)
TLDR; my 2 cents for function convertToJWT being private
use AccessTokenTrait{ convertToJWT as public parentconvertToJWT; }
on the topic of if this is needed - I agree that
- not exposing the builder
- having Builder as a final class, hence limiting the ability to work this in initJwtConfiguration (like withHeader)
- not returning Builder with convertToJWT (or an internal function one could override) yet Plain from convertToJWT, which does all the building
is limiting options for people who wanna type "yes, please do" into the code.
I think it's hard to argue that maintaining an out of library copy of convertToJWTor ending up with an disconnected fork is safer on average for someone who needs this.
That said, I can live with the current solution. Thx for this nice clean repo.
Currently looking to use this via Laravel passport and unfortunately going down rabbit holes trying to implement a custom public claim. Please consider making this more easily extensible since the alternative is that I will have to rewrite some of the inner workings which would be unfortunate just for the sake of an ideology.
I understand why you may think that adding this feature could allow people to make their systems less secure. I would still argue that we should educate and allow for informed programmers to extend rather than prohibit said extension. Prohibition of this fairly requested extension has already led to custom work arounds that are far less future proof and could lead to missing patches and bug fixes.
