From 8e3ba067a72c8aa3ac2bb2c17ce71341449b5dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Fri, 7 Nov 2025 15:17:31 +0100 Subject: [PATCH 1/7] fix(guard): avoid login lookup failures without RHA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invitation occasionally fails because we can’t fetch the inviter’s repo host account (RHA), so token lookup crashes the flow. Since these calls hit public endpoints, we now proceed without a token when the RHA is missing, keeping invitations functional. GitLab integration was also fixed. Previously never worked because it hit the wrong endpoint/token path. --- Makefile | 2 +- guard/lib/guard/invitees.ex | 109 +++++++++--------- .../fixture/vcr_cassettes/existing_user.json | 2 +- guard/test/guard/invitees_test.exs | 90 +++++++++++++-- 4 files changed, 135 insertions(+), 68 deletions(-) diff --git a/Makefile b/Makefile index e1573471a..50e4e2114 100644 --- a/Makefile +++ b/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 diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index fc5371a3e..121c44118 100644 --- a/guard/lib/guard/invitees.ex +++ b/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,28 @@ 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"} - - 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}"} + Logger.error("Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}") + {:error, "error finding #{provider} ID for #{login}"} end end @@ -100,44 +77,62 @@ defmodule Guard.Invitees do |> return_ok_tuple() end - defp get_api_token(inviter_id, "github") do - Guard.FrontRepo.RepoHostAccount.get_github_token(inviter_id) + 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} + e -> + Logger.warning( + "Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}" + ) + {:ok, nil} + end end - 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}"} + 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( + "Missing token for rha #{rha} and #{provider}, #{inspect(e)}" + ) + {:ok, nil} end end - defp get_api_token(_, provider) do - {:error, "Provider #{provider} not supported for token extraction"} + defp get_api_token(rha, "github") do + Guard.FrontRepo.RepoHostAccount.get_github_token(rha) end - defp get_github_uid(login) do - Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "github") + defp get_api_token(rha, "gitlab") do + Guard.FrontRepo.RepoHostAccount.get_gitlab_token(rha) end - defp get_gitlab_uid(login) do - Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "gitlab") + defp get_provider_uid(login, provider) do + Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, provider) end - defp http_call(resource, api_token, "github") do - resource |> HTTPoison.get([{"Authorization", "Token #{api_token}"}]) + defp http_call(resource, token, "github") when is_binary(token) and token != "" do + HTTPoison.get(resource, [{"Authorization", "Token #{token}"}]) end - defp http_call(resource, api_token, "gitlab") do - resource |> HTTPoison.get([{"PRIVATE-TOKEN", api_token}]) + defp http_call(resource, token, "gitlab") when is_binary(token) and token != "" do + HTTPoison.get(resource, [{"PRIVATE-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"), 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 return_ok_tuple(value), do: {:ok, value} diff --git a/guard/test/fixture/vcr_cassettes/existing_user.json b/guard/test/fixture/vcr_cassettes/existing_user.json index cdf614b40..003bac88c 100644 --- a/guard/test/fixture/vcr_cassettes/existing_user.json +++ b/guard/test/fixture/vcr_cassettes/existing_user.json @@ -41,4 +41,4 @@ "type": "ok" } } -] \ No newline at end of file +] diff --git a/guard/test/guard/invitees_test.exs b/guard/test/guard/invitees_test.exs index 61afb7a21..b76577cea 100644 --- a/guard/test/guard/invitees_test.exs +++ b/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 From 38c9c0384d88db8a8d4eb2bc8636557d73a9af90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Fri, 7 Nov 2025 15:34:55 +0100 Subject: [PATCH 2/7] reformat the code --- guard/lib/guard/invitees.ex | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index 121c44118..6023ba476 100644 --- a/guard/lib/guard/invitees.ex +++ b/guard/lib/guard/invitees.ex @@ -81,10 +81,9 @@ defmodule Guard.Invitees do case Guard.FrontRepo.RepoHostAccount.get_for_user_by_repo_host(inviter_id, provider) do {:ok, rha} -> {:ok, rha} + e -> - Logger.warning( - "Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}" - ) + Logger.warning("Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}") {:ok, nil} end end @@ -97,9 +96,7 @@ defmodule Guard.Invitees do {:ok, token} e -> - Logger.warning( - "Missing token for rha #{rha} and #{provider}, #{inspect(e)}" - ) + Logger.warning("Missing token for rha #{rha} and #{provider}, #{inspect(e)}") {:ok, nil} end end From e889d4af0cd378e0a36a055fb05c79adf19e65b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Fri, 7 Nov 2025 16:55:08 +0100 Subject: [PATCH 3/7] update logger messages --- guard/lib/guard/invitees.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index 6023ba476..ab2c53f36 100644 --- a/guard/lib/guard/invitees.ex +++ b/guard/lib/guard/invitees.ex @@ -64,7 +64,7 @@ defmodule Guard.Invitees do extract_uid(login, http_response, provider) else e -> - Logger.error("Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}") + Logger.error("[Invitees] Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}") {:error, "error finding #{provider} ID for #{login}"} end end @@ -83,7 +83,7 @@ defmodule Guard.Invitees do {:ok, rha} e -> - Logger.warning("Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}") + Logger.warning("[Invitees] Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}") {:ok, nil} end end @@ -96,7 +96,7 @@ defmodule Guard.Invitees do {:ok, token} e -> - Logger.warning("Missing token for rha #{rha} and #{provider}, #{inspect(e)}") + Logger.warning("[Invitees] Missing token for rha #{inspect(rha)} and #{provider}, #{inspect(e)}") {:ok, nil} end end From 68f34fea0c5c663fe0dd17cd307bc5efc5a5196f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Fri, 7 Nov 2025 16:55:35 +0100 Subject: [PATCH 4/7] reformat the code --- guard/lib/guard/invitees.ex | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index ab2c53f36..44c17320f 100644 --- a/guard/lib/guard/invitees.ex +++ b/guard/lib/guard/invitees.ex @@ -64,7 +64,10 @@ defmodule Guard.Invitees do extract_uid(login, http_response, provider) else e -> - Logger.error("[Invitees] Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}") + Logger.error( + "[Invitees] Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}" + ) + {:error, "error finding #{provider} ID for #{login}"} end end @@ -83,7 +86,10 @@ defmodule Guard.Invitees do {:ok, rha} e -> - Logger.warning("[Invitees] Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}") + Logger.warning( + "[Invitees] Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}" + ) + {:ok, nil} end end @@ -96,7 +102,10 @@ defmodule Guard.Invitees do {:ok, token} e -> - Logger.warning("[Invitees] Missing token for rha #{inspect(rha)} and #{provider}, #{inspect(e)}") + Logger.warning( + "[Invitees] Missing token for rha #{inspect(rha)} and #{provider}, #{inspect(e)}" + ) + {:ok, nil} end end From 811098af8927af4ec2380f17de87dc4ff5f34ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Sat, 8 Nov 2025 19:24:55 +0100 Subject: [PATCH 5/7] update debug msg --- guard/lib/guard/api/gitlab.ex | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/guard/lib/guard/api/gitlab.ex b/guard/lib/guard/api/gitlab.ex index 6c83b6f01..47c90f387 100644 --- a/guard/lib/guard/api/gitlab.ex +++ b/guard/lib/guard/api/gitlab.ex @@ -50,11 +50,12 @@ 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} -> From 19f8fe8f1cac48f1e2f92135a3d6dd2421ffbfe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:21:41 +0100 Subject: [PATCH 6/7] fetch users from gitlab without a token --- guard/lib/guard/api/gitlab.ex | 10 ++++++++-- guard/lib/guard/invitees.ex | 8 +------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/guard/lib/guard/api/gitlab.ex b/guard/lib/guard/api/gitlab.ex index 47c90f387..0b0dd825d 100644 --- a/guard/lib/guard/api/gitlab.ex +++ b/guard/lib/guard/api/gitlab.ex @@ -51,11 +51,17 @@ defmodule Guard.Api.Gitlab do OAuth.handle_ok_token_response(repo_host_account, body) {: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)}") + Logger.warning( + "Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}" + ) + {:error, :revoked} {:ok, %Tesla.Env{status: _status, body: body}} -> - Logger.debug("Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}") + Logger.debug( + "Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}" + ) + {:error, :failed} {:error, error} -> diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index 44c17320f..239f95d44 100644 --- a/guard/lib/guard/invitees.ex +++ b/guard/lib/guard/invitees.ex @@ -114,9 +114,7 @@ defmodule Guard.Invitees do Guard.FrontRepo.RepoHostAccount.get_github_token(rha) end - defp get_api_token(rha, "gitlab") do - Guard.FrontRepo.RepoHostAccount.get_gitlab_token(rha) - end + defp get_api_token(_, _), do: {:ok, {"", nil}} defp get_provider_uid(login, provider) do Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, provider) @@ -126,10 +124,6 @@ defmodule Guard.Invitees do HTTPoison.get(resource, [{"Authorization", "Token #{token}"}]) end - defp http_call(resource, token, "gitlab") when is_binary(token) and token != "" do - HTTPoison.get(resource, [{"PRIVATE-TOKEN", token}]) - end - defp http_call(resource, _, _), do: HTTPoison.get(resource, []) defp extract_uid(_, %{status_code: 200, body: body}, _) do From 15046a09f6870136f8fd98517f09546817f4c4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wo=C5=BAniak?= <184065+radwo@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:58:59 +0100 Subject: [PATCH 7/7] gitlab returns list --- guard/lib/guard/invitees.ex | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index 239f95d44..40f4399c9 100644 --- a/guard/lib/guard/invitees.ex +++ b/guard/lib/guard/invitees.ex @@ -128,12 +128,18 @@ defmodule Guard.Invitees do 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}, _), 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