diff --git a/coderd/apikey.go b/coderd/apikey.go index dea3525965..babde27084 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,15 +178,9 @@ 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) - return - } - keys, err := api.Database.GetAPIKeysByLoginType(ctx, database.LoginTypeToken) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -197,7 +190,16 @@ func (api *API) tokens(rw http.ResponseWriter, r *http.Request) { return } - apiKeys := []codersdk.APIKey{} + keys, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, keys) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching keys.", + Detail: err.Error(), + }) + return + } + + 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 1541fa2b11..1924ca2e41 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" @@ -32,9 +33,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{ @@ -75,7 +77,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, @@ -85,6 +87,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, @@ -139,11 +150,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}": { @@ -152,64 +163,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, @@ -225,7 +236,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, @@ -317,6 +328,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 +403,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()), @@ -507,7 +528,7 @@ type authCall struct { SubjectID string Roles []string Groups []string - Scope rbac.Scope + Scope rbac.ScopeName Action rbac.Action Object rbac.Object } @@ -521,11 +542,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, @@ -537,7 +558,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, @@ -557,7 +578,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 9c17691d00..487e8a7e6a 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -35,7 +35,7 @@ func (g Group) Auditable(users []User) AuditableGroup { const AllUsersGroup = "Everyone" -func (s APIKeyScope) ToRBAC() rbac.Scope { +func (s APIKeyScope) ToRBAC() rbac.ScopeName { switch s { case APIKeyScopeAll: return rbac.ScopeAll @@ -46,9 +46,14 @@ 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.InOrg(t.OrganizationID). + return rbac.ResourceTemplate.WithID(t.ID). + InOrg(t.OrganizationID). WithACLUserList(t.UserACL). WithGroupACL(t.GroupACL) } @@ -59,42 +64,61 @@ 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 +// 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 { diff --git a/coderd/files.go b/coderd/files.go index bf8a98f605..57e919d7da 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/gitsshkey.go b/coderd/gitsshkey.go index 5e0dfcfef5..22f1a5e9e6 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/organizations.go b/coderd/organizations.go index 7df9da184e..19b57a8958 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/README.md b/coderd/rbac/README.md index 66a8dcb863..2a73a59d7f 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,33 @@ 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. + +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 correlates 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. diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 4d43089fb7..d3e43b4e18 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 @@ -173,13 +173,13 @@ type authSubject struct { ID string `json:"id"` Roles []Role `json:"roles"` Groups []string `json:"groups"` - Scope Role `json:"scope"` + Scope Scope `json:"scope"` } // 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 @@ -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 } @@ -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 Scope, groups []string, action Action, object Object) error { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, @@ -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), @@ -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 } @@ -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 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 5aaa71eca7..204d65551f 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -22,7 +22,7 @@ type subject struct { // but test edge cases of the implementation. Roles []Role `json:"roles"` Groups []string `json:"groups"` - Scope Role `json:"scope"` + Scope Scope `json:"scope"` } type fakeObject struct { @@ -77,7 +77,7 @@ func TestFilter(t *testing.T) { SubjectID string Roles []string Action Action - Scope Scope + Scope ScopeName ObjectType string }{ { @@ -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, @@ -788,6 +788,148 @@ 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: Scope{ + 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 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} + 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}, + }, + ) + + // This scope can only create workspaces + user = subject{ + UserID: "me", + Roles: []Role{ + must(RoleByName(RoleMember())), + must(RoleByName(RoleOrgMember(defOrg))), + }, + Scope: Scope{ + 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/authz_test.go b/coderd/rbac/authz_test.go index b010674387..eb36fdb665 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. @@ -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 85ef6f6507..6a15e77f54 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 6797122a42..f762de96ba 100644 --- a/coderd/rbac/input.json +++ b/coderd/rbac/input.json @@ -1,6 +1,7 @@ { "action": "never-match-action", "object": { + "id": "9046b041-58ed-47a3-9c3a-de302577875a", "owner": "00000000-0000-0000-0000-000000000000", "org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6", "type": "workspace", @@ -24,7 +25,8 @@ } ], "org": {}, - "user": [] + "user": [], + "allow_list": ["*"] } } } diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 4852a4c269..79dfd1b619 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,32 @@ 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(), + 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 +223,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 +235,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 +246,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 b9673c809a..1f583ab808 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 Scope, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { input := map[string]interface{}{ "subject": authSubject{ ID: subjectID, @@ -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/partial.rego b/coderd/rbac/partial.rego deleted file mode 100644 index 0ea94a137a..0000000000 --- a/coderd/rbac/partial.rego +++ /dev/null @@ -1 +0,0 @@ -package partial diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index 095f1844bd..a6f3e62b73 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. @@ -148,6 +147,20 @@ 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 compilations from + # ever needing to include the object.id. + not "*" in input.subject.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". @@ -179,15 +192,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/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go index 0799816f2d..6c350b7834 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{ diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go index 7064ceccb1..475d317cd5 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(), ) diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 57ed21bb64..c500aa1333 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -6,41 +6,58 @@ import ( "golang.org/x/xerrors" ) -type Scope string +type ScopeName string + +// 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"` +} const ( - ScopeAll Scope = "all" - ScopeApplicationConnect Scope = "application_connect" + ScopeAll ScopeName = "all" + ScopeApplicationConnect ScopeName = "application_connect" ) -var builtinScopes map[Scope]Role = map[Scope]Role{ +// TODO: Support passing in scopeID list for allowlisting resources. +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: { - 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 ScopeName) (Scope, error) { role, ok := builtinScopes[scope] if !ok { - return Role{}, xerrors.Errorf("no scope named %q", scope) + return Scope{}, xerrors.Errorf("no scope named %q", scope) } return role, nil } diff --git a/coderd/rbac/trace.go b/coderd/rbac/trace.go index ee96fa7450..642d59b558 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), diff --git a/coderd/users.go b/coderd/users.go index ad2749163f..36cd45fdf6 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 } @@ -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 ccb647b3a5..de47b70616 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -689,7 +689,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 bfd5ee6415..59350e07d2 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()) @@ -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, diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go index cbf15787a3..7fbc5b42b1 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 diff --git a/enterprise/coderd/workspacequota.go b/enterprise/coderd/workspacequota.go index 0416324508..22d63c2ece 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 }