chore: authz 'any_org' to return if at least 1 org has perms #14009

Emyrk · 2024-07-25T02:15:58Z

What this does

The UI needs to ask the question, "Can this user create a template?". This question is now complicated with multi-organizations, since the answer is true if the user can create the template in any org they are a member of.

The UI today would have to on every page:

Fetch all organizations
Loop over the organizations, create a /authcheck for each org for create template
- Add create template at the site wide too for site admins

This feels like it could add to the number of db queries for a question that doesn't actually require the database. This question can be answered entirely by the policy in memory, since it has the roles and permissions.

So I added a field to an object called any_org, that when specified makes the policy return "true" for the org_allow section if any organization would return a truthy value.

This is a policy change, however we can rely on our extensive rbac testing to avoid regressions.

Closes #14003

Alternative

Rego supports, and we use, partial queries. This is possible with 0 modification of the original policy, meaning we do not need to input this as a first class feature. This was the original plan.

The problem with this plan, is the resulting query to inspect and determine if we should return true/false looks something like:

+---------+---------------------------------------------------------------------------------+
| Query 1 | "5750d635-82e0-4681-bd44-815b18669d65" = input.object.org_owner                 |
+---------+---------------------------------------------------------------------------------+
| Query 2 | input.object.org_owner != ""                                                    |
|         | input.object.org_owner in {"5750d635-82e0-4681-bd44-815b18669d65"}              |
|         | "create" in input.object.acl_group_list["b617a647-b5d0-4cbe-9e40-26f89710bf18"] |
+---------+---------------------------------------------------------------------------------+
| Query 3 | input.object.org_owner != ""                                                    |
|         | input.object.org_owner in {"5750d635-82e0-4681-bd44-815b18669d65"}              |
|         | "*" in input.object.acl_group_list["b617a647-b5d0-4cbe-9e40-26f89710bf18"]      |
+---------+---------------------------------------------------------------------------------+
| Query 4 | input.object.org_owner != ""                                                    |
|         | input.object.org_owner in {"5750d635-82e0-4681-bd44-815b18669d65"}              |
|         | "create" in input.object.acl_group_list[input.object.org_owner]                 |
+---------+---------------------------------------------------------------------------------+
| Query 5 | input.object.org_owner != ""                                                    |
|         | input.object.org_owner in {"5750d635-82e0-4681-bd44-815b18669d65"}              |
|         | "*" in input.object.acl_group_list[input.object.org_owner]                      |
+---------+---------------------------------------------------------------------------------+
| Query 6 | "create" in input.object.acl_user_list["10d03e62-7703-4df5-a358-4f76577d4e2f"]  |
+---------+---------------------------------------------------------------------------------+
| Query 7 | "*" in input.object.acl_user_list["10d03e62-7703-4df5-a358-4f76577d4e2f"]       |
+---------+---------------------------------------------------------------------------------+

There is two ways to take this output and make it useful.

Conduct some AST analysis and figure out if there exists and org_id in which at least 1 query returns true. This feels trivial as long as our output ASTs remain static. Any policy change could probably break any first draft AST analysis. FRAGILE

^--- That would be my choice given unlimited time

The second idea is to execute the queries with various inputs. We can eval() each query with a different input.object, and some weak ast analysis could probably pick out some uuids to throw back in. This would use proper rego evaluation, and be strong and consistent against changes. This however would result in some loop of execution, becoming a O(n) problem.

In the end, if we are going to use rego eval() to conclude the result, it makes the most optimization sense just to bake this into the policy itself. Determine the result on the first eval(), rather than trying to rig up something externally.

Allows checking if a user can do an action in any organization, rather than a specific one. Allows asking general questions on the UI to determine which elements to show.

johnstcn · 2024-07-26T10:40:17Z

coderd/rbac/roles_test.go

+		{
+			Name:     "CreateTemplateAnyOrg",
+			Actions:  []policy.Action{policy.ActionCreate},
+			Resource: rbac.ResourceTemplate.AnyOrganization(),
+			AuthorizeMap: map[bool][]hasAuthSubjects{
+				true: {owner, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, orgAdmin, otherOrgAdmin},
+				false: {
+					userAdmin, memberMe,
+					orgMemberMe, orgAuditor, orgUserAdmin,
+					otherOrgMember, otherOrgAuditor, otherOrgUserAdmin,
+				},
+			},
+		},


Can you extend these tests to other organization-scoped RBAC objects?
I tried this out for workspaces and ran into behaviour I didn't expect:

{ Name: "CreateWorkspaceAnyOrg", Actions: []policy.Action{policy.ActionCreate}, Resource: rbac.ResourceWorkspace.AnyOrganization(), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, otherOrgAdmin, orgMemberMe}, false: { memberMe, userAdmin, templateAdmin, orgAuditor, orgUserAdmin, orgTemplateAdmin, otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, }, },

