try_api: implement /patches endpoint (bug 1979247, bug 1983216) by shtrom · Pull Request #473 · mozilla-conduit/lando

shtrom · 2025-08-13T07:43:09Z

Reimplement the /try/patches endpoint from old lando, so mach try can submit Try job to this version of Lando using the same API.

repo: add is_try field (bug 1979247)
settings: add try_api to APPLICATIONS (bug 1979247)
create_environment_repos: add try repo in suite (bug 1986575)
try_api: patches endpoint (bug 1979247)
try_api: use django.test.clients (bug 2008895)
try_api: add error handling with RFC 7807 responses
try_api/tests: add client_post fixture
try_api: check user permission against target_repo
try_api: remove extra argument to logger
utils.ninja_auth: move AccessTokenLandoOIDCAuthenticationBackend to main.auth, and rename
CommitMap: add TRY_REPO_MAPPING to support determining where to lookup CommitMaps from
Repo: add user_can_push method to check direct required_permission on User
update for Repo.user_allowed
tests: update fixtures for Try repo parameters
create_environment_repos: disable commit message hook for try
auth: don't create Django users on API requests
try_api: use SCM rather than VCS in user messaging
try_api/tests: don't embed b64 literals
try_api: move some PatchHelper calls out of try/except

addressed + discussion

shtrom · 2026-01-12T07:34:31Z

Now based on #476 as it relies on the permission checking logic.

zzzeid

Few more changes requested. Overall looking good, main comments are around permission checking.

src/lando/api/legacy/transplants.py

src/lando/main/models/profile.py

src/lando/utils/ninja_auth.py

src/lando/ui/tests/test_ui.py

src/lando/try_api/api.py

src/lando/api/tests/test_transplants.py

shtrom · 2026-01-19T03:44:56Z

@zzzeid all comments addressed. The permission check changes are in #476, which can be reviewed/landed independently.

I'm not sure why this PR here shows conflicts at the moment, as I think it's up-to-date with main. In any case, it'll likely need a rebasing before landing, as I suspect we'll get conflicts when applying the patches in sequence.

zzzeid

Looking great! Final stretch I think.

src/lando/main/auth.py

src/lando/main/migrations/0043_repo_treeherder_name.py

src/lando/try_api/api.py

src/lando/utils/management/commands/create_environment_repos.py

src/lando/utils/management/commands/generate_requirements.py

src/lando/try_api/tests/test_api.py

zzzeid

Looks good - just a bit of cleanup in src/lando/main/auth.py and I think this will be ready to land.

I think we should clean up some of this auth backend business in a separate follow up, I'll file a QoL bug for this. Ideally we'd be able to reuse this functionality as a standard auth backend.

src/lando/main/auth.py

src/lando/try_api/api.py

shtrom · 2026-01-30T01:45:01Z

Looks good - just a bit of cleanup in src/lando/main/auth.py and I think this will be ready to land.

Ok, done 😅

I think we should clean up some of this auth backend business in a separate follow up, I'll file a QoL bug for this. Ideally we'd be able to reuse this functionality as a standard auth backend.

Yeah, Ninja diverges ever so slightly from Django auth. That said, I think the way the Token backend currently is, it should be already usable by Django. The Ninja-specific adaptation is done in ninja_auth.AccessTokenAuth.

shtrom · 2026-01-30T02:07:58Z

Ideally we'd be able to reuse this functionality as a standard auth backend.

Yeah, Ninja diverges ever so slightly from Django auth. That said, I think the way the Token backend currently is, it should be already usable by Django. The Ninja-specific adaptation is done in ninja_auth.AccessTokenAuth.

More on this: I think this is already the case.

The auth.AccessTokenLandoOIDCAuthenticationBackend is a plain Django authentication backend at the moment. The fallback mechanism was not the Django auth mechanism, but just something by virtue of this code shimming the mechanism from auth: Skip OIDC flow if Bearer access_token is present (bug 1979246) mozilla/mozilla-django-oidc#551; when that PR is merged, the need for this class vanishes, and we can just use a plain auth.LandoOIDCAuthenticationBackend.
The Ninja-specific logic is in ninja_auth.AccessTokenAuth. It does use the AccessTokenLandoOIDCAuthenticationBackend authenticator under the hood, but because ninja_auth.AccessTokenAuth requires an Authorization: bearer ... https://github.com/vitalik/django-ninja/blob/b17dcec55efa094092a965c72511cbe2d5631884/ninja/security/http.py#L26-L38, there is no chance we'd hit the fallback because we're already guaranteed to have a bearer token by the time we are in the shim, and that token is either valid, or not, in which case we'd re-throw the HTTPError.

