diff --git a/docs/calendar-integration.md b/docs/calendar-integration.md index cff83f8d..fe5b8d1a 100644 --- a/docs/calendar-integration.md +++ b/docs/calendar-integration.md @@ -110,11 +110,18 @@ appended. Wicket adds one whenever the client has not returned a cookie — prec for a calendar client fetching a feed. Such a URL is bound to one visitor's session: it stops working when the session expires, and sharing it hands out a live session identifier. -Scheme and host are taken from the incoming request. **Behind a TLS-terminating reverse -proxy this needs checking**: nothing in the application reads `X-Forwarded-Proto`, so unless -the servlet container is configured to honour it, the emitted feed URL will say `http://`. -The `webcal:` link survives that (the scheme is replaced anyway), but the Outlook subscribe -link and the copy-feed-URL entry would hand out an `http` address. +Scheme and host come from `NANODASH_WEBSITE_URL` (or the `websiteUrl` preference), not from +the incoming request. **A deployment behind a reverse proxy must set it.** The request says +only how the proxy reached the container — typically `http://127.0.1.1:37373/` — and that is +useless to the parties who fetch these URLs themselves: the user's calendar client, and +Google or Outlook on their behalf. Only the origin is taken from the configured URL; the path +and query stay as Wicket rendered them, and a path in the configured URL (an instance +published under `/nanodash/`) is prefixed unless the container is mounted there too. + +Where nothing is configured, the address is derived from the request as before — a local run +is then still correct, and guessing the `http://localhost:37373/` default would not be. Under +a TLS-terminating proxy that unconfigured case also emits `http://`, since nothing in the +application reads `X-Forwarded-Proto`. ## Feed contents diff --git a/src/main/java/com/knowledgepixels/nanodash/NanodashPreferences.java b/src/main/java/com/knowledgepixels/nanodash/NanodashPreferences.java index 3477965e..a38b496f 100644 --- a/src/main/java/com/knowledgepixels/nanodash/NanodashPreferences.java +++ b/src/main/java/com/knowledgepixels/nanodash/NanodashPreferences.java @@ -44,7 +44,7 @@ public static NanodashPreferences get() { private List nanopubActions = new ArrayList<>(); private boolean readOnlyMode = false; - private String websiteUrl = "http://localhost:37373/"; + private String websiteUrl; private boolean orcidLoginMode = false; private String orcidClientId; private String orcidClientSecret; @@ -54,6 +54,9 @@ public static NanodashPreferences get() { private String homeResource = "https://w3id.org/spaces/knowledgepixels/nanodash/r/home"; public static final String DEFAULT_SETTING_PATH = "/.nanopub/nanodash-preferences.yml"; + /** Where an instance is assumed to be reachable when nothing says otherwise: a local run. */ + public static final String DEFAULT_WEBSITE_URL = "http://localhost:37373/"; + /** * Return the list of nanopub actions. * @@ -102,16 +105,35 @@ public void setReadOnlyMode(boolean readOnlyMode) { /** * Get the website URL. * - * @return the website URL + * @return the website URL, falling back to {@link #DEFAULT_WEBSITE_URL} when this instance + * has not been told where it is reachable */ public String getWebsiteUrl() { + String s = getConfiguredWebsiteUrl(); + if (s != null) return s; + logger.debug("No website URL configured, using default: {}", DEFAULT_WEBSITE_URL); + return DEFAULT_WEBSITE_URL; + } + + /** + * The website URL as actually configured for this instance, from the + * {@code NANODASH_WEBSITE_URL} environment variable or the preferences file — or null if + * neither sets one. + * + *