--- FAIL: TestRolePermissions/CreateWorkspaceAnyOrg (0.01s) roles_test.go:675: Error Trace: /workspaces/coder/coderd/rbac/roles_test.go:675 Error: Received unexpected error: rbac: forbidden: (subject: { f8981f20-e4cc-4be9-a14c-c391200d086e [member organization-member:2c498c97-8c96-4742-9e37-3cd431780fd3] [] all <nil>}), (action: create), (object: { true workspace map[] map[]}), (output: []) Test: TestRolePermissions/CreateWorkspaceAnyOrg Messages: Should pass: CreateWorkspaceAnyOrg as "org_member_me" doing "create" on "workspace"

I can add more tests for sure. The test you wrote is missing with WithOwner. The test as is, asks the question "Can I create a workspace in any organization that is not owned by me?"

The correct question to see if you can make a workspace belonging to yourself is:

rbac.ResourceWorkspace.AnyOrganization().WithOwner(currentUser.String())

The reason the test without the owner works for some roles, is because owners and org admins are able to create workspaces on behalf of other users

Ah whoops 👍 I'm still holding the package wrong!

It's not perfect for sure. Maybe there is some api wrapper that could exist that translates everything into some more readable language 🤷‍♂️.

I agree it's nuanced

johnstcn · 2024-07-26T10:54:20Z

coderd/rbac/policy.rego

I checked the RBAC benchmarks before/after and didn't see any concerning differences here 👍

johnstcn · 2024-07-26T10:54:30Z

coderd/rbac/policy.rego

+	count(org_members) > 0
+}
+
+


Suggested change

coderd/rbac/authz.go

johnstcn

We need to be extremely careful of how we use AnyOrgOwner. I could see accidental usage of this in dbauthz or similar causing a permissions bug. As this is primarily meant for the UI, it might be good as a follow-up to add a linter for any usage of this outside the rbac package in Go, or in the endpoint for checking permissions.

Emyrk · 2024-07-29T13:16:49Z

We need to be extremely careful of how we use AnyOrgOwner. I could see accidental usage of this in dbauthz or similar causing a permissions bug. As this is primarily meant for the UI, it might be good as a follow-up to add a linter for any usage of this outside the rbac package in Go, or in the endpoint for checking permissions.

Good idea with the linter. I agree it should be protected somehow

Emyrk added 2 commits July 24, 2024 20:55

chore: authz 'any_org' to return if at least 1 org has perms

e8e940e

Allows checking if a user can do an action in any organization, rather than a specific one. Allows asking general questions on the UI to determine which elements to show.

more strict, add comments to policy

3ea9020

github-actions bot assigned Emyrk Jul 25, 2024

Emyrk marked this pull request as draft July 25, 2024 02:17

add unit tests and extend to /authcheck api

3b0fae0

Emyrk marked this pull request as ready for review July 25, 2024 11:30

Kira-Pilot requested review from johnstcn and spikecurtis July 25, 2024 11:33

make field optional

6b3e053

johnstcn reviewed Jul 26, 2024

View reviewed changes

Emyrk added 4 commits July 26, 2024 08:55

add unit tests for create a workspace in any org

9cd9c82

use const over uuid.Nil.String()

ad298a8

make fmt

f5c4f9a

add unit test

40a8786

johnstcn approved these changes Jul 29, 2024

View reviewed changes

Kira-Pilot mentioned this pull request Jul 29, 2024

Solidify permissions and licensing checks in the UI for Org feature set #13915

Closed

13 tasks

Emyrk merged commit 3209c86 into main Jul 30, 2024
33 checks passed

Emyrk deleted the stevenmasley/authcheck_any_org branch July 30, 2024 00:58

github-actions bot locked and limited conversation to collaborators Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: authz 'any_org' to return if at least 1 org has perms #14009

chore: authz 'any_org' to return if at least 1 org has perms #14009

Uh oh!

Emyrk commented Jul 25, 2024 •

edited

Loading

Uh oh!

johnstcn Jul 26, 2024

Uh oh!

Emyrk Jul 26, 2024

Uh oh!

johnstcn Jul 26, 2024 •

edited

Loading

Uh oh!

Emyrk Jul 26, 2024

Uh oh!

johnstcn Jul 26, 2024

Uh oh!

johnstcn Jul 26, 2024

Uh oh!

Uh oh!

johnstcn left a comment

Uh oh!

Emyrk commented Jul 29, 2024

Uh oh!

Uh oh!

Uh oh!

chore: authz 'any_org' to return if at least 1 org has perms #14009

chore: authz 'any_org' to return if at least 1 org has perms #14009

Uh oh!

Conversation

Emyrk commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

Alternative

Uh oh!

johnstcn Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

Emyrk Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

johnstcn Jul 26, 2024 • 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!

Emyrk Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

johnstcn Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

johnstcn Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

Emyrk commented Jul 29, 2024

Uh oh!

Uh oh!

Uh oh!

Emyrk commented Jul 25, 2024 •

edited

Loading

johnstcn Jul 26, 2024 •

edited

Loading