headless_api: gate request on new Repo.automation_permission (bug 1971103) by shtrom · Pull Request #1044 · mozilla-conduit/lando

shtrom · 2026-03-31T07:56:47Z

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)

github-actions · 2026-03-31T07:56:55Z

View this pull request in Lando to land it once approved.

shtrom · 2026-04-02T07:00:45Z

+    if not repo.user_can_use_automation(request.user):
+        error = f"User {request.user.email} is not allowed to use this API. Missing permission: {repo.automation_permission}."
+        logger.info(
+            error,
+            extra={"user": request.user.email, "token": request.auth.token_prefix},
+        )
+        return 403, {"details": error}


@cgsheeh With this in place, I wonder if

lando/src/lando/headless_api/api.py

Lines 83 to 86 in 48d0876

if not api_key.user.has_perm("headless_api.add_automationjob"):

raise APIPermissionDenied(

f"User {api_key.user.email} is not permitted to make automation changes."

)

is still necessary. What do you think?

I think it can be removed. That permission is currently used for "does this user have automation API access". With the more granular control, we can simply remove their automation-level permissions instead.

Ok, I'll file a bug to remove later, so we can confirm that the new system works adequately first.

https://bugzilla.mozilla.org/show_bug.cgi?id=2029863

cgsheeh

One more thing that isn't clear to me from this is whether we intend to use scm levels as the automation_permission. I guess these will be the SCM_ALLOW_DIRECT_PUSH permission in most cases? I think we could make an argument that we could manage these permissions without SCM levels, for example our own custom create_automation_firefox. This would allow us to use Django groups to manage access to teams (sheriffs, thunderbird, etc) without having to use the manual checks we have for SCM levels.

cgsheeh · 2026-04-02T18:55:18Z

+    def _user_has_direct_permission(self, user: User, required_permission: Permission):
        """
-        Test that the user has permission to push to this repo.
+        Test that the user has a specific permission directly rather than via a role.


Checking that a user has a specific permission from LDAP/mirrored groups makes sense for scm levels. I would imagine granting permissions to all users in a specific group would be useful for us in the future (for example, creating a sheriff group and allowing all sheriffs access to the automation API).

This is for Django roles, rather than LDAP groups. I'm not sure we have a mapping for groups to Django roles at the moment. It would certainly be useful.

FWIW, this _user_has_direct_permission is there avoid granting super-users SCM permissions they don't have outright. We could loosen the check and allow role-inheritane if needed, but I don't think that's the case at the moment.

Thinking more about this, in a way, if we grant sheriff_firefox or scm_sheriff_firefox we'd create an implicit group of users who have this permission. Unless we want to associate more than just that permission to them (maybe scm3 comes with at the same time), I think we are already here.

FWIW I think this will be resolved in a much cleaner way once we stop using superusers in Lando.

cgsheeh · 2026-04-02T18:57:57Z

    # Use this field to enable/disable access to this repo via the automation API.
    automation_enabled = models.BooleanField(default=False)
+    automation_permission = models.CharField(
+        default="main.scm_direct_push",


See my other comment about changing this to a ForeignKey.

However if we keep this as a CharField, the default should just be None/blank, which would be a proxy for "there is no permission which can grant you access". We could potentially remove the automation_enabled flag as well, though there's probably an argument to be made that enabling the flag without wiping the permission would be ideal.

None by default would be good, but this means that firefox repos will find themselves without direct push access when we deploy that change. Not a big deal, but something to remember to fix manually at the time of deploy.

We can also add a dedicated data-migration step that would set it correctly for fx repos on migrate, but we don't have a very clean way to do this at the moment https://bugzilla.mozilla.org/show_bug.cgi?id=2013991 @zzzeid maybe it's an opportunity to work on this pattern?

It would be good to set up the data migration "framework" so to speak, even if it's as simple as a management command that calls other management commands (the latter being the actual migrations). They can probably all sit in their own app.

However, for this particular migration I would say it's probably fine to do the changes manually in the admin, while Lando is in maintenance.

🌮 I added a stub migration script, that we can run manually during the upgrade.

cgsheeh · 2026-04-02T19:58:19Z

+    if not repo.user_can_use_automation(request.user):
+        error = f"User {request.user.email} is not allowed to use this API. Missing permission: {repo.automation_permission}."
+        logger.info(
+            error,
+            extra={"user": request.user.email, "token": request.auth.token_prefix},
+        )
+        return 403, {"details": error}


I think it can be removed. That permission is currently used for "does this user have automation API access". With the more granular control, we can simply remove their automation-level permissions instead.

shtrom · 2026-04-07T04:57:10Z

One more thing that isn't clear to me from this is whether we intend to use scm levels as the automation_permission. I guess these will be the SCM_ALLOW_DIRECT_PUSH permission in most cases? I think we could make an argument that we could manage these permissions without SCM levels, for example our own custom create_automation_firefox. This would allow us to use Django groups to manage access to teams (sheriffs, thunderbird, etc) without having to use the manual checks we have for SCM levels.

With the change, we're aiming to continue have that link to SCM levels in LDAP. My reasoning is that doing so lines up with current access practices and processes, which means we don't need to manage it in an ad hoc manner in Lando.

With the coming comm and nss migration, this will likely require the creation of dedicated LDAP groups similar to SCM_ALLOW_DIRECT_PUSH for those projects, but this is a one-off change.

Happy to discuss more, though (:

zzzeid · 2026-04-08T17:59:40Z

    # Use this field to enable/disable access to this repo via the automation API.
    automation_enabled = models.BooleanField(default=False)
+    automation_permission = models.CharField(
+        default="main.scm_direct_push",


It would be good to set up the data migration "framework" so to speak, even if it's as simple as a management command that calls other management commands (the latter being the actual migrations). They can probably all sit in their own app.

However, for this particular migration I would say it's probably fine to do the changes manually in the admin, while Lando is in maintenance.

shtrom · 2026-04-09T04:03:28Z

@zzzeid here's a very stubby data-migration script built for immediate purpose, and we can iterate on it when working on bug 2013991 for real.

Sounds good. I have some comments but I can save them to a different discussion when we are working on bug 2013991 for real.

addressed, I think

zzzeid · 2026-04-09T19:42:02Z

Sounds good. I have some comments but I can save them to a different discussion when we are working on bug 2013991 for real.

zzzeid · 2026-04-09T19:49:38Z

+    def _user_has_direct_permission(self, user: User, required_permission: Permission):
        """
