Skip to content

Commit

Permalink
fix(oauth2): fix a bug that refresh_token could be shared across inst…
Browse files Browse the repository at this point in the history 
…ances (#11342)

* fix(oauth2): fix a bug that refresh_token could be shared across
services when `global_credentials` is set as `true`.

With `global_credential=true`, `access_token` can be shared across
services, as well as `refresh_token` currently. This means that a
`refresh_token` belonging to a service can be used to refresh tokens
belonging to another service, which is consider as a bug.
In this PR, the scope is taken into account as a new creteria of the
request validation. Scopes associated with a token provided in the
request will be compared with those configured in the Oauth2 instance
hit by that request.

FTI-5173

Co-authored-by: Hans Hübner <[email protected]>
  • Loading branch information
@liverpool8056 @hanshuebner
liverpool8056 and hanshuebner authored Aug 10, 2023
1 parent 07475c4 commit 7821654
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
`max_retry_delay` must now be `number`s greater than 0.001
(seconds).
[#10840](https://github.com/Kong/kong/pull/10840)
- For OAuth2 plugin, `scope` has been taken into account as a new criterion of the request validation. When refreshing token with `refresh_token`, the scopes associated with the `refresh_token` provided in the request must be same with or a subset of the scopes configured in the OAuth2 plugin instance hit by the request.
[#11342](https://github.com/Kong/kong/pull/11342)

### Additions

Expand Down
21 changes: 17 additions & 4 deletions kong/plugins/oauth2/access.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,15 +757,28 @@ local function issue_token(conf)
error_description = "Invalid " .. REFRESH_TOKEN
}

else
-- Check that the token belongs to the client application
if token.credential.id ~= client.id then
-- Check that the token belongs to the client application
elseif token.credential.id ~= client.id then
response_params = {
[ERROR] = "invalid_client",
error_description = "Invalid client authentication"
}

else
else
-- Check scopes
if token.scope then
for scope in token.scope:gmatch("%S+") do
if not table_contains(conf.scopes, scope) then
response_params = {
[ERROR] = "invalid_scope",
error_description = "Scope mismatch",
}
break
end
end
end

if not response_params[ERROR] then
response_params = generate_token(conf, kong.router.get_service(),
client,
token.authenticated_userid,
Expand Down
201 changes: 201 additions & 0 deletions spec/03-plugins/25-oauth2/03-access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
local service15 = admin_api.services:insert()
local service16 = admin_api.services:insert()
local service17 = admin_api.services:insert()
local service18 = admin_api.services:insert()
local service19 = admin_api.services:insert()
local service20 = admin_api.services:insert()
local service21 = admin_api.services:insert()

local route1 = assert(admin_api.routes:insert({
hosts = { "oauth2.com" },
Expand Down Expand Up @@ -377,6 +381,30 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
service = service17,
}))

local route18 = assert(admin_api.routes:insert({
hosts = { "oauth2_18.com" },
protocols = { "http", "https" },
service = service18,
}))

local route19 = assert(admin_api.routes:insert({
hosts = { "oauth2_19.com" },
protocols = { "http", "https" },
service = service19,
}))

local route20 = assert(admin_api.routes:insert({
hosts = { "oauth2_20.com" },
protocols = { "http", "https" },
service = service20,
}))

local route21 = assert(admin_api.routes:insert({
hosts = { "oauth2_21.com" },
protocols = { "http", "https" },
service = service21,
}))

local service_grpc = assert(admin_api.services:insert {
name = "grpc",
url = helpers.grpcbin_url,
Expand Down Expand Up @@ -556,6 +584,38 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
global_credentials = true,
}
})

admin_api.oauth2_plugins:insert({
route = { id = route18.id },
config = {
scopes = { "scope18" },
global_credentials = true,
}
})

admin_api.oauth2_plugins:insert({
route = { id = route19.id },
config = {
scopes = { "scope19" },
global_credentials = true,
}
})

admin_api.oauth2_plugins:insert({
route = { id = route20.id },
config = {
scopes = { "scope20" },
global_credentials = true,
}
})

admin_api.oauth2_plugins:insert({
route = { id = route21.id },
config = {
scopes = { "scope20", "scope21" },
global_credentials = true,
}
})
end)

before_each(function ()
Expand Down Expand Up @@ -2905,6 +2965,147 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
})
assert.res_status(401, res)
end)

