Skip to content

Commit dee0ca8

Browse files
EmyrkEmyrk
committed
Add unit testS
1 parent 4c9902a commit dee0ca8

File tree

4 files changed

+149
-21
lines changed

4 files changed

+149
-21
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,27 +1248,11 @@ func (q *querier) GetApplicationName(ctx context.Context) (string, error) {
12481248
}
12491249

12501250
func (q *querier) GetAuditLogsOffset(ctx context.Context, arg database.GetAuditLogsOffsetParams) ([]database.GetAuditLogsOffsetRow, error) {
1251-
//// To optimize the authz checks for audit logs, do not run an authorize
1252-
//// check on each individual audit log row. In practice, audit logs are either
1253-
//// fetched from a global or an organization scope.
1254-
//// Applying a SQL filter would slow down the query for no benefit on how this query is
1255-
//// actually used.
1256-
//
1257-
//object := rbac.ResourceAuditLog
1258-
//if arg.OrganizationID != uuid.Nil {
1259-
// object = object.InOrg(arg.OrganizationID)
1260-
//}
1261-
//
1262-
//if err := q.authorizeContext(ctx, policy.ActionRead, object); err != nil {
1263-
// return nil, err
1264-
//}
1265-
12661251
prep, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceAuditLog.Type)
12671252
if err != nil {
12681253
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
12691254
}
12701255

1271-
12721256
return q.db.GetAuthorizedAuditLogsOffset(ctx, arg, prep)
12731257
}
12741258

