Skip to content

Commit

Permalink
Verification tweaks (plausible#4234)
Browse files Browse the repository at this point in the history
* Prioritize CSP over GTM error in case both detected

* Outline better message for persistent headless timeout

* Bump headless waiting times slightly

* Add generic type of error

For known cases, that we rather not reveal much
details about.

* Add the malformed script tag case
  • Loading branch information
aerosol authored Jun 18, 2024
1 parent 2eed1fd commit 1c820ba
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 8 deletions.
34 changes: 33 additions & 1 deletion lib/plausible/verification/diagnostics.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ defmodule Plausible.Verification.Diagnostics do
%Result{ok?: true}
end

def interpret(
%__MODULE__{plausible_installed?: false, gtm_likely?: true, disallowed_via_csp?: true},
_url
) do
error(@errors.csp)
end

def interpret(
%__MODULE__{plausible_installed?: false, gtm_likely?: true, cookie_banner_likely?: true},
_url
Expand Down Expand Up @@ -87,6 +94,21 @@ defmodule Plausible.Verification.Diagnostics do
error(@errors.no_snippet)
end

def interpret(
%__MODULE__{
plausible_installed?: true,
snippets_found_in_head: 0,
snippets_found_in_body: 0,
body_fetched?: true,
gtm_likely?: false,
callback_status: callback_status
},
_url
)
when is_integer(callback_status) and callback_status > 202 do
error(@errors.no_snippet)
end

def interpret(
%__MODULE__{
plausible_installed?: false,
Expand All @@ -111,6 +133,16 @@ defmodule Plausible.Verification.Diagnostics do
error(@errors.unreachable)
end

def interpret(
%__MODULE__{
plausible_installed?: false,
service_error: :timeout
},
_url
) do
error(@errors.generic)
end

def interpret(
%__MODULE__{
plausible_installed?: false,
Expand Down Expand Up @@ -141,7 +173,7 @@ defmodule Plausible.Verification.Diagnostics do
},
_url
) do
error(@errors.old_script)
error(@errors.generic)
end

def interpret(
Expand Down
8 changes: 4 additions & 4 deletions lib/plausible/verification/errors.ex
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ defmodule Plausible.Verification.Errors do
url:
"https://plausible.io/docs/troubleshoot-integration#how-to-manually-check-your-integration"
},
old_script: %{
message: "We couldn't verify your website",
generic: %{
message: "We couldn't automatically verify your website",
recommendation:
"You're running an older version of our script so we cannot verify it automatically. Please update to the latest script",
"Please manually check your integration by following the instructions provided",
url:
"https://plausible.io/docs/troubleshoot-integration#are-you-using-an-older-version-of-our-script"
"https://plausible.io/docs/troubleshoot-integration#how-to-manually-check-your-integration"
},
old_script_wp_no_plugin: %{
message: "We couldn't verify your website",
Expand Down
4 changes: 2 additions & 2 deletions priv/verification/verify_plausible_installed.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default async function({ page, context }) {
await page.goto(context.url);

try {
await page.waitForFunction('window.plausible', { timeout: 4000 });
await page.waitForFunction('window.plausible', { timeout: 5000 });
await page.evaluate(() => {
window.__plausible = true;
window.plausible('verification-agent-test', {
Expand All @@ -19,7 +19,7 @@ export default async function({ page, context }) {
});

try {
await page.waitForFunction('window.plausibleCallbackResult', { timeout: 3000 });
await page.waitForFunction('window.plausibleCallbackResult', { timeout: 5000 });
const status = await page.evaluate(() => { return window.plausibleCallbackResult() });
return { data: { plausibleInstalled: true, callbackStatus: status } };
} catch ({ err, message }) {
Expand Down
63 changes: 62 additions & 1 deletion test/plausible/site/verification/checks_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,21 @@ defmodule Plausible.Verification.ChecksTest do
</html>
"""

test "disallowd via content-security-policy and GTM should make CSP a priority" do
stub_fetch_body(fn conn ->
conn
|> put_resp_header("content-security-policy", "default-src 'self' foo.local")
|> put_resp_content_type("text/html")
|> send_resp(200, @gtm_body)
end)

stub_installation(200, plausible_installed(false))

run_checks()
|> Checks.interpret_diagnostics()
|> assert_error(@errors.csp)
end

test "detecting gtm" do
stub_fetch_body(200, @gtm_body)
stub_installation(200, plausible_installed(false))
Expand Down Expand Up @@ -689,7 +704,7 @@ defmodule Plausible.Verification.ChecksTest do

run_checks()
|> Checks.interpret_diagnostics()
|> assert_error(@errors.old_script)
|> assert_error(@errors.generic)
end

test "callback handling not found for wordpress site" do
Expand Down Expand Up @@ -773,6 +788,52 @@ defmodule Plausible.Verification.ChecksTest do
|> interpret_sentry_case()
|> assert_error(@errors.old_script_wp_no_plugin)
end

test "service timeout" do
%Plausible.Verification.Diagnostics{
plausible_installed?: false,
snippets_found_in_head: 1,
snippets_found_in_body: 0,
snippet_found_after_busting_cache?: false,
snippet_unknown_attributes?: false,
disallowed_via_csp?: false,
service_error: :timeout,
body_fetched?: true,
wordpress_likely?: true,
cookie_banner_likely?: false,
gtm_likely?: false,
callback_status: 0,
proxy_likely?: true,
manual_script_extension?: false,
data_domain_mismatch?: false,
wordpress_plugin?: false
}
|> interpret_sentry_case()
|> assert_error(@errors.generic)
end

test "malformed snippet code, that headless somewhat accepts" do
%Plausible.Verification.Diagnostics{
plausible_installed?: true,
snippets_found_in_head: 0,
snippets_found_in_body: 0,
snippet_found_after_busting_cache?: false,
snippet_unknown_attributes?: false,
disallowed_via_csp?: false,
service_error: nil,
body_fetched?: true,
wordpress_likely?: false,
cookie_banner_likely?: false,
gtm_likely?: false,
callback_status: 405,
proxy_likely?: false,
manual_script_extension?: false,
data_domain_mismatch?: false,
wordpress_plugin?: false
}
|> interpret_sentry_case()
|> assert_error(@errors.no_snippet)
end
end

defp interpret_sentry_case(diagnostics) do
Expand Down

0 comments on commit 1c820ba

Please sign in to comment.