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

Fix/jwt errorhandling #1362

Merged
merged 6 commits into from
Jul 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kong/plugins/jwt/api.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ return {
crud.paginated_set(self, dao_factory.jwt_secrets)
end,

PUT = function(self, dao_factory)
PUT = function(self, dao_factory, helpers)
crud.put(self.params, dao_factory.jwt_secrets)
end,

POST = function(self, dao_factory)
POST = function(self, dao_factory, helpers)
crud.post(self.params, dao_factory.jwt_secrets)
end
},
Expand Down
11 changes: 11 additions & 0 deletions kong/plugins/jwt/daos.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
local utils = require "kong.tools.utils"
local Errors = require "kong.dao.errors"
local crypto = require "crypto"

local SCHEMA = {
primary_key = {"id"},
Expand All @@ -12,6 +14,15 @@ local SCHEMA = {
rsa_public_key = {type = "string"},
algorithm = {type = "string", enum = {"HS256", "RS256"}, default = 'HS256'}
},
self_check = function(schema, plugin_t, dao, is_update)
if plugin_t.algorithm == "RS256" and plugin_t.rsa_public_key == nil then
return false, Errors.schema "no mandatory 'rsa_public_key'"
end
if plugin_t.algorithm == "RS256" and crypto.pkey.from_pem(plugin_t.rsa_public_key) == nil then
return false, Errors.schema "'rsa_public_key' format is invalid"
end
return true
end,
marshall_event = function(self, t)
return {id = t.id, consumer_id = t.consumer_id, key = t.key}
end
Expand Down
21 changes: 16 additions & 5 deletions kong/plugins/jwt/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ JwtHandler.PRIORITY = 1000
-- Checks for the JWT in URI parameters, then in the `Authorization` header.
-- @param request ngx request object
-- @param conf Plugin configuration
-- @return token JWT token contained in request or nil
-- @return token JWT token contained in request (can be a table) or nil
-- @return err
local function retrieve_token(request, conf)
local uri_parameters = request.get_uri_args()
Expand Down Expand Up @@ -56,14 +56,21 @@ function JwtHandler:access(conf)
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

if not token then
return responses.send_HTTP_UNAUTHORIZED()
local ttype = type(token)
if ttype ~= "string" then
if ttype == "nil" then
return responses.send_HTTP_UNAUTHORIZED()
elseif ttype == "table" then
return responses.send_HTTP_UNAUTHORIZED("Multiple tokens provided")
else
return responses.send_HTTP_UNAUTHORIZED("Unrecognizable token")
end
end

-- Decode token to find out who the consumer is
local jwt, err = jwt_decoder:new(token)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR()
return responses.send_HTTP_UNAUTHORIZED("Bad token; "..tostring(err))
end

local claims = jwt.claims
Expand Down Expand Up @@ -99,6 +106,10 @@ function JwtHandler:access(conf)
jwt_secret_value = jwt:b64_decode(jwt_secret_value)
end

if not jwt_secret_value then
return responses.send_HTTP_FORBIDDEN("Invalid key/secret")
end

-- Now verify the JWT signature
if not jwt:verify_signature(jwt_secret_value) then
return responses.send_HTTP_FORBIDDEN("Invalid signature")
Expand Down
19 changes: 14 additions & 5 deletions kong/plugins/jwt/jwt_parser.lua
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ local alg_verify = {
--["HS384"] = function(data, signature, key) return signature == alg_sign["HS384"](data, key) end,
--["HS512"] = function(data, signature, key) return signature == alg_sign["HS512"](data, key) end
["RS256"] = function(data, signature, key)
return crypto.verify('sha256', data, signature, crypto.pkey.from_pem(key))
local pkey = assert(crypto.pkey.from_pem(key),"Consumer Public Key is Invalid")
return crypto.verify('sha256', data, signature, pkey)
end
}

Expand Down Expand Up @@ -96,17 +97,25 @@ local function decode_token(token)
b64_decode(signature_64)
end)
if not ok then
return nil, "Invalid JSON"
return nil, "invalid JSON"
end

if header.typ and header.typ:upper() ~= "JWT" then
return nil, "Invalid typ"
return nil, "invalid typ"
end

if not header.alg or type(header.alg) ~= "string" or not alg_verify[header.alg] then
return nil, "Invalid alg"
return nil, "invalid alg"
end

if not claims then
return nil, "invalid claims"
end

if not signature then
return nil, "invalid signature"
end

