URL redirect handling strengthening

[allixender]

Hi @SudaisAkbar

Two things to review, now that we mix auth and no auth in the sessions/views:

1. Open redirect in authenticate/2. return_to comes straight from the POST body and is passed to redirect(conn, external: return_to). Phoenix's external: option skips the local-URL validation that to: enforces (confirmed in deps/phoenix/lib/phoenix/controller.ex), so a crafted form/request with return_to=https://evil.example redirects the browser off-site immediately after the session cookie is set, a phishing attempt could use that on a just-authenticated session. logout/2 correctly uses redirect(to: ~p"/stac/web/browse"); authenticate/2 should do the same (validate return_to is a local path, or drop the param and always redirect to /stac/web/browse).

2. UI-level inconsistency. The unlock form / "Private: ON" badge / logout button were only added to index.html.heex. landing.html.heex, search.html.heex, and item.html.heex have no such control at all, even though search.html.heex's own controller action (search/2, search_api/2) already honors browse_authenticated. So a user who lands directly on /stac/web/search has no way to unlock private results from that page, they'd have to know to detour through /browse first (the session, once set, does carry over). Same for item.html.heex: no indication whether you're currently authenticated.

Here is some additional claude assisted info:

1. Open redirect — lib/stac_api_web/controllers/stac_browser_controller.ex:445-461

def authenticate(conn, params) do
  api_key = Map.get(params, "api_key", "")
  return_to = Map.get(params, "return_to", "/stac/web/browse")   # L447: unvalidated client input
  ...
  |> redirect(external: return_to)   # L455 (success) and L459 (failure) both do this
end

redirect(conn, external: url) (deps/phoenix/lib/phoenix/controller.ex:499-503) is the one branch of Phoenix's url/1 helper that does not call validate_local_url/1 — to: rejects //host/...-style and backslash tricks, external: passes the string straight into the location header. Repro: curl -i -X POST -d "api_key=<valid RO key>&return_to=https://evil.example" http://localhost:4000/stac/web/auth → 302 to evil.example with the session cookie already set to browse_authenticated: true.

Simplest fix: drop the external: call entirely and validate return_to looks like a local path before using it, e.g.

defp safe_local_path("/" <> _ = path) when not (String.starts_with?(path, "//")), do: path
defp safe_local_path(_), do: "/stac/web/browse"

then redirect(to: safe_local_path(return_to)) in both branches. (logout/2 right below it already does redirect(to: ~p"/stac/web/browse") — no external: — so it's just the two call sites in authenticate/2 that need the same treatment.)

2. UI-level inconsistency — the unlock form / "Private: ON" badge / logout button were only added to lib/stac_api_web/controllers/stac_browser_html/index.html.heex. landing.html.heex, search.html.heex, and item.html.heex have no such control, even though search/2 and search_api/2 (in the same controller) already honor browse_authenticated server-side. A user landing straight on /stac/web/search can't unlock private results from that page — they have to know to detour through /browse first (the session does carry over once set). Lowest-effort fix: extract the badge/form block from index.html.heex into a function component and drop it into the other three templates' header rows.

For context, the private-catalog checks this UI gates are: index/2, show_catalog (both listing and direct fetch), the shared ensure_collection_visible/2 helper (used by show_collection/show_item), and Search.filter_by_private_catalogs/2 (lib/stac_api/data/search.ex:108-113) for search/2/search_api/2. All of those are one-hop checks (a resource's own private flag / its direct parent catalog's), consistent with each other — not something this issue needs to touch.

So, an extra bit of tightenting and consistency please.