Handle GA authentication errors (#2505)

lib/plausible/google/http.ex

@@ -135,17 +135,16 @@ defmodule Plausible.Google.HTTP do
 
     headers = [{"Authorization", "Bearer #{access_token}"}]
 
-    case HTTPClient.get(url, headers) do
+    case HTTPClient.impl().get(url, headers) do
       {:ok, %Finch.Response{body: body, status: 200}} ->
         {:ok, body}
 
-      {:error, %{reason: %Finch.Response{body: body}}} ->
-        Sentry.capture_message("Error fetching Google view ID", extra: %{body: inspect(body)})
-        {:error, body}
+      {:error, %HTTPClient.Non200Error{} = error} when error.reason.status in [401, 403] ->
+        {:error, :authentication_failed}
 
-      {:error, %{reason: reason} = e} ->
-        Sentry.capture_message("Error fetching Google view ID", extra: %{error: inspect(e)})
-        {:error, reason}
+      {:error, %HTTPClient.Non200Error{} = error} ->
+        Sentry.capture_message("Error listing GA views for user", extra: %{error: error})
+        {:error, :unknown}
     end
   end
 

lib/plausible_web/controllers/auth_controller.ex

@@ -561,6 +561,39 @@ defmodule PlausibleWeb.AuthController do
     |> redirect(to: redirect_to)
   end
 
+  def google_auth_callback(conn, %{"error" => error, "state" => state} = params) do
+    [site_id, _redirect_to] = Jason.decode!(state)
+    site = Repo.get(Plausible.Site, site_id)
+
+    case error do
+      "access_denied" ->
+        conn
+        |> put_flash(
+          :error,
+          "We were unable to authenticate your Google Analytics account. Please check that you have granted us permission to 'See and download your Google Analytics data' and try again."
+        )
+        |> redirect(to: Routes.site_path(conn, :settings_general, site.domain))
+
+      message when message in ["server_error", "temporarily_unavailable"] ->
+        conn
+        |> put_flash(
+          :error,
+          "We are unable to authenticate your Google Analytics account because Google's authentication service is temporarily unavailable. Please try again in a few moments."
+        )
+        |> redirect(to: Routes.site_path(conn, :settings_general, site.domain))
+
+      _any ->
+        Sentry.capture_message("Google OAuth callback failed. Reason: #{inspect(params)}")
+
+        conn
+        |> put_flash(
+          :error,
+          "We were unable to authenticate your Google Analytics account. If the problem persists, please contact support for assistance."
+        )
+        |> redirect(to: Routes.site_path(conn, :settings_general, site.domain))
+    end
+  end
+
   def google_auth_callback(conn, %{"code" => code, "state" => state}) do
     res = Plausible.Google.HTTP.fetch_access_token(code)
     [site_id, redirect_to] = Jason.decode!(state)

lib/plausible_web/controllers/site_controller.ex

@@ -686,19 +686,35 @@ defmodule PlausibleWeb.SiteController do
         "refresh_token" => refresh_token,
         "expires_at" => expires_at
       }) do
-    site = conn.assigns[:site]
-    view_ids = Plausible.Google.Api.list_views(access_token)
+    case Plausible.Google.Api.list_views(access_token) do
+      {:ok, view_ids} ->
+        conn
+        |> assign(:skip_plausible_tracking, true)
+        |> render("import_from_google_view_id_form.html",
+          access_token: access_token,
+          refresh_token: refresh_token,
+          expires_at: expires_at,
+          site: conn.assigns.site,
+          view_ids: view_ids,
+          layout: {PlausibleWeb.LayoutView, "focus.html"}
+        )
 
-    conn
-    |> assign(:skip_plausible_tracking, true)
-    |> render("import_from_google_view_id_form.html",
-      access_token: access_token,
-      refresh_token: refresh_token,
-      expires_at: expires_at,
-      site: site,
-      view_ids: view_ids,
-      layout: {PlausibleWeb.LayoutView, "focus.html"}
-    )
+      {:error, :authentication_failed} ->
+        conn
+        |> put_flash(
+          :error,
+          "We were unable to authenticate your Google Analytics account. Please check that you have granted us permission to 'See and download your Google Analytics data' and try again."
+        )
+        |> redirect(to: Routes.site_path(conn, :settings_general, conn.assigns.site.domain))
+
+      {:error, _any} ->
+        conn
+        |> put_flash(
+          :error,
+          "We were unable to list your Google Analytics properties. If the problem persists, please contact support for assistance."
+        )
+        |> redirect(to: Routes.site_path(conn, :settings_general, conn.assigns.site.domain))
+    end
   end
 
   # see https://stackoverflow.com/a/57416769
@@ -715,7 +731,7 @@ defmodule PlausibleWeb.SiteController do
     case start_date do
       {:ok, nil} ->
         site = conn.assigns[:site]
-        view_ids = Plausible.Google.Api.list_views(access_token)
+        {:ok, view_ids} = Plausible.Google.Api.list_views(access_token)
 
         conn
         |> assign(:skip_plausible_tracking, true)

lib/plausible_web/templates/site/import_from_google_view_id_form.html.eex

@@ -5,22 +5,15 @@
   <%= hidden_input(f, :refresh_token, value: @refresh_token) %>
   <%= hidden_input(f, :expires_at, value: @expires_at) %>
 
-  <%= case @view_ids do %>
-    <% {:ok, view_ids} -> %>
+  <div class="mt-6 text-sm text-gray-500 dark:text-gray-200">
+    Choose the view in your Google Analytics account that will be imported to the <%= @site.domain %> dashboard.
+  </div>
 
