Skip to content

Commit 9e90472

Browse files
committed
headless_api: gate request on new Repo.automation_permission (bug 1971103) r=sheehan,zeid
* repo: add `automation_permission` and `user_can_use_automation()` * headless_api: gate API request to user having the correct repo's `required_automation_permission` * test: test headless_api gating on repo `required_automation_permission` * repo: fail closed on empty permissions strings * utils: add stub data migration command (bug 2013991) Pull request: #1044
1 parent 7f8b078 commit 9e90472

7 files changed

Lines changed: 273 additions & 6 deletions

File tree

src/lando/conftest.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
Worker,
3636
)
3737
from lando.main.models.landing_job import LandingJob, add_job_with_revisions
38+
from lando.main.models.profile import SCM_ALLOW_DIRECT_PUSH
3839
from lando.main.models.revision import Revision
3940
from lando.main.scm import SCMType
4041
from lando.main.scm.commit import CommitData
@@ -696,6 +697,7 @@ def git_repo_mc(
696697
"approval_required": approval_required,
697698
"autoformat_enabled": autoformat_enabled,
698699
"automation_enabled": automation_enabled,
700+
"required_automation_permission": SCM_ALLOW_DIRECT_PUSH,
699701
"force_push": force_push,
700702
"hooks_enabled": hooks_enabled,
701703
"is_try": is_try,
@@ -1054,10 +1056,22 @@ def headless_permission():
10541056

10551057

10561058
@pytest.fixture
1057-
def headless_user(user, headless_permission):
1059+
def direct_push_permission():
1060+
content_type = ContentType.objects.get_for_model(Profile)
1061+
perm = Permission.objects.get(
1062+
codename="scm_allow_direct_push", content_type=content_type
1063+
)
1064+
return perm
1065+
1066+
1067+
@pytest.fixture
1068+
def headless_user(
1069+
user, headless_permission, direct_push_permission
1070+
) -> tuple[User, str]:
10581071
user.user_permissions.add(headless_permission)
1059-
user.save()
1072+
user.user_permissions.add(direct_push_permission)
10601073
user.profile.save()
1074+
user.save()
10611075

10621076
token = ApiToken.create_token(user)
10631077
return user, token

src/lando/headless_api/api.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,14 @@ def post_repo_actions(
428428
)
429429
return 400, {"details": error}
430430

431+
if not repo.user_can_use_automation(request.user):
432+
error = f"User {request.user.email} is not allowed to use this API for repo {repo_name}. Missing permission: {repo.required_automation_permission}."
433+
logger.info(
434+
error,
435+
extra={"user": request.user.email, "token": request.auth.token_prefix},
436+
)
437+
return 403, {"details": error}
438+
431439
with transaction.atomic():
432440
automation_job = AutomationJob.objects.create(
433441
status=JobStatus.SUBMITTED,

src/lando/headless_api/tests/test_automation_api.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
import pytest
1212
from django.contrib.auth.hashers import check_password
13+
from django.contrib.auth.models import Permission, User
14+
from django.test.client import Client
1315
from pydantic import ValidationError
1416

1517
from lando.api.legacy.workers.automation_worker import AutomationWorker
@@ -376,6 +378,65 @@ def test_automation_job_create_user_automation_disabled(
376378
)
377379

378380

381+
@pytest.mark.parametrize("as_superuser", (True, False))
382+
@pytest.mark.django_db
383+
def test_automation_job_create_user_no_repo_required_automation_permission(
384+
client: Client,
385+
headless_user: tuple[User, str],
386+
make_superuser: Callable,
387+
direct_push_permission: Permission,
388+
repo_mc: Callable,
389+
as_superuser: bool,
390+
):
391+
user, token = headless_user
392+
393+
# Disable automation enabled for user.
394+
user.user_permissions.remove(direct_push_permission)
395+
if as_superuser:
396+
user = make_superuser(user)
397+
user.save()
398+
user.profile.save()
399+
400+
repo = repo_mc(
401+
scm_type=SCMType.GIT,
402+
automation_enabled=True,
403+
)
404+
405+
# Send a valid request.
406+
body = {
407+
"actions": [
408+
{
409+
"action": "add-commit",
410+
"content": "0",
411+
"patch_format": "git-format-patch",
412+
},
413+
{
414+
"action": "add-commit",
415+
"content": "1",
416+
"patch_format": "git-format-patch",
417+
},
418+
],
419+
}
420+
response = client.post(
421+
f"/api/repo/{repo.name}",
422+
data=json.dumps(body),
423+
content_type="application/json",
424+
headers={
425+
"User-Agent": "Lando-User/testuser@example.org",
426+
"Authorization": f"Bearer {token}",
427+
},
428+
)
429+
430+
assert (
431+
response.status_code == 403
432+
), "User without automation permission for repo should return 403 status code."
433+
response_json = response.json()
434+
assert (
435+
response_json["details"]
436+
== f"User testuser@example.org is not allowed to use this API for repo {repo.name}. Missing permission: {repo.required_automation_permission}."
437+
)
438+
439+
379440
def is_isoformat_timestamp(date_string: str) -> bool:
380441
"""Return `True` if `date_string` is an ISO format datetime string."""
381442
try:
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Generated by Django 6.0.4 on 2026-04-13 07:29
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("main", "0049_profile_phabricator_phid"),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name="repo",
15+
name="required_automation_permission",
16+
field=models.CharField(
17+
blank=True,
18+
default="",
19+
help_text="The permission required to use the automation API against this repo. For example, main.scm_allow_direct_push.",
20+
),
21+
),
22+
]

