Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACL support is missing #446

Closed
tgupta-adc opened this issue Apr 19, 2022 · 8 comments · Fixed by #502
Closed

ACL support is missing #446

tgupta-adc opened this issue Apr 19, 2022 · 8 comments · Fixed by #502

Comments

@tgupta-adc
Copy link

Redis 6.2 provides way to add other users using aclfile. Will it be possible to provide support for that?

@root-expert
Copy link
Member

Hello @tgupta-adc, can you provide a PR adding this functionality?

@TwizzyDizzy
Copy link
Contributor

Do we have any indication of what the project wants this to look like?

Simply adding a few parameters (array of strings for the ACLs) and their respective parts in the template would be rather straight forward.

If one would want to verify the ACL's validities, that's a whole different story of course.

If you would be interested, I'd like to implement the following:

  • new parameter acls, data type: Array of Strings[1]
  • default user settings (on which requirepass and the likes act) will not be touched

If you have any questions, please let me know.

Cheers
Thomas

@tgupta-adc
Copy link
Author

sounds good to me. I am no longer using this module, it is not a requirement from my side now.

@linuxmail
Copy link

hi,

same for me, adding ACL for Redis and Sentinel would be good. I would may start with simple adding new params like:

  • auth-user
  • aclfile

I would manage aclfile by hand (template / file).

@TwizzyDizzy
Copy link
Contributor

Actually, right now, you can just manage an additional config file via the extra_config_file parameter - in there you can manage your ACLs (and maybe other settings this module does not yet manage), if you like.

Cheers
Thomas

@TwizzyDizzy
Copy link
Contributor

TwizzyDizzy commented Nov 15, 2023

Actually, right now, you can just manage an additional config file via the extra_config_file parameter - in there you can manage your ACLs (and maybe other settings this module does not yet manage), if you like.

One addition to that, unfortunately: redis/redis#8572 (comment)

This breaks my idea, as the config rewrite would then write the ACLs from the include-file to the main config (via CONFIG REWRITE) and will break on restart/config check, as there's duplicate user ACL declarations.

Cheers
Thomas

@RohanNagar
Copy link

Hello @TwizzyDizzy ! I see that you added support for ACLs to be defined - I was wondering if there are plans to support other ACL related configuration options, such as aclfile and acllog-max-len?

@TwizzyDizzy
Copy link
Contributor

TwizzyDizzy commented Mar 25, 2024

Hi @RohanNagar,

nope, it is neither planned to do that nor did the merge above implement it. Whether this is wanted for the module, I cannot say, yet I guess nobody would mind you taking a stab at it via PR. Obviously, those features would be mutually exclusive, as the doc states:

Instead of configuring users here in this file (author: the redis main config file is meant here), it is possible to use
a stand-alone file just listing users. The two methods cannot be mixed:
if you configure users here and at the same time you activate the external
ACL file, the server will refuse to start.

I'm not sure though how this relates to the information I linked to above. I would assume if aclfile was set, the CONFIG REWRITE would honour that file (despite it not working, if you have your ACLs in an include-file) - this would have to be tested though.

Cheers
Thomas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants