fix(guard): avoid login lookup failures without RHA

Makefile

@@ -42,7 +42,7 @@ endif
 # Locally we want to bind volumes we're working on.
 # On CI environment this is not necessary and would only slow us down. The data is already on the host.
 #
-DOCKER_COMPOSE_OPTS=-f docker-compose.yml
+DOCKER_COMPOSE_OPTS :=
 ifeq ($(CI),)
 	VOLUME_BIND?=--volume $(PWD):/app
 	export BUILDKIT_INLINE_CACHE=0

guard/lib/guard/api/gitlab.ex

@@ -50,11 +50,18 @@ defmodule Guard.Api.Gitlab do
       {:ok, %Tesla.Env{status: status, body: body}} when status in 200..299 ->
         OAuth.handle_ok_token_response(repo_host_account, body)
 
-      {:ok, %Tesla.Env{status: status}} when status in 400..499 ->
-        Logger.warning("Failed to refresh gitlab token, account might be revoked")
+      {:ok, %Tesla.Env{status: status, body: body}} when status in 400..499 ->
+        Logger.warning(
+          "Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}"
+        )
+
         {:error, :revoked}
 
-      {:ok, %Tesla.Env{status: _status}} ->
+      {:ok, %Tesla.Env{status: _status, body: body}} ->
+        Logger.debug(
+          "Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}"
+        )
+
         {:error, :failed}
 
       {:error, error} ->

guard/lib/guard/invitees.ex

@@ -22,8 +22,8 @@ defmodule Guard.Invitees do
     end
   end
 
-  def inject_github_uid(invitee, inviter_id) do
-    case extract_github_uid(invitee.provider.login, inviter_id) do
+  defp inject_github_uid(invitee, inviter_id) do
+    case extract_provider_uid(invitee.provider.login, inviter_id, "github") do
       {:ok, uid} ->
         provider = Map.merge(invitee.provider, %{uid: uid})
         {:ok, Map.merge(invitee, %{provider: provider})}
@@ -33,8 +33,8 @@ defmodule Guard.Invitees do
     end
   end
 
-  def inject_gitlab_uid(invitee, inviter_id) do
-    case extract_gitlab_uid(invitee.provider.login, inviter_id) do
+  defp inject_gitlab_uid(invitee, inviter_id) do
+    case extract_provider_uid(invitee.provider.login, inviter_id, "gitlab") do
       {:ok, uid} ->
         provider = Map.merge(invitee.provider, %{uid: uid})
         {:ok, Map.merge(invitee, %{provider: provider})}
@@ -44,51 +44,31 @@ defmodule Guard.Invitees do
     end
   end
 
-  defp extract_github_uid(login, inviter) do
-    case get_github_uid(login) do
+  defp extract_provider_uid(login, inviter, provider) do
+    case get_provider_uid(login, provider) do
       {:ok, uid} when not is_nil(uid) ->
         {:ok, uid}
 
       _ ->
-        extract_uid_from_github(login, inviter)
+        extract_uid_from_provider(login, inviter, provider)
     end
   end
 
-  defp extract_gitlab_uid(login, inviter) do
-    case get_gitlab_uid(login) do
-      {:ok, uid} when not is_nil(uid) ->
-        {:ok, uid}
-
-      _ ->
-        extract_uid_from_gitlab(login, inviter)
-    end
-  end
-
-  defp extract_uid_from_github("", _inviter_id), do: {:error, "empty login not allowed"}
+  defp extract_uid_from_provider("", _inviter_id, _), do: {:error, "empty login not allowed"}
 
-  defp extract_uid_from_github(login, inviter_id) do
-    with {:ok, resource} <- resource(login, "github"),
-         {:ok, api_token} <- get_api_token(inviter_id, "github"),
-         {:ok, http_response} <- http_call(resource, api_token, "github") do
-      extract_uid(login, http_response)
+  defp extract_uid_from_provider(login, inviter_id, provider) do
+    with {:ok, resource} <- resource(login, provider),
+         {:ok, rha} <- maybe_get_rha(inviter_id, provider),
+         {:ok, api_token} <- maybe_get_api_token(rha, provider),
+         {:ok, http_response} <- http_call(resource, api_token, provider) do
+      extract_uid(login, http_response, provider)
     else
       e ->
-        Logger.error("Error extracting github uid for #{inspect(login)}: #{inspect(e)}")
-        {:error, "error finding Github ID for #{login}"}
-    end
-  end
-
-  defp extract_uid_from_gitlab("", _inviter_id), do: {:error, "empty login not allowed"}
+        Logger.error(
+          "[Invitees] Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}"
+        )
 
-  defp extract_uid_from_gitlab(login, inviter_id) do
-    with {:ok, resource} <- resource(login, "gitlab"),
-         {:ok, api_token} <- get_api_token(inviter_id, "gitlab"),
-         {:ok, http_response} <- http_call(resource, api_token, "gitlab") do
-      extract_uid(login, http_response)
-    else
-      e ->
-        Logger.error("Error extracting gitlab uid for #{inspect(login)}: #{inspect(e)}")
-        {:error, "error finding Gitlab ID for #{login}"}
+        {:error, "error finding #{provider} ID for #{login}"}
     end
   end
 
@@ -100,45 +80,66 @@ defmodule Guard.Invitees do
     |> return_ok_tuple()
   end
 
-  defp get_api_token(inviter_id, "github") do
-    Guard.FrontRepo.RepoHostAccount.get_github_token(inviter_id)
-  end
+  defp maybe_get_rha(inviter_id, provider) do
+    case Guard.FrontRepo.RepoHostAccount.get_for_user_by_repo_host(inviter_id, provider) do
+      {:ok, rha} ->
+        {:ok, rha}
 
-  defp get_api_token(inviter_id, "gitlab") do
-    case Guard.FrontRepo.RepoHostAccount.get_gitlab_token(inviter_id) do
-      {:ok, {token, _expires_at}} -> {:ok, token}
-      _ -> {:error, "error finding Gitlab token for #{inviter_id}"}
+      e ->
+        Logger.warning(
+          "[Invitees] Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}"
+        )
+
+        {:ok, nil}
     end
   end
 
-  defp get_api_token(_, provider) do
-    {:error, "Provider #{provider} not supported for token extraction"}
-  end
+  defp maybe_get_api_token(nil, _), do: {:ok, nil}
+
+  defp maybe_get_api_token(rha, provider) do
+    case get_api_token(rha, provider) do
+      {:ok, {token, _expires_at}} ->
+        {:ok, token}
+
+      e ->
+        Logger.warning(
+          "[Invitees] Missing token for rha #{inspect(rha)} and #{provider}, #{inspect(e)}"
+        )
 
-  defp get_github_uid(login) do
-    Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "github")
+        {:ok, nil}
+    end
   end
 
-  defp get_gitlab_uid(login) do
-    Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "gitlab")
+  defp get_api_token(rha, "github") do
+    Guard.FrontRepo.RepoHostAccount.get_github_token(rha)
   end
 
-  defp http_call(resource, api_token, "github") do
-    resource |> HTTPoison.get([{"Authorization", "Token #{api_token}"}])
+  defp get_api_token(_, _), do: {:ok, {"", nil}}
+
+  defp get_provider_uid(login, provider) do
+    Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, provider)
   end
 
-  defp http_call(resource, api_token, "gitlab") do
-    resource |> HTTPoison.get([{"PRIVATE-TOKEN", api_token}])
+  defp http_call(resource, token, "github") when is_binary(token) and token != "" do
+    HTTPoison.get(resource, [{"Authorization", "Token #{token}"}])
   end
 
-  defp extract_uid(_, %{status_code: 200, body: body}) do
+  defp http_call(resource, _, _), do: HTTPoison.get(resource, [])
+
+  defp extract_uid(_, %{status_code: 200, body: body}, _) do
     with {:ok, body} <- body |> Jason.decode(),
-         {:ok, id} <- body |> Map.fetch("id"),
+         {:ok, id} <- fetch_id(body),
          do: id |> Integer.to_string() |> return_ok_tuple()
   end
 
-  defp extract_uid(login, %{status_code: status_code}),
+  defp extract_uid(login, %{status_code: status_code}, _),
     do: {:error, "error finding #{login}: #{status_code}"}
 
