Skip to content

Commit 3ae2a00

Browse files
fix: service account project creation and better error message (#635)
## 📝 Description Allows service account to create a project with `github_app` type of integration and returs better errors for `oauth` integrations. Adressing #632 ## ✅ Checklist - [x] I have tested this change - [ ] This change requires documentation update
1 parent 1cd10df commit 3ae2a00

File tree

9 files changed

+128
-14
lines changed

9 files changed

+128
-14
lines changed

repository_hub/lib/internal_api/user.pb.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ defmodule InternalApi.User.User.CreationSource do
5656

5757
field :NOT_SET, 0
5858
field :OKTA, 1
59+
field :SERVICE_ACCOUNT, 2
5960
end
6061

6162
defmodule InternalApi.User.ListFavoritesRequest do

repository_hub/lib/repository_hub/adapters/bitbucket_adapter.ex

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,21 @@ defmodule RepositoryHub.BitbucketAdapter do
8181
end
8282

8383
def fetch_token(user_id) do
84-
[integration_type] = integration_types()
84+
with {:ok, user} <- UserClient.describe(user_id),
85+
:ok <- validate_not_service_account(user, "Bitbucket") do
86+
[integration_type] = integration_types()
8587

86-
integration_type
87-
|> UserClient.get_repository_token(user_id)
88+
integration_type
89+
|> UserClient.get_repository_token(user_id)
90+
end
91+
end
92+
93+
defp validate_not_service_account(%{user: %{creation_source: :SERVICE_ACCOUNT}}, provider_name) do
94+
error("Service accounts cannot use #{provider_name} OAuth tokens.")
8895
end
8996

97+
defp validate_not_service_account(_user, _provider_name), do: :ok
98+
9099
def context(_adapter, repository_id, stream \\ nil) do
91100
with {:ok, context} <- UniversalAdapter.context(repository_id, stream),
92101
{:ok, bitbucket_token} <- BitbucketAdapter.fetch_token(context.project.metadata.owner_id) do

repository_hub/lib/repository_hub/adapters/github/create_action.ex

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ defimpl RepositoryHub.Server.CreateAction, for: RepositoryHub.GithubAdapter do
2020
import Toolkit
2121
@impl true
2222
def execute(adapter, request) do
23-
with {:ok, git_repository} <- GitRepository.from_github(request.repository_url),
23+
with {:ok, user} <- UserClient.describe(request.user_id),
24+
{:ok, git_repository} <- GitRepository.from_github(request.repository_url),
2425
{:ok, github_token} <- GithubAdapter.token(adapter, request.user_id, git_repository),
2526
{:ok, github_repository} <- get_github_repository(git_repository, github_token),
26-
{:ok, permissions} <- get_permissions(adapter, github_repository, request.user_id, github_token),
27+
{:ok, permissions} <- get_permissions(adapter, github_repository, user, github_token),
2728
{:ok, _} <- valid?(adapter, github_repository, request.only_public, permissions),
2829
{:ok, repository} <- insert_repository(adapter, request, git_repository, github_repository),
2930
grpc_repository <- Repositories.to_grpc_model(repository) do
@@ -98,11 +99,21 @@ defimpl RepositoryHub.Server.CreateAction, for: RepositoryHub.GithubAdapter do
9899
|> wrap()
99100
end
100101

101-
defp get_permissions(%{integration_type: "github_oauth_token"}, repo, _, _),
102+
defp get_permissions(%{integration_type: "github_oauth_token"}, repo, _user, _github_token),
102103
do: repo.permissions |> wrap
103104

104-
defp get_permissions(%{integration_type: "github_app"}, repo, user_id, github_token) do
105-
{:ok, [username | _]} = UserClient.get_repository_provider_logins(:GITHUB, user_id)
105+
defp get_permissions(
106+
%{integration_type: "github_app"},
107+
_repo,
108+
%{user: %{creation_source: :SERVICE_ACCOUNT}},
109+
_github_token
110+
) do
111+
%{"admin" => true, "push" => true}
112+
|> wrap()
113+
end
114+
115+
defp get_permissions(%{integration_type: "github_app"}, repo, user, github_token) do
116+
{:ok, [username | _]} = UserClient.get_repository_provider_logins(:GITHUB, user.user_id)
106117

107118
GithubClient.repository_permissions(
108119
%{

repository_hub/lib/repository_hub/adapters/github_adapter.ex

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,19 @@ defmodule RepositoryHub.GithubAdapter do
7070
end
7171

7272
def fetch_token_by_user_id(adapter, user_id) do
73-
adapter.integration_type
74-
|> UserClient.get_repository_token(user_id)
73+
with {:ok, user} <- UserClient.describe(user_id),
74+
:ok <- validate_not_service_account(user, "GitHub") do
75+
adapter.integration_type
76+
|> UserClient.get_repository_token(user_id)
77+
end
7578
end
7679

80+
defp validate_not_service_account(%{user: %{creation_source: :SERVICE_ACCOUNT}}, provider_name) do
81+
error("Service accounts cannot use #{provider_name} OAuth tokens. Please use the appropriate integration type.")
82+
end
83+
84+
defp validate_not_service_account(_user, _provider_name), do: :ok
85+
7786
def fetch_token_by_slug(adapter, slug) do
7887
adapter.integration_type
7988
|> to_integration_type

repository_hub/lib/repository_hub/adapters/gitlab_adapter.ex

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,21 @@ defmodule RepositoryHub.GitlabAdapter do
5252
end
5353

5454
def fetch_token(user_id) do
55-
[integration_type] = integration_types()
55+
with {:ok, user} <- UserClient.describe(user_id),
56+
:ok <- validate_not_service_account(user, "GitLab") do
57+
[integration_type] = integration_types()
5658

57-
integration_type
58-
|> UserClient.get_repository_token(user_id)
59+
integration_type
60+
|> UserClient.get_repository_token(user_id)
61+
end
62+
end
63+
64+
defp validate_not_service_account(%{user: %{creation_source: :SERVICE_ACCOUNT}}, provider_name) do
65+
error("Service accounts cannot use #{provider_name} OAuth tokens.")
5966
end
6067

68+
defp validate_not_service_account(_user, _provider_name), do: :ok
69+
6170
def context(_adapter, repository_id, stream \\ nil) do
6271
with {:ok, context} <- UniversalAdapter.context(repository_id, stream),
6372
{:ok, gitlab_token} <- GitlabAdapter.fetch_token(context.project.metadata.owner_id) do

repository_hub/test/repository_hub/adapters/github/create_action_test.exs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,26 @@ defmodule RepositoryHub.Server.Github.CreateActionTest do
1717
[
1818
{RepositoryHub.UserClient, [:passthrough],
1919
[
20-
get_repository_provider_logins: fn _, _ -> {:ok, ["radwo"]} end
20+
describe: fn user_id ->
21+
# Return regular user by default, service account for specific test user
22+
if user_id == "service-account-user-id" do
23+
{:ok,
24+
%{
25+
user_id: user_id,
26+
user: %{creation_source: :SERVICE_ACCOUNT}
27+
}}
28+
else
29+
{:ok,
30+
%{
31+
user_id: user_id,
32+
user: %{creation_source: :NOT_SET}
33+
}}
34+
end
35+
end,
36+
get_repository_provider_logins: fn _, _ -> {:ok, ["radwo"]} end,
37+
get_repository_token: fn _integration_type, _user_id ->
38+
{:ok, "mock-oauth-token"}
39+
end
2140
]}
2241
]
2342
) do
@@ -104,5 +123,28 @@ defmodule RepositoryHub.Server.Github.CreateActionTest do
104123
)
105124
end
106125
end
126+
127+
test "should succeed for service account with github_app integration", %{github_app_adapter: adapter} do
128+
request =
129+
InternalApiFactory.create_request(
130+
integration_type: :GITHUB_APP,
131+
user_id: "service-account-user-id"
132+
)
133+
134+
assert {:ok, %CreateResponse{repository: _repository}} = CreateAction.execute(adapter, request)
135+
end
136+
137+
test "should fail with clear error for service account with github_oauth_token", %{
138+
github_oauth_adapter: adapter
139+
} do
140+
request =
141+
InternalApiFactory.create_request(
142+
integration_type: :GITHUB_OAUTH_TOKEN,
143+
user_id: "service-account-user-id"
144+
)
145+
146+
assert {:error, error_message} = CreateAction.execute(adapter, request)
147+
assert error_message =~ "Service accounts cannot use GitHub OAuth tokens"
148+
end
107149
end
108150
end

repository_hub/test/repository_hub/adapters/github/list_accessible_repositories_action_test.exs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ defmodule RepositoryHub.Server.Github.ListAccessibleRepositoriesActionTest do
1414
setup_with_mocks([
1515
{RepositoryHub.UserClient, [],
1616
[
17+
describe: fn user_id ->
18+
{:ok,
19+
%{
20+
user_id: user_id,
21+
user: %{creation_source: :NOT_SET}
22+
}}
23+
end,
1724
get_repository_token: fn integration_type, user_id ->
1825
token = "#{user_id}-#{integration_type}-token"
1926
expires_at = %Google.Protobuf.Timestamp{seconds: 0, nanos: 0}

repository_hub/test/support/factories/git_clients/bitbucket_client_factory.ex

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,19 @@ defmodule RepositoryHub.BitbucketClientFactory do
2020
remove_deploy_key: &remove_deploy_key_mock/2,
2121
create_webhook: &create_webhook_mock/2,
2222
remove_webhook: &remove_webhook_mock/2
23+
]},
24+
{RepositoryHub.UserClient, [:passthrough],
25+
[
26+
describe: fn user_id ->
27+
{:ok,
28+
%{
29+
user_id: user_id,
30+
user: %{creation_source: :NOT_SET}
31+
}}
32+
end,
33+
get_repository_token: fn _integration_type, _user_id ->
34+
{:ok, "mock-bitbucket-token"}
35+
end
2336
]}
2437
]
2538
end

repository_hub/test/support/factories/git_clients/gitlab_client_factory.ex

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@ defmodule RepositoryHub.GitlabClientFactory do
2626
find_webhook: &find_webhook_mock/2,
2727
find_user: &find_user_mock/2,
2828
fork: &fork_mock/2
29+
]},
30+
{RepositoryHub.UserClient, [:passthrough],
31+
[
32+
describe: fn user_id ->
33+
{:ok,
34+
%{
35+
user_id: user_id,
36+
user: %{creation_source: :NOT_SET}
37+
}}
38+
end,
39+
get_repository_token: fn _integration_type, _user_id ->
40+
{:ok, "mock-gitlab-token"}
41+
end
2942
]}
3043
]
3144
end

0 commit comments

Comments
 (0)