Skip to content

feat(enterprise/audit): add user object to slog exporter #9456

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 6 additions & 4 deletions coderd/audit/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ type Request[T Auditable] struct {
Old T
New T

// This optional field can be passed in when the userID cannot be determined from the API Key
// such as in the case of login, when the audit log is created prior the API Key's existence.
// UserID is an optional field can be passed in when the userID cannot be
// determined from the API Key such as in the case of login, when the audit
// log is created prior the API Key's existence.
UserID uuid.UUID

// This optional field can be passed in if the AuditAction must be overridden
// such as in the case of new user authentication when the Audit Action is 'register', not 'login'.
// Action is an optional field can be passed in if the AuditAction must be
// overridden such as in the case of new user authentication when the Audit
// Action is 'register', not 'login'.
Action database.AuditAction
}

Expand Down
30 changes: 27 additions & 3 deletions enterprise/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,37 @@ package audit

import (
"context"
"database/sql"

"github.com/google/uuid"
"golang.org/x/xerrors"

"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
)

type BackendDetails struct {
Actor *Actor
}

type Actor struct {
ID uuid.UUID `json:"id"`
Email string `json:"email"`
Username string `json:"username"`
}

// Backends can store or send audit logs to arbitrary locations.
type Backend interface {
// Decision determines the FilterDecisions that the backend tolerates.
Decision() FilterDecision
// Export sends an audit log to the backend.
Export(ctx context.Context, alog database.AuditLog) error
Export(ctx context.Context, alog database.AuditLog, details BackendDetails) error
}