I've removed the fallback for now, but I think this is what would make the auth backend less standard, as it would now only allow Token-based auth... Though I think that I now see where you're going: do you mean to aim for dedicated auth backends for token vs. OIDC flow, rather than one implementing both, so we can mix and match depending on need?

In a way, that's something that would sit better in the mozilla-django-oidc lib, but in a different way than how I implemented it (or maybe very much in the current state of the auth.AccessTokenLandoOIDCAuthenticationBackend) We do retain the fact that the LandoOIDCAuthenticationBackend has the post-user creation hooks, that would need to be shared between the two classes.

Anyway, I'm just rambling at this point, and trying to capture my understanding somewhere. Let's chat about it next time we catch up!

src/lando/utils/tests/test_ninja_auth.py

zzzeid

lgtm! (with a couple of nits)

src/lando/main/auth.py

shtrom · 2026-02-04T05:22:53Z

@zzzeid I updated AccessTokenLandoOIDCAuthenticationBackend as per your suggestion, and I'm testing in develop. I'll merge this tomorrow.

shtrom · 2026-02-24T03:53:39Z

As part of rebasing this to avoid conflicts, I've managed to cleanly separate

scm: add SCMType enum to helpers

into its own PR: #945

shtrom · 2026-02-25T05:11:15Z

Pfiou. Rebased on main, with the SCMType logic split out (and landed) to #945, and the line-ending fix for generate_requirements.py split out to #947.

diff from the force-push: https://github.com/mozilla-conduit/lando/compare/03c4ef2be7bfa79219f5d102329b77c3c3b97e63..e51e03fe56fb1ef056b7f47fb67e9421035c2d5c

shtrom · 2026-02-26T08:55:07Z

Pushing one big squashed commit.

@shtrom

conftest: add try repo to mocked_repo_config repo: add try_enabled field (bug 1979247) settings: add try_api to APPLICATIONS (bug 1979247) create_environment_repos: add try repo in suite (bug 1986575) try_api: patches endpoint (bug 1979247) try_api: use PatchHelper to parse patches (bug 1979247) try_api: resolve base_commit at submission time (bug 1979247) try_api: test SCM1 auth auth: fix require_permission to return wrapped method utils.auth: add PermissionAccessTokenAuth (bug 1983216) Update src/lando/try_api/api.py Co-authored-by: Connor Sheehan <cosheehan@mozilla.com> utils.auth: don't allow delegated permissions in PermissionAccessTokenAuth utils.exceptions: move problem_exception_handler from treestatus (bug 2008894) This allows us to reuse it verbatim from other modules. utils.exception: add support for ForbiddenProblemException (bug 2008894) try_api: use django.test.clients (bug 2008895) try_api: add error handling with RFC 7807 responses repo: coalesce try_enabled and is_try try_api/tests: add client_post fixture test_transplants: test blocker_scm_permission and add integrated_transplant_without_permissions utils.auth: add user_has_direct_permission models: move user_has_direct_permission to Profile model try_api: check user permission against target_repo try_api: remove extra argument to logger utils.ninja_auth: move AccessTokenLandoOIDCAuthenticationBackend to main.auth, and rename try_api.api: don't set priority on Try jobs try_api.api: don't expose exceptions CommitMap: add TRY_REPO_MAPPING to support determining where to lookup CommitMaps from test_transplants: test blocker_scm_permission and add integrated_transplant_without_permissions utils.auth: add user_has_direct_permission models: move user_has_direct_permission to Profile model Profile: support app_label in has_direct_perm Update src/lando/main/models/profile.py Co-authored-by: Connor Sheehan <cosheehan@mozilla.com> Repo: add user_allowed method to check direct required_permission on User update for Repo.user_allowed tests: update fixtures for Try repo parameters create_environment_repos: disable commit message hook for try Update src/lando/try_api/api.py Co-authored-by: Zeid <2043828+zzzeid@users.noreply.github.com> auth: don't create Django users on API requests try_api: add test for base_commit_vcs values try_api: use SCM rather than VCS in user messaging try_api/tests: don't embed b64 literals create_environment_repos: DRY TRY_HOOKS try_api: move some patchhelper calls out of try/except auth: limit AccessTokenLandoOIDCAuthenticationBackend to token auth without fallback test: ensure that AccessTokenLandoOIDCAuthenticationBackend creates users correctly Apply suggestion from @shtrom Update src/lando/main/auth.py Co-authored-by: Zeid <2043828+zzzeid@users.noreply.github.com> auth: make AccessTokenLandoOIDCAuthenticationBackend behave more closely to OIDCAuthenticationBackend