return {
token = token,
header_64 = header_64,
Expand Down Expand Up @@ -158,7 +167,7 @@ _M.__index = _M
-- @return JWT parser
-- @return error if any
function _M:new(token)
if type(token) ~= "string" then error("JWT must be a string", 2) end
if type(token) ~= "string" then error("Token must be a string, got "..tostring(token), 2) end

local token, err = decode_token(token)
if err then
Expand Down
6 changes: 3 additions & 3 deletions spec/03-plugins/jwt/01-jwt_parser_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ describe("Plugin: jwt (parser)", function()
it("throws an error if not given a string", function()
assert.has_error(function()
jwt_parser:new()
end, "JWT must be a string")
end, "Token must be a string, got nil")
end)
it("refuses invalid typ", function()
local token = jwt_parser.encode({sub = "1234"}, "secret", nil, {typ = "foo"})
local _, err = jwt_parser:new(token)
assert.equal("Invalid typ", err)
assert.equal("invalid typ", err)
end)
it("refuses invalid alg", function()
local token = jwt_parser.encode({sub = "1234"}, "secret", nil, {
typ = "JWT",
alg = "foo"
})
local _, err = jwt_parser:new(token)
assert.equal("Invalid alg", err)
assert.equal("invalid alg", err)
end)
it("accepts a valid encoding request", function()
local token = jwt_parser.encode({sub = "1234"}, "secret", nil, {
Expand Down
98 changes: 96 additions & 2 deletions spec/03-plugins/jwt/02-api_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ local helpers = require "spec.helpers"
local cjson = require "cjson"

local jwt_secrets = helpers.dao.jwt_secrets
local fixtures = require "spec.03-plugins.jwt.fixtures"

describe("Plugin: jwt (API)", function()
local admin_client, consumer, jwt_secret
setup(function()
helpers.kill_all()
helpers.prepare_prefix()
assert(helpers.start_kong())
admin_client = helpers.admin_client()
admin_client = assert(helpers.admin_client())
end)
teardown(function()
if admin_client then admin_client:close() end
Expand Down Expand Up @@ -62,6 +63,99 @@ describe("Plugin: jwt (API)", function()
assert.equal("tooshort", body.secret)
jwt2 = body
end)
it("accepts a valid public key for RS256 when posted urlencoded", function()
local rsa_public_key = fixtures.rs256_public_key
rsa_public_key = rsa_public_key:gsub("\n", "\r\n")
rsa_public_key = rsa_public_key:gsub("([^%w %-%_%.%~])",
function(c) return string.format ("%%%02X", string.byte(c)) end)
rsa_public_key = rsa_public_key:gsub(" ", "+")

local res = assert(admin_client:send {
method = "POST",
path = "/consumers/bob/jwt/",
body = {
key = "bob3",
algorithm = "RS256",
rsa_public_key = rsa_public_key
},
headers = {
["Content-Type"] = "application/x-www-form-urlencoded"
}
})
assert.response(res).has.status(201)
local json = assert.response(res).has.jsonbody()
assert.equal("bob3", json.key)
end)
it("accepts a valid public key for RS256 when posted as json", function()
local rsa_public_key = fixtures.rs256_public_key

local res = assert(admin_client:send {
method = "POST",
path = "/consumers/bob/jwt/",
body = {
key = "bob4",
algorithm = "RS256",
rsa_public_key = rsa_public_key
},
headers = {
["Content-Type"] = "application/json"
}
})
assert.response(res).has.status(201)
local json = assert.response(res).has.jsonbody()
assert.equal("bob4", json.key)
end)
it("fails with missing `rsa_public_key` parameter for RS256 algorithms", function ()
local res = assert(admin_client:send {
method = "POST",
path = "/consumers/bob/jwt/",
body = {
key = "bob5",
algorithm = "RS256"
},
headers = {
["Content-Type"] = "application/json"
}
})
assert.response(res).has.status(400)
local json = assert.response(res).has.jsonbody()
assert.equal("no mandatory 'rsa_public_key'", json.message)
end)
it("fails with an invalid rsa_public_key for RS256 algorithms", function ()
local res = assert(admin_client:send {
method = "POST",
path = "/consumers/bob/jwt/",
body = {
key = "bob5",
algorithm = "RS256",
rsa_public_key = "test",
},
headers = {
["Content-Type"] = "application/json"
}
})
assert.response(res).has.status(400)
local json = assert.response(res).has.jsonbody()
assert.equal("'rsa_public_key' format is invalid", json.message)
end)
it("does not fail when `secret` parameter for HS256 algorithms is missing", function ()
local res = assert(admin_client:send {
method = "POST",
path = "/consumers/bob/jwt/",
body = {
key = "bob5",
algorithm = "HS256",
},
headers = {
["Content-Type"] = "application/json"
}
})
assert.response(res).has.status(201)
local json = assert.response(res).has.jsonbody()
assert.string(json.secret)
assert.equals(32, #json.secret)
assert.matches("^%x+$", json.secret)
end)
end)

describe("PUT", function()
Expand Down Expand Up @@ -89,7 +183,7 @@ describe("Plugin: jwt (API)", function()
path = "/consumers/bob/jwt/",
})
local body = cjson.decode(assert.res_status(200, res))
assert.equal(1, #(body.data))
assert.equal(4, #(body.data))
end)
end)
end)
Expand Down