diff --git a/weed/s3api/s3_constants/header.go b/weed/s3api/s3_constants/header.go index 71e56bff3..9cfc11f35 100644 --- a/weed/s3api/s3_constants/header.go +++ b/weed/s3api/s3_constants/header.go @@ -20,6 +20,7 @@ import ( "context" "net/http" "strings" + "sync/atomic" "github.com/gorilla/mux" ) @@ -248,24 +249,65 @@ type contextKey string const ( contextKeyIdentityName contextKey = "s3-identity-name" contextKeyIdentityObject contextKey = "s3-identity-object" + contextKeyIdentityHolder contextKey = "s3-identity-holder" ) +// identityHolder is a mutable container for the authenticated identity name, +// stored by pointer in the request context. Authentication runs as an inner +// handler that records the identity with r.WithContext, which returns a request +// copy; that copy's new context values are invisible to outer middleware (e.g. +// the audit logger) holding an earlier copy of the request. A pointer to a +// holder installed before authentication is shared across all copies, so the +// name written by the inner handler is readable by the outer middleware. +type identityHolder struct { + name atomic.Pointer[string] +} + +// EnsureIdentityHolder attaches a mutable identity holder to the request context +// if one is not already present, returning the (possibly new) request. Install +// it before authentication so the authenticated requester remains recoverable +// from the original request, e.g. when emitting an audit entry for a handler +// that does not log the requester itself. +func EnsureIdentityHolder(r *http.Request) *http.Request { + if r == nil { + return nil + } + if h, ok := r.Context().Value(contextKeyIdentityHolder).(*identityHolder); ok && h != nil { + return r + } + return r.WithContext(context.WithValue(r.Context(), contextKeyIdentityHolder, &identityHolder{})) +} + // SetIdentityNameInContext stores the authenticated identity name in the request context // This is the secure way to propagate identity - headers can be spoofed, context cannot func SetIdentityNameInContext(ctx context.Context, identityName string) context.Context { - if identityName != "" { - return context.WithValue(ctx, contextKeyIdentityName, identityName) + if identityName == "" { + return ctx } - return ctx + // Also record into the mutable holder (if installed) so the name survives the + // request-context copy and stays visible to outer middleware such as the + // audit logger. + if h, ok := ctx.Value(contextKeyIdentityHolder).(*identityHolder); ok && h != nil { + h.name.Store(&identityName) + } + return context.WithValue(ctx, contextKeyIdentityName, identityName) } // GetIdentityNameFromContext retrieves the authenticated identity name from the request context // Returns empty string if no identity is set (unauthenticated request) // This is the secure way to retrieve identity - never read from headers directly func GetIdentityNameFromContext(r *http.Request) string { - if name, ok := r.Context().Value(contextKeyIdentityName).(string); ok { + // Prefer the per-request context value; fall back to the mutable holder for + // callers (e.g. the audit middleware) that hold a copy of the request taken + // before authentication set the identity. + if name, ok := r.Context().Value(contextKeyIdentityName).(string); ok && name != "" { return name } + if h, ok := r.Context().Value(contextKeyIdentityHolder).(*identityHolder); ok && h != nil { + if name := h.name.Load(); name != nil { + return *name + } + } return "" } diff --git a/weed/s3api/s3_constants/header_test.go b/weed/s3api/s3_constants/header_test.go index 4b1c1b8ca..17769c6cd 100644 --- a/weed/s3api/s3_constants/header_test.go +++ b/weed/s3api/s3_constants/header_test.go @@ -1,6 +1,8 @@ package s3_constants import ( + "net/http" + "net/http/httptest" "testing" ) @@ -129,3 +131,55 @@ func TestRemoveDuplicateSlashes(t *testing.T) { }) } } + +// TestIdentityHolderPropagation reproduces the audit-log middleware chain: an +// outer handler installs the holder, an inner handler authenticates and records +// the identity on a request copy, and the outer handler must still recover the +// requester from its own (earlier) copy of the request. +func TestIdentityHolderPropagation(t *testing.T) { + outer := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) + + // Before the holder is installed there is no identity to recover. + if got := GetIdentityNameFromContext(outer); got != "" { + t.Fatalf("unauthenticated request reports requester %q, want empty", got) + } + + outer = EnsureIdentityHolder(outer) + + // The inner handler sets the identity on a copy, mirroring how auth wraps + // the request with r.WithContext before invoking the next handler. + innerCtx := SetIdentityNameInContext(outer.Context(), "admin") + inner := outer.WithContext(innerCtx) + + if got := GetIdentityNameFromContext(inner); got != "admin" { + t.Errorf("inner request requester = %q, want admin", got) + } + if got := GetIdentityNameFromContext(outer); got != "admin" { + t.Errorf("outer request requester = %q, want admin (must propagate via holder)", got) + } +} + +func TestEnsureIdentityHolderIdempotent(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) + + first := EnsureIdentityHolder(req) + if first == req { + t.Fatal("EnsureIdentityHolder should return a new request when no holder is present") + } + + again := EnsureIdentityHolder(first) + if again != first { + t.Error("EnsureIdentityHolder should be idempotent when a holder is already present") + } +} + +func TestGetIdentityNameFromContextWithoutHolder(t *testing.T) { + // Without a holder, the per-request context value is still honored so paths + // that set identity without installing a holder keep working. + req := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) + req = req.WithContext(SetIdentityNameInContext(req.Context(), "admin")) + + if got := GetIdentityNameFromContext(req); got != "admin" { + t.Errorf("requester = %q, want admin", got) + } +} diff --git a/weed/s3api/s3err/audit_fluent_test.go b/weed/s3api/s3err/audit_fluent_test.go index 710a705cc..d2e3747ca 100644 --- a/weed/s3api/s3err/audit_fluent_test.go +++ b/weed/s3api/s3err/audit_fluent_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util/request_id" "github.com/stretchr/testify/assert" ) @@ -86,6 +87,33 @@ func TestGetAccessLogRemoteIP(t *testing.T) { } } +// TestGetAccessLogRequesterFromFallback reproduces the fallback audit path for +// GET/HEAD/IAM operations: authentication records the requester on a request +// copy the track() middleware never sees, yet the fallback audit entry (built +// from the original request) must still report the authenticated user. +func TestGetAccessLogRequesterFromFallback(t *testing.T) { + // track() installs the holder before authentication runs. + outer := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "/bucket/object", nil)) + + // auth records the identity on a copy and hands that copy to the handler; + // the copy itself is discarded once the handler returns. + _ = outer.WithContext(s3_constants.SetIdentityNameInContext(outer.Context(), "admin")) + + // The handler returned without logging, so track() builds the fallback entry + // from the original request. + log := GetAccessLog(outer, http.StatusOK, ErrNone) + + assert.Equal(t, "admin", log.Requester, "fallback audit entry must report the authenticated requester") +} + +func TestGetAccessLogRequesterAnonymous(t *testing.T) { + req := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "/bucket/object", nil)) + + log := GetAccessLog(req, http.StatusOK, ErrNone) + + assert.Empty(t, log.Requester, "anonymous request must not report a requester") +} + func TestAuditTrackingFlag(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) assert.False(t, AuditAlreadyLogged(req), "untracked request reports not logged") diff --git a/weed/s3api/stats.go b/weed/s3api/stats.go index 00ff38dad..5bd6b89d6 100644 --- a/weed/s3api/stats.go +++ b/weed/s3api/stats.go @@ -25,6 +25,10 @@ func track(f http.HandlerFunc, action string) http.HandlerFunc { // PostLog directly mark it; we emit a fallback entry afterward for // handlers (e.g. successful GET/HEAD object) that don't. r = s3err.EnsureAuditTracking(r) + // Attach a mutable identity holder before authentication so the fallback + // entry can report the requester even though auth records it on a + // request copy this middleware never sees. + r = s3_constants.EnsureIdentityHolder(r) start := time.Now() f(recorder, r) if recorder.Status == http.StatusForbidden {