fix: let org members create automations - #496
Merged
Merged
Conversation
Splitting the automation permission into view and manage (#415) put every creation endpoint behind manage_automations, which only admins and owners hold. Members could therefore never create an automation, which left the creator escape hatch that lets them edit, run and delete their own automations unreachable. Gate the four creation endpoints on view_automations instead: - POST /v1 - POST /v1/uploads - POST /v1/preset/prompt - POST /v1/preset/plugin An automation is always created under the caller's own user_id, so the row-level creator check the other write endpoints rely on is trivially satisfied on create. manage_automations keeps its meaning. Admins and owners can still turn off or delete any automation, only the creator can edit one (#427), and members stay read-only on automations they did not create. Add one test per endpoint asserting that a member can create.
5 tasks
Contributor
|
Warning Your comment is too long (maximum is 65536 characters), so the coverage report was not added. See the job log for how to reduce it. |
Contributor
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
Contributor
|
🚀 Released in 1.14.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Org members cannot create automations. Every creation path returns
403 Requires manage_automations permission.This dates back to #415, which split the automation permission into
view_automationsandmanage_automationsto make org members view-only. Members were moved toview_automationsonly, and all four creation endpoints were put behindmanage_automations:POST /v1create_automationPOST /v1/uploadscreate_uploadPOST /v1/preset/promptcreate_automation_from_promptPOST /v1/preset/plugincreate_automation_from_pluginThe requirement behind that split was that admins, owners and the creator of an automation have full access to it, while members can only view org automations. #415 built the creator escape hatch for that (
_assert_can_manage:manage_automationsorautomation.user_id == user.user_id), but with creation itself gated onmanage_automationsa member can never become a creator, so the hatch is unreachable for them. The requirement restricts what a member can do to other people's automations; it was not meant to stop them creating their own.The existing creator tests did not catch this because they insert the automation row directly instead of creating it through the API.
This PR gates the four creation endpoints on
view_automations. The other write endpoints already depend onview_automationsplus a row-level creator check. On create the caller is always the creator (user_id=user.user_id), so that check is trivially satisfied and the view-level dependency is the consistent gate.Behaviour
manage_automationskeeps its meaning, so the rule from #427 still holds: an admin can turn off or delete any automation, and only the creator can edit one.A member's automation already runs fine with
view_automationsalone: the run API key is minted on org membership,complete_run/report_run_phaseneed view plus the run owner, and KV needs org membership only. Members had fullmanage_automationsuntil #415, so this restores strictly less than what they had before.Changes
openhands/automation/router.pycreate_automationdepends on_require_view_automations. The now-unused_require_manage_automationsis removed.openhands/automation/uploads.pycreate_uploaddepends on_require_view_automations.openhands/automation/preset_router.py_require_view_automations, used by both preset endpoints.tests/test_router.pyTestPermissionEnforcement: a member canPOST /v1.tests/test_preset_router.pytests/test_uploads.pyTestCreateUpload: a member can upload a tarball. This is the first HTTP-level test ofPOST /v1/uploads.Not changed:
delete_upload(not part of creating, and already scoped to the caller's own uploads), the webhook and git-sync routers (org-level resources, stillmanage_automations),_assert_can_manage,update_automation, and the enterprise permission enum. Local mode is unaffected since the local user already holds both permissions.Why not grant members
manage_automationsOpenHands/enterprise#436 and #488 fix this from the other side, by giving members
manage_automationsand adding a newmanage_all_automationsfor admins and owners. I went a different way because this service treatsmanage_automationsas "is an admin or owner":_assert_can_managereturns as soon as it sees it, for any automation in the org._assert_can_managefor every automation, so a member could delete, run, cancel or turn off a teammate's automation. That undoes the view-only split from feat: split automation permissions into view and manage #415.{"enabled": false}path stays open to any member.Gating creation on
view_automationsneeds no enterprise change and no new permission, and it is safe in either deploy order.Companion PR
OpenHands/OpenHands#17526 (Agent Canvas): the automations list hides the "Add automation" / "Import" menu unless the caller has
manage_automations. The companion PR shows it to members. The other create entry points in Canvas (templates, empty state, home launcher, onboarding) were never gated and currently run into the 403 above.Either order works. If Canvas ships first, the menu appears and hits the same 403 those other entry points already hit. Shipping this first avoids that.
Test plan
Run locally (Docker is needed for the Postgres testcontainer;
FILE_STORE=localkeeps a local.envfrom pointing the suite at S3):main, one per endpoint, each with:4 passed.436 passed, 9 warnings in 91.29s. No existing test needed updating, as nothing asserted a 403 on a creation endpoint.0 errors, 0 warnings, 0 informations.POST /api/automation/v1/preset/prompt→ 201,user_idis the memberPOST /api/automation/v1/uploads?name=xthenPOST /api/automation/v1with the returnedoh-internal://path → 201PATCH/DELETEon a teammate's automation → 403 (unchanged)PATCH {"prompt": "x"}on a member's automation → 403,PATCH {"enabled": false}→ 200 (unchanged)Notes
server/auth/authorization.py) still says create is admin/owner only. It needs a comment-only follow-up.find_existing_template_automation), so whoever enables a template first owns it, and a later enable returns that same automation with a 200. This already happens between two admins. It becomes a little more likely once members can create, so it is worth a look separately.