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

Commitdba0dfa

Browse files
authored
chore: correct 500 -> 404 on workspace agent mw (#11129)
* chore: correct 500 -> 404
1 parent0181e03 commitdba0dfa

File tree

6 files changed

+73
-32
lines changed

6 files changed

+73
-32
lines changed

‎coderd/coderdtest/authorize.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"runtime"
88
"strings"
99
"sync"
10+
"sync/atomic"
1011
"testing"
1112

1213
"github.com/google/uuid"
@@ -16,6 +17,7 @@ import (
1617
"golang.org/x/xerrors"
1718

1819
"github.com/coder/coder/v2/coderd"
20+
"github.com/coder/coder/v2/coderd/database"
1921
"github.com/coder/coder/v2/coderd/database/dbauthz"
2022
"github.com/coder/coder/v2/coderd/rbac"
2123
"github.com/coder/coder/v2/coderd/rbac/regosql"
@@ -451,3 +453,22 @@ func must[T any](value T, err error) T {
451453
}
452454
returnvalue
453455
}
456+
457+
typeFakeAccessControlStorestruct{}
458+
459+
func (FakeAccessControlStore)GetTemplateAccessControl(t database.Template) dbauthz.TemplateAccessControl {
460+
return dbauthz.TemplateAccessControl{
461+
RequireActiveVersion:t.RequireActiveVersion,
462+
}
463+
}
464+
465+
func (FakeAccessControlStore)SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, dbauthz.TemplateAccessControl)error {
466+
panic("not implemented")
467+
}
468+
469+
funcAccessControlStorePointer()*atomic.Pointer[dbauthz.AccessControlStore] {
470+
acs:=&atomic.Pointer[dbauthz.AccessControlStore]{}
471+
vartacs dbauthz.AccessControlStore=FakeAccessControlStore{}
472+
acs.Store(&tacs)
473+
returnacs
474+
}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"golang.org/x/xerrors"
1414

