-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
fix(oauth2) checking client app when refreshing a token #2461
Conversation
subnetmarco
commented
Apr 27, 2017
- When refreshing a token the client application ownership is properly checked
957acf1
to
385f21d
Compare
@@ -1501,6 +1512,27 @@ describe("#ci Plugin: oauth2 (access)", function() | |||
local body = assert.res_status(200, res) | |||
assert.is_table(ngx.re.match(body, [[^\{"refresh_token":"[\w]{32,32}","token_type":"bearer","state":"wot","access_token":"[\w]{32,32}","expires_in":5\}$]])) | |||
end) | |||
it("#only fails when the client used for the code is not the same clien used for the token", function() |
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.
do we want to leave the '#only' tag?
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.
also, typo, 'clien' => 'client'
kong/plugins/oauth2/access.lua
Outdated
@@ -357,8 +357,13 @@ local function issue_token(conf) | |||
if not token then | |||
response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..REFRESH_TOKEN} | |||
else | |||
response_params = generate_token(conf, ngx.ctx.api, client, token.authenticated_userid, token.scope, state) | |||
singletons.dao.oauth2_tokens:delete({id=token.id}) -- Delete old token | |||
-- Check that the token belongs the the client application |
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.
typo in comment: 'belongs to the client application'
dc12509
to
b83a684
Compare
lgtm, apart from the Changelog conflict; since this is a merge to master and I'm not overly familiar with Oauth2, would like to get a second pair of eyes here. |