mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-29 19:25:35 +00:00
be29f44d87
* s3: record requester identity before the authz verdict for audit Identity was only stored in request context on the success branch, so denied requests reached WriteErrorResponse without requester attribution and audit entries had empty requester/requester_arn/requester_identity. Authentication failures still resolve no identity, so unauthenticated denials stay unattributed. Fixes #11474 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: keep the resolved identity through authz denial in Auth Review follow-up on #11479 (devin): authRequest discarded the identity on every error, so a request that authenticated fine but failed the action check still reached handleAuthResult with no identity and the deny path could not audit a requester. Auth now calls authRequestWithAuthType directly, the same entry AuthPostPolicy uses, so the resolved identity reaches the error writer; a failed authN still resolves no identity and stays unattributed. The regression test now signs a denied request end to end through iam.Auth. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
255 lines
11 KiB
Go
255 lines
11 KiB
Go
package s3api
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/seaweedfs/seaweedfs/weed/iam/integration"
|
|
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
|
|
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
|
|
"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
|
)
|
|
|
|
// An STS session authenticates as an opaque session subject, so the audit entry
|
|
// must also carry the principal ARN — the role name and the role session name
|
|
// are only recoverable from there.
|
|
func TestAuditRequesterArnForSTSSession(t *testing.T) {
|
|
iam := &IdentityAccessManagement{
|
|
iamIntegration: &MockIAMIntegration{
|
|
validateSessionFunc: func(ctx context.Context, token string) (*sts.SessionInfo, error) {
|
|
return &sts.SessionInfo{
|
|
AssumedRoleUser: "ClientRole/dev-session",
|
|
Principal: "arn:aws:sts::000000000000:assumed-role/ClientRole/dev-session",
|
|
Subject: "47ad4828c45b3f337bc3146081ba8f0f",
|
|
SessionName: "dev-session",
|
|
Credentials: &sts.Credentials{
|
|
AccessKeyId: "ASIA0189777d42cba8e2",
|
|
SecretAccessKey: "secret",
|
|
},
|
|
ExpiresAt: time.Now().Add(time.Hour),
|
|
Policies: []string{"ClientPolicy"},
|
|
}, nil
|
|
},
|
|
},
|
|
}
|
|
|
|
// track() installs the holder before authentication runs.
|
|
outer := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "http://s3/test/", nil))
|
|
|
|
identity, _, errCode := iam.validateSTSSessionToken(outer, "session-token", "ASIA0189777d42cba8e2")
|
|
require.Equal(t, s3err.ErrNone, errCode)
|
|
|
|
iam.handleAuthResult(httptest.NewRecorder(), outer, identity, s3err.ErrNone, func(http.ResponseWriter, *http.Request) {})
|
|
|
|
log := s3err.GetAccessLog(outer, http.StatusOK, s3err.ErrNone)
|
|
assert.Equal(t, "47ad4828c45b3f337bc3146081ba8f0f", log.Requester)
|
|
assert.Equal(t, "arn:aws:sts::000000000000:assumed-role/ClientRole/dev-session", log.RequesterArn,
|
|
"audit entry must name the assumed role and session")
|
|
}
|
|
|
|
// An OIDC-federated STS session authenticates as an opaque session subject, so
|
|
// the requester field alone is useless for compliance auditing. The audit entry
|
|
// must surface the authoritative OIDC identity claim (preferred_username, email
|
|
// or sub) carried in the session request context, independent of the
|
|
// client-supplied role session name. See issue #11264.
|
|
func TestAuditRequesterIdentityForOIDCFederatedSession(t *testing.T) {
|
|
iam := &IdentityAccessManagement{
|
|
iamIntegration: &MockIAMIntegration{
|
|
validateSessionFunc: func(ctx context.Context, token string) (*sts.SessionInfo, error) {
|
|
return &sts.SessionInfo{
|
|
AssumedRoleUser: "S3ReadOnlyRole/boto3-session",
|
|
Principal: "arn:aws:sts::assumed-role/S3ReadOnlyRole/boto3-session",
|
|
Subject: "2a19e647c5d43a62a91ecf664d07dbe1",
|
|
SessionName: "boto3-session",
|
|
Credentials: &sts.Credentials{
|
|
AccessKeyId: "ASIA0189777d42cba8e2",
|
|
SecretAccessKey: "secret",
|
|
},
|
|
ExpiresAt: time.Now().Add(time.Hour),
|
|
Policies: []string{"S3ReadOnly"},
|
|
ParentUser: sts.ComputeParentUser("oidc-sub-123", "https://idp.example/"),
|
|
RequestContext: map[string]interface{}{
|
|
"preferred_username": "grant.west",
|
|
"email": "grant.west@digital.mod.uk",
|
|
"sub": "oidc-sub-123",
|
|
},
|
|
}, nil
|
|
},
|
|
},
|
|
}
|
|
|
|
outer := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "http://s3/test/", nil))
|
|
|
|
identity, _, errCode := iam.validateSTSSessionToken(outer, "session-token", "ASIA0189777d42cba8e2")
|
|
require.Equal(t, s3err.ErrNone, errCode)
|
|
|
|
iam.handleAuthResult(httptest.NewRecorder(), outer, identity, s3err.ErrNone, func(http.ResponseWriter, *http.Request) {})
|
|
|
|
log := s3err.GetAccessLog(outer, http.StatusOK, s3err.ErrNone)
|
|
assert.Equal(t, "2a19e647c5d43a62a91ecf664d07dbe1", log.Requester,
|
|
"requester stays the opaque session subject for backward compatibility")
|
|
assert.Equal(t, "grant.west", log.RequesterIdentity,
|
|
"audit entry must surface the authoritative OIDC identity claim, not the client-supplied role session name")
|
|
}
|
|
|
|
// A non-federated STS session has no OIDC identity. ValidateJWTWithClaims merges
|
|
// the JWT registered sub claim (the opaque session id) into RequestContext, so
|
|
// the context carries a sub even though it is not an OIDC subject. The audit
|
|
// entry must not surface that session id as a requester_identity — ParentUser
|
|
// gates the resolution so only federated sessions report an identity claim.
|
|
func TestAuditRequesterIdentityEmptyForNonFederatedSession(t *testing.T) {
|
|
iam := &IdentityAccessManagement{
|
|
iamIntegration: &MockIAMIntegration{
|
|
validateSessionFunc: func(ctx context.Context, token string) (*sts.SessionInfo, error) {
|
|
return &sts.SessionInfo{
|
|
AssumedRoleUser: "ClientRole/dev-session",
|
|
Principal: "arn:aws:sts::000000000000:assumed-role/ClientRole/dev-session",
|
|
Subject: "47ad4828c45b3f337bc3146081ba8f0f",
|
|
SessionName: "dev-session",
|
|
Credentials: &sts.Credentials{
|
|
AccessKeyId: "ASIA0189777d42cba8e2",
|
|
SecretAccessKey: "secret",
|
|
},
|
|
ExpiresAt: time.Now().Add(time.Hour),
|
|
Policies: []string{"ClientPolicy"},
|
|
// Simulate ValidateJWTWithClaims merging the registered sub
|
|
// (the session id) into RequestContext for a session that has
|
|
// no OIDC identity and therefore no ParentUser.
|
|
RequestContext: map[string]interface{}{
|
|
"sub": "47ad4828c45b3f337bc3146081ba8f0f",
|
|
},
|
|
}, nil
|
|
},
|
|
},
|
|
}
|
|
|
|
outer := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "http://s3/test/", nil))
|
|
|
|
identity, _, errCode := iam.validateSTSSessionToken(outer, "session-token", "ASIA0189777d42cba8e2")
|
|
require.Equal(t, s3err.ErrNone, errCode)
|
|
|
|
iam.handleAuthResult(httptest.NewRecorder(), outer, identity, s3err.ErrNone, func(http.ResponseWriter, *http.Request) {})
|
|
|
|
log := s3err.GetAccessLog(outer, http.StatusOK, s3err.ErrNone)
|
|
assert.Empty(t, log.RequesterIdentity, "non-federated session must not surface the session id as a requester_identity")
|
|
}
|
|
|
|
// Authentication resolves the identity before the policy verdict, so a denied
|
|
// request still has a requester and the audit entry must name it. See issue
|
|
// #11474.
|
|
func TestAuditRequesterForDeniedRequest(t *testing.T) {
|
|
iam := &IdentityAccessManagement{}
|
|
require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
|
|
Identities: []*iam_pb.Identity{{
|
|
Name: "read_only_user",
|
|
Credentials: []*iam_pb.Credential{{AccessKey: "readonly_access_key", SecretKey: "readonly_secret_key"}},
|
|
Actions: []string{"Read", "List"},
|
|
}},
|
|
}))
|
|
|
|
denied, err := newTestRequest(http.MethodDelete, "http://s3/bucket/obj", 0, nil)
|
|
require.NoError(t, err)
|
|
require.NoError(t, signRequestV4(denied, "readonly_access_key", "readonly_secret_key"))
|
|
denied = s3_constants.EnsureIdentityHolder(denied)
|
|
|
|
rec := httptest.NewRecorder()
|
|
iam.Auth(func(http.ResponseWriter, *http.Request) {
|
|
t.Error("denied request must not reach the handler")
|
|
}, s3_constants.ACTION_WRITE)(rec, denied)
|
|
|
|
require.Equal(t, http.StatusForbidden, rec.Code)
|
|
log := s3err.GetAccessLog(denied, rec.Code, s3err.ErrAccessDenied)
|
|
assert.Equal(t, "read_only_user", log.Requester, "denied request must still audit the requester")
|
|
assert.Equal(t, "arn:aws:iam::"+defaultAccountID+":user/read_only_user", log.RequesterArn)
|
|
|
|
anonymous := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodDelete, "http://s3/bucket/obj", nil))
|
|
rec = httptest.NewRecorder()
|
|
iam.handleAuthResult(rec, anonymous, nil, s3err.ErrAccessDenied, func(http.ResponseWriter, *http.Request) {
|
|
t.Error("denied request must not reach the handler")
|
|
})
|
|
log = s3err.GetAccessLog(anonymous, rec.Code, s3err.ErrAccessDenied)
|
|
assert.Empty(t, log.Requester, "unauthenticated denial must not attribute a requester")
|
|
}
|
|
|
|
// A JWT-authenticated identity carries no PrincipalArn of its own — the auth
|
|
// layer hands the principal over in a request header — so the audit entry has to
|
|
// resolve the ARN the same way policy evaluation does.
|
|
func TestAuditRequesterArnForJWTIdentity(t *testing.T) {
|
|
iam := &IdentityAccessManagement{}
|
|
|
|
outer := s3_constants.EnsureIdentityHolder(httptest.NewRequest(http.MethodGet, "http://s3/test/", nil))
|
|
outer.Header.Set(s3_constants.SeaweedFSPrincipalHeader, "arn:aws:sts::000000000000:assumed-role/ClientRole/oidc-session")
|
|
|
|
identity := &Identity{Name: "alice", Account: &Account{Id: "alice"}}
|
|
iam.handleAuthResult(httptest.NewRecorder(), outer, identity, s3err.ErrNone, func(http.ResponseWriter, *http.Request) {})
|
|
|
|
log := s3err.GetAccessLog(outer, http.StatusOK, s3err.ErrNone)
|
|
assert.Equal(t, "alice", log.Requester)
|
|
assert.Equal(t, "arn:aws:sts::000000000000:assumed-role/ClientRole/oidc-session", log.RequesterArn)
|
|
}
|
|
|
|
// The AssumeRole call itself is authenticated inside the STS handler, which the
|
|
// generic auth middleware never wraps; without recording the caller there the
|
|
// audit entry for minting a session has no requester at all.
|
|
func TestAuditRequesterForAssumeRole(t *testing.T) {
|
|
ctx := context.Background()
|
|
manager := newTestSTSIntegrationManager(t)
|
|
|
|
require.NoError(t, manager.CreatePolicy(ctx, "", "ClientPolicy", &policy.PolicyDocument{
|
|
Version: "2012-10-17",
|
|
Statement: []policy.Statement{{
|
|
Effect: "Allow",
|
|
Action: []string{"s3:*"},
|
|
Resource: []string{"arn:aws:s3:::*", "arn:aws:s3:::*/*"},
|
|
}},
|
|
}))
|
|
require.NoError(t, manager.CreateRole(ctx, "", "ClientRole", &integration.RoleDefinition{
|
|
RoleName: "ClientRole",
|
|
TrustPolicy: &policy.PolicyDocument{
|
|
Version: "2012-10-17",
|
|
Statement: []policy.Statement{{Effect: "Allow", Principal: "*", Action: []string{"sts:AssumeRole"}}},
|
|
},
|
|
AttachedPolicies: []string{"ClientPolicy"},
|
|
}))
|
|
|
|
const accessKey, secretKey = "adminkey", "adminsecret"
|
|
iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")}
|
|
require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
|
|
Identities: []*iam_pb.Identity{{
|
|
Name: "admin",
|
|
Credentials: []*iam_pb.Credential{{AccessKey: accessKey, SecretKey: secretKey}},
|
|
Actions: []string{"Admin"},
|
|
}},
|
|
}))
|
|
|
|
body := url.Values{
|
|
"Action": {"AssumeRole"},
|
|
"Version": {"2011-06-15"},
|
|
"RoleArn": {"arn:aws:iam::" + defaultAccountID + ":role/ClientRole"},
|
|
"RoleSessionName": {"dev-session"},
|
|
}.Encode()
|
|
req, err := newTestRequest(http.MethodPost, "http://sts.seaweedfs.test/", int64(len(body)), strings.NewReader(body))
|
|
require.NoError(t, err)
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
require.NoError(t, signRequestV4(req, accessKey, secretKey))
|
|
req = s3_constants.EnsureIdentityHolder(req)
|
|
|
|
rec := httptest.NewRecorder()
|
|
NewSTSHandlers(manager.GetSTSService(), iam).handleAssumeRole(rec, req)
|
|
require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
|
|
|
|
log := s3err.GetAccessLog(req, rec.Code, s3err.ErrNone)
|
|
assert.Equal(t, "admin", log.Requester, "AssumeRole must audit who asked for the session")
|
|
assert.Equal(t, "arn:aws:iam::"+defaultAccountID+":user/admin", log.RequesterArn)
|
|
}
|