More fixes to the verification process (#4136)

* More fixes to the verification process: - extend the list of known attributes - follow redirect chain host to verify data-domain difference - display the actual data-domain in error message, not URL * format * fix test * remove redundant pattern matching * Update lib/plausible/verification/checks/snippet.ex Co-authored-by: Adrian Gruntkowski <[email protected]> --------- Co-authored-by: Adrian Gruntkowski <[email protected]>
plausible · May 24, 2024 · a7b6971 · a7b6971
1 parent ee61094
commit a7b6971
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 15 deletions.
diff --git a/lib/plausible/verification/checks/fetch_body.ex b/lib/plausible/verification/checks/fetch_body.ex
@@ -24,12 +24,14 @@ defmodule Plausible.Verification.Checks.FetchBody do
         fetch_body_opts
       )
 
-    req = Req.new(opts)
+    {req, resp} = opts |> Req.new() |> Req.Request.run_request()
 
-    case Req.get(req) do
-      {:ok, %Req.Response{status: status, body: body} = response}
+    case resp do
+      %Req.Response{status: status, body: body}
       when is_binary(body) and status in 200..299 ->
-        extract_document(state, response)
+        state
+        |> assign(final_domain: req.url.host)
+        |> extract_document(resp)
 
       _ ->
         state

diff --git a/lib/plausible/verification/checks/snippet.ex b/lib/plausible/verification/checks/snippet.ex
@@ -21,7 +21,8 @@ defmodule Plausible.Verification.Checks.Snippet do
       snippets_found_in_body: Enum.count(in_body),
       proxy_likely?: proxy_likely?(all),
       snippet_unknown_attributes?: unknown_attributes?(all),
-      data_domain_mismatch?: data_domain_mismatch?(all, state.data_domain)
+      data_domain_mismatch?:
+        data_domain_mismatch?(all, state.data_domain, state.assigns[:final_domain])
     )
   end
 
@@ -33,20 +34,32 @@ defmodule Plausible.Verification.Checks.Snippet do
     |> Enum.any?(&(not String.starts_with?(&1, PlausibleWeb.Endpoint.url())))
   end
 
-  @known_attributes ["data-domain", "src", "defer", "data-api", "data-exclude", "data-include"]
-  @known_prefix "event-"
+  @known_attributes [
+    "data-domain",
+    "src",
+    "defer",
+    "data-api",
+    "data-exclude",
+    "data-include"
+  ]
 
   defp unknown_attributes?(nodes) do
     Enum.any?(nodes, fn {_, attrs, _} ->
-      Enum.any?(attrs, fn {key, _} ->
-        key not in @known_attributes and not String.starts_with?(key, @known_prefix)
+      Enum.any?(attrs, fn
+        {"type", "text/javascript"} -> false
+        {"event-" <> _, _} -> false
+        {key, _} -> key not in @known_attributes
       end)
     end)
   end
 
-  defp data_domain_mismatch?(nodes, data_domain) do
+  defp data_domain_mismatch?(nodes, data_domain, final_data_domain) do
     nodes
     |> Floki.attribute("data-domain")
-    |> Enum.any?(&(&1 != data_domain and data_domain not in String.split(&1, ",")))
+    |> Enum.any?(fn script_data_domain ->
+      multiple = String.split(script_data_domain, ",")
+
+      data_domain not in multiple and final_data_domain not in multiple
+    end)
   end
 end
diff --git a/lib/plausible/verification/diagnostics.ex b/lib/plausible/verification/diagnostics.ex
@@ -354,10 +354,10 @@ defmodule Plausible.Verification.Diagnostics do
     }
   end
 
-  def interpret(%__MODULE__{data_domain_mismatch?: true}, url) do
+  def interpret(%__MODULE__{data_domain_mismatch?: true}, "https://" <> domain) do
     %Result{
       ok?: false,
-      errors: ["Your data-domain is different than #{url}"],
+      errors: ["Your data-domain is different than #{domain}"],
       recommendations: [
         {"Please ensure that the site in the data-domain attribute is an exact match to the site as you added it to your Plausible account",
          "https://plausible.io/docs/troubleshoot-integration"}

diff --git a/test/plausible/site/verification/checks/fetch_body_test.exs b/test/plausible/site/verification/checks/fetch_body_test.exs
@@ -45,7 +45,7 @@ defmodule Plausible.Verification.Checks.FetchBodyTest do
     stub(200, @normal_body, "text/plain")
     state = @check.perform(state)
 
-    assert map_size(state.assigns) == 0
+    assert state.assigns == %{final_domain: "example.com"}
 
     refute state.diagnostics.body_fetched?
   end

diff --git a/test/plausible/site/verification/checks/snippet_test.exs b/test/plausible/site/verification/checks/snippet_test.exs
@@ -109,7 +109,7 @@ defmodule Plausible.Verification.Checks.SnippetTest do
 
   @valid_attributes """
   <head>
-  <script defer data-api="some" data-include="some" data-exclude="some" data-domain="example.com" src="http://my-domain.example.com/js/script.js"></script>
+  <script defer type="text/javascript" data-api="some" data-include="some" data-exclude="some" data-domain="example.com" src="http://my-domain.example.com/js/script.js"></script>
   </head>
   """
 

diff --git a/test/plausible/site/verification/checks_test.exs b/test/plausible/site/verification/checks_test.exs
@@ -827,6 +827,64 @@ defmodule Plausible.Verification.ChecksTest do
                 "https://plausible.io/docs/troubleshoot-integration"}
              ]
     end
+
+    @different_data_domain_body """
+    <html>
+    <head>
+    <script defer data-domain="www.example.com" src="http://localhost:8000/js/script.js"></script>
+    </head>
+    <body>Hello</body>
+    </html>
+    """
+
+    test "data-domain mismatch" do
+      stub_fetch_body(200, @different_data_domain_body)
+      stub_installation()
+
+      result = run_checks()
+
+      interpretation = Checks.interpret_diagnostics(result)
+      refute interpretation.ok?
+      assert interpretation.errors == ["Your data-domain is different than example.com"]
+
+      assert interpretation.recommendations == [
+               {
+                 "Please ensure that the site in the data-domain attribute is an exact match to the site as you added it to your Plausible account",
+                 "https://plausible.io/docs/troubleshoot-integration"
+               }
+             ]
+    end
+
+    test "data-domain mismatch on redirect chain" do
+      ref = :counters.new(1, [:atomics])
+      test = self()
+
+      Req.Test.stub(Plausible.Verification.Checks.FetchBody, fn conn ->
+        if :counters.get(ref, 1) == 0 do
+          :counters.add(ref, 1, 1)
+          send(test, :redirect_sent)
+
+          conn
+          |> put_resp_header("location", "https://www.example.com")
+          |> send_resp(302, "redirecting to https://www.example.com")
+        else
+          conn
+          |> put_resp_header("content-type", "text/html")
+          |> send_resp(200, @different_data_domain_body)
+        end
+      end)
+
+      stub_installation()
+
+      result = run_checks()
+
+      assert_receive :redirect_sent
+
+      interpretation = Checks.interpret_diagnostics(result)
+      assert interpretation.ok?
+      assert interpretation.errors == []
+      assert interpretation.recommendations == []
+    end
   end
 
   defp run_checks(extra_opts \\ []) do