Skip to content

Support for changing prefix and suffix in DelegatingPasswordEncoder #10278

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Support for changing prefix and suffix in DelegatingPasswordEncoder #10278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 41 additions & 12 deletions ...src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,19 @@
*
* @author Rob Winch
* @author Michael Simons
* @author heowc
* @since 5.0
* @see org.springframework.security.crypto.factory.PasswordEncoderFactories
*/
public class DelegatingPasswordEncoder implements PasswordEncoder {

private static final String PREFIX = "{";
private static final String DEFAULT_PREFIX = "{";

private static final String SUFFIX = "}";
private static final String DEFAULT_SUFFIX = "}";

private final String prefix;

private final String suffix;

private final String idForEncode;

Expand All @@ -142,9 +147,31 @@ public class DelegatingPasswordEncoder implements PasswordEncoder {
* {@link #matches(CharSequence, String)}
*/
public DelegatingPasswordEncoder(String idForEncode, Map<String, PasswordEncoder> idToPasswordEncoder) {
this(idForEncode, idToPasswordEncoder, DEFAULT_PREFIX, DEFAULT_SUFFIX);
}

/**
* Creates a new instance
* @param idForEncode the id used to lookup which {@link PasswordEncoder} should be
* used for {@link #encode(CharSequence)}
* @param idToPasswordEncoder a Map of id to {@link PasswordEncoder} used to determine
* which {@link PasswordEncoder} should be used for
* @param prefix the prefix that denotes the start of an {@code idForEncode}
* @param suffix the suffix that denotes the end of an {@code idForEncode}
* {@link #matches(CharSequence, String)}
*/
public DelegatingPasswordEncoder(String idForEncode, Map<String, PasswordEncoder> idToPasswordEncoder,
String prefix, String suffix) {
if (idForEncode == null) {
throw new IllegalArgumentException("idForEncode cannot be null");
}
if (prefix == null) {
throw new IllegalArgumentException("prefix cannot be null");
}
if (suffix == null || suffix.isEmpty()) {
throw new IllegalArgumentException("suffix cannot be empty");
}

if (!idToPasswordEncoder.containsKey(idForEncode)) {
throw new IllegalArgumentException(
"idForEncode " + idForEncode + "is not found in idToPasswordEncoder " + idToPasswordEncoder);
Expand All @@ -153,16 +180,18 @@ public DelegatingPasswordEncoder(String idForEncode, Map<String, PasswordEncoder
if (id == null) {
continue;
}
if (id.contains(PREFIX)) {
throw new IllegalArgumentException("id " + id + " cannot contain " + PREFIX);
if (!prefix.isEmpty() && id.contains(prefix)) {
throw new IllegalArgumentException("id " + id + " cannot contain " + prefix);
}
if (id.contains(SUFFIX)) {
throw new IllegalArgumentException("id " + id + " cannot contain " + SUFFIX);
if (id.contains(suffix)) {
throw new IllegalArgumentException("id " + id + " cannot contain " + suffix);
}
}
this.idForEncode = idForEncode;
this.passwordEncoderForEncode = idToPasswordEncoder.get(idForEncode);
this.idToPasswordEncoder = new HashMap<>(idToPasswordEncoder);
this.prefix = prefix;
this.suffix = suffix;
}

/**
Expand All @@ -188,7 +217,7 @@ public void setDefaultPasswordEncoderForMatches(PasswordEncoder defaultPasswordE

@Override
public String encode(CharSequence rawPassword) {
return PREFIX + this.idForEncode + SUFFIX + this.passwordEncoderForEncode.encode(rawPassword);
return this.prefix + this.idForEncode + this.suffix + this.passwordEncoderForEncode.encode(rawPassword);
}

@Override
Expand All @@ -209,15 +238,15 @@ private String extractId(String prefixEncodedPassword) {
if (prefixEncodedPassword == null) {
return null;
}
int start = prefixEncodedPassword.indexOf(PREFIX);
int start = prefixEncodedPassword.indexOf(this.prefix);
if (start != 0) {
return null;
}
int end = prefixEncodedPassword.indexOf(SUFFIX, start);
int end = prefixEncodedPassword.indexOf(this.suffix, start);
if (end < 0) {
return null;
}
return prefixEncodedPassword.substring(start + 1, end);
return prefixEncodedPassword.substring(start + this.prefix.length(), end);
}

@Override
Expand All @@ -233,8 +262,8 @@ public boolean upgradeEncoding(String prefixEncodedPassword) {
}

private String extractEncodedPassword(String prefixEncodedPassword) {
int start = prefixEncodedPassword.indexOf(SUFFIX);
return prefixEncodedPassword.substring(start + 1);
int start = prefixEncodedPassword.indexOf(this.suffix);
return prefixEncodedPassword.substring(start + this.suffix.length());
}

/**
Expand Down
64 changes: 63 additions & 1 deletion ...est/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -36,6 +36,7 @@
/**
* @author Rob Winch
* @author Michael Simons
* @author heowc
* @since 5.0
*/
@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -64,12 +65,16 @@ public class DelegatingPasswordEncoderTests {

private DelegatingPasswordEncoder passwordEncoder;

private DelegatingPasswordEncoder onlySuffixPasswordEncoder;

@BeforeEach
public void setup() {
this.delegates = new HashMap<>();
this.delegates.put(this.bcryptId, this.bcrypt);
this.delegates.put("noop", this.noop);
this.passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates);

this.onlySuffixPasswordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$");
}

@Test
Expand All @@ -83,6 +88,49 @@ public void constructorWhenIdForEncodeDoesNotExistThenIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId + "INVALID", this.delegates));
}

@Test
public void constructorWhenPrefixIsNull() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, null, "$"));
}

