Skip to content

Commit

Permalink
Merge pull request Kong#930 from Mashape/fix/oauth2-redirect-uri
Browse files Browse the repository at this point in the history 
fix(oauth) better handling of redirect_uri
  • Loading branch information
@thibaultcha
thibaultcha committed Feb 5, 2016
2 parents b9d61e7 + 7d2edc4 commit b4ce5c9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 11 deletions.
21 changes: 18 additions & 3 deletions kong/plugins/oauth2/access.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ local cache = require "kong.tools.database_cache"
local responses = require "kong.tools.responses"
local constants = require "kong.constants"
local timestamp = require "kong.tools.timestamp"
local url = require "socket.url"

local _M = {}

Expand Down Expand Up @@ -113,10 +114,9 @@ end

local function authorize(conf)
local response_params = {}

local parameters = retrieve_parameters()
local state = parameters[STATE]
local redirect_uri, client
local redirect_uri, client, parsed_redirect_uri

if not is_https(conf) then
response_params = {[ERROR] = "access_denied", error_description = "You must use HTTPS"}
Expand All @@ -140,8 +140,15 @@ local function authorize(conf)

-- Check client_id and redirect_uri
redirect_uri, client = get_redirect_uri(parameters[CLIENT_ID])
parsed_redirect_uri = url.parse(redirect_uri)
if not redirect_uri then
response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..CLIENT_ID}
elseif not parsed_redirect_uri then
response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..REDIRECT_URI }
redirect_uri = nil
elseif parsed_redirect_uri.fragment ~= nil then
response_params = {[ERROR] = "invalid_request", error_description = "Fragment not allowed in "..REDIRECT_URI }
redirect_uri = nil
elseif parameters[REDIRECT_URI] and parameters[REDIRECT_URI] ~= redirect_uri then
response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..REDIRECT_URI.." that does not match with the one created with the application"}
end
Expand Down Expand Up @@ -172,9 +179,17 @@ local function authorize(conf)
-- Adding the state if it exists. If the state == nil then it won't be added
response_params.state = state

-- Appending kong generated params to redirect_uri query string
if parsed_redirect_uri then
if not parsed_redirect_uri.query then
parsed_redirect_uri.query = ""
end
parsed_redirect_uri.query = utils.encode_args(utils.table_merge(ngx.decode_args(parsed_redirect_uri.query), response_params))
end

-- Sending response in JSON format
return responses.send(response_params[ERROR] and 400 or 200, redirect_uri and {
redirect_uri = redirect_uri.."?"..ngx.encode_args(response_params)
redirect_uri = url.build(parsed_redirect_uri)
} or response_params, false, {
["cache-control"] = "no-store",
["pragma"] = "no-cache"
Expand Down
36 changes: 28 additions & 8 deletions spec/plugins/oauth2/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ describe("Authentication Plugin", function()
{ name = "oauth2", config = { scopes = { "email", "profile", "user.email" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_implicit_grant = true, accept_http_if_already_terminated = true }, __api = 6 },
},
oauth2_credential = {
{ client_id = "clientid123", client_secret = "secret123", redirect_uri = "http://google.com/kong", name="testapp", __consumer = 1 }
{ client_id = "clientid123", client_secret = "secret123", redirect_uri = "http://google.com/kong", name="testapp", __consumer = 1 },
{ client_id = "clientid456", client_secret = "secret456", redirect_uri = "http://google.com/kong#withfragment", name="testapp2", __consumer = 1 },
{ client_id = "clientid789", client_secret = "secret789", redirect_uri = "http://google.com/kong?foo=bar&code=123", name="testapp3", __consumer = 1 }
}
}
spec_helper.start_kong()
Expand Down Expand Up @@ -135,7 +137,7 @@ describe("Authentication Plugin", function()
local body = cjson.decode(response)
assert.are.equal(400, status)
assert.are.equal(1, utils.table_size(body))
assert.are.equal("http://google.com/kong?error=unsupported_response_type&state=somestate&error_description=Invalid%20response_type", body.redirect_uri)
assert.are.equal("http://google.com/kong?error=unsupported_response_type&error_description=Invalid%20response_type&state=somestate", body.redirect_uri)
end)

it("should return error when the redirect_uri does not match", function()
Expand All @@ -146,6 +148,23 @@ describe("Authentication Plugin", function()
assert.are.equal("http://google.com/kong?error=invalid_request&error_description=Invalid%20redirect_uri%20that%20does%20not%20match%20with%20the%20one%20created%20with%20the%20application", body.redirect_uri)
end)

it("should return error when the redirect_uri contains a fragment", function()
local response, status = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid456", scope = "email", response_type = "code" }, {host = "oauth2.com"})
local body = cjson.decode(response)
assert.are.equal(400, status)
assert.are.equal(2, utils.table_size(body))
assert.are.equal("invalid_request", body.error)
assert.are.equal("Fragment not allowed in redirect_uri", body.error_description)
end)

it("should work even if redirect_uri contains a query string", function()
local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid789", scope = "email", response_type = "code" }, {host = "oauth2_6.com", ["X-Forwarded-Proto"] = "https"})
local body = cjson.decode(response)
assert.are.equal(200, status)
assert.are.equal(1, utils.table_size(body))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?code=[\\w]{32,32}&foo=bar$"))
end)

it("should fail when not under HTTPS", function()
local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com"})
local body = cjson.decode(response)
Expand Down Expand Up @@ -266,28 +285,29 @@ describe("Authentication Plugin", function()
local body = cjson.decode(response)
assert.are.equal(200, status)
assert.are.equal(1, utils.table_size(body))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?token_type=bearer&access_token=[\\w]{32,32}$"))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&token_type=bearer$"))

-- Checking headers
assert.are.equal("no-store", headers["cache-control"])
assert.are.equal("no-cache", headers["pragma"])
end)

it("should return success and the state", function()
local response, status = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "token", state = "wot" }, {host = "oauth2.com"})
local body = cjson.decode(response)
assert.are.equal(200, status)
assert.are.equal(1, utils.table_size(body))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?token_type=bearer&state=wot&access_token=[\\w]{32,32}$"))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&state=wot&token_type=bearer$"))
end)

it("should return success and store authenticated user properties", function()
local response, status = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email profile", response_type = "token", authenticated_userid = "userid123" }, {host = "oauth2.com"})
local body = cjson.decode(response)
assert.are.equal(200, status)
assert.are.equal(1, utils.table_size(body))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?token_type=bearer&access_token=[\\w]{32,32}$"))
assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&token_type=bearer$"))

local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?token_type=bearer&access_token=([\\w]{32,32})$")
local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&token_type=bearer$")
local access_token
for line in matches do
access_token = line
Expand All @@ -308,7 +328,7 @@ describe("Authentication Plugin", function()
local response = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email profile", response_type = "token", authenticated_userid = "userid123" }, {host = "oauth2.com"})
local body = cjson.decode(response)

local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?token_type=bearer&access_token=([\\w]{32,32})$")
local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&token_type=bearer$")
local access_token
for line in matches do
access_token = line
Expand Down Expand Up @@ -823,4 +843,4 @@ describe("Authentication Plugin", function()

end)

end)
end)

0 comments on commit b4ce5c9

Please sign in to comment.