synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-06 15:41:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Remove unused workspace routes in favor of list with filter (#2038)

Browse Source 
* fix: Remove unused workspace routes in favor of list with filter

This consolidates the workspace routes into a single place.
It allows users to fetch a workspace by their username and
workspace name, which will be used by the frontend for routing.

* Fix RBAC

* Fix CLI usages
This commit is contained in:
Kyle Carberry
2022-06-03 14:36:08 -05:00
committed by GitHub
parent d8c440188e
commit 43f622a52d
26 changed files with 72 additions and 424 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
coderd/coderd.go
View File

@ -152,15 +152,7 @@ func New(options *Options) *API {
r.Get("/", api.templatesByOrganization)
r.Get("/{templatename}", api.templateByOrganizationAndName)
})
r.Route("/workspaces", func(r chi.Router) {
r.Post("/", api.postWorkspacesByOrganization)
r.Get("/", api.workspacesByOrganization)
r.Route("/{user}", func(r chi.Router) {
r.Use(httpmw.ExtractUserParam(options.Database))
r.Get("/{workspacename}", api.workspaceByOwnerAndName)
r.Get("/", api.workspacesByOwner)
})
})
r.Post("/workspaces", api.postWorkspacesByOrganization)
r.Route("/members", func(r chi.Router) {
r.Get("/roles", api.assignableOrgRoles)
r.Route("/{user}", func(r chi.Router) {
@ -259,6 +251,7 @@ func New(options *Options) *API {
r.Get("/", api.organizationsByUser)
r.Get("/{organizationname}", api.organizationByUserAndName)
})
r.Get("/workspace/{workspacename}", api.workspaceByOwnerAndName)
r.Get("/gitsshkey", api.gitSSHKey)
r.Put("/gitsshkey", api.regenerateGitSSHKey)
})

16
coderd/coderd_test.go
View File

@ -147,22 +147,16 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {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)},
"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/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
"GET:/api/v2/users/{user}/workspace/{workspacename}": {
AssertObject: rbac.ResourceWorkspace,
AssertAction: rbac.ActionRead,
},
"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,

67
coderd/workspaces.go
View File

@ -100,35 +100,6 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(rw, http.StatusOK, convertWorkspace(workspace, build, job, template, owner))
}
func (api *API) workspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
organization := httpmw.OrganizationParam(r)
workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{
OrganizationID: organization.ID,
Deleted: false,
})
if errors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("get workspaces: %s", err),
})
return
}
// Rbac filter
workspaces = AuthorizeFilter(api, r, rbac.ActionRead, workspaces)
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("convert workspaces: %s", err),
})
return
}
httpapi.Write(rw, http.StatusOK, apiWorkspaces)
}
// workspaces returns all workspaces a user can read.
// Optional filters with query params
func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
@ -189,38 +160,8 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(rw, http.StatusOK, apiWorkspaces)
}
func (api *API) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
owner := httpmw.UserParam(r)
workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{
OwnerID: owner.ID,
Deleted: false,
})
if errors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("get workspaces: %s", err),
})
return
}
// Only return workspaces the user can read
workspaces = AuthorizeFilter(api, r, rbac.ActionRead, workspaces)
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("convert workspaces: %s", err),
})
return
}
httpapi.Write(rw, http.StatusOK, apiWorkspaces)
}
func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) {
owner := httpmw.UserParam(r)
organization := httpmw.OrganizationParam(r)
workspaceName := chi.URLParam(r, "workspacename")
workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
@ -238,14 +179,6 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
})
return
}
if workspace.OrganizationID != organization.ID {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("workspace is not owned by organization %q", organization.Name),
})
return
}
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
return
}

102
coderd/workspaces_test.go
View File

@ -210,107 +210,15 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
})
}
func TestWorkspacesByOrganization(t *testing.T) {
t.Parallel()
t.Run("ListEmpty", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
_, err := client.WorkspacesByOrganization(context.Background(), user.OrganizationID)
require.NoError(t, err)
})
t.Run("List", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, ws.LatestBuild.ID)
workspaces, err := client.WorkspacesByOrganization(context.Background(), user.OrganizationID)
require.NoError(t, err)
require.Len(t, workspaces, 1)
})
}
func TestWorkspacesByOwner(t *testing.T) {
t.Parallel()
t.Run("ListEmpty", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
_, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, codersdk.Me)
require.NoError(t, err)
})
t.Run("ListMine", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user := coderdtest.CreateFirstUser(t, client)
me, err := client.User(context.Background(), codersdk.Me)
require.NoError(t, err)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
_ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
// Create noise workspace that should be filtered out
other := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
_ = coderdtest.CreateWorkspace(t, other, user.OrganizationID, template.ID)
// Use a username
workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
OrganizationID: user.OrganizationID,
Owner: me.Username,
})
require.NoError(t, err)
require.Len(t, workspaces, 1)
})
t.Run("ListName", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
w := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
// Create noise workspace that should be filtered out
_ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
// Use name filter
workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
Name: w.Name,
})
require.NoError(t, err)
require.Len(t, workspaces, 1)
// Create same name workspace that should be included
other := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
_ = coderdtest.CreateWorkspace(t, other, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.Name = w.Name })
workspaces, err = client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
Name: w.Name,
})
require.NoError(t, err)
require.Len(t, workspaces, 2)
})
}
func TestWorkspaceByOwnerAndName(t *testing.T) {
t.Parallel()
t.Run("NotFound", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
_, err := client.WorkspaceByOwnerAndName(context.Background(), user.OrganizationID, codersdk.Me, "something")
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something")
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
})
t.Run("Get", func(t *testing.T) {
t.Parallel()
@ -320,7 +228,7 @@ func TestWorkspaceByOwnerAndName(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)
_, err := client.WorkspaceByOwnerAndName(context.Background(), user.OrganizationID, codersdk.Me, workspace.Name)
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name)
require.NoError(t, err)
})
}
@ -440,7 +348,9 @@ func TestPostWorkspaceBuild(t *testing.T) {
require.Equal(t, workspace.LatestBuild.BuildNumber+1, build.BuildNumber)
coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspaces, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, user.UserID.String())
workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
Owner: user.UserID.String(),
})
require.NoError(t, err)
require.Len(t, workspaces, 0)
})