-        Test that the user has permission to push to this repo.
+        Test that the user has a specific permission directly rather than via a role.


FWIW I think this will be resolved in a much cleaner way once we stop using superusers in Lando.

shtrom · 2026-04-14T03:04:43Z

Rebase and force-push to work around landing problems https://lando.moz.tools/landings/35783/

shtrom · 2026-04-14T05:07:38Z

Rebase and squash fixups... https://lando.moz.tools/landings/35790/

headless_api: gate API request to user having the correct repo's automation_permission test: test automation_api gating on repo automation_permission migrations: renumber after merge repo: fail closed on empty permissions strings tests: better coverage of Repo._user_has_direct_permissions repo: rename automation_permission to required_automation_permission utils: add stub data migration command (bug 2013991) tests: fix permission issues Update src/lando/main/models/repo.py Co-authored-by: Zeid <2043828+zzzeid@users.noreply.github.com> pr comments test: no need to create the scm_allow_direct_push permission

shtrom · 2026-04-14T05:25:19Z

Squish squash 🙄 https://lando.moz.tools/landings/35793/

…1103) 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

lando-worker · 2026-04-14T05:35:57Z

Pull request closed by commit 9e90472

shtrom marked this pull request as ready for review March 31, 2026 07:56

shtrom requested a review from a team as a code owner March 31, 2026 07:56

shtrom force-pushed the bug1971103/headless-repo-scope branch 2 times, most recently from b5850ff to 09a737f Compare March 31, 2026 08:00

shtrom commented Apr 2, 2026

View reviewed changes

cgsheeh previously requested changes Apr 2, 2026

View reviewed changes

shtrom force-pushed the bug1971103/headless-repo-scope branch from 72af2a4 to 87209ae Compare April 7, 2026 06:32

zzzeid approved these changes Apr 8, 2026

View reviewed changes

shtrom force-pushed the bug1971103/headless-repo-scope branch from e14ccbd to 4324fef Compare April 9, 2026 03:56

shtrom requested a review from zzzeid April 9, 2026 03:58

shtrom commented Apr 9, 2026

View reviewed changes

shtrom force-pushed the bug1971103/headless-repo-scope branch 2 times, most recently from 86c23c7 to b168ea8 Compare April 9, 2026 06:40

shtrom requested a review from cgsheeh April 9, 2026 06:48

zzzeid approved these changes Apr 9, 2026

View reviewed changes

cgsheeh approved these changes Apr 14, 2026

View reviewed changes

Comment thread src/lando/utils/management/commands/migrate_data.py Outdated

shtrom force-pushed the bug1971103/headless-repo-scope branch from d679156 to d6e0938 Compare April 14, 2026 03:04

shtrom force-pushed the bug1971103/headless-repo-scope branch from d6e0938 to 05cc56e Compare April 14, 2026 05:07

shtrom force-pushed the bug1971103/headless-repo-scope branch from 05cc56e to af050af Compare April 14, 2026 05:25

lando-worker bot closed this Apr 14, 2026

	if not api_key.user.has_perm("headless_api.add_automationjob"):
	raise APIPermissionDenied(
	f"User {api_key.user.email} is not permitted to make automation changes."
	)

Conversation

shtrom commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shtrom Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgsheeh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shtrom Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shtrom commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shtrom commented Apr 14, 2026

Uh oh!

shtrom commented Apr 14, 2026

Uh oh!

shtrom commented Apr 14, 2026

Uh oh!

lando-worker bot commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shtrom commented Mar 31, 2026 •

edited

Loading

shtrom Apr 7, 2026 •

edited

Loading

shtrom Apr 7, 2026 •

edited

Loading

shtrom commented Apr 7, 2026 •

edited

Loading