src/lando/main/models/repo.py

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ def path(self) -> str:
212212

213213
# Use this field to enable/disable access to this repo via the automation API.
214214
automation_enabled = models.BooleanField(default=False)
215+
required_automation_permission = models.CharField(
216+
help_text="The permission required to use the automation API against this repo. For example, main.scm_allow_direct_push.",
217+
blank=True,
218+
default="",
219+
)
215220

216221
# Use this field to enable/disable access to this repo via the try API.
217222
is_try = models.BooleanField(default=False)
@@ -395,28 +400,49 @@ def phab_identifier(self) -> str | None:
395400

396401
def user_can_push(self, user: User) -> bool:
397402
"""
398-
Test that the user has permission to push to this repo.
403+
Test that the user has the permission to push to this repo.
404+
"""
405+
return self._user_has_direct_permission(user, str(self.required_permission))
406+
407+
def user_can_use_automation(self, user: User) -> bool:
408+
"""
409+
Test that the user has the permission to create automation job for this repo.
410+
"""
411+
return self._user_has_direct_permission(
412+
user, str(self.required_automation_permission)
413+
)
414+
415+
def _user_has_direct_permission(self, user: User, permission: str):
416+
"""
417+
Test that the user has a specific permission directly rather than via a role.
399418
400419
If the user is a superuser, this checks that the user was given the permissions
401420
directly, rather than transitively by virtue of being an admin.
402421
403422
Parameters:
404423
405424
user: User
406-
User to check permmission.
425+
User for whom to check permission.
426+
427+
permissions: str
428+
Permission string to look for. If empty, fail closed denying any access.
407429
408430
Returns:
409431
bool: whether the user has the permission
410432
"""
433+
# Fail closed if the permission is empty.
434+
if not permission:
435+
return False
436+
411437
if user.is_superuser:
412438
# We can't rely on the `get_user_permissions()` method, as it returns all existing
413439
# permissions for superusers. Here, we want to check permissions that have been
414440
# explicitly given to the user from LDAP groups.
415441

416-
app_label, codename = self.required_permission.split(".", maxsplit=1)
442+
app_label, codename = permission.split(".", maxsplit=1)
417443
return user.user_permissions.filter(
418444
content_type__app_label=app_label, codename=codename
419445
).exists()
420446

421447
# If the user is not a superuser, we can skip the DB round-trip.
422-
return self.required_permission in user.get_user_permissions()
448+
return permission in user.get_user_permissions()

src/lando/main/tests/test_models.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import pytest
77
from django.conf import settings
8+
from django.contrib.auth.models import Permission, User
89
from django.core.exceptions import ValidationError
910

