selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

feat: Return more 404s vs 403s (#2194) 

* feat: Return more 404s vs 403s
* Return vague 404 in all cases
This commit is contained in:
Steven Masley 2022-06-14 10:14:05 -05:00 committed by GitHub
parent dc1de58857
commit 251316751e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 231 additions and 155 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cli/autostart_test.go
View File

@ -107,7 +107,7 @@ func TestAutostart(t *testing.T) {
clitest.SetupConfig(t, client, root)
err := cmd.Execute()
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
require.ErrorContains(t, err, "status code 404", "unexpected error")
})
t.Run("unset_NotFound", func(t *testing.T) {
@ -124,7 +124,7 @@ func TestAutostart(t *testing.T) {
clitest.SetupConfig(t, client, root)
err := cmd.Execute()
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
require.ErrorContains(t, err, "status code 404:", "unexpected error")
})
}

4
cli/ttl_test.go
View File

@ -178,7 +178,7 @@ func TestTTL(t *testing.T) {
clitest.SetupConfig(t, client, root)
err := cmd.Execute()
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
require.ErrorContains(t, err, "status code 404:", "unexpected error")
})
t.Run("Unset_NotFound", func(t *testing.T) {
@ -195,7 +195,7 @@ func TestTTL(t *testing.T) {
clitest.SetupConfig(t, client, root)
err := cmd.Execute()
require.ErrorContains(t, err, "status code 403: Forbidden", "unexpected error")
require.ErrorContains(t, err, "status code 404:", "unexpected error")
})
t.Run("TemplateMaxTTL", func(t *testing.T) {

13
coderd/authorize.go
View File

@ -7,7 +7,6 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/rbac"
)
@ -17,12 +16,18 @@ func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Act
return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
}
func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.Action, object rbac.Objecter) bool {
// Authorize will return false if the user is not authorized to do the action.
// This function will log appropriately, but the caller must return an
// error to the api client.
// Eg:
// if !api.Authorize(...) {
// httpapi.Forbidden(rw)
// return
// }
func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
roles := httpmw.AuthorizationUserRoles(r)
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
if err != nil {
httpapi.Forbidden(rw)
// Log the errors for debugging
internalError := new(rbac.UnauthorizedError)
logger := api.Logger

12
coderd/coderd_test.go
View File

@ -380,9 +380,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
// By default, all omitted routes check for just "authorize" called
routeAssertions = routeCheck{}
}
if routeAssertions.StatusCode == 0 {
routeAssertions.StatusCode = http.StatusForbidden
}
// Replace all url params with known values
route = strings.ReplaceAll(route, "{organization}", admin.OrganizationID.String())
@ -413,7 +410,14 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
if !routeAssertions.NoAuthorize {
assert.NotNil(t, authorizer.Called, "authorizer expected")
assert.Equal(t, routeAssertions.StatusCode, resp.StatusCode, "expect unauthorized")
if routeAssertions.StatusCode != 0 {
assert.Equal(t, routeAssertions.StatusCode, resp.StatusCode, "expect unauthorized")
} else {
// It's either a 404 or 403.
if resp.StatusCode != http.StatusNotFound {
assert.Equal(t, http.StatusForbidden, resp.StatusCode, "expect unauthorized")
}
}
if authorizer.Called != nil {
if routeAssertions.AssertAction != "" {
assert.Equal(t, routeAssertions.AssertAction, authorizer.Called.Action, "resource action")

9
coderd/files.go
View File

@ -22,7 +22,8 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
apiKey := httpmw.APIKey(r)
// This requires the site wide action to create files.
// Once created, a user can read their own files uploaded
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceFile) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceFile) {
httpapi.Forbidden(rw)
return
}
@ -86,7 +87,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
}
file, err := api.Database.GetFileByHash(r.Context(), hash)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Forbidden(rw)
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -97,8 +98,10 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(rw, r, rbac.ActionRead,
if !api.Authorize(r, rbac.ActionRead,
rbac.ResourceFile.WithOwner(file.CreatedBy.String()).WithID(file.Hash)) {
// Return 404 to not leak the file exists
httpapi.ResourceNotFound(rw)
return
}

2
coderd/files_test.go
View File

@ -50,7 +50,7 @@ func TestDownload(t *testing.T) {
_, _, err := client.Download(context.Background(), "something")
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
})
t.Run("Insert", func(t *testing.T) {

6
coderd/gitsshkey.go
View File

@ -14,7 +14,8 @@ import (
func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -62,7 +63,8 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}

8
coderd/httpapi/httpapi.go
View File

@ -76,6 +76,14 @@ type Error struct {
Detail string `json:"detail" validate:"required"`
}
// ResourceNotFound is intentionally vague. All 404 responses should be identical
// to prevent leaking existence of resources.
func ResourceNotFound(rw http.ResponseWriter) {
Write(rw, http.StatusNotFound, Response{
Message: "Resource not found or you do not have access to this resource",
})
}
func Forbidden(rw http.ResponseWriter) {
Write(rw, http.StatusForbidden, Response{
Message: "Forbidden.",

9
coderd/httpmw/organizationparam.go
View File

@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"github.com/coder/coder/coderd/database"
@ -45,9 +44,7 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
organization, err := db.GetOrganizationByID(r.Context(), orgID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Organization %q does not exist.", orgID),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -76,9 +73,7 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
UserID: user.ID,
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
Message: "Not a member of the organization.",
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {

2
coderd/httpmw/organizationparam_test.go
View File

@ -144,7 +144,7 @@ func TestOrganizationParam(t *testing.T) {
rtr.ServeHTTP(rw, r)
res := rw.Result()
defer res.Body.Close()
require.Equal(t, http.StatusForbidden, res.StatusCode)
require.Equal(t, http.StatusNotFound, res.StatusCode)
})
t.Run("Success", func(t *testing.T) {

14
coderd/httpmw/templateparam.go
View File

@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"github.com/go-chi/chi/v5"
@ -33,10 +32,8 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
return
}
template, err := db.GetTemplateByID(r.Context(), templateID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Template %q does not exist.", templateID),
})
if errors.Is(err, sql.ErrNoRows) || (err == nil && template.Deleted) {
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -47,13 +44,6 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
return
}
if template.Deleted {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Template %q does not exist.", templateID),
})
return
}
ctx := context.WithValue(r.Context(), templateParamContextKey{}, template)
chi.RouteContext(ctx).URLParams.Add("organization", template.OrganizationID.String())
next.ServeHTTP(rw, r.WithContext(ctx))

5
coderd/httpmw/templateversionparam.go
View File

@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"github.com/go-chi/chi/v5"
@ -34,9 +33,7 @@ func ExtractTemplateVersionParam(db database.Store) func(http.Handler) http.Hand
}
templateVersion, err := db.GetTemplateVersionByID(r.Context(), templateVersionID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Template version %q does not exist.", templateVersionID),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {

7
coderd/httpmw/userparam.go
View File

@ -2,8 +2,11 @@ package httpmw
import (
"context"
"database/sql"
"net/http"
"golang.org/x/xerrors"
"github.com/go-chi/chi/v5"
"github.com/google/uuid"
@ -47,6 +50,10 @@ func ExtractUserParam(db database.Store) func(http.Handler) http.Handler {
if userQuery == "me" {
user, err = db.GetUserByID(r.Context(), APIKey(r).UserID)
if xerrors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching user.",

5
coderd/httpmw/workspacebuildparam.go
View File

@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"github.com/go-chi/chi/v5"
@ -34,9 +33,7 @@ func ExtractWorkspaceBuildParam(db database.Store) func(http.Handler) http.Handl
}
workspaceBuild, err := db.GetWorkspaceBuildByID(r.Context(), workspaceBuildID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Workspace build %q does not exist.", workspaceBuildID),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {

5
coderd/httpmw/workspaceparam.go
View File

@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"github.com/coder/coder/coderd/database"
@ -32,9 +31,7 @@ func ExtractWorkspaceParam(db database.Store) func(http.Handler) http.Handler {
}
workspace, err := db.GetWorkspaceByID(r.Context(), workspaceID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Workspace %q does not exist.", workspaceID),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {

6
coderd/members.go
View File

@ -39,13 +39,15 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
for _, roleName := range added {
// Assigning a role requires the create permission.
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}
}
for _, roleName := range removed {
// Removing a role requires the delete permission.
if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}
}

7
coderd/organizations.go
View File

@ -19,9 +19,10 @@ import (
func (api *API) organization(rw http.ResponseWriter, r *http.Request) {
organization := httpmw.OrganizationParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceOrganization.
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrganization.
InOrg(organization.ID).
WithID(organization.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -32,8 +33,8 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
apiKey := httpmw.APIKey(r)
// Create organization uses the organization resource without an OrgID.
// This means you need the site wide permission to make a new organization.
if !api.Authorize(rw, r, rbac.ActionCreate,
rbac.ResourceOrganization) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrganization) {
httpapi.Forbidden(rw)
return
}

4
coderd/organizations_test.go
View File

@ -30,7 +30,7 @@ func TestOrganizationByUserAndName(t *testing.T) {
_, err := client.OrganizationByName(context.Background(), codersdk.Me, "nothing")
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
})
t.Run("NoMember", func(t *testing.T) {
@ -45,7 +45,7 @@ func TestOrganizationByUserAndName(t *testing.T) {
_, err = other.OrganizationByName(context.Background(), codersdk.Me, org.Name)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
})
t.Run("Valid", func(t *testing.T) {

20
coderd/parameters.go
View File

@ -26,7 +26,8 @@ func (api *API) postParameter(rw http.ResponseWriter, r *http.Request) {
if !ok {
return
}
if !api.Authorize(rw, r, rbac.ActionUpdate, obj) {
if !api.Authorize(r, rbac.ActionUpdate, obj) {
httpapi.ResourceNotFound(rw)
return
}
@ -85,7 +86,8 @@ func (api *API) parameters(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(rw, r, rbac.ActionRead, obj) {
if !api.Authorize(r, rbac.ActionRead, obj) {
httpapi.ResourceNotFound(rw)
return
}
@ -120,8 +122,9 @@ func (api *API) deleteParameter(rw http.ResponseWriter, r *http.Request) {
if !ok {
return
}
// A delete param is still updating the underlying resource for the scope.
if !api.Authorize(rw, r, rbac.ActionUpdate, obj) {
// A deleted param is still updating the underlying resource for the scope.
if !api.Authorize(r, rbac.ActionUpdate, obj) {
httpapi.ResourceNotFound(rw)
return
}
@ -132,10 +135,7 @@ func (api *API) deleteParameter(rw http.ResponseWriter, r *http.Request) {
Name: name,
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("No parameter found at the provided scope with name %q.", name),
Detail: err.Error(),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -223,7 +223,9 @@ func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, s
// Write error payload to rw if we cannot find the resource for the scope
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
httpapi.Forbidden(rw)
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Scope %q resource %q not found.", scope, scopeID),
})
} else {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: err.Error(),

9
coderd/roles.go
View File

@ -16,7 +16,8 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
// role of the user.
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceRoleAssignment) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceRoleAssignment) {
httpapi.Forbidden(rw)
return
}
@ -30,7 +31,8 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
// role of the user.
organization := httpmw.OrganizationParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}
@ -41,7 +43,8 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}

24
coderd/templates.go
View File

@ -29,6 +29,11 @@ var (
func (api *API) template(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(r.Context(), []uuid.UUID{template.ID})
if errors.Is(err, sql.ErrNoRows) {
err = nil
@ -41,7 +46,8 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(rw, r, rbac.ActionRead, template) {
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -64,7 +70,8 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) {
func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(rw, r, rbac.ActionDelete, template) {
if !api.Authorize(r, rbac.ActionDelete, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -108,7 +115,8 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
var createTemplate codersdk.CreateTemplateRequest
organization := httpmw.OrganizationParam(r)
apiKey := httpmw.APIKey(r)
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) {
httpapi.ResourceNotFound(rw)
return
}
@ -296,9 +304,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
})
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("No template found by name %q in the %q organization.", templateName, organization.Name),
})
httpapi.ResourceNotFound(rw)
return
}
@ -309,7 +315,8 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
return
}
if !api.Authorize(rw, r, rbac.ActionRead, template) {
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -344,7 +351,8 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, template) {
if !api.Authorize(r, rbac.ActionUpdate, template) {
httpapi.ResourceNotFound(rw)
return
}

52
coderd/templateversions.go
View File

@ -22,7 +22,8 @@ import (
func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) {
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
@ -40,7 +41,8 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) {
func (api *API) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Request) {
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, templateVersion) {
if !api.Authorize(r, rbac.ActionUpdate, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
@ -85,7 +87,8 @@ func (api *API) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Reque
func (api *API) templateVersionSchema(rw http.ResponseWriter, r *http.Request) {
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
@ -132,7 +135,8 @@ func (api *API) templateVersionSchema(rw http.ResponseWriter, r *http.Request) {
func (api *API) templateVersionParameters(rw http.ResponseWriter, r *http.Request) {
apiKey := httpmw.APIKey(r)
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
@ -175,13 +179,15 @@ func (api *API) templateVersionParameters(rw http.ResponseWriter, r *http.Reques
func (api *API) postTemplateVersionDryRun(rw http.ResponseWriter, r *http.Request) {
apiKey := httpmw.APIKey(r)
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
// We use the workspace RBAC check since we don't want to allow dry runs if
// the user can't create workspaces.
if !api.Authorize(rw, r, rbac.ActionCreate,
if !api.Authorize(r, rbac.ActionCreate,
rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(apiKey.UserID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -293,8 +299,9 @@ func (api *API) patchTemplateVersionDryRunCancel(rw http.ResponseWriter, r *http
if !ok {
return
}
if !api.Authorize(rw, r, rbac.ActionUpdate,
if !api.Authorize(r, rbac.ActionUpdate,
rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(job.InitiatorID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -336,7 +343,8 @@ func (api *API) fetchTemplateVersionDryRunJob(rw http.ResponseWriter, r *http.Re
templateVersion = httpmw.TemplateVersionParam(r)
jobID = chi.URLParam(r, "jobID")
)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return database.ProvisionerJob{}, false
}
@ -351,7 +359,9 @@ func (api *API) fetchTemplateVersionDryRunJob(rw http.ResponseWriter, r *http.Re
job, err := api.Database.GetProvisionerJobByID(r.Context(), jobUUID)
if xerrors.Is(err, sql.ErrNoRows) {
httpapi.Forbidden(rw)
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Provisioner job %q not found.", jobUUID),
})
return database.ProvisionerJob{}, false
}
if err != nil {
@ -366,8 +376,9 @@ func (api *API) fetchTemplateVersionDryRunJob(rw http.ResponseWriter, r *http.Re
return database.ProvisionerJob{}, false
}
// Do a workspace resource check since it's basically a workspace dry-run .
if !api.Authorize(rw, r, rbac.ActionRead,
if !api.Authorize(r, rbac.ActionRead,
rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(job.InitiatorID.String())) {
httpapi.Forbidden(rw)
return database.ProvisionerJob{}, false
}
@ -391,7 +402,8 @@ func (api *API) fetchTemplateVersionDryRunJob(rw http.ResponseWriter, r *http.Re
func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, template) {
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -478,7 +490,8 @@ func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque
func (api *API) templateVersionByName(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, template) {
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -517,7 +530,8 @@ func (api *API) templateVersionByName(rw http.ResponseWriter, r *http.Request) {
func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Request) {
template := httpmw.TemplateParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, template) {
if !api.Authorize(r, rbac.ActionUpdate, template) {
httpapi.ResourceNotFound(rw)
return
}
@ -603,11 +617,13 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
}
// Making a new template version is the same permission as creating a new template.
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) {
httpapi.ResourceNotFound(rw)
return
}
if !api.Authorize(rw, r, rbac.ActionRead, file) {
if !api.Authorize(r, rbac.ActionRead, file) {
httpapi.ResourceNotFound(rw)
return
}
@ -688,7 +704,8 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
// return agents associated with any particular workspace.
func (api *API) templateVersionResources(rw http.ResponseWriter, r *http.Request) {
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}
@ -709,7 +726,8 @@ func (api *API) templateVersionResources(rw http.ResponseWriter, r *http.Request
// Eg: Logs returned from 'terraform plan' when uploading a new terraform file.
func (api *API) templateVersionLogs(rw http.ResponseWriter, r *http.Request) {
templateVersion := httpmw.TemplateVersionParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, templateVersion) {
if !api.Authorize(r, rbac.ActionRead, templateVersion) {
httpapi.ResourceNotFound(rw)
return
}

54
coderd/users.go
View File

@ -135,7 +135,8 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
}
// Reading all users across the site.
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser) {
httpapi.Forbidden(rw)
return
}
@ -190,7 +191,8 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
// Creates a new user.
func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
// Create the user on the site.
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceUser) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceUser) {
httpapi.Forbidden(rw)
return
}
@ -200,8 +202,9 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
}
// Create the organization member in the org.
if !api.Authorize(rw, r, rbac.ActionCreate,
if !api.Authorize(r, rbac.ActionCreate,
rbac.ResourceOrganizationMember.InOrg(createUser.OrganizationID)) {
httpapi.ResourceNotFound(rw)
return
}
@ -258,7 +261,8 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
organizationIDs, err := userOrganizationIDs(r.Context(), api, user)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -276,7 +280,8 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) {
func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -343,7 +348,8 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW
user := httpmw.UserParam(r)
apiKey := httpmw.APIKey(r)
if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceUser.WithID(user.ID.String())) {
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -387,7 +393,8 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
params codersdk.UpdateUserPasswordRequest
)
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -411,7 +418,8 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
// admins can change passwords without sending old_password
if params.OldPassword == "" {
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.Forbidden(rw)
return
}
} else {
@ -464,8 +472,8 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
func (api *API) userRoles(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.
WithOwner(user.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -514,18 +522,25 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
// The member role is always implied.
impliedTypes := append(params.Roles, rbac.RoleMember())
added, removed := rbac.ChangeRoleSet(roles.Roles, impliedTypes)
for _, roleName := range added {
// Assigning a role requires the create permission.
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceRoleAssignment.WithID(roleName)) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceRoleAssignment.WithID(roleName)) {
httpapi.Forbidden(rw)
return
}
}
for _, roleName := range removed {
// Removing a role requires the delete permission.
if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceRoleAssignment.WithID(roleName)) {
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceRoleAssignment.WithID(roleName)) {
httpapi.Forbidden(rw)
return
}
}
@ -606,20 +621,22 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
organizationName := chi.URLParam(r, "organizationname")
organization, err := api.Database.GetOrganizationByName(r.Context(), organizationName)
if errors.Is(err, sql.ErrNoRows) {
// Return unauthorized rather than a 404 to not leak if the organization
// exists.
httpapi.Forbidden(rw)
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
httpapi.Forbidden(rw)
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching organization.",
Detail: err.Error(),
})
return
}
if !api.Authorize(rw, r, rbac.ActionRead,
if !api.Authorize(r, rbac.ActionRead,
rbac.ResourceOrganization.
InOrg(organization.ID).
WithID(organization.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -684,7 +701,8 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}

9
coderd/workspaceagents.go
View File

@ -33,7 +33,8 @@ import (
func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) {
workspaceAgent := httpmw.WorkspaceAgentParam(r)
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}
dbApps, err := api.Database.GetWorkspaceAppsByAgentID(r.Context(), workspaceAgent.ID)
@ -64,7 +65,8 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) {
workspaceAgent := httpmw.WorkspaceAgentParam(r)
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) {
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
httpapi.ResourceNotFound(rw)
return
}
apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency)
@ -379,7 +381,8 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
workspaceAgent := httpmw.WorkspaceAgentParam(r)
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) {
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
httpapi.ResourceNotFound(rw)
return
}
apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency)

7
coderd/workspaceapps.go
View File

@ -32,9 +32,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
Name: workspaceParts[0],
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: "Workspace not found.",
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -44,7 +42,8 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
})
return
}
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}

40
coderd/workspacebuilds.go
View File

@ -24,8 +24,9 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) {
workspaceBuild := httpmw.WorkspaceBuildParam(r)
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -55,8 +56,8 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) {
func (api *API) workspaceBuilds(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -178,9 +179,7 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
Name: workspaceName,
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("Workspace %q does not exist.", workspaceName),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -191,8 +190,8 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
return
}
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -239,20 +238,19 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
func (api *API) workspaceBuildByName(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
workspaceBuildName := chi.URLParam(r, "workspacebuildname")
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
workspaceBuildName := chi.URLParam(r, "workspacebuildname")
workspaceBuild, err := api.Database.GetWorkspaceBuildByWorkspaceIDAndName(r.Context(), database.GetWorkspaceBuildByWorkspaceIDAndNameParams{
WorkspaceID: workspace.ID,
Name: workspaceBuildName,
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
Message: fmt.Sprintf("No workspace build found by name %q.", workspaceBuildName),
})
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -305,8 +303,9 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
})
return
}
if !api.Authorize(rw, r, action, rbac.ResourceWorkspace.
if !api.Authorize(r, action, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -482,8 +481,9 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques
return
}
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace.
if !api.Authorize(r, rbac.ActionUpdate, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -536,8 +536,9 @@ func (api *API) workspaceBuildResources(rw http.ResponseWriter, r *http.Request)
return
}
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -562,8 +563,9 @@ func (api *API) workspaceBuildLogs(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -588,8 +590,8 @@ func (api *API) workspaceBuildState(rw http.ResponseWriter, r *http.Request) {
return
}
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}

2
coderd/workspacebuilds_test.go
View File

@ -92,7 +92,7 @@ func TestWorkspaceBuildByBuildNumber(t *testing.T) {
var apiError *codersdk.Error
require.ErrorAs(t, err, &apiError)
require.Equal(t, http.StatusNotFound, apiError.StatusCode())
require.ErrorContains(t, apiError, "Workspace \"workspaceName\" does not exist.")
require.ErrorContains(t, apiError, "Resource not found")
})
t.Run("WorkspaceBuildNotFound", func(t *testing.T) {

3
coderd/workspaceresources.go
View File

@ -18,7 +18,8 @@ func (api *API) workspaceResource(rw http.ResponseWriter, r *http.Request) {
workspaceBuild := httpmw.WorkspaceBuildParam(r)
workspaceResource := httpmw.WorkspaceResourceParam(r)
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}

26
coderd/workspaces.go
View File

@ -33,7 +33,8 @@ import (
func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -174,8 +175,7 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
})
}
if errors.Is(err, sql.ErrNoRows) {
// Do not leak information if the workspace exists or not
httpapi.Forbidden(rw)
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
@ -185,7 +185,8 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
})
return
}
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -230,8 +231,9 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
organization := httpmw.OrganizationParam(r)
apiKey := httpmw.APIKey(r)
if !api.Authorize(rw, r, rbac.ActionCreate,
if !api.Authorize(r, rbac.ActionCreate,
rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(apiKey.UserID.String())) {
httpapi.ResourceNotFound(rw)
return
}
@ -458,8 +460,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -501,8 +503,8 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) {
func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace.
InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -590,7 +592,8 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) {
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
httpapi.ResourceNotFound(rw)
return
}
@ -647,7 +650,8 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) {
func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
if !api.Authorize(r, rbac.ActionRead, workspace) {
httpapi.ResourceNotFound(rw)
return
}

7
coderd/workspaces_test.go
View File

@ -2,7 +2,6 @@ package coderd_test
import (
"context"
"fmt"
"net/http"
"strings"
"testing"
@ -296,7 +295,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
// Then:
// When we call without includes_deleted, we don't expect to get the workspace back
_, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{})
require.ErrorContains(t, err, "403")
require.ErrorContains(t, err, "404")
// Then:
// When we call with includes_deleted, we should get the workspace back
@ -860,7 +859,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
require.IsType(t, err, &codersdk.Error{}, "expected codersdk.Error")
coderSDKErr, _ := err.(*codersdk.Error) //nolint:errorlint
require.Equal(t, coderSDKErr.StatusCode(), 404, "expected status code 404")
require.Equal(t, fmt.Sprintf("Workspace %q does not exist.", wsid), coderSDKErr.Message, "unexpected response code")
require.Contains(t, coderSDKErr.Message, "Resource not found", "unexpected response code")
})
}
@ -975,7 +974,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
require.IsType(t, err, &codersdk.Error{}, "expected codersdk.Error")
coderSDKErr, _ := err.(*codersdk.Error) //nolint:errorlint
require.Equal(t, coderSDKErr.StatusCode(), 404, "expected status code 404")
require.Equal(t, fmt.Sprintf("Workspace %q does not exist.", wsid), coderSDKErr.Message, "unexpected response code")
require.Contains(t, coderSDKErr.Message, "Resource not found", "unexpected response code")
})
}

11
scripts/rules.go
View File

@ -150,3 +150,14 @@ func HttpAPIErrorMessage(m dsl.Matcher) {
At(m["m"]).
Report("Field \"Message\" should be a proper sentence with a capitalized first letter and ending in punctuation. $m")
}
// ProperRBACReturn ensures we always write to the response writer after a
// call to Authorize. If we just do a return, the client will get a status code
// 200, which is incorrect.
func ProperRBACReturn(m dsl.Matcher) {
m.Match(`
if !$_.Authorize($*_) {
return
}
`).Report("Must write to 'ResponseWriter' before returning'")
}