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

Commit38e5b96

Browse files
authored
chore: Rbac errors should be returned, and not hidden behind 404 (#7122)
* chore: Rbac errors should be returned, and not hidden behind 404SqlErrNoRows was hiding actual errors* Replace sql.ErrNoRow checks* Remove sql err no rows check from dbauthz test* Fix to use dbauthz system user
1 parentfa64c58 commit38e5b96

23 files changed

+50
-72
lines changed

‎coderd/apikey.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package coderd
33
import (
44
"context"
55
"crypto/sha256"
6-
"database/sql"
7-
"errors"
86
"fmt"
97
"net"
108
"net/http"
@@ -167,7 +165,7 @@ func (api *API) apiKeyByID(rw http.ResponseWriter, r *http.Request) {
167165

168166
keyID:=chi.URLParam(r,"keyid")
169167
key,err:=api.Database.GetAPIKeyByID(ctx,keyID)
170-
iferrors.Is(err,sql.ErrNoRows) {
168+
ifhttpapi.Is404Error(err) {
171169
httpapi.ResourceNotFound(rw)
172170
return
173171
}
@@ -202,7 +200,7 @@ func (api *API) apiKeyByName(rw http.ResponseWriter, r *http.Request) {
202200
TokenName:tokenName,
203201
UserID:user.ID,
204202
})
205-
iferrors.Is(err,sql.ErrNoRows) {
203+
ifhttpapi.Is404Error(err) {
206204
httpapi.ResourceNotFound(rw)
207205
return
208206
}
@@ -323,7 +321,7 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
323321
defercommitAudit()
324322

325323
err=api.Database.DeleteAPIKeyByID(ctx,keyID)
326-
iferrors.Is(err,sql.ErrNoRows) {
324+
ifhttpapi.Is404Error(err) {
327325
httpapi.ResourceNotFound(rw)
328326
return
329327
}

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ func (e NotAuthorizedError) Error() string {
3434

3535
// Unwrap will always unwrap to a sql.ErrNoRows so the API returns a 404.
3636
// So 'errors.Is(err, sql.ErrNoRows)' will always be true.
37-
func (NotAuthorizedError)Unwrap()error {
38-
returnsql.ErrNoRows
37+
func (eNotAuthorizedError)Unwrap()error {
38+
returne.Err
3939
}
4040

4141
funcIsNotAuthorizedError(errerror)bool {

‎coderd/database/dbauthz/setup_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package dbauthz_test
22

33
import (
44
"context"
5-
"database/sql"
65
"fmt"
76
"reflect"
87
"sort"
@@ -219,7 +218,6 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
219218
iferr!=nil||!hasEmptySliceResponse(resp) {
220219
s.ErrorContainsf(err,"unauthorized","error string should have a good message")
221220
s.Errorf(err,"method should an error with disallow authz")
222-
s.ErrorIsf(err,sql.ErrNoRows,"error should match sql.ErrNoRows")
223221
s.ErrorAs(err,&dbauthz.NotAuthorizedError{},"error should be NotAuthorizedError")
224222
}
225223
})

‎coderd/files.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) {
126126
}
127127

128128
file,err:=api.Database.GetFileByID(ctx,id)
129-
iferrors.Is(err,sql.ErrNoRows) {
129+
ifhttpapi.Is404Error(err) {
130130
httpapi.ResourceNotFound(rw)
131131
return
132132
}

‎coderd/httpapi/httpapi.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package httpapi
33
import (
44
"bytes"
55
"context"
6+
"database/sql"
67
"encoding/json"
78
"errors"
89
"flag"
@@ -15,6 +16,8 @@ import (
1516
"github.com/go-playground/validator/v10"
1617
"golang.org/x/xerrors"
1718

19+
"github.com/coder/coder/coderd/database/dbauthz"
20+
"github.com/coder/coder/coderd/rbac"
1821
"github.com/coder/coder/coderd/tracing"
1922
"github.com/coder/coder/codersdk"
2023
)
@@ -80,6 +83,16 @@ func init() {
8083
}
8184
}
8285

86+
// Is404Error returns true if the given error should return a 404 status code.
87+
// Both actual 404s and unauthorized errors should return 404s to not leak
88+
// information about the existence of resources.
89+
funcIs404Error(errerror)bool {
90+
iferr==nil {
91+
returnfalse
92+
}
93+
returnxerrors.Is(err,sql.ErrNoRows)||dbauthz.IsNotAuthorizedError(err)||rbac.IsUnauthorizedError(err)
94+
}
95+
8396
// Convenience error functions don't take contexts since their responses are
8497
// static, it doesn't make much sense to trace them.
8598

‎coderd/httpmw/groupparam.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@ package httpmw
22

33
import (
44
"context"
5-
"database/sql"
6-
"errors"
75
"net/http"
86

97
"github.com/go-chi/chi/v5"
10-
"golang.org/x/xerrors"
118

129
"github.com/coder/coder/coderd/database"
1310
"github.com/coder/coder/coderd/httpapi"
@@ -45,7 +42,7 @@ func ExtractGroupByNameParam(db database.Store) func(http.Handler) http.Handler
4542
OrganizationID:org.ID,
4643
Name:name,
4744
})
48-
ifxerrors.Is(err,sql.ErrNoRows) {
45+
ifhttpapi.Is404Error(err) {
4946
httpapi.ResourceNotFound(rw)
5047
return
5148
}
@@ -73,7 +70,7 @@ func ExtractGroupParam(db database.Store) func(http.Handler) http.Handler {
7370
}
7471

7572
group,err:=db.GetGroupByID(r.Context(),groupID)
76-
iferrors.Is(err,sql.ErrNoRows) {
73+
ifhttpapi.Is404Error(err) {
7774
httpapi.ResourceNotFound(rw)
7875
return
7976
}

‎coderd/httpmw/organizationparam.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package httpmw
22

33
import (
44
"context"
5-
"database/sql"
6-
"errors"
75
"net/http"
86

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

4947
organization,err:=db.GetOrganizationByID(ctx,orgID)
50-
iferrors.Is(err,sql.ErrNoRows) {
48+
ifhttpapi.Is404Error(err) {
5149
httpapi.ResourceNotFound(rw)
5250
return
5351
}
@@ -77,7 +75,7 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
7775
OrganizationID:organization.ID,
7876
UserID:user.ID,
7977
})
80-
iferrors.Is(err,sql.ErrNoRows) {
78+
ifhttpapi.Is404Error(err) {
8179
httpapi.ResourceNotFound(rw)
8280
return
8381
}

‎coderd/httpmw/templateparam.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package httpmw
22

33
import (
44
"context"
5-
"database/sql"
6-
"errors"
75
"net/http"
86

97
"github.com/go-chi/chi/v5"
@@ -34,7 +32,7 @@ func ExtractTemplateParam(db database.Store) func(http.Handler) http.Handler {
3432
return
3533
}
3634
template,err:=db.GetTemplateByID(r.Context(),templateID)
37-
iferrors.Is(err,sql.ErrNoRows)|| (err==nil&&template.Deleted) {
35+
ifhttpapi.Is404Error(err)|| (err==nil&&template.Deleted) {
3836
httpapi.ResourceNotFound(rw)
3937
return
4038
}

‎coderd/httpmw/templateversionparam.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package httpmw
33
import (
44
"context"
55
"database/sql"
6-
"errors"
76
"net/http"
87

98
"github.com/go-chi/chi/v5"
@@ -35,7 +34,7 @@ func ExtractTemplateVersionParam(db database.Store) func(http.Handler) http.Hand
3534
return
3635
}
3736
templateVersion,err:=db.GetTemplateVersionByID(ctx,templateVersionID)
38-
iferrors.Is(err,sql.ErrNoRows) {
37+
ifhttpapi.Is404Error(err) {
3938
httpapi.ResourceNotFound(rw)
4039
return
4140
}

‎coderd/httpmw/userparam.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@ package httpmw
22

33
import (
44
"context"
5-
"database/sql"
65
"net/http"
76

8-
"golang.org/x/xerrors"
9-
107
"github.com/go-chi/chi/v5"
118
"github.com/google/uuid"
129

@@ -71,7 +68,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
7168
}
7269
//nolint:gocritic // System needs to be able to get user from param.
7370
user,err=db.GetUserByID(dbauthz.AsSystemRestricted(ctx),apiKey.UserID)
74-
ifxerrors.Is(err,sql.ErrNoRows) {
71+
ifhttpapi.Is404Error(err) {
7572
httpapi.ResourceNotFound(rw)
7673
return
7774
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp