feat(basic-auth) set generic X-Credential-Identifier (deprecating X-Credential-Username)

bungle · bungle · commit f221ce915ecd · 2020-02-04T17:45:14.000+02:00
### Summary The PR Kong#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 `Basic Auth Plugin`.
diff --git a/kong/plugins/basic-auth/access.lua b/kong/plugins/basic-auth/access.lua
@@ -66,6 +66,7 @@ local function retrieve_credentials(header_name, conf)
   return username, password
 end
 
+
 --- Validate a credential in the Authorization header against one fetched from the database.
 -- @param credential The retrieved credential from the username passed in the request
 -- @param given_password The password as given in the Authorization header
@@ -79,6 +80,7 @@ local function validate_credentials(credential, given_password)
   return credential.password == digest
 end
 
+
 local function load_credential_into_memory(username)
   local credential, err = kong.db.basicauth_credentials:select_by_username(username)
   if err then
@@ -87,6 +89,7 @@ local function load_credential_into_memory(username)
   return credential
 end
 
+
 local function load_credential_from_db(username)
   if not username then
     return
@@ -104,7 +107,10 @@ local function load_credential_from_db(username)
   return credential
 end
 
+
 local function set_consumer(consumer, credential)
+  kong.client.authenticate(consumer, credential)
+
   local set_header = kong.service.request.set_header
   local clear_header = kong.service.request.clear_header
 
@@ -126,23 +132,22 @@ local function set_consumer(consumer, credential)
     clear_header(constants.HEADERS.CONSUMER_USERNAME)
   end
 
-  kong.client.authenticate(consumer, credential)
+  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
 
   if credential then
-    if credential.username then
-      set_header(constants.HEADERS.CREDENTIAL_USERNAME, credential.username)
-    else
-      clear_header(constants.HEADERS.CREDENTIAL_USERNAME)
-    end
-
     clear_header(constants.HEADERS.ANONYMOUS)
-
   else
-    clear_header(constants.HEADERS.CREDENTIAL_USERNAME)
     set_header(constants.HEADERS.ANONYMOUS, true)
   end
 end
 
+
 local function do_authentication(conf)
   -- If both headers are missing, return 401
   if not (kong.request.get_header("authorization") or kong.request.get_header("proxy-authorization")) then
@@ -156,18 +161,18 @@ local function do_authentication(conf)
   end
 
   local credential
-  local given_username, given_password = retrieve_credentials("proxy-authorization", conf)
-  if given_username then
-    credential = load_credential_from_db(given_username)
+  local username, password = retrieve_credentials("proxy-authorization", conf)
+  if username then
+    credential = load_credential_from_db(username)
   end
 
   -- Try with the authorization header
   if not credential then
-    given_username, given_password = retrieve_credentials("authorization", conf)
-    credential = load_credential_from_db(given_username)
+    username, password = retrieve_credentials("authorization", conf)
+    credential = load_credential_from_db(username)
   end
 
-  if not credential or not validate_credentials(credential, given_password) then
+  if not credential or not validate_credentials(credential, password) then
     return false, { status = 401, message = "Invalid authentication credentials" }
   end
 
@@ -207,7 +212,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/basic-auth/handler.lua b/kong/plugins/basic-auth/handler.lua
@@ -2,16 +2,15 @@
 local access = require "kong.plugins.basic-auth.access"
 
 
-local BasicAuthHandler = {}
+local BasicAuthHandler = {
+  PRIORITY = 1001,
+  VERSION = "2.2.0",
+}
 
 
 function BasicAuthHandler:access(conf)
   access.execute(conf)
 end
 
 
-BasicAuthHandler.PRIORITY = 1001
-BasicAuthHandler.VERSION = "2.1.0"
-
-
 return BasicAuthHandler
diff --git a/spec/03-plugins/10-basic-auth/03-access_spec.lua b/spec/03-plugins/10-basic-auth/03-access_spec.lua
@@ -215,6 +215,8 @@ for _, strategy in helpers.each_strategy() do
         })
         local body = cjson.decode(assert.res_status(200, res))
         assert.equal('bob', body.headers["x-consumer-username"])
+        assert.equal('user123', body.headers["x-credential-identifier"])
+        assert.equal('user123', body.headers["x-credential-username"])
       end)
 
       it("authenticates with a password containing ':'", function()
@@ -228,6 +230,8 @@ for _, strategy in helpers.each_strategy() do
         })
         local body = cjson.decode(assert.res_status(200, res))
         assert.equal("bob", body.headers["x-consumer-username"])
+        assert.equal("user321", body.headers["x-credential-identifier"])
+        assert.equal('user321', body.headers["x-credential-username"])
       end)
 
       it("returns 401 for valid Base64 encoding", function()
@@ -273,6 +277,8 @@ for _, strategy in helpers.each_strategy() do
         local json = cjson.decode(body)
         assert.is_string(json.headers["x-consumer-id"])
         assert.equal("bob", json.headers["x-consumer-username"])
+        assert.equal("bob", json.headers["x-credential-identifier"])
+        assert.equal('bob', json.headers["x-credential-username"])
       end)
 
     end)
@@ -323,6 +329,8 @@ for _, strategy in helpers.each_strategy() do
         })
         local body = cjson.decode(assert.res_status(200, res))
         assert.equal('bob', body.headers["x-consumer-username"])
+        assert.equal('user123', body.headers["x-credential-identifier"])
+        assert.equal('user123', body.headers["x-credential-username"])
         assert.is_nil(body.headers["x-anonymous-consumer"])
       end)
 
@@ -337,6 +345,8 @@ for _, strategy in helpers.each_strategy() do
         local body = cjson.decode(assert.res_status(200, res))
         assert.equal('true', body.headers["x-anonymous-consumer"])
         assert.equal('no-body', body.headers["x-consumer-username"])
+        assert.equal(nil, body.headers["x-credential-identifier"])
+        assert.equal(nil, body.headers["x-credential-username"])
       end)
 
       it("errors when anonymous user doesn't exist", function()
diff --git a/spec/03-plugins/10-basic-auth/05-declarative_spec.lua b/spec/03-plugins/10-basic-auth/05-declarative_spec.lua
@@ -180,6 +180,7 @@ for _, strategy in helpers.each_strategy() do
           assert.equal(consumer_def.id, json.headers["x-consumer-id"])
           assert.equal(consumer_def.username, json.headers["x-consumer-username"])
           assert.equal(consumer_def.custom_id, json.headers["x-consumer-custom-id"])
+          assert.equal(basicauth_credential_def.username, json.headers["x-credential-identifier"])
           assert.equal(basicauth_credential_def.username, json.headers["x-credential-username"])
         end)
       end)
