feat(iam): OIDC provider audit trail (Phase 3e) (#9325)

* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity

When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based
(or omits it entirely) and the matched OIDC provider has policyClaim set,
mint a session whose effective policies come from that JWT claim instead
of from a server-side role mapping. Accepts string, comma-separated
string, or array shapes — MinIO-compatible behaviour for IDPs that
already attach policies to the user.

Trust-policy validation is skipped in claim-mode: the IDP is the sole
authority for both authentication and authorization, mirroring the
contract MinIO documents for its DummyRoleARN flow. Concrete-role mode
is unchanged and still requires the role definition + trust policy.

* fix(iam): trim policy-claim array elements + clean up stale comments

Three medium-priority cleanups gemini flagged on the claim-based path:

- extractClaimPolicies's array branch was leaving whitespace on each
  element while the string/comma-separated branch trimmed via
  splitPolicyClaimString. An IDP that emits ["readonly", " billing "]
  would create a "billing" policy lookup that didn't match the stored
  name. Trim every array element, drop empties.
- The "synthetic ARN keyed on the session name" comment was wrong —
  effectiveRoleArn here is the literal sentinel; it's the assumed-role
  ARN generated downstream that's session-keyed. Reword.
- The empty if/else block at the start of
  validateAssumeRoleWithWebIdentityRequest existed only to host a
  comment about deferred validation; the comment now lives in the
  function godoc and the empty branch is gone.

Addresses three gemini medium reviews on PR #9322.

* feat(iam): account-scoped OIDC providers

Add OIDCProviderRecord.AccountID enforcement: when a role lives in
account A, the OIDC provider validating the assume-role token must be
either global (AccountID="") or also live in account A. Cross-account
use is rejected at the IAM-manager layer before reaching the trust
policy validator.

OIDCProviderStore gains GetProviderByIssuerAndAccount; both the in-
memory and filer-backed stores implement it. Static-config-only
deployments are unaffected since they don't populate the store.

* fix(iam): account-scoped lookup for cross-account check

enforceProviderAccountScope was calling GetProviderByIssuer, which
returns the first match arbitrarily when multiple providers share an
issuer (one global + one per tenant is the canonical setup). On a
two-record collision the wrong record could come back first and
falsely reject a valid same-account or global-provider request.

Use GetProviderByIssuerAndAccount as the primary lookup so the
filter happens in the store. On miss, fall back to
GetProviderByIssuer purely to distinguish "issuer entirely unknown"
(let the STS layer reject) from "issuer registered in a different
account" (surface a precise cross-account error).

Addresses gemini high-priority review on PR #9323.

* feat(iam): opt-in session revocation via JTI blocklist

Add SessionRevocationStore (memory + filer implementations) and wire
it into the IAMManager.IsActionAllowed path so a revoked session is
rejected on the next signed request. Session JWTs now embed the
session id as the JTI claim, giving the blocklist a stable key
without requiring a second secret.

Operators who don't configure a store keep the existing fully-stateless
behaviour: every session stays valid until natural expiry. Operators
who do configure one accept one filer lookup per signed request in
exchange for being able to invalidate compromised tokens before
expiry. Revocation entries carry the original session expiry so the
blocklist self-trims via PurgeRevokedSessions.

* fix(iam): hash JTI filenames + paginate Purge with proper EOF handling

Three reviewer-flagged issues on the filer-backed revocation store:

1. Path traversal (security-medium): RevokeSession is exported and takes
   an arbitrary string. Using the JTI verbatim as a filename meant a
   caller could pass "../../etc/passwd" to write outside the basePath.
   SHA-1 hash the JTI to a fixed-width hex name; lookups still find the
   entry because Revoke and IsRevoked share the same hash function.

2. Purge swallowed errors. The inner `err` from stream.Recv() shadowed
   the outer err and the loop just broke on any failure, so a mid-stream
   gRPC error returned (count, nil) and the caller had no idea the
   purge was incomplete. Switch to errors.Is(io.EOF) for end-of-stream
   and propagate everything else.

3. Purge had a hardcoded 10000-entry cap. Stream-paginate via
   StartFromFileName so the operator-cron can clean a backlog larger
   than that without losing rows.

* feat(iam): OIDC provider audit trail

Emit one structured event per IAM-managed OIDC provider lifecycle
mutation (Create, Delete, Add/Remove ClientID, UpdateThumbprints,
Tag, Untag). Three sinks ship in-tree:

- GlogAuditSink — default; events become structured log lines.
- MemoryAuditSink — in-process buffer for tests / inspection.
- FilerAuditSink — durable, one file per event under
  /etc/iam/audit/oidc-providers (operator-overridable basePath).

Audit emission is best-effort: a failing sink never blocks an IAM
mutation that has already succeeded. Use-events (per token validation)
are intentionally not emitted yet — too hot for unconditional sinks.

* fix(iam): collision-free audit filenames + correct file mtime

Two issues gemini flagged on FilerAuditSink.Emit:

1. Filename was %d-%s.json (UnixNano + Type). Two mutations at the
   same nano against different ARNs (think batch script touching
   several providers) collided on the same path and the second
   CreateEntry failed silently. Append a short ARN hash to make the
   name unique per-event without leaking the ARN.

2. File Mtime/Crtime were set to time.Now() at write time. For audit
   integrity those should reflect the event's occurrence time so
   filer-level "ls -lt" output matches the contents.

Addresses three medium-priority gemini reviews on PR #9325.
This commit is contained in:
Chris Lu
2026-05-05 13:37:24 -07:00
committed by GitHub
parent 9d6a699b94
commit 9af1b212d3
3 changed files with 319 additions and 2 deletions
+41 -2
View File
@@ -32,6 +32,7 @@ type IAMManager struct {
roleStore RoleStore
userStore UserStore
oidcProviderStore OIDCProviderStore
oidcAuditSink OIDCProviderAuditSink
revocationStore SessionRevocationStore
filerAddressProvider func() string // Function to get current filer address
initialized bool
@@ -39,6 +40,31 @@ type IAMManager struct {
runtimePolicyNames map[string]struct{}
}
// SetOIDCProviderAuditSink configures the lifecycle event sink. When nil
// (default), GlogAuditSink is used so events still surface in logs.
func (m *IAMManager) SetOIDCProviderAuditSink(sink OIDCProviderAuditSink) {
m.oidcAuditSink = sink
}
// emitOIDCAudit logs a lifecycle event. Errors are swallowed: an audit
// failure must never block an IAM mutation that has already succeeded.
func (m *IAMManager) emitOIDCAudit(ctx context.Context, eventType OIDCProviderAuditEventType, arn, url string, detail map[string]string) {
sink := m.oidcAuditSink
if sink == nil {
sink = GlogAuditSink{}
}
event := &OIDCProviderAuditEvent{
Type: eventType,
ARN: arn,
URL: url,
Detail: detail,
OccurredAt: time.Now().UTC(),
}
if err := sink.Emit(ctx, event); err != nil {
glog.Warningf("OIDC audit emit %s for %s: %v", eventType, arn, err)
}
}
// SetSessionRevocationStore configures the per-session revocation list. When
// nil, RevokeSession returns an error and IsSessionRevoked is a no-op (every
// session is considered live until natural expiry). Operators who want
@@ -138,6 +164,7 @@ func (m *IAMManager) CreateOIDCProvider(ctx context.Context, rec *OIDCProviderRe
return err
}
m.refreshOIDCProvidersBestEffort(ctx, "CreateOIDCProvider", rec.ARN)
m.emitOIDCAudit(ctx, OIDCAuditEventCreated, rec.ARN, rec.URL, nil)
return nil
}
@@ -150,6 +177,7 @@ func (m *IAMManager) DeleteOIDCProvider(ctx context.Context, arn string) error {
return err
}
m.refreshOIDCProvidersBestEffort(ctx, "DeleteOIDCProvider", arn)
m.emitOIDCAudit(ctx, OIDCAuditEventDeleted, arn, "", nil)
return nil
}
@@ -180,6 +208,7 @@ func (m *IAMManager) AddClientIDToOIDCProvider(ctx context.Context, arn, clientI
return err
}
m.refreshOIDCProvidersBestEffort(ctx, "AddClientIDToOIDCProvider", arn)
m.emitOIDCAudit(ctx, OIDCAuditEventClientIDAdded, rec.ARN, rec.URL, map[string]string{"clientId": clientID})
return nil
}
@@ -208,6 +237,7 @@ func (m *IAMManager) RemoveClientIDFromOIDCProvider(ctx context.Context, arn, cl
return err
}
m.refreshOIDCProvidersBestEffort(ctx, "RemoveClientIDFromOIDCProvider", arn)
m.emitOIDCAudit(ctx, OIDCAuditEventClientIDRemoved, rec.ARN, rec.URL, map[string]string{"clientId": clientID})
return nil
}
@@ -235,6 +265,7 @@ func (m *IAMManager) UpdateOIDCProviderThumbprints(ctx context.Context, arn stri
return err
}
m.refreshOIDCProvidersBestEffort(ctx, "UpdateOIDCProviderThumbprints", arn)
m.emitOIDCAudit(ctx, OIDCAuditEventThumbprintsSet, rec.ARN, rec.URL, map[string]string{"count": fmt.Sprintf("%d", len(thumbprints))})
return nil
}
@@ -254,7 +285,11 @@ func (m *IAMManager) TagOIDCProvider(ctx context.Context, arn string, tags map[s
rec.Tags[k] = v
}
rec.UpdatedAt = time.Now().UTC()
return m.oidcProviderStore.StoreProvider(ctx, m.getFilerAddress(), rec)
if err := m.oidcProviderStore.StoreProvider(ctx, m.getFilerAddress(), rec); err != nil {
return err
}
m.emitOIDCAudit(ctx, OIDCAuditEventTagsAdded, rec.ARN, rec.URL, map[string]string{"count": fmt.Sprintf("%d", len(tags))})
return nil
}
// UntagOIDCProvider removes the named tags from the provider's tag set.
@@ -270,7 +305,11 @@ func (m *IAMManager) UntagOIDCProvider(ctx context.Context, arn string, keys []s
delete(rec.Tags, k)
}
rec.UpdatedAt = time.Now().UTC()
return m.oidcProviderStore.StoreProvider(ctx, m.getFilerAddress(), rec)
if err := m.oidcProviderStore.StoreProvider(ctx, m.getFilerAddress(), rec); err != nil {
return err
}
m.emitOIDCAudit(ctx, OIDCAuditEventTagsRemoved, rec.ARN, rec.URL, map[string]string{"count": fmt.Sprintf("%d", len(keys))})
return nil
}
// validateOIDCProviderRecord enforces the invariants AWS imposes on the
+173
View File
@@ -0,0 +1,173 @@
package integration
import (
"context"
"crypto/sha1"
"encoding/hex"
"encoding/json"
"fmt"
"strings"
"sync"
"time"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/pb"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"google.golang.org/grpc"
)
// OIDCProviderAuditEventType enumerates the lifecycle events emitted when an
// IAM-managed OIDC provider record is mutated. Use-events (every successful
// token validation) are intentionally not emitted by the lifecycle path:
// they're too hot to stream into a filer-backed sink without spikes. A
// future PR can add a sampled-use sink behind an explicit opt-in flag.
type OIDCProviderAuditEventType string
const (
OIDCAuditEventCreated OIDCProviderAuditEventType = "Create"
OIDCAuditEventDeleted OIDCProviderAuditEventType = "Delete"
OIDCAuditEventClientIDAdded OIDCProviderAuditEventType = "AddClientID"
OIDCAuditEventClientIDRemoved OIDCProviderAuditEventType = "RemoveClientID"
OIDCAuditEventThumbprintsSet OIDCProviderAuditEventType = "UpdateThumbprints"
OIDCAuditEventTagsAdded OIDCProviderAuditEventType = "Tag"
OIDCAuditEventTagsRemoved OIDCProviderAuditEventType = "Untag"
)
// OIDCProviderAuditEvent is the payload emitted for each lifecycle event.
type OIDCProviderAuditEvent struct {
Type OIDCProviderAuditEventType `json:"type"`
ARN string `json:"arn"`
URL string `json:"url,omitempty"`
Detail map[string]string `json:"detail,omitempty"`
OccurredAt time.Time `json:"occurredAt"`
}
// OIDCProviderAuditSink consumes lifecycle events. Implementations must be
// safe for concurrent use; emit happens with the IAM manager's mutation lock
// held, so a slow sink slows the IAM API.
type OIDCProviderAuditSink interface {
Emit(ctx context.Context, event *OIDCProviderAuditEvent) error
}
// GlogAuditSink writes one structured log line per event. Always-on default
// when an explicit sink isn't configured — events still appear in stdout/the
// log aggregator, just not in a queryable record.
type GlogAuditSink struct{}
func (GlogAuditSink) Emit(_ context.Context, event *OIDCProviderAuditEvent) error {
if event == nil {
return nil
}
data, err := json.Marshal(event)
if err != nil {
glog.V(0).Infof("oidc-audit: %s arn=%s err-marshal=%v", event.Type, event.ARN, err)
return nil
}
glog.V(0).Infof("oidc-audit: %s", string(data))
return nil
}
// MemoryAuditSink keeps events in process memory for tests and short-lived
// inspection. Not durable; drop entries by reading from Events() and
// discarding.
type MemoryAuditSink struct {
mu sync.Mutex
events []*OIDCProviderAuditEvent
}
func NewMemoryAuditSink() *MemoryAuditSink {
return &MemoryAuditSink{}
}
func (m *MemoryAuditSink) Emit(_ context.Context, event *OIDCProviderAuditEvent) error {
m.mu.Lock()
defer m.mu.Unlock()
cp := *event
m.events = append(m.events, &cp)
return nil
}
// Events returns a copy of the captured event log so tests can assert.
func (m *MemoryAuditSink) Events() []*OIDCProviderAuditEvent {
m.mu.Lock()
defer m.mu.Unlock()
out := make([]*OIDCProviderAuditEvent, len(m.events))
copy(out, m.events)
return out
}
// FilerAuditSink appends event records as separate files under a filer
// directory. Each event is its own file so concurrent writers don't conflict;
// the filename is `<unixnano>-<type>-<arnhash>.json`. For higher volumes,
// switch to an append-only journal — the contract is just `Emit`.
type FilerAuditSink struct {
grpcDialOption grpc.DialOption
basePath string
filerAddressProvider func() string
}
// NewFilerAuditSink returns a filer-backed sink. Default basePath
// `/etc/iam/audit/oidc-providers` keeps audit records out of the active
// IAM data directories.
func NewFilerAuditSink(config map[string]interface{}, filerAddressProvider func() string) *FilerAuditSink {
sink := &FilerAuditSink{
basePath: "/etc/iam/audit/oidc-providers",
filerAddressProvider: filerAddressProvider,
}
if config != nil {
if bp, ok := config["basePath"].(string); ok && bp != "" {
sink.basePath = strings.TrimSuffix(bp, "/")
}
}
return sink
}
func (f *FilerAuditSink) resolveFilerAddress() string {
if f.filerAddressProvider != nil {
return f.filerAddressProvider()
}
return ""
}
func (f *FilerAuditSink) Emit(ctx context.Context, event *OIDCProviderAuditEvent) error {
addr := f.resolveFilerAddress()
if addr == "" {
return fmt.Errorf("filer address not available")
}
if event == nil {
return nil
}
if event.OccurredAt.IsZero() {
event.OccurredAt = time.Now().UTC()
}
data, err := json.MarshalIndent(event, "", " ")
if err != nil {
return fmt.Errorf("marshal audit event: %v", err)
}
// Filename: <unixnano>-<type>-<arnhash>.json. Two events at the same
// nano against the same ARN+type are vanishingly rare, but two events
// at the same nano against different ARNs are perfectly plausible
// (audit-via-batch script). The ARN hash gives us an effectively-
// collision-free name without leaking the ARN into the filer path.
arnSum := sha1.Sum([]byte(event.ARN))
name := fmt.Sprintf("%d-%s-%s.json", event.OccurredAt.UnixNano(), event.Type, hex.EncodeToString(arnSum[:8]))
occurredAt := event.OccurredAt.Unix()
return pb.WithGrpcFilerClient(false, 0, pb.ServerAddress(addr), f.grpcDialOption, func(client filer_pb.SeaweedFilerClient) error {
_, err := client.CreateEntry(ctx, &filer_pb.CreateEntryRequest{
Directory: f.basePath,
Entry: &filer_pb.Entry{
Name: name,
IsDirectory: false,
Attributes: &filer_pb.FuseAttributes{
// Reflect the event's occurrence time so file metadata
// matches the audit record itself, not the filer write.
Mtime: occurredAt,
Crtime: occurredAt,
FileMode: uint32(0o600),
},
Content: data,
},
})
return err
})
}
@@ -0,0 +1,105 @@
package integration
import (
"context"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
)
func newAuditableManager(t *testing.T) (*IAMManager, *MemoryAuditSink) {
t.Helper()
mgr := NewIAMManager()
sink := NewMemoryAuditSink()
cfg := &IAMConfig{
STS: &sts.STSConfig{
TokenDuration: sts.FlexibleDuration{Duration: time.Hour},
MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
Issuer: "test-sts",
SigningKey: []byte("test-signing-key-32-characters-long"),
AccountId: "111122223333",
},
Policy: &policy.PolicyEngineConfig{DefaultEffect: "Deny", StoreType: "memory"},
Roles: &RoleStoreConfig{StoreType: "memory"},
}
if err := mgr.Initialize(cfg, func() string { return "localhost:8888" }); err != nil {
t.Fatalf("Initialize: %v", err)
}
mgr.SetOIDCProviderAuditSink(sink)
return mgr, sink
}
func TestOIDCAuditLifecycleEventsAreEmitted(t *testing.T) {
mgr, sink := newAuditableManager(t)
ctx := context.Background()
rec := &OIDCProviderRecord{
AccountID: "111122223333",
ARN: "arn:aws:iam::111122223333:oidc-provider/idp.example",
URL: "https://idp.example",
ClientIDs: []string{"x"},
}
if err := mgr.CreateOIDCProvider(ctx, rec); err != nil {
t.Fatalf("Create: %v", err)
}
if err := mgr.AddClientIDToOIDCProvider(ctx, rec.ARN, "y"); err != nil {
t.Fatalf("AddClientID: %v", err)
}
if err := mgr.RemoveClientIDFromOIDCProvider(ctx, rec.ARN, "y"); err != nil {
t.Fatalf("RemoveClientID: %v", err)
}
if err := mgr.UpdateOIDCProviderThumbprints(ctx, rec.ARN, []string{"0000000000000000000000000000000000000000"}); err != nil {
t.Fatalf("UpdateThumbprints: %v", err)
}
if err := mgr.TagOIDCProvider(ctx, rec.ARN, map[string]string{"team": "infra"}); err != nil {
t.Fatalf("Tag: %v", err)
}
if err := mgr.UntagOIDCProvider(ctx, rec.ARN, []string{"team"}); err != nil {
t.Fatalf("Untag: %v", err)
}
if err := mgr.DeleteOIDCProvider(ctx, rec.ARN); err != nil {
t.Fatalf("Delete: %v", err)
}
want := []OIDCProviderAuditEventType{
OIDCAuditEventCreated,
OIDCAuditEventClientIDAdded,
OIDCAuditEventClientIDRemoved,
OIDCAuditEventThumbprintsSet,
OIDCAuditEventTagsAdded,
OIDCAuditEventTagsRemoved,
OIDCAuditEventDeleted,
}
events := sink.Events()
if len(events) != len(want) {
t.Fatalf("expected %d events, got %d (%+v)", len(want), len(events), events)
}
for i, e := range events {
if e.Type != want[i] {
t.Errorf("event %d: type=%s want=%s", i, e.Type, want[i])
}
if e.ARN != rec.ARN {
t.Errorf("event %d: ARN=%s want=%s", i, e.ARN, rec.ARN)
}
if e.OccurredAt.IsZero() {
t.Errorf("event %d has zero OccurredAt", i)
}
}
}
func TestOIDCAuditDefaultsToGlogSink(t *testing.T) {
// No SetOIDCProviderAuditSink call — should default to glog and not panic.
mgr, _ := newAuditableManager(t)
mgr.SetOIDCProviderAuditSink(nil)
rec := &OIDCProviderRecord{
AccountID: "111122223333",
ARN: "arn:aws:iam::111122223333:oidc-provider/idp.example",
URL: "https://idp.example",
ClientIDs: []string{"x"},
}
if err := mgr.CreateOIDCProvider(context.Background(), rec); err != nil {
t.Fatalf("Create with default sink: %v", err)
}
}