From 2cdbba50d0afd44e510b12927dc204d06d4906e0 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Fri, 24 May 2024 12:22:39 +0200 Subject: [PATCH 1/5] 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 --- .../verification/checks/fetch_body.ex | 10 ++-- lib/plausible/verification/checks/snippet.ex | 28 ++++++--- lib/plausible/verification/diagnostics.ex | 4 +- .../site/verification/checks/snippet_test.exs | 2 +- .../site/verification/checks_test.exs | 58 +++++++++++++++++++ 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/lib/plausible/verification/checks/fetch_body.ex b/lib/plausible/verification/checks/fetch_body.ex index c85a02448f1e..3527c1052e5d 100644 --- 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} = response when is_binary(body) and status in 200..299 -> - extract_document(state, response) + state + |> assign(final_domain: req.url.host) + |> extract_document(response) _ -> state diff --git a/lib/plausible/verification/checks/snippet.ex b/lib/plausible/verification/checks/snippet.ex index f43950ec66cf..95e0a57c5623 100644 --- 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,33 @@ 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, ",") + + script_data_domain not in [data_domain, final_data_domain] and 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 index 378e9486a815..6aa44c8df3ca 100644 --- 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/snippet_test.exs b/test/plausible/site/verification/checks/snippet_test.exs index 5a5b8f4335b1..fdba46643f06 100644 --- 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 """ - + """ diff --git a/test/plausible/site/verification/checks_test.exs b/test/plausible/site/verification/checks_test.exs index e8c482881f56..a80d3c034bc7 100644 --- 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 """ + + + + + Hello + + """ + + 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 From 8b938e44d667e62cb36d6a41a7f24e031d85044f Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Fri, 24 May 2024 12:26:31 +0200 Subject: [PATCH 2/5] format --- lib/plausible/verification/checks/snippet.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible/verification/checks/snippet.ex b/lib/plausible/verification/checks/snippet.ex index 95e0a57c5623..bc582ca56829 100644 --- a/lib/plausible/verification/checks/snippet.ex +++ b/lib/plausible/verification/checks/snippet.ex @@ -40,7 +40,7 @@ defmodule Plausible.Verification.Checks.Snippet do "defer", "data-api", "data-exclude", - "data-include", + "data-include" ] defp unknown_attributes?(nodes) do From 99e6e22685207a8dfd7d43b61b04a8c2208c0272 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Fri, 24 May 2024 12:29:38 +0200 Subject: [PATCH 3/5] fix test --- test/plausible/site/verification/checks/fetch_body_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plausible/site/verification/checks/fetch_body_test.exs b/test/plausible/site/verification/checks/fetch_body_test.exs index 0e579ccc054e..ca8eb3e2ee94 100644 --- 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 From 2c5628ad835055cb06ed7758c9f962dc1e831c7f Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Fri, 24 May 2024 12:29:59 +0200 Subject: [PATCH 4/5] remove redundant pattern matching --- lib/plausible/verification/checks/fetch_body.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/plausible/verification/checks/fetch_body.ex b/lib/plausible/verification/checks/fetch_body.ex index 3527c1052e5d..27315780e13b 100644 --- a/lib/plausible/verification/checks/fetch_body.ex +++ b/lib/plausible/verification/checks/fetch_body.ex @@ -27,11 +27,11 @@ defmodule Plausible.Verification.Checks.FetchBody do {req, resp} = opts |> Req.new() |> Req.Request.run_request() case resp do - %Req.Response{status: status, body: body} = response + %Req.Response{status: status, body: body} when is_binary(body) and status in 200..299 -> state |> assign(final_domain: req.url.host) - |> extract_document(response) + |> extract_document(resp) _ -> state From 189fdd6045d8654ea60de7b3c214518016372d29 Mon Sep 17 00:00:00 2001 From: hq1 Date: Fri, 24 May 2024 12:37:12 +0200 Subject: [PATCH 5/5] Update lib/plausible/verification/checks/snippet.ex Co-authored-by: Adrian Gruntkowski --- lib/plausible/verification/checks/snippet.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/plausible/verification/checks/snippet.ex b/lib/plausible/verification/checks/snippet.ex index bc582ca56829..31da18523e48 100644 --- a/lib/plausible/verification/checks/snippet.ex +++ b/lib/plausible/verification/checks/snippet.ex @@ -59,8 +59,7 @@ defmodule Plausible.Verification.Checks.Snippet do |> Enum.any?(fn script_data_domain -> multiple = String.split(script_data_domain, ",") - script_data_domain not in [data_domain, final_data_domain] and data_domain not in multiple and - final_data_domain not in multiple + data_domain not in multiple and final_data_domain not in multiple end) end end