@Test
public void constructorWhenSuffixIsNull() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", null));
}

@Test
public void constructorWhenPrefixIsEmpty() {
assertThat(new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$")).isNotNull();
}

@Test
public void constructorWhenSuffixIsEmpty() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", ""));
}

@Test
public void constructorWhenPrefixAndSuffixAreEmpty() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", ""));
}

@Test
public void constructorWhenIdContainsPrefixThenIllegalArgumentException() {
this.delegates.put('$' + this.bcryptId, this.bcrypt);
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", "$"));
}

@Test
public void constructorWhenIdContainsSuffixThenIllegalArgumentException() {
this.delegates.put(this.bcryptId + '$', this.bcrypt);
assertThatIllegalArgumentException()
.isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$"));
}

@Test
public void setDefaultPasswordEncoderForMatchesWhenNullThenIllegalArgumentException() {
assertThatIllegalArgumentException()
Expand All @@ -104,6 +152,12 @@ public void encodeWhenValidThenUsesIdForEncode() {
assertThat(this.passwordEncoder.encode(this.rawPassword)).isEqualTo(this.bcryptEncodedPassword);
}

@Test
public void encodeWhenValidBySpecifyDelegatingPasswordEncoderThenUsesIdForEncode() {
given(this.bcrypt.encode(this.rawPassword)).willReturn(this.encodedPassword);
assertThat(this.onlySuffixPasswordEncoder.encode(this.rawPassword)).isEqualTo("bcrypt$" + this.encodedPassword);
}

@Test
public void matchesWhenBCryptThenDelegatesToBCrypt() {
given(this.bcrypt.matches(this.rawPassword, this.encodedPassword)).willReturn(true);
Expand All @@ -112,6 +166,14 @@ public void matchesWhenBCryptThenDelegatesToBCrypt() {
verifyZeroInteractions(this.noop);
}

@Test
public void matchesWhenBCryptBySpecifyDelegatingPasswordEncoderThenDelegatesToBCrypt() {
given(this.bcrypt.matches(this.rawPassword, this.encodedPassword)).willReturn(true);
assertThat(this.onlySuffixPasswordEncoder.matches(this.rawPassword, "bcrypt$" + this.encodedPassword)).isTrue();
verify(this.bcrypt).matches(this.rawPassword, this.encodedPassword);
verifyZeroInteractions(this.noop);
}

@Test
public void matchesWhenNoopThenDelegatesToNoop() {
given(this.noop.matches(this.rawPassword, this.encodedPassword)).willReturn(true);
Expand Down