Skip to content

Commit d96e1a2

Browse files
committed
extended the user query to include email and added a handler for logger
injection
1 parent 25fb34c commit d96e1a2

File tree

17 files changed

+131
-36
lines changed

17 files changed

+131
-36
lines changed

Makefile

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ GEN_FILES := \
582582
coderd/database/pubsub/psmock/psmock.go \
583583
agent/agentcontainers/acmock/acmock.go \
584584
agent/agentcontainers/dcspec/dcspec_gen.go \
585-
coderd/httpmw/loggermock/loggermock.go
585+
coderd/httpmw/loggermw/loggermock/loggermock.go
586586

587587
# all gen targets should be added here and to gen/mark-fresh
588588
gen: gen/db gen/golden-files $(GEN_FILES)
@@ -631,8 +631,7 @@ gen/mark-fresh:
631631
coderd/database/pubsub/psmock/psmock.go \
632632
agent/agentcontainers/acmock/acmock.go \
633633
agent/agentcontainers/dcspec/dcspec_gen.go \
634-
coderd/httpmw/loggermock/loggermock.go \
635-
"
634+
coderd/httpmw/loggermw/loggermock/loggermock.go
636635

637636
for file in $$files; do
638637
echo "$$file"
@@ -671,8 +670,8 @@ agent/agentcontainers/acmock/acmock.go: agent/agentcontainers/containers.go
671670
go generate ./agent/agentcontainers/acmock/
672671
touch "$@"
673672

674-
coderd/httpmw/loggermock/loggermock.go: coderd/httpmw/logger.go
675-
go generate ./coderd/httpmw/loggermock/
673+
coderd/httpmw/loggermw/loggermock/loggermock.go: coderd/httpmw/loggermw/logger.go
674+
go generate ./coderd/httpmw/loggermw/loggermock/
676675
touch "$@"
677676

678677
agent/agentcontainers/dcspec/dcspec_gen.go: \

