workspace dormancy

coder · Emyrk · May 15, 2024 · May 9, 2024 · May 9, 2024 · May 9, 2024
commit 582fdcd64a774e303d65d9b47d18e460b360f94b
diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go
@@ -82,6 +82,21 @@ func (a ActionDefinition) Requires() string {
 	return strings.Join(fields, ",")
 }
 
+var workspaceActions = map[Action]ActionDefinition{
+	ActionCreate: actDef(fieldOwner|fieldOrg, "create a new workspace"),
+	ActionRead:   actDef(fieldOwner|fieldOrg|fieldACL, "read workspace data to view on the UI"),
+	// TODO: Make updates more granular
+	ActionUpdate: actDef(fieldOwner|fieldOrg|fieldACL, "edit workspace settings (scheduling, permissions, parameters)"),
+	ActionDelete: actDef(fieldOwner|fieldOrg|fieldACL, "delete workspace"),
+
+	// Workspace provisioning
+	ActionWorkspaceBuild: actDef(fieldOwner|fieldOrg|fieldACL, "allows starting, stopping, and updating a workspace"),
+
+	// Running a workspace
+	ActionSSH:                actDef(fieldOwner|fieldOrg|fieldACL, "ssh into a given workspace"),
+	ActionApplicationConnect: actDef(fieldOwner|fieldOrg|fieldACL, "connect to workspace apps via browser"),
+}
+
 // RBACPermissions is indexed by the type
 var RBACPermissions = map[string]PermissionDefinition{
 	// Wildcard is every object, and the action "*" provides all actions.
@@ -104,23 +119,11 @@ var RBACPermissions = map[string]PermissionDefinition{
 		},
 	},
 	"workspace": {
-		Actions: map[Action]ActionDefinition{
-			ActionCreate: actDef(fieldOwner|fieldOrg, "create a new workspace"),
-			ActionRead:   actDef(fieldOwner|fieldOrg|fieldACL, "read workspace data to view on the UI"),
-			// TODO: Make updates more granular
-			ActionUpdate: actDef(fieldOwner|fieldOrg|fieldACL, "edit workspace settings (scheduling, permissions, parameters)"),
-			ActionDelete: actDef(fieldOwner|fieldOrg|fieldACL, "delete workspace"),
-
-			// Workspace provisioning
-			ActionWorkspaceBuild: actDef(fieldOwner|fieldOrg|fieldACL, "allows starting, stopping, and updating a workspace"),
-
-			// Running a workspace
-			ActionSSH:                actDef(fieldOwner|fieldOrg|fieldACL, "ssh into a given workspace"),
-			ActionApplicationConnect: actDef(fieldOwner|fieldOrg|fieldACL, "connect to workspace apps via browser"),
-		},
+		Actions: workspaceActions,
 	},
+	// Dormant workspaces have the same perms as workspaces.
 	"workspace_dormant": {
-		Actions: map[Action]ActionDefinition{},
+		Actions: workspaceActions,
 	},
 	"workspace_proxy": {
 		Actions: map[Action]ActionDefinition{
@@ -194,8 +197,9 @@ var RBACPermissions = map[string]PermissionDefinition{
 	"organization": {
 		Actions: map[Action]ActionDefinition{
 			ActionCreate: actDef(0, "create an organization"),
-			ActionRead:   actDef(0, "read organizations"),
-			ActionDelete: actDef(0, "delete a organization"),
+			ActionRead:   actDef(fieldOrg, "read organizations"),
+			ActionUpdate: actDef(fieldOrg, "update an organization"),
+			ActionDelete: actDef(fieldOrg, "delete an organization"),
 		},
 	},
 	"organization_member": {

diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go
@@ -145,7 +145,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 			allPermsExcept(ResourceWorkspaceDormant, ResourceWorkspace),
 			// This adds back in the Workspace permissions.
 			Permissions(map[string][]policy.Action{
-				ResourceWorkspace.Type: ownerWorkspaceActions,
+				ResourceWorkspace.Type:        ownerWorkspaceActions,
+				ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
 			})...),
 		Org:  map[string][]Permission{},
 		User: []Permission{},
@@ -165,6 +166,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 		Org: map[string][]Permission{},
 		User: append(allPermsExcept(ResourceWorkspaceDormant, ResourceUser, ResourceOrganizationMember),
 			Permissions(map[string][]policy.Action{
+				// Reduced permission set on dormant workspaces. No build, ssh, or exec
+				ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
+
 				// Users cannot do create/update/delete on themselves, but they
 				// can read their own details.
 				ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
@@ -268,7 +272,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 				Org: map[string][]Permission{
 					// Org admins should not have workspace exec perms.
 					organizationID: append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant), Permissions(map[string][]policy.Action{
-						ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH),
+						ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
+						ResourceWorkspace.Type:        slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH),
 					})...),
 				},
 				User: []Permission{},

diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go
@@ -265,7 +265,7 @@ func TestRolePermissions(t *testing.T) {
 		},
 		{
 			Name:     "APIKey",
-			Actions:  []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
+			Actions:  []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete},
 			Resource: rbac.ResourceApiKey.WithID(apiKeyID).WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
 				true:  {owner, orgMemberMe, memberMe},
@@ -332,7 +332,16 @@ func TestRolePermissions(t *testing.T) {
 		},
 		{
 			Name:     "WorkspaceDormant",
-			Actions:  rbac.AllActions(),
+			Actions:  crud,
+			Resource: rbac.ResourceWorkspaceDormant.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID),
+			AuthorizeMap: map[bool][]authSubject{
+				true:  {orgMemberMe, orgAdmin, owner},
+				false: {userAdmin, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin},
+			},
+		},
+		{
+			Name:     "WorkspaceDormantUse",
+			Actions:  []policy.Action{policy.ActionWorkspaceBuild, policy.ActionApplicationConnect, policy.ActionSSH},
 			Resource: rbac.ResourceWorkspaceDormant.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID),
 			AuthorizeMap: map[bool][]authSubject{
 				true:  {},
@@ -478,7 +487,7 @@ func TestRolePermissions(t *testing.T) {
 		},
 		{
 			Name:     "Oauth2Token",
-			Actions:  crud,
+			Actions:  []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete},
 			Resource: rbac.ResourceOauth2AppCodeToken,
 			AuthorizeMap: map[bool][]authSubject{
 				true:  {owner},
@@ -514,6 +523,7 @@ func TestRolePermissions(t *testing.T) {
 		}
 	}
 
+	passed := true
 	for _, c := range testCases {
 		c := c
 		// nolint:tparallel -- These share the same remainingPermissions map
@@ -524,6 +534,13 @@ func TestRolePermissions(t *testing.T) {
 			}
 
 			for _, action := range c.Actions {
+				err := c.Resource.ValidAction(action)
+				ok := assert.NoError(t, err, "%q is not a valid action for type %q", action, c.Resource.Type)
+				if !ok {
+					passed = passed && assert.NoError(t, err, "%q is not a valid action for type %q", action, c.Resource.Type)
+					continue
+				}
+
 				for result, subjs := range c.AuthorizeMap {
 					for _, subj := range subjs {
 						delete(remainingSubjs, subj.Name)
@@ -538,9 +555,9 @@ func TestRolePermissions(t *testing.T) {
 						delete(remainingPermissions[c.Resource.Type], action)
 						err := auth.Authorize(context.Background(), actor, action, c.Resource)
 						if result {
-							assert.NoError(t, err, fmt.Sprintf("Should pass: %s", msg))
+							passed = passed && assert.NoError(t, err, fmt.Sprintf("Should pass: %s", msg))
 						} else {
-							assert.ErrorContains(t, err, "forbidden", fmt.Sprintf("Should fail: %s", msg))
+							passed = passed && assert.ErrorContains(t, err, "forbidden", fmt.Sprintf("Should fail: %s", msg))
 						}
 					}
 				}
@@ -549,13 +566,16 @@ func TestRolePermissions(t *testing.T) {
 		})
 	}
 
-	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)
-			}
-		})
+	// 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)
+				}
+			})
+		}
 	}
 }