From 19c99a3b8af8bb9a53859ad3e6e5f7cf8fbfabbc Mon Sep 17 00:00:00 2001 From: rahul MALAWADKAR <49741611+relaxedboi@users.noreply.github.com> Date: Fri, 15 Aug 2025 23:46:47 +0530 Subject: [PATCH] fix(deps) fix : (commons-httpclient:commons-httpclient) (#14436) Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> --- build.gradle | 1 + .../controllers/AuthenticationController.java | 9 ++--- datahub-frontend/play.gradle | 1 + .../AuthenticationControllerTest.java | 39 ++++++++++++++++--- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index 6c51fcb017..d0d8694feb 100644 --- a/build.gradle +++ b/build.gradle @@ -397,6 +397,7 @@ configure(subprojects.findAll {! it.name.startsWith('spark-lineage')}) { exclude group: 'com.typesafe.play', module: 'shaded-asynchttpclient' exclude group: "com.typesafe.akka", module: "akka-protobuf-v3_$playScalaVersion" exclude group: 'com.typesafe.play', module: 'shaded-oauth' + exclude group: 'commons-httpclient', module: 'commons-httpclient' exclude group: 'commons-collections', module: 'commons-collections' // Tomcat excluded for jetty diff --git a/datahub-frontend/app/controllers/AuthenticationController.java b/datahub-frontend/app/controllers/AuthenticationController.java index 71de1e7cf3..d8fbd7e9f8 100644 --- a/datahub-frontend/app/controllers/AuthenticationController.java +++ b/datahub-frontend/app/controllers/AuthenticationController.java @@ -31,8 +31,8 @@ import java.util.Base64; import java.util.Optional; import javax.annotation.Nonnull; import javax.inject.Inject; -import org.apache.commons.httpclient.InvalidRedirectLocationException; import org.apache.commons.lang3.StringUtils; +import org.apache.http.client.RedirectException; import org.pac4j.core.client.Client; import org.pac4j.core.context.CallContext; import org.pac4j.core.context.WebContext; @@ -109,13 +109,12 @@ public class AuthenticationController extends Controller { try { URI redirectUri = new URI(redirectPath); if (redirectUri.getScheme() != null || redirectUri.getAuthority() != null) { - throw new InvalidRedirectLocationException( + throw new RedirectException( "Redirect location must be relative to the base url, cannot " + "redirect to other domains: " - + redirectPath, - redirectPath); + + redirectPath); } - } catch (URISyntaxException | InvalidRedirectLocationException e) { + } catch (URISyntaxException | RedirectException e) { logger.warn(e.getMessage()); redirectPath = "/"; } diff --git a/datahub-frontend/play.gradle b/datahub-frontend/play.gradle index e62971e32b..84d18c02b4 100644 --- a/datahub-frontend/play.gradle +++ b/datahub-frontend/play.gradle @@ -88,6 +88,7 @@ dependencies { testImplementation 'org.seleniumhq.selenium:htmlunit-driver:2.67.0' testImplementation externalDependency.mockito + testImplementation externalDependency.mockitoInline testImplementation externalDependency.playTest testImplementation 'org.awaitility:awaitility:4.2.0' testImplementation 'no.nav.security:mock-oauth2-server:2.1.9' diff --git a/datahub-frontend/test/controllers/AuthenticationControllerTest.java b/datahub-frontend/test/controllers/AuthenticationControllerTest.java index 6960a03962..8b9d094b11 100644 --- a/datahub-frontend/test/controllers/AuthenticationControllerTest.java +++ b/datahub-frontend/test/controllers/AuthenticationControllerTest.java @@ -1,14 +1,12 @@ package controllers; import static auth.AuthUtils.REDIRECT_URL_COOKIE_NAME; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; +import auth.AuthUtils; import auth.sso.SsoManager; import auth.sso.SsoProvider; import client.AuthServiceClient; @@ -20,6 +18,7 @@ import java.util.Optional; import org.apache.commons.compress.utils.Lists; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import org.pac4j.core.client.Client; import org.pac4j.core.context.CallContext; import org.pac4j.core.exception.http.FoundAction; @@ -212,4 +211,34 @@ public class AuthenticationControllerTest { assertTrue(allCookiesSecure, "All cookies should be secure"); } + + @Test + public void testAuthenticateWithAbsoluteRedirectUriResetsToRoot() { + // No SSO + when(ssoManager.isSsoEnabled()).thenReturn(false); + + // Absolute URI triggers RedirectException in authenticate() + Http.Request request = + new Http.RequestBuilder() + .method("GET") + .uri("/authenticate?redirect_uri=http://evil.com") + .build(); + + try (MockedStatic authUtilsMock = mockStatic(auth.AuthUtils.class)) { + // Make hasValidSessionCookie return true so we take the direct redirect path + authUtilsMock.when(() -> auth.AuthUtils.hasValidSessionCookie(any())).thenReturn(true); + + Result result = controller.authenticate(request); + + // Should redirect (303) to "/" after catching RedirectException + assertEquals(303, result.status()); + assertEquals("/", result.redirectLocation().orElse("")); + + // We should not have any redirect cookie here because we bypass SSO + boolean hasRedirectCookie = + Lists.newArrayList(result.cookies().iterator()).stream() + .anyMatch(cookie -> cookie.name().equals(REDIRECT_URL_COOKIE_NAME)); + assertFalse(hasRedirectCookie, "No redirect cookie expected when redirectPath reset to '/'"); + } + } }