Distinct from {@link #getWebsiteUrl()} because a caller that builds URLs for the + * outside world must be able to tell a real deployment address from the localhost + * fallback: guessing {@code localhost} would be worse than deriving the address from the + * request. See {@link Utils#absolutePageUrl}.

+ * + * @return the configured website URL, or null when unconfigured + */ + public String getConfiguredWebsiteUrl() { String s = System.getenv("NANODASH_WEBSITE_URL"); if (s != null && !s.isBlank()) { logger.debug("Found environment variable NANODASH_WEBSITE_URL with value: {}", s); return s; } - logger.debug("Environment variable NANODASH_WEBSITE_URL not set, using default: {}", websiteUrl); - return websiteUrl; + return (websiteUrl == null || websiteUrl.isBlank()) ? null : websiteUrl; } /** diff --git a/src/main/java/com/knowledgepixels/nanodash/Utils.java b/src/main/java/com/knowledgepixels/nanodash/Utils.java index 15141a17..41b0e17c 100644 --- a/src/main/java/com/knowledgepixels/nanodash/Utils.java +++ b/src/main/java/com/knowledgepixels/nanodash/Utils.java @@ -52,6 +52,7 @@ import java.time.format.DateTimeParseException; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Matcher; import java.util.regex.Pattern; import static java.nio.charset.StandardCharsets.UTF_8; @@ -169,6 +170,9 @@ public static String stripToNanopubId(String id) { // outlives the request — see absolutePageUrl. private static final Pattern JSESSIONID_PATTERN = Pattern.compile(";jsessionid=[^?/;]*", Pattern.CASE_INSENSITIVE); + // Splits an absolute URL into its origin (scheme and authority) and everything after it. + private static final Pattern ORIGIN_PATTERN = Pattern.compile("^([a-zA-Z][a-zA-Z0-9+.\\-]*://[^/?#]*)(.*)$"); + /** * The absolute URL of a mounted page, for handing to something outside this request: * embedding in a downloaded file, giving to a third-party service to fetch, or showing @@ -179,6 +183,11 @@ public static String stripToNanopubId(String id) { * once the session expired, and would hand out a live session identifier to anyone the * user shared it with.

* + *

The host comes from the configured website URL where there is one, not from the + * request. Behind a reverse proxy the request reveals only how the proxy reached this + * container — {@code http://127.0.1.1:37373/} — which is useless to a calendar client or + * to Google, both of which have to fetch the URL themselves from outside.

+ * * @param pageClass the mounted page * @param params the page parameters * @return the full URL, including scheme and host @@ -186,7 +195,37 @@ public static String stripToNanopubId(String id) { public static String absolutePageUrl(Class pageClass, PageParameters params) { RequestCycle cycle = RequestCycle.get(); String url = cycle.urlFor(pageClass, params).toString(); - return stripSessionId(cycle.getUrlRenderer().renderFullUrl(Url.parse(url))); + String requestUrl = stripSessionId(cycle.getUrlRenderer().renderFullUrl(Url.parse(url))); + return rebaseOnWebsiteUrl(requestUrl, NanodashPreferences.get().getConfiguredWebsiteUrl()); + } + + /** + * Moves a request-derived absolute URL onto the address this instance is published at. + * + *

Only the origin is taken from the website URL; the path and query stay as Wicket + * rendered them, so the mount paths remain the single source of truth for where a page + * lives. A website URL that carries a path of its own (an instance published under + * {@code /nanodash/}, say) has it prefixed, unless the request path already includes it + * because the servlet container is mounted there too.

+ * + * @param requestUrl the absolute URL derived from the current request + * @param websiteUrl the configured website URL, or null when this instance has none + * @return the URL rebased on the website URL, or {@code requestUrl} unchanged if there is + * no usable website URL + */ + static String rebaseOnWebsiteUrl(String requestUrl, String websiteUrl) { + if (websiteUrl == null || websiteUrl.isBlank()) return requestUrl; + Matcher website = ORIGIN_PATTERN.matcher(websiteUrl.trim()); + Matcher request = ORIGIN_PATTERN.matcher(requestUrl); + if (!website.matches() || !request.matches()) { + logger.warn("Cannot rebase '{}' on website URL '{}'; leaving it as it is", requestUrl, websiteUrl); + return requestUrl; + } + String prefix = website.group(2).replaceFirst("[?#].*$", "").replaceFirst("/+$", ""); + String rest = request.group(2); + boolean alreadyPrefixed = rest.equals(prefix) || rest.startsWith(prefix + "/") + || rest.startsWith(prefix + "?") || rest.startsWith(prefix + "#"); + return website.group(1) + (alreadyPrefixed ? "" : prefix) + rest; } /** diff --git a/src/test/java/com/knowledgepixels/nanodash/NanodashPreferencesTest.java b/src/test/java/com/knowledgepixels/nanodash/NanodashPreferencesTest.java index 97953fda..d5c9d514 100644 --- a/src/test/java/com/knowledgepixels/nanodash/NanodashPreferencesTest.java +++ b/src/test/java/com/knowledgepixels/nanodash/NanodashPreferencesTest.java @@ -221,6 +221,30 @@ void getWebsiteUrlFromSystemEnvBlankOrNull() { assertEquals("http://localhost:37373/", preferences.getWebsiteUrl()); } + // Whether an address was configured at all is a different question from what it is: a URL + // handed to an outside fetcher must not be built on the localhost fallback. See + // Utils.absolutePageUrl. + @Test + void getConfiguredWebsiteUrlIsNullWhenNothingSetsOne() { + assertNull(get().getConfiguredWebsiteUrl()); + } + + @Test + void getConfiguredWebsiteUrlFromSystemEnv() { + envVars.set("NANODASH_WEBSITE_URL", "https://nanodash.example.org/"); + assertEquals("https://nanodash.example.org/", get().getConfiguredWebsiteUrl()); + } + + @Test + void getConfiguredWebsiteUrlFromPreferencesFile() throws IOException { + Files.copy( + Path.of("src/test/resources/nanodash-preferences-test.yml"), + new File(System.getProperty("user.home") + DEFAULT_SETTING_PATH).toPath(), + StandardCopyOption.REPLACE_EXISTING + ); + assertEquals("http://localhost:37373/", get().getConfiguredWebsiteUrl()); + } + @Test void getOrcidClientIdWithDefaultValue() { NanodashPreferences preferences = get(); diff --git a/src/test/java/com/knowledgepixels/nanodash/UtilsAbsoluteUrlTest.java b/src/test/java/com/knowledgepixels/nanodash/UtilsAbsoluteUrlTest.java index 431a0ae8..f9fe2ed5 100644 --- a/src/test/java/com/knowledgepixels/nanodash/UtilsAbsoluteUrlTest.java +++ b/src/test/java/com/knowledgepixels/nanodash/UtilsAbsoluteUrlTest.java @@ -34,4 +34,61 @@ void urlsWithoutASessionIdAreLeftAlone() { assertEquals(url, Utils.stripSessionId(url)); } + // Behind a reverse proxy the request says only how the proxy reached this container. A + // calendar client, or Google fetching a feed on the user's behalf, has to reach the + // instance from outside — so the published address wins over the observed one. + @Test + void theOriginComesFromTheWebsiteUrlRatherThanTheRequest() { + assertEquals("https://nanodash.example.org/calendar.ics?space=https%3A%2F%2Fw3id.org%2Fnp%2Fx", + Utils.rebaseOnWebsiteUrl( + "http://127.0.1.1:37373/calendar.ics?space=https%3A%2F%2Fw3id.org%2Fnp%2Fx", + "https://nanodash.example.org/")); + } + + @Test + void aWebsiteUrlWithoutATrailingSlashWorksToo() { + assertEquals("https://nanodash.example.org/space?id=x", + Utils.rebaseOnWebsiteUrl("http://127.0.1.1:37373/space?id=x", "https://nanodash.example.org")); + } + + // An instance published under a path keeps it: the proxy strips it before the container + // sees the request, so it is missing from the rendered URL and has to be put back. + @Test + void aPathInTheWebsiteUrlIsPrefixed() { + assertEquals("https://example.org/nanodash/calendar.ics?space=x", + Utils.rebaseOnWebsiteUrl("http://127.0.1.1:37373/calendar.ics?space=x", + "https://example.org/nanodash/")); + } + + // ...but not twice, when the container is mounted under that path itself and Wicket has + // therefore already rendered it. + @Test + void aPathAlreadyPresentInTheRequestUrlIsNotDuplicated() { + assertEquals("https://example.org/nanodash/calendar.ics?space=x", + Utils.rebaseOnWebsiteUrl("http://127.0.1.1:37373/nanodash/calendar.ics?space=x", + "https://example.org/nanodash/")); + } + + // A path that merely starts with the same characters is a different path. + @Test + void aPathThatOnlyLooksLikeThePrefixIsStillPrefixed() { + assertEquals("https://example.org/nano/nanodash-x?id=1", + Utils.rebaseOnWebsiteUrl("http://127.0.1.1:37373/nanodash-x?id=1", "https://example.org/nano/")); + } + + // Without a configured address, deriving one from the request is the best available guess: + // substituting the localhost default would be a downgrade, not a fix. + @Test + void anUnconfiguredWebsiteUrlLeavesTheRequestUrlAlone() { + String url = "http://127.0.1.1:37373/calendar.ics?space=x"; + assertEquals(url, Utils.rebaseOnWebsiteUrl(url, null)); + assertEquals(url, Utils.rebaseOnWebsiteUrl(url, " ")); + } + + @Test + void anUnparsableWebsiteUrlLeavesTheRequestUrlAlone() { + String url = "http://127.0.1.1:37373/calendar.ics?space=x"; + assertEquals(url, Utils.rebaseOnWebsiteUrl(url, "nanodash.example.org")); + } + }