adds sha1 support for basic auth plugin (issue #33)

[tyiss]

Note: this code is not ready yet, just wanted to ask some questions related to it.

The code add support for sha1 passowrds.
it adds a configuration key 'encryption_method' that have the following values:

• plain: plain passwords .
• sha1: sha1 encrypted passwords. password salted with consumer id.

questions:

1. in sha1 as mention in [basicauth] Encrypt password field #33, salt the sha1 with consumer id.
   i was wondering how it to test: is there a way the i could set consumer id, so that i could test the return sha1 (i could not find one) ?
2. tests: i have diffrent plugin configuration to test. plain passwords, sha1 passwords.
   how can i add and use different configuration for a plugin? should i find a way to remove plugin conf ?
3. what is the preferred way to get plugin configuration in access phase ?

[thibaultcha]

this is not possible, config is mandatory.

[thibaultcha]

i was wondering how it to test: is there a way the i could set consumer id, so that i could test the return sha1 (i could not find one) ?

You can retrieve a consumer id before creating an encrypted credential.

tests: i have diffrent plugin configuration to test. plain passwords, sha1 passwords.
how can i add and use different configuration for a plugin? should i find a way to remove plugin conf ?

Use different APIs and apply one plugin config on each (with insert_fixtures), or 1 API and 1 plugin and update it with the DAO.

what is the preferred way to get plugin configuration in access phase ?

In the access phase the plugin config is given as a parameter of the :access() function

[thibaultcha]

Where is the part for the 8000 port? Now that it is encrypted, consumers still set their password in plain in their requests, and it needs to be compared with the digest during the proxy part.

[thibaultcha]

I am just realising now but... This is retrieving any configuration? That won't do it. We need to update the core to introduce some kind of link between a plugin configuration and related entities (such as credentials) created by plugins.

[tyiss]

exactly, i was hitting that wall, (this code just retrive the first plugin conf) wanted someone that knows the internals to advice.
currently there is only one encryption method (plain).
i thought it will be more flexible to have encryption method along side others, so that, we can be smoothly backward compatible, and a consumer/api (not sure what entity) could have different encryption methods.

do you see how this link could be achieved ?

[tyiss]

that port 8000 code is indeed missing.

here is the patch (i don't update the PR cause there will be other changes- reflected by you're review).

 local constants = require "kong.constants"
+local basic_auth_utils = require "kong.plugins.basic-auth.utils"
 local AUTHORIZATION = "authorization"
 local PROXY_AUTHORIZATION = "proxy-authorization"
@@ -57,11 +58,14 @@ end
 -- @param {table} credential The retrieved credential from the username
passed in the request
 -- @param {string} username
 -- @param {string} password
+-- @param {table} conf
 -- @return {boolean} Success of authentication
-local function validate_credentials(credential, username, password)
+local function validate_credentials(credential, username, password, conf)
   if credential then
-    -- TODO: No encryption yet
-    return credential.password == password
+    local method = conf.encryption_method or "plain"
+    local transform_function = basic_auth_utils.encryption_methods[method]
or encryption_methods.plain
+    local consumer = {password = password, consumer_id =
credential.consumer_id}
+    return transform_function(consumer) == credential.password

[thibaultcha]

There is actually an issue with this. Since we want the encryption to happen at the DAO level, this will be included in the DAO's schema. However, when running unit tests, resty.string cannot be required since it is not a module installed by Luarocks, but only available inside of Openresty... It prevents from running unit tests against it but more important, it breaks the basicauth_credentials DAO in the unit tests... I am not sure what to do about it. We could either use another module for the encryption or try to mock it somehow.

[tyiss]

Correct, there is no way to unit test inside functionality, which include - Openresty modules.
imho, in addition to the current test suite, we need new test suite that could test inside modules. maybe we could use resty-cli for this.
for this particular case, i can include self-contained sha1 lib (or lua rock), so that i will not require resty module.

if this should be at the DAO level (how cassandra specific?), when we add support for other databases, any plugin will need to have its own sha1 (and upcoming encryption) implementations ?

[thibaultcha]

See #533

[thibaultcha]

Closing this in favour of #533 (which now needs review) thanks!