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

Bugfix/password decoding #996

Merged
merged 3 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ private static void addCredentials(final LinkedHashMap<String, String> parameter
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)
thjaeckle marked this conversation as resolved.
Show resolved Hide resolved
addParameter(parameters, USERNAME, tryDecode(username.get()));
addParameter(parameters, PASSWORD, tryDecode(password.get()));
addParameter(parameters, USERNAME, username.get());
addParameter(parameters, PASSWORD, password.get());
}
}

Expand All @@ -171,14 +171,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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" +
Expand All @@ -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)
Expand All @@ -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" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down