1011
from lando.main.models import CommitMap, Repo
@@ -191,6 +192,42 @@ def test__models__Repo__git_repo_name(
191192
assert repo.git_repo_name == expected_git_repo_name
192193

193194

195+
@pytest.mark.parametrize(
196+
"has_permission,as_superuser",
197+
(
198+
(False, False),
199+
(False, True),
200+
(True, False),
201+
(True, True),
202+
),
203+
)
204+
@pytest.mark.django_db
205+
def test__models__Repo___user_has_direct_permissions(
206+
user: User,
207+
make_superuser: Callable,
208+
repo_mc: Callable,
209+
direct_push_permission: Permission,
210+
has_permission: bool,
211+
as_superuser: bool,
212+
):
213+
permission = direct_push_permission
214+
permission_string = f"{permission.content_type.app_label}.{permission.codename}"
215+
216+
if has_permission:
217+
user.user_permissions.add(permission)
218+
if as_superuser:
219+
user = make_superuser(user)
220+
user.save()
221+
user.profile.save()
222+
223+
repo = repo_mc(
224+
scm_type=SCMType.GIT,
225+
automation_enabled=True,
226+
)
227+
228+
assert repo._user_has_direct_permission(user, permission_string) == has_permission
229+
230+
194231
@pytest.mark.parametrize(
195232
"pulse_routing_key,expected_valid",
196233
(
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import argparse
2+
3+
from django.core.management import BaseCommand, CommandError, CommandParser
4+
5+
from lando.main.models import SCM_ALLOW_DIRECT_PUSH
6+
from lando.main.models.repo import Repo
7+
8+
9+
class Command(BaseCommand):
10+
help = """This stub commands allows to run data migrations.
11+
12+
It should be fleshed out as part of bug 2013991.
13+
"""
14+
# TODO: record already-applied migrations.
15+
16+
def add_arguments(self, parser: CommandParser):
17+
parser.add_argument(
18+
"-l", "--list", default=False, action=argparse.BooleanOptionalAction
19+
)
20+
parser.add_argument(
21+
"-y",
22+
"--yes",
23+
default=False,
24+
help="Don't ask for confirmation",
25+
action=argparse.BooleanOptionalAction,
26+
)
27+
parser.add_argument("migration", help="Migration to apply", nargs="?")
28+
29+
def handle(self, *args, **options):
30+
ask_confirm = not options.get("yes", False)
31+
32+
if options.get("list"):
33+
self.stdout.write("Available data migrations:\n")
34+
for migration in [
35+
method.removeprefix("migrate_")
36+
for method in dir(self)
37+
if method.startswith("migrate_")
38+
]:
39+
self.stdout.write(f"* {migration}")
40+
raise SystemExit()
41+
42+
if not (migration := options.get("migration")):
43+
raise CommandError("No migration specified")
44+
45+
method_name = f"migrate_{migration}"
46+
47+
if not (migration_method := getattr(self, method_name)):
48+
raise CommandError("Migration method not found: {method_name}")
49+
50+
migration_method(ask_confirm)
51+
52+
def _get_confirmation(self, ask_confirm: bool, prompt: str, details: str):
53+
"""Show a confirmation prompt with details.
54+
55+
If ask_confirm is set, details are printed prior to requesting confirmation.
56+
57+
Otherwise, let the caller show the details as it proceeds.
58+
"""
59+
self.stdout.write(prompt, ending="")
60+
if ask_confirm:
61+
self.stdout.write(f"{details}. Confirm [yN]? ", ending="")
62+
if input().lower() != "y":
63+
raise CommandError("User interruption")
64+
65+
self.stdout.write("Progress: ", ending="")
66+
67+
#
68+
# MIGRATION METHODS
69+
#
70+
71+
def migrate_1_bug1971103_add_firefox_required_automation_permissions(
72+
self, ask_confirm: bool = True
73+
):
74+
"""
75+
Explicitly set the `required_automation_permission` for all Firefox repos.
76+
77+
This is to match the currectly implicit authorisation for Firefox sheriffs.
78+
"""
79+
fx_repos = Repo.objects.filter(
80+
name__startswith="firefox-", required_automation_permission=""
81+
)
82+
if not fx_repos:
83+
self.stdout.write(
84+
"No firefox-% repo found with NULL required_automation_permission."
85+
)
86+
raise SystemExit()
87+
88+
fx_repo_names = ", ".join(repo.name for repo in fx_repos)
89+
self._get_confirmation(
90+
ask_confirm, "Updating required_automation_permission for: ", fx_repo_names
91+
)
92+
93+
for repo in fx_repos:
94+
if not repo.required_automation_permission:
95+
repo.required_automation_permission = SCM_ALLOW_DIRECT_PUSH
96+
repo.save()
97+
self.stdout.write(f"{repo.name} ", ending="")
98+
99+
self.stdout.write("done.")

0 commit comments

Comments
 (0)