-
Notifications
You must be signed in to change notification settings - Fork 531
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
Redis AUTH support #748
base: dev
Are you sure you want to change the base?
Redis AUTH support #748
Conversation
This commit is based on orange-cloudfoundry@7aa41a4 from @axelfauvel in Netflix#576 and tries to close Netflix#46. Unfortunatelly the initial commit was already so old and the dynomite code base already evolved, that it was easier to not jump directly on this. Especically as there were some refactorings requested. Redis Datastore Authentification If Dynomite is configured to require a password via config option `requirepass` the following behaviour will be applied: 1. On Dynomite startup, the server authenticates with the backend itself by calling the datastore agnostic function g_datastore_auth. 2. The corresponding Redis response will be handeled in g_is_authenticated. Dynomite will exit if authentification to the datatstore was not successful. 3. Each newly created client connection will require authentification. 4. Clients can authentificate itself by issue the AUTH command against dynomite. 5. Dynomite will check the password and simulate an AUTH response. 6. If AUTH was successful, the auth_required flag on the connection is reset and the client can process further commands through this connection.
Do you think you can merge this PR ? Many thanks :) |
any news about this pr ? :) |
hello,any news about this merging ? |
@smukil could you merge this ? it actually looks like author take all your remarks from original PR and do the changes and you needed only those changes before merging on original PR. Not supporting auth is a real show blocker for many company, thanks for the time took on merging it |
I've proven this PR against my fork of dev and the basics appear fine. It correctly requires the auth password & fails when given the wrong one, or none. I'd love to see this PR get updated & re-reviewed for inclusion. |
+1 |
I tried applied your PR (682daa3) to v0.8, the code builds but has runtime error on AUTH command:
|
You can see the original code that PR was prepared here: https://github.com/reimannf/dynomite/tree/redis-auth Nevertheless, it seems that this PR is not wanted by the repo owners and I will not dump more time into it, feel free to pick it up. |
@reimannf It turns out your change works with |
This superseeds #576 as I cannot push to the original feature branch.
@axelfauvel if you are not ok with this, can you please merge the current state of dynomite in your branch, so I could pick from there?
@smukil I think I have all your remarks covered in the origanl PR. Hopefully we can take it from here.
This commit is based on
orange-cloudfoundry@7aa41a4
from @axelfauvel in #576 and tries to
close #46.
Unfortunatelly the initial commit was already so old and the dynomite code base already evolved,
that it was easier to not jump directly on this. Especically as there were some refactorings
requested.
Redis Datastore Authentification
If Dynomite is configured to require a password via config option
requirepass
the followingbehaviour will be applied:
by calling the datastore agnostic function g_datastore_auth.
Dynomite will exit if authentification to the datatstore was not successful.
the client can process further commands through this connection.