mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-20 06:52:24 +00:00
fix(s3/audit): populate requester for GET/HEAD/IAM operations (#9581)
Authentication records the identity with r.WithContext, which returns a request copy. Handlers that log their own audit entry (PUT, DELETE, tagging) see it, but GET/HEAD object and IAM operations rely on track()'s fallback entry, which is built from the original request the auth copy never reached - so requester came out empty. Install a mutable identity holder on the request before authentication and have SetIdentityNameInContext record into it. The holder is shared by pointer across every request copy, so the fallback entry recovers the authenticated requester. The per-request context value still takes precedence, so nothing changes for handlers that see the auth copy.
This commit is contained in:
@@ -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 ""
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user