coder · Emyrk · Apr 13, 2023 · Apr 13, 2023 · Apr 13, 2023 · Apr 13, 2023
diff --git a/coderd/apikey.go b/coderd/apikey.go
@@ -3,8 +3,6 @@ package coderd
 import (
 	"context"
 	"crypto/sha256"
-	"database/sql"
-	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -167,7 +165,7 @@ func (api *API) apiKeyByID(rw http.ResponseWriter, r *http.Request) {
 
 	keyID := chi.URLParam(r, "keyid")
 	key, err := api.Database.GetAPIKeyByID(ctx, keyID)
-	if errors.Is(err, sql.ErrNoRows) {
+	if httpapi.Is404Error(err) {
 		httpapi.ResourceNotFound(rw)
 		return
 	}
@@ -202,7 +200,7 @@ func (api *API) apiKeyByName(rw http.ResponseWriter, r *http.Request) {
 		TokenName: tokenName,
 		UserID:    user.ID,
 	})
-	if errors.Is(err, sql.ErrNoRows) {
+	if httpapi.Is404Error(err) {
 		httpapi.ResourceNotFound(rw)
 		return
 	}
@@ -323,7 +321,7 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
 	defer commitAudit()
 
 	err = api.Database.DeleteAPIKeyByID(ctx, keyID)
-	if errors.Is(err, sql.ErrNoRows) {
+	if httpapi.Is404Error(err) {
 		httpapi.ResourceNotFound(rw)
 		return
 	}

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -34,8 +34,8 @@ func (e NotAuthorizedError) Error() string {
 
 // Unwrap will always unwrap to a sql.ErrNoRows so the API returns a 404.
 // So 'errors.Is(err, sql.ErrNoRows)' will always be true.
-func (NotAuthorizedError) Unwrap() error {
-	return sql.ErrNoRows
+func (e NotAuthorizedError) Unwrap() error {
+	return e.Err
 }
 
 func IsNotAuthorizedError(err error) bool {

diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go
@@ -2,7 +2,6 @@ package dbauthz_test
 
 import (
 	"context"
-	"database/sql"
 	"fmt"
 	"reflect"
 	"sort"
@@ -219,7 +218,6 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
 		if err != nil || !hasEmptySliceResponse(resp) {
 			s.ErrorContainsf(err, "unauthorized", "error string should have a good message")
 			s.Errorf(err, "method should an error with disallow authz")
-			s.ErrorIsf(err, sql.ErrNoRows, "error should match sql.ErrNoRows")
 			s.ErrorAs(err, &dbauthz.NotAuthorizedError{}, "error should be NotAuthorizedError")
 		}
 	})

diff --git a/coderd/files.go b/coderd/files.go
@@ -126,7 +126,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	file, err := api.Database.GetFileByID(ctx, id)
-	if errors.Is(err, sql.ErrNoRows) {
+	if httpapi.Is404Error(err) {
 		httpapi.ResourceNotFound(rw)
 		return
 	}

diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -3,6 +3,7 @@ package httpapi
 import (
 	"bytes"
 	"context"
+	"database/sql"
 	"encoding/json"
 	"errors"
 	"flag"
@@ -15,6 +16,8 @@ import (
 	"github.com/go-playground/validator/v10"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/coder/coderd/database/dbauthz"
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/coderd/tracing"
 	"github.com/coder/coder/codersdk"
 )
@@ -80,6 +83,16 @@ func init() {
 	}
 }
 
+// Is404Error returns true if the given error should return a 404 status code.
+// Both actual 404s and unauthorized errors should return 404s to not leak
+// information about the existence of resources.
+func Is404Error(err error) bool {
+	if err == nil {
+		return false
+	}
+	return xerrors.Is(err, sql.ErrNoRows) || dbauthz.IsNotAuthorizedError(err) || rbac.IsUnauthorizedError(err)
+}
+
 // Convenience error functions don't take contexts since their responses are
 // static, it doesn't make much sense to trace them.
 

diff --git a/coderd/httpmw/groupparam.go b/coderd/httpmw/groupparam.go
@@ -2,12 +2,9 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
-	"errors"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
-	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
@@ -45,7 +42,7 @@ func ExtractGroupByNameParam(db database.Store) func(http.Handler) http.Handler
 				OrganizationID: org.ID,
 				Name:           name,
 			})
-			if xerrors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}
@@ -73,7 +70,7 @@ func ExtractGroupParam(db database.Store) func(http.Handler) http.Handler {
 			}
 
 			group, err := db.GetGroupByID(r.Context(), groupID)
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}

diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go
@@ -2,8 +2,6 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
-	"errors"
 	"net/http"
 
 	"github.com/coder/coder/coderd/database"
@@ -47,7 +45,7 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
 			}
 
 			organization, err := db.GetOrganizationByID(ctx, orgID)
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}
@@ -77,7 +75,7 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
 				OrganizationID: organization.ID,
 				UserID:         user.ID,
 			})
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}

