extended the user query to include email and added a handler for logger

injection
coder · ibetitsmike · Apr 15, 2025 · Apr 9, 2025 · Apr 10, 2025 · Apr 10, 2025
commit 1239656ab4a9e1bd13d82eb344956b7941cb3e5d
diff --git a/Makefile b/Makefile
@@ -582,7 +582,7 @@ GEN_FILES := \
 	coderd/database/pubsub/psmock/psmock.go \
 	agent/agentcontainers/acmock/acmock.go \
 	agent/agentcontainers/dcspec/dcspec_gen.go \
-	coderd/httpmw/loggermock/loggermock.go
+	coderd/httpmw/loggermw/loggermock/loggermock.go
 
 # all gen targets should be added here and to gen/mark-fresh
 gen: gen/db gen/golden-files $(GEN_FILES)
@@ -631,8 +631,7 @@ gen/mark-fresh:
 		coderd/database/pubsub/psmock/psmock.go \
 		agent/agentcontainers/acmock/acmock.go \
 		agent/agentcontainers/dcspec/dcspec_gen.go \
-		coderd/httpmw/loggermock/loggermock.go \
-		"
+		coderd/httpmw/loggermw/loggermock/loggermock.go
 
 	for file in $$files; do
 		echo "$$file"
@@ -671,8 +670,8 @@ agent/agentcontainers/acmock/acmock.go: agent/agentcontainers/containers.go
 	go generate ./agent/agentcontainers/acmock/
 	touch "$@"
 
