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

Add support for external user identity / authentication management #1916

Closed
wants to merge 3 commits into from
Closed
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: 4 additions & 2 deletions src/main/java/io/lettuce/core/AbstractRedisClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,12 @@ protected <K, V, T extends RedisChannelHandler<K, V>> ConnectionFuture<T> initia
CompletableFuture<SocketAddress> socketAddressFuture = new CompletableFuture<>();
CompletableFuture<Channel> channelReadyFuture = new CompletableFuture<>();

String uriString = connectionBuilder.getRedisURI().toString();

EventRecorder.getInstance().record(
new ConnectionCreatedEvent(connectionBuilder.getRedisURI().toString(), connectionBuilder.endpoint().getId()));
new ConnectionCreatedEvent(uriString, connectionBuilder.endpoint().getId()));
EventRecorder.RecordableEvent event = EventRecorder.getInstance()
.start(new ConnectEvent(connectionBuilder.getRedisURI().toString(), connectionBuilder.endpoint().getId()));
.start(new ConnectEvent(uriString, connectionBuilder.endpoint().getId()));

channelReadyFuture.whenComplete((channel, throwable) -> {
event.record();
Expand Down
39 changes: 10 additions & 29 deletions src/main/java/io/lettuce/core/ConnectionState.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.lettuce.core;

import java.util.List;
import java.util.function.Supplier;

import io.lettuce.core.protocol.ProtocolVersion;

Expand All @@ -24,15 +25,14 @@
* and connection state restoration. This class is part of the internal API.
*
* @author Mark Paluch
* @author Jon Iantosca
* @since 6.0
*/
public class ConnectionState {

private volatile HandshakeResponse handshakeResponse;

private volatile String username;

private volatile char[] password;
private volatile Supplier<Credentials> credentialsSupplier;

private volatile int db;

Expand All @@ -48,8 +48,7 @@ public class ConnectionState {
public void apply(RedisURI redisURI) {

setClientName(redisURI.getClientName());
setUsername(redisURI.getUsername());
setPassword(redisURI.getPassword());
setCredentialsSupplier(redisURI.getCredentialsSupplier());
}

/**
Expand Down Expand Up @@ -113,36 +112,18 @@ protected void setUserNamePassword(List<char[]> args) {
}

if (args.size() > 1) {
setUsername(new String(args.get(0)));
setPassword(args.get(1));
this.credentialsSupplier = new DefaultCredentialsSupplier(new String(args.get(0)), args.get(1));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case which this.credentialsSupplier has a delegate which takes the passwd from another service: after sending an AUTH query to the server, we override the credentialsSupplier that has delegate with a new DefaultCredentialsSupplier which holds only the passwd at the time of the query.
In the case of a passwd rotation, the passwd at the time of the AUTH query will be expired.
I think there is supposed to be a check for existence credentialsSupplier and only if it does not exists, then to set it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's intended if the Redis connection is authenticated manually. In an arrangement where credentials are supplied externally, there's no need to call AUTH from a client application.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mp911de, we're looking into adding support for recurring authentication using ephemeral passwords.
This will mean that the client will need to repeatedly reauthenticate, and on each reauthentication attempt receive a fresh password from the credentials supplier.
Overwriting the credentials supplier will mean that this flow will break after the first reauthentication.
I can see 2 option to come over it.

The 1st:
To check if the this.credentialsSupplier is of type DefaultCredentialsSupplier, meaning there is no delegate by the customer.

if (!(credentialsSupplier instanceof DefaultCredentialsSupplier)) {
            return;
}

In order to be sure the customer didn't create a new CredentialsSupplier which inherits from DefaultCredentialsSupplier, we put DefaultCredentialsSupplier as final so no inheritance from it.

The 2nd:
To set this.credentialsSupplier using the DefaultCredentialsSupplier itself from the RedisUri , and then to check if it has delegate or not, if there is delegate, meaning the user/passwd is supplied externally and not supposed be to be overridden by any auth command.

Anyway, I think we should change this function name as it doesn't always change the username/passwd anymore and also not on all cases

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you add or remove a password on the Redis ACL, does that affect connected clients?

Generally as I understand the scenario, you want to call AUTH yourself so there doesn't seem to be need to have a credentials provider at all.

} else {
setUsername(null);
setPassword(args.get(0));
this.credentialsSupplier = new DefaultCredentialsSupplier(args.get(0));
}
}

protected void setUsername(String username) {
this.username = username;
}

String getUsername() {
return username;
}

protected void setPassword(char[] password) {
this.password = password;
}

char[] getPassword() {
return password;
}

boolean hasPassword() {
return this.password != null && this.password.length > 0;
Credentials getCredentials() {
return this.credentialsSupplier.get();
}

boolean hasUsername() {
return this.username != null && !this.username.isEmpty();
protected void setCredentialsSupplier(Supplier<Credentials> credentialsSupplier) {
this.credentialsSupplier = credentialsSupplier;
}

protected void setDb(int db) {
Expand Down
60 changes: 60 additions & 0 deletions src/main/java/io/lettuce/core/Credentials.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package io.lettuce.core;

import java.util.Arrays;
import java.util.Objects;

/**
* Value object representing credentials used to authenticate with Redis.
*
* @author Jon Iantosca
* @since 6.2
*/
public class Credentials {

private String username;

private char[] password;

public Credentials(char[] password) {
this(null, password);
}

public Credentials(String username, char[] password) {
this.username = username;
this.password = password;
}

public String getUsername() {
return username;
}

public boolean hasUsername() {
return username != null && !username.isEmpty();
}

public char[] getPassword() {
return password;
}

public boolean hasPassword() {
return password != null && password.length > 0;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof Credentials)) {
return false;
}
Credentials credentials = (Credentials) o;

return Objects.equals(username, credentials.username) && Arrays.equals(password, credentials.password);
}

@Override
public int hashCode() {
return Objects.hash(username) + Arrays.hashCode(password);
}
}
56 changes: 56 additions & 0 deletions src/main/java/io/lettuce/core/DefaultCredentialsSupplier.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package io.lettuce.core;

import java.util.function.Supplier;

/**
* Internal {@link Supplier<Credentials>} with support to delegate to a client supplied {@link Supplier<Credentials>}.
*
* @author Jon Iantosca
* @since 6.2
*/
class DefaultCredentialsSupplier implements Supplier<Credentials> {

private String username;

private char[] password;

private Supplier<Credentials> delegate;

DefaultCredentialsSupplier() {
}

DefaultCredentialsSupplier(char[] password) {
this(null, password);
}

DefaultCredentialsSupplier(String username, char[] password) {
this.username = username;
this.password = password;
}

@Override
public Credentials get() {
return hasDelegate() ? delegate.get() : new Credentials(this.username, this.password);
}

void setUsername(String username) {
this.username = username;
}

void setPassword(char[] password) {
this.password = password;
}

Supplier<Credentials> getCredentialsSupplier() {
return hasDelegate() ? delegate : this;
}

void setDelegate(Supplier<Credentials> delegate) {
this.delegate = delegate;
}

boolean hasDelegate() {
return delegate != null;
}

}
2 changes: 1 addition & 1 deletion src/main/java/io/lettuce/core/RedisClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ private <K, V> ConnectionFuture<StatefulRedisConnection<K, V>> connectStandalone
assertNotNull(codec);
checkValidRedisURI(redisURI);

logger.debug("Trying to get a Redis connection for: " + redisURI);
logger.debug("Trying to get a Redis connection for: {}", redisURI);

DefaultEndpoint endpoint = new DefaultEndpoint(getOptions(), getResources());
RedisChannelWriter writer = endpoint;
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/io/lettuce/core/RedisHandshake.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,12 @@ private CompletableFuture<Void> initializeResp3(Channel channel) {
*/
private CompletableFuture<?> initiateHandshakeResp2(Channel channel) {

if (connectionState.hasUsername()) {
return dispatch(channel, this.commandBuilder.auth(connectionState.getUsername(), connectionState.getPassword()));
} else if (connectionState.hasPassword()) {
return dispatch(channel, this.commandBuilder.auth(connectionState.getPassword()));
Credentials credentials = connectionState.getCredentials();

if (credentials.hasUsername()) {
return dispatch(channel, this.commandBuilder.auth(credentials.getUsername(), credentials.getPassword()));
} else if (credentials.hasPassword()) {
return dispatch(channel, this.commandBuilder.auth(credentials.getPassword()));
} else if (this.pingOnConnect) {
return dispatch(channel, this.commandBuilder.ping());
}
Expand All @@ -175,11 +177,13 @@ private CompletableFuture<?> initiateHandshakeResp2(Channel channel) {
*/
private AsyncCommand<String, String, Map<String, Object>> initiateHandshakeResp3(Channel channel) {

if (connectionState.hasPassword()) {
Credentials credentials = connectionState.getCredentials();

if (credentials.hasPassword()) {

return dispatch(channel, this.commandBuilder.hello(3,
LettuceStrings.isNotEmpty(connectionState.getUsername()) ? connectionState.getUsername() : "default",
connectionState.getPassword(), connectionState.getClientName()));
LettuceStrings.isNotEmpty(credentials.getUsername()) ? credentials.getUsername() : "default",
credentials.getPassword(), connectionState.getClientName()));
}

return dispatch(channel, this.commandBuilder.hello(3, null, null, connectionState.getClientName()));
Expand Down
Loading