Skip to content

Commit ea84bc6

Browse files
EmyrkEmyrk
committed
Fix authorize calls for group endpoints
1 parent 8cf12e9 commit ea84bc6

File tree

2 files changed

+36
-23
lines changed

2 files changed

+36
-23
lines changed

‎coderd/coderdtest/authorize.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
2727
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
2828
workspaceExecObj := rbac.ResourceWorkspaceExecution.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
2929
applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
30+
groupObj := rbac.ResourceGroup.InOrg(a.Organization.ID)
3031

3132
// skipRoutes allows skipping routes from being checked.
3233
skipRoutes := map[string]string{
@@ -243,16 +244,29 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
243244
"GET:/api/v2/users": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceUser},
244245
"GET:/api/v2/applications/auth-redirect": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceAPIKey},
245246

247+
"DELETE:/api/v2/groups/{group}": {
248+
AssertAction: rbac.ActionDelete,
249+
AssertObject: groupObj,
250+
},
251+
"PATCH:/api/v2/groups/{group}": {
252+
AssertAction: rbac.ActionUpdate,
253+
AssertObject: groupObj,
254+
},
255+
"GET:/api/v2/groups/{group}": {
256+
AssertAction: rbac.ActionRead,
257+
AssertObject: groupObj,
258+
},
259+
"GET:/api/v2/organizations/{organization}/groups/": {
260+
StatusCode: http.StatusOK,
261+
AssertAction: rbac.ActionRead,
262+
AssertObject: groupObj,
263+
},
264+
246265
// These endpoints need payloads to get to the auth part. Payloads will be required
247266
"PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
248267
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
249268
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
250269
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
251-
252-
// TODO: @emyrk @jonayers Fix this unit test by using a valid group
253-
"DELETE:/api/v2/groups/{group}": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
254-
"PATCH:/api/v2/groups/{group}": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
255-
"GET:/api/v2/groups/{group}": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
256270
}
257271

258272
// Routes like proxy routes support all HTTP methods. A helper func to expand
@@ -360,6 +374,10 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a
360374
ParameterValues: []codersdk.CreateParameterRequest{},
361375
})
362376
require.NoError(t, err, "template version dry-run")
377+
group, err := client.CreateGroup(ctx, admin.OrganizationID, codersdk.CreateGroupRequest{
378+
Name: "testgroup",
379+
})
380+
require.NoError(t, err, "create group")
363381

364382
templateParam, err := client.CreateParameter(ctx, codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
365383
Name: "test-param",
@@ -385,6 +403,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a
385403
"{jobID}": templateVersionDryRun.ID.String(),
386404
"{templatename}": template.Name,
387405
"{workspace_and_agent}": workspace.Name + "." + workspaceResources[0].Agents[0].Name,
406+
"{group}": group.ID.String(),
388407
// Only checking template scoped params here
389408
"parameters/{scope}/{id}": fmt.Sprintf("parameters/%s/%s",
390409
string(templateParam.Scope), templateParam.ScopeID.String()),

‎coderd/groups.go

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
5656
group = httpmw.GroupParam(r)
5757
)
5858

59-
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceGroup) {
59+
if !api.Authorize(r, rbac.ActionUpdate, group) {
6060
http.NotFound(rw, r)
6161
return
6262
}
@@ -162,7 +162,7 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) {
162162
group = httpmw.GroupParam(r)
163163
)
164164

165-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceGroup) {
165+
if !api.Authorize(r, rbac.ActionDelete, group) {
166166
httpapi.ResourceNotFound(rw)
167167
return
168168
}
@@ -184,7 +184,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) {
184184
group = httpmw.GroupParam(r)
185185
)
186186

187-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceGroup) {
187+
if !api.Authorize(r, rbac.ActionRead, group) {
188188
httpapi.ResourceNotFound(rw)
189189
return
190190
}
@@ -204,27 +204,21 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) {
204204
org = httpmw.OrganizationParam(r)
205205
)
206206

207-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceGroup) {
208-
httpapi.ResourceNotFound(rw)
209-
return
210-
}
211-
212207
groups, err := api.Database.GetGroupsByOrganizationID(ctx, org.ID)
213208
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
214209
httpapi.InternalServerError(rw, err)
215210
return
216211
}
217212

218-
// Filter templates based on rbac permissions
219-
// TODO: authorize filters.
220-
// groups, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, groups)
221-
// if err != nil {
222-
// httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
223-
// Message: "Internal error fetching templates.",
224-
// Detail: err.Error(),
225-
// })
226-
// return
227-
// }
213+
// Filter groups based on rbac permissions
214+
groups, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, groups)
215+
if err != nil {
216+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
217+
Message: "Internal error fetching groups.",
218+
Detail: err.Error(),
219+
})
220+
return
221+
}
228222

229223
resp := make([]codersdk.Group, 0, len(groups))
230224
for _, group := range groups {

0 commit comments

Comments
 (0)