feat(redis) add support for username/password auth #121
Conversation
lua-resty-redis supports username/password authentication:
```
local res, err = red:auth("userexample", "passexample")
if not res then
ngx.say("failed to authenticate: ", err)
return
end
```
lib/resty/acme/storage/redis.lua
Outdated
| local _, err = client:auth(self.auth) | ||
| local _, err | ||
| if type(self.auth) == "table" then | ||
| _, err = client:auth(self.auth.username, self.auth.password) |
There was a problem hiding this comment.
I would actually suggest new fields being added (username, password) in addition to auth, otherwise we may need to write complex compat code on kong side.
There was a problem hiding this comment.
I think from style perspective I'd rather prefer to have separate fields as well. However I'm wondering - will it really require compat code on kong side? Even the way it's implemented here? 🤔 CP/DP will run different versions of this lib so I think we're good, right?
There was a problem hiding this comment.
we do, unfortunately, because we need to define the schema at kong side (https://github.com/Kong/kong/blob/master/kong/plugins/acme/schema.lua#L85).
actually, i'm thinking to implement a new redis storage, using the kong provided redis library + schema in the kong acme plugin, that skipped the redis storage provided by this library.
basically we only need to add a kong/plugins/acme/storage/redis.lua then do something similar as this line https://github.com/Kong/kong/blob/master/kong/plugins/acme/client.lua#L91.
then we can from now on have aligned redis schema in the acme plugin with other plugins too.
There was a problem hiding this comment.
@fffonion Please correct me if I'm wrong but I think with the introduction of config adapter: https://github.com/Kong/kong/blob/master/kong/plugins/acme/storage/config_adapters/redis.lua
There's no need to write any compat code since plugin's schema is separated from this library's config.
|
added you as collaborator so don't need to approve the CI run everytime @gruceo you might need to click some button to accept it |
|
FYI there are some flaky tests unrelated to the PR: Restarted the build, and the error went away. Full build with logs: https://github.com/fffonion/lua-resty-acme/actions/runs/10373050111/attempts/1?pr=121 Please squash and merge: |
|
i fixed a typo in code and seems tests need to be adjusted, could you take a look @gruceo ? |
|
done, thanks @fffonion failure seems unrelated now |
|
Had to restart twice to get the tests to pass. |
|
Thank you @gruceo ! |
|
released |
Fixed an issue where username and password were not accepted as valid authentication methods. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Fixed an issue where username and password were not accepted as valid authentication methods. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Fixed an issue where username and password were not accepted as valid authentication methods. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
### Summary ### [0.15.0] - 2024-08-14 #### bug fixes - **tests:** use tlsv1.2 in dual cert test [415be3f](fffonion/lua-resty-acme@415be3f) - **tests:** uses v3 protocol for etcd [c3928b5](fffonion/lua-resty-acme@c3928b5) #### features - **etcd:** etcd storage to use v3 protocol [a3353b3](fffonion/lua-resty-acme@a3353b3) - **redis:** add support for username/password auth ([#121](fffonion/lua-resty-acme#121)) [186ab23](fffonion/lua-resty-acme@186ab23) KAG-5189 Signed-off-by: Aapo Talvensaari <[email protected]>
### Summary ### [0.15.0] - 2024-08-14 #### bug fixes - **tests:** use tlsv1.2 in dual cert test [415be3f](fffonion/lua-resty-acme@415be3f) - **tests:** uses v3 protocol for etcd [c3928b5](fffonion/lua-resty-acme@c3928b5) #### features - **etcd:** etcd storage to use v3 protocol [a3353b3](fffonion/lua-resty-acme@a3353b3) - **redis:** add support for username/password auth ([#121](fffonion/lua-resty-acme#121)) [186ab23](fffonion/lua-resty-acme@186ab23) KAG-5189 Signed-off-by: Aapo Talvensaari <[email protected]>
Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
…#13496) * fix(plugins/acme): username/password is a valid authentication method Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
lua-resty-redis supports username/password authentication:
Squash and merge: