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/api/gitlab.ex b/guard/lib/guard/api/gitlab.ex index 6c83b6f01..0b0dd825d 100644 --- a/guard/lib/guard/api/gitlab.ex +++ b/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} -> diff --git a/guard/lib/guard/invitees.ex b/guard/lib/guard/invitees.ex index fc5371a3e..40f4399c9 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,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 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