-
Notifications
You must be signed in to change notification settings - Fork 112
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
Allow to set a TokenGenerator to override default implementation. #94
base: master
Are you sure you want to change the base?
Conversation
It eases the use of JWT or other token types.
@GuilhemN Hi, is there any chance to get this merged in the nearest time? |
ping @GuilhemN |
Well this needs to be reviewed first but I don't know this library well enough to do it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good implementation and very useful.
Especially for people who has to implement oauth extensions like openid connect (such as me).
*/ | ||
public function setTokenGenerator(TokenGeneratorInterface $tokenGenerator) | ||
{ | ||
$this->tokenGenerator = $tokenGenerator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DI container should inject the specific generator so the value is set.
If no specific generator is available, then the default generator should be used.
@@ -1413,6 +1445,8 @@ private function createAuthCode(IOAuth2Client $client, $data, $redirectUri, $sco | |||
* | |||
* @ingroup oauth2_section_4 | |||
* @see OAuth2::genAuthCode() | |||
* | |||
* @deprecated since 1.3, will be removed in 2.0. Use a TokenGenerator instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be call the default generator method internally.
|
||
class RandomTokenGenerator implements TokenGeneratorInterface | ||
{ | ||
public function genAccessToken(IOAuth2Client $client, $data, $scope = null, $access_token_lifetime = null, $issue_refresh_token = true, $refresh_token_lifetime = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameters should be extraced as class members and set over an constructor or setter methods.
Are there any plans to merge it? |
@Valantir007 I guess @iamluc would have to make an effort and fix the comments added before merging is on the table. The PR has been sitting like this since it was created and I have my doubts whether the OP is keen on implementing the changes. |
@Valantir007 If you'd like to submit a new PR fixing the comments that would definitely help :) |
It eases the use of JWT or other token types.
Could also fix #86