Skip to content

Commit

Permalink
[Test] More robust comparison for OIDC URLs (#86374)
Browse files Browse the repository at this point in the history
The String representation of OIDC url is not stable because the query
parameters can be different orders (underlying backed by a HashMap).
This PR improves the robustness of URL comparison by comparing the
parsed query parameters instead of the non-stable serialised string
form.

Resolves: #86291
  • Loading branch information
ywangd committed May 3, 2022
1 parent 6ebe562 commit 3e8f729
Showing 1 changed file with 51 additions and 46 deletions.
Expand Up @@ -59,6 +59,7 @@
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -280,16 +281,14 @@ public void testBuildRelyingPartyConfigWithoutOpenIdScope() {
final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null);
final String state = response.getState();
final String nonce = response.getNonce();
assertThat(
assertEqualUrlStrings(
response.getAuthenticationRequestUrl(),
equalTo(
"https://op.example.com/login?scope=scope1+scope2+openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
)
"https://op.example.com/login?scope=scope1+scope2+openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
);
assertThat(response.getRealmName(), equalTo(REALM_NAME));
}
Expand All @@ -313,16 +312,14 @@ public void testBuildingAuthenticationRequest() {
final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null);
final String state = response.getState();
final String nonce = response.getNonce();
assertThat(
assertEqualUrlStrings(
response.getAuthenticationRequestUrl(),
equalTo(
"https://op.example.com/login?scope=openid+scope1+scope2&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
)
"https://op.example.com/login?scope=openid+scope1+scope2&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
);
assertThat(response.getRealmName(), equalTo(REALM_NAME));
}
Expand All @@ -343,16 +340,14 @@ public void testBuilidingAuthenticationRequestWithDefaultScope() {
final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null);
final String state = response.getState();
final String nonce = response.getNonce();
assertThat(
assertEqualUrlStrings(
response.getAuthenticationRequestUrl(),
equalTo(
"https://op.example.com/login?scope=openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
)
"https://op.example.com/login?scope=openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
);
assertThat(response.getRealmName(), equalTo(REALM_NAME));
}
Expand Down Expand Up @@ -413,16 +408,14 @@ public void testBuildingAuthenticationRequestWithExistingStateAndNonce() {
final String nonce = new Nonce().getValue();
final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(state, nonce, null);

assertThat(
assertEqualUrlStrings(
response.getAuthenticationRequestUrl(),
equalTo(
"https://op.example.com/login?scope=openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
)
"https://op.example.com/login?scope=openid&response_type=code"
+ "&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
);
assertThat(response.getRealmName(), equalTo(REALM_NAME));
}
Expand All @@ -445,21 +438,33 @@ public void testBuildingAuthenticationRequestWithLoginHint() {
final String thehint = randomAlphaOfLength(8);
final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(state, nonce, thehint);

assertThat(
assertEqualUrlStrings(
response.getAuthenticationRequestUrl(),
equalTo(
"https://op.example.com/login?login_hint="
+ thehint
+ "&scope=openid&response_type=code&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
)
"https://op.example.com/login?login_hint="
+ thehint
+ "&scope=openid&response_type=code&redirect_uri=https%3A%2F%2Frp.my.com%2Fcb&state="
+ state
+ "&nonce="
+ nonce
+ "&client_id=rp-my"
);
assertThat(response.getRealmName(), equalTo(REALM_NAME));
}

private void assertEqualUrlStrings(String actual, String expected) {
final int endOfPath = actual.indexOf('?');
assertThat(endOfPath, greaterThan(-1));
assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1)));

final HashMap<String, String> actualParams = new HashMap<>();
RestUtils.decodeQueryString(actual, endOfPath + 1, actualParams);

final HashMap<String, String> expectedParams = new HashMap<>();
RestUtils.decodeQueryString(expected, endOfPath + 1, expectedParams);

assertThat(actualParams, equalTo(expectedParams));
}

private AuthenticationResult<User> authenticateWithOidc(
String principal,
UserRoleMapper roleMapper,
Expand Down

0 comments on commit 3e8f729

Please sign in to comment.