Skip to content

Commit 87fd6e6

Browse files
authored
Merge branch 'main' into feature/add-keys
2 parents b202cc1 + 738cd97 commit 87fd6e6

7 files changed

Lines changed: 209 additions & 23 deletions

File tree

backend/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,4 @@ EXPOSE 8000
6464
# Declare /dwhat is the openml-data volume used for?ata as a mountable volume
6565
VOLUME ["/data"]
6666

67-
CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"]
67+
CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "4"]

backend/app/config.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
DEFAULT_CORS_ALLOWED_ORIGINS = "http://localhost:5173,http://127.0.0.1:5173"
3636
DEFAULT_GITHUB_ISSUES_OWNER = "koevoet1221"
3737
DEFAULT_GITHUB_ISSUES_REPO = "openmlupload-testing"
38+
GITHUB_PERMISSION_OWNER_ENV = "GITHUB_PERMISSION_OWNER"
39+
GITHUB_PERMISSION_REPO_ENV = "GITHUB_PERMISSION_REPO"
3840

3941

4042
def _get_bool_env(name: str, default: bool) -> bool:
@@ -178,6 +180,15 @@ class GitHubIssuesSettings:
178180
private_key: str = ""
179181
owner: str = DEFAULT_GITHUB_ISSUES_OWNER
180182
repo: str = DEFAULT_GITHUB_ISSUES_REPO
183+
# Role checks may use a separate repository, but default to the issue repo.
184+
permission_owner: str | None = None
185+
permission_repo: str | None = None
186+
187+
def __post_init__(self) -> None:
188+
object.__setattr__(
189+
self, "permission_owner", self.permission_owner or self.owner
190+
)
191+
object.__setattr__(self, "permission_repo", self.permission_repo or self.repo)
181192

182193
@classmethod
183194
def from_env(cls) -> "GitHubIssuesSettings":
@@ -191,13 +202,17 @@ def get_env_val(key_name: str) -> str:
191202
app_id_str = get_env_val("GH_APP_ID")
192203
install_id_str = get_env_val("GH_INSTALL_ID")
193204
priv_key_str = get_env_val("GH_PRIV_KEY").replace("\\n", "\n")
205+
owner = get_env_val("GITHUB_ISSUES_OWNER") or DEFAULT_GITHUB_ISSUES_OWNER
206+
repo = get_env_val("GITHUB_ISSUES_REPO") or DEFAULT_GITHUB_ISSUES_REPO
194207

195208
return cls(
196209
app_id=int(app_id_str) if app_id_str else None,
197210
install_id=int(install_id_str) if install_id_str else None,
198211
private_key=priv_key_str,
199-
owner=get_env_val("GITHUB_ISSUES_OWNER") or DEFAULT_GITHUB_ISSUES_OWNER,
200-
repo=get_env_val("GITHUB_ISSUES_REPO") or DEFAULT_GITHUB_ISSUES_REPO,
212+
owner=owner,
213+
repo=repo,
214+
permission_owner=get_env_val(GITHUB_PERMISSION_OWNER_ENV) or owner,
215+
permission_repo=get_env_val(GITHUB_PERMISSION_REPO_ENV) or repo,
201216
)
202217

203218

backend/app/services/github_roles.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
from requests import Response
77

88
from app.database.models import Roles
9-
from app.config import GitHubIssuesSettings
9+
from app.config import (
10+
DEFAULT_GITHUB_ISSUES_OWNER,
11+
DEFAULT_GITHUB_ISSUES_REPO,
12+
GitHubIssuesSettings,
13+
)
1014

1115
logger = logging.getLogger(__name__)
1216

1317
GITHUB_API_VERSION = "2022-11-28"
14-
DEFAULT_GITHUB_PERMISSION_OWNER = "koevoet1221"
15-
DEFAULT_GITHUB_PERMISSION_REPO = "openmlupload-testing"
1618
# GitHub exposes comparable access levels through both role_name and permission.
1719
EXPERT_GITHUB_ACCESS_LEVELS = {"maintain", "admin"}
1820

