Skip to content

Commit e930fb2

Browse files
EmyrkEmyrk
committed
Add more dbauthz tests
Catch dbmem panic bug
1 parent c31b190 commit e930fb2

File tree

4 files changed

+70
-2
lines changed

4 files changed

+70
-2
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,10 +2266,12 @@ func (q *querier) InsertWorkspaceAgent(ctx context.Context, arg database.InsertW
22662266
}
22672267

22682268
func (q *querier) InsertWorkspaceAgentLogSources(ctx context.Context, arg database.InsertWorkspaceAgentLogSourcesParams) ([]database.WorkspaceAgentLogSource, error) {
2269+
// TODO: This is used by the agent, should we have an rbac check here?
22692270
return q.db.InsertWorkspaceAgentLogSources(ctx, arg)
22702271
}
22712272

22722273
func (q *querier) InsertWorkspaceAgentLogs(ctx context.Context, arg database.InsertWorkspaceAgentLogsParams) ([]database.WorkspaceAgentLog, error) {
2274+
// TODO: This is used by the agent, should we have an rbac check here?
22732275
return q.db.InsertWorkspaceAgentLogs(ctx, arg)
22742276
}
22752277

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,19 @@ func (s *MethodTestSuite) TestAPIKey() {
225225
ID: a.ID,
226226
}).Asserts(a, rbac.ActionUpdate).Returns()
227227
}))
228+
s.Run("DeleteApplicationConnectAPIKeysByUserID", s.Subtest(func(db database.Store, check *expects) {
229+
a, _ := dbgen.APIKey(s.T(), db, database.APIKey{
230+
Scope: database.APIKeyScopeApplicationConnect,
231+
})
232+
check.Args(a.UserID).Asserts(rbac.ResourceAPIKey.WithOwner(a.UserID.String()), rbac.ActionDelete).Returns()
233+
}))
234+
s.Run("DeleteExternalAuthLink", s.Subtest(func(db database.Store, check *expects) {
235+
a := dbgen.ExternalAuthLink(s.T(), db, database.ExternalAuthLink{})
236+
check.Args(database.DeleteExternalAuthLinkParams{
237+
ProviderID: a.ProviderID,
238+
UserID: a.UserID,
239+
}).Asserts(a, rbac.ActionDelete).Returns()
240+
}))
228241
}
229242

230243
func (s *MethodTestSuite) TestAuditLogs() {
@@ -1048,6 +1061,11 @@ func (s *MethodTestSuite) TestUser() {
10481061
rbac.ResourceRoleAssignment, rbac.ActionDelete,
10491062
).Returns(o)
10501063
}))
1064+
s.Run("AllUserIDs", s.Subtest(func(db database.Store, check *expects) {
1065+
a := dbgen.User(s.T(), db, database.User{})
1066+
b := dbgen.User(s.T(), db, database.User{})
1067+
check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(slice.New(a.ID, b.ID))
1068+
}))
10511069
}
10521070

10531071
func (s *MethodTestSuite) TestWorkspace() {
@@ -1405,6 +1423,14 @@ func (s *MethodTestSuite) TestWorkspace() {
14051423
app := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: agt.ID})
14061424
check.Args(app.ID).Asserts(ws, rbac.ActionRead).Returns(ws)
14071425
}))
1426+
s.Run("ActivityBumpWorkspace", s.Subtest(func(db database.Store, check *expects) {
1427+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1428+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1429+
dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1430+
check.Args(database.ActivityBumpWorkspaceParams{
1431+
WorkspaceID: ws.ID,
1432+
}).Asserts(ws, rbac.ActionUpdate).Returns()
1433+
}))
14081434
}
14091435

