Browse Source

Refactor authorization logic to avoid duplication

Centralized the authorization logic into IdentityAccessManagement.VerifyActionPermission.
Updated auth_signature_v4.go and auth_credentials.go to use this new helper.
Updated tests to clarify that they mirror the centralized logic.
pull/7988/head
Chris Lu 6 days ago
parent
commit
7990003710
  1. 32
      weed/s3api/auth_credentials.go
  2. 15
      weed/s3api/auth_signature_v4.go
  3. 7
      weed/s3api/auth_signature_v4_sts_test.go

32
weed/s3api/auth_credentials.go

@ -702,18 +702,9 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac
// Only check IAM if bucket policy didn't explicitly allow
if !policyAllows {
// Traditional identities (with Actions from -s3.config) use legacy auth,
// JWT/STS identities (no Actions) use IAM authorization
if len(identity.Actions) > 0 {
if !identity.canDo(action, bucket, object) {
return identity, s3err.ErrAccessDenied, reqAuthType
}
} else if iam.iamIntegration != nil {
if errCode := iam.authorizeWithIAM(r, identity, action, bucket, object); errCode != s3err.ErrNone {
return identity, errCode, reqAuthType
}
} else {
return identity, s3err.ErrAccessDenied, reqAuthType
// Use centralized permission check
if errCode := iam.VerifyActionPermission(r, identity, action, bucket, object); errCode != s3err.ErrNone {
return identity, errCode, reqAuthType
}
}
}
@ -990,6 +981,23 @@ func determineIAMAuthPath(sessionToken, principal, principalArn string) iamAuthP
return iamAuthPathNone
}
// VerifyActionPermission checks if the identity is allowed to perform the action on the resource.
// It handles both traditional identities (via Actions) and IAM/STS identities (via Policy).
func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, identity *Identity, action Action, bucket, object string) s3err.ErrorCode {
// Traditional identities (with Actions from -s3.config) use legacy auth,
// JWT/STS identities (no Actions) use IAM authorization
if len(identity.Actions) > 0 {
if !identity.canDo(action, bucket, object) {
return s3err.ErrAccessDenied
}
return s3err.ErrNone
} else if iam.iamIntegration != nil {
return iam.authorizeWithIAM(r, identity, action, bucket, object)
}
return s3err.ErrAccessDenied
}
// authorizeWithIAM authorizes requests using the IAM integration policy engine
func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode {
ctx := r.Context()

15
weed/s3api/auth_signature_v4.go

@ -249,18 +249,9 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh
action = s3_constants.ACTION_WRITE
}
// Traditional identities (with Actions from -s3.config) use legacy auth,
// JWT/STS identities (no Actions) use IAM authorization
if len(identity.Actions) > 0 {
if !identity.canDo(Action(action), bucket, object) {
return nil, nil, "", nil, s3err.ErrAccessDenied
}
} else if iam.iamIntegration != nil {
if errCode := iam.authorizeWithIAM(r, identity, Action(action), bucket, object); errCode != s3err.ErrNone {
return nil, nil, "", nil, errCode
}
} else {
return nil, nil, "", nil, s3err.ErrAccessDenied
// Use centralized permission check
if errCode = iam.VerifyActionPermission(r, identity, Action(action), bucket, object); errCode != s3err.ErrNone {
return nil, nil, "", nil, errCode
}
}

7
weed/s3api/auth_signature_v4_sts_test.go

@ -137,7 +137,7 @@ func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) {
require.NoError(t, err)
req.Header.Set("Host", "s3.amazonaws.com")
// Mock the permission check logic from verifyV4Signature
// Mock the permission check logic from verifyV4Signature (now centralized in VerifyActionPermission)
var errCode s3err.ErrorCode
if tt.shouldCheckPermissions {
bucket, object := s3_constants.GetBucketAndObject(req)
@ -146,7 +146,10 @@ func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) {
action = s3_constants.ACTION_WRITE
}
// This is the logic we're testing - it should match the implementation in verifyV4Signature
// The logic below mirrors IdentityAccessManagement.VerifyActionPermission
// We replicate it here to verify the behavior without needing to construct a full S3IAMIntegration stack
// Note: Ideally we would call iam.VerifyActionPermission(req, tt.identity, Action(action), bucket, object)
// but constructing a valid IdentityAccessManagement with a mock S3IAMIntegration is difficult due to concrete struct dependencies.
if len(tt.identity.Actions) > 0 {
if !tt.identity.canDo(Action(action), bucket, object) {
errCode = s3err.ErrAccessDenied

Loading…
Cancel
Save