diff --git a/coderd/httpmw/templateparam.go b/coderd/httpmw/templateparam.go
@@ -2,8 +2,6 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
-	"errors"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
@@ -34,7 +32,7 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
 				return
 			}
 			template, err := db.GetTemplateByID(r.Context(), templateID)
-			if errors.Is(err, sql.ErrNoRows) || (err == nil && template.Deleted) {
+			if httpapi.Is404Error(err) || (err == nil && template.Deleted) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}

diff --git a/coderd/httpmw/templateversionparam.go b/coderd/httpmw/templateversionparam.go
@@ -3,7 +3,6 @@ package httpmw
 import (
 	"context"
 	"database/sql"
-	"errors"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
@@ -35,7 +34,7 @@ func ExtractTemplateVersionParam(db database.Store) func(http.Handler) http.Hand
 				return
 			}
 			templateVersion, err := db.GetTemplateVersionByID(ctx, templateVersionID)
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}

diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go
@@ -2,11 +2,8 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
 	"net/http"
 
-	"golang.org/x/xerrors"
-
 	"github.com/go-chi/chi/v5"
 	"github.com/google/uuid"
 
@@ -71,7 +68,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
 				}
 				//nolint:gocritic // System needs to be able to get user from param.
 				user, err = db.GetUserByID(dbauthz.AsSystemRestricted(ctx), apiKey.UserID)
-				if xerrors.Is(err, sql.ErrNoRows) {
+				if httpapi.Is404Error(err) {
 					httpapi.ResourceNotFound(rw)
 					return
 				}

diff --git a/coderd/httpmw/workspacebuildparam.go b/coderd/httpmw/workspacebuildparam.go
@@ -2,8 +2,6 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
-	"errors"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
@@ -34,7 +32,7 @@ func ExtractWorkspaceBuildParam(db database.Store) func(http.Handler) http.Handl
 				return
 			}
 			workspaceBuild, err := db.GetWorkspaceBuildByID(ctx, workspaceBuildID)
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}

diff --git a/coderd/httpmw/workspaceparam.go b/coderd/httpmw/workspaceparam.go
@@ -2,8 +2,6 @@ package httpmw
 
 import (
 	"context"
-	"database/sql"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -37,7 +35,7 @@ func ExtractWorkspaceParam(db database.Store) func(http.Handler) http.Handler {
 				return
 			}
 			workspace, err := db.GetWorkspaceByID(ctx, workspaceID)
-			if errors.Is(err, sql.ErrNoRows) {
+			if httpapi.Is404Error(err) {
 				httpapi.ResourceNotFound(rw)
 				return
 			}
@@ -74,7 +72,7 @@ func ExtractWorkspaceAndAgentParam(db database.Store) func(http.Handler) http.Ha
 				Name:    workspaceParts[0],
 			})
 			if err != nil {
-				if errors.Is(err, sql.ErrNoRows) {
+				if httpapi.Is404Error(err) {
 					httpapi.ResourceNotFound(rw)
 					return
 				}

diff --git a/coderd/parameters.go b/coderd/parameters.go
@@ -141,7 +141,7 @@ func (api *API) deleteParameter(rw http.ResponseWriter, r *http.Request) {
 		ScopeID: scopeID,
 		Name:    name,
 	})
-	if errors.Is(err, sql.ErrNoRows) {
+	if httpapi.Is404Error(err) {
 		httpapi.ResourceNotFound(rw)
 		return
 	}

diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go
@@ -224,7 +224,7 @@ func (api *API) provisionerJobResources(rw http.ResponseWriter, r *http.Request,
 	}
 
 	// nolint:gocritic // GetWorkspaceAppsByAgentIDs is a system function.
-	apps, err := api.Database.GetWorkspaceAppsByAgentIDs(ctx, resourceAgentIDs)
+	apps, err := api.Database.GetWorkspaceAppsByAgentIDs(dbauthz.AsSystemRestricted(ctx), resourceAgentIDs)
 	if errors.Is(err, sql.ErrNoRows) {
 		err = nil
 	}

diff --git a/coderd/templates.go b/coderd/templates.go
@@ -407,7 +407,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
 		Name:           templateName,
 	})
 	if err != nil {
-		if errors.Is(err, sql.ErrNoRows) {
+		if httpapi.Is404Error(err) {
 			httpapi.ResourceNotFound(rw)
 			return
 		}
@@ -419,11 +419,6 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
 		return
 	}
 
-	if !api.Authorize(r, rbac.ActionRead, template) {
-		httpapi.ResourceNotFound(rw)
-		return
-	}
-
 	createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, []database.Template{template})
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -583,10 +578,6 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	template := httpmw.TemplateParam(r)
-	if !api.Authorize(r, rbac.ActionRead, template) {
-		httpapi.ResourceNotFound(rw)
-		return
-	}
 
 	resp, _ := api.metricsCache.TemplateDAUs(template.ID)
 	if resp == nil || resp.Entries == nil {