diff --git a/coderd/coderd.go b/coderd/coderd.go index cde85eec1c..c474aeb785 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -170,6 +170,7 @@ func newRouter(options *Options, a *api) chi.Router { r.Route("/{user}", func(r chi.Router) { r.Use( httpmw.ExtractUserParam(options.Database), + httpmw.ExtractOrganizationMemberParam(options.Database), ) r.Put("/roles", a.putMemberRoles) }) @@ -201,6 +202,7 @@ func newRouter(options *Options, a *api) chi.Router { r.Route("/templateversions/{templateversion}", func(r chi.Router) { r.Use( apiKeyMiddleware, + authRolesMiddleware, httpmw.ExtractTemplateVersionParam(options.Database), ) @@ -289,6 +291,7 @@ func newRouter(options *Options, a *api) chi.Router { r.Route("/workspaceresources/{workspaceresource}", func(r chi.Router) { r.Use( apiKeyMiddleware, + authRolesMiddleware, httpmw.ExtractWorkspaceResourceParam(options.Database), httpmw.ExtractWorkspaceParam(options.Database), ) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 4cd7a55e45..1b1f7b4559 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -17,6 +17,8 @@ import ( "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" + "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/provisionersdk/proto" ) func TestMain(m *testing.M) { @@ -47,13 +49,32 @@ func TestAuthorizeAllEndpoints(t *testing.T) { require.NoError(t, err, "fetch org") // Setup some data in the database. - version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + Provision: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + // Return a workspace resource + Resources: []*proto.Resource{{ + Name: "some", + Type: "example", + Agents: []*proto.Agent{{ + Id: "something", + Auth: &proto.Agent_Token{}, + }}, + }}, + }, + }, + }}, + }) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) workspace := coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) file, err := client.Upload(ctx, codersdk.ContentTypeTar, make([]byte, 1024)) require.NoError(t, err, "upload file") + workspaceResources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID) + require.NoError(t, err, "workspace resources") // Always fail auth from this point forward authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil) @@ -78,6 +99,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "POST:/api/v2/users/logout": {NoAuthorize: true}, "GET:/api/v2/users/authmethods": {NoAuthorize: true}, + // Has it's own auth + "GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true}, + // All workspaceagents endpoints do not use rbac "POST:/api/v2/workspaceagents/aws-instance-identity": {NoAuthorize: true}, "POST:/api/v2/workspaceagents/azure-instance-identity": {NoAuthorize: true}, @@ -94,11 +118,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true}, // TODO: @emyrk these need to be fixed by adding authorize calls - "GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true}, - - "GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true}, - - "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, "GET:/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true}, "GET:/api/v2/organizations/{organization}/templates/{templatename}": {NoAuthorize: true}, "POST:/api/v2/organizations/{organization}/templateversions": {NoAuthorize: true}, @@ -108,17 +127,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/parameters/{scope}/{id}": {NoAuthorize: true}, "DELETE:/api/v2/parameters/{scope}/{id}/{name}": {NoAuthorize: true}, - "GET:/api/v2/templates/{template}/versions": {NoAuthorize: true}, - "PATCH:/api/v2/templates/{template}/versions": {NoAuthorize: true}, - "GET:/api/v2/templates/{template}/versions/{templateversionname}": {NoAuthorize: true}, - - "GET:/api/v2/templateversions/{templateversion}": {NoAuthorize: true}, - "PATCH:/api/v2/templateversions/{templateversion}/cancel": {NoAuthorize: true}, - "GET:/api/v2/templateversions/{templateversion}/logs": {NoAuthorize: true}, - "GET:/api/v2/templateversions/{templateversion}/parameters": {NoAuthorize: true}, - "GET:/api/v2/templateversions/{templateversion}/resources": {NoAuthorize: true}, - "GET:/api/v2/templateversions/{templateversion}/schema": {NoAuthorize: true}, - "POST:/api/v2/users/{user}/organizations": {NoAuthorize: true}, "GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize: true}, @@ -164,6 +172,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertAction: rbac.ActionUpdate, AssertObject: workspaceRBACObj, }, + "GET:/api/v2/workspaceresources/{workspaceresource}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, "PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": { AssertAction: rbac.ActionUpdate, AssertObject: workspaceRBACObj, @@ -199,12 +211,51 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), }, "POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile}, - "GET:/api/v2/files/{fileHash}": {AssertAction: rbac.ActionRead, - AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash)}, + "GET:/api/v2/files/{fileHash}": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash), + }, + "GET:/api/v2/templates/{template}/versions": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "PATCH:/api/v2/templates/{template}/versions": { + AssertAction: rbac.ActionUpdate, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templates/{template}/versions/{templateversionname}": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templateversions/{templateversion}": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "PATCH:/api/v2/templateversions/{templateversion}/cancel": { + AssertAction: rbac.ActionUpdate, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templateversions/{templateversion}/logs": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templateversions/{templateversion}/parameters": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templateversions/{templateversion}/resources": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, + "GET:/api/v2/templateversions/{templateversion}/schema": { + AssertAction: rbac.ActionRead, + AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), + }, // These endpoints need payloads to get to the auth part. Payloads will be required - "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, - "POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, + "POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, } for k, v := range assertRoute { @@ -240,6 +291,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) { route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name) route = strings.ReplaceAll(route, "{template}", template.ID.String()) route = strings.ReplaceAll(route, "{hash}", file.Hash) + route = strings.ReplaceAll(route, "{workspaceresource}", workspaceResources[0].ID.String()) + route = strings.ReplaceAll(route, "{templateversion}", version.ID.String()) resp, err := client.Request(context.Background(), method, route, nil) require.NoError(t, err, "do req") diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index d0494738c6..7ee5a2bc4d 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -276,8 +276,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui for orgID, roles := range orgRoles { organizationID, err := uuid.Parse(orgID) require.NoError(t, err, fmt.Sprintf("parse org id %q", orgID)) - // TODO: @Emyrk add the member to the organization if they do not already belong. - _, err = other.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(), + _, err = client.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(), codersdk.UpdateRoles{Roles: append(roles, rbac.RoleOrgMember(organizationID))}) require.NoError(t, err, "update org membership roles") } diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index e00628e144..2718ab8eec 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -6,6 +6,11 @@ func (t Template) RBACObject() rbac.Object { return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String()) } +func (t TemplateVersion) RBACObject() rbac.Object { + // Just use the parent template resource for controlling versions + return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.TemplateID.UUID.String()) +} + func (w Workspace) RBACObject() rbac.Object { return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String()) } diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go index 8b088ee76b..1bac16d4a7 100644 --- a/coderd/httpmw/organizationparam.go +++ b/coderd/httpmw/organizationparam.go @@ -28,12 +28,12 @@ func OrganizationParam(r *http.Request) database.Organization { func OrganizationMemberParam(r *http.Request) database.OrganizationMember { organizationMember, ok := r.Context().Value(organizationMemberParamContextKey{}).(database.OrganizationMember) if !ok { - panic("developer error: organization param middleware not provided") + panic("developer error: organization member param middleware not provided") } return organizationMember } -// ExtractOrganizationParam grabs an organization and user membership from the "organization" URL parameter. +// ExtractOrganizationParam grabs an organization from the "organization" URL parameter. // This middleware requires the API key middleware higher in the call stack for authentication. func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { @@ -56,11 +56,23 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler }) return } + ctx := context.WithValue(r.Context(), organizationParamContextKey{}, organization) + next.ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} + +// ExtractOrganizationMemberParam grabs a user membership from the "organization" and "user" URL parameter. +// This middleware requires the ExtractUser and ExtractOrganization middleware higher in the stack +func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + organization := OrganizationParam(r) + user := UserParam(r) - apiKey := APIKey(r) organizationMember, err := db.GetOrganizationMemberByUserID(r.Context(), database.GetOrganizationMemberByUserIDParams{ OrganizationID: organization.ID, - UserID: apiKey.UserID, + UserID: user.ID, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusForbidden, httpapi.Response{ @@ -74,9 +86,8 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler }) return } + ctx := context.WithValue(r.Context(), organizationMemberParamContextKey{}, organizationMember) - ctx := context.WithValue(r.Context(), organizationParamContextKey{}, organization) - ctx = context.WithValue(ctx, organizationMemberParamContextKey{}, organizationMember) next.ServeHTTP(rw, r.WithContext(ctx)) }) } diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go index 2b3a01cc6a..ec01d992bd 100644 --- a/coderd/httpmw/organizationparam_test.go +++ b/coderd/httpmw/organizationparam_test.go @@ -122,7 +122,7 @@ func TestOrganizationParam(t *testing.T) { var ( db = databasefake.New() rw = httptest.NewRecorder() - r, _ = setupAuthentication(db) + r, u = setupAuthentication(db) rtr = chi.NewRouter() ) organization, err := db.InsertOrganization(r.Context(), database.InsertOrganizationParams{ @@ -133,9 +133,12 @@ func TestOrganizationParam(t *testing.T) { }) require.NoError(t, err) chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) + chi.RouteContext(r.Context()).URLParams.Add("user", u.ID.String()) rtr.Use( httpmw.ExtractAPIKey(db, nil), + httpmw.ExtractUserParam(db), httpmw.ExtractOrganizationParam(db), + httpmw.ExtractOrganizationMemberParam(db), ) rtr.Get("/", nil) rtr.ServeHTTP(rw, r) @@ -167,9 +170,12 @@ func TestOrganizationParam(t *testing.T) { }) require.NoError(t, err) chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) + chi.RouteContext(r.Context()).URLParams.Add("user", user.ID.String()) rtr.Use( httpmw.ExtractAPIKey(db, nil), httpmw.ExtractOrganizationParam(db), + httpmw.ExtractUserParam(db), + httpmw.ExtractOrganizationMemberParam(db), ) rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) { _ = httpmw.OrganizationParam(r) diff --git a/coderd/members.go b/coderd/members.go index 670e43fb68..4c716c537a 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -17,27 +17,29 @@ import ( ) func (api *api) putMemberRoles(rw http.ResponseWriter, r *http.Request) { - // User is the user to modify - // TODO: Until rbac authorize is implemented, only be able to change your - // own roles. This also means you can grant yourself whatever roles you want. user := httpmw.UserParam(r) - apiKey := httpmw.APIKey(r) organization := httpmw.OrganizationParam(r) - // TODO: @emyrk add proper `Authorize()` check here instead of a uuid match. - // Proper authorize should check the granted roles are able to given within - // the selected organization. Until then, allow anarchy - if apiKey.UserID != user.ID { - httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: "modifying other users is not supported at this time", - }) - return - } + member := httpmw.OrganizationMemberParam(r) var params codersdk.UpdateRoles if !httpapi.Read(rw, r, ¶ms) { return } + added, removed := rbac.ChangeRoleSet(member.Roles, params.Roles) + for _, roleName := range added { + // Assigning a role requires the create permission. + if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) { + return + } + } + for _, roleName := range removed { + // Removing a role requires the delete permission. + if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) { + return + } + } + updatedUser, err := api.updateOrganizationMemberRoles(r.Context(), database.UpdateMemberRolesParams{ GrantedRoles: params.Roles, UserID: user.ID, diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 5e56649609..b9f9e1f825 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -3,6 +3,7 @@ package rbac import ( "context" _ "embed" + "golang.org/x/xerrors" "github.com/open-policy-agent/opa/rego" diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 0bb5e413a4..b530e3bbac 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -135,6 +135,12 @@ var ( Action: ActionRead, ResourceID: "*", }, + { + // Can read available roles. + ResourceType: ResourceOrgRoleAssignment.Type, + ResourceID: "*", + Action: ActionRead, + }, }, }, } @@ -217,6 +223,37 @@ func SiteRoles() []Role { return roles } +// ChangeRoleSet is a helper function that finds the difference of 2 sets of +// roles. When setting a user's new roles, it is equivalent to adding and +// removing roles. This set determines the changes, so that the appropriate +// RBAC checks can be applied using "ActionCreate" and "ActionDelete" for +// "added" and "removed" roles respectively. +func ChangeRoleSet(from []string, to []string) (added []string, removed []string) { + has := make(map[string]struct{}) + for _, exists := range from { + has[exists] = struct{}{} + } + + for _, roleName := range to { + // If the user already has the role assigned, we don't need to check the permission + // to reassign it. Only run permission checks on the difference in the set of + // roles. + if _, ok := has[roleName]; ok { + delete(has, roleName) + continue + } + + added = append(added, roleName) + } + + // Remaining roles are the ones removed/deleted. + for roleName := range has { + removed = append(removed, roleName) + } + + return added, removed +} + // roleName is a quick helper function to return // role_name:scopeID // If no scopeID is required, only 'role_name' is returned diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index 72cde1eea4..44f54a3626 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -93,3 +93,52 @@ func TestListRoles(t *testing.T) { }, orgRoleNames) } + +func TestChangeSet(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + From []string + To []string + ExpAdd []string + ExpRemove []string + }{ + { + Name: "Empty", + }, + { + Name: "Same", + From: []string{"a", "b", "c"}, + To: []string{"a", "b", "c"}, + ExpAdd: []string{}, + ExpRemove: []string{}, + }, + { + Name: "AllRemoved", + From: []string{"a", "b", "c"}, + ExpRemove: []string{"a", "b", "c"}, + }, + { + Name: "AllAdded", + To: []string{"a", "b", "c"}, + ExpAdd: []string{"a", "b", "c"}, + }, + { + Name: "AddAndRemove", + From: []string{"a", "b", "c"}, + To: []string{"a", "b", "d", "e"}, + ExpAdd: []string{"d", "e"}, + ExpRemove: []string{"c"}, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + add, remove := rbac.ChangeRoleSet(c.From, c.To) + require.ElementsMatch(t, c.ExpAdd, add, "expect added") + require.ElementsMatch(t, c.ExpRemove, remove, "expect removed") + }) + } +} diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index c40e0e9aba..9ebbb30493 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -22,6 +22,10 @@ var ( Type: "workspace", } + // ResourceTemplate CRUD. Org owner only. + // create/delete = Make or delete a new template + // update = Update the template, make new template versions + // read = read the template and all versions associated ResourceTemplate = Object{ Type: "template", } @@ -41,6 +45,7 @@ var ( // ResourceRoleAssignment might be expanded later to allow more granular permissions // to modifying roles. For now, this covers all possible roles, so having this permission // allows granting/deleting **ALL** roles. + // Never has an owner or org. // create = Assign roles // update = ?? // read = View available roles to assign @@ -49,6 +54,11 @@ var ( Type: "assign_role", } + // ResourceOrgRoleAssignment is just like ResourceRoleAssignment but for organization roles. + ResourceOrgRoleAssignment = Object{ + Type: "assign_org_role", + } + // ResourceAPIKey is owned by a user. // create = Create a new api key for user // update = ?? diff --git a/coderd/roles.go b/coderd/roles.go index 308b1bf791..58d7ea96c6 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -29,7 +29,7 @@ func (api *api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { // role of the user. organization := httpmw.OrganizationParam(r) - if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceRoleAssignment.InOrg(organization.ID)) { + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) { return } diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 22397b7b34..87b994949d 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -112,7 +112,7 @@ func TestListRoles(t *testing.T) { }) require.NoError(t, err, "create org") - const notMember = "not a member of the organization" + const forbidden = "forbidden" testCases := []struct { Name string @@ -141,7 +141,7 @@ func TestListRoles(t *testing.T) { APICall: func() ([]codersdk.Role, error) { return member.ListOrganizationRoles(ctx, otherOrg.ID) }, - AuthorizedError: notMember, + AuthorizedError: forbidden, }, // Org admin { @@ -163,7 +163,7 @@ func TestListRoles(t *testing.T) { APICall: func() ([]codersdk.Role, error) { return orgAdmin.ListOrganizationRoles(ctx, otherOrg.ID) }, - AuthorizedError: notMember, + AuthorizedError: forbidden, }, // Admin { diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 560281908d..bf8438d322 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -15,11 +15,16 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/parameter" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" ) func (api *api) templateVersion(rw http.ResponseWriter, r *http.Request) { templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -33,6 +38,10 @@ func (api *api) templateVersion(rw http.ResponseWriter, r *http.Request) { func (api *api) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Request) { templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -72,6 +81,10 @@ func (api *api) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Reque func (api *api) templateVersionSchema(rw http.ResponseWriter, r *http.Request) { templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -112,6 +125,10 @@ func (api *api) templateVersionSchema(rw http.ResponseWriter, r *http.Request) { func (api *api) templateVersionParameters(rw http.ResponseWriter, r *http.Request) { apiKey := httpmw.APIKey(r) templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -148,6 +165,9 @@ func (api *api) templateVersionParameters(rw http.ResponseWriter, r *http.Reques func (api *api) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Request) { template := httpmw.TemplateParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, template) { + return + } paginationParams, ok := parsePagination(rw, r) if !ok { @@ -203,6 +223,10 @@ func (api *api) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque func (api *api) templateVersionByName(rw http.ResponseWriter, r *http.Request) { template := httpmw.TemplateParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, template) { + return + } + templateVersionName := chi.URLParam(r, "templateversionname") templateVersion, err := api.Database.GetTemplateVersionByTemplateIDAndName(r.Context(), database.GetTemplateVersionByTemplateIDAndNameParams{ TemplateID: uuid.NullUUID{ @@ -235,11 +259,15 @@ func (api *api) templateVersionByName(rw http.ResponseWriter, r *http.Request) { } func (api *api) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Request) { + template := httpmw.TemplateParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, template) { + return + } + var req codersdk.UpdateActiveTemplateVersion if !httpapi.Read(rw, r, &req) { return } - template := httpmw.TemplateParam(r) version, err := api.Database.GetTemplateVersionByID(r.Context(), req.ID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusNotFound, httpapi.Response{ @@ -382,8 +410,17 @@ func (api *api) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht httpapi.Write(rw, http.StatusCreated, convertTemplateVersion(templateVersion, convertProvisionerJob(provisionerJob))) } +// templateVersionResources returns the workspace agent resources associated +// with a template version. A template can specify more than one resource to be +// provisioned, each resource can have an agent that dials back to coderd. +// The agents returned are informative of the template version, and do not +// return agents associated with any particular workspace. func (api *api) templateVersionResources(rw http.ResponseWriter, r *http.Request) { templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -394,8 +431,16 @@ func (api *api) templateVersionResources(rw http.ResponseWriter, r *http.Request api.provisionerJobResources(rw, r, job) } +// templateVersionLogs returns the logs returned by the provisioner for the given +// template version. These logs are only associated with the template version, +// and not any build logs for a workspace. +// Eg: Logs returned from 'terraform plan' when uploading a new terraform file. func (api *api) templateVersionLogs(rw http.ResponseWriter, r *http.Request) { templateVersion := httpmw.TemplateVersionParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ diff --git a/coderd/users.go b/coderd/users.go index 2d1c1b58d1..0e0ce9bda2 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -431,28 +431,15 @@ func (api *api) putUserRoles(rw http.ResponseWriter, r *http.Request) { return } - has := make(map[string]struct{}) - for _, exists := range roles.Roles { - has[exists] = struct{}{} - } - - for _, roleName := range params.Roles { - // If the user already has the role assigned, we don't need to check the permission - // to reassign it. Only run permission checks on the difference in the set of - // roles. - if _, ok := has[roleName]; ok { - delete(has, roleName) - continue - } - + added, removed := rbac.ChangeRoleSet(roles.Roles, params.Roles) + for _, roleName := range added { // Assigning a role requires the create permission. if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceRoleAssignment.WithID(roleName)) { return } } - - // Any roles that were removed also need to be checked. - for roleName := range has { + for _, roleName := range removed { + // Removing a role requires the delete permission. if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceRoleAssignment.WithID(roleName)) { return } diff --git a/coderd/users_test.go b/coderd/users_test.go index 6d452f1b3f..b1c3bddc2e 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -328,42 +328,64 @@ func TestUpdateUserPassword(t *testing.T) { func TestGrantRoles(t *testing.T) { t.Parallel() + + requireStatusCode := func(t *testing.T, err error, statusCode int) { + t.Helper() + var e *codersdk.Error + require.ErrorAs(t, err, &e, "error is codersdk error") + require.Equal(t, statusCode, e.StatusCode(), "correct status code") + } + t.Run("UpdateIncorrectRoles", func(t *testing.T) { t.Parallel() ctx := context.Background() admin := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, admin) member := coderdtest.CreateAnotherUser(t, admin, first.OrganizationID) + memberUser, err := member.User(ctx, codersdk.Me) + require.NoError(t, err, "member user") - _, err := admin.UpdateUserRoles(ctx, codersdk.Me, codersdk.UpdateRoles{ + _, err = admin.UpdateUserRoles(ctx, codersdk.Me, codersdk.UpdateRoles{ Roles: []string{rbac.RoleOrgMember(first.OrganizationID)}, }) require.Error(t, err, "org role in site") + requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateUserRoles(ctx, uuid.New().String(), codersdk.UpdateRoles{ Roles: []string{rbac.RoleOrgMember(first.OrganizationID)}, }) require.Error(t, err, "user does not exist") + requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, codersdk.Me, codersdk.UpdateRoles{ Roles: []string{rbac.RoleMember()}, }) require.Error(t, err, "site role in org") + requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateOrganizationMemberRoles(ctx, uuid.New(), codersdk.Me, codersdk.UpdateRoles{ Roles: []string{rbac.RoleMember()}, }) require.Error(t, err, "role in org without membership") + requireStatusCode(t, err, http.StatusNotFound) _, err = member.UpdateUserRoles(ctx, first.UserID.String(), codersdk.UpdateRoles{ Roles: []string{rbac.RoleMember()}, }) require.Error(t, err, "member cannot change other's roles") + requireStatusCode(t, err, http.StatusForbidden) + + _, err = member.UpdateUserRoles(ctx, memberUser.ID.String(), codersdk.UpdateRoles{ + Roles: []string{rbac.RoleMember()}, + }) + require.Error(t, err, "member cannot change any roles") + requireStatusCode(t, err, http.StatusForbidden) _, err = member.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, first.UserID.String(), codersdk.UpdateRoles{ Roles: []string{rbac.RoleMember()}, }) require.Error(t, err, "member cannot change other's org roles") + requireStatusCode(t, err, http.StatusForbidden) }) t.Run("FirstUserRoles", func(t *testing.T) { diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index a337b3828f..1bdf1f83e9 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -10,12 +10,18 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" ) func (api *api) workspaceResource(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) workspaceResource := httpmw.WorkspaceResourceParam(r) + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, workspace) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), workspaceBuild.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{