feat: extend request logs with auth & DB info (#17304)

Closes #16903

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,

coderd/httpmw/loggermw/logger.go

@@ -1,13 +1,17 @@
-package httpmw
+package loggermw
 
 import (
 	"context"
 	"fmt"
 	"net/http"
+	"sync"
 	"time"
 
+	"github.com/go-chi/chi/v5"
+
 	"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,6 +66,7 @@ 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 {
@@ -69,6 +74,9 @@ type SlogRequestLogger struct {
 	written bool
 	message string
 	start   time.Time
+	// Protects actors map for concurrent writes.
+	mu     sync.RWMutex
+	actors map[rbac.SubjectType]rbac.Subject
 }
 
 var _ RequestLogger = &SlogRequestLogger{}
@@ -79,6 +87,7 @@ func NewRequestLogger(log slog.Logger, message string, start time.Time) RequestL
 		written: false,
 		message: message,
 		start:   start,
+		actors:  make(map[rbac.SubjectType]rbac.Subject),
 	}
 }
 
@@ -86,6 +95,52 @@ func (c *SlogRequestLogger) WithFields(fields ...slog.Field) {
 	c.log = c.log.With(fields...)
 }
 
+func (c *SlogRequestLogger) WithAuthContext(actor rbac.Subject) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.actors[actor.Type] = actor
+}
+
+func (c *SlogRequestLogger) addAuthContextFields() {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+
+	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 there is no user, we log the requestor name for the first
+		// actor in a defined order.
+		for _, v := range actorLogOrder {
+			subj, ok := c.actors[v]
+			if !ok {
+				continue
+			}
+			c.log = c.log.With(
+				slog.F("requestor_name", subj.FriendlyName),
+			)
+			break
+		}
+	}
+}
+
+var actorLogOrder = []rbac.SubjectType{
+	rbac.SubjectTypeAutostart,
+	rbac.SubjectTypeCryptoKeyReader,
+	rbac.SubjectTypeCryptoKeyRotator,
+	rbac.SubjectTypeHangDetector,
+	rbac.SubjectTypeNotifier,
+	rbac.SubjectTypePrebuildsOrchestrator,
+	rbac.SubjectTypeProvisionerd,
+	rbac.SubjectTypeResourceMonitor,
+	rbac.SubjectTypeSystemReadProvisionerDaemons,
+	rbac.SubjectTypeSystemRestricted,
+}
+
 func (c *SlogRequestLogger) WriteLog(ctx context.Context, status int) {
 	if c.written {
 		return
@@ -93,11 +148,32 @@ func (c *SlogRequestLogger) WriteLog(ctx context.Context, status int) {
 	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)),
 	)
+
+	// If the request is routed, add the route parameters to the log.
+	if chiCtx := chi.RouteContext(ctx); chiCtx != nil {
+		urlParams := chiCtx.URLParams
+		routeParamsFields := make([]slog.Field, 0, len(urlParams.Keys))
+
+		for k, v := range urlParams.Keys {
+			if urlParams.Values[k] != "" {
+				routeParamsFields = append(routeParamsFields, slog.F("params_"+v, urlParams.Values[k]))
+			}
+		}
+
+		if len(routeParamsFields) > 0 {
+			logger = logger.With(routeParamsFields...)
+		}
+	}
+
 	// 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) {

coderd/httpmw/loggermw/logger_internal_test.go

@@ -1,13 +1,16 @@
-package httpmw
+package loggermw
 
 import (
 	"context"
 	"net/http"
 	"net/http/httptest"
+	"slices"
+	"strings"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/go-chi/chi/v5"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -79,7 +82,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
 	}
@@ -156,6 +159,140 @@ func TestLoggerMiddleware_WebSocket(t *testing.T) {
 	require.Len(t, sink.entries, 1, "log was written twice")
 }
 
