Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -158,7 +159,9 @@ public static boolean isInvalidSavedRequestUrl(String url) {
return true;
}

String urlLower = url.toLowerCase();
// Locale.ROOT: under a Turkish default locale, "I".toLowerCase() is a
// dotless ı, so "/API" would no longer match the "/api" prefix.
String urlLower = url.toLowerCase(Locale.ROOT);

return Arrays.stream(INVALID_SAVED_REQUEST_URL_SUFFIXES).anyMatch(urlLower::endsWith)
|| Arrays.stream(INVALID_SAVED_REQUEST_URL_PREFIXES)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.util.Locale;

import org.junit.Test;

public class LoginModuleUtilsTest {
Expand Down Expand Up @@ -55,6 +57,20 @@ public void testRestAndApiUrlsAreInvalid() {
assertTrue(LoginModuleUtils.isInvalidSavedRequestUrl("/REST/menu"));
}

@Test
public void testMatchingIsDefaultLocaleIndependent() {
// Under a Turkish default locale, locale-sensitive toLowerCase() turns
// "I" into a dotless ı, so "/API" would slip past the "/api" prefix.
final Locale defaultLocale = Locale.getDefault();
try {
Locale.setDefault(new Locale("tr", "TR"));
assertTrue(LoginModuleUtils.isInvalidSavedRequestUrl("/API/v2/nodes"));
assertTrue(LoginModuleUtils.isInvalidSavedRequestUrl("/API"));
} finally {
Locale.setDefault(defaultLocale);
}
}

@Test
public void testPageUrlsAreValid() {
assertFalse(LoginModuleUtils.isInvalidSavedRequestUrl("/index.jsp"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,11 @@
<intercept-url pattern="/assets/**" access="hasAnyRole('ROLE_ANONYMOUS','ROLE_USER','ROLE_DASHBOARD')" />
<!-- Vue menu bundle (static js/css/fonts). Must be anonymous: bootstrap.jsp preloads it from the
login page, and an auth redirect there gets cached by Safari/Firefox as text/html for the
asset URL, breaking the menu on all JSP pages after login (NMS-20174). -->
<intercept-url pattern="/ui-components/**" access="hasAnyRole('ROLE_ANONYMOUS','ROLE_USER','ROLE_DASHBOARD')" />
asset URL, breaking the menu on all JSP pages after login (NMS-20174). Deliberately limited
to assets/**: the unpacked dist-menu artifact also ships an index.html (the Vite build
input) at /ui-components/, which nothing links to and which must not be anonymously
reachable (NMS-20180). -->
<intercept-url pattern="/ui-components/assets/**" access="hasAnyRole('ROLE_ANONYMOUS','ROLE_USER','ROLE_DASHBOARD')" />

<intercept-url pattern="/admin/ng-requisitions/**" access="hasAnyRole('ROLE_PROVISION','ROLE_ADMIN')" />
<intercept-url pattern="/admin/classification/index.jsp" access="hasAnyRole('ROLE_FLOW_MANAGER','ROLE_ADMIN')" />
Expand Down
12 changes: 11 additions & 1 deletion opennms-webapp/src/main/webapp/includes/bootstrap.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,17 @@
</script>
<link rel="stylesheet" href="<%= __baseHref %>ui-components/assets/index.css<%= __menuAssetsVersion %>" media="screen" />
<%-- Start fetching/compiling the menu bundle now rather than when the parser
reaches its <script type="module"> tag near the end of the body. --%>
reaches its <script type="module"> tag near the end of the body.

These two links are deliberately emitted on 'quiet' pages too, including
the unauthenticated login page: preloading there warms the cache while
the user types their credentials, so the menu mounts instantly on the
first post-login page. This is only safe because the bundle is
anonymously accessible (see the /ui-components/assets/** rule in
applicationContext-spring-security.xml) — without that rule the preload
would cache an auth redirect as text/html under the asset URL and break
the menu on every JSP page after login (NMS-20174). Keep the two in
sync. --%>
<link rel="modulepreload" href="<%= __baseHref %>ui-components/assets/index.js<%= __menuAssetsVersion %>" />
</head>

Expand Down
63 changes: 63 additions & 0 deletions smoke-test/src/test/java/org/opennms/smoketest/WebappIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@
package org.opennms.smoketest;

import static io.restassured.RestAssured.given;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.text.MatchesPattern.matchesPattern;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import java.util.Arrays;
import java.util.regex.Pattern;

import org.apache.http.client.ClientProtocolException;
import org.junit.After;
Expand Down Expand Up @@ -137,6 +140,66 @@ public void verifyCachingOnStaticAssets() {
.header("Pragma", not("no-cache"));
}

/**
* NMS-20174: the Vue menu bundle must be accessible without authentication.
* bootstrap.jsp preloads it from the (unauthenticated) login page; if that
* request is answered with an auth redirect instead, Safari and Firefox cache
* the text/html response under the asset URL and the menu never mounts on any
* JSP page after login.
*/
@Test
public void verifyMenuBundleAssetsAreAnonymouslyAccessible() {
given().redirects().follow(false)
.get("ui-components/assets/index.js")
.then().assertThat()
.statusCode(200)
.header("Content-Type", containsString("javascript"));

given().redirects().follow(false)
.get("ui-components/assets/index.css")
.then().assertThat()
.statusCode(200)
.header("Content-Type", containsString("css"));
}

/**
* NMS-20180: only the menu bundle's assets/ directory is anonymous. The
* dist-menu artifact also ships an index.html (the Vite build input) at
* /ui-components/, which nothing links to and which must keep requiring
* authentication like any other page.
*/
@Test
public void verifyMenuIndexHtmlRequiresAuthentication() {
given().redirects().follow(false)
.get("ui-components/index.html")
.then().assertThat()
.statusCode(302);
}

/**
* NMS-20174: the login page references the menu bundle only as preload links
* (deliberate cache warming for the first post-login page) and must never
* execute it. An unauthenticated menu run fires REST calls that trigger the
* browser's native basic-auth popup on the login page and pollute the
* post-login saved request (the password gate's Skip button then redirects
* to /rest, which browsers download as a file).
*/
@Test
public void verifyLoginPageDoesNotExecuteMenuBundle() {
final String body = given()
.get("login.jsp")
.then().assertThat()
.statusCode(200)
.extract().response().body().asString();

// Positive control: the page still references the bundle (as a preload) —
// this keeps the negative assertion below meaningful if URLs change shape.
assertTrue("expected login.jsp to preload the menu bundle. Body: " + body,
body.contains("ui-components/assets/index.js"));
assertFalse("login.jsp must not contain a script tag executing the menu bundle. Body: " + body,
Pattern.compile("<script[^>]+ui-components/assets/index\\.js").matcher(body).find());
}

/**
* The Vue UI's static assets may be cached but must be revalidated on each use,
* because their file names are not content-hashed and change between releases.
Expand Down
1 change: 1 addition & 0 deletions ui/tests/components/Menu/UserSelfServiceMenuItem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,6 @@ describe('UserSelfServiceMenuItem.vue', () => {
await nextTick()

expect(performLogout).not.toHaveBeenCalled()
expect(event.defaultPrevented).toBe(false)
})
})
Loading