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 1541fa2b119ae..1924ca2e41b9d 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 9c17691d009a5..487e8a7e6a250 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 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/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/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/README.md b/coderd/rbac/README.md index 66a8dcb863293..2a73a59d7febc 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 4d43089fb72ea..d3e43b4e181a3 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 5aaa71eca7ea6..204d65551f47e 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 b010674387c3d..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. @@ -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 6797122a42a55..f762de96baa38 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 4852a4c269638..79dfd1b6194c4 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 b9673c809a84e..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 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 0ea94a137ae0d..0000000000000 --- 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 095f1844bd78d..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. @@ -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 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{ 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(), ) diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 57ed21bb644b6..c500aa13334fe 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 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), diff --git a/coderd/users.go b/coderd/users.go index ad2749163f49f..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 } @@ -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 ccb647b3a5f52..de47b70616440 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 bfd5ee64152f0..59350e07d2940 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 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 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 }