Skip to content

Commit

Permalink
feat(oauth2) optionally hash client secret
Browse files Browse the repository at this point in the history
### Summary

This was originally implemented with #4866 by @janza.

After some review comments, and the introduction of DAO transformations,
I decided to make changes, thus opening this new PR.

This PR adds a new `boolean` column `hash_secret` to `oauth2_credentials`
that is used to determine whether or not the `client_secret` will be hashed.

The PR adds support for `argon2`, and `bcrypt` and it uses `argon2` if the
system library for argon2 is installed. Otherwise it will fallback to
`bcrypt`. The plugin will also check if the `client_secret` needs to be
rehashed (on usage). One caveat. If you run this on cluster and some nodes
have `argon2` and some don't, it is possible that you cannot use the
credentials on those that don't. So keep your environments similar.

### Issues resolved

Close #4866, Fix #1237 (at least on OAuth2)
  • Loading branch information
bungle committed Mar 19, 2020
1 parent c111d4a commit a6d48d8
Show file tree
Hide file tree
Showing 10 changed files with 540 additions and 10 deletions.
2 changes: 2 additions & 0 deletions kong-2.0.2-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ build = {
["kong.plugins.oauth2.migrations"] = "kong/plugins/oauth2/migrations/init.lua",
["kong.plugins.oauth2.migrations.000_base_oauth2"] = "kong/plugins/oauth2/migrations/000_base_oauth2.lua",
["kong.plugins.oauth2.migrations.003_130_to_140"] = "kong/plugins/oauth2/migrations/003_130_to_140.lua",
["kong.plugins.oauth2.migrations.004_200_to_210"] = "kong/plugins/oauth2/migrations/004_200_to_210.lua",
["kong.plugins.oauth2.handler"] = "kong/plugins/oauth2/handler.lua",
["kong.plugins.oauth2.secret"] = "kong/plugins/oauth2/secret.lua",
["kong.plugins.oauth2.access"] = "kong/plugins/oauth2/access.lua",
["kong.plugins.oauth2.schema"] = "kong/plugins/oauth2/schema.lua",
["kong.plugins.oauth2.daos"] = "kong/plugins/oauth2/daos.lua",
Expand Down
40 changes: 31 additions & 9 deletions kong/plugins/oauth2/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ local url = require "socket.url"
local utils = require "kong.tools.utils"
local constants = require "kong.constants"
local timestamp = require "kong.tools.timestamp"
local secret = require "kong.plugins.oauth2.secret"


local kong = kong
Expand Down Expand Up @@ -431,17 +432,38 @@ local function issue_token(conf)
end
end

if client and client.client_secret ~= client_secret then
response_params = {
[ERROR] = "invalid_client",
error_description = "Invalid client authentication"
}
if client then
local authenticated
if client.hash_secret then
authenticated = secret.verify(client_secret, client.client_secret)
if authenticated and secret.needs_rehash(client.client_secret) then
local pk = kong.db.oauth2_credentials.schema:extract_pk_values(client)
local ok, err = kong.db.oauth2_credentials:update(pk, {
client_secret = client_secret,
hash_secret = true,
})

if not ok then
kong.log.warn(err)
end
end

if from_authorization_header then
invalid_client_properties = {
status = 401,
www_authenticate = "Basic realm=\"OAuth2.0\""
else
authenticated = client.client_secret == client_secret
end

if not authenticated then
response_params = {
[ERROR] = "invalid_client",
error_description = "Invalid client authentication"
}

if from_authorization_header then
invalid_client_properties = {
status = 401,
www_authenticate = "Basic realm=\"OAuth2.0\""
}
end
end
end

Expand Down
20 changes: 20 additions & 0 deletions kong/plugins/oauth2/daos.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
local url = require "socket.url"
local typedefs = require "kong.db.schema.typedefs"
local secret = require "kong.plugins.oauth2.secret"


local assert = assert


local function validate_uri(uri)
Expand Down Expand Up @@ -28,6 +32,7 @@ local oauth2_credentials = {
{ name = { type = "string", required = true }, },
{ client_id = { type = "string", required = false, unique = true, auto = true }, },
{ client_secret = { type = "string", required = false, auto = true }, },
{ hash_secret = { type = "boolean", required = true, default = false }, },
{ redirect_uris = {
type = "array",
required = false,
Expand All @@ -37,6 +42,21 @@ local oauth2_credentials = {
}, }, },
{ tags = typedefs.tags },
},
transformations = {
{
input = { "hash_secret" },
needs = { "client_secret" },
on_write = function(hash_secret, client_secret)
if not hash_secret then
return {}
end
local hash = assert(secret.hash(client_secret))
return {
client_secret = hash,
}
end,
},
},
}


Expand Down
18 changes: 18 additions & 0 deletions kong/plugins/oauth2/migrations/004_200_to_210.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
return {
postgres = {
up = [[
DO $$
BEGIN
ALTER TABLE IF EXISTS ONLY oauth2_credentials ADD hash_secret BOOLEAN;
EXCEPTION WHEN DUPLICATE_COLUMN THEN
-- Do nothing, accept existing state
END$$;
]],
},

cassandra = {
up = [[
ALTER TABLE oauth2_credentials ADD hash_secret boolean;
]],
},
}
1 change: 1 addition & 0 deletions kong/plugins/oauth2/migrations/init.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
return {
"000_base_oauth2",
"003_130_to_140",
"004_200_to_210",
}
Loading

0 comments on commit a6d48d8

Please sign in to comment.