always_issue_new_refresh_token does not work in OAuth2\Server constructor about oauth2-server-php-docs HOT 12 CLOSED

Can you explain what you mean by "For me this does not work"?

I have looked at the code, and we have tests that ensure always_issue_new_refresh_token works, and although we don't have test coverage, you can see in the code here that the refresh_token_lifetime configuration is also passed in from the Server class.

Perhaps you can post the code you use to instantiate the OAuth2\Server object, or submit a PR with a test case that proves this fails, and then I can better address your problem.

from oauth2-server-php-docs.

Hi bshaffer,

the test "testValidRefreshTokenWithNoRefreshTokenInResponse" only tests "always_issue_new_refresh_token" when given to the RefreshToken constructor, not to the OAuth\Server constructor.

Please try exactly what is written in the docs, and as quoted above. Try to set "always_issue_new_refresh_token" in the OAuth\Server constructor, you will see that it does not work, you don't get a new refresh token, because it's not merged into the RefreshToken object where it's used.

from oauth2-server-php-docs.

Aha, yes, my mistake. But this also has test coverage here

from oauth2-server-php-docs.

If you check the source code here:
https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/GrantType/RefreshToken.php#L32
you can see that only the RefreshToken constructor parameter $config is used to configure "always_issue_new_refresh_token". If you set this option in the OAuth2\Server constructor it's not used.

In the docs it says this is possible, but it isn't:

$server = new OAuth2\Server($storage, array(
  'always_issue_new_refresh_token' => true,
));

Maybe you can try it yourself?

from oauth2-server-php-docs.

I've now added this test as well, which is the same as the one referenced above, but setting the always_issue_new_refresh_token config on a server level. The test passes.

So I think there must be something else in your implementation that is different from the tests.

from oauth2-server-php-docs.

I think I know the problem...

The configuration to OAuth2\Server is ONLY used if the refresh token grant type isn't explicitly provided. For instance, if you do this:

$server = new OAuth2\Server($storage, array(
  'always_issue_new_refresh_token' => true,
));

$server->addGrantType(new RefreshToken());

Then the configuration supplied in the constructor for OAuth2\Server will be overwritten by the configuration supplied to RefreshToken.

My guess is this is the problem you're having. This behavior is working as designed, as the OAuth2\Server class is only a convenience class which wraps the other controllers and instantiates defaults. If you override these classes, then this will happen.

I agree this may not be expected behavior. If you have a proposed fix, I will take it into consideration. Otherwise, I'm closing this issue.

from oauth2-server-php-docs.

Thanks for the explanation, but I don't really understand, that makes no sense to me... If I don't add the RefreshToken grant type explicitly, I cannot use the refresh token feature...

If I just do this:
$server = new OAuth2\Server($storage, array(
'always_issue_new_refresh_token' => true,
));

then I get:
{"error":"unsupported_grant_type","error_description":"Grant type "refresh_token" not supported"}

That means "always_issue_new_refresh_token" is not used anywhere (because it's only used in the RefreshToken grant type, and that is disabled if I don#'t add it explicitly).
And if I add the grant type explicitly, then it is not used either, then I have to use the option parameter of the RefreshToken constructor...

So I'm not sure in which case this option makes any sense in the OAuth\Server constructor... Maybe you can enlight me?

from oauth2-server-php-docs.

Yeah, no problem. The only reason that grant type wouldn't be added is if 1) your $storage object does not implement OAuth2\Storage\RefreshTokenInterface, or if you explicitly set your grant types in the constructor of the Server class, or if you specify a storage array and map the storages to keys, such as:

$server = OAuth2\Server(array('client_credentials'=> $storage));
—
Brent Shaffer

On Tue, Jul 1, 2014 at 6:22 PM, Michael Kliewe [email protected]
wrote:

Thanks for the explanation, but I don't really understand, that makes no sense to me... If I don't add the RefreshToken grant type explicitly, I cannot use the refresh token feature...
If I just do this:
$server = new OAuth2\Server($storage, array(
'always_issue_new_refresh_token' => true,
));
then I get:
{"error":"unsupported_grant_type","error_description":"Grant type "refresh_token" not supported"}
That means "always_issue_new_refresh_token" is not used anywhere (because it's only used in the RefreshToken grant type, and that is disabled if I don#'t add it explicitly).
And if I add the grant type explicitly, then it is not used either, then I have to use the option parameter of the RefreshToken constructor...

