Require login to export files; allow downloading the original too

?download=... on GET /api/gpx/file now requires login, same as
upload/move/delete/create-folder -- plain viewing (no ?download)
stays public. Added ?download=original alongside the existing
GeoJSON export, serving the untouched, as-uploaded file directly via
send_file -- the file name link still downloads GeoJSON, a new
button next to delete downloads the original.

Since both are real <a href> links (so right-click/middle-click still
work), a logged-out click can't just be left as a plain navigation --
it would land on a raw JSON error page instead of being sent to log
in. _download (gpx.js) fetches instead, reuses the existing
redirectIfUnauthorized 401 check, and only then turns the response
into a blob and clicks a throwaway <a download> to trigger the save.

Verified in a real browser: an anonymous click on either download
correctly redirects through /login to the real OAuth login page
instead of showing raw JSON.

app.rb

@@ -191,11 +191,29 @@ module Maptool
       json paths: paths, tree: GPX_STORE.tree
     end
 
-    # ?download=1 exports the file's GeoJSON as an attachment instead of
+    # ?download=geojson exports the file's GeoJSON as an attachment instead
-    # rendering it on the map — the only way to get a .gpx/.fit's converted
+    # of rendering it on the map — the only way to get a .gpx/.fit's
-    # form out of the app, since GpxStore#geojson's cache is otherwise an
+    # converted form out of the app, since GpxStore#geojson's cache is
-    # internal implementation detail with no filename of its own to expose.
+    # otherwise an internal implementation detail with no filename of its
+    # own to expose. ?download=original exports the untouched, as-uploaded
+    # file instead (a .geojson source's "original" is that same file, so
+    # the two downloads are identical there — harmless, not special-cased).
+    # Both gated the same as the other write/manage actions below (upload,
+    # move, create folder) — viewing a file on the map stays open to
+    # anyone, exporting it as a file does not.
     get "/api/gpx/file/*" do
+      require_login! if params[:download]
+
+      if params[:download] == "original"
+        begin
+          path = GPX_STORE.resolve_file(params[:splat].first)
+        rescue GpxStore::InvalidPath
+          halt 400
+        end
+        halt 404 unless File.file?(path)
+        return send_file(path, filename: File.basename(path), disposition: "attachment")
+      end
+
       begin
         body = GPX_STORE.geojson(params[:splat].first)
       rescue GpxStore::InvalidPath

docs/architecture.adoc

@@ -343,18 +343,29 @@ name instead — `"ride.gpx"` reads as `"ride.gpx.geojson"`, matching
 the real cache file sitting next to it on disk — since the GeoJSON,
 not the original, is what's actually rendered on the map from that
 point on. The original is never deleted or re-parsed once cached, just
