From 212ebce004fc781fa75c47213e3fcf85395eefde Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 10:09:10 -0600 Subject: [PATCH 01/16] feat: Implement allow_list for scopes for resource specific permissions Feature that adds an allow_list for scopes to specify particular resources. This enables workspace agent tokens to use the same RBAC system as users. --- coderd/rbac/README.md | 15 ++++++++++ coderd/rbac/authz.go | 4 +-- coderd/rbac/authz_internal_test.go | 20 ++++++------- coderd/rbac/input.json | 2 ++ coderd/rbac/object.go | 17 +++++++++++ coderd/rbac/partial.go | 1 + coderd/rbac/policy.rego | 20 +++++++++++++ coderd/rbac/scopes.go | 47 +++++++++++++++++++----------- 8 files changed, 97 insertions(+), 29 deletions(-) diff --git a/coderd/rbac/README.md b/coderd/rbac/README.md index 66a8dcb863293..5ec7e81fd7501 100644 --- a/coderd/rbac/README.md +++ b/coderd/rbac/README.md @@ -48,6 +48,7 @@ This can be represented by the following truth table, where Y represents _positi - `level` is either `site`, `org`, or `user`. - `object` is any valid resource type. - `id` is any valid UUID v4. + - `id` is included in the permission syntax, however only scopes may use `id` to specify a specific object. - `action` is `create`, `read`, `modify`, or `delete`. ## Example Permissions @@ -72,6 +73,20 @@ Y indicates that the role provides positive permissions, N indicates the role pr | | \_ | \_ | N | N | | unauthenticated | \_ | \_ | \_ | N | +## Scopes + +Scopes can restrict a given set of permissions. The format of a scope matches a role with the addition of a list of resource ids. For a authorization call to be successful, the subject's roles and the subject's scopes must both allow the action. This means the resulting permissions is the intersection of the subject's roles and the subject's scopes. + +An example to give a readonly token is to grant a readonly scope across all resources `+site.*.*.read`. The intersection with the user's permissions will be the readonly set of their permissions. + + +### Resource IDs + +There exists use cases that require specifying a specific resource. If resource IDs are allowed in the roles, then there is +an unbounded set of resource IDs that be added to an "allow_list", as the number of roles a user can have is unbounded. This also adds a level of complexity to the role evaluation logic that has large costs at scale. + +The use case for specifying this type of permission in a role is limited, and does not justify the extra cost. To solve this for the remaining cases (eg. workspace agent tokens), we can apply an `allow_list` on a scope. For most cases, the `allow_list` will just be `["*"]` which means the scope is allowed to be applied to any resource. This adds negligible cost to the role evaluation logic and 0 cost to partial evaluations. + # Testing You can test outside of golang by using the `opa` cli. diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 4d43089fb72ea..42a22688f8350 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -197,7 +197,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa return err } - scopeRole, err := ScopeRole(scope) + scopeRole, err := ExpandScope(scope) if err != nil { return err } @@ -252,7 +252,7 @@ func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, return nil, err } - scopeRole, err := ScopeRole(scope) + scopeRole, err := ExpandScope(scope) if err != nil { return nil, err } diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 5aaa71eca7ea6..245191b69f3ea 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -200,7 +200,7 @@ func TestAuthorizeDomain(t *testing.T) { user := subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Groups: []string{allUsersGroup}, Roles: []Role{ must(RoleByName(RoleMember())), @@ -299,7 +299,7 @@ func TestAuthorizeDomain(t *testing.T) { user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{{ Name: "deny-all", // List out deny permissions explicitly @@ -340,7 +340,7 @@ func TestAuthorizeDomain(t *testing.T) { user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{ must(RoleByName(RoleOrgAdmin(defOrg))), must(RoleByName(RoleMember())), @@ -374,7 +374,7 @@ func TestAuthorizeDomain(t *testing.T) { user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{ must(RoleByName(RoleOwner())), must(RoleByName(RoleMember())), @@ -408,7 +408,7 @@ func TestAuthorizeDomain(t *testing.T) { user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeApplicationConnect)), + Scope: must(ExpandScope(ScopeApplicationConnect)), Roles: []Role{ must(RoleByName(RoleOrgMember(defOrg))), must(RoleByName(RoleMember())), @@ -507,7 +507,7 @@ func TestAuthorizeDomain(t *testing.T) { // In practice this is a token scope on a regular subject user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{ { Name: "ReadOnlyOrgAndUser", @@ -600,7 +600,7 @@ func TestAuthorizeLevels(t *testing.T) { user := subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{ must(RoleByName(RoleOwner())), { @@ -661,7 +661,7 @@ func TestAuthorizeLevels(t *testing.T) { user = subject{ UserID: "me", - Scope: must(ScopeRole(ScopeAll)), + Scope: must(ExpandScope(ScopeAll)), Roles: []Role{ { Name: "site-noise", @@ -726,7 +726,7 @@ func TestAuthorizeScope(t *testing.T) { user := subject{ UserID: "me", Roles: []Role{must(RoleByName(RoleOwner()))}, - Scope: must(ScopeRole(ScopeApplicationConnect)), + Scope: must(ExpandScope(ScopeApplicationConnect)), } testAuthorize(t, "Admin_ScopeApplicationConnect", user, @@ -760,7 +760,7 @@ func TestAuthorizeScope(t *testing.T) { must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, - Scope: must(ScopeRole(ScopeApplicationConnect)), + Scope: must(ExpandScope(ScopeApplicationConnect)), } testAuthorize(t, "User_ScopeApplicationConnect", user, diff --git a/coderd/rbac/input.json b/coderd/rbac/input.json index 6797122a42a55..062733c6d0304 100644 --- a/coderd/rbac/input.json +++ b/coderd/rbac/input.json @@ -1,6 +1,7 @@ { "action": "never-match-action", "object": { + "id": "00000000-0000-0000-0000-000000000000", "owner": "00000000-0000-0000-0000-000000000000", "org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6", "type": "workspace", @@ -16,6 +17,7 @@ "scope": { "name": "Scope_all", "display_name": "All operations", + "allow_list": ["123", "*"], "site": [ { "negate": false, diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 4852a4c269638..9061be196b8ad 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -158,6 +158,8 @@ var ( // that represents the set of workspaces you are trying to get access too. // Do not export this type, as it can be created from a resource type constant. type Object struct { + // ID is the resource's uuid + ID string `json:"id"` Owner string `json:"owner"` // OrgID specifies which org the object is a part of. OrgID string `json:"org_owner"` @@ -184,9 +186,21 @@ func (z Object) All() Object { } } +func (z Object) WithID(id uuid.UUID) Object { + return Object{ + ID: id.String(), + Owner: z.Owner, + OrgID: z.OrgID, + Type: z.Type, + ACLUserList: z.ACLUserList, + ACLGroupList: z.ACLGroupList, + } +} + // InOrg adds an org OwnerID to the resource func (z Object) InOrg(orgID uuid.UUID) Object { return Object{ + ID: z.ID, Owner: z.Owner, OrgID: orgID.String(), Type: z.Type, @@ -198,6 +212,7 @@ func (z Object) InOrg(orgID uuid.UUID) Object { // WithOwner adds an OwnerID to the resource func (z Object) WithOwner(ownerID string) Object { return Object{ + ID: z.ID, Owner: ownerID, OrgID: z.OrgID, Type: z.Type, @@ -209,6 +224,7 @@ func (z Object) WithOwner(ownerID string) Object { // WithACLUserList adds an ACL list to a given object func (z Object) WithACLUserList(acl map[string][]Action) Object { return Object{ + ID: z.ID, Owner: z.Owner, OrgID: z.OrgID, Type: z.Type, @@ -219,6 +235,7 @@ func (z Object) WithACLUserList(acl map[string][]Action) Object { func (z Object) WithGroupACL(groups map[string][]Action) Object { return Object{ + ID: z.ID, Owner: z.Owner, OrgID: z.OrgID, Type: z.Type, diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index b9673c809a84e..e918537069795 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -141,6 +141,7 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, s rego.Query("data.authz.allow = true"), rego.Module("policy.rego", policy), rego.Unknowns([]string{ + "input.object.id", "input.object.owner", "input.object.org_owner", "input.object.acl_user_list", diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index 095f1844bd78d..ca7c1113d59e2 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -148,6 +148,23 @@ user_allow(roles) := num { num := number(allow) } +# Scope allow_list is a list of resource IDs explicitly allowed by the scope. +# If the list is '*', then all resources are allowed. +scope_allow_list { + "*" in input.subject.scope.allow_list +} + +scope_allow_list { + # If the wildcard is listed in the allow_list, we do not care about the + # object.id. This line is included to prevent partial compliations from + # ever needing to include the object.id. + not "*" in input.subject.scope.allow_list + input.object.id != "" + 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! # Note that the absence of `allow` means "unauthorized". @@ -179,15 +196,18 @@ role_allow { } scope_allow { + scope_allow_list scope_site = 1 } scope_allow { + scope_allow_list not scope_site = -1 scope_org = 1 } scope_allow { + scope_allow_list not scope_site = -1 not scope_org = -1 # If we are not a member of an org, and the object has an org, then we are diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 57ed21bb644b6..e1d54a5f98a1e 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -8,39 +8,52 @@ import ( type Scope string +// TODO: @emyrk rename this struct +type ScopeRole struct { + Role + AllowIDList []string `json:"allow_list"` +} + const ( ScopeAll Scope = "all" ScopeApplicationConnect Scope = "application_connect" ) -var builtinScopes map[Scope]Role = map[Scope]Role{ +// TODO: Support passing in scopeID list for whitelisting allowed resources. +var builtinScopes map[Scope]ScopeRole = map[Scope]ScopeRole{ // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. ScopeAll: { - Name: fmt.Sprintf("Scope_%s", ScopeAll), - DisplayName: "All operations", - Site: permissions(map[string][]Action{ - ResourceWildcard.Type: {WildcardSymbol}, - }), - Org: map[string][]Permission{}, - User: []Permission{}, + Role: Role{ + Name: fmt.Sprintf("Scope_%s", ScopeAll), + DisplayName: "All operations", + Site: permissions(map[string][]Action{ + ResourceWildcard.Type: {WildcardSymbol}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, + }, + AllowIDList: []string{WildcardSymbol}, }, ScopeApplicationConnect: { - Name: fmt.Sprintf("Scope_%s", ScopeApplicationConnect), - DisplayName: "Ability to connect to applications", - Site: permissions(map[string][]Action{ - ResourceWorkspaceApplicationConnect.Type: {ActionCreate}, - }), - Org: map[string][]Permission{}, - User: []Permission{}, + Role: Role{ + Name: fmt.Sprintf("Scope_%s", ScopeApplicationConnect), + DisplayName: "Ability to connect to applications", + Site: permissions(map[string][]Action{ + ResourceWorkspaceApplicationConnect.Type: {ActionCreate}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, + }, + AllowIDList: []string{WildcardSymbol}, }, } -func ScopeRole(scope Scope) (Role, error) { +func ExpandScope(scope Scope) (ScopeRole, error) { role, ok := builtinScopes[scope] if !ok { - return Role{}, xerrors.Errorf("no scope named %q", scope) + return ScopeRole{}, xerrors.Errorf("no scope named %q", scope) } return role, nil } From 655e8dbf159444efededcb26a892a90a435f93ca Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 10:34:05 -0600 Subject: [PATCH 02/16] Fix rbac benchmark --- coderd/rbac/authz.go | 12 ++++++------ coderd/rbac/authz_internal_test.go | 6 +++--- coderd/rbac/partial.go | 2 +- coderd/rbac/policy.rego | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 42a22688f8350..d5e6759086e83 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -170,10 +170,10 @@ func NewAuthorizer(registry prometheus.Registerer) *RegoAuthorizer { } type authSubject struct { - ID string `json:"id"` - Roles []Role `json:"roles"` - Groups []string `json:"groups"` - Scope Role `json:"scope"` + ID string `json:"id"` + Roles []Role `json:"roles"` + Groups []string `json:"groups"` + Scope ScopeRole `json:"scope"` } // ByRoleName will expand all roleNames into roles before calling Authorize(). @@ -216,7 +216,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa // Authorize allows passing in custom Roles. // This is really helpful for unit testing, as we can create custom roles to exercise edge cases. -func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, scope Role, groups []string, action Action, object Object) error { +func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, object Object) error { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, @@ -275,7 +275,7 @@ func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, // Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). // This will vastly speed up performance if batch authorization on the same type of objects is needed. -func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope Role, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { +func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { auth, err := newPartialAuthorizer(ctx, subjectID, roles, scope, groups, action, objectType) if err != nil { return nil, xerrors.Errorf("new partial authorizer: %w", err) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 245191b69f3ea..3417e412185d8 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -20,9 +20,9 @@ type subject struct { // For the unit test we want to pass in the roles directly, instead of just // by name. This allows us to test custom roles that do not exist in the product, // but test edge cases of the implementation. - Roles []Role `json:"roles"` - Groups []string `json:"groups"` - Scope Role `json:"scope"` + Roles []Role `json:"roles"` + Groups []string `json:"groups"` + Scope ScopeRole `json:"scope"` } type fakeObject struct { diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index e918537069795..bacd2db0c0ba7 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -121,7 +121,7 @@ EachQueryLoop: return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), pa.input, nil) } -func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope Role, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { +func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index ca7c1113d59e2..c133935fbe575 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -156,7 +156,7 @@ scope_allow_list { scope_allow_list { # If the wildcard is listed in the allow_list, we do not care about the - # object.id. This line is included to prevent partial compliations from + # object.id. This line is included to prevent partial compilations from # ever needing to include the object.id. not "*" in input.subject.scope.allow_list input.object.id != "" From 5f5653d4922277bee7c6383b831f0b364361a485 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 11:28:12 -0600 Subject: [PATCH 03/16] Update coderd/rbac/scopes.go Co-authored-by: Cian Johnston --- coderd/rbac/scopes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index e1d54a5f98a1e..deacd3c70cb72 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -20,7 +20,7 @@ const ( ) // TODO: Support passing in scopeID list for whitelisting allowed resources. -var builtinScopes map[Scope]ScopeRole = map[Scope]ScopeRole{ +var builtinScopes = map[Scope]ScopeRole{ // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. ScopeAll: { From 4d66a03b544638dcbf2d391752d739038fff5163 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 11:28:26 -0600 Subject: [PATCH 04/16] Update coderd/rbac/scopes.go Co-authored-by: Cian Johnston --- coderd/rbac/scopes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index deacd3c70cb72..7371509aae6c1 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -19,7 +19,7 @@ const ( ScopeApplicationConnect Scope = "application_connect" ) -// TODO: Support passing in scopeID list for whitelisting allowed resources. +// TODO: Support passing in scopeID list for allowlisting resources. var builtinScopes = map[Scope]ScopeRole{ // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. From f9245785f64bdf31a5c30c9805e3b6f74899bf4a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 11:28:43 -0600 Subject: [PATCH 05/16] Include workspace agent token example in rbac readme --- coderd/rbac/README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/coderd/rbac/README.md b/coderd/rbac/README.md index 5ec7e81fd7501..5f65d808541e9 100644 --- a/coderd/rbac/README.md +++ b/coderd/rbac/README.md @@ -87,6 +87,19 @@ an unbounded set of resource IDs that be added to an "allow_list", as the number The use case for specifying this type of permission in a role is limited, and does not justify the extra cost. To solve this for the remaining cases (eg. workspace agent tokens), we can apply an `allow_list` on a scope. For most cases, the `allow_list` will just be `["*"]` which means the scope is allowed to be applied to any resource. This adds negligible cost to the role evaluation logic and 0 cost to partial evaluations. +Example of a scope for a workspace agent token, using an `allow_list` containing a single resource id. +```javascript + "scope": { + "name": "workspace_agent", + "display_name": "Workspace_Agent", + // The ID of the given workspace the agent token corrolates to. + "allow_list": ["10d03e62-7703-4df5-a358-4f76577d4e2f"], + "site": [/* ... perms ... */], + "org": {/* ... perms ... */}, + "user": [/* ... perms ... */] + } +``` + # Testing You can test outside of golang by using the `opa` cli. From b9fed910857b6ffdc0b6ab5f17fa08f6db491794 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 11:57:08 -0600 Subject: [PATCH 06/16] WithID unit tests and model method support - Add ID to compileSQL matchers --- coderd/database/modelmethods.go | 40 ++++++++++++----- coderd/rbac/authz_internal_test.go | 70 ++++++++++++++++++++++++++++++ coderd/rbac/regosql/configs.go | 7 +++ 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 5257bec1c3a2f..30f41cc035d35 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -19,7 +19,8 @@ func (s APIKeyScope) ToRBAC() rbac.Scope { func (t Template) RBACObject() rbac.Object { obj := rbac.ResourceTemplate - return obj.InOrg(t.OrganizationID). + return obj.WithID(t.ID). + InOrg(t.OrganizationID). WithACLUserList(t.UserACL). WithGroupACL(t.GroupACL) } @@ -30,42 +31,57 @@ func (TemplateVersion) RBACObject(template Template) rbac.Object { } func (g Group) RBACObject() rbac.Object { - return rbac.ResourceGroup.InOrg(g.OrganizationID) + return rbac.ResourceGroup.WithID(g.ID). + InOrg(g.OrganizationID) } func (w Workspace) RBACObject() rbac.Object { - return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) + return rbac.ResourceWorkspace.WithID(w.ID). + InOrg(w.OrganizationID). + WithOwner(w.OwnerID.String()) } func (w Workspace) ExecutionRBAC() rbac.Object { - return rbac.ResourceWorkspaceExecution.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) + return rbac.ResourceWorkspaceExecution. + WithID(w.ID). + InOrg(w.OrganizationID). + WithOwner(w.OwnerID.String()) } func (w Workspace) ApplicationConnectRBAC() rbac.Object { - return rbac.ResourceWorkspaceApplicationConnect.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) + return rbac.ResourceWorkspaceApplicationConnect. + WithID(w.ID). + InOrg(w.OrganizationID). + WithOwner(w.OwnerID.String()) } func (m OrganizationMember) RBACObject() rbac.Object { - return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID) + return rbac.ResourceOrganizationMember. + WithID(m.UserID). + InOrg(m.OrganizationID) } func (o Organization) RBACObject() rbac.Object { - return rbac.ResourceOrganization.InOrg(o.ID) + return rbac.ResourceOrganization. + WithID(o.ID). + InOrg(o.ID) } -func (ProvisionerDaemon) RBACObject() rbac.Object { - return rbac.ResourceProvisionerDaemon +func (p ProvisionerDaemon) RBACObject() rbac.Object { + return rbac.ResourceProvisionerDaemon.WithID(p.ID) } func (f File) RBACObject() rbac.Object { - return rbac.ResourceFile.WithOwner(f.CreatedBy.String()) + return rbac.ResourceFile. + WithID(f.ID). + WithOwner(f.CreatedBy.String()) } // RBACObject returns the RBAC object for the site wide user resource. // If you are trying to get the RBAC object for the UserData, use // rbac.ResourceUserData -func (User) RBACObject() rbac.Object { - return rbac.ResourceUser +func (u User) RBACObject() rbac.Object { + return rbac.ResourceUser.WithID(u.ID) } func (License) RBACObject() rbac.Object { diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 3417e412185d8..2af549a160573 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -788,6 +788,76 @@ func TestAuthorizeScope(t *testing.T) { {resource: ResourceWorkspaceApplicationConnect.InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, }, ) + + workspaceID := uuid.New() + user = subject{ + UserID: "me", + Roles: []Role{ + must(RoleByName(RoleMember())), + must(RoleByName(RoleOrgMember(defOrg))), + }, + Scope: ScopeRole{ + Role: Role{ + Name: "workspace_agent", + DisplayName: "Workspace Agent", + Site: permissions(map[string][]Action{ + // Only read access for workspaces. + ResourceWorkspace.Type: {ActionRead}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, + }, + AllowIDList: []string{workspaceID.String()}, + }, + } + + testAuthorize(t, "User_WorkspaceAgent", user, + // Test all cases with the workspace id + cases(func(c authTestCase) authTestCase { + c.actions = []Action{ActionCreate, ActionUpdate, ActionDelete} + c.allow = false + c.resource.WithID(workspaceID) + return c + }, []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg)}, + {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.All()}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + }), + // Test cases with random ids. These should always fail from the scope. + cases(func(c authTestCase) authTestCase { + c.actions = []Action{ActionRead, ActionCreate, ActionUpdate, ActionDelete} + c.allow = false + c.resource.WithID(uuid.New()) + return c + }, []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg)}, + {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.All()}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + }), + // Allowed by scope: + []authTestCase{ + {resource: ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionRead}, allow: true}, + // The scope will return true, but the user perms return false for resources not owned by the user. + {resource: ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionRead}, allow: false}, + {resource: ResourceWorkspace.WithID(workspaceID).InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionRead}, allow: false}, + }, + ) } // cases applies a given function to all test cases. This makes generalities easier to create. diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go index 7064ceccb1d23..475d317cd53ab 100644 --- a/coderd/rbac/regosql/configs.go +++ b/coderd/rbac/regosql/configs.go @@ -2,6 +2,10 @@ package regosql import "github.com/coder/coder/coderd/rbac/regosql/sqltypes" +func resourceIDMatcher() sqltypes.VariableMatcher { + return sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "id"}) +} + func organizationOwnerMatcher() sqltypes.VariableMatcher { return sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"}) } @@ -20,6 +24,7 @@ func userACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher { func TemplateConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( + resourceIDMatcher(), organizationOwnerMatcher(), // Templates have no user owner, only owner by an organization. sqltypes.AlwaysFalse(userOwnerMatcher()), @@ -35,6 +40,7 @@ func TemplateConverter() *sqltypes.VariableConverter { // group or user ACL columns. func NoACLConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( + resourceIDMatcher(), organizationOwnerMatcher(), userOwnerMatcher(), ) @@ -48,6 +54,7 @@ func NoACLConverter() *sqltypes.VariableConverter { func DefaultVariableConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( + resourceIDMatcher(), organizationOwnerMatcher(), userOwnerMatcher(), ) From 504c54b6eaccfaee9792ad1ba3007c7510a860de Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 13:40:25 -0600 Subject: [PATCH 07/16] Fix typo and fmt of readme --- coderd/rbac/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/rbac/README.md b/coderd/rbac/README.md index 5f65d808541e9..2a73a59d7febc 100644 --- a/coderd/rbac/README.md +++ b/coderd/rbac/README.md @@ -48,7 +48,7 @@ This can be represented by the following truth table, where Y represents _positi - `level` is either `site`, `org`, or `user`. - `object` is any valid resource type. - `id` is any valid UUID v4. - - `id` is included in the permission syntax, however only scopes may use `id` to specify a specific object. +- `id` is included in the permission syntax, however only scopes may use `id` to specify a specific object. - `action` is `create`, `read`, `modify`, or `delete`. ## Example Permissions @@ -79,7 +79,6 @@ Scopes can restrict a given set of permissions. The format of a scope matches a An example to give a readonly token is to grant a readonly scope across all resources `+site.*.*.read`. The intersection with the user's permissions will be the readonly set of their permissions. - ### Resource IDs There exists use cases that require specifying a specific resource. If resource IDs are allowed in the roles, then there is @@ -88,11 +87,12 @@ an unbounded set of resource IDs that be added to an "allow_list", as the number The use case for specifying this type of permission in a role is limited, and does not justify the extra cost. To solve this for the remaining cases (eg. workspace agent tokens), we can apply an `allow_list` on a scope. For most cases, the `allow_list` will just be `["*"]` which means the scope is allowed to be applied to any resource. This adds negligible cost to the role evaluation logic and 0 cost to partial evaluations. Example of a scope for a workspace agent token, using an `allow_list` containing a single resource id. + ```javascript "scope": { "name": "workspace_agent", "display_name": "Workspace_Agent", - // The ID of the given workspace the agent token corrolates to. + // The ID of the given workspace the agent token correlates to. "allow_list": ["10d03e62-7703-4df5-a358-4f76577d4e2f"], "site": [/* ... perms ... */], "org": {/* ... perms ... */}, From 257701ff8863329befe4614fa40709a01f31493c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 14:41:18 -0600 Subject: [PATCH 08/16] Plumbing WithID for rbac objects (incomplete) --- coderd/apikey.go | 38 +++++----- coderd/coderdtest/authorize.go | 45 +++++------ coderd/database/modelmethods.go | 8 +- coderd/files.go | 3 +- coderd/organizations.go | 3 +- coderd/rbac/authz_test.go | 1 + coderd/rbac/builtin_test.go | 37 +++++---- coderd/rbac/input.json | 76 ++++++++++++++----- coderd/rbac/object.go | 11 +++ coderd/users.go | 4 +- coderd/workspaceapps.go | 2 +- .../coderdenttest/coderdenttest_test.go | 2 +- enterprise/coderd/provisionerdaemons.go | 5 -- 13 files changed, 143 insertions(+), 92 deletions(-) diff --git a/coderd/apikey.go b/coderd/apikey.go index dea3525965043..babde27084d2a 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -143,15 +143,9 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { // @Router /users/{user}/keys/{keyid} [get] func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - user = httpmw.UserParam(r) + ctx = r.Context() ) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAPIKey.WithOwner(user.ID.String())) { - httpapi.ResourceNotFound(rw) - return - } - keyID := chi.URLParam(r, "keyid") key, err := api.Database.GetAPIKeyByID(ctx, keyID) if errors.Is(err, sql.ErrNoRows) { @@ -166,6 +160,11 @@ func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) { return } + if !api.Authorize(r, rbac.ActionRead, key) { + httpapi.ResourceNotFound(rw) + return + } + httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(key)) } @@ -179,25 +178,28 @@ func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) { // @Router /users/{user}/keys/tokens [get] func (api *API) tokens(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - user = httpmw.UserParam(r) + ctx = r.Context() ) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAPIKey.WithOwner(user.ID.String())) { - httpapi.ResourceNotFound(rw) + keys, err := api.Database.GetAPIKeysByLoginType(ctx, database.LoginTypeToken) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching API keys.", + Detail: err.Error(), + }) return } - keys, err := api.Database.GetAPIKeysByLoginType(ctx, database.LoginTypeToken) + keys, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, keys) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching API keys.", + Message: "Internal error fetching keys.", Detail: err.Error(), }) return } - apiKeys := []codersdk.APIKey{} + var apiKeys []codersdk.APIKey for _, key := range keys { apiKeys = append(apiKeys, convertAPIKey(key)) } @@ -215,16 +217,16 @@ func (api *API) tokens(rw http.ResponseWriter, r *http.Request) { // @Router /users/{user}/keys/{keyid} [delete] func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - user = httpmw.UserParam(r) + ctx = r.Context() + user = httpmw.UserParam(r) + keyID = chi.URLParam(r, "keyid") ) - if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithOwner(user.ID.String())) { + if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithIDString(keyID).WithOwner(user.ID.String())) { httpapi.ResourceNotFound(rw) return } - keyID := chi.URLParam(r, "keyid") err := api.Database.DeleteAPIKeyByID(ctx, keyID) if errors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(rw) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index f03370376c7cb..b979f54605899 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -32,9 +32,10 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { _, isMemoryDB := a.api.Database.(databasefake.FakeDatabase) // Some quick reused objects - workspaceRBACObj := rbac.ResourceWorkspace.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) - workspaceExecObj := rbac.ResourceWorkspaceExecution.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) - applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + workspaceRBACObj := rbac.ResourceWorkspace.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + workspaceExecObj := rbac.ResourceWorkspaceExecution.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String()) + templateObj := rbac.ResourceTemplate.WithID(a.Template.ID).InOrg(a.Template.OrganizationID) // skipRoutes allows skipping routes from being checked. skipRoutes := map[string]string{ @@ -74,7 +75,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { "POST:/api/v2/workspaceagents/me/report-stats": {NoAuthorize: true}, // These endpoints have more assertions. This is good, add more endpoints to assert if you can! - "GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(a.Admin.OrganizationID)}, + "GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.WithID(a.Admin.OrganizationID).InOrg(a.Admin.OrganizationID)}, "GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization}, "GET:/api/v2/users/{user}/workspace/{workspacename}": { AssertObject: rbac.ResourceWorkspace, @@ -138,11 +139,11 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { }, "DELETE:/api/v2/templates/{template}": { AssertAction: rbac.ActionDelete, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templates/{template}": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile}, "GET:/api/v2/files/{fileID}": { @@ -151,64 +152,64 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { }, "GET:/api/v2/templates/{template}/versions": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "PATCH:/api/v2/templates/{template}/versions": { AssertAction: rbac.ActionUpdate, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templates/{template}/versions/{templateversionname}": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "PATCH:/api/v2/templateversions/{templateversion}/cancel": { AssertAction: rbac.ActionUpdate, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/logs": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/parameters": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/rich-parameters": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/resources": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/schema": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "POST:/api/v2/templateversions/{templateversion}/dry-run": { // The first check is to read the template AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/resources": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID), + AssertObject: templateObj, }, "GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/logs": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID), + AssertObject: templateObj, }, "PATCH:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/cancel": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID), + AssertObject: templateObj, }, "POST:/api/v2/parameters/{scope}/{id}": { AssertAction: rbac.ActionUpdate, @@ -224,7 +225,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { }, "GET:/api/v2/organizations/{organization}/templates/{templatename}": { AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), + AssertObject: templateObj, }, "POST:/api/v2/organizations/{organization}/members/{user}/workspaces": { AssertAction: rbac.ActionCreate, diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 30f41cc035d35..d6a50f5fadd96 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -17,9 +17,13 @@ func (s APIKeyScope) ToRBAC() rbac.Scope { } } +func (k APIKey) RBACObject() rbac.Object { + return rbac.ResourceAPIKey.WithIDString(k.ID). + WithOwner(k.UserID.String()) +} + func (t Template) RBACObject() rbac.Object { - obj := rbac.ResourceTemplate - return obj.WithID(t.ID). + return rbac.ResourceTemplate.WithID(t.ID). InOrg(t.OrganizationID). WithACLUserList(t.UserACL). WithGroupACL(t.GroupACL) diff --git a/coderd/files.go b/coderd/files.go index bf8a98f6054f3..57e919d7dab3d 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -138,8 +138,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { return } - if !api.Authorize(r, rbac.ActionRead, - rbac.ResourceFile.WithOwner(file.CreatedBy.String())) { + if !api.Authorize(r, rbac.ActionRead, file) { // Return 404 to not leak the file exists httpapi.ResourceNotFound(rw) return diff --git a/coderd/organizations.go b/coderd/organizations.go index 7df9da184e351..19b57a8958e9e 100644 --- a/coderd/organizations.go +++ b/coderd/organizations.go @@ -28,8 +28,7 @@ func (api *API) organization(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() organization := httpmw.OrganizationParam(r) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrganization. - InOrg(organization.ID)) { + if !api.Authorize(r, rbac.ActionRead, organization) { httpapi.ResourceNotFound(rw) return } diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go index b010674387c3d..5a5774af49848 100644 --- a/coderd/rbac/authz_test.go +++ b/coderd/rbac/authz_test.go @@ -200,6 +200,7 @@ func benchmarkSetup(orgs []uuid.UUID, users []uuid.UUID, size int, opts ...func( objectList := make([]rbac.Object, size) for i := range objectList { objectList[i] = rbac.ResourceWorkspace. + WithID(uuid.New()). InOrg(orgs[i%len(orgs)]). WithOwner(users[i%len(users)].String()). WithACLUserList(aclList). diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index 85ef6f6507578..6a15e77f5415d 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -32,6 +32,11 @@ func TestRolePermissions(t *testing.T) { templateAdminID := uuid.New() orgID := uuid.New() otherOrg := uuid.New() + workspaceID := uuid.New() + templateID := uuid.New() + fileID := uuid.New() + groupID := uuid.New() + apiKeyID := uuid.New() // Subjects to user memberMe := authSubject{Name: "member_me", UserID: currentUser.String(), Roles: []string{rbac.RoleMember()}} @@ -66,7 +71,7 @@ func TestRolePermissions(t *testing.T) { { Name: "MyUser", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceUser, + Resource: rbac.ResourceUser.WithID(currentUser), AuthorizeMap: map[bool][]authSubject{ true: {owner, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin}, false: {}, @@ -85,7 +90,7 @@ func TestRolePermissions(t *testing.T) { Name: "ReadMyWorkspaceInOrg", // When creating the WithID won't be set, but it does not change the result. Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceWorkspace.InOrg(orgID).WithOwner(currentUser.String()), + Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgMemberMe, orgAdmin, templateAdmin}, false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin}, @@ -95,7 +100,7 @@ func TestRolePermissions(t *testing.T) { Name: "C_RDMyWorkspaceInOrg", // When creating the WithID won't be set, but it does not change the result. Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceWorkspace.InOrg(orgID).WithOwner(currentUser.String()), + Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgMemberMe, orgAdmin}, false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin, templateAdmin}, @@ -105,7 +110,7 @@ func TestRolePermissions(t *testing.T) { Name: "MyWorkspaceInOrgExecution", // When creating the WithID won't be set, but it does not change the result. Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceWorkspaceExecution.InOrg(orgID).WithOwner(currentUser.String()), + Resource: rbac.ResourceWorkspaceExecution.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, orgMemberMe}, false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, @@ -115,7 +120,7 @@ func TestRolePermissions(t *testing.T) { Name: "MyWorkspaceInOrgAppConnect", // When creating the WithID won't be set, but it does not change the result. Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceWorkspaceApplicationConnect.InOrg(orgID).WithOwner(currentUser.String()), + Resource: rbac.ResourceWorkspaceApplicationConnect.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, orgMemberMe}, false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, @@ -124,7 +129,7 @@ func TestRolePermissions(t *testing.T) { { Name: "Templates", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceTemplate.InOrg(orgID), + Resource: rbac.ResourceTemplate.WithID(templateID).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, templateAdmin}, false: {memberMe, orgMemberMe, otherOrgAdmin, otherOrgMember, userAdmin}, @@ -142,7 +147,7 @@ func TestRolePermissions(t *testing.T) { { Name: "Files", Actions: []rbac.Action{rbac.ActionCreate}, - Resource: rbac.ResourceFile, + Resource: rbac.ResourceFile.WithID(fileID), AuthorizeMap: map[bool][]authSubject{ true: {owner, templateAdmin}, false: {orgMemberMe, orgAdmin, memberMe, otherOrgAdmin, otherOrgMember, userAdmin}, @@ -151,7 +156,7 @@ func TestRolePermissions(t *testing.T) { { Name: "MyFile", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceFile.WithOwner(currentUser.String()), + Resource: rbac.ResourceFile.WithID(fileID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, memberMe, orgMemberMe, templateAdmin}, false: {orgAdmin, otherOrgAdmin, otherOrgMember, userAdmin}, @@ -169,7 +174,7 @@ func TestRolePermissions(t *testing.T) { { Name: "Organizations", Actions: []rbac.Action{rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceOrganization.InOrg(orgID), + Resource: rbac.ResourceOrganization.WithID(orgID).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin}, false: {otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe, templateAdmin, userAdmin}, @@ -178,7 +183,7 @@ func TestRolePermissions(t *testing.T) { { Name: "ReadOrganizations", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceOrganization.InOrg(orgID), + Resource: rbac.ResourceOrganization.WithID(orgID).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, orgMemberMe}, false: {otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, @@ -223,7 +228,7 @@ func TestRolePermissions(t *testing.T) { { Name: "APIKey", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceAPIKey.WithOwner(currentUser.String()), + Resource: rbac.ResourceAPIKey.WithID(apiKeyID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgMemberMe, memberMe}, false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, @@ -232,7 +237,7 @@ func TestRolePermissions(t *testing.T) { { Name: "UserData", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceUserData.WithOwner(currentUser.String()), + Resource: rbac.ResourceUserData.WithID(currentUser).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgMemberMe, memberMe}, false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, @@ -241,7 +246,7 @@ func TestRolePermissions(t *testing.T) { { Name: "ManageOrgMember", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceOrganizationMember.InOrg(orgID), + Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, userAdmin}, false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, @@ -250,7 +255,7 @@ func TestRolePermissions(t *testing.T) { { Name: "ReadOrgMember", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceOrganizationMember.InOrg(orgID), + Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, orgMemberMe, userAdmin}, false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, @@ -259,7 +264,7 @@ func TestRolePermissions(t *testing.T) { { Name: "AllUsersGroupACL", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceTemplate.InOrg(orgID).WithGroupACL( + Resource: rbac.ResourceTemplate.WithID(templateID).InOrg(orgID).WithGroupACL( map[string][]rbac.Action{ orgID.String(): {rbac.ActionRead}, }), @@ -272,7 +277,7 @@ func TestRolePermissions(t *testing.T) { { Name: "Groups", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceGroup.InOrg(orgID), + Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, userAdmin, orgMemberMe}, false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, diff --git a/coderd/rbac/input.json b/coderd/rbac/input.json index 062733c6d0304..1465c198b427a 100644 --- a/coderd/rbac/input.json +++ b/coderd/rbac/input.json @@ -1,32 +1,68 @@ { - "action": "never-match-action", + "action": "read", "object": { - "id": "00000000-0000-0000-0000-000000000000", - "owner": "00000000-0000-0000-0000-000000000000", - "org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6", + "id": "9046b041-58ed-47a3-9c3a-de302577875a", + "owner": "me", + "org_owner": "1fef0c81-8f3c-4ee8-9667-94351578ed7f", "type": "workspace", - "acl_user_list": { - "f041847d-711b-40da-a89a-ede39f70dc7f": ["create"] - }, - "acl_group_list": {} + "acl_user_list": null, + "acl_group_list": null }, "subject": { - "id": "10d03e62-7703-4df5-a358-4f76577d4e2f", - "roles": [], - "groups": ["b617a647-b5d0-4cbe-9e40-26f89710bf18"], + "id": "me", + "roles": [ + { + "name": "member", + "display_name": "", + "site": [ + { "negate": false, "resource_type": "user", "action": "read" }, + { "negate": false, "resource_type": "assign_role", "action": "read" }, + { + "negate": false, + "resource_type": "provisioner_daemon", + "action": "read" + } + ], + "org": null, + "user": [{ "negate": false, "resource_type": "*", "action": "*" }] + }, + { + "name": "organization-member:1fef0c81-8f3c-4ee8-9667-94351578ed7f", + "display_name": "", + "site": null, + "org": { + "1fef0c81-8f3c-4ee8-9667-94351578ed7f": [ + { + "negate": false, + "resource_type": "organization_member", + "action": "read" + }, + { + "negate": false, + "resource_type": "organization", + "action": "read" + }, + { + "negate": false, + "resource_type": "assign_org_role", + "action": "read" + }, + { "negate": false, "resource_type": "group", "action": "read" } + ] + }, + "user": null + } + ], + "groups": null, "scope": { - "name": "Scope_all", - "display_name": "All operations", - "allow_list": ["123", "*"], + "name": "workspace_agent", + "display_name": "Workspace Agent", "site": [ - { - "negate": false, - "resource_type": "*", - "action": "*" - } + { "negate": false, "resource_type": "workspace", "action": "read" } ], "org": {}, - "user": [] + "user": [], + "allow_list": ["9046b041-58ed-47a3-9c3a-de302577875a"] } } } diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 9061be196b8ad..79dfd1b6194c4 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -186,6 +186,17 @@ func (z Object) All() Object { } } +func (z Object) WithIDString(id string) Object { + return Object{ + ID: id, + Owner: z.Owner, + OrgID: z.OrgID, + Type: z.Type, + ACLUserList: z.ACLUserList, + ACLGroupList: z.ACLGroupList, + } +} + func (z Object) WithID(id uuid.UUID) Object { return Object{ ID: id.String(), diff --git a/coderd/users.go b/coderd/users.go index ad2749163f49f..49a028b4d818b 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -976,9 +976,7 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques return } - if !api.Authorize(r, rbac.ActionRead, - rbac.ResourceOrganization. - InOrg(organization.ID)) { + if !api.Authorize(r, rbac.ActionRead, organization) { httpapi.ResourceNotFound(rw) return } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index fe6cd564dfd8d..396e38a6795d5 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -638,7 +638,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request } apiKey := httpmw.APIKey(r) - if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(apiKey.UserID.String())) { + if !api.Authorize(r, rbac.ActionCreate, apiKey) { httpapi.ResourceNotFound(rw) return } diff --git a/enterprise/coderd/coderdenttest/coderdenttest_test.go b/enterprise/coderd/coderdenttest/coderdenttest_test.go index bfd5ee64152f0..63421a2edc6d0 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest_test.go +++ b/enterprise/coderd/coderdenttest/coderdenttest_test.go @@ -44,7 +44,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { }) require.NoError(t, err) - groupObj := rbac.ResourceGroup.InOrg(admin.OrganizationID) + groupObj := rbac.ResourceGroup.WithID(group.ID).InOrg(admin.OrganizationID) a := coderdtest.NewAuthTester(ctx, t, client, api.AGPL, admin) a.URLParams["licenses/{id}"] = fmt.Sprintf("licenses/%d", lic.ID) a.URLParams["groups/{group}"] = fmt.Sprintf("groups/%s", group.ID.String()) diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go index cbf15787a3e1b..7fbc5b42b17c1 100644 --- a/enterprise/coderd/provisionerdaemons.go +++ b/enterprise/coderd/provisionerdaemons.go @@ -55,11 +55,6 @@ func (api *API) provisionerDaemonsEnabledMW(next http.Handler) http.Handler { // @Router /organizations/{organization}/provisionerdaemons [get] func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - org := httpmw.OrganizationParam(r) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceProvisionerDaemon.InOrg(org.ID)) { - httpapi.Forbidden(rw) - return - } daemons, err := api.Database.GetProvisionerDaemons(ctx) if errors.Is(err, sql.ErrNoRows) { err = nil From a9cf8f48fe56d9e3ffce161620b243bd3e1acdee Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 Jan 2023 14:48:05 -0600 Subject: [PATCH 09/16] Plumb through WithID on rbac objects (complete) --- coderd/database/modelmethods.go | 6 +++++- coderd/gitsshkey.go | 4 ++-- coderd/users.go | 14 +++++++------- enterprise/coderd/workspacequota.go | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index d6a50f5fadd96..00c1f40802139 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -83,11 +83,15 @@ func (f File) RBACObject() rbac.Object { // RBACObject returns the RBAC object for the site wide user resource. // If you are trying to get the RBAC object for the UserData, use -// rbac.ResourceUserData +// u.UserDataRBACObject() instead. func (u User) RBACObject() rbac.Object { return rbac.ResourceUser.WithID(u.ID) } +func (u User) UserDataRBACObject() rbac.Object { + return rbac.ResourceUser.WithID(u.ID).WithOwner(u.ID.String()) +} + func (License) RBACObject() rbac.Object { return rbac.ResourceLicense } diff --git a/coderd/gitsshkey.go b/coderd/gitsshkey.go index 5e0dfcfef56f0..22f1a5e9e6c26 100644 --- a/coderd/gitsshkey.go +++ b/coderd/gitsshkey.go @@ -34,7 +34,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) { ) defer commitAudit() - if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) { + if !api.Authorize(r, rbac.ActionUpdate, user.UserDataRBACObject()) { httpapi.ResourceNotFound(rw) return } @@ -93,7 +93,7 @@ func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) { + if !api.Authorize(r, rbac.ActionRead, user.UserDataRBACObject()) { httpapi.ResourceNotFound(rw) return } diff --git a/coderd/users.go b/coderd/users.go index 49a028b4d818b..36cd45fdf6411 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -416,7 +416,7 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) organizationIDs, err := userOrganizationIDs(ctx, api, user) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser) { + if !api.Authorize(r, rbac.ActionRead, user) { httpapi.ResourceNotFound(rw) return } @@ -457,7 +457,7 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) { defer commitAudit() aReq.Old = user - if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUser) { + if !api.Authorize(r, rbac.ActionUpdate, user) { httpapi.ResourceNotFound(rw) return } @@ -563,7 +563,7 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW defer commitAudit() aReq.Old = user - if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceUser) { + if !api.Authorize(r, rbac.ActionDelete, user) { httpapi.ResourceNotFound(rw) return } @@ -640,7 +640,7 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) { defer commitAudit() aReq.Old = user - if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) { + if !api.Authorize(r, rbac.ActionUpdate, user.UserDataRBACObject()) { httpapi.ResourceNotFound(rw) return } @@ -665,7 +665,7 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) { // admins can change passwords without sending old_password if params.OldPassword == "" { - if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUser) { + if !api.Authorize(r, rbac.ActionUpdate, user) { httpapi.Forbidden(rw) return } @@ -753,7 +753,7 @@ func (api *API) userRoles(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) { + if !api.Authorize(r, rbac.ActionRead, user.UserDataRBACObject()) { httpapi.ResourceNotFound(rw) return } @@ -832,7 +832,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { return } - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser) { + if !api.Authorize(r, rbac.ActionRead, user) { httpapi.ResourceNotFound(rw) return } diff --git a/enterprise/coderd/workspacequota.go b/enterprise/coderd/workspacequota.go index 0416324508cb1..22d63c2ece2fb 100644 --- a/enterprise/coderd/workspacequota.go +++ b/enterprise/coderd/workspacequota.go @@ -110,7 +110,7 @@ func (c *committer) CommitQuota( func (api *API) workspaceQuota(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) - if !api.AGPL.Authorize(r, rbac.ActionRead, rbac.ResourceUser) { + if !api.AGPL.Authorize(r, rbac.ActionRead, user) { httpapi.ResourceNotFound(rw) return } From fa44bfea0f9b02b53fbcfffad493198f668be4c6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 08:33:00 -0600 Subject: [PATCH 10/16] Fix TestAuthorizeAllEndpoints token routes --- coderd/coderdtest/authorize.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index b979f54605899..4f5f65270c151 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/coder/coder/coderd/database/databasefake" @@ -85,6 +86,15 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { AssertObject: rbac.ResourceWorkspace, AssertAction: rbac.ActionRead, }, + "GET:/api/v2/users/{user}/keys/tokens": { + AssertObject: rbac.ResourceAPIKey, + AssertAction: rbac.ActionRead, + StatusCode: http.StatusOK, + }, + "GET:/api/v2/users/{user}/keys/{keyid}": { + AssertObject: rbac.ResourceAPIKey, + AssertAction: rbac.ActionRead, + }, "GET:/api/v2/workspacebuilds/{workspacebuild}": { AssertAction: rbac.ActionRead, AssertObject: workspaceRBACObj, @@ -317,6 +327,15 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a if !ok { t.Fail() } + _, err := client.CreateToken(ctx, admin.UserID.String(), codersdk.CreateTokenRequest{ + Lifetime: time.Hour, + Scope: codersdk.APIKeyScopeAll, + }) + require.NoError(t, err, "create token") + + apiKeys, err := client.GetTokens(ctx, admin.UserID.String()) + require.NoError(t, err, "get tokens") + apiKey := apiKeys[0] organization, err := client.Organization(ctx, admin.OrganizationID) require.NoError(t, err, "fetch org") @@ -383,6 +402,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a "{jobID}": templateVersionDryRun.ID.String(), "{templatename}": template.Name, "{workspace_and_agent}": workspace.Name + "." + workspace.LatestBuild.Resources[0].Agents[0].Name, + "{keyid}": apiKey.ID, // Only checking template scoped params here "parameters/{scope}/{id}": fmt.Sprintf("parameters/%s/%s", string(templateParam.Scope), templateParam.ScopeID.String()), From 94ad5682b94d9635fa408993df127cdf51966515 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 09:03:31 -0600 Subject: [PATCH 11/16] Fix TestAuthorizeAllEndpoints provisonerdaemon route --- enterprise/coderd/coderdenttest/coderdenttest_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/enterprise/coderd/coderdenttest/coderdenttest_test.go b/enterprise/coderd/coderdenttest/coderdenttest_test.go index 63421a2edc6d0..59350e07d2940 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest_test.go +++ b/enterprise/coderd/coderdenttest/coderdenttest_test.go @@ -94,10 +94,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { assertRoute["GET:/api/v2/organizations/{organization}/provisionerdaemons"] = coderdtest.RouteCheck{ AssertAction: rbac.ActionRead, AssertObject: rbac.ResourceProvisionerDaemon, - } - assertRoute["GET:/api/v2/organizations/{organization}/provisionerdaemons"] = coderdtest.RouteCheck{ - AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceProvisionerDaemon, + StatusCode: http.StatusOK, } assertRoute["GET:/api/v2/groups/{group}"] = coderdtest.RouteCheck{ AssertAction: rbac.ActionRead, From 043a1d5aed3f7787da0a3f50ad6067670e99cc58 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 09:08:26 -0600 Subject: [PATCH 12/16] Rename Scope -> ScopeName --- coderd/coderdtest/authorize.go | 10 +++++----- coderd/database/modelmethods.go | 2 +- coderd/rbac/authz.go | 10 +++++----- coderd/rbac/authz_internal_test.go | 2 +- coderd/rbac/authz_test.go | 2 +- coderd/rbac/scopes.go | 10 +++++----- coderd/rbac/trace.go | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 4f5f65270c151..9d8c78d82a5a7 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -527,7 +527,7 @@ type authCall struct { SubjectID string Roles []string Groups []string - Scope rbac.Scope + Scope rbac.ScopeName Action rbac.Action Object rbac.Object } @@ -541,11 +541,11 @@ var _ rbac.Authorizer = (*RecordingAuthorizer)(nil) // ByRoleNameSQL does not record the call. This matches the postgres behavior // of not calling Authorize() -func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.Scope, _ []string, _ rbac.Action, _ rbac.Object) error { +func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error { return r.AlwaysReturn } -func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, groups []string, action rbac.Action, object rbac.Object) error { +func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error { r.Called = &authCall{ SubjectID: subjectID, Roles: roleNames, @@ -557,7 +557,7 @@ func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, ro return r.AlwaysReturn } -func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { +func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { return &fakePreparedAuthorizer{ Original: r, SubjectID: subjectID, @@ -577,7 +577,7 @@ type fakePreparedAuthorizer struct { Original *RecordingAuthorizer SubjectID string Roles []string - Scope rbac.Scope + Scope rbac.ScopeName Action rbac.Action Groups []string HardCodedSQLString string diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 00c1f40802139..4afba790042af 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -6,7 +6,7 @@ import ( const AllUsersGroup = "Everyone" -func (s APIKeyScope) ToRBAC() rbac.Scope { +func (s APIKeyScope) ToRBAC() rbac.ScopeName { switch s { case APIKeyScopeAll: return rbac.ScopeAll diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index d5e6759086e83..ec45c7b889a8a 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -18,8 +18,8 @@ import ( ) type Authorizer interface { - ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, groups []string, action Action, object Object) error - PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, groups []string, action Action, objectType string) (PreparedAuthorized, error) + ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error + PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) } type PreparedAuthorized interface { @@ -33,7 +33,7 @@ type PreparedAuthorized interface { // // Ideally the 'CompileToSQL' is used instead for large sets. This cost scales // linearly with the number of objects passed in. -func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope Scope, groups []string, action Action, objects []O) ([]O, error) { +func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) { if len(objects) == 0 { // Nothing to filter return objects, nil @@ -179,7 +179,7 @@ type authSubject struct { // ByRoleName will expand all roleNames into roles before calling Authorize(). // This is the function intended to be used outside this package. // The role is fetched from the builtin map located in memory. -func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, groups []string, action Action, object Object) error { +func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error { start := time.Now() ctx, span := tracing.StartSpan(ctx, trace.WithTimestamp(start), // Reuse the time.Now for metric and trace @@ -239,7 +239,7 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [ return nil } -func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope Scope, groups []string, action Action, objectType string) (PreparedAuthorized, error) { +func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) { start := time.Now() ctx, span := tracing.StartSpan(ctx, trace.WithTimestamp(start), diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 2af549a160573..16ef5bb4687ac 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -77,7 +77,7 @@ func TestFilter(t *testing.T) { SubjectID string Roles []string Action Action - Scope Scope + Scope ScopeName ObjectType string }{ { diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go index 5a5774af49848..eb36fdb665537 100644 --- a/coderd/rbac/authz_test.go +++ b/coderd/rbac/authz_test.go @@ -16,7 +16,7 @@ type benchmarkCase struct { Roles []string Groups []string UserID uuid.UUID - Scope rbac.Scope + Scope rbac.ScopeName } // benchmarkUserCases builds a set of users with different roles and groups. diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 7371509aae6c1..45c347526e9c0 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -6,7 +6,7 @@ import ( "golang.org/x/xerrors" ) -type Scope string +type ScopeName string // TODO: @emyrk rename this struct type ScopeRole struct { @@ -15,12 +15,12 @@ type ScopeRole struct { } const ( - ScopeAll Scope = "all" - ScopeApplicationConnect Scope = "application_connect" + ScopeAll ScopeName = "all" + ScopeApplicationConnect ScopeName = "application_connect" ) // TODO: Support passing in scopeID list for allowlisting resources. -var builtinScopes = map[Scope]ScopeRole{ +var builtinScopes = map[ScopeName]ScopeRole{ // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. ScopeAll: { @@ -50,7 +50,7 @@ var builtinScopes = map[Scope]ScopeRole{ }, } -func ExpandScope(scope Scope) (ScopeRole, error) { +func ExpandScope(scope ScopeName) (ScopeRole, error) { role, ok := builtinScopes[scope] if !ok { return ScopeRole{}, xerrors.Errorf("no scope named %q", scope) diff --git a/coderd/rbac/trace.go b/coderd/rbac/trace.go index ee96fa745063f..642d59b5587cf 100644 --- a/coderd/rbac/trace.go +++ b/coderd/rbac/trace.go @@ -7,7 +7,7 @@ import ( // rbacTraceAttributes are the attributes that are added to all spans created by // the rbac package. These attributes should help to debug slow spans. -func rbacTraceAttributes(roles []string, groupCount int, scope Scope, action Action, objectType string, extra ...attribute.KeyValue) trace.SpanStartOption { +func rbacTraceAttributes(roles []string, groupCount int, scope ScopeName, action Action, objectType string, extra ...attribute.KeyValue) trace.SpanStartOption { return trace.WithAttributes( append(extra, attribute.StringSlice("subject_roles", roles), From e0e6312be841e65ffad69acba43269a14ca1b8b6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 09:10:14 -0600 Subject: [PATCH 13/16] Update input.json with scope allow_list --- coderd/rbac/input.json | 72 +++++++++++------------------------------- 1 file changed, 18 insertions(+), 54 deletions(-) diff --git a/coderd/rbac/input.json b/coderd/rbac/input.json index 1465c198b427a..f762de96baa38 100644 --- a/coderd/rbac/input.json +++ b/coderd/rbac/input.json @@ -1,68 +1,32 @@ { - "action": "read", + "action": "never-match-action", "object": { "id": "9046b041-58ed-47a3-9c3a-de302577875a", - "owner": "me", - "org_owner": "1fef0c81-8f3c-4ee8-9667-94351578ed7f", + "owner": "00000000-0000-0000-0000-000000000000", + "org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6", "type": "workspace", - "acl_user_list": null, - "acl_group_list": null + "acl_user_list": { + "f041847d-711b-40da-a89a-ede39f70dc7f": ["create"] + }, + "acl_group_list": {} }, "subject": { - "id": "me", - "roles": [ - { - "name": "member", - "display_name": "", - "site": [ - { "negate": false, "resource_type": "user", "action": "read" }, - { "negate": false, "resource_type": "assign_role", "action": "read" }, - { - "negate": false, - "resource_type": "provisioner_daemon", - "action": "read" - } - ], - "org": null, - "user": [{ "negate": false, "resource_type": "*", "action": "*" }] - }, - { - "name": "organization-member:1fef0c81-8f3c-4ee8-9667-94351578ed7f", - "display_name": "", - "site": null, - "org": { - "1fef0c81-8f3c-4ee8-9667-94351578ed7f": [ - { - "negate": false, - "resource_type": "organization_member", - "action": "read" - }, - { - "negate": false, - "resource_type": "organization", - "action": "read" - }, - { - "negate": false, - "resource_type": "assign_org_role", - "action": "read" - }, - { "negate": false, "resource_type": "group", "action": "read" } - ] - }, - "user": null - } - ], - "groups": null, + "id": "10d03e62-7703-4df5-a358-4f76577d4e2f", + "roles": [], + "groups": ["b617a647-b5d0-4cbe-9e40-26f89710bf18"], "scope": { - "name": "workspace_agent", - "display_name": "Workspace Agent", + "name": "Scope_all", + "display_name": "All operations", "site": [ - { "negate": false, "resource_type": "workspace", "action": "read" } + { + "negate": false, + "resource_type": "*", + "action": "*" + } ], "org": {}, "user": [], - "allow_list": ["9046b041-58ed-47a3-9c3a-de302577875a"] + "allow_list": ["*"] } } } From 7492385bcd8bfc1973a27f9e0ae2cab3ea14421d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 09:32:16 -0600 Subject: [PATCH 14/16] Allow empty string allow_list --- coderd/rbac/authz_internal_test.go | 72 +++++++++++++++++++++++++++++ coderd/rbac/policy.rego | 1 - coderd/rbac/regosql/compile_test.go | 11 +++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 16ef5bb4687ac..6d63290d43012 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -812,6 +812,25 @@ func TestAuthorizeScope(t *testing.T) { } testAuthorize(t, "User_WorkspaceAgent", user, + // Test cases without ID + cases(func(c authTestCase) authTestCase { + c.actions = []Action{ActionCreate, ActionUpdate, ActionDelete} + c.allow = false + return c + }, []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg)}, + {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.All()}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + }), + // Test all cases with the workspace id cases(func(c authTestCase) authTestCase { c.actions = []Action{ActionCreate, ActionUpdate, ActionDelete} @@ -858,6 +877,59 @@ func TestAuthorizeScope(t *testing.T) { {resource: ResourceWorkspace.WithID(workspaceID).InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionRead}, allow: false}, }, ) + + // This scope can only create workspaces + user = subject{ + UserID: "me", + Roles: []Role{ + must(RoleByName(RoleMember())), + must(RoleByName(RoleOrgMember(defOrg))), + }, + Scope: ScopeRole{ + Role: Role{ + Name: "create_workspace", + DisplayName: "Create Workspace", + Site: permissions(map[string][]Action{ + // Only read access for workspaces. + ResourceWorkspace.Type: {ActionCreate}, + }), + Org: map[string][]Permission{}, + User: []Permission{}, + }, + // Empty string allow_list is allowed for actions like 'create' + AllowIDList: []string{""}, + }, + } + + testAuthorize(t, "CreatWorkspaceScope", user, + // All these cases will fail because a resource ID is set. + cases(func(c authTestCase) authTestCase { + c.actions = []Action{ActionCreate, ActionRead, ActionUpdate, ActionDelete} + c.allow = false + c.resource.ID = uuid.NewString() + return c + }, []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg)}, + {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.All()}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me")}, + {resource: ResourceWorkspace.InOrg(unusedID)}, + {resource: ResourceWorkspace.WithOwner("not-me")}, + }), + + // Test create allowed by scope: + []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionCreate}, allow: true}, + // The scope will return true, but the user perms return false for resources not owned by the user. + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, + }, + ) } // cases applies a given function to all test cases. This makes generalities easier to create. diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index c133935fbe575..1368327412eb0 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -159,7 +159,6 @@ scope_allow_list { # object.id. This line is included to prevent partial compilations from # ever needing to include the object.id. not "*" in input.subject.scope.allow_list - input.object.id != "" input.object.id in input.subject.scope.allow_list } diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go index 0799816f2ddea..6c350b7834639 100644 --- a/coderd/rbac/regosql/compile_test.go +++ b/coderd/rbac/regosql/compile_test.go @@ -142,6 +142,17 @@ func TestRegoQueries(t *testing.T) { ExpectedSQL: p("(false) OR (false)"), VariableConverter: regosql.NoACLConverter(), }, + { + Name: "AllowList", + Queries: []string{ + `input.object.id != "" `, + `input.object.id in ["9046b041-58ed-47a3-9c3a-de302577875a"]`, + }, + // Special case where the bool is wrapped + ExpectedSQL: p(`(id :: text != '') OR ` + + `(id :: text = ANY(ARRAY ['9046b041-58ed-47a3-9c3a-de302577875a']))`), + VariableConverter: regosql.NoACLConverter(), + }, { Name: "TwoExpressions", Queries: []string{ From 64a246dacfe0a0ffce74b477fec83b7204f0ae9d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 09:35:29 -0600 Subject: [PATCH 15/16] Delete unused file. --- coderd/rbac/partial.rego | 1 - 1 file changed, 1 deletion(-) delete mode 100644 coderd/rbac/partial.rego diff --git a/coderd/rbac/partial.rego b/coderd/rbac/partial.rego deleted file mode 100644 index 0ea94a137ae0d..0000000000000 --- a/coderd/rbac/partial.rego +++ /dev/null @@ -1 +0,0 @@ -package partial From ca88a3821bad984df61e6f2cc04ebe8ac773c441 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Jan 2023 10:01:54 -0600 Subject: [PATCH 16/16] Rename Scope back to 'Scope' - Add comments to Scope - Remove extra lines in rego policy --- coderd/rbac/authz.go | 12 ++++++------ coderd/rbac/authz_internal_test.go | 10 +++++----- coderd/rbac/partial.go | 2 +- coderd/rbac/policy.rego | 3 --- coderd/rbac/scopes.go | 14 +++++++++----- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index ec45c7b889a8a..d3e43b4e181a3 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -170,10 +170,10 @@ func NewAuthorizer(registry prometheus.Registerer) *RegoAuthorizer { } type authSubject struct { - ID string `json:"id"` - Roles []Role `json:"roles"` - Groups []string `json:"groups"` - Scope ScopeRole `json:"scope"` + ID string `json:"id"` + Roles []Role `json:"roles"` + Groups []string `json:"groups"` + Scope Scope `json:"scope"` } // ByRoleName will expand all roleNames into roles before calling Authorize(). @@ -216,7 +216,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa // Authorize allows passing in custom Roles. // This is really helpful for unit testing, as we can create custom roles to exercise edge cases. -func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, object Object) error { +func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, object Object) error { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, @@ -275,7 +275,7 @@ func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, // Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). // This will vastly speed up performance if batch authorization on the same type of objects is needed. -func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { +func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { auth, err := newPartialAuthorizer(ctx, subjectID, roles, scope, groups, action, objectType) if err != nil { return nil, xerrors.Errorf("new partial authorizer: %w", err) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 6d63290d43012..204d65551f47e 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -20,9 +20,9 @@ type subject struct { // For the unit test we want to pass in the roles directly, instead of just // by name. This allows us to test custom roles that do not exist in the product, // but test edge cases of the implementation. - Roles []Role `json:"roles"` - Groups []string `json:"groups"` - Scope ScopeRole `json:"scope"` + Roles []Role `json:"roles"` + Groups []string `json:"groups"` + Scope Scope `json:"scope"` } type fakeObject struct { @@ -796,7 +796,7 @@ func TestAuthorizeScope(t *testing.T) { must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, - Scope: ScopeRole{ + Scope: Scope{ Role: Role{ Name: "workspace_agent", DisplayName: "Workspace Agent", @@ -885,7 +885,7 @@ func TestAuthorizeScope(t *testing.T) { must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, - Scope: ScopeRole{ + Scope: Scope{ Role: Role{ Name: "create_workspace", DisplayName: "Create Workspace", diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index bacd2db0c0ba7..1f583ab808df9 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -121,7 +121,7 @@ EachQueryLoop: return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), pa.input, nil) } -func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope ScopeRole, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { +func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index 1368327412eb0..a6f3e62b73453 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -59,7 +59,6 @@ number(set) = c { c := 1 } - # site, org, and user rules are all similar. Each rule should return a number # from [-1, 1]. The number corresponds to "negative", "abstain", and "positive" # for the given level. See the 'allow' rules for how these numbers are used. @@ -162,8 +161,6 @@ scope_allow_list { 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! # Note that the absence of `allow` means "unauthorized". diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 45c347526e9c0..c500aa13334fe 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -8,8 +8,12 @@ import ( type ScopeName string -// TODO: @emyrk rename this struct -type ScopeRole struct { +// Scope acts the exact same as a Role with the addition that is can also +// apply an AllowIDList. Any resource being checked against a Scope will +// reject any resource that is not in the AllowIDList. +// To not use an AllowIDList to reject authorization, use a wildcard for the +// AllowIDList. Eg: 'AllowIDList: []string{WildcardSymbol}' +type Scope struct { Role AllowIDList []string `json:"allow_list"` } @@ -20,7 +24,7 @@ const ( ) // TODO: Support passing in scopeID list for allowlisting resources. -var builtinScopes = map[ScopeName]ScopeRole{ +var builtinScopes = map[ScopeName]Scope{ // ScopeAll is a special scope that allows access to all resources. During // authorize checks it is usually not used directly and skips scope checks. ScopeAll: { @@ -50,10 +54,10 @@ var builtinScopes = map[ScopeName]ScopeRole{ }, } -func ExpandScope(scope ScopeName) (ScopeRole, error) { +func ExpandScope(scope ScopeName) (Scope, error) { role, ok := builtinScopes[scope] if !ok { - return ScopeRole{}, xerrors.Errorf("no scope named %q", scope) + return Scope{}, xerrors.Errorf("no scope named %q", scope) } return role, nil }