rombert commented on code in PR #40:
URL:
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/40#discussion_r2664838517
##########
src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java:
##########
@@ -232,7 +233,7 @@ void accessTokenIsPresentOnSuccessfulLogin() throws
Exception {
break;
}
// Otherwise, we wait for a while and retry
- Thread.sleep(100);
+ Thread.sleep(RETRY_MILLIS);
Review Comment:
Was this encountered locally or on Jenkins? I would be inclined to ask you
to push a separate change for just the IT 'fix'. It might be better to use
something like Awaitility since we're getting larger potential delays.
##########
src/test/java/org/apache/sling/auth/oauth_client/impl/RedirectHelperTest.java:
##########
@@ -134,4 +141,113 @@ void testValidateRedirectWithInvalidUrl(String url) {
assertTrue(exception.getMessage().contains("Invalid redirect URL"));
assertTrue(exception.getCause() instanceof IllegalArgumentException);
}
+
+ @Test
+ void testBuildRedirectTargetWithSingleAudience() {
+ ResolvedConnection conn = createMockResolvedConnection();
+ CryptoService cryptoService = new StubCryptoService();
+ OAuthCookieValue oAuthCookieValue =
+ new OAuthCookieValue("perRequestKey", "connectionName",
"/redirect", new Nonce("nonce"), null);
+ String[] audience = new String[] {"https://api.example.com"};
+
+ RedirectTarget result = RedirectHelper.buildRedirectTarget(
+ new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+
+ assertNotNull(result);
+ assertNotNull(result.uri());
+ String uriString = result.uri().toString();
+ assertTrue(
+ uriString.contains("resource=https%3A%2F%2Fapi.example.com"),
+ "Expected resource parameter in URI but got: " + uriString);
+ }
+
+ @Test
+ void testBuildRedirectTargetWithMultipleAudiences() {
+ ResolvedConnection conn = createMockResolvedConnection();
+ CryptoService cryptoService = new StubCryptoService();
+ OAuthCookieValue oAuthCookieValue =
+ new OAuthCookieValue("perRequestKey", "connectionName",
"/redirect", new Nonce("nonce"), null);
+ String[] audience = new String[] {"https://api1.example.com";,
"https://api2.example.com"};
+
+ RedirectTarget result = RedirectHelper.buildRedirectTarget(
+ new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+
+ assertNotNull(result);
+ assertNotNull(result.uri());
+ String uriString = result.uri().toString();
+ // Using Nimbus SDK resources() method properly handles multiple
resource values
+ assertTrue(
+ uriString.contains("resource=https%3A%2F%2Fapi1.example.com"),
+ "Expected first resource parameter in URI but got: " +
uriString);
+ assertTrue(
+ uriString.contains("resource=https%3A%2F%2Fapi2.example.com"),
+ "Expected second resource parameter in URI but got: " +
uriString);
+ }
+
+ @Test
+ void testBuildRedirectTargetWithEmptyAudience() {
+ ResolvedConnection conn = createMockResolvedConnection();
+ CryptoService cryptoService = new StubCryptoService();
+ OAuthCookieValue oAuthCookieValue =
+ new OAuthCookieValue("perRequestKey", "connectionName",
"/redirect", new Nonce("nonce"), null);
+ String[] audience = new String[] {};
+
+ RedirectTarget result = RedirectHelper.buildRedirectTarget(
+ new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+
+ assertNotNull(result);
+ assertNotNull(result.uri());
+ String uriString = result.uri().toString();
+ assertFalse(uriString.contains("resource="), "Expected no resource
parameter in URI but got: " + uriString);
Review Comment:
This pattern is duplicated in multiple tests
- result not null
- result.uri() not null
- extract to String
- final assertion
I think it would be more readable to either extract an utility method or use
some chained calls from AssertJ.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
