DATAREDIS-1060 - Redis password should not automatically be applied to Sentinel.

We now expose two separate properties in RedisSentinelConfiguration to configure the data password individually from the sentinel password.

Original pull request: #495.

src/main/asciidoc/reference/redis.adoc

@@ -174,13 +174,14 @@ public RedisConnectionFactory lettuceConnectionFactory() {
 .Configuration Properties
 * `spring.redis.sentinel.master`: name of the master node.
 * `spring.redis.sentinel.nodes`: Comma delimited list of host:port pairs.
+* `spring.redis.sentinel.password`: The password to apply when authenticating with Redis Sentinel
 ====
 
 Sometimes, direct interaction with one of the Sentinels is required. Using `RedisConnectionFactory.getSentinelConnection()` or `RedisConnection.getSentinelCommands()` gives you access to the first active Sentinel configured.
 
 [NOTE]
 ====
-Configuration options like password, timeouts, SSL... also get applied to Redis Sentinel when using https://lettuce.io/[Lettuce].
+Sentinel authentication is only available using https://lettuce.io/[Lettuce].
 ====
 
 [[redis:template]]

src/main/java/org/springframework/data/redis/connection/RedisConfiguration.java

@@ -337,6 +337,79 @@ public interface RedisConfiguration {
 		 * @return {@link Set} of sentinels. Never {@literal null}.
 		 */
 		Set<RedisNode> getSentinels();
+
+		/**
+		 * Get the {@link RedisPassword} used when authenticating with a Redis Server.
+		 * 
+		 * @return never {@literal null}.
+		 */
+		default RedisPassword getDataNodePassword() {
+			return getPassword();
+		}
+
+		/**
+		 * Create and set a {@link RedisPassword} to be used when authenticating with Sentinel from the given {@link String}
+		 *
+		 * @param password can be {@literal null}.
+		 * @throws IllegalStateException if the {@link #useDataNodeAuthenticationForSentinel(boolean) Data Node Password}
+		 *           should be used for authenticating with Redis Sentinel.
+		 * @since 2.2.2
+		 */
+		default void setSentinelPassword(@Nullable String password) {
+			setSentinelPassword(RedisPassword.of(password));
+		}
+
+		/**
+		 * Create and set a {@link RedisPassword} to be used when authenticating with Sentinel from the given
+		 * {@link Character} sequence.
+		 *
+		 * @param password can be {@literal null}.
+		 * @throws IllegalStateException if the {@link #useDataNodeAuthenticationForSentinel(boolean) Data Node Password}
+		 *           should be used for authenticating with Redis Sentinel.
+		 * @since 2.2.2
+		 */
+		default void setSentinelPassword(@Nullable char[] password) {
+			setSentinelPassword(RedisPassword.of(password));
+		}
+
+		/**
+		 * Set a {@link RedisPassword} to be used when authenticating with Sentinel.
+		 *
+		 * @param password must not be {@literal null} use {@link RedisPassword#none()} instead.
+		 * @throws IllegalStateException if the {@link #useDataNodeAuthenticationForSentinel(boolean) Data Node Password}
+		 *           should be used for authenticating with Redis Sentinel.
+		 * @since 2.2.2
+		 */
+		void setSentinelPassword(RedisPassword password);
+
+		/**
+		 * Get the {@link RedisPassword} to use when connecting to a Redis Sentinel. <br />
+		 * This can be the password explicitly set via {@link #setSentinelPassword(RedisPassword)}, or the
+		 * {@link #getDataNodePassword() Data Node password} if it should be also used for
+		 * {@link #getUseDataNodeAuthenticationForSentinel() sentinel}, or {@link RedisPassword#none()} if no password has
+		 * been set.
+		 *
+		 * @return the {@link RedisPassword} for authenticating with Sentinel.
+		 * @since 2.2.2
+		 */
+		RedisPassword getSentinelPassword();
+
+		/**
+		 * Use the {@link #getDataNodePassword() RedisPassword} also for authentication with Redis Sentinel.
+		 * 
+		 * @param useDataNodeAuthenticationForSentinel
+		 * @throws IllegalStateException if a {@link #getSentinelPassword() Sentinel Password} is already set.
+		 * @since 2.2.2
+		 */
+		void useDataNodeAuthenticationForSentinel(boolean useDataNodeAuthenticationForSentinel);
+
+		/**
+		 * Use the {@link #getDataNodePassword() RedisPassword} also for authentication with Redis Sentinel.
+		 *
+		 * @return
+		 * @since 2.2.2
+		 */
+		boolean getUseDataNodeAuthenticationForSentinel();
 	}
 
 	/**

src/main/java/org/springframework/data/redis/connection/RedisSentinelConfiguration.java

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.redis.connection;
 
-import static org.springframework.util.Assert.*;
-import static org.springframework.util.Assert.hasText;
 import static org.springframework.util.StringUtils.*;
 
 import java.util.Collections;
@@ -46,11 +44,15 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 
 	private static final String REDIS_SENTINEL_MASTER_CONFIG_PROPERTY = "spring.redis.sentinel.master";
 	private static final String REDIS_SENTINEL_NODES_CONFIG_PROPERTY = "spring.redis.sentinel.nodes";
+	private static final String REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY = "spring.redis.sentinel.password";
 
 	private @Nullable NamedNode master;
 	private Set<RedisNode> sentinels;
 	private int database;
-	private RedisPassword password = RedisPassword.none();
+
+	private RedisPassword dataNodePassword = RedisPassword.none();
+	private RedisPassword sentinelPassword = RedisPassword.none();
+	private boolean useDataNodePasswordForSentinel = false;
 
 	/**
 	 * Creates new {@link RedisSentinelConfiguration}.
@@ -89,7 +91,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	public RedisSentinelConfiguration(PropertySource<?> propertySource) {
 
-		notNull(propertySource, "PropertySource must not be null!");
+		Assert.notNull(propertySource, "PropertySource must not be null!");
 
 		this.sentinels = new LinkedHashSet<>();
 
@@ -101,6 +103,10 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 			appendSentinels(
 					commaDelimitedListToSet(propertySource.getProperty(REDIS_SENTINEL_NODES_CONFIG_PROPERTY).toString()));
 		}
+
+		if (propertySource.containsProperty(REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY)) {
+			this.setSentinelPassword(propertySource.getProperty(REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY).toString());
+		}
 	}
 
 	/**
@@ -110,7 +116,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	public void setSentinels(Iterable<RedisNode> sentinels) {
 
-		notNull(sentinels, "Cannot set sentinels to 'null'.");
+		Assert.notNull(sentinels, "Cannot set sentinels to 'null'.");
 
 		this.sentinels.clear();
 
@@ -134,7 +140,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	public void addSentinel(RedisNode sentinel) {
 
-		notNull(sentinel, "Sentinel must not be 'null'.");
+		Assert.notNull(sentinel, "Sentinel must not be 'null'.");
 		this.sentinels.add(sentinel);
 	}
 
@@ -144,7 +150,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	public void setMaster(NamedNode master) {
 
-		notNull(master, "Sentinel master node must not be 'null'.");
+		Assert.notNull(master, "Sentinel master node must not be 'null'.");
 		this.master = master;
 	}
 
@@ -230,7 +236,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	@Override
 	public RedisPassword getPassword() {
-		return password;
+		return dataNodePassword;
 	}
 
 	/*
@@ -242,15 +248,60 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 
 		Assert.notNull(password, "RedisPassword must not be null!");
 
-		this.password = password;
+		this.dataNodePassword = password;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.WithPassword#setSentinelPassword(org.springframework.data.redis.connection.RedisPassword)
+	 */
+	public void setSentinelPassword(RedisPassword sentinelPassword) {
+
+		Assert.state(!useDataNodePasswordForSentinel,
+				"Configuration uses Redis Data Node password for authenticating with Sentinel. Please set 'RedisSentinelConfiguration.useDataNodeAuthenticationForSentinel(false)' before using this option.");
+		Assert.notNull(sentinelPassword, "SentinelPassword must not be null!");
+		this.sentinelPassword = sentinelPassword;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.WithPassword#setSentinelPassword()
+	 */
+	@Override
+	public RedisPassword getSentinelPassword() {
+		return getUseDataNodeAuthenticationForSentinel() ? this.dataNodePassword : sentinelPassword;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.WithPassword#useDataNodeAuthenticationForSentinel(boolean)
+	 */
+	@Override
+	public void useDataNodeAuthenticationForSentinel(boolean useDataNodeAuthenticationForSentinel) {
+
+		if (useDataNodeAuthenticationForSentinel) {
+			Assert.state(!this.sentinelPassword.isPresent(),
+					"Configuration already defines a password for authenticating with Sentinel. Please use 'RedisSentinelConfiguration.setSentinelPassword(RedisPassword.none())' remove the password.");
+		}
+
+		this.useDataNodePasswordForSentinel = useDataNodeAuthenticationForSentinel;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.WithPassword#getUseDataNodeAuthenticationForSentinel()
+	 */
+	@Override
+	public boolean getUseDataNodeAuthenticationForSentinel() {
+		return this.useDataNodePasswordForSentinel;
 	}
 
 	private RedisNode readHostAndPortFromString(String hostAndPort) {
 
 		String[] args = split(hostAndPort, ":");
 
-		notNull(args, "HostAndPort need to be seperated by  ':'.");
+		Assert.notNull(args, "HostAndPort need to be seperated by  ':'.");
-		isTrue(args.length == 2, "Host and Port String needs to specified as host:port");
+		Assert.isTrue(args.length == 2, "Host and Port String needs to specified as host:port");
 		return new RedisNode(args[0], Integer.valueOf(args[1]).intValue());
 	}
 
@@ -261,8 +312,8 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 	 */
 	private static Map<String, Object> asMap(String master, Set<String> sentinelHostAndPorts) {
 
-		hasText(master, "Master address must not be null or empty!");
+		Assert.hasText(master, "Master address must not be null or empty!");
-		notNull(sentinelHostAndPorts, "SentinelHostAndPorts must not be null!");
+		Assert.notNull(sentinelHostAndPorts, "SentinelHostAndPorts must not be null!");
 
 		Map<String, Object> map = new HashMap<>();
 		map.put(REDIS_SENTINEL_MASTER_CONFIG_PROPERTY, master);

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java

@@ -1080,7 +1080,6 @@ public class LettuceConnectionFactory
 
 		applyToAll(redisUri, it -> {
 
-			getRedisPassword().toOptional().ifPresent(it::setPassword);
 			clientConfiguration.getClientName().ifPresent(it::setClientName);
 
 			it.setSsl(clientConfiguration.isUseSsl());

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java

@@ -50,6 +50,7 @@ import org.springframework.data.redis.connection.RedisGeoCommands.GeoRadiusComma
 import org.springframework.data.redis.connection.RedisListCommands.Position;
 import org.springframework.data.redis.connection.RedisNode;
 import org.springframework.data.redis.connection.RedisNode.NodeType;
+import org.springframework.data.redis.connection.RedisPassword;
 import org.springframework.data.redis.connection.RedisSentinelConfiguration;
 import org.springframework.data.redis.connection.RedisServer;
 import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
@@ -198,7 +199,8 @@ abstract public class LettuceConverters extends Converters {
 		};
 
 		SCORED_VALUE_TO_TUPLE = source -> source != null
-				? new DefaultTuple(source.getValue(), Double.valueOf(source.getScore())) : null;
+				? new DefaultTuple(source.getValue(), Double.valueOf(source.getScore()))
+				: null;
 
 		BYTES_LIST_TO_TUPLE_LIST_CONVERTER = source -> {
 
@@ -297,7 +299,8 @@ abstract public class LettuceConverters extends Converters {
 		};
 
 		GEO_COORDINATE_TO_POINT_CONVERTER = geoCoordinate -> geoCoordinate != null
-				? new Point(geoCoordinate.getX().doubleValue(), geoCoordinate.getY().doubleValue()) : null;
+				? new Point(geoCoordinate.getX().doubleValue(), geoCoordinate.getY().doubleValue())
+				: null;
 		GEO_COORDINATE_LIST_TO_POINT_LIST_CONVERTER = new ListConverter<>(GEO_COORDINATE_TO_POINT_CONVERTER);
 
 		KEY_VALUE_UNWRAPPER = source -> source.getValueOrElse(null);
@@ -637,15 +640,21 @@ abstract public class LettuceConverters extends Converters {
 		Assert.notNull(sentinelConfiguration, "RedisSentinelConfiguration is required");
 
 		Set<RedisNode> sentinels = sentinelConfiguration.getSentinels();
-		RedisURI.Builder builder = null;
+		RedisURI.Builder builder = RedisURI.builder();
 		for (RedisNode sentinel : sentinels) {
 
-			if (builder == null) {
+			RedisURI.Builder uri = RedisURI.Builder.sentinel(sentinel.getHost(), sentinel.getPort(),
-				builder = RedisURI.Builder.sentinel(sentinel.getHost(), sentinel.getPort(),
+					sentinelConfiguration.getMaster().getName());
-						sentinelConfiguration.getMaster().getName());
+
-			} else {
+			if (sentinelConfiguration.getSentinelPassword().isPresent()) {
-				builder.withSentinel(sentinel.getHost(), sentinel.getPort());
+				uri.withPassword(sentinelConfiguration.getSentinelPassword().get());
 			}
+			builder.withSentinel(uri.build());
+		}
+
+		RedisPassword password = sentinelConfiguration.getPassword();
+		if (password.isPresent()) {
+			builder.withPassword(password.get());
 		}
 
 		return builder.build();

src/test/java/org/springframework/data/redis/connection/RedisSentinelConfigurationUnitTests.java

@@ -22,7 +22,6 @@ import java.util.Collections;
 import java.util.HashSet;
 
 import org.junit.Test;
-
 import org.springframework.core.env.PropertySource;
 import org.springframework.mock.env.MockPropertySource;
 import org.springframework.util.StringUtils;
@@ -51,8 +50,7 @@ public class RedisSentinelConfigurationUnitTests {
 	public void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndMultipleHostAndPortStrings() {
 
 		RedisSentinelConfiguration config = new RedisSentinelConfiguration("mymaster",
-				new HashSet<>(Arrays.asList(
+				new HashSet<>(Arrays.asList(HOST_AND_PORT_1, HOST_AND_PORT_2, HOST_AND_PORT_3)));
-				HOST_AND_PORT_1, HOST_AND_PORT_2, HOST_AND_PORT_3)));
 
 		assertThat(config.getSentinels().size()).isEqualTo(3);
 		assertThat(config.getSentinels()).contains(new RedisNode("127.0.0.1", 123), new RedisNode("localhost", 456),
@@ -121,4 +119,65 @@ public class RedisSentinelConfigurationUnitTests {
 		assertThat(config.getSentinels()).contains(new RedisNode("127.0.0.1", 123), new RedisNode("localhost", 456),
 				new RedisNode("localhost", 789));
 	}
+
+	@Test // DATAREDIS-1060
+	public void throwsExceptionOnSentinelPasswordAlreadySetWhenTryingToReuseDataNodePassword() {
+
+		RedisSentinelConfiguration configuration = new RedisSentinelConfiguration("myMaster",
+				Collections.singleton(HOST_AND_PORT_1));
+		configuration.setSentinelPassword(RedisPassword.of("so-secret-you'll-never-guess-123"));
+
+		assertThatExceptionOfType(IllegalStateException.class)
+				.isThrownBy(() -> configuration.useDataNodeAuthenticationForSentinel(true));
+	}
+
+	@Test // DATAREDIS-1060
+	public void throwsExceptionOnSettingSentinelPasswordWhenAlreadyReusingDataNodePassword() {
+
+		RedisSentinelConfiguration configuration = new RedisSentinelConfiguration("myMaster",
+				Collections.singleton(HOST_AND_PORT_1));
+		configuration.setPassword(RedisPassword.of("qwerty"));
+		configuration.useDataNodeAuthenticationForSentinel(true);
+
+		assertThatExceptionOfType(IllegalStateException.class)
+				.isThrownBy(() -> configuration.setSentinelPassword(RedisPassword.of("who-needs-security-anyway")));
+	}
+
+	@Test // DATAREDIS-1060
+	public void settingSentinelPasswordReturnsDataNodePasswordIfUseDataNodeAuthIsTrue() {
+
+		RedisPassword password = RedisPassword.of("monkey-dragon->yeah-getting-better-combining-trivial-ones");
+		RedisSentinelConfiguration configuration = new RedisSentinelConfiguration("myMaster",
+				Collections.singleton(HOST_AND_PORT_1));
+		configuration.setPassword(password);
+		configuration.useDataNodeAuthenticationForSentinel(true);
+
+		assertThat(configuration.getSentinelPassword()).isEqualTo(password);
+	}
+
+	@Test // DATAREDIS-1060
+	public void dataNodePasswordDoesNotAffectSentinelPassword() {
+
+		RedisPassword password = RedisPassword.of("88888888-8x8-getting-creative-now");
+		RedisSentinelConfiguration configuration = new RedisSentinelConfiguration("myMaster",
+				Collections.singleton(HOST_AND_PORT_1));
+		configuration.setPassword(password);
+
+		assertThat(configuration.getSentinelPassword()).isEqualTo(RedisPassword.none());
+	}
+
+	@Test // DATAREDIS-1060
+	public void readSentinelPasswordFromConfigProperty() {
+
+		MockPropertySource propertySource = new MockPropertySource();
+		propertySource.setProperty("spring.redis.sentinel.master", "myMaster");
+		propertySource.setProperty("spring.redis.sentinel.nodes", HOST_AND_PORT_1);
+		propertySource.setProperty("spring.redis.sentinel.password", "computer-says-no");
+
+		RedisSentinelConfiguration config = new RedisSentinelConfiguration(propertySource);
+
+		assertThat(config.getSentinelPassword()).isEqualTo(RedisPassword.of("computer-says-no"));
+		assertThat(config.getSentinels().size()).isEqualTo(1);
+		assertThat(config.getSentinels()).contains(new RedisNode("127.0.0.1", 123));
+	}
 }

src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java

@@ -46,7 +46,6 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentMatchers;
-
 import org.springframework.beans.DirectFieldAccessor;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.data.redis.ConnectionFactoryTracker;
@@ -179,8 +178,8 @@ public class LettuceConnectionFactoryUnitTests {
 		}
 	}
 
-	@Test // DATAREDIS-524, DATAREDIS-1045
+	@Test // DATAREDIS-524, DATAREDIS-1045, DATAREDIS-1060
-	public void passwordShouldBeSetCorrectlyOnSentinelClient() {
+	public void passwordShouldNotBeSetOnSentinelClient() {
 
 		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(
 				new RedisSentinelConfiguration("mymaster", Collections.singleton("host:1234")));
@@ -197,8 +196,78 @@ public class LettuceConnectionFactoryUnitTests {
 		assertThat(redisUri.getPassword()).isEqualTo(connectionFactory.getPassword().toCharArray());
 
 		for (RedisURI sentinel : redisUri.getSentinels()) {
-			assertThat(sentinel.getPassword())
+			assertThat(sentinel.getPassword()).isNull();
-					.isEqualTo(connectionFactory.getPassword().toCharArray());
+		}
+	}
+
+	@Test // DATAREDIS-1060
+	public void sentinelPasswordShouldBeSetOnSentinelClient() {
+
+		RedisSentinelConfiguration config = new RedisSentinelConfiguration("mymaster", Collections.singleton("host:1234"));
+		config.setSentinelPassword("sentinel-pwd");
+
+		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(config);
+		connectionFactory.setClientResources(getSharedClientResources());
+		connectionFactory.setPassword("o_O");
+		connectionFactory.afterPropertiesSet();
+		ConnectionFactoryTracker.add(connectionFactory);
+
+		AbstractRedisClient client = (AbstractRedisClient) getField(connectionFactory, "client");
+		assertThat(client).isInstanceOf(RedisClient.class);
+
+		RedisURI redisUri = (RedisURI) getField(client, "redisURI");
+
+		assertThat(redisUri.getPassword()).isEqualTo(connectionFactory.getPassword().toCharArray());
+
+		for (RedisURI sentinel : redisUri.getSentinels()) {
+			assertThat(sentinel.getPassword()).isEqualTo("sentinel-pwd".toCharArray());
+		}
+	}
+
+	@Test // DATAREDIS-1060
+	public void redisPasswordShouldBeSetOnSentinelClientIfItShouldBeReused() {
+
+		RedisSentinelConfiguration config = new RedisSentinelConfiguration("mymaster", Collections.singleton("host:1234"));
+		config.useDataNodeAuthenticationForSentinel(true);
+
+		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(config);
+		connectionFactory.setClientResources(getSharedClientResources());
+		connectionFactory.setPassword("o_O");
+		connectionFactory.afterPropertiesSet();
+		ConnectionFactoryTracker.add(connectionFactory);
+
+		AbstractRedisClient client = (AbstractRedisClient) getField(connectionFactory, "client");
+		assertThat(client).isInstanceOf(RedisClient.class);
+
+		RedisURI redisUri = (RedisURI) getField(client, "redisURI");
+
+		assertThat(redisUri.getPassword()).isEqualTo(connectionFactory.getPassword().toCharArray());
+
+		for (RedisURI sentinel : redisUri.getSentinels()) {
+			assertThat(sentinel.getPassword()).isEqualTo("o_O".toCharArray());
+		}
+	}
+
+	@Test // DATAREDIS-1060
+	public void sentinelPasswordShouldNotLeakIntoDataNodeClient() {
+
+		RedisSentinelConfiguration config = new RedisSentinelConfiguration("mymaster", Collections.singleton("host:1234"));
+		config.setSentinelPassword("sentinel-pwd");
+
+		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(config);
+		connectionFactory.setClientResources(getSharedClientResources());
+		connectionFactory.afterPropertiesSet();
+		ConnectionFactoryTracker.add(connectionFactory);
+
+		AbstractRedisClient client = (AbstractRedisClient) getField(connectionFactory, "client");
+		assertThat(client).isInstanceOf(RedisClient.class);
+
+		RedisURI redisUri = (RedisURI) getField(client, "redisURI");
+
+		assertThat(redisUri.getPassword()).isNull();
+
+		for (RedisURI sentinel : redisUri.getSentinels()) {
+			assertThat(sentinel.getPassword()).isEqualTo("sentinel-pwd".toCharArray());
 		}
 	}