fix: preserve multi-delete authentication and audit context

Authenticate DeleteObjects before validating entry count, remove the obsolete per-version auth helper, and pin the marker-only request to the ordinary authorization path.\n\nRefs: #58

Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-09-02 02:16:51 +08:00
parent c4140609b7
commit dee2c3a02b
3 changed files with 25 additions and 16 deletions
-11
View File
@@ -363,17 +363,6 @@ func checkRequestAuthTypeWithRequestTags(ctx context.Context, r *http.Request, a
return authorizeRequestWithTags(ctx, r, action, "", requestTags) return authorizeRequestWithTags(ctx, r, action, "", requestTags)
} }
// checkRequestAuthTypeWithVID is similar to checkRequestAuthType
// passes versionID additionally.
func checkRequestAuthTypeWithVID(ctx context.Context, r *http.Request, action policy.Action, bucketName, objectName, versionID string) (s3Err APIErrorCode) {
logger.GetReqInfo(ctx).BucketName = bucketName
logger.GetReqInfo(ctx).ObjectName = objectName
logger.GetReqInfo(ctx).VersionID = versionID
_, _, s3Err = checkRequestAuthTypeCredential(ctx, r, action)
return s3Err
}
func authenticateRequest(ctx context.Context, r *http.Request, action policy.Action) (s3Err APIErrorCode) { func authenticateRequest(ctx context.Context, r *http.Request, action policy.Action) (s3Err APIErrorCode) {
if logger.GetReqInfo(ctx) == nil { if logger.GetReqInfo(ctx) == nil {
bugLogIf(ctx, errors.New("unexpected context.Context does not have a logger.ReqInfo"), logger.ErrorKind) bugLogIf(ctx, errors.New("unexpected context.Context does not have a logger.ReqInfo"), logger.ErrorKind)
+5 -5
View File
@@ -465,11 +465,6 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
deleteObjectsFn := objectAPI.DeleteObjects deleteObjectsFn := objectAPI.DeleteObjects
// Return Malformed XML as S3 spec if the number of objects is empty
if len(deleteObjectsReq.Objects) == 0 || len(deleteObjectsReq.Objects) > maxDeleteList {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL)
return
}
reqInfo := logger.GetReqInfo(ctx) reqInfo := logger.GetReqInfo(ctx)
if reqInfo == nil { if reqInfo == nil {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL) writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
@@ -481,6 +476,11 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL) writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
return return
} }
// Return Malformed XML as S3 spec if the number of objects is empty.
if len(deleteObjectsReq.Objects) == 0 || len(deleteObjectsReq.Objects) > maxDeleteList {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL)
return
}
objectsToDelete := map[ObjectToDelete]int{} objectsToDelete := map[ObjectToDelete]int{}
getObjectInfoFn := objectAPI.GetObjectInfo getObjectInfoFn := objectAPI.GetObjectInfo
+20
View File
@@ -319,6 +319,7 @@ func testAPIDeleteObjectVersionDenyAndReplicationCompatibility(obj ObjectLayer,
{"Effect":"Allow","Action":["s3:DeleteObject","s3:DeleteObjectVersion","s3:ReplicateDelete"],"Resource":["arn:aws:s3:::`+bucket+`/*"]}, {"Effect":"Allow","Action":["s3:DeleteObject","s3:DeleteObjectVersion","s3:ReplicateDelete"],"Resource":["arn:aws:s3:::`+bucket+`/*"]},
{"Effect":"Deny","Action":"s3:DeleteObjectVersion","Resource":"arn:aws:s3:::`+bucket+`/deny/*"} {"Effect":"Deny","Action":"s3:DeleteObjectVersion","Resource":"arn:aws:s3:::`+bucket+`/deny/*"}
]`) ]`)
deleteOnly := newObjectAttributesAuthzUser(t, instanceType, bucket, `"s3:DeleteObject"`)
t.Run("ordinary explicit deny wins", func(t *testing.T) { t.Run("ordinary explicit deny wins", func(t *testing.T) {
object := "deny/ordinary" object := "deny/ordinary"
@@ -351,6 +352,25 @@ func testAPIDeleteObjectVersionDenyAndReplicationCompatibility(obj ObjectLayer,
t.Fatalf("status %d, want 403: %s", rec.Code, rec.Body.String()) t.Fatalf("status %d, want 403: %s", rec.Code, rec.Body.String())
} }
}) })
t.Run("marker alone cannot enter the replication path", func(t *testing.T) {
object := "replication/fake-marker"
versionID := put(t, object)
target := getDeleteObjectURL("", bucket, object) + "?" + url.Values{xhttp.VersionID: {versionID}}.Encode()
req, err := newTestSignedRequestV4(http.MethodDelete, target, 0, nil, deleteOnly.AccessKey, deleteOnly.SecretKey,
map[string]string{xhttp.MinIOSourceReplicationRequest: "true"})
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != http.StatusForbidden {
t.Fatalf("status %d, want 403: %s", rec.Code, rec.Body.String())
}
if _, err = obj.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: versionID}); err != nil {
t.Fatalf("fake marker removed the version: %v", err)
}
})
} }
func newDeleteAuthzPolicyUser(t *testing.T, instanceType, bucket, statements string) auth.Credentials { func newDeleteAuthzPolicyUser(t *testing.T, instanceType, bucket, statements string) auth.Credentials {