Fix subscribe URLs to use configured website URL

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
 

src/main/java/com/knowledgepixels/nanodash/NanodashPreferences.java

@@ -44,7 +44,7 @@ public static NanodashPreferences get() {
 
     private List<String> 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.
+     *
+     * <p>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}.</p>
+     *
+     * @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;
     }
 
     /**

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,14 +183,49 @@ 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.</p>
      *
+     * <p>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.</p>
+     *
      * @param pageClass the mounted page
      * @param params    the page parameters
      * @return the full URL, including scheme and host
      */
     public static String absolutePageUrl(Class<? extends org.apache.wicket.Page> 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.
+     *
+     * <p>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.</p>
+     *
+     * @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;
     }
 
     /**

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();

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"));
+    }
+
 }