Merge pull request #996 from bosch-io/bugfix/password-decoding

Bugfix/password decoding

model/connectivity/src/main/java/org/eclipse/ditto/model/connectivity/ImmutableConnection.java

@@ -15,8 +15,10 @@
 import static org.eclipse.ditto.model.base.common.ConditionChecker.checkArgument;
 import static org.eclipse.ditto.model.base.common.ConditionChecker.checkNotNull;
 
+import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URLDecoder;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -786,8 +788,8 @@ private ConnectionUri(final String theUriString) {
             final String userInfo = uri.getUserInfo();
             if (userInfo != null && userInfo.contains(USERNAME_PASSWORD_SEPARATOR)) {
                 final int separatorIndex = userInfo.indexOf(USERNAME_PASSWORD_SEPARATOR);
-                userName = userInfo.substring(0, separatorIndex);
-                password = userInfo.substring(separatorIndex + 1);
+                userName = tryDecodeUriComponent(userInfo.substring(0, separatorIndex));
+                password = tryDecodeUriComponent(userInfo.substring(separatorIndex + 1));
             } else {
                 userName = null;
                 password = null;
@@ -797,6 +799,15 @@ private ConnectionUri(final String theUriString) {
             uriStringWithMaskedPassword = createUriStringWithMaskedPassword();
         }
 
+        private static String tryDecodeUriComponent(final String string) {
+            try {
+                final String withoutPlus = string.replace("+", "%2B");
+                return URLDecoder.decode(withoutPlus, "UTF-8");
+            } catch (final IllegalArgumentException|UnsupportedEncodingException e) {
+                return string;
+            }
+        }
+
         private String createUriStringWithMaskedPassword() {
             return MessageFormat.format(MASKED_URI_PATTERN, protocol, getUserCredentialsOrEmptyString(), hostname, port,
                     getPathOrEmptyString());

model/connectivity/src/test/java/org/eclipse/ditto/model/connectivity/ImmutableConnectionTest.java

@@ -409,6 +409,24 @@ public void parseUriAsExpected() {
         assertThat(underTest.getPath()).contains("/vhost");
     }
 
+    @Test
+    public void parsePasswordWithPlusSign() {
+        final ConnectionUri underTest = ConnectionUri.of("amqps://foo:bar+baz@hono.eclipse.org:5671/vhost");
+        assertThat(underTest.getPassword()).contains("bar+baz");
+    }
+
+    @Test
+    public void parsePasswordWithPlusSignEncoded() {
+        final ConnectionUri underTest = ConnectionUri.of("amqps://foo:bar%2Bbaz@hono.eclipse.org:5671/vhost");
+        assertThat(underTest.getPassword()).contains("bar+baz");
+    }
+
+    @Test
+    public void parsePasswordWithPlusSignDoubleEncoded() {
+        final ConnectionUri underTest = ConnectionUri.of("amqps://foo:bar%252Bbaz@hono.eclipse.org:5671/vhost");
+        assertThat(underTest.getPassword()).contains("bar+baz");
+    }
+
     @Test
     public void parseUriWithoutCredentials() {
         final ConnectionUri underTest = ConnectionUri.of("amqps://hono.eclipse.org:5671");

model/messages/src/main/java/org/eclipse/ditto/model/messages/MessageSendNotAllowedException.java

@@ -43,7 +43,8 @@ public final class MessageSendNotAllowedException extends DittoRuntimeException
             "You are not allowed to send messages for the Thing with id ''{0}''!";
 
     private static final String DEFAULT_DESCRIPTION =
-            "Please make sure that the Thing exists and that you have a WRITE permission on the Thing.";
+            "Please make sure that the Thing exists and that you have WRITE permissions on the message resource in " +
+                    "the policy of the Thing.";
 
     private static final long serialVersionUID = -7767643705375184154L;
 

services/connectivity/messaging/src/main/java/org/eclipse/ditto/services/connectivity/messaging/amqp/AmqpSpecificConfig.java

@@ -147,9 +147,8 @@ private static void addCredentials(final LinkedHashMap<String, String> parameter
         final var username = connection.getUsername();
         final var password = connection.getPassword();
         if (username.isPresent() && password.isPresent()) {
-            // URL-decode the username and password to preserve previous behavior (no apparent encoding of parameters)
-            addParameter(parameters, USERNAME, tryDecode(username.get()));
-            addParameter(parameters, PASSWORD, tryDecode(password.get()));
+            addParameter(parameters, USERNAME, username.get());
+            addParameter(parameters, PASSWORD, password.get());
         }
     }
 
@@ -171,14 +170,6 @@ private static String encode(final String string) {
         return URLEncoder.encode(string, StandardCharsets.UTF_8);
     }
 
-    private static String tryDecode(final String string) {
-        try {
-            return URLDecoder.decode(string, StandardCharsets.UTF_8);
-        } catch (final IllegalArgumentException e) {
-            return string;
-        }
-    }
-
     private static boolean isSecuredConnection(final Connection connection) {
         return ConnectionBasedJmsConnectionFactory.isSecuredConnection(connection);
     }

services/connectivity/messaging/src/test/java/org/eclipse/ditto/services/connectivity/messaging/amqp/AmqpSpecificConfigTest.java

@@ -14,6 +14,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Map;
 
 import org.eclipse.ditto.services.connectivity.config.DefaultAmqp10Config;
@@ -29,7 +31,7 @@ public final class AmqpSpecificConfigTest {
 
     @Test
     public void decodeDoublyEncodedUsernameAndPassword() {
-        final var uri = "amqps://%2525u%2525s%2525e%2525r:%2525p%2525a%2525s%2525s@localhost:1234/";
+        final var uri = "amqps://%2525u%2525s%2525e%2525r:%2525p%2525a%2525%252Bs%2525s@localhost:1234/";
         final var connection = TestConstants.createConnection()
                 .toBuilder()
                 .uri(uri)
@@ -39,7 +41,26 @@ public void decodeDoublyEncodedUsernameAndPassword() {
 
         assertThat(underTest.render("amqps://localhost:1234/"))
                 .isEqualTo("failover:(amqps://localhost:1234/?amqp.saslMechanisms=PLAIN)" +
-                        "?jms.clientID=CID&jms.username=%25u%25s%25e%25r&jms.password=%25p%25a%25s%25s" +
+                        "?jms.clientID=CID&jms.username=%25u%25s%25e%25r&jms.password=%25p%25a%25%2Bs%25s" +
+                        "&failover.startupMaxReconnectAttempts=5&failover.maxReconnectAttempts=-1" +
+                        "&failover.initialReconnectDelay=128&failover.reconnectDelay=128" +
+                        "&failover.maxReconnectDelay=900000&failover.reconnectBackOffMultiplier=2" +
+                        "&failover.useReconnectBackOff=true");
+    }
+
+    @Test
+    public void decodeSinglyEncodedUsernameAndPasswordContainingPercentageSign() {
+        final var uri = "amqps://%25u%25s%25e%25r:%25p%25a%25%2Bs%25s@localhost:1234/";
+        final var connection = TestConstants.createConnection()
+                .toBuilder()
+                .uri(uri)
+                .build();
+
+        final var underTest = AmqpSpecificConfig.withDefault("CID", connection, Map.of());
+
+        assertThat(underTest.render("amqps://localhost:1234/"))
+                .isEqualTo("failover:(amqps://localhost:1234/?amqp.saslMechanisms=PLAIN)" +
+                        "?jms.clientID=CID&jms.username=%25u%25s%25e%25r&jms.password=%25p%25a%25%2Bs%25s" +
                         "&failover.startupMaxReconnectAttempts=5&failover.maxReconnectAttempts=-1" +
                         "&failover.initialReconnectDelay=128&failover.reconnectDelay=128" +
                         "&failover.maxReconnectDelay=900000&failover.reconnectBackOffMultiplier=2" +
@@ -48,7 +69,7 @@ public void decodeDoublyEncodedUsernameAndPassword() {
 
     @Test
     public void decodeSinglyEncodedUsernameAndPassword() {
-        final var uri = "amqps://%25u%25s%25e%25r:%25p%25a%25s%25s@localhost:1234/";
+        final var uri = "amqps://user:pa%2Bss@localhost:1234/";
         final var connection = TestConstants.createConnection()
                 .toBuilder()
                 .uri(uri)
@@ -58,7 +79,7 @@ public void decodeSinglyEncodedUsernameAndPassword() {
 
         assertThat(underTest.render("amqps://localhost:1234/"))
                 .isEqualTo("failover:(amqps://localhost:1234/?amqp.saslMechanisms=PLAIN)" +
-                        "?jms.clientID=CID&jms.username=%25u%25s%25e%25r&jms.password=%25p%25a%25s%25s" +
+                        "?jms.clientID=CID&jms.username=user&jms.password=pa%2Bss" +
                         "&failover.startupMaxReconnectAttempts=5&failover.maxReconnectAttempts=-1" +
                         "&failover.initialReconnectDelay=128&failover.reconnectDelay=128" +
                         "&failover.maxReconnectDelay=900000&failover.reconnectBackOffMultiplier=2" +

signals/commands/things/src/main/java/org/eclipse/ditto/signals/commands/things/exceptions/EventSendNotAllowedException.java

@@ -44,7 +44,8 @@ public final class EventSendNotAllowedException extends DittoRuntimeException im
             "You are not allowed to send events for the Thing with id ''{0}''!";
 
     private static final String DEFAULT_DESCRIPTION =
-            "Please make sure that the Thing exists and that you have a WRITE permission on the Thing.";
+            "Please make sure that the Thing exists and that you have WRITE permissions on the message resource in " +
+                    "the policy of the Thing.";
 
     private static final long serialVersionUID = 9055307625270596757L;