Skip to content

Commit 186eb5f

Browse files
EmyrkEmyrk
committed
Fix 401 -> 403
Fix comments
1 parent d123b9f commit 186eb5f

File tree

7 files changed

+24
-16
lines changed

7 files changed

+24
-16
lines changed

‎coderd/authorize.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func (api *api) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
1515
roles := httpmw.UserRoles(r)
1616
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object)
1717
if err != nil {
18-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
18+
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
1919
Message: err.Error(),
2020
})
2121

‎coderd/httpapi/httpapi.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ type Error struct {
6262
Detail string `json:"detail" validate:"required"`
6363
}
6464

65+
func Forbidden(rw http.ResponseWriter) {
66+
Write(rw, http.StatusForbidden, Response{
67+
Message: "forbidden",
68+
})
69+
}
70+
6571
// Write outputs a standardized format to an HTTP response body.
6672
func Write(rw http.ResponseWriter, status int, response interface{}) {
6773
buf := &bytes.Buffer{}

‎coderd/organizations.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
428428
}
429429

430430
if workspace.OrganizationID != organization.ID {
431-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
431+
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
432432
Message: fmt.Sprintf("workspace is not owned by organization %q", organization.Name),
433433
})
434434
return
@@ -493,7 +493,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
493493
}
494494

495495
if organization.ID != template.OrganizationID {
496-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
496+
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
497497
Message: fmt.Sprintf("template is not in organization %q", organization.Name),
498498
})
499499
return
@@ -503,7 +503,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
503503
UserID: apiKey.UserID,
504504
})
505505
if errors.Is(err, sql.ErrNoRows) {
506-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
506+
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
507507
Message: "you aren't allowed to access templates in that organization",
508508
})
509509
return

‎coderd/rbac/error.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const (
66
// errUnauthorized is the error message that should be returned to
77
// clients when an action is forbidden. It is intentionally vague to prevent
88
// disclosing information that a client should not have access to.
9-
errUnauthorized = "unauthorized"
9+
errUnauthorized = "forbidden"
1010
)
1111

1212
// UnauthorizedError is the error type for authorization errors

‎coderd/rbac/object.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var (
2525
Type: "file",
2626
}
2727

28-
// ResourceOrganization CRUD. Always has an org owner.
28+
// ResourceOrganization CRUD. Has an org owner on all but 'create'.
2929
// create/delete = make or delete organizations
3030
// read = view org information (Can add user owner for read)
3131
// update = ??
@@ -56,8 +56,8 @@ var (
5656
// ResourceUser is the user in the 'users' table.
5757
// ResourceUser never has any owners or in an org, as it's site wide.
5858
// create/delete = make or delete a new user.
59-
// read = view all user's settings
60-
// update = update all user field & settings
59+
// read = view all 'user' table data
60+
// update = update all 'user' table data
6161
ResourceUser = Object{
6262
Type: "user",
6363
}

‎coderd/roles.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,16 @@ func (api *api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
3838
}
3939

4040
func (api *api) checkPermissions(rw http.ResponseWriter, r *http.Request) {
41-
roles := httpmw.UserRoles(r)
4241
user := httpmw.UserParam(r)
4342

44-
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
43+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithOwner(user.ID.String())) {
44+
return
45+
}
46+
47+
// use the roles of the user specified, not the person making the request.
48+
roles, err := api.Database.GetAllUserRoles(r.Context(), user.ID)
49+
if err != nil {
50+
httpapi.Forbidden(rw)
4551
return
4652
}
4753

‎coderd/users.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -538,15 +538,11 @@ func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
538538
if errors.Is(err, sql.ErrNoRows) {
539539
// Return unauthorized rather than a 404 to not leak if the organization
540540
// exists.
541-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
542-
Message: "unauthorized",
543-
})
541+
httpapi.Forbidden(rw)
544542
return
545543
}
546544
if err != nil {
547-
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
548-
Message: fmt.Sprintf("get organization by name: %s", err),
549-
})
545+
httpapi.Forbidden(rw)
550546
return
551547
}
552548

0 commit comments

Comments
 (0)