Make the Role schema honestly reflect the composite values it accepts - #1342
Merged
Conversation
Permission is an IntFlag (READ=1, WRITE=2, DELETE=4), so pydantic's
runtime validation accepts any bitwise combination via IntFlag's
_missing_ hook -- but OpenAPI generation only lists Permission's
explicit members (1 | 2 | 4), never the composite values (0, 3, 7)
every role-granting endpoint actually requires. The frontend has been
working around this with manual type-widening.
Introduce Role, a plain IntEnum with exactly the four values the API
grants (NONE=0, READER=1, WRITER=3, OWNER=7), and use it everywhere a
role is accepted or returned over the API, so generated clients see
the real value set. Values Permission tolerated but Role doesn't (2,
4, 5, 6, 8, -1) are now rejected with 422 instead of silently
accepted.
Add exhaustive coverage: schema-level lock-in tests for Role's member
set, and endpoint tests for every valid Role value plus rejection of
every meaningful invalid value, on both PATCH /users/role and POST
/rbac/{type}/{id}/update-members.
tomchop
added a commit
to yeti-platform/yeti-docker
that referenced
this pull request
Aug 11, 2026
…eal RBAC enforcement (#41) * Add exhaustive e2e coverage for the Permission/Role schema fix Exercises the two UI paths that grant a role (yeti-platform/yeti#1342, yeti-platform/yeti-feeds-frontend#307): ACLEdit's per-object grants and a user's global role. rbac-group-membership.spec.ts now loops over every role ACLEdit's own UI offers (Reader/Writer/Owner) instead of just one, reopening the ACL dialog fresh each iteration since Escape (used to dismiss the identities combobox's own dropdown) isn't guaranteed to only close the topmost nested overlay. The new user-global-role.spec.ts covers UserProfile.vue's "Global role" combobox across all four values, including "No access" (0) -- the one boundary value ACLEdit's picker never offers, and the value the old Permission-flag-derived type could never express. * Add an e2e test for real RBAC permission enforcement, not just ACL storage The existing RBAC specs only ever run as admin, who bypasses every permission check, and RBAC is disabled on the integration stack -- so nothing so far actually exercises enforcement, only that ACL values get persisted correctly. Enable YETI_RBAC_ENABLED on the stack and add a test that logs a second, non-admin user's own browser session in alongside the admin's, then walks a single entity through group-based Owner access, revocation, Reader, Writer, and Owner again, asserting at each step both that the UI hides the actions that role shouldn't have (Edit/Delete/Share gating in ObjectDetails.vue/EditObject.vue) and that the backend rejects an unauthorized action attempted directly against the API, not just that the button is hidden. Parameterize helpers.ts's login() to accept a username/password, since this is the first spec that needs the browser itself logged in as a second user rather than just holding their bearer token for API calls. The initial grant targets the "All users" group rather than the second user directly, mirroring what yeti.conf's default_acls is meant to do automatically for every new object (it grants that group Owner, not merely read access, despite the config comment). That automatic grant turned out to intermittently and silently miss under load (Group.find() failing to find an existing group, permanently under-sharing the object with no retry) -- worth knowing about, but out of scope here -- so the test grants it explicitly through the same UI action instead of relying on it, for a deterministic starting point.
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
1 participant
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
Permissionis anIntFlag, so pydantic accepts any bitwise combination at runtime, but OpenAPI generation only lists its explicit members (1 | 2 | 4) -- never the composite values (0,3,7) that role-granting endpoints actually require. The frontend has been working around this with manual type-widening.Role, a plainIntEnumwith exactly the four values the API grants (NONE=0,READER=1,WRITER=3,OWNER=7), and uses it everywhere a role is accepted or returned over the API.Permissionused to silently accept (2,4,5,6,8,-1) are now correctly rejected with 422.Test plan
ruff check/ruff format --checkclean on all touched linesty checkcleanRole's member set, plus endpoint tests for every validRolevalue and rejection of every meaningful invalid value, on bothPATCH /users/roleandPOST /rbac/{type}/{id}/update-memberstests/schemas,tests/apiv2,tests/core_testsrun clean (only 2 pre-existing unrelated failures intasks.py, confirmed present on a cleanmainbaseline too)