+func TestRequestLogger_HTTPRouteParams(t *testing.T) {
+	t.Parallel()
+
+	sink := &fakeSink{}
+	logger := slog.Make(sink)
+	logger = logger.Leveled(slog.LevelDebug)
+
+	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
+	defer cancel()
+
+	chiCtx := chi.NewRouteContext()
+	chiCtx.URLParams.Add("workspace", "test-workspace")
+	chiCtx.URLParams.Add("agent", "test-agent")
+
+	ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx)
+
+	// Create a test handler to simulate an HTTP request
+	testHandler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.WriteHeader(http.StatusOK)
+		_, _ = rw.Write([]byte("OK"))
+	})
+
+	// Wrap the test handler with the Logger middleware
+	loggerMiddleware := Logger(logger)
+	wrappedHandler := loggerMiddleware(testHandler)
+
+	// Create a test HTTP request
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, "/test-path/}", nil)
+	require.NoError(t, err, "failed to create request")
+
+	sw := &tracing.StatusWriter{ResponseWriter: httptest.NewRecorder()}
+
+	// Serve the request
+	wrappedHandler.ServeHTTP(sw, req)
+
+	fieldsMap := make(map[string]any)
+	for _, field := range sink.entries[0].Fields {
+		fieldsMap[field.Name] = field.Value
+	}
+
+	// Check that the log contains the expected fields
+	requiredFields := []string{"workspace", "agent"}
+	for _, field := range requiredFields {
+		_, exists := fieldsMap["params_"+field]
+		require.True(t, exists, "field %q is missing in log fields", field)
+	}
+}
+
+func TestRequestLogger_RouteParamsLogging(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		params         map[string]string
+		expectedFields []string
+	}{
+		{
+			name:           "EmptyParams",
+			params:         map[string]string{},
+			expectedFields: []string{},
+		},
+		{
+			name: "SingleParam",
+			params: map[string]string{
+				"workspace": "test-workspace",
+			},
+			expectedFields: []string{"params_workspace"},
+		},
+		{
+			name: "MultipleParams",
+			params: map[string]string{
+				"workspace": "test-workspace",
+				"agent":     "test-agent",
+				"user":      "test-user",
+			},
+			expectedFields: []string{"params_workspace", "params_agent", "params_user"},
+		},
+		{
+			name: "EmptyValueParam",
+			params: map[string]string{
+				"workspace": "test-workspace",
+				"agent":     "",
+			},
+			expectedFields: []string{"params_workspace"},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			sink := &fakeSink{}
+			logger := slog.Make(sink)
+			logger = logger.Leveled(slog.LevelDebug)
+
+			// Create a route context with the test parameters
+			chiCtx := chi.NewRouteContext()
+			for key, value := range tt.params {
+				chiCtx.URLParams.Add(key, value)
+			}
+
+			ctx := context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx)
+			logCtx := NewRequestLogger(logger, "GET", time.Now())
+
+			// Write the log
+			logCtx.WriteLog(ctx, http.StatusOK)
+
+			require.Len(t, sink.entries, 1, "expected exactly one log entry")
+
+			// Convert fields to map for easier checking
+			fieldsMap := make(map[string]any)
+			for _, field := range sink.entries[0].Fields {
+				fieldsMap[field.Name] = field.Value
+			}
+
+			// Verify expected fields are present
+			for _, field := range tt.expectedFields {
+				value, exists := fieldsMap[field]
+				require.True(t, exists, "field %q should be present in log", field)
+				require.Equal(t, tt.params[strings.TrimPrefix(field, "params_")], value, "field %q has incorrect value", field)
+			}
+
+			// Verify no unexpected fields are present
+			for field := range fieldsMap {
+				if field == "took" || field == "status_code" || field == "latency_ms" {
+					continue // Skip standard fields
+				}
+				require.True(t, slices.Contains(tt.expectedFields, field), "unexpected field %q in log", field)
+			}
+		})
+	}
+}
+
 type fakeSink struct {
 	entries    []slog.SinkEntry
 	newEntries chan slog.SinkEntry

coderd/httpmw/loggermw/loggermock/loggermock.go

@@ -1,5 +1,5 @@
 // Code generated by MockGen. DO NOT EDIT.
-// Source: github.com/coder/coder/v2/coderd/httpmw (interfaces: RequestLogger)
+// Source: github.com/coder/coder/v2/coderd/httpmw/loggermw (interfaces: RequestLogger)
 //
 // Generated by this command:
 //
@@ -14,6 +14,7 @@ import (
 	reflect "reflect"
 
 	slog "cdr.dev/slog"
+	rbac "github.com/coder/coder/v2/coderd/rbac"
 	gomock "go.uber.org/mock/gomock"
 )
 
@@ -41,6 +42,18 @@ func (m *MockRequestLogger) EXPECT() *MockRequestLoggerMockRecorder {
 	return m.recorder
 }
 
+// WithAuthContext mocks base method.
+func (m *MockRequestLogger) WithAuthContext(actor rbac.Subject) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "WithAuthContext", actor)
+}
+
+// WithAuthContext indicates an expected call of WithAuthContext.
+func (mr *MockRequestLoggerMockRecorder) WithAuthContext(actor any) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithAuthContext", reflect.TypeOf((*MockRequestLogger)(nil).WithAuthContext), actor)
+}
+
 // WithFields mocks base method.
 func (m *MockRequestLogger) WithFields(fields ...slog.Field) {
 	m.ctrl.T.Helper()

coderd/httpmw/workspaceagentparam.go

@@ -6,8 +6,11 @@ import (
 
 	"github.com/go-chi/chi/v5"
 
+	"cdr.dev/slog"
+
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/httpapi"
+	"github.com/coder/coder/v2/coderd/httpmw/loggermw"
 	"github.com/coder/coder/v2/codersdk"
 )
 
@@ -81,6 +84,14 @@ func ExtractWorkspaceAgentParam(db database.Store) func(http.Handler) http.Handl
 
 			ctx = context.WithValue(ctx, workspaceAgentParamContextKey{}, agent)
 			chi.RouteContext(ctx).URLParams.Add("workspace", build.WorkspaceID.String())
+
+			if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
+				rlogger.WithFields(
+					slog.F("workspace_name", resource.Name),
+					slog.F("agent_name", agent.Name),
+				)
+			}
+
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
 	}

coderd/httpmw/workspaceparam.go

@@ -9,8 +9,11 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/google/uuid"
 
+	"cdr.dev/slog"
+
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/httpapi"
+	"github.com/coder/coder/v2/coderd/httpmw/loggermw"
 	"github.com/coder/coder/v2/codersdk"
 )
 
@@ -48,6 +51,11 @@ func ExtractWorkspaceParam(db database.Store) func(http.Handler) http.Handler {
 			}
 
 			ctx = context.WithValue(ctx, workspaceParamContextKey{}, workspace)
+
+			if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
+				rlogger.WithFields(slog.F("workspace_name", workspace.Name))
+			}
+
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
 	}
@@ -154,6 +162,13 @@ func ExtractWorkspaceAndAgentParam(db database.Store) func(http.Handler) http.Ha
 
 			ctx = context.WithValue(ctx, workspaceParamContextKey{}, workspace)
 			ctx = context.WithValue(ctx, workspaceAgentParamContextKey{}, agent)
+
+			if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
+				rlogger.WithFields(
+					slog.F("workspace_name", workspace.Name),
+					slog.F("agent_name", agent.Name),
+				)
+			}
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
 	}