+  defp fetch_id(%{"id" => id}), do: {:ok, id}
+
+  defp fetch_id([%{"id" => id} | _rest]), do: {:ok, id}
+
+  defp fetch_id(body), do: {:error, "error finding id in the body #{inspect(body)}"}
+
   defp return_ok_tuple(value), do: {:ok, value}
 end

guard/test/fixture/vcr_cassettes/existing_user.json

@@ -41,4 +41,4 @@
       "type": "ok"
     }
   }
-]
+]

guard/test/guard/invitees_test.exs

@@ -1,6 +1,8 @@
 defmodule Guard.InviteesTest do
   use Guard.RepoCase, async: true
-  use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
+
+  import Mock
+  import ExUnit.CaptureLog
 
   alias Guard.Invitees
 
@@ -67,21 +69,91 @@ defmodule Guard.InviteesTest do
     end
 
     test "inject uid for github provider" do
-      use_cassette "existing user" do
-        invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
+      invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
 
-        invitee_with_uid = %{
-          provider: %{login: "radwo", uid: "184065", type: :GITHUB}
-        }
+      invitee_with_uid = %{
+        provider: %{login: "radwo", uid: "184065", type: :GITHUB}
+      }
 
+      response_body = Jason.encode!(%{"id" => 184_065})
+
+      with_mocks([
+        {Guard.FrontRepo.RepoHostAccount, [:passthrough],
+         get_github_token: fn _ -> {:ok, {"token", nil}} end},
+        {HTTPoison, [:passthrough],
+         get: fn url, headers ->
+           assert url == "https://api.github.com/users/radwo"
+           assert headers == [{"Authorization", "Token token"}]
+           {:ok, %{status_code: 200, body: response_body}}
+         end}
+      ]) do
         assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
       end
     end
 
-    test "return error for unknown github user" do
-      use_cassette "unknown user" do
-        invitee = %{provider: %{login: "unknown331123", uid: "", type: :GITHUB}}
+    test "falls back to github api without inviter token" do
+      Guard.FrontRepo.delete_all(Guard.FrontRepo.RepoHostAccount)
+
+      invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
+      response_body = Jason.encode!(%{"id" => 123})
+
+      log =
+        capture_log(fn ->
+          with_mock HTTPoison, [:passthrough],
+            get: fn url, headers ->
+              assert url == "https://api.github.com/users/radwo"
+              assert headers == []
+              {:ok, %{status_code: 200, body: response_body}}
+            end do
+            invitee_with_uid = %{
+              provider: %{login: "radwo", uid: "123", type: :GITHUB}
+            }
+
+            assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
+          end
+        end)
+
+      assert log =~ "Missing RHA for inviter"
+    end
+
+    test "falls back to gitlab api without inviter token" do
+      Guard.FrontRepo.delete_all(Guard.FrontRepo.RepoHostAccount)
 
+      invitee = %{provider: %{login: "radwo_gitlab", uid: "", type: :GITLAB}}
+      response_body = Jason.encode!(%{"id" => 456})
+
+      log =
+        capture_log(fn ->
+          with_mock HTTPoison, [:passthrough],
+            get: fn url, headers ->
+              assert url == "https://gitlab.com/api/v4/users?username=radwo_gitlab"
+              assert headers == []
+              {:ok, %{status_code: 200, body: response_body}}
+            end do
+            invitee_with_uid = %{
+              provider: %{login: "radwo_gitlab", uid: "456", type: :GITLAB}
+            }
+
+            assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
+          end
+        end)
+
+      assert log =~ "Missing RHA for inviter"
+    end
+
+    test "return error for unknown github user" do
+      invitee = %{provider: %{login: "unknown331123", uid: "", type: :GITHUB}}
+
+      with_mocks([
+        {Guard.FrontRepo.RepoHostAccount, [:passthrough],
+         get_github_token: fn _ -> {:ok, {"token", nil}} end},
+        {HTTPoison, [:passthrough],
+         get: fn url, headers ->
+           assert url == "https://api.github.com/users/unknown331123"
+           assert headers == [{"Authorization", "Token token"}]
+           {:ok, %{status_code: 404}}
+         end}
+      ]) do
         assert Invitees.inject_provider_uid(invitee, @inviter_id) ==
                  {:error, "error finding unknown331123: 404"}
       end