shtrom · 2026-02-27T01:35:30Z

rebased on main

lando-prod-mozilla · 2026-02-27T01:51:02Z

Pull request closed by commit a9f2b20

shtrom force-pushed the bug1979247/try/patches branch 5 times, most recently from 9beacc1 to 19161a2 Compare August 15, 2025 06:44

shtrom force-pushed the bug1909723/auth0-access_token branch 2 times, most recently from 6d1386e to 3ac4ed5 Compare August 18, 2025 08:06

shtrom force-pushed the bug1979247/try/patches branch from 6bd2ed4 to 62e6e99 Compare August 26, 2025 04:28

Base automatically changed from bug1909723/auth0-access_token to main August 27, 2025 01:49

shtrom force-pushed the bug1979247/try/patches branch 2 times, most recently from 59e52b1 to e28e179 Compare September 3, 2025 05:26

shtrom changed the base branch from main to bug1983746/patchhelper-bytes September 3, 2025 05:46

shtrom force-pushed the bug1979247/try/patches branch from e28e179 to 8ddf2d4 Compare September 3, 2025 08:20

Base automatically changed from bug1983746/patchhelper-bytes to main September 4, 2025 01:01

shtrom force-pushed the bug1979247/try/patches branch 9 times, most recently from 04e0d91 to 69ac9bf Compare September 16, 2025 06:28

shtrom force-pushed the bug1979247/try/patches branch 7 times, most recently from a82fb77 to 24c340d Compare September 22, 2025 08:55

shtrom changed the base branch from main to perm-check January 12, 2026 07:34

shtrom force-pushed the bug1979247/try/patches branch from 9b87c41 to cf95e87 Compare January 12, 2026 07:39

shtrom changed the title ~~try_api: implement /patches endpoint (bug 1979247)~~ try_api: implement /patches endpoint (bug 1979247, bug 1983216) Jan 12, 2026

shtrom force-pushed the perm-check branch from fcad9e0 to 7c47ce6 Compare January 16, 2026 00:37

zzzeid requested changes Jan 16, 2026

View reviewed changes

shtrom requested a review from zzzeid January 19, 2026 03:43

zzzeid mentioned this pull request Jan 20, 2026

transplants: only check user's direct SCM permissions (Bug 1973426) #476

Closed

shtrom force-pushed the perm-check branch from ee1474e to c889d39 Compare January 22, 2026 04:18

shtrom changed the base branch from perm-check to main January 22, 2026 05:03

zzzeid requested changes Jan 28, 2026

View reviewed changes

zzzeid requested changes Jan 29, 2026

View reviewed changes

src/lando/main/auth.py Outdated Show resolved Hide resolved

src/lando/main/auth.py Outdated Show resolved Hide resolved

src/lando/main/auth.py Show resolved Hide resolved

src/lando/main/auth.py Outdated Show resolved Hide resolved

src/lando/try_api/api.py Show resolved Hide resolved

shtrom requested a review from zzzeid January 30, 2026 02:08

shtrom commented Jan 30, 2026

View reviewed changes

src/lando/utils/tests/test_ninja_auth.py Outdated Show resolved Hide resolved

zzzeid approved these changes Jan 30, 2026

View reviewed changes

src/lando/main/auth.py Outdated Show resolved Hide resolved

src/lando/main/auth.py Outdated Show resolved Hide resolved

shtrom force-pushed the bug1979247/try/patches branch 2 times, most recently from a1eb891 to c3318bb Compare February 4, 2026 04:14

shtrom mentioned this pull request Feb 24, 2026

scm: add SCMType enum to helpers (bug 2019139) #945

Closed

shtrom force-pushed the bug1979247/try/patches branch from 03c4ef2 to e51e03f Compare February 25, 2026 05:08

Conversation

shtrom commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shtrom commented Jan 19, 2026

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shtrom commented Jan 30, 2026

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shtrom commented Feb 4, 2026

Uh oh!

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

Uh oh!

Uh oh!

shtrom commented Feb 25, 2026

Uh oh!

shtrom commented Feb 26, 2026

Uh oh!

shtrom commented Feb 27, 2026

Uh oh!

lando-prod-mozilla bot commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shtrom commented Aug 13, 2025 •

edited

Loading

shtrom commented Jan 12, 2026 •

edited

Loading

shtrom commented Jan 30, 2026 •

edited

Loading

shtrom commented Feb 24, 2026 •

edited

Loading