So I'm not sure in which case this option makes any sense in the OAuth\Server constructor... Maybe you can enlight me?

Reply to this email directly or view it on GitHub:
#40 (comment)

from oauth2-server-php-docs.

I'm sorry, but all 3 things do not apply... Here is my code:

$config = array(
    'access_lifetime'                => 7*86400,    // 7 days
    'refresh_token_lifetime'         => 6*31*86400, // 6 month
    'always_issue_new_refresh_token' => true
);

$storage = new \App\OAuth2\Storage\Pdo();
$server = new OAuth2\Server($storage, $config);
$server->addGrantType(new OAuth2\GrantType\UserCredentials($storage));
$server->handleTokenRequest(OAuth2\Request::createFromGlobals())->send();

My own Pdo class looks like this:

class Pdo implements AccessTokenInterface, ClientCredentialsInterface, UserCredentialsInterface, RefreshTokenInterface

As you can see

1. I'm implementing RefreshTokenInterface in my storage class
2. I don't set the grant types in the constructor of the Server class
3. I don't have a storage array and I'm not mapping the storages to keys

But the RefreshToken grant type is not added automatically, I have to do it explicitly...

Maybe I should not kill your time any longer, it's working if I set the parameter in the constructor of the RefreshToken, that's all I want at the moment. It would be nice if all configuration settings could be in one place (the server constructor), but that's not the case, I'm fine with that at the moment.

from oauth2-server-php-docs.

Try removing the line where you add a user credentials grant type.  If that works, then I think I know the problem…—
Brent Shaffer

On Tue, Jul 1, 2014 at 7:05 PM, Michael Kliewe [email protected]
wrote:

I'm sorry, but all 3 things do not apply... Here is my code:
$config = array(
'access_lifetime' => 7_86400, // 7 days
'refresh_token_lifetime' => 6_31*86400, // 6 month
'always_issue_new_refresh_token' => true
);

$storage = new \App\OAuth2\Storage\Pdo();
$server = new OAuth2\Server($storage, $config);
$server->addGrantType(new OAuth2\GrantType\UserCredentials($storage));
$server->handleTokenRequest(OAuth2\Request::createFromGlobals())->send();

My own Pdo class looks like this:
class Pdo implements AccessTokenInterface, ClientCredentialsInterface, UserCredentialsInterface, RefreshTokenInterface
As you can see

1. I'm implementing RefreshTokenInterface in my storage class
2. I don't set the grant types in the constructor of the Server class
3. I don't have a storage array and I'm not mapping the storages to keys
   But the RefreshToken grant type is not added automatically, I have to do it explicitly...

Maybe I should not kill your time any longer, it's working if I set the parameter in the constructor of the RefreshToken, that's all I want at the moment. It would be nice if all configuration settings could be in one place (the server constructor), but that's not the case, I'm fine with that at the moment.

Reply to this email directly or view it on GitHub:
#40 (comment)

from oauth2-server-php-docs.

You are right, if I remove the addGrantType() call, then it works as you said...

But in my case I want to only allow the grant_type=password (and refresh_token), and disallow grant_type=client_credentials. To do that I had to add UserCredentials and RefreshToken grant types explicitly. And then I had to move the configuration of "always_issue_new_refresh_token" from the server constructor to the RefreshToken constructor. That was the problem I had, that was not obvious and a bit strange.

Thanks for you help, let's hope nobody else faces this problem ;-)

from oauth2-server-php-docs.

Great! Yes, I agree the convenience of the server class is a double – edge sword. However, I cannot think of a better way to handle this, other than maybe by throwing an exception when this problem occurs. What are your thoughts on this?—
Brent Shaffer

On Tue, Jul 1, 2014 at 7:21 PM, Michael Kliewe [email protected]
wrote:

You are right, if I remove the addGrantType() call, then it works as you said...
But in my case I want to only allow the grant_type=password (and refresh_token), and disallow grant_type=client_credentials. To do that I had to add UserCredentials and RefreshToken grant types explicitly. And then I had to move the configuration of "always_issue_new_refresh_token" from the server constructor to the RefreshToken constructor. That was the problem I had, that was not obvious and a bit strange.

Thanks for you help, let's hope nobody else faces this problem ;-)

Reply to this email directly or view it on GitHub:
#40 (comment)

from oauth2-server-php-docs.