1515
"cdr.dev/slog"
16+
1617
"github.com/coder/coder/v2/coderd/coderdtest"
1718
"github.com/coder/coder/v2/coderd/database"
1819
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -62,7 +63,7 @@ func TestAsNoActor(t *testing.T) {
6263
funcTestPing(t*testing.T) {
6364
t.Parallel()
6465

65-
q:=dbauthz.New(dbmem.New(),&coderdtest.RecordingAuthorizer{},slog.Make(),accessControlStorePointer())
66+
q:=dbauthz.New(dbmem.New(),&coderdtest.RecordingAuthorizer{},slog.Make(),coderdtest.AccessControlStorePointer())
6667
_,err:=q.Ping(context.Background())
6768
require.NoError(t,err,"must not error")
6869
}
@@ -74,7 +75,7 @@ func TestInTX(t *testing.T) {
7475
db:=dbmem.New()
7576
q:=dbauthz.New(db,&coderdtest.RecordingAuthorizer{
7677
Wrapped:&coderdtest.FakeAuthorizer{AlwaysReturn:xerrors.New("custom error")},
77-
},slog.Make(),accessControlStorePointer())
78+
},slog.Make(),coderdtest.AccessControlStorePointer())
7879
actor:= rbac.Subject{
7980
ID:uuid.NewString(),
8081
Roles: rbac.RoleNames{rbac.RoleOwner()},
@@ -110,8 +111,8 @@ func TestNew(t *testing.T) {
110111

111112
// Double wrap should not cause an actual double wrap. So only 1 rbac call
112113
// should be made.
113-
az:=dbauthz.New(db,rec,slog.Make(),accessControlStorePointer())
114-
az=dbauthz.New(az,rec,slog.Make(),accessControlStorePointer())
114+
az:=dbauthz.New(db,rec,slog.Make(),coderdtest.AccessControlStorePointer())
115+
az=dbauthz.New(az,rec,slog.Make(),coderdtest.AccessControlStorePointer())
115116

116117
w,err:=az.GetWorkspaceByID(ctx,exp.ID)
117118
require.NoError(t,err,"must not error")
@@ -128,7 +129,7 @@ func TestDBAuthzRecursive(t *testing.T) {
128129
t.Parallel()
129130
q:=dbauthz.New(dbmem.New(),&coderdtest.RecordingAuthorizer{
130131
Wrapped:&coderdtest.FakeAuthorizer{AlwaysReturn:nil},
131-
},slog.Make(),accessControlStorePointer())
132+
},slog.Make(),coderdtest.AccessControlStorePointer())
132133
actor:= rbac.Subject{
133134
ID:uuid.NewString(),
134135
Roles: rbac.RoleNames{rbac.RoleOwner()},

‎coderd/database/dbauthz/setup_test.go‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"reflect"
77
"sort"
88
"strings"
9-
"sync/atomic"
109
"testing"
1110

1211
"github.com/golang/mock/gomock"
@@ -17,6 +16,7 @@ import (
1716
"golang.org/x/xerrors"
1817

1918
"cdr.dev/slog"
19+
2020
"github.com/coder/coder/v2/coderd/coderdtest"
2121
"github.com/coder/coder/v2/coderd/database"
2222
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -60,7 +60,7 @@ func (s *MethodTestSuite) SetupSuite() {
6060
mockStore:=dbmock.NewMockStore(ctrl)
6161
// We intentionally set no expectations apart from this.
6262
mockStore.EXPECT().Wrappers().Return([]string{}).AnyTimes()
63-
az:=dbauthz.New(mockStore,nil,slog.Make(),accessControlStorePointer())
63+
az:=dbauthz.New(mockStore,nil,slog.Make(),coderdtest.AccessControlStorePointer())
6464
// Take the underlying type of the interface.
6565
azt:=reflect.TypeOf(az).Elem()
6666
s.methodAccounting=make(map[string]int)
@@ -111,7 +111,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
111111
rec:=&coderdtest.RecordingAuthorizer{
112112
Wrapped:fakeAuthorizer,
113113
}
114-
az:=dbauthz.New(db,rec,slog.Make(),accessControlStorePointer())
114+
az:=dbauthz.New(db,rec,slog.Make(),coderdtest.AccessControlStorePointer())
115115
actor:= rbac.Subject{
116116
ID:uuid.NewString(),
117117
Roles: rbac.RoleNames{rbac.RoleOwner()},
@@ -399,22 +399,3 @@ func (emptyPreparedAuthorized) Authorize(_ context.Context, _ rbac.Object) error
399399
func (emptyPreparedAuthorized)CompileToSQL(_ context.Context,_ regosql.ConvertConfig) (string,error) {
400400
return"",nil
401401
}
402-
403-
funcaccessControlStorePointer()*atomic.Pointer[dbauthz.AccessControlStore] {
404-
acs:=&atomic.Pointer[dbauthz.AccessControlStore]{}
405-
vartacs dbauthz.AccessControlStore=fakeAccessControlStore{}
406-
acs.Store(&tacs)
407-
returnacs
408-
}
409-
410-
typefakeAccessControlStorestruct{}
411-
412-
func (fakeAccessControlStore)GetTemplateAccessControl(t database.Template) dbauthz.TemplateAccessControl {
413-
return dbauthz.TemplateAccessControl{
414-
RequireActiveVersion:t.RequireActiveVersion,
415-
}
416-
}
417-
418-
func (fakeAccessControlStore)SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, dbauthz.TemplateAccessControl)error {
419-
panic("not implemented")
420-
}

‎coderd/httpmw/workspaceagentparam.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/go-chi/chi/v5"
@@ -35,9 +33,9 @@ func ExtractWorkspaceAgentParam(db database.Store) func(http.Handler) http.Handl
3533
}
3634

3735
agent,err:=db.GetWorkspaceAgentByID(ctx,agentUUID)
38-
iferrors.Is(err,sql.ErrNoRows) {
36+
ifhttpapi.Is404Error(err) {
3937
httpapi.Write(ctx,rw,http.StatusNotFound, codersdk.Response{
40-
Message:"Agent doesn't exist with that id.",
38+
Message:"Agent doesn't exist with that id, or you do not have access to it.",
4139
})
4240
return
4341
}

‎coderd/httpmw/workspaceagentparam_test.go‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ import (
66
"net/http/httptest"
77
"testing"
88

9+
"cdr.dev/slog"
910
"github.com/go-chi/chi/v5"
1011
"github.com/google/uuid"
1112
"github.com/stretchr/testify/require"
13+
"golang.org/x/xerrors"
1214

15+
"github.com/coder/coder/v2/coderd/coderdtest"
1316
"github.com/coder/coder/v2/coderd/database"
17+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1418
"github.com/coder/coder/v2/coderd/database/dbgen"
1519
"github.com/coder/coder/v2/coderd/database/dbmem"
1620
"github.com/coder/coder/v2/coderd/httpmw"
@@ -26,8 +30,10 @@ func TestWorkspaceAgentParam(t *testing.T) {
2630
_,token=dbgen.APIKey(t,db, database.APIKey{
2731
UserID:user.ID,
2832
})
33+
tpl=dbgen.Template(t,db, database.Template{})
2934
workspace=dbgen.Workspace(t,db, database.Workspace{
30-
OwnerID:user.ID,
35+
OwnerID:user.ID,
36+
TemplateID:tpl.ID,
3137
})
3238
build=dbgen.WorkspaceBuild(t,db, database.WorkspaceBuild{
3339
WorkspaceID:workspace.ID,
@@ -91,6 +97,36 @@ func TestWorkspaceAgentParam(t *testing.T) {
9197
require.Equal(t,http.StatusNotFound,res.StatusCode)
9298
})
9399

100+
t.Run("NotAuthorized",func(t*testing.T) {
101+
t.Parallel()
102+
db:=dbmem.New()
103+
fakeAuthz:=&coderdtest.FakeAuthorizer{AlwaysReturn:xerrors.Errorf("constant failure")}
104+
dbFail:=dbauthz.New(db,fakeAuthz,slog.Make(),coderdtest.AccessControlStorePointer())
105+
106+
rtr:=chi.NewRouter()
107+
rtr.Use(
108+
httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
109+
DB:db,
110+
RedirectToLogin:false,
111+
}),
112+
// Only fail authz in this middleware
113+
httpmw.ExtractWorkspaceAgentParam(dbFail),
114+
)
115+
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
116+
_=httpmw.WorkspaceAgentParam(r)
117+
rw.WriteHeader(http.StatusOK)
118+
})
119+
120+
r,_:=setupAuthentication(db)
121+
122+
rw:=httptest.NewRecorder()
123+
rtr.ServeHTTP(rw,r)
124+
125+
res:=rw.Result()
126+
deferres.Body.Close()
127+
require.Equal(t,http.StatusNotFound,res.StatusCode)
128+
})
129+
94130
t.Run("WorkspaceAgent",func(t*testing.T) {
95131
t.Parallel()
96132
db:=dbmem.New()

‎coderd/workspaceagents.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) {
7878
returnerr
7979
})
8080
err:=eg.Wait()
81+
ifhttpapi.Is404Error(err) {
82+
httpapi.ResourceNotFound(rw)
83+
return
84+
}
8185
iferr!=nil {
8286
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
8387
Message:"Internal error fetching workspace agent.",

0 commit comments

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