Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: Rbac errors should be returned, and not hidden behind 404#7122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 5 commits intomainfromstevenmasley/rbac_error
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletionscoderd/apikey.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,6 @@ package coderd
import (
"context"
"crypto/sha256"
"database/sql"
"errors"
"fmt"
"net"
"net/http"
Expand DownExpand Up@@ -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)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand DownExpand Up@@ -202,7 +200,7 @@ func (api *API) apiKeyByName(rw http.ResponseWriter, r *http.Request) {
TokenName: tokenName,
UserID: user.ID,
})
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand DownExpand Up@@ -323,7 +321,7 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
defer commitAudit()

err = api.Database.DeleteAPIKeyByID(ctx, keyID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
4 changes: 2 additions & 2 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
returnsql.ErrNoRows
func (eNotAuthorizedError) Unwrap() error {
returne.Err
}

func IsNotAuthorizedError(err error) bool {
Expand Down
2 changes: 0 additions & 2 deletionscoderd/database/dbauthz/setup_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,6 @@ package dbauthz_test

import (
"context"
"database/sql"
"fmt"
"reflect"
"sort"
Expand DownExpand Up@@ -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")
}
})
Expand Down
2 changes: 1 addition & 1 deletioncoderd/files.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -126,7 +126,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) {
}

file, err := api.Database.GetFileByID(ctx, id)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
13 changes: 13 additions & 0 deletionscoderd/httpapi/httpapi.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,7 @@ package httpapi
import (
"bytes"
"context"
"database/sql"
"encoding/json"
"errors"
"flag"
Expand All@@ -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"
)
Expand DownExpand Up@@ -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.

Expand Down
7 changes: 2 additions & 5 deletionscoderd/httpmw/groupparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -45,7 +42,7 @@ func ExtractGroupByNameParam(db database.Store) func(http.Handler) http.Handler
OrganizationID: org.ID,
Name: name,
})
ifxerrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand DownExpand Up@@ -73,7 +70,7 @@ func ExtractGroupParam(db database.Store) func(http.Handler) http.Handler {
}

group, err := db.GetGroupByID(r.Context(), groupID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
6 changes: 2 additions & 4 deletionscoderd/httpmw/organizationparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,8 +2,6 @@ package httpmw

import (
"context"
"database/sql"
"errors"
"net/http"

"github.com/coder/coder/coderd/database"
Expand DownExpand Up@@ -47,7 +45,7 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
}

organization, err := db.GetOrganizationByID(ctx, orgID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand DownExpand Up@@ -77,7 +75,7 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
OrganizationID: organization.ID,
UserID: user.ID,
})
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
4 changes: 1 addition & 3 deletionscoderd/httpmw/templateparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,8 +2,6 @@ package httpmw

import (
"context"
"database/sql"
"errors"
"net/http"

"github.com/go-chi/chi/v5"
Expand DownExpand Up@@ -34,7 +32,7 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
return
}
template, err := db.GetTemplateByID(r.Context(), templateID)
iferrors.Is(err, sql.ErrNoRows) || (err == nil && template.Deleted) {
ifhttpapi.Is404Error(err) || (err == nil && template.Deleted) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
3 changes: 1 addition & 2 deletionscoderd/httpmw/templateversionparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,6 @@ package httpmw
import (
"context"
"database/sql"
"errors"
"net/http"

"github.com/go-chi/chi/v5"
Expand DownExpand Up@@ -35,7 +34,7 @@ func ExtractTemplateVersionParam(db database.Store) func(http.Handler) http.Hand
return
}
templateVersion, err := db.GetTemplateVersionByID(ctx, templateVersionID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
5 changes: 1 addition & 4 deletionscoderd/httpmw/userparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"

Expand DownExpand Up@@ -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)
ifxerrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
4 changes: 1 addition & 3 deletionscoderd/httpmw/workspacebuildparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,8 +2,6 @@ package httpmw

import (
"context"
"database/sql"
"errors"
"net/http"

"github.com/go-chi/chi/v5"
Expand DownExpand Up@@ -34,7 +32,7 @@ func ExtractWorkspaceBuildParam(db database.Store) func(http.Handler) http.Handl
return
}
workspaceBuild, err := db.GetWorkspaceBuildByID(ctx, workspaceBuildID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
6 changes: 2 additions & 4 deletionscoderd/httpmw/workspaceparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,8 +2,6 @@ package httpmw

import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"strings"
Expand DownExpand Up@@ -37,7 +35,7 @@ func ExtractWorkspaceParam(db database.Store) func(http.Handler) http.Handler {
return
}
workspace, err := db.GetWorkspaceByID(ctx, workspaceID)
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand DownExpand Up@@ -74,7 +72,7 @@ func ExtractWorkspaceAndAgentParam(db database.Store) func(http.Handler) http.Ha
Name: workspaceParts[0],
})
if err != nil {
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
2 changes: 1 addition & 1 deletioncoderd/parameters.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,7 +141,7 @@ func (api *API) deleteParameter(rw http.ResponseWriter, r *http.Request) {
ScopeID: scopeID,
Name: name,
})
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down
2 changes: 1 addition & 1 deletioncoderd/provisionerjobs.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ooops, I missed this

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The sql.ErrNoRows hid it☹️

if errors.Is(err, sql.ErrNoRows) {
err = nil
}
Expand Down
11 changes: 1 addition & 10 deletionscoderd/templates.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -407,7 +407,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
Name: templateName,
})
if err != nil {
iferrors.Is(err, sql.ErrNoRows) {
ifhttpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
Expand All@@ -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{
Expand DownExpand Up@@ -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 {
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp