Skip to content

Commit 6ef1647

Browse files
EmyrkEmyrk
committed
chore: Manually forbidden after Authorize
1 parent c7f976e commit 6ef1647

21 files changed

+171
-114
lines changed

‎coderd/authorize.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"cdr.dev/slog"
99

10-
"github.com/coder/coder/coderd/httpapi"
1110
"github.com/coder/coder/coderd/httpmw"
1211
"github.com/coder/coder/coderd/rbac"
1312
)
@@ -17,12 +16,17 @@ func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Act
1716
return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
1817
}
1918

20-
func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.Action, object rbac.Objecter) bool {
19+
// Authorize will return false if the user is not authorized to do the action.
20+
// This function will log appropriately, but the caller must return an
21+
// error to the api client.
22+
// Eg:
23+
// if !api.Authorize(...) {
24+
// httpapi.Forbidden(rw)
25+
// }
26+
func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
2127
roles := httpmw.AuthorizationUserRoles(r)
2228
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
2329
if err != nil {
24-
httpapi.Forbidden(rw)
25-
2630
// Log the errors for debugging
2731
internalError := new(rbac.UnauthorizedError)
2832
logger := api.Logger

‎coderd/files.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
2222
apiKey := httpmw.APIKey(r)
2323
// This requires the site wide action to create files.
2424
// Once created, a user can read their own files uploaded
25-
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceFile) {
25+
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceFile) {
26+
httpapi.Forbidden(rw)
2627
return
2728
}
2829

@@ -86,9 +87,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
8687
}
8788
file, err := api.Database.GetFileByHash(r.Context(), hash)
8889
if errors.Is(err, sql.ErrNoRows) {
89-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
90-
Message: fmt.Sprintf("File %q not found.", hash),
91-
})
90+
httpapi.ResourceNotFound(rw, fmt.Sprintf("File %s", hash))
9291
return
9392
}
9493
if err != nil {
@@ -99,8 +98,10 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
9998
return
10099
}
101100

102-
if !api.Authorize(rw, r, rbac.ActionRead,
101+
if !api.Authorize(r, rbac.ActionRead,
103102
rbac.ResourceFile.WithOwner(file.CreatedBy.String()).WithID(file.Hash)) {
103+
// Return 404 to not leak the file exists
104+
httpapi.ResourceNotFound(rw, fmt.Sprintf("File %s", hash))
104105
return
105106
}
106107

‎coderd/gitsshkey.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import (
1414
func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
1515
user := httpmw.UserParam(r)
1616

17-
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
17+
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
18+
httpapi.Forbidden(rw)
1819
return
1920
}
2021

@@ -62,7 +63,8 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
6263
func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
6364
user := httpmw.UserParam(r)
6465

65-
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
66+
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
67+
httpapi.Forbidden(rw)
6668
return
6769
}
6870

‎coderd/httpapi/httpapi.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ type Error struct {
7676
Detail string `json:"detail" validate:"required"`
7777
}
7878

79+
func ResourceNotFound(rw http.ResponseWriter, resource string) {
80+
Write(rw, http.StatusNotFound, Response{
81+
Message: fmt.Sprintf("%s does not exist.", resource),
82+
})
83+
}
84+
7985
func Forbidden(rw http.ResponseWriter) {
8086
Write(rw, http.StatusForbidden, Response{
8187
Message: "Forbidden.",

‎coderd/httpmw/organizationparam.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
4545

4646
organization, err := db.GetOrganizationByID(r.Context(), orgID)
4747
if errors.Is(err, sql.ErrNoRows) {
48-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
49-
Message: fmt.Sprintf("Organization %q does not exist.", orgID),
50-
})
48+
httpapi.ResourceNotFound(rw, fmt.Sprintf("Organization %q", orgID))
5149
return
5250
}
5351
if err != nil {

‎coderd/httpmw/templateparam.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
4747
}
4848

4949
if template.Deleted {
50-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
51-
Message: fmt.Sprintf("Template %q does not exist.", templateID),
52-
})
50+
httpapi.ResourceNotFound(rw, fmt.Sprintf("Template %s", templateID))
5351
return
5452
}
5553

‎coderd/httpmw/templateversionparam.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ func ExtractTemplateVersionParam(db database.Store) func(http.Handler) http.Hand
3434
}
3535
templateVersion, err := db.GetTemplateVersionByID(r.Context(), templateVersionID)
3636
if errors.Is(err, sql.ErrNoRows) {
37-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
38-
Message: fmt.Sprintf("Template version %q does not exist.", templateVersionID),
39-
})
37+
httpapi.ResourceNotFound(rw, fmt.Sprintf("Template version %s", templateVersionID))
4038
return
4139
}
4240
if err != nil {

‎coderd/httpmw/workspacebuildparam.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ func ExtractWorkspaceBuildParam(db database.Store) func(http.Handler) http.Handl
3434
}
3535
workspaceBuild, err := db.GetWorkspaceBuildByID(r.Context(), workspaceBuildID)
3636
if errors.Is(err, sql.ErrNoRows) {
37-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
38-
Message: fmt.Sprintf("Workspace build %q does not exist.", workspaceBuildID),
39-
})
37+
httpapi.ResourceNotFound(rw, fmt.Sprintf("Workspace build %q", workspaceBuildID))
4038
return
4139
}
4240
if err != nil {

‎coderd/httpmw/workspaceparam.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ func ExtractWorkspaceParam(db database.Store) func(http.Handler) http.Handler {
3232
}
3333
workspace, err := db.GetWorkspaceByID(r.Context(), workspaceID)
3434
if errors.Is(err, sql.ErrNoRows) {
35-
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
36-
Message: fmt.Sprintf("Workspace %q does not exist.", workspaceID),
37-
})
35+
httpapi.ResourceNotFound(rw, fmt.Sprintf("Workspace %s", workspaceID))
3836
return
3937
}
4038
if err != nil {

‎coderd/members.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
3939
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
4040
for _, roleName := range added {
4141
// Assigning a role requires the create permission.
42-
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
42+
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
43+
httpapi.Forbidden(rw)
4344
return
4445
}
4546
}
4647
for _, roleName := range removed {
4748
// Removing a role requires the delete permission.
48-
if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
49+
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
50+
httpapi.Forbidden(rw)
4951
return
5052
}
5153
}

0 commit comments

Comments
 (0)