From 6663f08470f52f04ae4ecee0350671d8cd20d579 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Fri, 31 Jan 2020 15:30:08 +0200 Subject: [PATCH] feat(ldap-auth) set generic X-Credential-Identifier (deprecating X-Credential-Username) ### Summary The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided at time that we should add support for this less opinionated field name on other auth plugins too. This commit adds it to `LDAP Auth Plugin`. --- kong/plugins/ldap-auth/access.lua | 49 +++++++++---------- kong/plugins/ldap-auth/handler.lua | 9 ++-- .../20-ldap-auth/01-access_spec.lua | 21 +++++++- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/kong/plugins/ldap-auth/access.lua b/kong/plugins/ldap-auth/access.lua index 77b84fe3235..991978e2f97 100644 --- a/kong/plugins/ldap-auth/access.lua +++ b/kong/plugins/ldap-auth/access.lua @@ -165,42 +165,37 @@ local function set_consumer(consumer, credential) local set_header = kong.service.request.set_header local clear_header = kong.service.request.clear_header - if consumer then - -- this can only be the Anonymous user in this case - if consumer.id then - set_header(constants.HEADERS.CONSUMER_ID, consumer.id) - else - clear_header(constants.HEADERS.CONSUMER_ID) - end - - if consumer.custom_id then - set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id) - else - clear_header(constants.HEADERS.CONSUMER_CUSTOM_ID) - end - - if consumer.username then - set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username) - else - clear_header(constants.HEADERS.CONSUMER_USERNAME) - end + if consumer and consumer.id then + set_header(constants.HEADERS.CONSUMER_ID, consumer.id) + else + clear_header(constants.HEADERS.CONSUMER_ID) + end - set_header(constants.HEADERS.ANONYMOUS, true) + if consumer and consumer.custom_id then + set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id) + else + clear_header(constants.HEADERS.CONSUMER_CUSTOM_ID) + end - return + if consumer and consumer.username then + set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username) + else + clear_header(constants.HEADERS.CONSUMER_USERNAME) end if credential and credential.username then + set_header(constants.HEADERS.CREDENTIAL_IDENTIFIER, credential.username) set_header(constants.HEADERS.CREDENTIAL_USERNAME, credential.username) else + clear_header(constants.HEADERS.CREDENTIAL_IDENTIFIER) clear_header(constants.HEADERS.CREDENTIAL_USERNAME) end - -- in case of auth plugins concatenation, remove remnants of anonymous - clear_header(constants.HEADERS.ANONYMOUS) - clear_header(constants.HEADERS.CONSUMER_ID) - clear_header(constants.HEADERS.CONSUMER_CUSTOM_ID) - clear_header(constants.HEADERS.CONSUMER_USERNAME) + if credential then + clear_header(constants.HEADERS.ANONYMOUS) + else + set_header(constants.HEADERS.ANONYMOUS, true) + end end @@ -264,7 +259,7 @@ function _M.execute(conf) return kong.response.exit(500, { message = "An unexpected error occurred" }) end - set_consumer(consumer, nil) + set_consumer(consumer) else return kong.response.exit(err.status, { message = err.message }, err.headers) diff --git a/kong/plugins/ldap-auth/handler.lua b/kong/plugins/ldap-auth/handler.lua index e7ee55de650..100de66b6e5 100644 --- a/kong/plugins/ldap-auth/handler.lua +++ b/kong/plugins/ldap-auth/handler.lua @@ -1,7 +1,10 @@ local access = require "kong.plugins.ldap-auth.access" -local LdapAuthHandler = {} +local LdapAuthHandler = { + PRIORITY = 1002, + VERSION = "2.2.0", +} function LdapAuthHandler:access(conf) @@ -9,8 +12,4 @@ function LdapAuthHandler:access(conf) end -LdapAuthHandler.PRIORITY = 1002 -LdapAuthHandler.VERSION = "2.1.0" - - return LdapAuthHandler diff --git a/spec/03-plugins/20-ldap-auth/01-access_spec.lua b/spec/03-plugins/20-ldap-auth/01-access_spec.lua index 53ccc411626..4c25047d22e 100644 --- a/spec/03-plugins/20-ldap-auth/01-access_spec.lua +++ b/spec/03-plugins/20-ldap-auth/01-access_spec.lua @@ -287,6 +287,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do } }) assert.response(res).has.status(200) + local value = assert.request(res).has.header("x-credential-identifier") + assert.are.equal("einstein", value) local value = assert.request(res).has.header("x-credential-username") assert.are.equal("einstein", value) assert.request(res).has_not.header("x-anonymous-username") @@ -411,6 +413,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do } }) assert.response(res).has.status(200) + local value = assert.request(res).has.header("x-credential-identifier") + assert.are.equal("einstein", value) local value = assert.request(res).has.header("x-credential-username") assert.are.equal("einstein", value) assert.request(res).has_not.header("x-anonymous-username") @@ -464,6 +468,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do }) assert.response(res).has.status(200) + local value = assert.request(res).has.header("x-credential-identifier") + assert.are.equal("einstein", value) local value = assert.request(res).has.header("x-credential-username") assert.are.equal("einstein", value) assert.request(res).has_not.header("x-anonymous-username") @@ -481,6 +487,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do assert.are.equal("true", value) value = assert.request(res).has.header("x-consumer-username") assert.equal('no-body', value) + assert.request(res).has.no.header("x-credential-identifier") + assert.request(res).has.no.header("x-credential-username") end) it("errors when anonymous user doesn't exist", function() local res = assert(proxy_client:send { @@ -499,6 +507,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do local proxy_client local user local anonymous + local keyauth lazy_setup(function() local bp = helpers.get_db_utils(strategy, { @@ -573,7 +582,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do }, } - bp.keyauth_credentials:insert { + keyauth = bp.keyauth_credentials:insert { key = "Mouse", consumer = { id = user.id }, } @@ -665,6 +674,9 @@ for _, ldap_strategy in pairs(ldap_strategies) do local id = assert.request(res).has.header("x-consumer-id") assert.not_equal(id, anonymous.id) assert(id == user.id) + local value = assert.request(res).has.header("x-credential-identifier") + assert.equal(keyauth.id, value) + assert.request(res).has.no.header("x-credential-username") end) it("passes with only the first credential provided", function() @@ -681,6 +693,9 @@ for _, ldap_strategy in pairs(ldap_strategies) do local id = assert.request(res).has.header("x-consumer-id") assert.not_equal(id, anonymous.id) assert.equal(user.id, id) + local value = assert.request(res).has.header("x-credential-identifier") + assert.equal(keyauth.id, value) + assert.request(res).has.no.header("x-credential-username") end) it("passes with only the second credential provided", function() @@ -694,6 +709,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do }) assert.response(res).has.status(200) assert.request(res).has.no.header("x-anonymous-consumer") + local id = assert.request(res).has.header("x-credential-identifier") + assert.equal("einstein", id) local id = assert.request(res).has.header("x-credential-username") assert.equal("einstein", id) end) @@ -710,6 +727,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do assert.request(res).has.header("x-anonymous-consumer") local id = assert.request(res).has.header("x-consumer-id") assert.equal(id, anonymous.id) + assert.request(res).has.no.header("x-credential-identifier") + assert.request(res).has.no.header("x-credential-username") end) end) end)