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

Refresh Token gets new token without client id and client secret #1150

Closed
hszeto opened this issue Sep 25, 2018 · 7 comments
Closed

Refresh Token gets new token without client id and client secret #1150

hszeto opened this issue Sep 25, 2018 · 7 comments
Assignees
Labels

Comments

@hszeto
Copy link

hszeto commented Sep 25, 2018

According to the docs, https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#curl-command-refresh-token-grant

client id and secret are required. But, refresh token grant type works even though client id and client secret are not specified.

In /initializers/doorkeeper.rb , use_refresh_token has been specified.

Steps to reproduce

curl -X POST
http://localhost:3030/oauth/token
-H 'Cache-Control: no-cache'
-H 'Content-Type: application/json'
-d '{
"grant_type": "refresh_token",
"refresh_token": "8682......fabee5"
}'

Expected behavior

Expected a 401

Actual behavior

Got a new access token

Ruby version: 2.51

Gemfile.lock: doorkeeper (5.0.0)

@hszeto
Copy link
Author

hszeto commented Sep 25, 2018

also, re-using the same refresh token (without specify client id and secret) will generate new access tokens. Do refresh token get revoked after one time use?

@nbulaj nbulaj self-assigned this Sep 25, 2018
@nbulaj nbulaj added the bug label Sep 25, 2018
@nbulaj
Copy link
Member

nbulaj commented Sep 25, 2018

Hi @hszeto . I took a deep look into OAuth 2.0 draft, Section 6. Refreshing an Access Token and wrote a spec for mentioned behavior.

Yes, it seems like we have a bug here.

What about revoke refresh token after use, take a look into RFC:

The authorization server MAY revoke the old
refresh token after issuing a new refresh token to the client.

As far as I remember we have such an option in the Doorkeeper configuration, but fix me if I wrong

@nbulaj
Copy link
Member

nbulaj commented Sep 25, 2018

Take a look at 574749b

@hszeto
Copy link
Author

hszeto commented Sep 25, 2018

thanks for the quick work. Looks good to me.

@nbulaj
Copy link
Member

nbulaj commented Sep 26, 2018

Dooh, closed automatically with GitHub.

Merged to master and will be released with next release. Also I need to backport it to 4.x version

Thanks @hszeto for clear explanation and upping this issue!

@seanstory
Copy link

seanstory commented May 8, 2020

Also I need to backport it to 4.x version

@nbulaj this issue is years old so I may be too late, but it looks like this never made it into 4.x: https://github.com/doorkeeper-gem/doorkeeper/blob/v4.4.3/lib/doorkeeper/oauth/refresh_token_request.rb#L83-L85

@nbulaj
Copy link
Member

nbulaj commented May 8, 2020

Yep, my fault @seanstory . Thanks for reporting!

But for now I don't have plans to support 4.x version of the gem. I'm even planning to drop support for 5.0 and 5.1. So I recommend to update the gem to latest version. Thanks once again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants