From a2fdb4268de3cdeffbfa2eb6ba3d865bbe3ca35d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 13:50:06 -0500 Subject: [PATCH 1/8] feat: Add RBAC to /workspace endpoints --- coderd/coderd.go | 5 +- coderd/coderd_test.go | 80 +++++++++++++++++++++++++------- coderd/users.go | 8 ++++ coderd/workspacebuilds.go | 98 ++++++++++++++++++++++++++++++++++++++- coderd/workspaces.go | 25 ++++++---- 5 files changed, 186 insertions(+), 30 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 1f91e97ccd889..ab3d18fe0e774 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -149,7 +149,7 @@ func New(options *Options) (http.Handler, func()) { r.Get("/", api.workspacesByOrganization) r.Route("/{user}", func(r chi.Router) { r.Use(httpmw.ExtractUserParam(options.Database)) - r.Get("/{workspace}", api.workspaceByOwnerAndName) + r.Get("/{workspacename}", api.workspaceByOwnerAndName) r.Get("/", api.workspacesByOwner) }) }) @@ -237,8 +237,6 @@ func New(options *Options) (http.Handler, func()) { r.Route("/password", func(r chi.Router) { r.Put("/", api.putUserPassword) }) - r.Get("/organizations", api.organizationsByUser) - r.Post("/organizations", api.postOrganizationsByUser) // These roles apply to the site wide permissions. r.Put("/roles", api.putUserRoles) r.Get("/roles", api.userRoles) @@ -311,6 +309,7 @@ func New(options *Options) (http.Handler, func()) { r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) { r.Use( apiKeyMiddleware, + authRolesMiddleware, httpmw.ExtractWorkspaceBuildParam(options.Database), httpmw.ExtractWorkspaceParam(options.Database), ) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index d096682957538..2ec6516b51a69 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -2,6 +2,7 @@ package coderd_test import ( "context" + "io" "net/http" "strings" "testing" @@ -48,13 +49,18 @@ func TestAuthorizeAllEndpoints(t *testing.T) { 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) // Always fail auth from this point forward authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil) + // Some quick reused objects + workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()) + // skipRoutes allows skipping routes from being checked. type routeCheck struct { NoAuthorize bool + AssertAction rbac.Action AssertObject rbac.Object StatusCode int } @@ -85,13 +91,7 @@ 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/workspacebuilds/{workspacebuild}": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/logs": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/resources": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/state": {NoAuthorize: true}, - "PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": {NoAuthorize: true}, - "GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": {NoAuthorize: true}, + "GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true}, "GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true}, @@ -125,12 +125,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "POST:/api/v2/users/{user}/organizations": {NoAuthorize: true}, - "GET:/api/v2/workspaces/{workspace}": {NoAuthorize: true}, - "PUT:/api/v2/workspaces/{workspace}/autostart": {NoAuthorize: true}, - "PUT:/api/v2/workspaces/{workspace}/autostop": {NoAuthorize: true}, - "GET:/api/v2/workspaces/{workspace}/builds": {NoAuthorize: true}, - "POST:/api/v2/workspaces/{workspace}/builds": {NoAuthorize: true}, - "POST:/api/v2/files": {NoAuthorize: true}, "GET:/api/v2/files/{hash}": {NoAuthorize: true}, @@ -139,13 +133,55 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization}, "GET:/api/v2/users/{user}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, "GET:/api/v2/organizations/{organization}/workspaces/{user}": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, - "GET:/api/v2/organizations/{organization}/workspaces/{user}/{workspace}": { - AssertObject: rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()), + "GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/organizations/{organization}/workspaces/{user}/{workspacename}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, }, "GET:/api/v2/organizations/{organization}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, + "GET:/api/v2/workspacebuilds/{workspacebuild}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspacebuilds/{workspacebuild}/logs": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspaces/{workspace}/builds": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspaces/{workspace}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "PUT:/api/v2/workspaces/{workspace}/autostart": { + AssertAction: rbac.ActionUpdate, + AssertObject: workspaceRBACObj, + }, + "PUT:/api/v2/workspaces/{workspace}/autostop": { + AssertAction: rbac.ActionUpdate, + AssertObject: workspaceRBACObj, + }, + "PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": { + AssertAction: rbac.ActionUpdate, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspacebuilds/{workspacebuild}/resources": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspacebuilds/{workspacebuild}/state": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, - // These endpoints need payloads to get to the auth part. - "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + // 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}, } c, _ := srv.Config.Handler.(*chi.Mux) @@ -166,16 +202,24 @@ func TestAuthorizeAllEndpoints(t *testing.T) { route = strings.ReplaceAll(route, "{organization}", admin.OrganizationID.String()) route = strings.ReplaceAll(route, "{user}", admin.UserID.String()) route = strings.ReplaceAll(route, "{organizationname}", organization.Name) - route = strings.ReplaceAll(route, "{workspace}", workspace.Name) + route = strings.ReplaceAll(route, "{workspace}", workspace.ID.String()) + route = strings.ReplaceAll(route, "{workspacebuild}", workspace.LatestBuild.ID.String()) + route = strings.ReplaceAll(route, "{workspacename}", workspace.Name) + route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name) resp, err := client.Request(context.Background(), method, route, nil) require.NoError(t, err, "do req") + body, _ := io.ReadAll(resp.Body) + t.Logf("Response Body: %q", string(body)) _ = resp.Body.Close() if !routeAssertions.NoAuthorize { assert.NotNil(t, authorizer.Called, "authorizer expected") assert.Equal(t, routeAssertions.StatusCode, resp.StatusCode, "expect unauthorized") if authorizer.Called != nil { + if routeAssertions.AssertAction != "" { + assert.Equal(t, routeAssertions.AssertAction, authorizer.Called.Action, "resource action") + } if routeAssertions.AssertObject.Type != "" { assert.Equal(t, routeAssertions.AssertObject.Type, authorizer.Called.Object.Type, "resource type") } diff --git a/coderd/users.go b/coderd/users.go index fbbbde5e250c1..72a3f491bb02c 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -568,6 +568,14 @@ func (api *api) postOrganizationsByUser(rw http.ResponseWriter, r *http.Request) if !httpapi.Read(rw, r, &req) { return } + + // Create organization uses the organization resource without an OrgID. + // This means you need the site wide permission to make a new organization. + if !api.Authorize(rw, r, rbac.ActionCreate, + rbac.ResourceOrganization) { + return + } + _, err := api.Database.GetOrganizationByName(r.Context(), req.Name) if err == nil { httpapi.Write(rw, http.StatusConflict, httpapi.Response{ diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index e9e908bc660eb..2c49d04e015b1 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -7,6 +7,8 @@ import ( "fmt" "net/http" + "github.com/coder/coder/coderd/rbac" + "github.com/go-chi/chi/v5" "github.com/google/uuid" "github.com/moby/moby/pkg/namesgenerator" @@ -20,6 +22,19 @@ import ( func (api *api) workspaceBuild(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) + workspace, err := api.Database.GetWorkspaceByID(r.Context(), workspaceBuild.WorkspaceID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "no workspace exists for this job", + }) + return + } + + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), workspaceBuild.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -34,6 +49,11 @@ func (api *api) workspaceBuild(rw http.ResponseWriter, r *http.Request) { func (api *api) workspaceBuilds(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + builds, err := api.Database.GetWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if xerrors.Is(err, sql.ErrNoRows) { err = nil @@ -80,6 +100,11 @@ func (api *api) workspaceBuilds(rw http.ResponseWriter, r *http.Request) { func (api *api) workspaceBuildByName(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + workspaceBuildName := chi.URLParam(r, "workspacebuildname") workspaceBuild, err := api.Database.GetWorkspaceBuildByWorkspaceIDAndName(r.Context(), database.GetWorkspaceBuildByWorkspaceIDAndNameParams{ WorkspaceID: workspace.ID, @@ -111,10 +136,30 @@ func (api *api) workspaceBuildByName(rw http.ResponseWriter, r *http.Request) { func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { apiKey := httpmw.APIKey(r) workspace := httpmw.WorkspaceParam(r) + var createBuild codersdk.CreateWorkspaceBuildRequest if !httpapi.Read(rw, r, &createBuild) { return } + + // Rbac action depends on the transition + var action rbac.Action + switch createBuild.Transition { + case database.WorkspaceTransitionDelete: + action = rbac.ActionDelete + case database.WorkspaceTransitionStart, database.WorkspaceTransitionStop: + action = rbac.ActionUpdate + default: + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("transition not supported: %q", createBuild.Transition), + }) + return + } + if !api.Authorize(rw, r, action, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + if createBuild.TemplateVersionID == uuid.Nil { latestBuild, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) if err != nil { @@ -278,6 +323,19 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { func (api *api) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) + workspace, err := api.Database.GetWorkspaceByID(r.Context(), workspaceBuild.WorkspaceID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "no workspace exists for this job", + }) + return + } + + if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), workspaceBuild.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -317,6 +375,19 @@ func (api *api) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques func (api *api) workspaceBuildResources(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) + workspace, err := api.Database.GetWorkspaceByID(r.Context(), workspaceBuild.WorkspaceID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "no workspace exists for this job", + }) + return + } + + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), workspaceBuild.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -329,6 +400,19 @@ func (api *api) workspaceBuildResources(rw http.ResponseWriter, r *http.Request) func (api *api) workspaceBuildLogs(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) + workspace, err := api.Database.GetWorkspaceByID(r.Context(), workspaceBuild.WorkspaceID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "no workspace exists for this job", + }) + return + } + + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + job, err := api.Database.GetProvisionerJobByID(r.Context(), workspaceBuild.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -339,8 +423,20 @@ func (api *api) workspaceBuildLogs(rw http.ResponseWriter, r *http.Request) { api.provisionerJobLogs(rw, r, job) } -func (*api) workspaceBuildState(rw http.ResponseWriter, r *http.Request) { +func (api *api) workspaceBuildState(rw http.ResponseWriter, r *http.Request) { workspaceBuild := httpmw.WorkspaceBuildParam(r) + workspace, err := api.Database.GetWorkspaceByID(r.Context(), workspaceBuild.WorkspaceID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "no workspace exists for this job", + }) + return + } + + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } rw.Header().Set("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 0e473fbdbcd79..a33f8d12f3c38 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -24,6 +24,10 @@ import ( func (api *api) workspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, + rbac.ResourceWorkspace.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } build, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) if err != nil { @@ -58,11 +62,6 @@ func (api *api) workspace(rw http.ResponseWriter, r *http.Request) { return } - if !api.Authorize(rw, r, rbac.ActionRead, - rbac.ResourceWorkspace.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { - return - } - httpapi.Write(rw, http.StatusOK, convertWorkspace(workspace, convertWorkspaceBuild(build, convertProvisionerJob(job)), template, owner)) } @@ -176,7 +175,7 @@ func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) { func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) { owner := httpmw.UserParam(r) organization := httpmw.OrganizationParam(r) - workspaceName := chi.URLParam(r, "workspace") + workspaceName := chi.URLParam(r, "workspacename") workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ OwnerID: owner.ID, @@ -433,6 +432,12 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req } func (api *api) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + var req codersdk.UpdateWorkspaceAutostartRequest if !httpapi.Read(rw, r, &req) { return @@ -451,7 +456,6 @@ func (api *api) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { dbSched.Valid = true } - workspace := httpmw.WorkspaceParam(r) err := api.Database.UpdateWorkspaceAutostart(r.Context(), database.UpdateWorkspaceAutostartParams{ ID: workspace.ID, AutostartSchedule: dbSched, @@ -465,6 +469,12 @@ func (api *api) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { } func (api *api) putWorkspaceAutostop(rw http.ResponseWriter, r *http.Request) { + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + var req codersdk.UpdateWorkspaceAutostopRequest if !httpapi.Read(rw, r, &req) { return @@ -483,7 +493,6 @@ func (api *api) putWorkspaceAutostop(rw http.ResponseWriter, r *http.Request) { dbSched.Valid = true } - workspace := httpmw.WorkspaceParam(r) err := api.Database.UpdateWorkspaceAutostop(r.Context(), database.UpdateWorkspaceAutostopParams{ ID: workspace.ID, AutostopSchedule: dbSched, From 7d10d2f70647446508b492dc79a9994c6ff06a73 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 13:58:55 -0500 Subject: [PATCH 2/8] Fix unit test --- coderd/coderd_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 4fe8375c18570..e17be0817d8c3 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -90,13 +90,7 @@ 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/workspacebuilds/{workspacebuild}": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/logs": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/resources": {NoAuthorize: true}, - "GET:/api/v2/workspacebuilds/{workspacebuild}/state": {NoAuthorize: true}, - "PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": {NoAuthorize: true}, - "GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": {NoAuthorize: true}, + "GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true}, "GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true}, @@ -139,6 +133,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/organizations/{organization}/workspaces/{user}": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, "GET:/api/v2/organizations/{organization}/workspaces/{user}/{workspace}": { AssertObject: rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()), + }, "GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": { AssertAction: rbac.ActionRead, AssertObject: workspaceRBACObj, @@ -184,6 +179,11 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertAction: rbac.ActionRead, AssertObject: workspaceRBACObj, }, + "GET:/api/v2/workspaces/": { + StatusCode: http.StatusOK, + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, // 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}, From d8f3f0e3ff0c667dc526124188963e9ab93941ea Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 14:02:47 -0500 Subject: [PATCH 3/8] Go fmt --- coderd/workspacebuilds.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index aa71db48c9395..3ee251c2c424e 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -7,8 +7,6 @@ import ( "fmt" "net/http" - "github.com/coder/coder/coderd/rbac" - "github.com/go-chi/chi/v5" "github.com/google/uuid" "github.com/moby/moby/pkg/namesgenerator" @@ -17,6 +15,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" ) @@ -49,7 +48,7 @@ func (api *api) workspaceBuild(rw http.ResponseWriter, r *http.Request) { func (api *api) workspaceBuilds(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) -if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. + if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace. InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { return } From 22a55cef350aeb85782260198643ab12d997882d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 16:03:50 -0500 Subject: [PATCH 4/8] Creating an org requires admin role --- coderd/users_test.go | 2 +- coderd/workspaces_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 9c2846ed96cbd..e99cc68b68d98 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -173,7 +173,7 @@ func TestPostUsers(t *testing.T) { client := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, client) notInOrg := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) - other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleAdmin(), rbac.RoleMember()) org, err := other.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ Name: "another", }) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 58140b1d00e1d..44c5ef0995b0f 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/coder/coder/coderd/rbac" + "github.com/google/uuid" "github.com/stretchr/testify/require" @@ -52,7 +54,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) { client := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, client) - other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleMember(), rbac.RoleAdmin()) org, err := other.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ Name: "another", }) From 91c19c0f9ef19968773c27923219e8b18d8e3322 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 17:10:25 -0500 Subject: [PATCH 5/8] Fix test --- coderd/coderd_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index e17be0817d8c3..d462263dcd0ae 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -36,7 +36,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { t.Parallel() authorizer := &fakeAuthorizer{} - srv, client := coderdtest.NewMemoryCoderd(t, &coderdtest.Options{ + srv, client := coderdtest.NewWithServer(t, &coderdtest.Options{ Authorizer: authorizer, }) admin := coderdtest.CreateFirstUser(t, client) @@ -123,8 +123,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "POST:/api/v2/users/{user}/organizations": {NoAuthorize: true}, - "POST:/api/v2/files": {NoAuthorize: true}, - "GET:/api/v2/files/{hash}": {NoAuthorize: true}, + "POST:/api/v2/files": {NoAuthorize: true}, + "GET:/api/v2/files/{hash}": {NoAuthorize: true}, + "GET:/api/v2/workspaces/{workspace}/watch": {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(admin.OrganizationID)}, From b2ace2c5436a248f1cae55486bea0bacc4631616 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 17:19:53 -0500 Subject: [PATCH 6/8] add more checks to unit test --- coderd/workspaces_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 1aa7a75117c17..05e3229487028 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -20,7 +20,7 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -func TestWorkspace(t *testing.T) { +func TestAdminViewAllWorkspaces(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) @@ -31,6 +31,22 @@ func TestWorkspace(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) _, err := client.Workspace(context.Background(), workspace.ID) require.NoError(t, err) + + otherOrg, err := client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + Name: "default-test", + }) + + // This other user is not in the first user's org. Since other is an admin, they can + // still see the "first" user's workspace. + other := coderdtest.CreateAnotherUser(t, client, otherOrg.ID, rbac.RoleAdmin(), rbac.RoleMember()) + otherWorkspaces, err := other.Workspaces(context.Background(), codersdk.WorkspaceFilter{}) + require.NoError(t, err, "(other) fetch workspaces") + + firstWorkspaces, err := other.Workspaces(context.Background(), codersdk.WorkspaceFilter{}) + require.NoError(t, err, "(first) fetch workspaces") + + require.ElementsMatch(t, otherWorkspaces, firstWorkspaces) + } func TestPostWorkspacesByOrganization(t *testing.T) { From 18eb91bb9348ce57d269a2a89c85d7d601309146 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 17:25:42 -0500 Subject: [PATCH 7/8] Add require --- coderd/workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 05e3229487028..6ff3bf6fda5ca 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -35,6 +35,7 @@ func TestAdminViewAllWorkspaces(t *testing.T) { otherOrg, err := client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ Name: "default-test", }) + require.NoError(t, err, "create other org") // This other user is not in the first user's org. Since other is an admin, they can // still see the "first" user's workspace. @@ -46,7 +47,6 @@ func TestAdminViewAllWorkspaces(t *testing.T) { require.NoError(t, err, "(first) fetch workspaces") require.ElementsMatch(t, otherWorkspaces, firstWorkspaces) - } func TestPostWorkspacesByOrganization(t *testing.T) { From b37aead4257863cecd901ec47de446d2cbbeef46 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 17:36:21 -0500 Subject: [PATCH 8/8] Wait for workspace job to be completed --- coderd/workspaces_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 6ff3bf6fda5ca..a5f118c76542d 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -29,6 +29,7 @@ func TestAdminViewAllWorkspaces(t *testing.T) { coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) _, err := client.Workspace(context.Background(), workspace.ID) require.NoError(t, err)