@@ -31,6 +33,8 @@ def __init__(
3133
session=None,
3234
*,
3335
token: str | None = None,
36+
owner: str | None = None,
37+
repo: str | None = None,
3438
api_base_url: str = "https://api.github.com",
3539
):
3640
# If a token is provided (App token), we use a fresh session to avoid
@@ -44,8 +48,8 @@ def __init__(
4448
self.token = None
4549
self.auth_method = "user_session" if session else "unauthenticated"
4650

47-
self.owner = DEFAULT_GITHUB_PERMISSION_OWNER
48-
self.repo = DEFAULT_GITHUB_PERMISSION_REPO
51+
self.owner = owner or DEFAULT_GITHUB_ISSUES_OWNER
52+
self.repo = repo or DEFAULT_GITHUB_ISSUES_REPO
4953
self.api_base_url = api_base_url.rstrip("/")
5054

5155
def _get_headers(self) -> dict[str, str]:
@@ -169,9 +173,15 @@ def resolve_github_repository_role(
169173
"Failed to get GitHub App installation token for role resolution",
170174
extra={"error": str(error)},
171175
)
176+
return Roles.USER
172177

173178
# If we have an App token, we use it with a fresh session to avoid conflicts
174179
# with the user's OAuth session.
175180
client_session = session if not token else None
176-
client = GitHubRepositoryPermissionClient(client_session, token=token)
181+
client = GitHubRepositoryPermissionClient(
182+
client_session,
183+
token=token,
184+
owner=settings.permission_owner if settings else None,
185+
repo=settings.permission_repo if settings else None,
186+
)
177187
return GitHubRepositoryRoleResolver(client).resolve_role(username)

backend/app/tests/integration/test_github_auth_sync.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
from requests import exceptions as requests_exceptions
55

66
from app.database import models
7-
from app.services.github_issues import GitHubAPIError
87
from app.services.github_sync import GitHubProfileSyncConflictError
98

109

1110
class _FakeGitHubResponse:
12-
def __init__(self, status_code: int, payload):
11+
def __init__(self, status_code: int, payload, text: str = ""):
1312
self.status_code = status_code
1413
self._payload = payload
14+
self.text = text
1515

1616
def json(self):
1717
return self._payload
@@ -26,21 +26,35 @@ def _mock_github_oauth(
2626
emails_status: int = 200,
2727
permission_payload: dict | None = None,
2828
permission_status: int = 404,
29+
app_token_error: Exception | None = None,
2930
token_error: Exception | None = None,
3031
user_error: Exception | None = None,
3132
emails_error: Exception | None = None,
3233
):
3334
monkeypatch.setenv("AUTH_DEV_MODE_APPROVE_ALL_LOGINS", "false")
3435
monkeypatch.setenv("ENVIRONMENT", "production")
3536

36-
def raise_missing_app_token(_settings):
37-
raise GitHubAPIError("GitHub App credentials are not fully configured")
37+
def get_app_token(_settings):
38+
if app_token_error:
39+
raise app_token_error
40+
return "github-app-token"
3841

3942
monkeypatch.setattr(
4043
"app.services.github_issues.get_installation_token",
41-
raise_missing_app_token,
44+
get_app_token,
4245
)
4346

47+
class FakeAppSession:
48+
def get(self, url: str, **_kwargs):
49+
if "/repos/" in url and url.endswith("/permission"):
50+
return _FakeGitHubResponse(
51+
permission_status,
52+
permission_payload or {"permission": "none", "role_name": "none"},
53+
)
54+
raise AssertionError(f"Unexpected GitHub App URL: {url}")
55+
56+
monkeypatch.setattr("app.services.github_roles.requests.Session", FakeAppSession)
57+
4458
class FakeOAuth2Session:
4559
def __init__(self, *_args, **_kwargs):
4660
pass
@@ -59,11 +73,6 @@ def get(self, url: str, **_kwargs):
5973
if emails_error:
6074
raise emails_error
6175
return _FakeGitHubResponse(emails_status, emails_payload)
62-
if "/repos/" in url and url.endswith("/permission"):
63-
return _FakeGitHubResponse(
64-
permission_status,
65-
permission_payload or {"permission": "none", "role_name": "none"},
66-
)
6776
raise AssertionError(f"Unexpected GitHub URL: {url}")
6877

6978
monkeypatch.setattr("app.routers.auth.OAuth2Session", FakeOAuth2Session)

backend/app/tests/unit/storage/test_config_and_factory.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ def test_settings_defaults(monkeypatch):
1717
monkeypatch.delenv("UPLOAD_URL_EXPIRES_SECONDS", raising=False)
1818
monkeypatch.delenv("COOKIE_SECURE", raising=False)
1919
monkeypatch.delenv("APP_BASE_URL", raising=False)
20+
monkeypatch.delenv("GITHUB_ISSUES_OWNER", raising=False)
21+
monkeypatch.delenv("GITHUB_ISSUES_REPO", raising=False)
22+
monkeypatch.delenv("GITHUB_PERMISSION_OWNER", raising=False)
23+
monkeypatch.delenv("GITHUB_PERMISSION_REPO", raising=False)
2024

2125
settings = Settings.from_env()
2226

@@ -30,6 +34,8 @@ def test_settings_defaults(monkeypatch):
3034
assert settings.storage.clamd_timeout_seconds == 60.0
3135
assert settings.github_issues.owner == "koevoet1221"
3236
assert settings.github_issues.repo == "openmlupload-testing"
37+
assert settings.github_issues.permission_owner == "koevoet1221"
38+
assert settings.github_issues.permission_repo == "openmlupload-testing"
3339
assert settings.app_base_url == "http://localhost:8000"
3440
assert settings.upload.target == "uploads"
3541
assert settings.upload.location == "default"

backend/app/tests/unit/test_github_role_service.py

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@
22

33
import pytest
44

5+
from app.config import GitHubIssuesSettings
56
from app.database.models import Roles
7+
from app.services.github_issues import GitHubAPIError
68
from app.services.github_roles import (
79
GitHubPermissionLookupError,
810
GitHubRepositoryPermissionClient,
911
GitHubRepositoryRoleResolver,
1012
GITHUB_API_VERSION,
1113
map_github_repository_role,
14+
resolve_github_repository_role,
1215
)
1316

1417

@@ -32,14 +35,23 @@ def is_contributor(self, username: str) -> bool:
3235

3336

3437
class FakeResponse:
35-
def __init__(self, status_code: int, payload: list | dict):
38+
def __init__(self, status_code: int, payload: list | dict, text: str = ""):
3639
self.status_code = status_code
3740
self.payload = payload
41+
self.text = text
3842

3943
def json(self):
4044
return self.payload
4145

4246

47+
class InvalidJsonResponse(FakeResponse):
48+
def __init__(self):
49+
super().__init__(200, {}, text="not-json")
50+
51+
def json(self):
52+
raise ValueError("invalid json")
53+
54+
4355
class RecordingSession:
4456
def __init__(self, response: FakeResponse):
4557
self.response = response
@@ -87,6 +99,21 @@ def test_repository_permission_client_defaults_to_openml_upload_testing(monkeypa
8799
)
88100

89101

102+
def test_repository_permission_client_uses_configured_repository():
103+
session = RecordingSession(FakeResponse(200, {"role_name": "maintain"}))
104+
client = GitHubRepositoryPermissionClient(
105+
session,
106+
owner="openml",
107+
repo="expert-checks",
108+
)
109+
110+
assert client.get_repository_permission("octocat") == {"role_name": "maintain"}
111+
assert session.requests[0]["url"] == (
112+
"https://api.github.com/repos/openml/expert-checks"
113+
"/collaborators/octocat/permission"
114+
)
115+
116+
90117
def test_repository_permission_client_applies_auth_header_when_token_provided():
91118
session = RecordingSession(FakeResponse(200, {"role_name": "maintain"}))
92119
# We pass session explicitly to the client even with a token for testing
@@ -107,6 +134,34 @@ def test_repository_permission_client_wraps_http_errors_as_lookup_failures():
107134
raise AssertionError("Expected GitHubPermissionLookupError")
108135

109136

137+
def test_repository_permission_client_maps_404_to_no_permission():
138+
session = RecordingSession(FakeResponse(404, {"message": "Not Found"}))
139+
client = GitHubRepositoryPermissionClient(session)
140+
141+
assert client.get_repository_permission("octocat") == {
142+
"permission": "none",
143+
"role_name": "none",
144+
}
145+
146+
147+
def test_repository_permission_client_wraps_non_200_responses():
148+
session = RecordingSession(
149+
FakeResponse(500, {"message": "Server Error"}, text="server error")
150+
)
151+
client = GitHubRepositoryPermissionClient(session)
152+
153+
with pytest.raises(GitHubPermissionLookupError, match="GitHub returned 500"):
154+
client.get_repository_permission("octocat")
155+
156+
157+
def test_repository_permission_client_wraps_invalid_json_responses():
158+
session = RecordingSession(InvalidJsonResponse())
159+
client = GitHubRepositoryPermissionClient(session)
160+
161+
with pytest.raises(GitHubPermissionLookupError, match="invalid JSON"):
162+
client.get_repository_permission("octocat")
163+
164+
110165
@pytest.mark.parametrize(
111166
"permission_payload",
112167
[
@@ -169,3 +224,81 @@ def test_permission_lookup_failure_falls_back_to_user_and_logs(caplog):
169224

170225
assert role == Roles.USER
171226
assert "GitHub repository permission lookup failed" in caplog.text
227+
228+
229+
def test_resolve_role_uses_configured_permission_repository(monkeypatch):
230+
class RecordingPermissionClient:
231+
def __init__(
232+
self,
233+
session=None,
234+
*,
235+
token=None,
236+
owner=None,
237+
repo=None,
238+
):
239+
self.session = session
240+
self.token = token
241+
self.owner = owner
242+
self.repo = repo
243+
created_clients.append(self)
244+
245+
def get_repository_permission(self, username: str) -> dict[str, object]:
246+
return {"role_name": "maintain"}
247+
248+
created_clients: list[RecordingPermissionClient] = []
249+
250+
monkeypatch.setattr(
251+
"app.services.github_roles.GitHubRepositoryPermissionClient",
252+
RecordingPermissionClient,
253+
)
254+
monkeypatch.setattr(
255+
"app.services.github_issues.get_installation_token",
256+
lambda _settings: "gh-app-token",
257+
)
258+
settings = GitHubIssuesSettings(
259+
app_id=123,
260+
install_id=456,
261+
private_key="test-key",
262+
owner="issue-owner",
263+
repo="issue-repo",
264+
permission_owner="permission-owner",
265+
permission_repo="permission-repo",
266+
)
267+
268+
role = resolve_github_repository_role(
269+
"octocat", session=object(), settings=settings
270+
)
271+
272+
assert role == Roles.EXPERT
273+
assert len(created_clients) == 1
274+
created_client = created_clients[0]
275+
assert created_client.session is None
276+
assert created_client.token == "gh-app-token"
277+
assert created_client.owner == "permission-owner"
278+
assert created_client.repo == "permission-repo"
279+
280+
281+
def test_resolve_role_falls_back_to_user_when_app_token_lookup_fails(monkeypatch):
282+
session = RecordingSession(FakeResponse(200, {"role_name": "maintain"}))
283+
284+
def fail_token_lookup(_settings):
285+
raise GitHubAPIError("token unavailable")
286+
287+
monkeypatch.setattr(
288+
"app.services.github_issues.get_installation_token",
289+
fail_token_lookup,
290+
)
291+
settings = GitHubIssuesSettings(
292+
app_id=123,
293+
install_id=456,
294+
private_key="test-key",
295+
owner="issue-owner",
296+
repo="issue-repo",
297+
permission_owner="permission-owner",
298+
permission_repo="permission-repo",
299+
)
300+
301+
role = resolve_github_repository_role("octocat", session=session, settings=settings)
302+
303+
assert role == Roles.USER
304+
assert session.requests == []

0 commit comments

Comments
 (0)