Browse Source

fix: prevent duplicates and race conditions in merge logic

Data Integrity:
- Prevent service account credential duplicates on repeated merges
- Clean up stale accessKeyIdent entries when replacing identities
- Check existing credentials before appending

Concurrency Safety:
- Add synchronization to IsStaticConfig method

Test Improvements:
- Add mux route vars for proper GetBucketAndObject extraction
- Add STS session token header to trigger correct auth path
pull/7989/head
Chris Lu 4 days ago
parent
commit
f4d1c5729f
  1. 26
      weed/s3api/auth_credentials.go
  2. 12
      weed/s3api/auth_signature_v4_sts_test.go

26
weed/s3api/auth_credentials.go

@ -550,6 +550,14 @@ func (iam *IdentityAccessManagement) mergeS3ApiConfiguration(config *iam_pb.S3Ap
} }
if existingIdx >= 0 { if existingIdx >= 0 {
// Before replacing, remove stale accessKeyIdent entries for the old identity
oldIdentity := identities[existingIdx]
for _, oldCred := range oldIdentity.Credentials {
// Only remove if it still points to this identity
if accessKeyIdent[oldCred.AccessKey] == oldIdentity {
delete(accessKeyIdent, oldCred.AccessKey)
}
}
// Replace existing dynamic identity // Replace existing dynamic identity
identities[existingIdx] = t identities[existingIdx] = t
} else { } else {
@ -584,6 +592,22 @@ func (iam *IdentityAccessManagement) mergeS3ApiConfiguration(config *iam_pb.S3Ap
continue continue
} }
// Check if this access key already exists in parent's credentials to avoid duplicates
alreadyExists := false
for _, existingCred := range parentIdent.Credentials {
if existingCred.AccessKey == sa.Credential.AccessKey {
alreadyExists = true
break
}
}
if alreadyExists {
glog.V(3).Infof("Service account %s credential already exists for parent %s, skipping", sa.Id, sa.ParentUser)
// Ensure accessKeyIdent mapping is correct
accessKeyIdent[sa.Credential.AccessKey] = parentIdent
continue
}
// Add service account credential to parent identity // Add service account credential to parent identity
cred := &Credential{ cred := &Credential{
AccessKey: sa.Credential.AccessKey, AccessKey: sa.Credential.AccessKey,
@ -634,6 +658,8 @@ func (iam *IdentityAccessManagement) isEnabled() bool {
} }
func (iam *IdentityAccessManagement) IsStaticConfig() bool { func (iam *IdentityAccessManagement) IsStaticConfig() bool {
iam.m.RLock()
defer iam.m.RUnlock()
return iam.useStaticConfig return iam.useStaticConfig
} }

12
weed/s3api/auth_signature_v4_sts_test.go

@ -5,6 +5,7 @@ import (
"net/http" "net/http"
"testing" "testing"
"github.com/gorilla/mux"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -144,6 +145,17 @@ func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
req.Header.Set("Host", "s3.amazonaws.com") req.Header.Set("Host", "s3.amazonaws.com")
// Set up mux route vars for GetBucketAndObject to work
req = mux.SetURLVars(req, map[string]string{
"bucket": "test-bucket",
"object": "test-object",
})
// For STS identities, add session token header to trigger STS-v4 auth path
if len(tt.identity.Actions) == 0 && tt.iamIntegration != nil {
req.Header.Set("X-Amz-Security-Token", "test-session-token")
}
// Mock the permission check logic from verifyV4Signature (now centralized in VerifyActionPermission) // Mock the permission check logic from verifyV4Signature (now centralized in VerifyActionPermission)
var errCode s3err.ErrorCode var errCode s3err.ErrorCode
if tt.shouldCheckPermissions { if tt.shouldCheckPermissions {

Loading…
Cancel
Save