Skip to content

Commit 1047391

Browse files
EmyrkEmyrk
committed
Fix some routes
1 parent ed9be78 commit 1047391

File tree

2 files changed

+46
-7
lines changed

2 files changed

+46
-7
lines changed

‎coderd/coderd_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
4747
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
4848
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
4949
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
50-
coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID)
50+
workspace := coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID)
5151

5252
// Always fail auth from this point forward
5353
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
@@ -139,6 +139,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
139139
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
140140
"GET:/api/v2/users/{user}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
141141
"GET:/api/v2/organizations/{organization}/workspaces/{user}": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
142+
"GET:/api/v2/organizations/{organization}/workspaces/{user}/{workspace}": {
143+
AssertObject: rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()),
144+
},
145+
"GET:/api/v2/organizations/{organization}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
142146
}
143147

144148
c, _ := srv.Config.Handler.(*chi.Mux)
@@ -152,13 +156,14 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
152156
routeAssertions = routeCheck{}
153157
}
154158
if routeAssertions.StatusCode == 0 {
155-
routeAssertions.StatusCode = http.StatusUnauthorized
159+
routeAssertions.StatusCode = http.StatusForbidden
156160
}
157161

158162
// Replace all url params with known values
159163
route = strings.ReplaceAll(route, "{organization}", admin.OrganizationID.String())
160164
route = strings.ReplaceAll(route, "{user}", admin.UserID.String())
161165
route = strings.ReplaceAll(route, "{organizationname}", organization.Name)
166+
route = strings.ReplaceAll(route, "{workspace}", workspace.Name)
162167

163168
resp, err := client.Request(context.Background(), method, route, nil)
164169
require.NoError(t, err, "do req")

‎coderd/workspaces.go

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,18 @@ func (api *api) workspace(rw http.ResponseWriter, r *http.Request) {
5858
return
5959
}
6060

61+
if !api.Authorize(rw, r, rbac.ActionRead,
62+
rbac.ResourceWorkspace.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
63+
return
64+
}
65+
6166
httpapi.Write(rw, http.StatusOK,
6267
convertWorkspace(workspace, convertWorkspaceBuild(build, convertProvisionerJob(job)), template, owner))
6368
}
6469

6570
func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
6671
organization := httpmw.OrganizationParam(r)
72+
roles := httpmw.UserRoles(r)
6773
workspaces, err := api.Database.GetWorkspacesByOrganizationID(r.Context(), database.GetWorkspacesByOrganizationIDParams{
6874
OrganizationID: organization.ID,
6975
Deleted: false,
@@ -77,7 +83,18 @@ func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request
7783
})
7884
return
7985
}
80-
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
86+
87+
allowedWorkspaces := make([]database.Workspace, 0)
88+
for _, ws := range workspaces {
89+
ws := ws
90+
err = api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
91+
rbac.ResourceWorkspace.InOrg(ws.OrganizationID).WithOwner(ws.OwnerID.String()).WithID(ws.ID.String()))
92+
if err == nil {
93+
allowedWorkspaces = append(allowedWorkspaces, ws)
94+
}
95+
}
96+
97+
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, allowedWorkspaces)
8198
if err != nil {
8299
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
83100
Message: fmt.Sprintf("convert workspaces: %s", err),
@@ -102,6 +119,7 @@ func (api *api) workspacesByUser(rw http.ResponseWriter, r *http.Request) {
102119
return
103120
}
104121
for _, ws := range userWorkspaces {
122+
ws := ws
105123
err = api.Authorizer.ByRoleName(r.Context(), user.ID.String(), roles.Roles, rbac.ActionRead,
106124
rbac.ResourceWorkspace.InOrg(ws.OrganizationID).WithOwner(ws.OwnerID.String()).WithID(ws.ID.String()))
107125
if err == nil {
@@ -121,6 +139,7 @@ func (api *api) workspacesByUser(rw http.ResponseWriter, r *http.Request) {
121139

122140
func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
123141
owner := httpmw.UserParam(r)
142+
roles := httpmw.UserRoles(r)
124143
workspaces, err := api.Database.GetWorkspacesByOwnerID(r.Context(), database.GetWorkspacesByOwnerIDParams{
125144
OwnerID: owner.ID,
126145
})
@@ -133,7 +152,18 @@ func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
133152
})
134153
return
135154
}
136-
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
155+
156+
allowedWorkspaces := make([]database.Workspace, 0)
157+
for _, ws := range workspaces {
158+
ws := ws
159+
err = api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
160+
rbac.ResourceWorkspace.InOrg(ws.OrganizationID).WithOwner(ws.OwnerID.String()).WithID(ws.ID.String()))
161+
if err == nil {
162+
allowedWorkspaces = append(allowedWorkspaces, ws)
163+
}
164+
}
165+
166+
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, allowedWorkspaces)
137167
if err != nil {
138168
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
139169
Message: fmt.Sprintf("convert workspaces: %s", err),
@@ -153,9 +183,8 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
153183
Name: workspaceName,
154184
})
155185
if errors.Is(err, sql.ErrNoRows) {
156-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
157-
Message: fmt.Sprintf("no workspace found by name %q", workspaceName),
158-
})
186+
// Do not leak information if the workspace exists or not
187+
httpapi.Forbidden(rw)
159188
return
160189
}
161190
if err != nil {
@@ -172,6 +201,11 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
172201
return
173202
}
174203

204+
if !api.Authorize(rw, r, rbac.ActionRead,
205+
rbac.ResourceWorkspace.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
206+
return
207+
}
208+
175209
build, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID)
176210
if err != nil {
177211
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

0 commit comments

Comments
 (0)