14101436
func (s *MethodTestSuite) TestExtraMethods() {
@@ -1417,6 +1443,15 @@ func (s *MethodTestSuite) TestExtraMethods() {
14171443
s.NoError(err, "insert provisioner daemon")
14181444
check.Args().Asserts(d, rbac.ActionRead)
14191445
}))
1446+
s.Run("DeleteOldProvisionerDaemons", s.Subtest(func(db database.Store, check *expects) {
1447+
_, err := db.UpsertProvisionerDaemon(context.Background(), database.UpsertProvisionerDaemonParams{
1448+
Tags: database.StringMap(map[string]string{
1449+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
1450+
}),
1451+
})
1452+
s.NoError(err, "insert provisioner daemon")
1453+
check.Args().Asserts(rbac.ResourceSystem, rbac.ActionDelete)
1454+
}))
14201455
}
14211456

14221457
// All functions in this method test suite are not implemented in dbmem, but
@@ -1877,4 +1912,25 @@ func (s *MethodTestSuite) TestSystemFunctions() {
18771912
Transition: database.WorkspaceTransitionStart,
18781913
}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
18791914
}))
1915+
s.Run("DeleteOldWorkspaceAgentLogs", s.Subtest(func(db database.Store, check *expects) {
1916+
check.Args().Asserts(rbac.ResourceSystem, rbac.ActionDelete)
1917+
}))
1918+
s.Run("InsertWorkspaceAgentStats", s.Subtest(func(db database.Store, check *expects) {
1919+
check.Args(database.InsertWorkspaceAgentStatsParams{}).Asserts(rbac.ResourceSystem, rbac.ActionCreate).Errors(matchAnyError)
1920+
}))
1921+
s.Run("InsertWorkspaceAppStats", s.Subtest(func(db database.Store, check *expects) {
1922+
check.Args(database.InsertWorkspaceAppStatsParams{}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
1923+
}))
1924+
s.Run("InsertWorkspaceAgentScripts", s.Subtest(func(db database.Store, check *expects) {
1925+
check.Args(database.InsertWorkspaceAgentScriptsParams{}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
1926+
}))
1927+
s.Run("InsertWorkspaceAgentMetadata", s.Subtest(func(db database.Store, check *expects) {
1928+
check.Args(database.InsertWorkspaceAgentMetadataParams{}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
1929+
}))
1930+
s.Run("InsertWorkspaceAgentLogs", s.Subtest(func(db database.Store, check *expects) {
1931+
check.Args(database.InsertWorkspaceAgentLogsParams{}).Asserts()
1932+
}))
1933+
s.Run("InsertWorkspaceAgentLogSources", s.Subtest(func(db database.Store, check *expects) {
1934+
check.Args(database.InsertWorkspaceAgentLogSourcesParams{}).Asserts()
1935+
}))
18801936
}

‎coderd/database/dbauthz/setup_test.go

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

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"reflect"
78
"sort"
@@ -27,6 +28,10 @@ import (
2728
"github.com/coder/coder/v2/coderd/util/slice"
2829
)
2930

31+
var (
32+
matchAnyError = errors.New("match any error")
33+
)
34+
3035
var skipMethods = map[string]string{
3136
"InTx": "Not relevant",
3237
"Ping": "Not relevant",
@@ -174,7 +179,12 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
174179
if testCase.err == nil {
175180
s.NoError(err, "method %q returned an error", methodName)
176181
} else {
177-
s.EqualError(err, testCase.err.Error(), "method %q returned an unexpected error", methodName)
182+
if errors.Is(testCase.err, matchAnyError) {
183+
// This means we do not care exactly what the error is.
184+
s.Error(err, "method %q returned an error", methodName)
185+
} else {
186+
s.EqualError(err, testCase.err.Error(), "method %q returned an unexpected error", methodName)
187+
}
178188
}
179189

180190
// Some tests may not care about the outputs, so we only assert if

‎coderd/database/dbmem/dbmem.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,7 +880,7 @@ func (q *FakeQuerier) AllUserIDs(_ context.Context) ([]uuid.UUID, error) {
880880
defer q.mutex.RUnlock()
881881
userIDs := make([]uuid.UUID, 0, len(q.users))
882882
for idx := range q.users {
883-
userIDs[idx] = q.users[idx].ID
883+
userIDs = append(userIDs, q.users[idx].ID)
884884
}
885885
return userIDs, nil
886886
}

0 commit comments

Comments
 (0)