‎coderd/database/dbgen/dbgen.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,11 @@ var genCtx = dbauthz.As(context.Background(), rbac.Subject{
4040

4141
func AuditLog(t testing.TB, db database.Store, seed database.AuditLog) database.AuditLog {
4242
log, err := db.InsertAuditLog(genCtx, database.InsertAuditLogParams{
43-
ID: takeFirst(seed.ID, uuid.New()),
44-
Time: takeFirst(seed.Time, dbtime.Now()),
45-
UserID: takeFirst(seed.UserID, uuid.New()),
46-
OrganizationID: takeFirst(seed.OrganizationID, uuid.New()),
43+
ID: takeFirst(seed.ID, uuid.New()),
44+
Time: takeFirst(seed.Time, dbtime.Now()),
45+
UserID: takeFirst(seed.UserID, uuid.New()),
46+
// Default to the nil uuid. So by default audit logs are not org scoped.
47+
OrganizationID: takeFirst(seed.OrganizationID),
4748
Ip: pqtype.Inet{
4849
IPNet: takeFirstIP(seed.Ip.IPNet, net.IPNet{}),
4950
Valid: takeFirst(seed.Ip.Valid, false),

‎coderd/database/querier_test.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,19 @@ import (
1212
"time"
1313

1414
"github.com/google/uuid"
15+
"github.com/prometheus/client_golang/prometheus"
1516
"github.com/stretchr/testify/require"
1617

18+
"cdr.dev/slog/sloggers/slogtest"
19+
"github.com/coder/coder/v2/coderd/coderdtest"
1720
"github.com/coder/coder/v2/coderd/database"
1821
"github.com/coder/coder/v2/coderd/database/db2sdk"
22+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1923
"github.com/coder/coder/v2/coderd/database/dbgen"
24+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2025
"github.com/coder/coder/v2/coderd/database/dbtime"
2126
"github.com/coder/coder/v2/coderd/database/migrations"
27+
"github.com/coder/coder/v2/coderd/rbac"
2228
"github.com/coder/coder/v2/testutil"
2329
)
2430

@@ -767,6 +773,143 @@ func TestReadCustomRoles(t *testing.T) {
767773
}
768774
}
769775

776+
func TestAuthorizedAuditLogs(t *testing.T) {
777+
t.Parallel()
778+
779+
var allLogs []database.AuditLog
780+
db, _ := dbtestutil.NewDB(t)
781+
authz := rbac.NewAuthorizer(prometheus.NewRegistry())
782+
db = dbauthz.New(db, authz, slogtest.Make(t, &slogtest.Options{}), coderdtest.AccessControlStorePointer())
783+
784+
siteWideIDs := []uuid.UUID{uuid.New(), uuid.New()}
785+
for _, id := range siteWideIDs {
786+
allLogs = append(allLogs, dbgen.AuditLog(t, db, database.AuditLog{
787+
ID: id,
788+
OrganizationID: uuid.Nil,
789+
}))
790+
791+
}
792+
793+
orgAuditLogs := map[uuid.UUID][]uuid.UUID{
794+
uuid.New(): {uuid.New(), uuid.New()},
795+
uuid.New(): {uuid.New(), uuid.New()},
796+
}
797+
orgIDs := make([]uuid.UUID, 0, len(orgAuditLogs))
798+
for orgID := range orgAuditLogs {
799+
orgIDs = append(orgIDs, orgID)
800+
}
801+
for orgID, ids := range orgAuditLogs {
802+
dbgen.Organization(t, db, database.Organization{
803+
ID: orgID,
804+
})
805+
for _, id := range ids {
806+
allLogs = append(allLogs, dbgen.AuditLog(t, db, database.AuditLog{
807+
ID: id,
808+
OrganizationID: orgID,
809+
}))
810+
}
811+
}
812+
813+
// Now fetch all the logs
814+
ctx := testutil.Context(t, testutil.WaitLong)
815+
auditorRole, err := rbac.RoleByName(rbac.RoleAuditor())
816+
require.NoError(t, err)
817+
818+
memberRole, err := rbac.RoleByName(rbac.RoleMember())
819+
require.NoError(t, err)
820+
821+
orgAuditorRoles := func(t *testing.T, orgID uuid.UUID) rbac.Role {
822+
t.Helper()
823+
824+
role, err := rbac.RoleByName(rbac.ScopedRoleOrgAuditor(orgID))
825+
require.NoError(t, err)
826+
return role
827+
}
828+
829+
t.Run("NoAccess", func(t *testing.T) {
830+
siteAuditorCtx := dbauthz.As(ctx, rbac.Subject{
831+
FriendlyName: "member",
832+
ID: uuid.NewString(),
833+
Roles: rbac.Roles{memberRole},
834+
Scope: rbac.ScopeAll,
835+
})
836+
837+
logs, err := db.GetAuditLogsOffset(siteAuditorCtx, database.GetAuditLogsOffsetParams{})
838+
require.NoError(t, err)
839+
require.Len(t, logs, 0, "no logs should be returned")
840+
})
841+
842+
t.Run("SiteWideAuditor", func(t *testing.T) {
843+
siteAuditorCtx := dbauthz.As(ctx, rbac.Subject{
844+
FriendlyName: "owner",
845+
ID: uuid.NewString(),
846+
Roles: rbac.Roles{auditorRole},
847+
Scope: rbac.ScopeAll,
848+
})
849+
850+
logs, err := db.GetAuditLogsOffset(siteAuditorCtx, database.GetAuditLogsOffsetParams{})
851+
require.NoError(t, err)
852+
require.ElementsMatch(t, auditOnlyIDs(allLogs), auditOnlyIDs(logs))
853+
})
854+
855+
t.Run("SingleOrgAuditor", func(t *testing.T) {
856+
orgID := orgIDs[0]
857+
siteAuditorCtx := dbauthz.As(ctx, rbac.Subject{
858+
FriendlyName: "org-auditor",
859+
ID: uuid.NewString(),
860+
Roles: rbac.Roles{orgAuditorRoles(t, orgID)},
861+
Scope: rbac.ScopeAll,
862+
})
863+
864+
logs, err := db.GetAuditLogsOffset(siteAuditorCtx, database.GetAuditLogsOffsetParams{})
865+
require.NoError(t, err)
866+
require.ElementsMatch(t, orgAuditLogs[orgID], auditOnlyIDs(logs))
867+
})
868+
869+
t.Run("TwoOrgAuditors", func(t *testing.T) {
870+
first := orgIDs[0]
871+
second := orgIDs[1]
872+
siteAuditorCtx := dbauthz.As(ctx, rbac.Subject{
873+
FriendlyName: "org-auditor",
874+
ID: uuid.NewString(),
875+
Roles: rbac.Roles{orgAuditorRoles(t, first), orgAuditorRoles(t, second)},
876+
Scope: rbac.ScopeAll,
877+
})
878+
879+
logs, err := db.GetAuditLogsOffset(siteAuditorCtx, database.GetAuditLogsOffsetParams{})
880+
require.NoError(t, err)
881+
require.ElementsMatch(t, append(orgAuditLogs[first], orgAuditLogs[second]...), auditOnlyIDs(logs))
882+
})
883+
884+
t.Run("ErroneousOrg", func(t *testing.T) {
885+
siteAuditorCtx := dbauthz.As(ctx, rbac.Subject{
886+
FriendlyName: "org-auditor",
887+
ID: uuid.NewString(),
888+
Roles: rbac.Roles{orgAuditorRoles(t, uuid.New())},
889+
Scope: rbac.ScopeAll,
890+
})
891+
892+
logs, err := db.GetAuditLogsOffset(siteAuditorCtx, database.GetAuditLogsOffsetParams{})
893+
require.NoError(t, err)
894+
require.Len(t, logs, 0, "no logs should be returned")
895+
})
896+
}
897+
898+
func auditOnlyIDs[T database.AuditLog | database.GetAuditLogsOffsetRow](logs []T) []uuid.UUID {
899+
ids := make([]uuid.UUID, 0, len(logs))
900+
for _, log := range logs {
901+
switch log := any(log).(type) {
902+
case database.AuditLog:
903+
ids = append(ids, log.ID)
904+
case database.GetAuditLogsOffsetRow:
905+
ids = append(ids, log.AuditLog.ID)
906+
default:
907+
panic("unreachable")
908+
}
909+
}
910+
return ids
911+
}
912+
770913
type tvArgs struct {
771914
Status database.ProvisionerJobStatus
772915
// CreateWorkspace is true if we should create a workspace for the template version

‎coderd/rbac/regosql/configs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TemplateConverter() *sqltypes.VariableConverter {
3939
func AuditLogConverter() *sqltypes.VariableConverter {
4040
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
4141
resourceIDMatcher(),
42-
organizationOwnerMatcher(),
42+
sqltypes.StringVarMatcher("COALESCE(audit_logs.organization_id :: text, '')", []string{"input", "object", "org_owner"}),
4343
// Templates have no user owner, only owner by an organization.
4444
sqltypes.AlwaysFalse(userOwnerMatcher()),
4545
)

0 commit comments

Comments
 (0)