WIP

Author: apata

lib/plausible/stats/dashboard_query_serializer.ex

@@ -51,7 +51,7 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
   defp get_serialized_fields({:filters, [_ | _] = filters}) do
     filters
     |> Enum.map(fn [operator, dimension, clauses] ->
-      clauses = Enum.map_join(clauses, ",", &uri_encode_permissive/1)
+      clauses = Enum.map_join(clauses, ",", &PlausibleWeb.URIEncoding.uri_encode_permissive/1)
       dimension = String.split(dimension, ":", parts: 2) |> List.last()
       {"f", "#{operator},#{dimension},#{clauses}"}
     end)
@@ -68,18 +68,4 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
   defp get_serialized_fields(_) do
     []
   end
-
-  # These characters are not URL encoded to have more readable URLs.
-  # Browsers seem to handle this just fine. `?f=is,page,/my/page/:some_param`
-  # vs `?f=is,page,%2Fmy%2Fpage%2F%3Asome_param`
-  @do_not_url_encode [":", "/"]
-  @do_not_url_encode_map Enum.into(@do_not_url_encode, %{}, fn char ->
-                           {URI.encode_www_form(char), char}
-                         end)
-
-  defp uri_encode_permissive(input) do
-    input
-    |> URI.encode_www_form()
-    |> String.replace(Map.keys(@do_not_url_encode_map), &@do_not_url_encode_map[&1])
-  end
 end

lib/plausible_web/controllers/stats_controller.ex

@@ -282,7 +282,9 @@ defmodule PlausibleWeb.StatsController do
       )
 
     if shared_link do
-      new_link_format = Routes.stats_path(conn, :shared_link, shared_link.site.domain, auth: slug)
+      new_link_format =
+        Routes.stats_path(conn, :shared_link, shared_link.site.domain, [], auth: slug)
+
       redirect(conn, to: new_link_format)
     else
       render_error(conn, 404)
@@ -306,6 +308,15 @@ defmodule PlausibleWeb.StatsController do
   defp render_password_protected_shared_link(conn, shared_link) do
     conn = Plug.Conn.fetch_cookies(conn)
 
+    star_path = conn.path_params["path"]
+
+    star_path_fragment = serialize_star_path_as_query_string_fragment(star_path)
+
+    query_string =
+      [conn.query_string, star_path_fragment]
+      |> Enum.filter(fn v -> is_binary(v) and String.length(v) > 0 end)
+      |> Enum.join("&")
+
     case validate_shared_link_password(conn, shared_link) do
       {:ok, shared_link} ->
         render_shared_link(conn, shared_link)
@@ -314,7 +325,7 @@ defmodule PlausibleWeb.StatsController do
         conn
         |> render("shared_link_password.html",
           link: shared_link,
-          query_string: conn.query_string,
+          query_string: query_string,
           dogfood_page_path: "/share/:dashboard"
         )
     end
@@ -349,12 +360,23 @@ defmodule PlausibleWeb.StatsController do
       if Plausible.Auth.Password.match?(password, shared_link.password_hash) do
         token = Plausible.Auth.Token.sign_shared_link(slug)
 
+        star_path = parse_star_path(conn)
+
+        query_string_fragment =
+          get_rest_of_query_string(conn) |> omit_from_query_string("return_to")
+
         conn
         |> put_resp_cookie(shared_link_cookie_name(slug), token)
         |> redirect(
           to:
-            Routes.stats_path(conn, :shared_link, shared_link.site.domain, auth: slug) <>
-              qs_appendix(conn)
+            Routes.stats_path(
+              conn,
+              :shared_link,
+              shared_link.site.domain,
+              star_path,
+              auth: slug
+            ) <>
+              query_string_fragment
         )
       else
         conn
@@ -370,12 +392,47 @@ defmodule PlausibleWeb.StatsController do
     end
   end
 
-  def qs_appendix(conn)
-      when is_nil(conn.query_string) or
-             (is_binary(conn.query_string) and byte_size(conn.query_string)) == 0,
-      do: ""
+  defp serialize_star_path_as_query_string_fragment(star_path) do
+    if length(star_path) > 0 do
+      # make the path start with a /
+      # to be able to reject values that don't start with a /
+      serialized_value =
+        "/#{Enum.join(star_path, "/")}"
+        |> PlausibleWeb.URIEncoding.uri_encode_permissive()
 
-  def qs_appendix(conn), do: "&#{conn.query_string}"
+      "return_to=#{serialized_value}"
+    else
+      nil
+    end
+  end
+
+  defp parse_star_path(conn) do
+    return_to_value = conn.query_params["return_to"]
+
+    if not is_nil(return_to_value) and String.starts_with?(return_to_value, "/") do
+      # get rid of the / character that we added
+      "/" <> trimmed_return_to_value = return_to_value
+      String.split(trimmed_return_to_value, "/")
+    else
+      []
+    end
+  end
+
+  defp get_rest_of_query_string(conn)
+       when is_nil(conn.query_string) or
+              (is_binary(conn.query_string) and byte_size(conn.query_string)) == 0,
+       do: ""
+
+  defp get_rest_of_query_string(conn), do: "&#{conn.query_string}"
+
+  defp omit_from_query_string(query_string, key) do
+    query_string
+    |> String.split("&")
+    |> Enum.reject(fn key_and_value ->
+      key_and_value == key || String.starts_with?(key_and_value, key)
+    end)
+    |> Enum.join("&")
+  end
 
   defp render_shared_link(conn, shared_link) do
     shared_links_feature_access? =

lib/plausible_web/router.ex

