chore: remove rbac psuedo resources, add custom verbs by Emyrk · Pull Request #13276 · coder/coder

Emyrk · 2024-05-14T20:17:47Z

Supports #13226

What this does

Removes our pseudo rbac resources like WorkspaceApplicationConnect in favor of additional verbs like ssh. This is to make more intuitive permissions for building custom roles.

Prior to this change, trying to create a custom role creator would not be intuitive or dynamic.

Implementation

The source of truth is now policy.go .

To prevent even more line changes by drastically changing the golang code, the previous resource list is autogenerated from this source of truth:

coder/coderd/rbac/object_gen.go

Lines 218 to 220 in 2255162

    
           ResourceWorkspace = Object{ 
        
           	Type: "workspace", 
        
           }

The autogen is the easiest way to ensure consistency while still using the rbac.Object.

The codersdk authz constants are now also in sync via auto gen: https://github.com/coder/coder/blob/eeb6ac20988b77ae7bf577c79b304fa34a7848f5/codersdk/rbacresources_gen.go

Additional checks

The prior and current implementation do not force the caller to populate the object fields (owner, org, id, acls, etc) or use a correct action when calling the rego.

Now that we have a definitive policy location, we can add at least runtime checks for correctness. I have only added correctness checks for actions on unit tests.

coder/coderd/rbac/authz.go

Lines 243 to 247 in d60f65a

    
           func NewStrictCachingAuthorizer(registry prometheus.Registerer) Authorizer { 
        
           	auth := NewAuthorizer(registry) 
        
           	auth.strict = true 
        
           	return Cacher(auth) 
        
           }

This will prevent asserting actions that are not available for a given object.

Emyrk · 2024-05-14T20:18:02Z

chore: remove rbac psuedo resources, add custom verbs #13276 👈
chore: push rbac actions to policy package #13274
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Emyrk and the rest of your teammates on Graphite

Emyrk · 2024-05-14T22:17:34Z

+			ActionReadPersonal:   actDef(fieldOwner, "read personal user data like password"),
+			ActionUpdatePersonal: actDef(fieldOwner, "update personal data"),


This used to be ResourceUserData, but is now just additional verbs

Emyrk · 2024-05-15T00:36:03Z

+codersdk/rbacresources_gen.go: scripts/rbacgen/main.go coderd/rbac/object.go
+	go run scripts/rbacgen/main.go codersdk > codersdk/rbacresources_gen.go


Before this was manually kept in sync. Or so we told ourselves

Emyrk · 2024-05-15T00:38:11Z

-					rbac.ResourceFile.Type:           {policy.ActionRead},
-					rbac.ResourceSystem.Type:         {rbac.WildcardSymbol},
-					rbac.ResourceTemplate.Type:       {policy.ActionRead, policy.ActionUpdate},
-					rbac.ResourceUser.Type:           {policy.ActionRead},
-					rbac.ResourceWorkspace.Type:      {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
-					rbac.ResourceWorkspaceBuild.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
-					rbac.ResourceUserData.Type:       {policy.ActionRead, policy.ActionUpdate},
-					rbac.ResourceAPIKey.Type:         {rbac.WildcardSymbol},
+					rbac.ResourceFile.Type:     {policy.ActionRead},
+					rbac.ResourceSystem.Type:   {rbac.WildcardSymbol},
+					rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
+					// Unsure why provisionerd needs update and read personal
+					rbac.ResourceUser.Type:      {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
+					rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild},
+					rbac.ResourceApiKey.Type:    {rbac.WildcardSymbol},


ResourceWorkspaceBuild -> ResourceWorkspace: ActionWorkspaceBuild

(unsure why it needs this, keeping status quo)
ResourceUserData -> ResourceUser: ActionReadPersonal, ActionUpdatePersonal

Emyrk · 2024-05-15T00:38:40Z

-					rbac.ResourceSystem.Type:         {rbac.WildcardSymbol},
-					rbac.ResourceTemplate.Type:       {policy.ActionRead, policy.ActionUpdate},
-					rbac.ResourceWorkspace.Type:      {policy.ActionRead, policy.ActionUpdate},
-					rbac.ResourceWorkspaceBuild.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
-					rbac.ResourceUser.Type:           {policy.ActionRead},
+					rbac.ResourceSystem.Type:    {rbac.WildcardSymbol},
+					rbac.ResourceTemplate.Type:  {policy.ActionRead, policy.ActionUpdate},
+					rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceBuild},
+					rbac.ResourceUser.Type:      {policy.ActionRead},


ResourceWorkspaceBuild -> ResourceWorkspace: ActionWorkspaceBuild

Emyrk · 2024-05-15T00:39:14Z

-					rbac.ResourceWorkspaceBuild.Type:     {policy.ActionUpdate},
-					rbac.ResourceWorkspaceExecution.Type: {policy.ActionCreate},
+					rbac.ResourceUser.Type:               rbac.ResourceUser.AvailableActions(),
+					rbac.ResourceWorkspace.Type:          {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild, policy.ActionSSH},


ResourceWorkspaceExecution -> ResourceWorkspace: ActionSSH

Emyrk · 2024-05-15T00:44:10Z

 **Evaluation**

-opa eval --format=pretty 'false' -d policy.rego -i input.json
+opa eval --format=pretty "data.authz.allow" -d policy.rego -i input.json


This was a typo I found in the readme.

Emyrk · 2024-05-15T00:47:31Z

+// RBACPermissions is indexed by the type
+var RBACPermissions = map[string]PermissionDefinition{


The new source of truth for our policies.

Emyrk · 2024-05-15T00:51:07Z

+	// Only run these if the tests on top passed. Otherwise, the error output is too noisy.
+	if passed {
+		for rtype, v := range remainingPermissions {
+			// nolint:tparallel -- Making a subtest for easier diagnosing failures.
+			t.Run(fmt.Sprintf("%s-AllActions", rtype), func(t *testing.T) {
+				if len(v) > 0 {
+					assert.Equal(t, map[policy.Action]bool{}, v, "remaining permissions should be empty for type %q", rtype)
+				}
+			})
+		}
+	}


I added this to force us to update this test if resources or actions are added. This is sort of our source of truth for our "policy".

Would be really awesome to come up with a custom syntax for making this more compressed...

github-actions Bot assigned Emyrk May 14, 2024

Emyrk mentioned this pull request May 14, 2024

chore: push rbac actions to policy package #13274

Merged

Emyrk force-pushed the stevenmasley/rego_to_policy branch from 46dbb72 to b2f9524 Compare May 14, 2024 22:00

Emyrk force-pushed the stevenmasley/rego_upgrade branch 4 times, most recently from e8977b6 to 0089e1d Compare May 14, 2024 22:16

Emyrk commented May 14, 2024

View reviewed changes

Emyrk force-pushed the stevenmasley/rego_upgrade branch 2 times, most recently from 50aee62 to 103ed1f Compare May 14, 2024 22:34

Emyrk changed the title ~~chore: custom verbs for RBAC actions~~ chore: remove rbac psuedo resources, add custom verbs May 15, 2024

Emyrk force-pushed the stevenmasley/rego_upgrade branch 2 times, most recently from eeb6ac2 to 116a38e Compare May 15, 2024 00:12

Emyrk commented May 15, 2024

View reviewed changes

Emyrk force-pushed the stevenmasley/rego_upgrade branch 14 times, most recently from 574d565 to 6020cc2 Compare May 15, 2024 03:28

Emyrk marked this pull request as ready for review May 15, 2024 03:39

Base automatically changed from stevenmasley/rego_to_policy to main May 15, 2024 14:46

Emyrk added 24 commits May 15, 2024 14:47

wip

6e15df8

Fix readme

ca7e8a5

work on policy

4b69926

WIP: delete all old resources, generating them from a policy

a9b8984

Start the move to policy.Action

48f0593

update gen to be sorted

6964513

fix some compile issus

22861a1

begin some aciton work

1b7112c

more progrss

cd68fef

More dbauthz

1e1778d

roles are personal user data

c170068

remove user data object, and just use a data object

3cc6ae0

Update roles

c70c526

update roles_test to include all actions and resources

66107d3

workspace dormancy

582fdcd

remove rbac object fields from policy def

fc4af1e

add codersdk autogen

282bc93

fix ui enums

0fab76f

cleanup

ba4c327

cleanup

e4d59ae

make start/stop workspaces different actions

11bf1d5

make fmt

a5c2fd5

reset input.json

6be67fc

make gen

7262bd8

Emyrk force-pushed the stevenmasley/rego_upgrade branch from 6020cc2 to 7262bd8 Compare May 15, 2024 14:47

Emyrk mentioned this pull request May 15, 2024

Add ability to create custom roles for site + orgs #13226

Closed

7 tasks

Emyrk merged commit 1f5788f into main May 15, 2024

Emyrk deleted the stevenmasley/rego_upgrade branch May 15, 2024 16:09

github-actions Bot locked and limited conversation to collaborators May 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: remove rbac psuedo resources, add custom verbs#13276

chore: remove rbac psuedo resources, add custom verbs#13276
Emyrk merged 24 commits into
mainfrom
stevenmasley/rego_upgrade

Emyrk commented May 14, 2024 •

edited

Loading

Uh oh!

Emyrk commented May 14, 2024 •

edited

Loading

Uh oh!

Emyrk May 14, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Emyrk May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	func NewStrictCachingAuthorizer(registry prometheus.Registerer) Authorizer {
	auth := NewAuthorizer(registry)
	auth.strict = true
	return Cacher(auth)
	}

		ActionReadPersonal: actDef(fieldOwner, "read personal user data like password"),
		ActionUpdatePersonal: actDef(fieldOwner, "update personal data"),

		codersdk/rbacresources_gen.go: scripts/rbacgen/main.go coderd/rbac/object.go
		go run scripts/rbacgen/main.go codersdk > codersdk/rbacresources_gen.go

		// RBACPermissions is indexed by the type
		var RBACPermissions = map[string]PermissionDefinition{

Conversation

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

Uh oh!

What this does

Implementation

Additional checks

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Emyrk commented May 14, 2024 •

edited

Loading

Emyrk commented May 14, 2024 •

edited

Loading