-coderd/httpmw/loggermock/loggermock.go: coderd/httpmw/logger.go
-	go generate ./coderd/httpmw/loggermock/
+coderd/httpmw/loggermw/loggermock/loggermock.go: coderd/httpmw/loggermw/logger.go
+	go generate ./coderd/httpmw/loggermw/loggermock/
 	touch "$@"
 
 agent/agentcontainers/dcspec/dcspec_gen.go: \

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -65,6 +65,7 @@ import (
 	"github.com/coder/coder/v2/coderd/healthcheck/derphealth"
 	"github.com/coder/coder/v2/coderd/httpapi"
 	"github.com/coder/coder/v2/coderd/httpmw"
+	"github.com/coder/coder/v2/coderd/httpmw/loggermw"
 	"github.com/coder/coder/v2/coderd/metricscache"
 	"github.com/coder/coder/v2/coderd/notifications"
 	"github.com/coder/coder/v2/coderd/portsharing"
@@ -810,7 +811,7 @@ func New(options *Options) *API {
 		tracing.Middleware(api.TracerProvider),
 		httpmw.AttachRequestID,
 		httpmw.ExtractRealIP(api.RealIPConfig),
-		httpmw.Logger(api.Logger),
+		loggermw.Logger(api.Logger),
 		singleSlashMW,
 		rolestore.CustomRoleMW,
 		prometheusMW,

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -25,6 +25,7 @@ import (
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
+	"github.com/coder/coder/v2/coderd/httpmw/loggermw"
 	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/coderd/util/slice"
 	"github.com/coder/coder/v2/provisionersdk"
@@ -163,6 +164,7 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
 
 var (
 	subjectProvisionerd = rbac.Subject{
+		Type:         rbac.SubjectTypeProvisionerd,
 		FriendlyName: "Provisioner Daemon",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -197,6 +199,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectAutostart = rbac.Subject{
+		Type:         rbac.SubjectTypeAutostart,
 		FriendlyName: "Autostart",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -220,6 +223,7 @@ var (
 
 	// See unhanger package.
 	subjectHangDetector = rbac.Subject{
+		Type:         rbac.SubjectTypeHangDetector,
 		FriendlyName: "Hang Detector",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -240,6 +244,7 @@ var (
 
 	// See cryptokeys package.
 	subjectCryptoKeyRotator = rbac.Subject{
+		Type:         rbac.SubjectTypeCryptoKeyRotator,
 		FriendlyName: "Crypto Key Rotator",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -258,6 +263,7 @@ var (
 
 	// See cryptokeys package.
 	subjectCryptoKeyReader = rbac.Subject{
+		Type:         rbac.SubjectTypeCryptoKeyReader,
 		FriendlyName: "Crypto Key Reader",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -275,6 +281,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectNotifier = rbac.Subject{
+		Type:         rbac.SubjectTypeNotifier,
 		FriendlyName: "Notifier",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -295,6 +302,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectResourceMonitor = rbac.Subject{
+		Type:         rbac.SubjectTypeResourceMonitor,
 		FriendlyName: "Resource Monitor",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -313,6 +321,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectSystemRestricted = rbac.Subject{
+		Type:         rbac.SubjectTypeSystemRestricted,
 		FriendlyName: "System",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -347,6 +356,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectSystemReadProvisionerDaemons = rbac.Subject{
+		Type:         rbac.SubjectTypeSystemReadProvisionerDaemons,
 		FriendlyName: "Provisioner Daemons Reader",
 		ID:           uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -364,6 +374,7 @@ var (
 	}.WithCachedASTValue()
 
 	subjectPrebuildsOrchestrator = rbac.Subject{
+		Type:         rbac.SubjectTypePrebuildsOrchestrator,
 		FriendlyName: "Prebuilds Orchestrator",
 		ID:           prebuilds.SystemUserID.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -388,59 +399,59 @@ var (
 // AsProvisionerd returns a context with an actor that has permissions required
 // for provisionerd to function.
 func AsProvisionerd(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectProvisionerd)
+	return As(ctx, subjectProvisionerd)
 }
 
 // AsAutostart returns a context with an actor that has permissions required
 // for autostart to function.
 func AsAutostart(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectAutostart)
+	return As(ctx, subjectAutostart)
 }
 
 // AsHangDetector returns a context with an actor that has permissions required
 // for unhanger.Detector to function.
 func AsHangDetector(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectHangDetector)
+	return As(ctx, subjectHangDetector)
 }
 
 // AsKeyRotator returns a context with an actor that has permissions required for rotating crypto keys.
 func AsKeyRotator(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectCryptoKeyRotator)
+	return As(ctx, subjectCryptoKeyRotator)
 }
 
 // AsKeyReader returns a context with an actor that has permissions required for reading crypto keys.
 func AsKeyReader(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectCryptoKeyReader)
+	return As(ctx, subjectCryptoKeyReader)
 }
 
 // AsNotifier returns a context with an actor that has permissions required for
 // creating/reading/updating/deleting notifications.
 func AsNotifier(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectNotifier)
+	return As(ctx, subjectNotifier)
 }
 
 // AsResourceMonitor returns a context with an actor that has permissions required for
 // updating resource monitors.
 func AsResourceMonitor(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectResourceMonitor)
+	return As(ctx, subjectResourceMonitor)
 }
 
 // AsSystemRestricted returns a context with an actor that has permissions
 // required for various system operations (login, logout, metrics cache).
 func AsSystemRestricted(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectSystemRestricted)
+	return As(ctx, subjectSystemRestricted)
 }
 
 // AsSystemReadProvisionerDaemons returns a context with an actor that has permissions
 // to read provisioner daemons.
 func AsSystemReadProvisionerDaemons(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectSystemReadProvisionerDaemons)
+	return As(ctx, subjectSystemReadProvisionerDaemons)
 }
 
 // AsPrebuildsOrchestrator returns a context with an actor that has permissions
 // to read orchestrator workspace prebuilds.
 func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authContextKey{}, subjectPrebuildsOrchestrator)
+	return As(ctx, subjectPrebuildsOrchestrator)
 }
 
 var AsRemoveActor = rbac.Subject{
@@ -458,6 +469,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context {
 		// should be removed from the context.
 		return context.WithValue(ctx, authContextKey{}, nil)
 	}
+	if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
+		rlogger.WithAuthContext(actor)
+	}
 	return context.WithValue(ctx, authContextKey{}, actor)
 }
 

diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
@@ -300,10 +300,10 @@ WHERE
 -- This function returns roles for authorization purposes. Implied member roles
 -- are included.
 SELECT
-	-- username is returned just to help for logging purposes
+	-- username and email are returned just to help for logging purposes
 	-- status is used to enforce 'suspended' users, as all roles are ignored
 	--	when suspended.
-	id, username, status,
+	id, username, status, email,
 	-- All user roles, including their org roles.
 	array_cat(
 		-- All users are members

diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -465,7 +465,9 @@ func UserRBACSubject(ctx context.Context, db database.Store, userID uuid.UUID, s
 	}
 
 	actor := rbac.Subject{
+		Type:         rbac.SubjectTypeUser,
 		FriendlyName: roles.Username,
+		Email:        roles.Email,
 		ID:           userID.String(),
 		Roles:        rbacRoles,
 		Groups:       roles.Groups,

diff --git a/coderd/httpmw/logger.go → coderd/httpmw/loggermw/logger.go b/coderd/httpmw/logger.go → coderd/httpmw/loggermw/logger.go
@@ -1,4 +1,4 @@
-package httpmw
+package loggermw
 
 import (
 	"context"
@@ -8,6 +8,7 @@ import (
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/v2/coderd/httpapi"
+	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/coderd/tracing"
 )
 
@@ -62,13 +63,15 @@ func Logger(log slog.Logger) func(next http.Handler) http.Handler {
 type RequestLogger interface {
 	WithFields(fields ...slog.Field)
 	WriteLog(ctx context.Context, status int)
+	WithAuthContext(actor rbac.Subject)
 }
 
 type SlogRequestLogger struct {
 	log     slog.Logger
 	written bool
 	message string
 	start   time.Time
+	actors  map[rbac.SubjectType]rbac.Subject
 }
 
 var _ RequestLogger = &SlogRequestLogger{}
@@ -79,25 +82,53 @@ func NewRequestLogger(log slog.Logger, message string, start time.Time) RequestL
 		written: false,
 		message: message,
 		start:   start,
+		actors:  make(map[rbac.SubjectType]rbac.Subject),
 	}
 }
 
 func (c *SlogRequestLogger) WithFields(fields ...slog.Field) {
 	c.log = c.log.With(fields...)
 }
 
+func (c *SlogRequestLogger) WithAuthContext(actor rbac.Subject) {
+	c.actors[actor.Type] = actor
+}
+
+func (c *SlogRequestLogger) addAuthContextFields() {
+	usr, ok := c.actors[rbac.SubjectTypeUser]
+	if ok {
+		c.log = c.log.With(
+			slog.F("requestor_id", usr.ID),
+			slog.F("requestor_name", usr.FriendlyName),
+			slog.F("requestor_email", usr.Email),
+		)
+	} else if len(c.actors) > 0 {
+		for _, v := range c.actors {
+			c.log = c.log.With(
+				slog.F("requestor_name", v.FriendlyName),
+			)
+			break
+		}
+	}
+}
+
 func (c *SlogRequestLogger) WriteLog(ctx context.Context, status int) {
 	if c.written {
 		return
 	}
 	c.written = true
 	end := time.Now()
 
+	// Right before we write the log, we try to find the user in the actors
+	// and add the fields to the log.
+	c.addAuthContextFields()
+
 	logger := c.log.With(
 		slog.F("took", end.Sub(c.start)),
 		slog.F("status_code", status),
 		slog.F("latency_ms", float64(end.Sub(c.start)/time.Millisecond)),
 	)
+
 	// We already capture most of this information in the span (minus
 	// the response body which we don't want to capture anyways).
 	tracing.RunWithoutSpan(ctx, func(ctx context.Context) {

diff --git a/coderd/httpmw/logger_internal_test.go → ...d/httpmw/loggermw/logger_internal_test.go b/coderd/httpmw/logger_internal_test.go → ...d/httpmw/loggermw/logger_internal_test.go
@@ -1,4 +1,4 @@
-package httpmw
+package loggermw
 
 import (
 	"context"
@@ -79,7 +79,7 @@ func TestLoggerMiddleware_SingleRequest(t *testing.T) {
 
 	require.Equal(t, sink.entries[0].Message, "GET")
 
-	fieldsMap := make(map[string]interface{})
+	fieldsMap := make(map[string]any)
 	for _, field := range sink.entries[0].Fields {
 		fieldsMap[field.Name] = field.Value
 	}

diff --git a/coderd/httpmw/loggermock/loggermock.go → .../httpmw/loggermw/loggermock/loggermock.go b/coderd/httpmw/loggermock/loggermock.go → .../httpmw/loggermw/loggermock/loggermock.go