Skip to content

Commit

Permalink
fix(ldap-auth) remove bin.mod and negative cache
Browse files Browse the repository at this point in the history
- Remove negative caching as there is no way for invalidation
- Use `%` instead of `bin.mod`
  • Loading branch information
Shashi Ranjan committed Jul 13, 2018
1 parent 0005b2c commit cac30e1
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 5 deletions.
3 changes: 1 addition & 2 deletions kong/plugins/ldap-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ local function authenticate(conf, given_credentials)
end

local credential, err = singletons.cache:get(cache_key(conf, given_username), {
ttl = conf.cache_ttl,
neg_ttl = conf.cache_ttl,
ttl = conf.cache_ttl
}, load_credential, given_username, given_password, conf)
if err or credential == nil then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
Expand Down
7 changes: 4 additions & 3 deletions kong/plugins/ldap-auth/asn1.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ local setmetatable = setmetatable
local table = table
local string_reverse = string.reverse
local string_char = string.char
local pairs = pairs

local _M = {}

Expand Down Expand Up @@ -271,10 +272,10 @@ _M.ASN1Encoder = {

encode_oid_component = function(n)
local parts = {}
parts[1] = string_char(bit.mod(n, 128))
parts[1] = string_char(n % 128)
while n >= 128 do
n = bit.rshift(n, 7)
parts[#parts + 1] = string_char(bit.mod(n, 128) + 0x80)
parts[#parts + 1] = string_char(n % 128 + 0x80)
end
return string_reverse(table.concat(parts))
end,
Expand Down Expand Up @@ -319,7 +320,7 @@ _M.ASN1Encoder = {
local parts = {}

while len > 0 do
parts[#parts + 1] = string_char(bit.mod(len, 256))
parts[#parts + 1] = string_char(len % 256)
len = bit.rshift(len, 8)
end

Expand Down
2 changes: 2 additions & 0 deletions kong/plugins/ldap-auth/ldap.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ local function claculate_payload_length(encStr, pos, socket)
end

function _M.bind_request(socket, username, password)
local inspect = require("inspect")
print(inspect(username), ": ", password)
local encoder = asn1.ASN1Encoder:new()
local decoder = asn1.ASN1Decoder:new()

Expand Down
11 changes: 11 additions & 0 deletions spec/03-plugins/21-ldap-auth/01-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,17 @@ for _, strategy in helpers.each_strategy() do
})
assert.response(res).has.status(403)
end)
it("authorization fails with correct status with wrong very long password", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/request",
headers = {
host = "ldap.com",
authorization = "ldap " .. ngx.encode_base64("einstein:e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566e0d91f53c566")
}
})
assert.response(res).has.status(403)
end)
it("authorization fails if credential has multiple encoded usernames or passwords separated by ':' in get request", function()
local res = assert(proxy_client:send {
method = "GET",
Expand Down
122 changes: 122 additions & 0 deletions spec/03-plugins/21-ldap-auth/02-invalidations_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
local helpers = require "spec.helpers"
local cjson = require "cjson"
local fmt = string.format
local lower = string.lower
local md5 = ngx.md5

local ldap_host_aws = "ec2-54-172-82-117.compute-1.amazonaws.com"

for _, strategy in helpers.each_strategy() do
describe("Plugin: ldap-auth (invalidations) [#" .. strategy .. "]", function()
local proxy_client
local admin_client
local plugin

setup(function()
local bp
bp = helpers.get_db_utils(strategy)

local route = bp.routes:insert {
hosts = { "ldapauth.com" },
}

plugin = bp.plugins:insert {
route_id = route.id,
name = "ldap-auth",
config = {
ldap_host = ldap_host_aws,
ldap_port = "389",
start_tls = false,
base_dn = "ou=scientists,dc=ldap,dc=mashape,dc=com",
attribute = "uid"
}
}

assert(helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
}))

proxy_client = helpers.proxy_client()
admin_client = helpers.admin_client()
end)

teardown(function()
if proxy_client and admin_client then
proxy_client:close()
admin_client:close()
end

helpers.stop_kong(nil, true)
end)

local function cache_key(conf, username)
local ldap_config_cache = md5(fmt("%s:%u:%s:%s:%u",
lower(conf.ldap_host),
conf.ldap_port,
conf.base_dn,
conf.attribute,
conf.cache_ttl
))

return fmt("ldap_auth_cache:%s:%s", ldap_config_cache, username)
end

describe("authenticated LDAP user get cached", function()
it("should invalidate when Hmac Auth Credential entity is deleted", function()
-- It should work
local res = assert(proxy_client:send {
method = "GET",
path = "/requests",
body = {},
headers = {
["HOST"] = "ldapauth.com",
authorization = "ldap " .. ngx.encode_base64("einstein:password")
}
})
assert.res_status(200, res)

-- Check that cache is populated
local cache_key = cache_key(plugin.config, "einstein")
res = assert(admin_client:send {
method = "GET",
path = "/cache/" .. cache_key,
body = {},
})
assert.res_status(200, res)
end)
it("should not do negative cache", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/requests",
body = {},
headers = {
["HOST"] = "ldapauth.com",
authorization = "ldap " .. ngx.encode_base64("einstein:wrongpassword")
}
})
assert.res_status(403, res)

local cache_key = cache_key(plugin.config, "einstein")
res = assert(admin_client:send {
method = "GET",
path = "/cache/" .. cache_key,
body = {},
})
local body = assert.res_status(200, res)
assert.is_equal("password", cjson.decode(body).password)

local res = assert(proxy_client:send {
method = "GET",
path = "/requests",
body = {},
headers = {
["HOST"] = "ldapauth.com",
authorization = "ldap " .. ngx.encode_base64("einstein:password")
}
})
assert.res_status(200, res)
end)
end)
end)
end

0 comments on commit cac30e1

Please sign in to comment.