From 2ac2e957d1a75aa2e7cd09451d0f1e95e82a8650 Mon Sep 17 00:00:00 2001 From: Radek Janicki <13463966+rdk08@users.noreply.github.com> Date: Mon, 17 Apr 2023 18:15:47 +0200 Subject: [PATCH 1/5] Do not reject events from headless Chrome on staging environment --- config/runtime.exs | 1 + .../controllers/api/external_controller.ex | 9 +++++-- .../api/external_controller_test.exs | 24 ++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/config/runtime.exs b/config/runtime.exs index bf9ce9084e8c..6edc270b1827 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -189,6 +189,7 @@ config :plausible, admin_email: admin_email, admin_pwd: admin_pwd, environment: env, + system_environment: env, mailer_email: mailer_email, super_admin_user_ids: super_admin_user_ids, site_limit: site_limit, diff --git a/lib/plausible_web/controllers/api/external_controller.ex b/lib/plausible_web/controllers/api/external_controller.ex index f2ebc627ef61..cd63c70a88ab 100644 --- a/lib/plausible_web/controllers/api/external_controller.ex +++ b/lib/plausible_web/controllers/api/external_controller.ex @@ -167,8 +167,13 @@ defmodule PlausibleWeb.Api.ExternalController do defp is_bot?(%UAInspector.Result.Bot{}), do: true - defp is_bot?(%UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}}), - do: true + defp is_bot?(%UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}}) do + if Application.get_env(:plausible, :system_environment) == "staging" do + false + else + true + end + end defp is_bot?(_), do: false diff --git a/test/plausible_web/controllers/api/external_controller_test.exs b/test/plausible_web/controllers/api/external_controller_test.exs index 10a6248eed82..ed1334140be4 100644 --- a/test/plausible_web/controllers/api/external_controller_test.exs +++ b/test/plausible_web/controllers/api/external_controller_test.exs @@ -169,7 +169,29 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do ) |> post("/api/event", params) - assert get_event("headless-chrome-test.com") == nil + refute get_event("headless-chrome-test.com") + end + + test "Headless Chrome is not ignored on staging environment", %{conn: conn} do + initial_env = Application.get_env(:plausible, :system_environment) + Application.put_env(:plausible, :system_environment, "staging") + + params = %{ + name: "pageview", + url: "http://www.example.com/", + domain: "headless-chrome-stg-test.com" + } + + conn + |> put_req_header( + "user-agent", + "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/85.0.4183.83 Safari/537.36" + ) + |> post("/api/event", params) + + assert get_event("headless-chrome-stg-test.com") + + Application.put_env(:plausible, :system_environment, initial_env) end test "parses user_agent", %{conn: conn} do From d56e8dd8f1a34c3cc88dbae4a05c589c0bdf3521 Mon Sep 17 00:00:00 2001 From: Radek Janicki <13463966+rdk08@users.noreply.github.com> Date: Mon, 17 Apr 2023 18:26:42 +0200 Subject: [PATCH 2/5] Correct failing tests --- .../api/external_controller_test.exs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/test/plausible_web/controllers/api/external_controller_test.exs b/test/plausible_web/controllers/api/external_controller_test.exs index ed1334140be4..9bb3fe2a9617 100644 --- a/test/plausible_web/controllers/api/external_controller_test.exs +++ b/test/plausible_web/controllers/api/external_controller_test.exs @@ -498,11 +498,11 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do url: "http://gigride.live/", domain: "special-props-test.com", props: %{ - campaign_id: 8, + campaign_id: "vendor-8", company_id: 10, job_id: 12, page_id: 15, - product_id: 11, + product_id: "vendor-11", site_id: 9, careers_application_form_uuid: "313a26c2-741c-421c-9a6b-f39c02c8d35c" } @@ -513,8 +513,8 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do event = get_event("special-props-test.com") - assert Map.get(event, :campaign_id) == 8 - assert Map.get(event, :product_id) == 11 + assert Map.get(event, :campaign_id) == "vendor-8" + assert Map.get(event, :product_id) == "vendor-11" assert Map.get(event, :company_id) == 10 assert Map.get(event, :job_id) == 12 assert Map.get(event, :page_id) == 15 @@ -900,9 +900,7 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do |> post("/api/event", params) assert json_response(conn, 400) == %{ - "errors" => %{ - "hostname" => ["can't be blank"] - } + "errors" => %{"request" => "Unable to process request"} } end @@ -919,9 +917,7 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do |> post("/api/event", params) assert json_response(conn, 400) == %{ - "errors" => %{ - "domain" => ["can't be blank"] - } + "errors" => %{"request" => "Unable to process request"} } end end @@ -1099,11 +1095,12 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do assert pageview.hostname == "liipgellkffekalgefpjolodblggkmjg" end - describe "GET /api/health" do - test "returns 200 OK", %{conn: conn} do - conn = get(conn, "/api/health") + # /api/health path is disabled in PlausibleWeb.Router module + # describe "GET /api/health" do + # test "returns 200 OK", %{conn: conn} do + # conn = get(conn, "/api/health") - assert conn.status == 200 - end - end + # assert conn.status == 200 + # end + # end end From 402e3d396268e87301aed39308ef3df367ee2327 Mon Sep 17 00:00:00 2001 From: Radek Janicki <13463966+rdk08@users.noreply.github.com> Date: Tue, 18 Apr 2023 10:26:09 +0200 Subject: [PATCH 3/5] Do not reject events from headless Chrome on release candidate environment --- lib/plausible_web/controllers/api/external_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible_web/controllers/api/external_controller.ex b/lib/plausible_web/controllers/api/external_controller.ex index cd63c70a88ab..b00bc00c0c64 100644 --- a/lib/plausible_web/controllers/api/external_controller.ex +++ b/lib/plausible_web/controllers/api/external_controller.ex @@ -168,7 +168,7 @@ defmodule PlausibleWeb.Api.ExternalController do defp is_bot?(%UAInspector.Result.Bot{}), do: true defp is_bot?(%UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}}) do - if Application.get_env(:plausible, :system_environment) == "staging" do + if Application.get_env(:plausible, :system_environment) in ["staging", "rc"] do false else true From bd43ed4f7de063d120046d8d7d7d67d29f930fb7 Mon Sep 17 00:00:00 2001 From: Radek Janicki <13463966+rdk08@users.noreply.github.com> Date: Tue, 18 Apr 2023 12:48:09 +0200 Subject: [PATCH 4/5] Refactor if statement --- lib/plausible_web/controllers/api/external_controller.ex | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/plausible_web/controllers/api/external_controller.ex b/lib/plausible_web/controllers/api/external_controller.ex index b00bc00c0c64..db17f2d9e9ec 100644 --- a/lib/plausible_web/controllers/api/external_controller.ex +++ b/lib/plausible_web/controllers/api/external_controller.ex @@ -168,11 +168,7 @@ defmodule PlausibleWeb.Api.ExternalController do defp is_bot?(%UAInspector.Result.Bot{}), do: true defp is_bot?(%UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}}) do - if Application.get_env(:plausible, :system_environment) in ["staging", "rc"] do - false - else - true - end + Application.get_env(:plausible, :system_environment) not in ["staging", "rc"] end defp is_bot?(_), do: false From b74f1c1700a44e237c4d69ceba2158f4b612e559 Mon Sep 17 00:00:00 2001 From: Radek Janicki <13463966+rdk08@users.noreply.github.com> Date: Tue, 18 Apr 2023 14:06:17 +0200 Subject: [PATCH 5/5] Adjust order --- lib/plausible_web/controllers/api/external_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible_web/controllers/api/external_controller.ex b/lib/plausible_web/controllers/api/external_controller.ex index db17f2d9e9ec..1ed85120eed3 100644 --- a/lib/plausible_web/controllers/api/external_controller.ex +++ b/lib/plausible_web/controllers/api/external_controller.ex @@ -168,7 +168,7 @@ defmodule PlausibleWeb.Api.ExternalController do defp is_bot?(%UAInspector.Result.Bot{}), do: true defp is_bot?(%UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}}) do - Application.get_env(:plausible, :system_environment) not in ["staging", "rc"] + Application.get_env(:plausible, :system_environment) not in ["rc", "staging"] end defp is_bot?(_), do: false