Fix: Allow deleting multiple objects anonymously if policy supports it (#7439)

Fixes #5683
This commit is contained in:
kannappanr 2019-04-22 07:54:43 -07:00 committed by Nitish Tiwari
parent 188cf1d5ce
commit 0c75395abe
2 changed files with 42 additions and 38 deletions

View File

@ -263,16 +263,6 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
return return
} }
var s3Error APIErrorCode
if s3Error = checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, ""); s3Error != ErrNone {
// In the event access is denied, a 200 response should still be returned
// http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html
if s3Error != ErrAccessDenied {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r))
return
}
}
// Content-Length is required and should be non-zero // Content-Length is required and should be non-zero
// http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html // http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html
if r.ContentLength <= 0 { if r.ContentLength <= 0 {
@ -324,37 +314,32 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
deleteObject = api.CacheAPI().DeleteObject deleteObject = api.CacheAPI().DeleteObject
} }
var dErrs = make([]error, len(deleteObjects.Objects)) var dErrs = make([]APIErrorCode, len(deleteObjects.Objects))
for index, object := range deleteObjects.Objects { for index, object := range deleteObjects.Objects {
// If the request is denied access, each item if dErrs[index] = checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName); dErrs[index] != ErrNone {
// should be marked as 'AccessDenied' if dErrs[index] == ErrSignatureDoesNotMatch || dErrs[index] == ErrInvalidAccessKeyID {
if s3Error == ErrAccessDenied { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(dErrs[index]), r.URL, guessIsBrowserReq(r))
dErrs[index] = PrefixAccessDenied{ return
Bucket: bucket,
Object: object.ObjectName,
} }
continue continue
} }
dErrs[index] = deleteObject(ctx, bucket, object.ObjectName) err := deleteObject(ctx, bucket, object.ObjectName)
if err != nil {
dErrs[index] = toAPIErrorCode(ctx, err)
}
} }
// Collect deleted objects and errors if any. // Collect deleted objects and errors if any.
var deletedObjects []ObjectIdentifier var deletedObjects []ObjectIdentifier
var deleteErrors []DeleteError var deleteErrors []DeleteError
for index, err := range dErrs { for index, errCode := range dErrs {
object := deleteObjects.Objects[index] object := deleteObjects.Objects[index]
// Success deleted objects are collected separately. // Success deleted objects are collected separately.
if err == nil { if errCode == ErrNone || errCode == ErrNoSuchKey {
deletedObjects = append(deletedObjects, object) deletedObjects = append(deletedObjects, object)
continue continue
} }
if _, ok := err.(ObjectNotFound); ok { apiErr := getAPIError(errCode)
// If the object is not found it should be
// accounted as deleted as per S3 spec.
deletedObjects = append(deletedObjects, object)
continue
}
apiErr := toAPIError(ctx, err)
// Error during delete should be collected separately. // Error during delete should be collected separately.
deleteErrors = append(deleteErrors, DeleteError{ deleteErrors = append(deleteErrors, DeleteError{
Code: apiErr.Code, Code: apiErr.Code,

View File

@ -583,8 +583,23 @@ func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs,
claims, owner, authErr := webRequestAuthenticate(r) claims, owner, authErr := webRequestAuthenticate(r)
if authErr != nil { if authErr != nil {
if authErr == errNoAuthToken {
// Check if all objects are allowed to be deleted anonymously
for _, object := range args.Objects {
if !globalPolicySys.IsAllowed(policy.Args{
Action: policy.DeleteObjectAction,
BucketName: args.BucketName,
ConditionValues: getConditionValues(r, "", ""),
IsOwner: false,
ObjectName: object,
}) {
return toJSONError(errAuthentication)
}
}
} else {
return toJSONError(authErr) return toJSONError(authErr)
} }
}
if args.BucketName == "" || len(args.Objects) == 0 { if args.BucketName == "" || len(args.Objects) == 0 {
return toJSONError(errInvalidArgument) return toJSONError(errInvalidArgument)
@ -640,7 +655,10 @@ next:
return toJSONError(errMethodNotAllowed) return toJSONError(errMethodNotAllowed)
} }
} }
// Check for permissions only in the case of
// non-anonymous login. For anonymous login, policy has already
// been checked.
if authErr != errNoAuthToken {
if !globalIAMSys.IsAllowed(iampolicy.Args{ if !globalIAMSys.IsAllowed(iampolicy.Args{
AccountName: claims.Subject, AccountName: claims.Subject,
Action: iampolicy.DeleteObjectAction, Action: iampolicy.DeleteObjectAction,
@ -651,6 +669,7 @@ next:
}) { }) {
return toJSONError(errAccessDenied) return toJSONError(errAccessDenied)
} }
}
if err = deleteObject(context.Background(), objectAPI, web.CacheAPI(), args.BucketName, objectName, r); err != nil { if err = deleteObject(context.Background(), objectAPI, web.CacheAPI(), args.BucketName, objectName, r); err != nil {
break next break next