Clarify authz comments - #11028
Conversation
Updates the authorization policy documentation in `omicron.polar`. - Emphasizes that role effects are resource-specific and similarly named roles do not guarantee identical permissions across scopes. - Clarifies that roles on parent resources do not automatically grant the same role or unrestricted access on child resources. Any such inheritance is explicitly defined. - Removes mentions of the `Organization` resource from the top-level predefined role list, as direct role assignments are currently limited to Fleet, Silo, and Project.
davepacheco
left a comment
davepacheco
left a comment
There was a problem hiding this comment.
Thanks for the cleanup.
| # granted by each role below. A role on one resource grants permissions or | ||
| # roles on related resources only where an explicit relation rule says so. | ||
| # Consequently, similarly named roles do not necessarily have identical effects | ||
| # at different scopes. |
There was a problem hiding this comment.
I appreciate what you're trying to clarify here, but I think it's also useful to say that we do reuse the terms "admin", "collaborator", and "viewer" (rather than creating lots of different synonyms) and that broadly they mean "full privileges", "read-only", and "something in between". What do you think?
There was a problem hiding this comment.
I think that makes sense! I had removed them to try and get ahead of any potential confusing regarding scope, but the heuristic descriptions are useful context for the rest of the doc. I'll add those back in.
| # - organization.collaborator (can manage Projects) | ||
| # - organization.viewer (can read most resources within the Organization) | ||
| # - project.admin (complete control over a Project) | ||
| # - fleet.admin (full control of a given Fleet and explicitly delegated fleet-scoped resources) |
There was a problem hiding this comment.
nit: These lines are over 80 columns
| # - fleet.viewer (can read most non-siloed resources in the Fleet, as well as explicitly exposed fleet-scoped resources) | ||
| # - silo.admin (full control of the Silo and its resources) | ||
| # - silo.collaborator (can create Projects and grants project.admin on all Projects within a given Silo) | ||
| # - silo.limited-collaborator (can create Projects and grants project.admin on all Projects within a given Silo) |
There was a problem hiding this comment.
I think the role this grants on the project is wrong. It's limited-collaborator, right?
There was a problem hiding this comment.
Whoops! My mistake, I copy-pasted the same line twice.
| # "Fleet" is a global singleton representing the whole system. The name comes | ||
| # from the idea described in RFD 24, but it's not quite right. This probably | ||
| # should be more like "Region" or "AvailabilityZone". The precise boundaries |
There was a problem hiding this comment.
nit: Is the only change here (and at L144) the change from two spaces to one after periods? I'm not sure I'd bother, given the rest of the file still uses two spaces.
There was a problem hiding this comment.
Good point. Those were the instances that I noticed by eye, but I should've just let ctrl+F do the work. I'll leave the convention be.
| # One reason this is necessary is because if an unprivileged user tries to | ||
| # create an Organization using "POST /organizations", they should get back a 403 | ||
| # (which implies they're able to see /organizations, which is essentially seeing | ||
| # the Silo itself) rather than a 404. This behavior isn't a hard constraint | ||
| # (i.e., you could reasonably get a 404 for an API you're not allowed to call). | ||
| # Nor is the implementation (i.e., we could special-case this endpoint somehow). | ||
| # But granting this permission is the simplest way to keep this endpoint's | ||
| # behavior consistent with the rest of the API. |
There was a problem hiding this comment.
I believe this is still true if you s/organization/project/, FWIW. We may not need the comment any more now that we have the policy test.
There was a problem hiding this comment.
Ah! In that case, it could still be worth mentioning the why between the two status codes, so I'll add the bulk of the comment back in, just focussing on projects instead of organizations.
Thank you for taking a look! Going through your comments now. |
2 participants
Updates the authorization policy documentation in
omicron.polar.Organizationresource from the top-level predefined role list.