@@ -468,7 +468,7 @@ defmodule PlausibleWeb.Router do
   scope "/", PlausibleWeb do
     pipe_through [:shared_link]
 
-    get "/share/:domain", StatsController, :shared_link
+    get "/share/:domain/*path", StatsController, :shared_link
     post "/share/:slug/authenticate", StatsController, :authenticate_shared_link
   end
 

lib/plausible_web/uri_encoding.ex

@@ -0,0 +1,15 @@
+defmodule PlausibleWeb.URIEncoding do
+  # These characters are not URL encoded to have more readable URLs.
+  # Browsers seem to handle this just fine. `?f=is,page,/my/page/:some_param`
+  # vs `?f=is,page,%2Fmy%2Fpage%2F%3Asome_param`
+  @do_not_url_encode [":", "/"]
+  @do_not_url_encode_map Enum.into(@do_not_url_encode, %{}, fn char ->
+                           {URI.encode_www_form(char), char}
+                         end)
+
+  def uri_encode_permissive(input) do
+    input
+    |> URI.encode_www_form()
+    |> String.replace(Map.keys(@do_not_url_encode_map), &@do_not_url_encode_map[&1])
+  end
+end

test/plausible_web/controllers/stats_controller_test.exs

@@ -1500,7 +1500,7 @@ defmodule PlausibleWeb.StatsControllerTest do
       site_link = insert(:shared_link, site: site, inserted_at: ~N[2021-12-31 00:00:00])
 
       conn = get(conn, "/share/#{site_link.slug}")
-      assert redirected_to(conn, 302) == "/share/#{site.domain}?auth=#{site_link.slug}"
+      assert redirected_to(conn, 302) == "/share/#{site.domain}/?auth=#{site_link.slug}"
     end
 
     test "it does nothing for newer links", %{conn: conn} do
@@ -1520,7 +1520,7 @@ defmodule PlausibleWeb.StatsControllerTest do
         insert(:shared_link, site: site, password_hash: Plausible.Auth.Password.hash("password"))
 
       conn = post(conn, "/share/#{link.slug}/authenticate", %{password: "password"})
-      assert redirected_to(conn, 302) == "/share/#{site.domain}?auth=#{link.slug}"
+      assert redirected_to(conn, 302) == "/share/#{site.domain}/?auth=#{link.slug}"
 
       conn = get(conn, "/share/#{site.domain}?auth=#{link.slug}")
       assert html_response(conn, 200) =~ "stats-react-container"
@@ -1550,7 +1550,7 @@ defmodule PlausibleWeb.StatsControllerTest do
         )
 
       conn = post(conn, "/share/#{link.slug}/authenticate", %{password: "password"})
-      assert redirected_to(conn, 302) == "/share/#{site.domain}?auth=#{link.slug}"
+      assert redirected_to(conn, 302) == "/share/#{site.domain}/?auth=#{link.slug}"
 
       conn = get(conn, "/share/#{site2.domain}?auth=#{link2.slug}")
       assert html_response(conn, 200) =~ "Enter password"
@@ -1586,7 +1586,7 @@ defmodule PlausibleWeb.StatsControllerTest do
         )
 
       expected_redirect =
-        "/share/#{URI.encode_www_form(site.domain)}?auth=#{link.slug}&#{filters}"
+        "/share/#{URI.encode_www_form(site.domain)}/?auth=#{link.slug}&#{filters}"
 
       assert redirected_to(conn, 302) == expected_redirect
 
@@ -1629,6 +1629,42 @@ defmodule PlausibleWeb.StatsControllerTest do
     end
   end
 
+  test "handles return_to during password authentication", %{conn: conn} do
+    site = new_site()
+
+    link =
+      insert(:shared_link, site: site, password_hash: Plausible.Auth.Password.hash("password"))
+
+    filters = "f=is,country,EE&l=EE,Estonia&f=is,browser,Firefox"
+
+    deep_path = "/filter/source"
+
+    conn =
+      get(
+        conn,
+        "/share/#{site.domain}#{deep_path}?auth=#{link.slug}&#{filters}"
+      )
+
+    assert html_response(conn, 200) =~ "Enter password"
+    html = html_response(conn, 200)
+
+    expected_action_string = "/share/#{link.slug}/authenticate?auth=#{link.slug}&#{filters}&return_to=#{deep_path}"
+    assert html =~ ~s(action="#{expected_action_string |> String.replace("&", "&")}")
+
+    conn =
+      post(
+        conn,
+        expected_action_string,
+        %{password: "password"}
+      )
+
+    expected_redirect =
+      "/share/#{URI.encode_www_form(site.domain)}#{deep_path}?auth=#{link.slug}&#{filters}"
+
+    assert redirected_to(conn, 302) == expected_redirect
+  end
+
+
   describe "dogfood tracking" do
     @describetag :ee_only
 

test/plausible_web/uri_encoding_test.exs

@@ -0,0 +1,16 @@
+defmodule PlausibleWeb.URIEncodingTest do
+  use ExUnit.Case, async: true
+
+  for {value, expected} <- [
+        {"/:dashboard/settings", "/:dashboard/settings"},
+        {"&", "%26"},
+        {"=", "%3D"},
+        {",", "%2C"},
+        # should be {"hello world", "hello%20world"}, is
+        {"hello world", "hello+world"},
+      ] do
+    test "permissively uri-encoding value #{inspect(value)} yields #{inspect(expected)}" do
+      assert PlausibleWeb.URIEncoding.uri_encode_permissive(unquote(value)) == unquote(expected)
+    end
+  end
+end