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

Password Rotation support - Add a CredentialProvider interface/class and set your own provider #1774

Closed
oridool opened this issue Jun 17, 2021 · 7 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@oridool
Copy link

oridool commented Jun 17, 2021

Hello,

Feature Request

Is your feature request related to a problem? Please describe

I'm using AWS SecretsManager to fetch my Redis credentials and provide them to Lettuce upon startup
However, in case I rotate the password (in Redis and in SecretsManager), I must restart my application.
Because, if the connection to Redis is lost, it will use the previous old password rather than the current one.

Describe the solution you'd like

Instead of providing a simple pair of username+password, I'd like to provide my own class, implementing a CredentialsProvider interface, and have Lettuce call it whenever a new connection is created.
This way, I will be able to always fetch the current password in my provider.
This interface just implements getUserName() / getPassword() .
Similar example for RabbitMQ:
https://github.com/rabbitmq/rabbitmq-java-client/blob/master/src/main/java/com/rabbitmq/client/impl/CredentialsProvider.java

Describe alternatives you've considered

I know Redis 6 with ACL can support multiple passwords.
However, the Lettuce client doesn't support that either as far as I know.
In addition, even if multiple passwords are supported by Lettuce - I will need to know ahead all the passwords for the near future.
This is not the most convenient and secured solution.

Thank you

@oridool oridool changed the title Password Rotation support - a CredentialProvider interface/class to be able to set your own provider Password Rotation support - Add a CredentialProvider interface/class to be able to set your own provider Jun 17, 2021
@oridool oridool changed the title Password Rotation support - Add a CredentialProvider interface/class to be able to set your own provider Password Rotation support - Add a CredentialProvider interface/class and set your own provider Jun 17, 2021
@mp911de
Copy link
Collaborator

mp911de commented Jun 17, 2021

Thanks for your feature request. Are you interested in submitting a pull request? While such a change can touch a lot of places, I think it makes absolutely sense to introduce a CredentialsProvider. I'm not sure we want refresh() methods in the actual interface. Representing username/password as Credentials object makes more sense in terms of a consistent username/password pair than individual calls to CredentialsProvider.getUsername/getPassword.

@mp911de mp911de added the type: enhancement A general enhancement label Jun 17, 2021
@oridool
Copy link
Author

oridool commented Jun 17, 2021

@mp911de , I think that refresh() is indeed not required. Only getUsername/getPassword should be enough.
Unfortunately, I'm not really familiar with the internals of Lettuce and I think such a change (as you wrote) can affect a lot of places.
Any chance someone else who is more familiar with Lettuce can take it?

@mp911de mp911de self-assigned this Jun 17, 2021
@oridool
Copy link
Author

oridool commented Jun 23, 2021

@mp911de , since you self-assigned this, does it mean the task is on progress?
If so, I'd implement a DefaultCredentialsProvider (implementing interface CredentialsProvider) that contains the regular username/password pair as today.

@mp911de
Copy link
Collaborator

mp911de commented Jun 23, 2021

since you self-assigned this, does it mean the task is on progress?

As per your request.

Any chance someone else who is more familiar with Lettuce can take it?

I haven't started yet and probably it will take another week or two until I get there.

Feel free to submit a pull request.

@mp911de mp911de assigned oridool and unassigned mp911de Jun 23, 2021
@oridool oridool removed their assignment Jul 17, 2021
@oridool
Copy link
Author

oridool commented Jul 17, 2021

Hi @mp911de ,
I don't seem to have the time and enough knowledge of Lettuce project to handle this at the moment.
Please take ownership back if possible.
Thanks.

@oridool
Copy link
Author

oridool commented Sep 29, 2021

@mp911de ,
Is this feature under development ?

@mp911de
Copy link
Collaborator

mp911de commented Sep 29, 2021

No, it's not. I don't have the bandwidth for features right now. Likely will have more time at the end of this year.

jiantosca added a commit to jiantosca/lettuce-core that referenced this issue Nov 21, 2021
@mp911de mp911de added this to the 6.2.0 milestone Mar 10, 2022
@mp911de mp911de linked a pull request Mar 16, 2022 that will close this issue
4 tasks
mp911de pushed a commit that referenced this issue Mar 16, 2022
mp911de added a commit that referenced this issue Mar 16, 2022
Refactor code to introduce RedisCredentialsProvider and RedisCredentials interfaces to provide a credentials provider abstraction.

Original pull request: #1916.
@mp911de mp911de closed this as completed Mar 16, 2022
mp911de added a commit that referenced this issue Aug 24, 2022
Document implication of the credential retrieval duration on connection creation timeouts.
mp911de added a commit that referenced this issue Aug 24, 2022
Document implication of the credential retrieval duration on connection creation timeouts.
mp911de added a commit that referenced this issue Aug 24, 2022
Document implication of the credential retrieval duration on connection creation timeouts.
mp911de added a commit that referenced this issue Aug 24, 2022
Document implication of the credential retrieval duration on connection creation timeouts.
sazzad16 added a commit to redis/jedis that referenced this issue Feb 14, 2023
References:

1. #1602 and related PRs. Current PR is probably better than handling in JedisFactory 
2. redis/redis-py#2261 - main reason of this PR 
3. redis/lettuce#1774 
4. #632 

---

* Introduce credentials provider

* use volatile

* Test in Sentineled mode

* Support CharSequence in DefaultRedisCredentials

* Added doc for prepare() and cleanUp()

* Test the provider interface

* Added example

* Removed deprecations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants