Skip to content

Commit

Permalink
fix: RBAC should default deny missing variables. (#5105)
Browse files Browse the repository at this point in the history
* fix: RBAC should default deny missing variables.

The default behavior was to use 'true' for missing variables. This
was an incorrect assumption. If the variable is missing, the new
default is to deny (fail secure).

* Assert 1 workspace is returned for the owners
  • Loading branch information
Emyrk committed Nov 16, 2022
1 parent 1fcc7ca commit 015a6f9
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
2 changes: 1 addition & 1 deletion coderd/rbac/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ func (t opInternalMember2) SQLString(cfg SQLConfig) string {
}

if sqlType == VarTypeSkip {
return "true"
return "false"
}
}

Expand Down
2 changes: 1 addition & 1 deletion coderd/rbac/query_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestCompileQuery(t *testing.T) {
`"*" in input.object.acl_group_list["4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75"]`,
))
require.NoError(t, err, "compile")
require.Equal(t, `true`,
require.Equal(t, `false`,
expression.SQLString(NoACLConfig()), "literal dereference")
})
}
Expand Down
12 changes: 9 additions & 3 deletions coderd/workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,20 @@ func TestAdminViewAllWorkspaces(t *testing.T) {

// This other user is not in the first user's org. Since other is an admin, they can
// still see the "first" user's workspace.
other := coderdtest.CreateAnotherUser(t, client, otherOrg.ID, rbac.RoleOwner())
otherWorkspaces, err := other.Workspaces(ctx, codersdk.WorkspaceFilter{})
otherOwner := coderdtest.CreateAnotherUser(t, client, otherOrg.ID, rbac.RoleOwner())
otherWorkspaces, err := otherOwner.Workspaces(ctx, codersdk.WorkspaceFilter{})
require.NoError(t, err, "(other) fetch workspaces")

firstWorkspaces, err := other.Workspaces(ctx, codersdk.WorkspaceFilter{})
firstWorkspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{})
require.NoError(t, err, "(first) fetch workspaces")

require.ElementsMatch(t, otherWorkspaces.Workspaces, firstWorkspaces.Workspaces)
require.Equal(t, len(firstWorkspaces.Workspaces), 1, "should be 1 workspace present")

memberView := coderdtest.CreateAnotherUser(t, client, otherOrg.ID)
memberViewWorkspaces, err := memberView.Workspaces(ctx, codersdk.WorkspaceFilter{})
require.NoError(t, err, "(member) fetch workspaces")
require.Equal(t, 0, len(memberViewWorkspaces.Workspaces), "member in other org should see 0 workspaces")
}

func TestPostWorkspacesByOrganization(t *testing.T) {
Expand Down

0 comments on commit 015a6f9

Please sign in to comment.