it("refreshing token fails when scope is mismatching", function ()
-- provision code
local code, body, res
local request_client = helpers.proxy_ssl_client()
body = {
provision_key = "provision123",
client_id = "clientid123",
response_type = "code",
scope = "scope18",
state = "hello",
authenticated_userid = "userid123",
}
res = assert(request_client:send {
method = "POST",
path = "/oauth2/authorize",
body = body,
headers = kong.table.merge({
["Host"] = "oauth2_18.com",
["Content-Type"] = "application/json"
})
})
res = assert(cjson.decode(assert.res_status(200, res)))
if res.redirect_uri then
local iterator, err = ngx.re.gmatch(res.redirect_uri, "^http://google\\.com/kong\\?code=([\\w]{32,32})&state=hello$")
assert.is_nil(err)
local m, err = iterator()
assert.is_nil(err)
code = m[1]
end

-- provision token
body = {
code = code,
client_id = "clientid123",
client_secret = "secret123",
grant_type = "authorization_code",
redirect_uri = "http://google.com/kong",
}
res = assert(request_client:send {
method = "POST",
path = "/oauth2/token",
body = body,
headers = {
["Host"] = "oauth2_18.com",
["Content-Type"] = "application/json"
}
})
local token = assert(cjson.decode(assert.res_status(200, res)))

-- refresh token with mismatching scope
res = assert(request_client:send {
method = "POST",
path = "/oauth2/token",
body = {
refresh_token = token.refresh_token,
client_id = "clientid123",
client_secret = "secret123",
grant_type = "refresh_token",
},
headers = {
["Host"] = "oauth2_19.com",
["Content-Type"] = "application/json"
}
})
res = assert(cjson.decode(assert.res_status(400, res)))
assert.same({
error = "invalid_scope",
error_description = "Scope mismatch"
}, res)
request_client:close()
end)

it("refreshing token succeeds when scope is a subset", function()
-- provision code
local code, body, res
local request_client = helpers.proxy_ssl_client()
body = {
provision_key = "provision123",
client_id = "clientid123",
response_type = "code",
scope = "scope20",
state = "hello",
authenticated_userid = "userid123",
}
res = assert(request_client:send {
method = "POST",
path = "/oauth2/authorize",
body = body,
headers = kong.table.merge({
["Host"] = "oauth2_20.com",
["Content-Type"] = "application/json"
})
})
res = assert(cjson.decode(assert.res_status(200, res)))
if res.redirect_uri then
local iterator, err = ngx.re.gmatch(res.redirect_uri, "^http://google\\.com/kong\\?code=([\\w]{32,32})&state=hello$")
assert.is_nil(err)
local m, err = iterator()
assert.is_nil(err)
code = m[1]
end

-- provision token
body = {
code = code,
client_id = "clientid123",
client_secret = "secret123",
grant_type = "authorization_code",
redirect_uri = "http://google.com/kong",
}
res = assert(request_client:send {
method = "POST",
path = "/oauth2/token",
body = body,
headers = {
["Host"] = "oauth2_20.com",
["Content-Type"] = "application/json"
}
})
local token = assert(cjson.decode(assert.res_status(200, res)))

-- refresh token with mismatching scope
local res = assert(request_client:send {
method = "POST",
path = "/oauth2/token",
body = {
refresh_token = token.refresh_token,
client_id = "clientid123",
client_secret = "secret123",
grant_type = "refresh_token",
},
headers = {
["Host"] = "oauth2_21.com",
["Content-Type"] = "application/json"
}
})
assert(cjson.decode(assert.res_status(200, res)))
request_client:close()
end)

it("fails when a correct access_token is being sent in the wrong header", function()
local token = provision_token("oauth2_11.com",nil,"clientid1011","secret1011")

Expand Down

1 comment on commit 7821654

@khcp-gha-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:78216546f06ce4e8c4352c5bb187a1ce099406cf
Artifacts available https://github.com/Kong/kong/actions/runs/5821882514

Please sign in to comment.