func NewAuditor(filter Filter, backends ...Backend) audit.Auditor {
func NewAuditor(db database.Store, filter Filter, backends ...Backend) audit.Auditor {
return &auditor{
db: db,
filter: filter,
backends: backends,
Differ: audit.Differ{DiffFn: func(old, new any) audit.Map {
Expand All @@ -29,6 +43,7 @@ func NewAuditor(filter Filter, backends ...Backend) audit.Auditor {

// auditor is the enterprise implementation of the Auditor interface.
type auditor struct {
db database.Store
filter Filter
backends []Backend

Expand All @@ -41,12 +56,21 @@ func (a *auditor) Export(ctx context.Context, alog database.AuditLog) error {
return xerrors.Errorf("filter check: %w", err)
}

actor, err := a.db.GetUserByID(dbauthz.AsSystemRestricted(ctx), alog.UserID) //nolint
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
return err
}

for _, backend := range a.backends {
if decision&backend.Decision() != backend.Decision() {
continue
}

err = backend.Export(ctx, alog)
err = backend.Export(ctx, alog, BackendDetails{Actor: &Actor{
ID: actor.ID,
Email: actor.Email,
Username: actor.Username,
}})
if err != nil {
// naively return the first error. should probably make this smarter
// by returning multiple errors.
Expand Down
16 changes: 13 additions & 3 deletions enterprise/audit/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"golang.org/x/xerrors"

"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/audit/audittest"
)
Expand Down Expand Up @@ -90,6 +91,7 @@ func TestAuditor(t *testing.T) {
var (
backend = &testBackend{decision: test.backendDecision, err: test.backendError}
exporter = audit.NewAuditor(
dbfake.New(),
audit.FilterFunc(func(_ context.Context, _ database.AuditLog) (audit.FilterDecision, error) {
return test.filterDecision, test.filterError
}),
Expand All @@ -113,18 +115,26 @@ type testBackend struct {
decision audit.FilterDecision
err error

alogs []database.AuditLog
alogs []testExport
}

type testExport struct {
alog database.AuditLog
details audit.BackendDetails
}

func (t *testBackend) Decision() audit.FilterDecision {
return t.decision
}

func (t *testBackend) Export(_ context.Context, alog database.AuditLog) error {
func (t *testBackend) Export(_ context.Context, alog database.AuditLog, details audit.BackendDetails) error {
if t.err != nil {
return t.err
}

t.alogs = append(t.alogs, alog)
t.alogs = append(t.alogs, testExport{
alog: alog,
details: details,
})
return nil
}
2 changes: 1 addition & 1 deletion enterprise/audit/backends/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (b *postgresBackend) Decision() audit.FilterDecision {
return audit.FilterDecisionExport
}

func (b *postgresBackend) Export(ctx context.Context, alog database.AuditLog) error {
func (b *postgresBackend) Export(ctx context.Context, alog database.AuditLog, _ audit.BackendDetails) error {
_, err := b.db.InsertAuditLog(ctx, database.InsertAuditLogParams(alog))
if err != nil {
return xerrors.Errorf("insert audit log: %w", err)
Expand Down
3 changes: 2 additions & 1 deletion enterprise/audit/backends/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/audit/audittest"
"github.com/coder/coder/v2/enterprise/audit/backends"
)
Expand All @@ -25,7 +26,7 @@ func TestPostgresBackend(t *testing.T) {
)
defer cancel()

err := pgb.Export(ctx, alog)
err := pgb.Export(ctx, alog, audit.BackendDetails{})
require.NoError(t, err)

got, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{
Expand Down
6 changes: 5 additions & 1 deletion enterprise/audit/backends/slog.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (*slogBackend) Decision() audit.FilterDecision {
return audit.FilterDecisionExport
}

func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog) error {
func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog, details audit.BackendDetails) error {
// We don't use structs.Map because we don't want to recursively convert
// fields into maps. When we keep the type information, slog can more
// pleasantly format the output. For example, the clean result of
Expand All @@ -35,6 +35,10 @@ func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog) error
fields = append(fields, b.fieldToSlog(sf))
}

if details.Actor != nil {
fields = append(fields, slog.F("actor", details.Actor))
}

b.log.Info(ctx, "audit_log", fields...)
return nil
}
Expand Down
13 changes: 9 additions & 4 deletions enterprise/audit/backends/slog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"cdr.dev/slog"
"cdr.dev/slog/sloggers/slogjson"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/audit/audittest"
"github.com/coder/coder/v2/enterprise/audit/backends"
)
Expand All @@ -39,7 +40,7 @@ func TestSlogBackend(t *testing.T) {
)
defer cancel()

err := backend.Export(ctx, alog)
err := backend.Export(ctx, alog, audit.BackendDetails{})
require.NoError(t, err)
require.Len(t, sink.entries, 1)
require.Equal(t, sink.entries[0].Message, "audit_log")
Expand All @@ -59,7 +60,7 @@ func TestSlogBackend(t *testing.T) {
_, inet, _ = net.ParseCIDR("127.0.0.1/32")
alog = database.AuditLog{
ID: uuid.UUID{1},
Time: time.Unix(1257894000, 0),
Time: time.Unix(1257894000, 0).UTC(),
UserID: uuid.UUID{2},
OrganizationID: uuid.UUID{3},
Ip: pqtype.Inet{
Expand All @@ -80,7 +81,11 @@ func TestSlogBackend(t *testing.T) {
)
defer cancel()

err := backend.Export(ctx, alog)
err := backend.Export(ctx, alog, audit.BackendDetails{Actor: &audit.Actor{
ID: uuid.UUID{2},
Username: "coadler",
Email: "doug@coder.com",
}})
require.NoError(t, err)
logger.Sync()

Expand All @@ -90,7 +95,7 @@ func TestSlogBackend(t *testing.T) {
err = json.Unmarshal(buf.Bytes(), &s)
require.NoError(t, err)

expected := `{"ID":"01000000-0000-0000-0000-000000000000","Time":"2009-11-10T23:00:00Z","UserID":"02000000-0000-0000-0000-000000000000","OrganizationID":"03000000-0000-0000-0000-000000000000","Ip":"127.0.0.1","UserAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36","ResourceType":"organization","ResourceID":"04000000-0000-0000-0000-000000000000","ResourceTarget":"colin's organization","Action":"delete","Diff":{"1":2},"StatusCode":204,"AdditionalFields":{"name":"doug","species":"cat"},"RequestID":"05000000-0000-0000-0000-000000000000","ResourceIcon":"photo.png"}`
expected := `{"ID":"01000000-0000-0000-0000-000000000000","Time":"2009-11-10T23:00:00Z","UserID":"02000000-0000-0000-0000-000000000000","OrganizationID":"03000000-0000-0000-0000-000000000000","Ip":"127.0.0.1","UserAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36","ResourceType":"organization","ResourceID":"04000000-0000-0000-0000-000000000000","ResourceTarget":"colin's organization","Action":"delete","Diff":{"1":2},"StatusCode":204,"AdditionalFields":{"name":"doug","species":"cat"},"RequestID":"05000000-0000-0000-0000-000000000000","ResourceIcon":"photo.png","actor":{"id":"02000000-0000-0000-0000-000000000000","email":"doug@coder.com","username":"coadler"}}`
assert.Equal(t, expected, string(s.Fields))
})
}
Expand Down
4 changes: 3 additions & 1 deletion enterprise/cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ func (r *RootCmd) server() *clibase.Cmd {
}
}
options.DERPServer.SetMeshKey(meshKey)
options.Auditor = audit.NewAuditor(audit.DefaultFilter,
options.Auditor = audit.NewAuditor(
options.Database,
audit.DefaultFilter,
backends.NewPostgres(options.Database, true),
backends.NewSlog(options.Logger),
)
Expand Down
11 changes: 5 additions & 6 deletions enterprise/coderd/coderd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,16 @@ import (
"time"

"github.com/google/uuid"

"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/rbac"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"

agplaudit "github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/coderd"
Expand Down Expand Up @@ -185,7 +184,7 @@ func TestAuditLogging(t *testing.T) {
_, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
Auditor: audit.NewAuditor(audit.DefaultFilter),
Auditor: audit.NewAuditor(dbfake.New(), audit.DefaultFilter),
},
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
Expand All @@ -194,7 +193,7 @@ func TestAuditLogging(t *testing.T) {
},
})
auditor := *api.AGPL.Auditor.Load()
ea := audit.NewAuditor(audit.DefaultFilter)
ea := audit.NewAuditor(dbfake.New(), audit.DefaultFilter)
t.Logf("%T = %T", auditor, ea)
assert.EqualValues(t, reflect.ValueOf(ea).Type(), reflect.ValueOf(auditor).Type())
})
Expand Down