-      <div class="mt-6 text-sm text-gray-500 dark:text-gray-200">
-        Choose the view in your Google Analytics account that will be imported to the <%= @site.domain %> dashboard.
-      </div>
+  <div class="mt-3">
+    <%= styled_label(f, :view_id, "Google Analytics view") %>
+    <%= styled_select f, :view_id, @view_ids, prompt: "(Choose view)", required: "true" %>
+    <%= styled_error(@conn.assigns[:selected_view_id_error]) %>
+  </div>
 
-      <div class="mt-3">
-        <%= styled_label(f, :view_id, "Google Analytics view") %>
-        <%= styled_select f, :view_id, view_ids, prompt: "(Choose view)", required: "true" %>
-        <%= styled_error(@conn.assigns[:selected_view_id_error]) %>
-      </div>
-    <% {:error, error} -> %>
-      <p class="text-gray-700 dark:text-gray-300 mt-6">The following error occurred when fetching your Google Analytics view ids.</p>
-      <p class="text-red-700 font-medium mt-3"><%= error %></p>
-    <% end %>
-
-    <%= submit "Continue ->", class: "button mt-6" %>
+  <%= submit "Continue ->", class: "button mt-6" %>
 <% end %>

test/plausible/google/api_test.exs

@@ -344,4 +344,60 @@ defmodule Plausible.Google.ApiTest do
       end
     end
   end
+
+  test "list_views/1 returns view IDs grouped by hostname" do
+    expect(
+      Plausible.HTTPClient.Mock,
+      :get,
+      fn url, _headers ->
+        assert url ==
+                 "https://www.googleapis.com/analytics/v3/management/accounts/~all/webproperties/~all/profiles"
+
+        response = "fixture/ga_list_views.json" |> File.read!() |> Jason.decode!()
+        {:ok, %Finch.Response{status: 200, body: response}}
+      end
+    )
+
+    assert {:ok,
+            %{
+              "one.test" => [{"57238190 - one.test", "57238190"}],
+              "two.test" => [{"54460083 - two.test", "54460083"}]
+            }} == Plausible.Google.Api.list_views("access_token")
+  end
+
+  test "list_views/1 returns authentication_failed when request fails with HTTP 403" do
+    expect(
+      Plausible.HTTPClient.Mock,
+      :get,
+      fn _url, _headers ->
+        {:error, %Plausible.HTTPClient.Non200Error{reason: %{status: 403, body: %{}}}}
+      end
+    )
+
+    assert {:error, :authentication_failed} == Plausible.Google.Api.list_views("access_token")
+  end
+
+  test "list_views/1 returns authentication_failed when request fails with HTTP 401" do
+    expect(
+      Plausible.HTTPClient.Mock,
+      :get,
+      fn _url, _headers ->
+        {:error, %Plausible.HTTPClient.Non200Error{reason: %{status: 401, body: %{}}}}
+      end
+    )
+
+    assert {:error, :authentication_failed} == Plausible.Google.Api.list_views("access_token")
+  end
+
+  test "list_views/1 returns error when request fails with HTTP 500" do
+    expect(
+      Plausible.HTTPClient.Mock,
+      :get,
+      fn _url, _headers ->
+        {:error, %Plausible.HTTPClient.Non200Error{reason: %{status: 500, body: "server error"}}}
+      end
+    )
+
+    assert {:error, :unknown} == Plausible.Google.Api.list_views("access_token")
+  end
 end

test/plausible_web/controllers/auth_controller_test.exs

@@ -759,6 +759,17 @@ defmodule PlausibleWeb.AuthControllerTest do
     end
   end
 
+  describe "GET /auth/google/callback" do
+    test "shows error and redirects back to settings when authentication fails", %{conn: conn} do
+      site = insert(:site)
+      callback_params = %{"error" => "access_denied", "state" => "[#{site.id},\"import\"]"}
+      conn = get(conn, Routes.auth_path(conn, :google_auth_callback), callback_params)
+
+      assert redirected_to(conn, 302) == Routes.site_path(conn, :settings_general, site.domain)
+      assert get_flash(conn, :error) =~ "unable to authenticate your Google Analytics"
+    end
+  end
+
   defp mock_captcha_success() do
     mock_captcha(true)
   end

test/plausible_web/controllers/site_controller_test.exs

@@ -1013,26 +1013,15 @@ defmodule PlausibleWeb.SiteControllerTest do
     setup [:create_user, :log_in, :create_new_site]
 
     test "lists Google Analytics views", %{conn: conn, site: site} do
-      bypass = Bypass.open()
-
-      Bypass.expect_once(
-        bypass,
-        "GET",
-        "/analytics/v3/management/accounts/~all/webproperties/~all/profiles",
-        fn conn ->
-          response_body = File.read!("fixture/ga_list_views.json")
-
-          conn
-          |> Plug.Conn.put_resp_header("content-type", "application/json")
-          |> Plug.Conn.resp(200, response_body)
+      expect(
+        Plausible.HTTPClient.Mock,
+        :get,
+        fn _url, _body ->
+          body = "fixture/ga_list_views.json" |> File.read!() |> Jason.decode!()
+          {:ok, %Finch.Response{body: body, status: 200}}
         end
       )
 
-      :plausible
-      |> Application.get_env(:google)
-      |> Keyword.put(:api_url, "http://0.0.0.0:#{bypass.port}")
-      |> then(&Application.put_env(:plausible, :google, &1))
-
       response =
         conn
         |> get("/#{site.domain}/import/google-analytics/view-id", %{