-not re-shown as itself. Two ways to get that converted form out of the
+not re-shown as itself, so both forms need a way out of the app as an
-app as an actual file (`GET /api/gpx/file/<path>?download=1`) — the
+actual file: the file name itself (`GET /api/gpx/file/<path>?download=geojson`)
-cache is otherwise an implementation detail with no name of its own to
+is a real link, not a `<span>` — clicking it downloads the GeoJSON
-expose: the file name itself is a real link (an `<a>`, not a `<span>`,
+rather than also toggling the row's checkbox, since a native
-specifically so clicking it downloads instead of also toggling the
+`<a>`/`<button>` nested inside a `<label>` claims its own click
-row's checkbox — a native `<a>`/`<button>` nested inside a `<label>`
+instead of the label forwarding it to the associated control — and a
-claims its own click instead of the label forwarding it to the
+second button next to delete (`?download=original`) downloads the
-associated control, so the two behaviors don't fight over the same
+untouched, as-uploaded file, since the name alone only covers half of
-click), and a dedicated download button sits next to delete for anyone
+what's now downloadable and isn't an obvious click target regardless.
-who wouldn't guess the name itself is clickable. A directly-uploaded
+A directly-uploaded `.geojson` keeps its own name and both buttons
-`.geojson` keeps its own name (nothing to convert) and downloads
+download the same (identical) content — redundant there, not wrong.
-unchanged either way. `Gpx` parses the GPX with REXML: `<wpt>`
+Both kinds of download require login, same as upload/move/delete/
+create-folder — `GET /api/gpx/file` itself stays open to anyone
+(viewing a file on the map isn't a write action), just not
+`?download=...` on it. Since both links are real `<a href>`s (so
+right-click/middle-click still do something sensible), a logged-out
+click can't just be a plain navigation the way delete/upload already
+aren't — `_download` (`gpx.js`) fetches instead, checks for a 401 the
+same way `redirectIfUnauthorized` already does for every other write
+action, and only then turns the response into a blob and clicks a
+throwaway `<a download>` to trigger the actual save — a plain
+navigation would otherwise land on a raw JSON error page instead of
+being sent to log in. `Gpx` parses the GPX with REXML: `<wpt>`
 waypoints become `Point` features (with `name`, `desc` and `time`
 carried through as properties), each `<trk>` becomes one
 `MultiLineString` feature (one line per `<trkseg>`), and each `<rte>`
@@ -676,7 +687,10 @@ The GPX store's read endpoints (viewing the map, the folder tree,
 individual files) are public — anyone with the URL can look. Its write
 endpoints (`POST /api/gpx`, `POST /api/gpx/folder`, `DELETE
 /api/gpx/file/*`, `DELETE /api/gpx/folder/*`, `POST /api/gpx/move`)
-require login, guarded by `require_login!` in `app.rb`.
+require login, guarded by `require_login!` in `app.rb` — and so does
+exporting a file (`GET /api/gpx/file/*?download=...`), even though
+plain viewing of that same URL (no `?download`) doesn't; not a write
+exactly, but treated the same as the others rather than left open.
 
 Login delegates to the Gitea instance at `git.priesol.net` via OAuth2
 (`GET /login` redirects to Gitea's authorize endpoint, `GET

public/js/gpx.js

@@ -278,17 +278,26 @@ export class GpxPanelControl {
     // disk) rather than still reading as a plain, un-converted .gpx/.fit.
     // A direct .geojson upload already ends in .geojson, so it's left
     // alone. It's an <a>, not a <span>: clicking it downloads that GeoJSON
-    // (?download=1) — the only way to get it out of the app, since the
+    // — the only way to get it out of the app, since the original .gpx/.fit
-    // original .gpx/.fit is never deleted but also never re-shown once
+    // is never deleted but also never re-shown once converted. An <a>
-    // converted. An <a> nested in a <label> doesn't also toggle the
+    // nested in a <label> doesn't also toggle the checkbox on click (unlike
-    // checkbox on click (unlike a plain element), so this doesn't fight
+    // a plain element), so this doesn't fight the row's own
-    // the row's own toggle-on-map behavior.
+    // toggle-on-map behavior. href kept pointing at the real endpoint (so
+    // hover/right-click/middle-click still do something sensible) but the
+    // actual click goes through _download, which — like every other write
+    // action in this file — needs to redirect to /login on a 401 rather
+    // than navigate to a raw JSON error page (this endpoint now requires
+    // login the same as upload/move/delete, see app.rb).
     const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
     const name = document.createElement("a");
     name.className = "gpx-tree__file-name";
-    name.href = `${gpxFileUrl(file.path)}?download=1`;
+    name.href = `${gpxFileUrl(file.path)}?download=geojson`;
     name.textContent = displayName;
     name.title = `Download ${displayName}`;
+    name.addEventListener("click", (event) => {
+      event.preventDefault();
+      this._download(name.href, displayName);
+    });
     label.appendChild(checkbox);
     (file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind)));
     if (!file.cached) label.appendChild(this._pendingIcon());
@@ -324,18 +333,50 @@ export class GpxPanelControl {
     return icon;
   }
 
