Fix STS identity authorization by populating PolicyNames (#7985) (#7986)

* Fix STS identity authorization by populating PolicyNames (#7985)

This commit fixes GitHub issue #7985 where STS-assumed identities
received empty identity.Actions, causing all S3 operations to be denied
even when the role had valid IAM policies attached.

Changes:
1. Populate PolicyNames field from sessionInfo.Policies in
   validateSTSSessionToken() to enable IAM-based authorization for
   STS identities
2. Fix bucket+objectKey path construction in canDo() method to include
   proper slash separator between bucket and object key
3. Add comprehensive test suite to validate the fix and prevent
   regression

The fix ensures that STS-assumed identities are properly authorized
through the IAM path when iamIntegration is available, allowing roles
with valid IAM policies to perform S3 operations as expected.

* Update STS identity tests to be more rigorous and use actual implementation path

* Fix regression in canDo() path concatenation

The previous fix blindly added a slash separator, which caused double
slashes when objectKey already started with a slash (common in existing
tests and some code paths). This broke TestCanDo and
TestObjectLevelListPermissions.

This commit updates the logic to only add the slash separator if
objectKey is not empty and does not already start with a slash.
This fixes the regressions while maintaining the fix for issue #7985.

* Refactor STS identity tests: extract helpers and simplify redundant logic

- Extracted setupTestSTSService and newTestIdentity helper functions
- Removed redundant if-else verification blocks that were already covered by assertions
- Cleaned up test cases to improve maintainability as suggested in code review.

* Add canDo() verification to STS identity tests

Address code review suggestion: verify that identities with empty
Actions correctly return false for canDo() checks, which confirms the
behavior that forces authorization to fall back to the IAM path.

* Simplify TestCanDoPathConstruction variable names

Rename expectedPath to fullPath and simplify logging/assertion logic
based on code review feedback.

* Refactor path construction and logging in canDo()

- Compute fullPath early and use it for logging to prevent double slashes
- Update TestCanDoPathConstruction to use robust path verification
- Add test case for objectKey with leading slash to ensure correct handling
This commit is contained in:
Chris Lu
2026-01-07 13:01:26 -08:00
committed by GitHub
parent e67973dc53
commit 6432019d08
3 changed files with 345 additions and 5 deletions

View File

@@ -794,13 +794,21 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string)
return true
}
}
// Intelligent path concatenation to avoid double slashes
fullPath := bucket
if objectKey != "" && !strings.HasPrefix(objectKey, "/") {
fullPath += "/"
}
fullPath += objectKey
if bucket == "" {
glog.V(3).Infof("identity %s is not allowed to perform action %s on %s -- bucket is empty", identity.Name, action, bucket+objectKey)
glog.V(3).Infof("identity %s is not allowed to perform action %s on %s -- bucket is empty", identity.Name, action, "/"+strings.TrimPrefix(objectKey, "/"))
return false
}
glog.V(3).Infof("checking if %s can perform %s on bucket '%s'", identity.Name, action, bucket+objectKey)
target := string(action) + ":" + bucket + objectKey
adminTarget := s3_constants.ACTION_ADMIN + ":" + bucket + objectKey
glog.V(3).Infof("checking if %s can perform %s on bucket '%s'", identity.Name, action, fullPath)
target := string(action) + ":" + fullPath
adminTarget := s3_constants.ACTION_ADMIN + ":" + fullPath
limitedByBucket := string(action) + ":" + bucket
adminLimitedByBucket := s3_constants.ACTION_ADMIN + ":" + bucket
@@ -823,7 +831,7 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string)
}
}
//log error
glog.V(3).Infof("identity %s is not allowed to perform action %s on %s", identity.Name, action, bucket+objectKey)
glog.V(3).Infof("identity %s is not allowed to perform action %s on %s", identity.Name, action, bucket+"/"+objectKey)
return false
}