Skip to content

Commit 7047857

Browse files
authored
fix(guard): avoid login lookup failures without RHA (#702)
## 📝 Description 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. ## ✅ Checklist - [x] I have tested this change - [x] ~This change requires documentation update~
1 parent 7a35aa1 commit 7047857

File tree

5 files changed

+153
-73
lines changed

5 files changed

+153
-73
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ endif
4242
# Locally we want to bind volumes we're working on.
4343
# On CI environment this is not necessary and would only slow us down. The data is already on the host.
4444
#
45-
DOCKER_COMPOSE_OPTS=-f docker-compose.yml
45+
DOCKER_COMPOSE_OPTS :=
4646
ifeq ($(CI),)
4747
VOLUME_BIND?=--volume $(PWD):/app
4848
export BUILDKIT_INLINE_CACHE=0

guard/lib/guard/api/gitlab.ex

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,18 @@ defmodule Guard.Api.Gitlab do
5050
{:ok, %Tesla.Env{status: status, body: body}} when status in 200..299 ->
5151
OAuth.handle_ok_token_response(repo_host_account, body)
5252

53-
{:ok, %Tesla.Env{status: status}} when status in 400..499 ->
54-
Logger.warning("Failed to refresh gitlab token, account might be revoked")
53+
{:ok, %Tesla.Env{status: status, body: body}} when status in 400..499 ->
54+
Logger.warning(
55+
"Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}"
56+
)
57+
5558
{:error, :revoked}
5659

57-
{:ok, %Tesla.Env{status: _status}} ->
60+
{:ok, %Tesla.Env{status: _status, body: body}} ->
61+
Logger.debug(
62+
"Failed to refresh gitlab token for #{repo_host_account.login}, with: #{inspect(body)}"
63+
)
64+
5865
{:error, :failed}
5966

6067
{:error, error} ->

guard/lib/guard/invitees.ex

Lines changed: 60 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ defmodule Guard.Invitees do
2222
end
2323
end
2424

25-
def inject_github_uid(invitee, inviter_id) do
26-
case extract_github_uid(invitee.provider.login, inviter_id) do
25+
defp inject_github_uid(invitee, inviter_id) do
26+
case extract_provider_uid(invitee.provider.login, inviter_id, "github") do
2727
{:ok, uid} ->
2828
provider = Map.merge(invitee.provider, %{uid: uid})
2929
{:ok, Map.merge(invitee, %{provider: provider})}
@@ -33,8 +33,8 @@ defmodule Guard.Invitees do
3333
end
3434
end
3535

36-
def inject_gitlab_uid(invitee, inviter_id) do
37-
case extract_gitlab_uid(invitee.provider.login, inviter_id) do
36+
defp inject_gitlab_uid(invitee, inviter_id) do
37+
case extract_provider_uid(invitee.provider.login, inviter_id, "gitlab") do
3838
{:ok, uid} ->
3939
provider = Map.merge(invitee.provider, %{uid: uid})
4040
{:ok, Map.merge(invitee, %{provider: provider})}
@@ -44,51 +44,31 @@ defmodule Guard.Invitees do
4444
end
4545
end
4646

47-
defp extract_github_uid(login, inviter) do
48-
case get_github_uid(login) do
47+
defp extract_provider_uid(login, inviter, provider) do
48+
case get_provider_uid(login, provider) do
4949
{:ok, uid} when not is_nil(uid) ->
5050
{:ok, uid}
5151

5252
_ ->
53-
extract_uid_from_github(login, inviter)
53+
extract_uid_from_provider(login, inviter, provider)
5454
end
5555
end
5656

57-
defp extract_gitlab_uid(login, inviter) do
58-
case get_gitlab_uid(login) do
59-
{:ok, uid} when not is_nil(uid) ->
60-
{:ok, uid}
61-
62-
_ ->
63-
extract_uid_from_gitlab(login, inviter)
64-
end
65-
end
66-
67-
defp extract_uid_from_github("", _inviter_id), do: {:error, "empty login not allowed"}
57+
defp extract_uid_from_provider("", _inviter_id, _), do: {:error, "empty login not allowed"}
6858

69-
defp extract_uid_from_github(login, inviter_id) do
70-
with {:ok, resource} <- resource(login, "github"),
71-
{:ok, api_token} <- get_api_token(inviter_id, "github"),
72-
{:ok, http_response} <- http_call(resource, api_token, "github") do
73-
extract_uid(login, http_response)
59+
defp extract_uid_from_provider(login, inviter_id, provider) do
60+
with {:ok, resource} <- resource(login, provider),
61+
{:ok, rha} <- maybe_get_rha(inviter_id, provider),
62+
{:ok, api_token} <- maybe_get_api_token(rha, provider),
63+
{:ok, http_response} <- http_call(resource, api_token, provider) do
64+
extract_uid(login, http_response, provider)
7465
else
7566
e ->
76-
Logger.error("Error extracting github uid for #{inspect(login)}: #{inspect(e)}")
77-
{:error, "error finding Github ID for #{login}"}
78-
end
79-
end
80-
81-
defp extract_uid_from_gitlab("", _inviter_id), do: {:error, "empty login not allowed"}
67+
Logger.error(
68+
"[Invitees] Error extracting #{provider} uid for #{inspect(login)}: #{inspect(e)}"
69+
)
8270

83-
defp extract_uid_from_gitlab(login, inviter_id) do
84-
with {:ok, resource} <- resource(login, "gitlab"),
85-
{:ok, api_token} <- get_api_token(inviter_id, "gitlab"),
86-
{:ok, http_response} <- http_call(resource, api_token, "gitlab") do
87-
extract_uid(login, http_response)
88-
else
89-
e ->
90-
Logger.error("Error extracting gitlab uid for #{inspect(login)}: #{inspect(e)}")
91-
{:error, "error finding Gitlab ID for #{login}"}
71+
{:error, "error finding #{provider} ID for #{login}"}
9272
end
9373
end
9474

@@ -100,45 +80,66 @@ defmodule Guard.Invitees do
10080
|> return_ok_tuple()
10181
end
10282

103-
defp get_api_token(inviter_id, "github") do
104-
Guard.FrontRepo.RepoHostAccount.get_github_token(inviter_id)
105-
end
83+
defp maybe_get_rha(inviter_id, provider) do
84+
case Guard.FrontRepo.RepoHostAccount.get_for_user_by_repo_host(inviter_id, provider) do
85+
{:ok, rha} ->
86+
{:ok, rha}
10687

107-
defp get_api_token(inviter_id, "gitlab") do
108-
case Guard.FrontRepo.RepoHostAccount.get_gitlab_token(inviter_id) do
109-
{:ok, {token, _expires_at}} -> {:ok, token}
110-
_ -> {:error, "error finding Gitlab token for #{inviter_id}"}
88+
e ->
89+
Logger.warning(
90+
"[Invitees] Missing RHA for inviter #{inviter_id} and #{provider}, #{inspect(e)}"
91+
)
92+
93+
{:ok, nil}
11194
end
11295
end
11396

114-
defp get_api_token(_, provider) do
115-
{:error, "Provider #{provider} not supported for token extraction"}
116-
end
97+
defp maybe_get_api_token(nil, _), do: {:ok, nil}
98+
99+
defp maybe_get_api_token(rha, provider) do
100+
case get_api_token(rha, provider) do
101+
{:ok, {token, _expires_at}} ->
102+
{:ok, token}
103+
104+
e ->
105+
Logger.warning(
106+
"[Invitees] Missing token for rha #{inspect(rha)} and #{provider}, #{inspect(e)}"
107+
)
117108

118-
defp get_github_uid(login) do
119-
Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "github")
109+
{:ok, nil}
110+
end
120111
end
121112

122-
defp get_gitlab_uid(login) do
123-
Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, "gitlab")
113+
defp get_api_token(rha, "github") do
114+
Guard.FrontRepo.RepoHostAccount.get_github_token(rha)
124115
end
125116

126-
defp http_call(resource, api_token, "github") do
127-
resource |> HTTPoison.get([{"Authorization", "Token #{api_token}"}])
117+
defp get_api_token(_, _), do: {:ok, {"", nil}}
118+
119+
defp get_provider_uid(login, provider) do
120+
Guard.FrontRepo.RepoHostAccount.get_uid_by_login(login, provider)
128121
end
129122

130-
defp http_call(resource, api_token, "gitlab") do
131-
resource |> HTTPoison.get([{"PRIVATE-TOKEN", api_token}])
123+
defp http_call(resource, token, "github") when is_binary(token) and token != "" do
124+
HTTPoison.get(resource, [{"Authorization", "Token #{token}"}])
132125
end
133126

134-
defp extract_uid(_, %{status_code: 200, body: body}) do
127+
defp http_call(resource, _, _), do: HTTPoison.get(resource, [])
128+
129+
defp extract_uid(_, %{status_code: 200, body: body}, _) do
135130
with {:ok, body} <- body |> Jason.decode(),
136-
{:ok, id} <- body |> Map.fetch("id"),
131+
{:ok, id} <- fetch_id(body),
137132
do: id |> Integer.to_string() |> return_ok_tuple()
138133
end
139134

140-
defp extract_uid(login, %{status_code: status_code}),
135+
defp extract_uid(login, %{status_code: status_code}, _),
141136
do: {:error, "error finding #{login}: #{status_code}"}
142137

138+
defp fetch_id(%{"id" => id}), do: {:ok, id}
139+
140+
defp fetch_id([%{"id" => id} | _rest]), do: {:ok, id}
141+
142+
defp fetch_id(body), do: {:error, "error finding id in the body #{inspect(body)}"}
143+
143144
defp return_ok_tuple(value), do: {:ok, value}
144145
end

guard/test/fixture/vcr_cassettes/existing_user.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,4 @@
4141
"type": "ok"
4242
}
4343
}
44-
]
44+
]

guard/test/guard/invitees_test.exs

Lines changed: 81 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
defmodule Guard.InviteesTest do
22
use Guard.RepoCase, async: true
3-
use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
3+
4+
import Mock
5+
import ExUnit.CaptureLog
46

57
alias Guard.Invitees
68

@@ -67,21 +69,91 @@ defmodule Guard.InviteesTest do
6769
end
6870

6971
test "inject uid for github provider" do
70-
use_cassette "existing user" do
71-
invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
72+
invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
7273

73-
invitee_with_uid = %{
74-
provider: %{login: "radwo", uid: "184065", type: :GITHUB}
75-
}
74+
invitee_with_uid = %{
75+
provider: %{login: "radwo", uid: "184065", type: :GITHUB}
76+
}
7677

78+
response_body = Jason.encode!(%{"id" => 184_065})
79+
80+
with_mocks([
81+
{Guard.FrontRepo.RepoHostAccount, [:passthrough],
82+
get_github_token: fn _ -> {:ok, {"token", nil}} end},
83+
{HTTPoison, [:passthrough],
84+
get: fn url, headers ->
85+
assert url == "https://api.github.com/users/radwo"
86+
assert headers == [{"Authorization", "Token token"}]
87+
{:ok, %{status_code: 200, body: response_body}}
88+
end}
89+
]) do
7790
assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
7891
end
7992
end
8093

81-
test "return error for unknown github user" do
82-
use_cassette "unknown user" do
83-
invitee = %{provider: %{login: "unknown331123", uid: "", type: :GITHUB}}
94+
test "falls back to github api without inviter token" do
95+
Guard.FrontRepo.delete_all(Guard.FrontRepo.RepoHostAccount)
96+
97+
invitee = %{provider: %{login: "radwo", uid: "", type: :GITHUB}}
98+
response_body = Jason.encode!(%{"id" => 123})
99+
100+
log =
101+
capture_log(fn ->
102+
with_mock HTTPoison, [:passthrough],
103+
get: fn url, headers ->
104+
assert url == "https://api.github.com/users/radwo"
105+
assert headers == []
106+
{:ok, %{status_code: 200, body: response_body}}
107+
end do
108+
invitee_with_uid = %{
109+
provider: %{login: "radwo", uid: "123", type: :GITHUB}
110+
}
111+
112+
assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
113+
end
114+
end)
115+
116+
assert log =~ "Missing RHA for inviter"
117+
end
118+
119+
test "falls back to gitlab api without inviter token" do
120+
Guard.FrontRepo.delete_all(Guard.FrontRepo.RepoHostAccount)
84121

122+
invitee = %{provider: %{login: "radwo_gitlab", uid: "", type: :GITLAB}}
123+
response_body = Jason.encode!(%{"id" => 456})
124+
125+
log =
126+
capture_log(fn ->
127+
with_mock HTTPoison, [:passthrough],
128+
get: fn url, headers ->
129+
assert url == "https://gitlab.com/api/v4/users?username=radwo_gitlab"
130+
assert headers == []
131+
{:ok, %{status_code: 200, body: response_body}}
132+
end do
133+
invitee_with_uid = %{
134+
provider: %{login: "radwo_gitlab", uid: "456", type: :GITLAB}
135+
}
136+
137+
assert Invitees.inject_provider_uid(invitee, @inviter_id) == {:ok, invitee_with_uid}
138+
end
139+
end)
140+
141+
assert log =~ "Missing RHA for inviter"
142+
end
143+
144+
test "return error for unknown github user" do
145+
invitee = %{provider: %{login: "unknown331123", uid: "", type: :GITHUB}}
146+
147+
with_mocks([
148+
{Guard.FrontRepo.RepoHostAccount, [:passthrough],
149+
get_github_token: fn _ -> {:ok, {"token", nil}} end},
150+
{HTTPoison, [:passthrough],
151+
get: fn url, headers ->
152+
assert url == "https://api.github.com/users/unknown331123"
153+
assert headers == [{"Authorization", "Token token"}]
154+
{:ok, %{status_code: 404}}
155+
end}
156+
]) do
85157
assert Invitees.inject_provider_uid(invitee, @inviter_id) ==
86158
{:error, "error finding unknown331123: 404"}
87159
end

0 commit comments

Comments
 (0)