-  // Same destination as clicking the file's name (gpxFileUrl + ?download=1)
+  // The file's name already downloads its GeoJSON (?download=geojson,
-  // — a second, more obviously clickable entry point for the same action.
+  // matching what's displayed) — this covers the other half: the
+  // untouched, as-uploaded file, which otherwise has no way out of the
+  // app once it stops being shown as itself. For a native .geojson
+  // upload the two downloads are identical (nothing to convert), so this
+  // isn't hidden for those — just redundant, not wrong.
   _downloadButton(file) {
     const link = document.createElement("a");
     link.className = "gpx-tree__download";
-    link.href = `${gpxFileUrl(file.path)}?download=1`;
+    link.href = `${gpxFileUrl(file.path)}?download=original`;
-    link.title = "Download GeoJSON";
+    link.title = "Download original file";
-    link.setAttribute("aria-label", "Download GeoJSON");
+    link.setAttribute("aria-label", "Download original file");
     link.innerHTML = DOWNLOAD_ICON;
+    link.addEventListener("click", (event) => {
+      event.preventDefault();
+      this._download(link.href, file.name);
+    });
     return link;
   }
 
+  // fetch-based rather than a plain navigation so a 401 (not logged in —
+  // both download kinds require login now, see app.rb) redirects to
+  // /login like every other write action in this file
+  // (redirectIfUnauthorized), instead of navigating to a raw JSON error
+  // page. The filename is already known client-side, so there's no need
+  // to parse it back out of a Content-Disposition header.
+  async _download(url, filename) {
+    const response = await fetch(url);
+    if (redirectIfUnauthorized(response)) return;
+    if (!response.ok) {
+      window.alert("Could not download the file.");
+      return;
+    }
+
+    const blobUrl = URL.createObjectURL(await response.blob());
+    const link = document.createElement("a");
+    link.href = blobUrl;
+    link.download = filename;
+    document.body.appendChild(link);
+    link.click();
+    link.remove();
+    URL.revokeObjectURL(blobUrl);
+  }
+
   _deleteButton(onClick) {
     const button = document.createElement("button");
     button.type = "button";

test/test_app.rb

@@ -76,7 +76,7 @@ class AppTest < Minitest::Test
     content = File.read(fixture_path("sample.gpx"))
     post_file("/api/gpx", filename: "ride.gpx", content: content, env: logged_in_env)
 
-    response = request.get("/api/gpx/file/ride.gpx?download=1")
+    response = request.get("/api/gpx/file/ride.gpx?download=geojson", **logged_in_env)
     assert_equal 200, response.status
     assert_includes response.headers["Content-Disposition"], 'filename="ride.gpx.geojson"'
   end
@@ -85,11 +85,31 @@ class AppTest < Minitest::Test
     content = '{"type":"FeatureCollection","features":[]}'
     post_file("/api/gpx", filename: "spots.geojson", content: content, env: logged_in_env)
 
-    response = request.get("/api/gpx/file/spots.geojson?download=1")
+    response = request.get("/api/gpx/file/spots.geojson?download=geojson", **logged_in_env)
     assert_equal 200, response.status
     assert_includes response.headers["Content-Disposition"], 'filename="spots.geojson"'
   end
 
+  def test_download_of_the_original_file_serves_it_unconverted
+    content = File.read(fixture_path("sample.gpx"))
+    post_file("/api/gpx", filename: "ride.gpx", content: content, env: logged_in_env)
+
+    response = request.get("/api/gpx/file/ride.gpx?download=original", **logged_in_env)
+    assert_equal 200, response.status
+    assert_includes response.headers["Content-Disposition"], 'filename="ride.gpx"'
+    assert_equal content, response.body
+  end
+
+  def test_download_requires_login
+    content = File.read(fixture_path("sample.gpx"))
+    post_file("/api/gpx", filename: "ride.gpx", content: content, env: logged_in_env)
+
+    assert_equal 401, request.get("/api/gpx/file/ride.gpx?download=geojson").status
+    assert_equal 401, request.get("/api/gpx/file/ride.gpx?download=original").status
+    # Plain viewing (no ?download) stays open to anyone.
+    assert_equal 200, request.get("/api/gpx/file/ride.gpx").status
+  end
+
   def test_unparseable_gpx_returns_422
     post_file("/api/gpx", filename: "bad.gpx", content: "<gpx><trk unclosed>", env: logged_in_env)