Fixes browser delete issue for anon and authorized users (minio#9440)

kindofblue · May 3, 2020 · fbd15cb · fbd15cb
1 parent f7c91ef
commit fbd15cb
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 40 deletions.
diff --git a/cmd/object-lock.go b/cmd/object-lock.go
@@ -31,7 +31,7 @@ import (
 )
 
 // Similar to enforceRetentionBypassForDelete but for WebUI
-func enforceRetentionBypassForDeleteWeb(ctx context.Context, r *http.Request, bucket, object string, getObjectInfoFn GetObjectInfoFn) APIErrorCode {
+func enforceRetentionBypassForDeleteWeb(ctx context.Context, r *http.Request, bucket, object string, getObjectInfoFn GetObjectInfoFn, govBypassPerms bool) APIErrorCode {
 	opts, err := getOpts(ctx, r, bucket, object)
 	if err != nil {
 		return toAPIErrorCode(ctx, err)
@@ -80,7 +80,7 @@ func enforceRetentionBypassForDeleteWeb(ctx context.Context, r *http.Request, bu
 			// and must explicitly include x-amz-bypass-governance-retention:true
 			// as a request header with any request that requires overriding
 			// governance mode.
-			byPassSet := objectlock.IsObjectLockGovernanceBypassSet(r.Header)
+			byPassSet := govBypassPerms && objectlock.IsObjectLockGovernanceBypassSet(r.Header)
 			if !byPassSet {
 				t, err := objectlock.UTCNowNTP()
 				if err != nil {
@@ -91,6 +91,11 @@ func enforceRetentionBypassForDeleteWeb(ctx context.Context, r *http.Request, bu
 				if !ret.RetainUntilDate.Before(t) {
 					return ErrObjectLocked
 				}
+
+				if !govBypassPerms {
+					return ErrObjectLocked
+				}
+
 				return ErrNone
 			}
 		}

diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go
@@ -667,10 +667,8 @@ next:
 	for _, objectName := range args.Objects {
 		// If not a directory, remove the object.
 		if !HasSuffix(objectName, SlashSeparator) && objectName != "" {
-			// Check for permissions only in the case of
-			// non-anonymous login. For anonymous login, policy has already
-			// been checked.
-			govBypassPerms := ErrAccessDenied
+			// Check permissions for non-anonymous user.
+			govBypassPerms := false
 			if authErr != errNoAuthToken {
 				if !globalIAMSys.IsAllowed(iampolicy.Args{
 					AccountName:     claims.AccessKey,
@@ -692,22 +690,12 @@ next:
 					ObjectName:      objectName,
 					Claims:          claims.Map(),
 				}) {
-					govBypassPerms = ErrNone
-				}
-				if globalIAMSys.IsAllowed(iampolicy.Args{
-					AccountName:     claims.AccessKey,
-					Action:          iampolicy.GetBucketObjectLockConfigurationAction,
-					BucketName:      args.BucketName,
-					ConditionValues: getConditionValues(r, "", claims.AccessKey, claims.Map()),
-					IsOwner:         owner,
-					ObjectName:      objectName,
-					Claims:          claims.Map(),
-				}) {
-					govBypassPerms = ErrNone
+					govBypassPerms = true
 				}
 			}
+
 			if authErr == errNoAuthToken {
-				// Check if object is allowed to be deleted anonymously
+				// Check if object is allowed to be deleted anonymously.
 				if !globalPolicySys.IsAllowed(policy.Args{
 					Action:          policy.DeleteObjectAction,
 					BucketName:      args.BucketName,
@@ -726,30 +714,13 @@ next:
 					IsOwner:         false,
 					ObjectName:      objectName,
 				}) {
-					govBypassPerms = ErrNone
+					govBypassPerms = true
 				}
-
-				// Check if object is allowed to be deleted anonymously
-				if globalPolicySys.IsAllowed(policy.Args{
-					Action:          policy.GetBucketObjectLockConfigurationAction,
-					BucketName:      args.BucketName,
-					ConditionValues: getConditionValues(r, "", "", nil),
-					IsOwner:         false,
-					ObjectName:      objectName,
-				}) {
-					govBypassPerms = ErrNone
-				}
-			}
-			if govBypassPerms != ErrNone {
-				return toJSONError(ctx, errAccessDenied)
 			}
 
-			apiErr := ErrNone
-			if _, ok := globalBucketObjectLockConfig.Get(args.BucketName); ok && (apiErr == ErrNone) {
-				apiErr = enforceRetentionBypassForDeleteWeb(ctx, r, args.BucketName, objectName, getObjectInfo)
-				if apiErr != ErrNone && apiErr != ErrNoSuchKey {
-					return toJSONError(ctx, errAccessDenied)
-				}
+			apiErr := enforceRetentionBypassForDeleteWeb(ctx, r, args.BucketName, objectName, getObjectInfo, govBypassPerms)
+			if apiErr != ErrNone && apiErr != ErrNoSuchKey {
+				return toJSONError(ctx, errAccessDenied)
 			}
 			if apiErr == ErrNone {
 				if err = deleteObject(ctx, objectAPI, web.CacheAPI(), args.BucketName, objectName, r); err != nil {