chore: improve rbac and add benchmark tooling (#18584)

## Description

This PR improves the RBAC package by refactoring the policy, enhancing
documentation, and adding utility scripts.

## Changes

* Refactored `policy.rego` for clarity and readability
* Updated README with OPA section
* Added `benchmark_authz.sh` script for authz performance testing and
comparison
* Added `gen_input.go` to generate input for `opa eval` testing

coderd/rbac/README.md

@@ -102,18 +102,106 @@ Example of a scope for a workspace agent token, using an `allow_list` containing
     }
 ```
 
+## OPA (Open Policy Agent)
+
+Open Policy Agent (OPA) is an open source tool used to define and enforce policies.
+Policies are written in a high-level, declarative language called Rego.
+Coder’s RBAC rules are defined in the [`policy.rego`](policy.rego) file under the `authz` package.
+
+When OPA evaluates policies, it binds input data to a global variable called `input`.
+In the `rbac` package, this structured data is defined as JSON and contains the action, object and subject (see `regoInputValue` in [astvalue.go](astvalue.go)).
+OPA evaluates whether the subject is allowed to perform the action on the object across three levels: `site`, `org`, and `user`.
+This is determined by the final rule `allow`, which aggregates the results of multiple rules to decide if the user has the necessary permissions.
+Similarly to the input, OPA produces structured output data, which includes the `allow` variable as part of the evaluation result.
+Authorization succeeds only if `allow` explicitly evaluates to `true`. If no `allow` is returned, it is considered unauthorized.
+To learn more about OPA and Rego, see https://www.openpolicyagent.org/docs.
+
+### Application and Database Integration
+
+- [`rbac/authz.go`](authz.go) – Application layer integration: provides the core authorization logic that integrates with Rego for policy evaluation.
+- [`database/dbauthz/dbauthz.go`](../database/dbauthz/dbauthz.go) – Database layer integration: wraps the database layer with authorization checks to enforce access control.
+
+There are two types of evaluation in OPA:
+
+- **Full evaluation**: Produces a decision that can be enforced.
+This is the default evaluation mode, where OPA evaluates the policy using `input` data that contains all known values and returns output data with the `allow` variable.
+- **Partial evaluation**: Produces a new policy that can be evaluated later when the _unknowns_ become _known_.
+This is an optimization in OPA where it evaluates as much of the policy as possible without resolving expressions that depend on _unknown_ values from the `input`.
+To learn more about partial evaluation, see this [OPA blog post](https://blog.openpolicyagent.org/partial-evaluation-162750eaf422).
+
+Application of Full and Partial evaluation in `rbac` package:
+
+- **Full Evaluation** is handled by the `RegoAuthorizer.Authorize()` method in [`authz.go`](authz.go).
+This method determines whether a subject (user) can perform a specific action on an object.
+It performs a full evaluation of the Rego policy, which returns the `allow` variable to decide whether access is granted (`true`) or denied (`false` or undefined).
+- **Partial Evaluation** is handled by the `RegoAuthorizer.Prepare()` method in [`authz.go`](authz.go).
+This method compiles OPA’s partial evaluation queries into `SQL WHERE` clauses.
+These clauses are then used to enforce authorization directly in database queries, rather than in application code.
+
+Authorization Patterns:
+
+- Fetch-then-authorize: an object is first retrieved from the database, and a single authorization check is performed using full evaluation via `Authorize()`.
+- Authorize-while-fetching: Partial evaluation via `Prepare()` is used to inject SQL filters directly into queries, allowing efficient authorization of many objects of the same type.
+`dbauthz` methods that enforce authorization directly in the SQL query are prefixed with `Authorized`, for example, `GetAuthorizedWorkspaces`.
+
 ## Testing
 
-You can test outside of golang by using the `opa` cli.
+- OPA Playground: https://play.openpolicyagent.org/
+- OPA CLI (`opa eval`): useful for experimenting with different inputs and understanding how the policy behaves under various conditions.
+`opa eval` returns the constraints that must be satisfied for a rule to evaluate to `true`.
+  - `opa eval` requires an `input.json` file containing the input data to run the policy against.
+    You can generate this file using the [gen_input.go](../../scripts/rbac-authz/gen_input.go) script.
+    Note: the script currently produces a fixed input. You may need to tweak it for your specific use case.
 
-**Evaluation**
+### Full Evaluation
 
 ```bash
 opa eval --format=pretty "data.authz.allow" -d policy.rego -i input.json
 ```
 
-**Partial Evaluation**
+This command fully evaluates the policy in the `policy.rego` file using the input data from `input.json`, and returns the result of the `allow` variable:
+
+- `data.authz.allow` accesses the `allow` rule within the `authz` package.
+- `data.authz` on its own would return the entire output object of the package.
+
+This command answers the question: “Is the user allowed?”
+
+### Partial Evaluation
 
 ```bash
 opa eval --partial --format=pretty 'data.authz.allow' -d policy.rego --unknowns input.object.owner --unknowns input.object.org_owner --unknowns input.object.acl_user_list --unknowns input.object.acl_group_list -i input.json
 ```
+
+This command performs a partial evaluation of the policy, specifying a set of unknown input parameters.
+The result is a set of partial queries that can be converted into `SQL WHERE` clauses and injected into SQL queries.
+
+This command answers the question: “What conditions must be met for the user to be allowed?”
+
+### Benchmarking
+
+Benchmark tests to evaluate the performance of full and partial evaluation can be found in `authz_test.go`.
+You can run these tests with the `-bench` flag, for example:
+
+```bash
+go test -bench=BenchmarkRBACFilter -run=^$
+```
+
+To capture memory and CPU profiles, use the following flags:
+
+- `-memprofile memprofile.out`
+- `-cpuprofile cpuprofile.out`
+
+The script [`benchmark_authz.sh`](../../scripts/rbac-authz/benchmark_authz.sh) runs the `authz` benchmark tests on the current Git branch or compares benchmark results between two branches using [`benchstat`](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat).
+`benchstat` compares the performance of a baseline benchmark against a new benchmark result and highlights any statistically significant differences.
+
+- To run benchmark on the current branch:
+
+    ```bash
+    benchmark_authz.sh --single
+    ```
+
+- To compare benchmarks between 2 branches:
+
+    ```bash
+    benchmark_authz.sh --compare main prebuild_policy
+    ```

coderd/rbac/authz_test.go

@@ -148,7 +148,7 @@ func benchmarkUserCases() (cases []benchmarkCase, users uuid.UUID, orgs []uuid.U
 
 // BenchmarkRBACAuthorize benchmarks the rbac.Authorize method.
 //
-//	go test -run=^$ -bench BenchmarkRBACAuthorize -benchmem -memprofile memprofile.out -cpuprofile profile.out
+//	go test -run=^$ -bench '^BenchmarkRBACAuthorize$' -benchmem -memprofile memprofile.out -cpuprofile profile.out
 func BenchmarkRBACAuthorize(b *testing.B) {
 	benchCases, user, orgs := benchmarkUserCases()
 	users := append([]uuid.UUID{},
@@ -178,7 +178,7 @@ func BenchmarkRBACAuthorize(b *testing.B) {
 // BenchmarkRBACAuthorizeGroups benchmarks the rbac.Authorize method and leverages
 // groups for authorizing rather than the permissions/roles.
 //
-//	go test -bench BenchmarkRBACAuthorizeGroups -benchmem -memprofile memprofile.out -cpuprofile profile.out
+//	go test -bench '^BenchmarkRBACAuthorizeGroups$' -benchmem -memprofile memprofile.out -cpuprofile profile.out
 func BenchmarkRBACAuthorizeGroups(b *testing.B) {
 	benchCases, user, orgs := benchmarkUserCases()
 	users := append([]uuid.UUID{},
@@ -229,7 +229,7 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) {
 
 // BenchmarkRBACFilter benchmarks the rbac.Filter method.
 //
-//	go test -bench BenchmarkRBACFilter -benchmem -memprofile memprofile.out -cpuprofile profile.out
+//	go test -bench '^BenchmarkRBACFilter$' -benchmem -memprofile memprofile.out -cpuprofile profile.out
 func BenchmarkRBACFilter(b *testing.B) {
 	benchCases, user, orgs := benchmarkUserCases()
 	users := append([]uuid.UUID{},

coderd/rbac/policy.rego

@@ -29,76 +29,93 @@ import rego.v1
 #    different code branches based on the org_owner. 'num's value does, but
 #    that is the whole point of partial evaluation.
 
-# bool_flip lets you assign a value to an inverted bool.
+# bool_flip(b) returns the logical negation of a boolean value 'b'.
 # You cannot do 'x := !false', but you can do 'x := bool_flip(false)'
-bool_flip(b) := flipped if {
+bool_flip(b) := false if {
 	b
-	flipped = false
 }
 
-bool_flip(b) := flipped if {
+bool_flip(b) := true if {
 	not b
-	flipped = true
 }
 
-# number is a quick way to get a set of {true, false} and convert it to
+# number(set) maps a set of boolean values to one of the following numbers:
-#  -1: {false, true} or {false}
+#  -1: deny (if 'false' value is in the set)		=> set is {true, false} or {false}
-#   0: {}
+#   0: no decision (if the set is empty)			=> set is {}
-#   1: {true}
+#   1: allow (if only 'true' values are in the set)	=> set is {true}
-number(set) := c if {
-	count(set) == 0
-	c := 0
-}
 
-number(set) := c if {
+# Return -1 if the set contains any 'false' value (i.e., an explicit deny)
+number(set) := -1 if {
 	false in set
-	c := -1
 }
 
-number(set) := c if {
+# Return 0 if the set is empty (no matching permissions)
+number(set) := 0 if {
+	count(set) == 0
+}
+
+# Return 1 if the set is non-empty and contains no 'false' values (i.e., only allows)
+number(set) := 1 if {
 	not false in set
 	set[_]
-	c := 1
 }
 
-# site, org, and user rules are all similar. Each rule should return a number
+# Permission evaluation is structured into three levels: site, org, and user.
-# from [-1, 1]. The number corresponds to "negative", "abstain", and "positive"
+# For each level, two variables are computed:
-# for the given level. See the 'allow' rules for how these numbers are used.
+#   - <level>: the decision based on the subject's full set of roles for that level
-default site := 0
+#   - scope_<level>: the decision based on the subject's scoped roles for that level
+#
+# Each of these variables is assigned one of three values:
+#   -1 => negative (deny)
+#    0 => abstain (no matching permission)
+#    1 => positive (allow)
+#
+# These values are computed by calling the corresponding <level>_allow functions.
+# The final decision is derived from combining these values (see 'allow' rule).
 
+# -------------------
+# Site Level Rules
+# -------------------
+
+default site := 0
 site := site_allow(input.subject.roles)
 
 default scope_site := 0
-
 scope_site := site_allow([input.subject.scope])
 
+# site_allow receives a list of roles and returns a single number:
+#  -1 if any matching permission denies access
+#   1 if there's at least one allow and no denies
+#   0 if there are no matching permissions
 site_allow(roles) := num if {
-	# allow is a set of boolean values without duplicates.
+	# allow is a set of boolean values (sets don't contain duplicates)
-	allow := {x |
+	allow := {is_allowed |
 		# Iterate over all site permissions in all roles
 		perm := roles[_].site[_]
 		perm.action in [input.action, "*"]
 		perm.resource_type in [input.object.type, "*"]
 
-		# x is either 'true' or 'false' if a matching permission exists.
+		# is_allowed is either 'true' or 'false' if a matching permission exists.
-		x := bool_flip(perm.negate)
+		is_allowed := bool_flip(perm.negate)
 	}
 	num := number(allow)
 }
 
+# -------------------
+# Org Level Rules
+# -------------------
+
 # org_members is the list of organizations the actor is apart of.
 org_members := {orgID |
 	input.subject.roles[_].org[orgID]
 }
 
-# org is the same as 'site' except we need to iterate over each organization
+# 'org' is the same as 'site' except we need to iterate over each organization
 # that the actor is a member of.
 default org := 0
-
 org := org_allow(input.subject.roles)
 
 default scope_org := 0
-
 scope_org := org_allow([input.scope])
 
 # org_allow_set is a helper function that iterates over all orgs that the actor
@@ -114,11 +131,14 @@ scope_org := org_allow([input.scope])
 org_allow_set(roles) := allow_set if {
 	allow_set := {id: num |
 		id := org_members[_]
-		set := {x |
+		set := {is_allowed |
+			# Iterate over all org permissions in all roles
 			perm := roles[_].org[id][_]
 			perm.action in [input.action, "*"]
 			perm.resource_type in [input.object.type, "*"]
-			x := bool_flip(perm.negate)
+
+			# is_allowed is either 'true' or 'false' if a matching permission exists.
+			is_allowed := bool_flip(perm.negate)
 		}
 		num := number(set)
 	}
@@ -191,24 +211,30 @@ org_ok if {
 	not input.object.any_org
 }
 
-# User is the same as the site, except it only applies if the user owns the object and
+# -------------------
+# User Level Rules
+# -------------------
+
+# 'user' is the same as 'site', except it only applies if the user owns the object and
 # the user is apart of the org (if the object has an org).
 default user := 0
-
 user := user_allow(input.subject.roles)
 
-default user_scope := 0
+default scope_user := 0
-
 scope_user := user_allow([input.scope])
 
 user_allow(roles) := num if {
 	input.object.owner != ""
 	input.subject.id = input.object.owner
-	allow := {x |
+
+	allow := {is_allowed |
+		# Iterate over all user permissions in all roles
 		perm := roles[_].user[_]
 		perm.action in [input.action, "*"]
 		perm.resource_type in [input.object.type, "*"]
-		x := bool_flip(perm.negate)
+
+		# is_allowed is either 'true' or 'false' if a matching permission exists.
+		is_allowed := bool_flip(perm.negate)
 	}
 	num := number(allow)
 }
@@ -227,17 +253,9 @@ scope_allow_list if {
 	input.object.id in input.subject.scope.allow_list
 }
 
-# The allow block is quite simple. Any set with `-1` cascades down in levels.
+# -------------------
-# Authorization looks for any `allow` statement that is true. Multiple can be true!
+# Role-Specific Rules
-# Note that the absence of `allow` means "unauthorized".
+# -------------------
-# An explicit `"allow": true` is required.
-#
-# Scope is also applied. The default scope is "wildcard:wildcard" allowing
-# all actions. If the scope is not "1", then the action is not authorized.
-#
-#
-# Allow query:
-#	 data.authz.role_allow = true data.authz.scope_allow = true
 
 role_allow if {
 	site = 1
@@ -258,6 +276,10 @@ role_allow if {
 	user = 1
 }
 
+# -------------------
+# Scope-Specific Rules
+# -------------------
+
 scope_allow if {
 	scope_allow_list
 	scope_site = 1
@@ -280,6 +302,11 @@ scope_allow if {
 	scope_user = 1
 }
 
+# -------------------
+# ACL-Specific Rules
+# Access Control List
+# -------------------
+
 # ACL for users
 acl_allow if {
 	# Should you have to be a member of the org too?
@@ -308,11 +335,24 @@ acl_allow if {
 	[input.action, "*"][_] in perms
 }
 
-###############
+# -------------------
 # Final Allow
+#
+# The 'allow' block is quite simple. Any set with `-1` cascades down in levels.
+# Authorization looks for any `allow` statement that is true. Multiple can be true!
+# Note that the absence of `allow` means "unauthorized".
+# An explicit `"allow": true` is required.
+#
+# Scope is also applied. The default scope is "wildcard:wildcard" allowing
+# all actions. If the scope is not "1", then the action is not authorized.
+#
+# Allow query:
+#	data.authz.role_allow = true
+#	data.authz.scope_allow = true
+# -------------------
+
 # The role or the ACL must allow the action. Scopes can be used to limit,
 # so scope_allow must always be true.
-
 allow if {
 	role_allow
 	scope_allow

scripts/rbac-authz/benchmark_authz.sh

@@ -0,0 +1,85 @@
+#!/usr/bin/env bash
+
+# Run rbac authz benchmark tests on the current Git branch or compare benchmark results
+# between two branches using `benchstat`.
+#
+# The script supports:
+# 1) Running benchmarks and saving output to a file.
+# 2) Checking out two branches, running benchmarks on each, and saving the `benchstat`
+# comparison results to a file.
+# Benchmark results are saved with filenames based on the branch name.
+#
+# Usage:
+#   benchmark_authz.sh --single                       # Run benchmarks on current branch
+#   benchmark_authz.sh --compare <branchA> <branchB>  # Compare benchmarks between two branches
+
+set -euo pipefail
+
+# Go benchmark parameters
+GOMAXPROCS=16
+TIMEOUT=30m
+BENCHTIME=5s
+COUNT=5
+
+# Script configuration
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+OUTPUT_DIR="${SCRIPT_DIR}/benchmark_outputs"
+
+# List of benchmark tests
+BENCHMARKS=(
+	BenchmarkRBACAuthorize
+	BenchmarkRBACAuthorizeGroups
+	BenchmarkRBACFilter
+)
+
+# Create output directory
+mkdir -p "$OUTPUT_DIR"
+
+function run_benchmarks() {
+	local branch=$1
+	# Replace '/' with '-' for branch names with format user/branchName
+	local filename_branch=${branch//\//-}
+	local output_file_prefix="$OUTPUT_DIR/${filename_branch}"
+
+	echo "Checking out $branch..."
+	git checkout "$branch"
+
+	# Move into the rbac directory to run the benchmark tests
+	pushd ../../coderd/rbac/ >/dev/null
+
+	for bench in "${BENCHMARKS[@]}"; do
+		local output_file="${output_file_prefix}_${bench}.txt"
+		echo "Running benchmark $bench on $branch..."
+		GOMAXPROCS=$GOMAXPROCS go test -timeout $TIMEOUT -bench="^${bench}$" -run=^$ -benchtime=$BENCHTIME -count=$COUNT | tee "$output_file"
+	done
+
+	# Return to original directory
+	popd >/dev/null
+}
+
+if [[ $# -eq 0 || "${1:-}" == "--single" ]]; then
+	current_branch=$(git rev-parse --abbrev-ref HEAD)
+	run_benchmarks "$current_branch"
+elif [[ "${1:-}" == "--compare" ]]; then
+	base_branch=$2
+	test_branch=$3
+
+	# Run all benchmarks on both branches
+	run_benchmarks "$base_branch"
+	run_benchmarks "$test_branch"
+
+	# Compare results benchmark by benchmark
+	for bench in "${BENCHMARKS[@]}"; do
+		# Replace / with - for branch names with format user/branchName
+		filename_base_branch=${base_branch//\//-}
+		filename_test_branch=${test_branch//\//-}
+
+		echo -e "\nGenerating benchmark diff for $bench using benchstat..."
+		benchstat "$OUTPUT_DIR/${filename_base_branch}_${bench}.txt" "$OUTPUT_DIR/${filename_test_branch}_${bench}.txt" | tee "$OUTPUT_DIR/${bench}_diff.txt"
+	done
+else
+	echo "Usage:"
+	echo "  $0 --single                   # run benchmarks on current branch"
+	echo "  $0 --compare branchA branchB  # compare benchmarks between two branches"
+	exit 1
+fi

scripts/rbac-authz/gen_input.go

@@ -0,0 +1,100 @@
+// This program generates an input.json file containing action, object, and subject fields
+// to be used as input for `opa eval`, e.g.:
+// > opa eval --format=pretty "data.authz.allow" -d policy.rego -i input.json
+// This helps verify that the policy returns the expected authorization decision.
+package main
+
+import (
+	"encoding/json"
+	"log"
+	"os"
+
+	"github.com/google/uuid"
+	"golang.org/x/xerrors"
+
+	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/rbac/policy"
+)
+
+type SubjectJSON struct {
+	ID     string      `json:"id"`
+	Roles  []rbac.Role `json:"roles"`
+	Groups []string    `json:"groups"`
+	Scope  rbac.Scope  `json:"scope"`
+}
+type OutputData struct {
+	Action  policy.Action `json:"action"`
+	Object  rbac.Object   `json:"object"`
+	Subject *SubjectJSON  `json:"subject"`
+}
+
+func newSubjectJSON(s rbac.Subject) (*SubjectJSON, error) {
+	roles, err := s.Roles.Expand()
+	if err != nil {
+		return nil, xerrors.Errorf("failed to expand subject roles: %w", err)
+	}
+	scopes, err := s.Scope.Expand()
+	if err != nil {
+		return nil, xerrors.Errorf("failed to expand subject scopes: %w", err)
+	}
+	return &SubjectJSON{
+		ID:     s.ID,
+		Roles:  roles,
+		Groups: s.Groups,
+		Scope:  scopes,
+	}, nil
+}
+
+// TODO: Support optional CLI flags to customize the input:
+// --action=[one of the supported actions]
+// --subject=[one of the built-in roles]
+// --object=[one of the supported resources]
+func main() {
+	// Template Admin user
+	subject := rbac.Subject{
+		FriendlyName: "Test Name",
+		Email:        "test@coder.com",
+		Type:         "user",
+		ID:           uuid.New().String(),
+		Roles: rbac.RoleIdentifiers{
+			rbac.RoleTemplateAdmin(),
+		},
+		Scope: rbac.ScopeAll,
+	}
+
+	subjectJSON, err := newSubjectJSON(subject)
+	if err != nil {
+		log.Fatalf("Failed to convert to subject to JSON: %v", err)
+	}
+
+	// Delete action
+	action := policy.ActionDelete
+
+	// Prebuilt Workspace object
+	object := rbac.Object{
+		ID:    uuid.New().String(),
+		Owner: "c42fdf75-3097-471c-8c33-fb52454d81c0",
+		OrgID: "663f8241-23e0-41c4-a621-cec3a347318e",
+		Type:  "prebuilt_workspace",
+	}
+
+	// Output file path
+	outputPath := "input.json"
+
+	output := OutputData{
+		Action:  action,
+		Object:  object,
+		Subject: subjectJSON,
+	}
+
+	outputBytes, err := json.MarshalIndent(output, "", "  ")
+	if err != nil {
+		log.Fatalf("Failed to marshal output to json: %v", err)
+	}
+
+	if err := os.WriteFile(outputPath, outputBytes, 0o600); err != nil {
+		log.Fatalf("Failed to generate input file: %v", err)
+	}
+
+	log.Println("Input JSON written to", outputPath)
+}