coderd/coderd.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import (
6464
"github.com/coder/coder/v2/coderd/healthcheck/derphealth"
6565
"github.com/coder/coder/v2/coderd/httpapi"
6666
"github.com/coder/coder/v2/coderd/httpmw"
67+
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
6768
"github.com/coder/coder/v2/coderd/metricscache"
6869
"github.com/coder/coder/v2/coderd/notifications"
6970
"github.com/coder/coder/v2/coderd/portsharing"
@@ -799,7 +800,7 @@ func New(options *Options) *API {
799800
tracing.Middleware(api.TracerProvider),
800801
httpmw.AttachRequestID,
801802
httpmw.ExtractRealIP(api.RealIPConfig),
802-
httpmw.Logger(api.Logger),
803+
loggermw.Logger(api.Logger),
803804
singleSlashMW,
804805
rolestore.CustomRoleMW,
805806
prometheusMW,

coderd/database/dbauthz/dbauthz.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/coder/coder/v2/coderd/database"
2626
"github.com/coder/coder/v2/coderd/database/dbtime"
2727
"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
28+
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
2829
"github.com/coder/coder/v2/coderd/rbac"
2930
"github.com/coder/coder/v2/coderd/util/slice"
3031
"github.com/coder/coder/v2/provisionersdk"
@@ -163,6 +164,7 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
163164

164165
var (
165166
subjectProvisionerd = rbac.Subject{
167+
Type: rbac.SubjectTypeProvisionerd,
166168
FriendlyName: "Provisioner Daemon",
167169
ID: uuid.Nil.String(),
168170
Roles: rbac.Roles([]rbac.Role{
@@ -197,6 +199,7 @@ var (
197199
}.WithCachedASTValue()
198200

199201
subjectAutostart = rbac.Subject{
202+
Type: rbac.SubjectTypeAutostart,
200203
FriendlyName: "Autostart",
201204
ID: uuid.Nil.String(),
202205
Roles: rbac.Roles([]rbac.Role{
@@ -220,6 +223,7 @@ var (
220223

221224
// See unhanger package.
222225
subjectHangDetector = rbac.Subject{
226+
Type: rbac.SubjectTypeHangDetector,
223227
FriendlyName: "Hang Detector",
224228
ID: uuid.Nil.String(),
225229
Roles: rbac.Roles([]rbac.Role{
@@ -240,6 +244,7 @@ var (
240244

241245
// See cryptokeys package.
242246
subjectCryptoKeyRotator = rbac.Subject{
247+
Type: rbac.SubjectTypeCryptoKeyRotator,
243248
FriendlyName: "Crypto Key Rotator",
244249
ID: uuid.Nil.String(),
245250
Roles: rbac.Roles([]rbac.Role{
@@ -258,6 +263,7 @@ var (
258263

259264
// See cryptokeys package.
260265
subjectCryptoKeyReader = rbac.Subject{
266+
Type: rbac.SubjectTypeCryptoKeyReader,
261267
FriendlyName: "Crypto Key Reader",
262268
ID: uuid.Nil.String(),
263269
Roles: rbac.Roles([]rbac.Role{
@@ -275,6 +281,7 @@ var (
275281
}.WithCachedASTValue()
276282

277283
subjectNotifier = rbac.Subject{
284+
Type: rbac.SubjectTypeNotifier,
278285
FriendlyName: "Notifier",
279286
ID: uuid.Nil.String(),
280287
Roles: rbac.Roles([]rbac.Role{
@@ -295,6 +302,7 @@ var (
295302
}.WithCachedASTValue()
296303

297304
subjectResourceMonitor = rbac.Subject{
305+
Type: rbac.SubjectTypeResourceMonitor,
298306
FriendlyName: "Resource Monitor",
299307
ID: uuid.Nil.String(),
300308
Roles: rbac.Roles([]rbac.Role{
@@ -313,6 +321,7 @@ var (
313321
}.WithCachedASTValue()
314322

315323
subjectSystemRestricted = rbac.Subject{
324+
Type: rbac.SubjectTypeSystemRestricted,
316325
FriendlyName: "System",
317326
ID: uuid.Nil.String(),
318327
Roles: rbac.Roles([]rbac.Role{
@@ -347,6 +356,7 @@ var (
347356
}.WithCachedASTValue()
348357

349358
subjectSystemReadProvisionerDaemons = rbac.Subject{
359+
Type: rbac.SubjectTypeSystemReadProvisionerDaemons,
350360
FriendlyName: "Provisioner Daemons Reader",
351361
ID: uuid.Nil.String(),
352362
Roles: rbac.Roles([]rbac.Role{
@@ -364,6 +374,7 @@ var (
364374
}.WithCachedASTValue()
365375

366376
subjectPrebuildsOrchestrator = rbac.Subject{
377+
Type: rbac.SubjectTypePrebuildsOrchestrator,
367378
FriendlyName: "Prebuilds Orchestrator",
368379
ID: prebuilds.SystemUserID.String(),
369380
Roles: rbac.Roles([]rbac.Role{
@@ -388,59 +399,59 @@ var (
388399
// AsProvisionerd returns a context with an actor that has permissions required
389400
// for provisionerd to function.
390401
func AsProvisionerd(ctx context.Context) context.Context {
391-
return context.WithValue(ctx, authContextKey{}, subjectProvisionerd)
402+
return As(ctx, subjectProvisionerd)
392403
}
393404

394405
// AsAutostart returns a context with an actor that has permissions required
395406
// for autostart to function.
396407
func AsAutostart(ctx context.Context) context.Context {
397-
return context.WithValue(ctx, authContextKey{}, subjectAutostart)
408+
return As(ctx, subjectAutostart)
398409
}
399410

400411
// AsHangDetector returns a context with an actor that has permissions required
401412
// for unhanger.Detector to function.
402413
func AsHangDetector(ctx context.Context) context.Context {
403-
return context.WithValue(ctx, authContextKey{}, subjectHangDetector)
414+
return As(ctx, subjectHangDetector)
404415
}
405416

406417
// AsKeyRotator returns a context with an actor that has permissions required for rotating crypto keys.
407418
func AsKeyRotator(ctx context.Context) context.Context {
408-
return context.WithValue(ctx, authContextKey{}, subjectCryptoKeyRotator)
419+
return As(ctx, subjectCryptoKeyRotator)
409420
}
410421

411422
// AsKeyReader returns a context with an actor that has permissions required for reading crypto keys.
412423
func AsKeyReader(ctx context.Context) context.Context {
413-
return context.WithValue(ctx, authContextKey{}, subjectCryptoKeyReader)
424+
return As(ctx, subjectCryptoKeyReader)
414425
}
415426

416427
// AsNotifier returns a context with an actor that has permissions required for
417428
// creating/reading/updating/deleting notifications.
418429
func AsNotifier(ctx context.Context) context.Context {
419-
return context.WithValue(ctx, authContextKey{}, subjectNotifier)
430+
return As(ctx, subjectNotifier)
420431
}
421432

422433
// AsResourceMonitor returns a context with an actor that has permissions required for
423434
// updating resource monitors.
424435
func AsResourceMonitor(ctx context.Context) context.Context {
425-
return context.WithValue(ctx, authContextKey{}, subjectResourceMonitor)
436+
return As(ctx, subjectResourceMonitor)
426437
}
427438

428439
// AsSystemRestricted returns a context with an actor that has permissions
429440
// required for various system operations (login, logout, metrics cache).
430441
func AsSystemRestricted(ctx context.Context) context.Context {
431-
return context.WithValue(ctx, authContextKey{}, subjectSystemRestricted)
442+
return As(ctx, subjectSystemRestricted)
432443
}
433444

434445
// AsSystemReadProvisionerDaemons returns a context with an actor that has permissions
435446
// to read provisioner daemons.
436447
func AsSystemReadProvisionerDaemons(ctx context.Context) context.Context {
437-
return context.WithValue(ctx, authContextKey{}, subjectSystemReadProvisionerDaemons)
448+
return As(ctx, subjectSystemReadProvisionerDaemons)
438449
}
439450

440451
// AsPrebuildsOrchestrator returns a context with an actor that has permissions
441452
// to read orchestrator workspace prebuilds.
442453
func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
443-
return context.WithValue(ctx, authContextKey{}, subjectPrebuildsOrchestrator)
454+
return As(ctx, subjectPrebuildsOrchestrator)
444455
}
445456

446457
var AsRemoveActor = rbac.Subject{
@@ -458,6 +469,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context {
458469
// should be removed from the context.
459470
return context.WithValue(ctx, authContextKey{}, nil)
460471
}
472+
if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
473+
rlogger.WithAuthContext(actor)
474+
}
461475
return context.WithValue(ctx, authContextKey{}, actor)
462476
}
463477

coderd/database/queries.sql.go

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/users.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,10 @@ WHERE
300300
-- This function returns roles for authorization purposes. Implied member roles
301301
-- are included.
302302
SELECT
303-
-- username is returned just to help for logging purposes
303+
-- username and email are returned just to help for logging purposes
304304
-- status is used to enforce 'suspended' users, as all roles are ignored
305305
-- when suspended.
306-
id, username, status,
306+
id, username, status, email,
307307
-- All user roles, including their org roles.
308308
array_cat(
309309
-- All users are members

coderd/httpmw/apikey.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,9 @@ func UserRBACSubject(ctx context.Context, db database.Store, userID uuid.UUID, s
465465
}
466466

467467
actor := rbac.Subject{
468+
Type: rbac.SubjectTypeUser,
468469
FriendlyName: roles.Username,
470+
Email: roles.Email,
469471
ID: userID.String(),
470472
Roles: rbacRoles,
471473
Groups: roles.Groups,

coderd/httpmw/logger.go renamed to coderd/httpmw/loggermw/logger.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package httpmw
1+
package loggermw
22

33
import (
44
"context"
@@ -8,6 +8,7 @@ import (
88

99
"cdr.dev/slog"
1010
"github.com/coder/coder/v2/coderd/httpapi"
11+
"github.com/coder/coder/v2/coderd/rbac"
1112
"github.com/coder/coder/v2/coderd/tracing"
1213
)
1314

@@ -62,13 +63,15 @@ func Logger(log slog.Logger) func(next http.Handler) http.Handler {
6263
type RequestLogger interface {
6364
WithFields(fields ...slog.Field)
6465
WriteLog(ctx context.Context, status int)
66+
WithAuthContext(actor rbac.Subject)
6567
}
6668

6769
type SlogRequestLogger struct {
6870
log slog.Logger
6971
written bool
7072
message string
7173
start time.Time
74+
actors map[rbac.SubjectType]rbac.Subject
7275
}
7376

7477
var _ RequestLogger = &SlogRequestLogger{}
@@ -79,25 +82,53 @@ func NewRequestLogger(log slog.Logger, message string, start time.Time) RequestL
7982
written: false,
8083
message: message,
8184
start: start,
85+
actors: make(map[rbac.SubjectType]rbac.Subject),
8286
}
8387
}
8488

8589
func (c *SlogRequestLogger) WithFields(fields ...slog.Field) {
8690
c.log = c.log.With(fields...)
8791
}
8892

93+
func (c *SlogRequestLogger) WithAuthContext(actor rbac.Subject) {
94+
c.actors[actor.Type] = actor
95+
}
96+
97+
func (c *SlogRequestLogger) addAuthContextFields() {
98+
usr, ok := c.actors[rbac.SubjectTypeUser]
99+
if ok {
100+
c.log = c.log.With(
101+
slog.F("requestor_id", usr.ID),
102+
slog.F("requestor_name", usr.FriendlyName),
103+
slog.F("requestor_email", usr.Email),
104+
)
105+
} else if len(c.actors) > 0 {
106+
for _, v := range c.actors {
107+
c.log = c.log.With(
108+
slog.F("requestor_name", v.FriendlyName),
109+
)
110+
break
111+
}
112+
}
113+
}
114+
89115
func (c *SlogRequestLogger) WriteLog(ctx context.Context, status int) {
90116
if c.written {
91117
return
92118
}
93119
c.written = true
94120
end := time.Now()
95121

122+
// Right before we write the log, we try to find the user in the actors
123+
// and add the fields to the log.
124+
c.addAuthContextFields()
125+
96126
logger := c.log.With(
97127
slog.F("took", end.Sub(c.start)),
98128
slog.F("status_code", status),
99129
slog.F("latency_ms", float64(end.Sub(c.start)/time.Millisecond)),
100130
)
131+
101132
// We already capture most of this information in the span (minus
102133
// the response body which we don't want to capture anyways).
103134
tracing.RunWithoutSpan(ctx, func(ctx context.Context) {

coderd/httpmw/logger_internal_test.go renamed to coderd/httpmw/loggermw/logger_internal_test.go

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

33
import (
44
"context"
@@ -79,7 +79,7 @@ func TestLoggerMiddleware_SingleRequest(t *testing.T) {
7979

8080
require.Equal(t, sink.entries[0].Message, "GET")
8181

82-
fieldsMap := make(map[string]interface{})
82+
fieldsMap := make(map[string]any)
8383
for _, field := range sink.entries[0].Fields {
8484
fieldsMap[field.Name] = field.Value
8585
}

coderd/httpmw/loggermock/loggermock.go renamed to coderd/httpmw/loggermw/loggermock/loggermock.go

Lines changed: 14 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)