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

Java/acarbo valkey 493 password update #425

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions glide-core/redis-rs/redis/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ pub enum ErrorKind {

/// Not all slots are covered by the cluster
NotAllSlotsCovered,

/// Used when an error occurs on when user perform wrong usage of management operation.
/// E.g. not allowed configuration change.
UserOperationError,
}

#[derive(PartialEq, Debug)]
Expand Down Expand Up @@ -900,6 +904,7 @@ impl RedisError {
ErrorKind::RESP3NotSupported => "resp3 is not supported by server",
ErrorKind::ParseError => "parse error",
ErrorKind::NotAllSlotsCovered => "not all slots are covered",
ErrorKind::UserOperationError => "Wrong usage of management operation",
}
}

Expand Down Expand Up @@ -1095,6 +1100,7 @@ impl RedisError {
ErrorKind::NotAllSlotsCovered => RetryMethod::NoRetry,
ErrorKind::FatalReceiveError => RetryMethod::Reconnect,
ErrorKind::FatalSendError => RetryMethod::ReconnectAndRetry,
ErrorKind::UserOperationError => RetryMethod::NoRetry,
}
}
}
Expand Down
67 changes: 51 additions & 16 deletions glide-core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,28 +479,63 @@ impl Client {

/// Update the password used to authenticate with the servers.
/// If None is passed, the password will be removed.
/// If `re_auth` is true, the new password will be used to re-authenticate with all of the nodes.
/// If `immediate_auth` is true, the password will be used to authenticate with the servers immediately using the `AUTH` command.
/// The default behavior is to update the password without authenticating immediately.
/// If the password is empty or None, and `immediate_auth` is true, the password will be updated and an error will be returned.
pub async fn update_connection_password(
&mut self,
password: Option<String>,
re_auth: bool,
immediate_auth: bool,
) -> RedisResult<Value> {
if re_auth {
let routing = RoutingInfo::MultiNode((
MultipleNodeRoutingInfo::AllNodes,
Some(ResponsePolicy::AllSucceeded),
));
let mut cmd = redis::cmd("AUTH");
cmd.arg(&password);
self.send_command(&cmd, Some(routing)).await?;
let timeout = self.request_timeout;
// The password update operation is wrapped in a timeout to prevent it from blocking indefinitely.
// If the operation times out, an error is returned.
// Since the password update operation is not a command that go through the regular command pipeline,
// it is not have the regular timeout handling, as such we need to handle it separately.
match tokio::time::timeout(timeout, async {
match self.internal_client {
ClientWrapper::Standalone(ref mut client) => {
client.update_connection_password(password.clone()).await
}
ClientWrapper::Cluster { ref mut client } => {
client.update_connection_password(password.clone()).await
}
}
})
.await
{
Ok(result) => {
if immediate_auth {
self.send_immediate_auth(password).await
} else {
result
}
}
Err(_elapsed) => Err(RedisError::from((
ErrorKind::IoError,
"Password update operation timed out, please check the connection",
))),
}
}

match self.internal_client {
ClientWrapper::Standalone(ref mut client) => {
client.update_connection_password(password).await
}
ClientWrapper::Cluster { ref mut client } => {
client.update_connection_password(password).await
async fn send_immediate_auth(&mut self, password: Option<String>) -> RedisResult<Value> {
match &password {
Some(pw) if pw.is_empty() => Err(RedisError::from((
ErrorKind::UserOperationError,
"Empty password provided for authentication",
))),
None => Err(RedisError::from((
ErrorKind::UserOperationError,
"No password provided for authentication",
))),
Some(password) => {
let routing = RoutingInfo::MultiNode((
MultipleNodeRoutingInfo::AllNodes,
Some(ResponsePolicy::AllSucceeded),
));
let mut cmd = redis::cmd("AUTH");
cmd.arg(password);
self.send_command(&cmd, Some(routing)).await
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion glide-core/src/protobuf/command_request.proto
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ message ClusterScan {

message UpdateConnectionPassword {
optional string password = 1;
bool re_auth = 2;
bool immediate_auth = 2;
}

message CommandRequest {
Expand Down
2 changes: 1 addition & 1 deletion glide-core/src/socket_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ fn handle_request(request: CommandRequest, mut client: Client, writer: Rc<Writer
update_connection_password_command
.password
.map(|chars| chars.to_string()),
update_connection_password_command.re_auth,
update_connection_password_command.immediate_auth,
)
.await
.map_err(|err| err.into()),
Expand Down
26 changes: 18 additions & 8 deletions java/client/src/main/java/glide/api/BaseClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@
import glide.api.models.commands.LInsertOptions.InsertPosition;
import glide.api.models.commands.LPosOptions;
import glide.api.models.commands.ListDirection;
import glide.api.models.commands.PasswordUpdateMode;
import glide.api.models.commands.RangeOptions;
import glide.api.models.commands.RangeOptions.LexRange;
import glide.api.models.commands.RangeOptions.RangeQuery;
Expand Down Expand Up @@ -787,10 +786,15 @@ protected Map<String, Object> handleLcsIdxResponse(Map<String, Object> response)
* that the internal reconnection mechanism can handle reconnection seamlessly, preventing the
* loss of in-flight commands.
*
* @param immediateAuth A <code>boolean</code> flag. If <code>true</code>, the client will
* authenticate immediately with the new password against all connections, Using <code>AUTH
* </code> command. <br>
* If password supplied is an empty string, the client will not perform auth and a warning
* will be returned. <br>
* The default is `false`.
* @apiNote This method updates the client's internal password configuration and does not perform
* password rotation on the server side.
* @param password A new password to set.
* @param mode Password update mode, see {@link PasswordUpdateMode}.
* @return <code>"OK"</code>.
* @example
* <pre>{@code
Expand All @@ -799,9 +803,9 @@ protected Map<String, Object> handleLcsIdxResponse(Map<String, Object> response)
* }</pre>
*/
public CompletableFuture<String> updateConnectionPassword(
@NonNull String password, @NonNull PasswordUpdateMode mode) {
@NonNull String password, boolean immediateAuth) {
return commandManager.submitPasswordUpdate(
Optional.of(password), mode, this::handleStringResponse);
Optional.of(password), immediateAuth, this::handleStringResponse);
}

/**
Expand All @@ -815,16 +819,22 @@ public CompletableFuture<String> updateConnectionPassword(
*
* @apiNote This method updates the client's internal password configuration and does not perform
* password rotation on the server side.
* @param mode Password update mode, see {@link PasswordUpdateMode}.
* @param immediateAuth A <code>boolean</code> flag. If <code>true</code>, the client will
* authenticate immediately with the new password against all connections, Using <code>AUTH
* </code> command. <br>
* If password supplied is an empty string, the client will not perform auth and a warning
* will be returned. <br>
* The default is `false`.
* @return <code>"OK"</code>.
* @example
* <pre>{@code
* String response = client.resetConnectionPassword(RE_AUTHENTICATE).get();
* String response = client.resetConnectionPassword(true).get();
* assert response.equals("OK");
* }</pre>
*/
public CompletableFuture<String> updateConnectionPassword(@NonNull PasswordUpdateMode mode) {
return commandManager.submitPasswordUpdate(Optional.empty(), mode, this::handleStringResponse);
public CompletableFuture<String> updateConnectionPassword(boolean immediateAuth) {
return commandManager.submitPasswordUpdate(
Optional.empty(), immediateAuth, this::handleStringResponse);
}

@Override
Expand Down
7 changes: 3 additions & 4 deletions java/client/src/main/java/glide/managers/CommandManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import glide.api.models.GlideString;
import glide.api.models.Script;
import glide.api.models.Transaction;
import glide.api.models.commands.PasswordUpdateMode;
import glide.api.models.commands.scan.ClusterScanCursor;
import glide.api.models.commands.scan.ScanOptions;
import glide.api.models.configuration.RequestRoutingConfiguration.ByAddressRoute;
Expand Down Expand Up @@ -224,16 +223,16 @@ public <T> CompletableFuture<T> submitClusterScan(
* Submit a password update request to GLIDE core.
*
* @param password A new password to set or empty value to remove the password.
* @param mode Password update mode.
* @param immediateAuth immediately perform auth.
* @param responseHandler A response handler.
* @return A request promise.
* @param <T> Type of the response.
*/
public <T> CompletableFuture<T> submitPasswordUpdate(
Optional<String> password,
PasswordUpdateMode mode,
boolean immediateAuth,
GlideExceptionCheckedFunction<Response, T> responseHandler) {
var builder = UpdateConnectionPassword.newBuilder().setReAuth(mode.getValue());
var builder = UpdateConnectionPassword.newBuilder().setImmediateAuth(immediateAuth);
password.ifPresent(builder::setPassword);

var command = CommandRequest.newBuilder().setUpdateConnectionPassword(builder.build());
Expand Down
3 changes: 2 additions & 1 deletion java/integTest/src/test/java/glide/SharedClientTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@
import net.bytebuddy.utility.RandomString;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

// @Timeout(35) // seconds
@Timeout(35) // seconds
public class SharedClientTests {

private static GlideClient standaloneClient = null;
Expand Down
Loading
Loading