fix!: require org membership for user ACLs (#26852) (#27884)

jdomeracki-coder · sreya · web-flow · commit 85bcd5d3a3d7 · 2026-08-06T14:45:00.000+02:00
Backport of #26852 Original PR: #26852 — fix!: require org membership for user ACLs Merge commit: 6b3341a Requested by: @jdomeracki-coder Clean cherry-pick, no conflicts. --- _Opened by Coder Agents on behalf of @jdomeracki-coder._ Co-authored-by: Jon Ayers <jon@coder.com>
diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go
@@ -312,19 +312,35 @@ func TestAuthorizeDomain(t *testing.T) {
 
 	testAuthorize(t, "UserACLList", user, []authTestCase{
 		{
-			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(unusedID).WithACLUserList(map[string][]policy.Action{
+			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(defOrg).WithACLUserList(map[string][]policy.Action{
 				user.ID: ResourceWorkspace.AvailableActions(),
 			}),
 			actions: ResourceWorkspace.AvailableActions(),
 			allow:   true,
 		},
 		{
-			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(unusedID).WithACLUserList(map[string][]policy.Action{
+			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(defOrg).WithACLUserList(map[string][]policy.Action{
 				user.ID: {policy.WildcardSymbol},
 			}),
 			actions: ResourceWorkspace.AvailableActions(),
 			allow:   true,
 		},
+		{
+			// User ACLs only grant permissions in organizations where the
+			// subject is currently a member.
+			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(unusedID).WithACLUserList(map[string][]policy.Action{
+				user.ID: ResourceWorkspace.AvailableActions(),
+			}),
+			actions: ResourceWorkspace.AvailableActions(),
+			allow:   false,
+		},
+		{
+			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(unusedID).WithACLUserList(map[string][]policy.Action{
+				user.ID: {policy.WildcardSymbol},
+			}),
+			actions: ResourceWorkspace.AvailableActions(),
+			allow:   false,
+		},
 		{
 			resource: ResourceWorkspace.WithOwner(unusedID.String()).InOrg(unusedID).WithACLUserList(map[string][]policy.Action{
 				user.ID: {policy.ActionRead, policy.ActionUpdate},
@@ -714,6 +730,93 @@ func TestAuthorizeDomain(t *testing.T) {
 		}))
 }
 
+// TestAuthorizeUserACLOrgMembership verifies that user ACL grants require org
+// membership, while site-wide roles still authorize independent of ACLs.
+func TestAuthorizeUserACLOrgMembership(t *testing.T) {
+	t.Parallel()
+
+	orgID := uuid.New()
+
+	// Site template-admin, not a member of orgID.
+	siteTemplateAdmin := Subject{
+		ID:    "site-template-admin",
+		Scope: must(ExpandScope(ScopeAll)),
+		Roles: Roles{
+			must(RoleByName(RoleMember())),
+			must(RoleByName(RoleTemplateAdmin())),
+		},
+	}
+	testAuthorize(t, "SiteTemplateAdminNotInOrg", siteTemplateAdmin, []authTestCase{
+		{
+			// Authorized by the site role, no ACL needed.
+			resource: ResourceTemplate.InOrg(orgID),
+			actions:  []policy.Action{policy.ActionUpdate},
+			allow:    true,
+		},
+		{
+			// Redundant ACL entry; still authorized by the site role.
+			resource: ResourceTemplate.InOrg(orgID).WithACLUserList(map[string][]policy.Action{
+				siteTemplateAdmin.ID: {policy.ActionUpdate},
+			}),
+			actions: []policy.Action{policy.ActionUpdate},
+			allow:   true,
+		},
+	})
+
+	// Site user-admin (no template perms), not a member of orgID.
+	siteUserAdmin := Subject{
+		ID:    "site-user-admin",
+		Scope: must(ExpandScope(ScopeAll)),
+		Roles: Roles{
+			must(RoleByName(RoleMember())),
+			must(RoleByName(RoleUserAdmin())),
+		},
+	}
+	testAuthorize(t, "SiteUserAdminNotInOrg", siteUserAdmin, []authTestCase{
+		{
+			// No template role and no ACL entry: denied.
+			resource: ResourceTemplate.InOrg(orgID),
+			actions:  []policy.Action{policy.ActionUpdate},
+			allow:    false,
+		},
+		{
+			// An ACL grant must not authorize a non-member.
+			resource: ResourceTemplate.InOrg(orgID).WithACLUserList(map[string][]policy.Action{
+				siteUserAdmin.ID: {policy.ActionUpdate},
+			}),
+			actions: []policy.Action{policy.ActionUpdate},
+			allow:   false,
+		},
+	})
+
+	// Same site user-admin, now also a member of orgID.
+	siteUserAdminOrgMember := Subject{
+		ID:    "site-user-admin-org-member",
+		Scope: must(ExpandScope(ScopeAll)),
+		Roles: Roles{
+			must(RoleByName(RoleMember())),
+			must(RoleByName(RoleUserAdmin())),
+			orgMemberRole(orgID),
+		},
+	}
+	testAuthorize(t, "SiteUserAdminOrgMember", siteUserAdminOrgMember, []authTestCase{
+		{
+			// Org membership alone does not grant template update.
+			resource: ResourceTemplate.InOrg(orgID),
+			actions:  []policy.Action{policy.ActionUpdate},
+			allow:    false,
+		},
+		{
+			// As an org member, the ACL grant takes effect.
+			resource: ResourceTemplate.InOrg(orgID).WithACLUserList(map[string][]policy.Action{
+				siteUserAdminOrgMember.ID: {policy.ActionUpdate},
+			}),
+			actions: []policy.Action{policy.ActionUpdate},
+			allow:   true,
+		},
+	})
+}
+
 // TestAuthorizeLevels ensures level overrides are acting appropriately
 func TestAuthorizeLevels(t *testing.T) {
 	t.Parallel()
diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego
@@ -330,7 +330,9 @@ object_is_included_in_scope_allow_list if {
 
 # ACL for users
 acl_allow if {
-	# TODO: Should you have to be a member of the org too?
+	# The subject must be a member of the object's organization for a
+	# user ACL grant to apply.
+	is_org_member
 	perms := input.object.acl_user_list[input.subject.id]
 
 	# Check if either the action or * is allowed
