[#1480] ape: Remove SoftAPECheck flag

Previous release was EACL-compatible.
Starting from now all EACL should've been migrated to APE chains.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

pkg/services/common/ape/checker.go

@@ -44,9 +44,6 @@ type CheckPrm struct {
 
 	// The request's bearer token. It is used in order to check APE overrides with the token.
 	BearerToken *bearer.Token
-
-	// If SoftAPECheck is set to true, then NoRuleFound is interpreted as allow.
-	SoftAPECheck bool
 }
 
 // CheckCore provides methods to perform the common logic of APE check.
@@ -104,7 +101,7 @@ func (c *checkerCoreImpl) CheckAPE(prm CheckPrm) error {
 	if err != nil {
 		return err
 	}
-	if !found && prm.SoftAPECheck || status == apechain.Allow {
+	if found && status == apechain.Allow {
 		return nil
 	}
 	err = fmt.Errorf("access to operation %s is denied by access policy engine: %s", prm.Request.Operation(), status.String())

pkg/services/object/acl/acl.go

@@ -3,7 +3,6 @@ package acl
 import (
 	"context"
 	"crypto/ecdsa"
-	"crypto/elliptic"
 	"errors"
 	"fmt"
 	"io"
@@ -22,7 +21,6 @@ import (
 	objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
 	oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
-	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 )
 
 // Checker implements v2.ACLChecker interfaces and provides
@@ -72,33 +70,6 @@ func NewChecker(
 	}
 }
 
-// CheckBasicACL is a main check function for basic ACL.
-func (c *Checker) CheckBasicACL(info v2.RequestInfo) bool {
-	// check basic ACL permissions
-	return info.BasicACL().IsOpAllowed(info.Operation(), info.RequestRole())
-}
-
-// StickyBitCheck validates owner field in the request if sticky bit is enabled.
-func (c *Checker) StickyBitCheck(info v2.RequestInfo, owner user.ID) bool {
-	// According to FrostFS specification sticky bit has no effect on system nodes
-	// for correct intra-container work with objects (in particular, replication).
-	if info.RequestRole() == acl.RoleContainer {
-		return true
-	}
-
-	if !info.BasicACL().Sticky() {
-		return true
-	}
-
-	if len(info.SenderKey()) == 0 {
-		return false
-	}
-
-	requestSenderKey := unmarshalPublicKey(info.SenderKey())
-
-	return isOwnerFromKey(owner, requestSenderKey)
-}
-
 // CheckEACL is a main check function for extended ACL.
 func (c *Checker) CheckEACL(msg any, reqInfo v2.RequestInfo) error {
 	basicACL := reqInfo.BasicACL()
@@ -241,22 +212,3 @@ func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) error {
 
 	return nil
 }
-
-func isOwnerFromKey(id user.ID, key *keys.PublicKey) bool {
-	if key == nil {
-		return false
-	}
-
-	var id2 user.ID
-	user.IDFromKey(&id2, (ecdsa.PublicKey)(*key))
-
-	return id.Equals(id2)
-}
-
-func unmarshalPublicKey(bs []byte) *keys.PublicKey {
-	pub, err := keys.NewPublicKeyFromBytes(bs, elliptic.P256())
-	if err != nil {
-		return nil
-	}
-	return pub
-}

pkg/services/object/acl/acl_test.go

@@ -1,89 +0,0 @@
-package acl
-
-import (
-	"testing"
-
-	"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container"
-	"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/engine"
-	v2 "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/acl/v2"
-	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl"
-	cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
-	eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl"
-	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
-	usertest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user/test"
-	"github.com/stretchr/testify/require"
-)
-
-type emptyEACLSource struct{}
-
-func (e emptyEACLSource) GetEACL(_ cid.ID) (*container.EACL, error) {
-	return nil, nil
-}
-
-type emptyNetmapState struct{}
-
-func (e emptyNetmapState) CurrentEpoch() uint64 {
-	return 0
-}
-
-func TestStickyCheck(t *testing.T) {
-	checker := NewChecker(
-		emptyNetmapState{},
-		emptyEACLSource{},
-		eaclSDK.NewValidator(),
-		&engine.StorageEngine{})
-
-	t.Run("system role", func(t *testing.T) {
-		var info v2.RequestInfo
-
-		info.SetSenderKey(make([]byte, 33)) // any non-empty key
-		info.SetRequestRole(acl.RoleContainer)
-
-		require.True(t, checker.StickyBitCheck(info, usertest.ID()))
-
-		var basicACL acl.Basic
-		basicACL.MakeSticky()
-
-		info.SetBasicACL(basicACL)
-
-		require.True(t, checker.StickyBitCheck(info, usertest.ID()))
-	})
-
-	t.Run("owner ID and/or public key emptiness", func(t *testing.T) {
-		var info v2.RequestInfo
-
-		info.SetRequestRole(acl.RoleOthers) // should be non-system role
-
-		assertFn := func(isSticky, withKey, withOwner, expected bool) {
-			info := info
-			if isSticky {
-				var basicACL acl.Basic
-				basicACL.MakeSticky()
-
-				info.SetBasicACL(basicACL)
-			}
-
-			if withKey {
-				info.SetSenderKey(make([]byte, 33))
-			} else {
-				info.SetSenderKey(nil)
-			}
-
-			var ownerID user.ID
-
-			if withOwner {
-				ownerID = usertest.ID()
-			}
-
-			require.Equal(t, expected, checker.StickyBitCheck(info, ownerID))
-		}
-
-		assertFn(true, false, false, false)
-		assertFn(true, true, false, false)
-		assertFn(true, false, true, false)
-		assertFn(false, false, false, true)
-		assertFn(false, true, false, true)
-		assertFn(false, false, true, true)
-		assertFn(false, true, true, true)
-	})
-}

pkg/services/object/acl/v2/errors.go

@@ -26,13 +26,6 @@ const (
 	accessDeniedEACLReasonFmt = "access to operation %s is denied by extended ACL check: %v"
 )
 
-func basicACLErr(info RequestInfo) error {
-	errAccessDenied := &apistatus.ObjectAccessDenied{}
-	errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedACLReasonFmt, info.operation))
-
-	return errAccessDenied
-}
-
 func eACLErr(info RequestInfo, err error) error {
 	errAccessDenied := &apistatus.ObjectAccessDenied{}
 	errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedEACLReasonFmt, info.operation, err))

pkg/services/object/acl/v2/errors_test.go

@@ -8,16 +8,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestBasicACLErr(t *testing.T) {
-	var reqInfo RequestInfo
-	err := basicACLErr(reqInfo)
-
-	var errAccessDenied *apistatus.ObjectAccessDenied
-
-	require.ErrorAs(t, err, &errAccessDenied,
-		"basicACLErr must be able to be casted to apistatus.ObjectAccessDenied")
-}
-
 func TestEACLErr(t *testing.T) {
 	var reqInfo RequestInfo
 	testErr := errors.New("test-eacl")

pkg/services/object/acl/v2/request.go

@@ -104,13 +104,6 @@ func (r RequestInfo) RequestRole() acl.Role {
 	return r.requestRole
 }
 
-// IsSoftAPECheck states if APE should perform soft checks.
-// Soft APE check allows a request if CheckAPE returns NoRuleFound for it,
-// otherwise it denies the request.
-func (r RequestInfo) IsSoftAPECheck() bool {
-	return r.BasicACL().Bits() != 0
-}
-
 // MetaWithToken groups session and bearer tokens,
 // verification header and raw API request.
 type MetaWithToken struct {

pkg/services/object/acl/v2/service.go

@@ -123,7 +123,6 @@ func (w *wrappedGetObjectStream) Context() context.Context {
 		ContainerOwner: w.requestInfo.ContainerOwner(),
 		SenderKey:      w.requestInfo.SenderKey(),
 		Role:           w.requestInfo.RequestRole(),
-		SoftAPECheck:   w.requestInfo.IsSoftAPECheck(),
 		BearerToken:    w.requestInfo.Bearer(),
 	})
 }
@@ -149,7 +148,6 @@ func (w *wrappedRangeStream) Context() context.Context {
 		ContainerOwner: w.requestInfo.ContainerOwner(),
 		SenderKey:      w.requestInfo.SenderKey(),
 		Role:           w.requestInfo.RequestRole(),
-		SoftAPECheck:   w.requestInfo.IsSoftAPECheck(),
 		BearerToken:    w.requestInfo.Bearer(),
 	})
 }
@@ -175,7 +173,6 @@ func (w *wrappedSearchStream) Context() context.Context {
 		ContainerOwner: w.requestInfo.ContainerOwner(),
 		SenderKey:      w.requestInfo.SenderKey(),
 		Role:           w.requestInfo.RequestRole(),
-		SoftAPECheck:   w.requestInfo.IsSoftAPECheck(),
 		BearerToken:    w.requestInfo.Bearer(),
 	})
 }
@@ -231,14 +228,6 @@ func (b Service) Get(request *objectV2.GetRequest, stream object.GetObjectStream
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.Get(request, &getStreamBasicChecker{
 		GetObjectStream: newWrappedGetObjectStreamStream(stream, reqInfo),
 		info:            reqInfo,
@@ -309,14 +298,6 @@ func (b Service) Head(
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return nil, basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return nil, eACLErr(reqInfo, err)
-		}
-	}
-
 	resp, err := b.next.Head(requestContext(ctx, reqInfo), request)
 	if err == nil {
 		if err = b.checker.CheckEACL(resp, reqInfo); err != nil {
@@ -362,14 +343,6 @@ func (b Service) Search(request *objectV2.SearchRequest, stream object.SearchStr
 		return err
 	}
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.Search(request, &searchStreamBasicChecker{
 		checker:      b.checker,
 		SearchStream: newWrappedSearchStream(stream, reqInfo),
@@ -422,14 +395,6 @@ func (b Service) Delete(
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return nil, basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return nil, eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.Delete(requestContext(ctx, reqInfo), request)
 }
 
@@ -475,14 +440,6 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.GetRange(request, &rangeStreamBasicChecker{
 		checker:              b.checker,
 		GetObjectRangeStream: newWrappedRangeStream(stream, reqInfo),
@@ -496,7 +453,6 @@ func requestContext(ctx context.Context, reqInfo RequestInfo) context.Context {
 		ContainerOwner: reqInfo.ContainerOwner(),
 		SenderKey:      reqInfo.SenderKey(),
 		Role:           reqInfo.RequestRole(),
-		SoftAPECheck:   reqInfo.IsSoftAPECheck(),
 		BearerToken:    reqInfo.Bearer(),
 	})
 }
@@ -546,14 +502,6 @@ func (b Service) GetRangeHash(
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) {
-			return nil, basicACLErr(reqInfo)
-		} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return nil, eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.GetRangeHash(requestContext(ctx, reqInfo), request)
 }
 
@@ -605,15 +553,6 @@ func (b Service) PutSingle(ctx context.Context, request *objectV2.PutSingleReque
 
 	reqInfo.obj = obj
 
-	if reqInfo.IsSoftAPECheck() {
-		if !b.checker.CheckBasicACL(reqInfo) || !b.checker.StickyBitCheck(reqInfo, idOwner) {
-			return nil, basicACLErr(reqInfo)
-		}
-		if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-			return nil, eACLErr(reqInfo, err)
-		}
-	}
-
 	return b.next.PutSingle(requestContext(ctx, reqInfo), request)
 }
 
@@ -679,12 +618,6 @@ func (p putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutRe
 
 		reqInfo.obj = obj
 
-		if reqInfo.IsSoftAPECheck() {
-			if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) {
-				return basicACLErr(reqInfo)
-			}
-		}
-
 		ctx = requestContext(ctx, reqInfo)
 	}
 

pkg/services/object/acl/v2/types.go

@@ -1,22 +1,11 @@
 package v2
 
-import (
-	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
-)
-
 // ACLChecker is an interface that must provide
 // ACL related checks.
 type ACLChecker interface {
-	// CheckBasicACL must return true only if request
-	// passes basic ACL validation.
-	CheckBasicACL(RequestInfo) bool
 	// CheckEACL must return non-nil error if request
 	// doesn't pass extended ACL validation.
 	CheckEACL(any, RequestInfo) error
-	// StickyBitCheck must return true only if sticky bit
-	// is disabled or enabled but request contains correct
-	// owner field.
-	StickyBitCheck(RequestInfo, user.ID) bool
 }
 
 // InnerRingFetcher is an interface that must provide

pkg/services/object/ape/checker.go

@@ -64,9 +64,6 @@ type Prm struct {
 	// An encoded container's owner user ID.
 	ContainerOwner user.ID
 
-	// If SoftAPECheck is set to true, then NoRuleFound is interpreted as allow.
-	SoftAPECheck bool
-
 	// The request's bearer token. It is used in order to check APE overrides with the token.
 	BearerToken *bearer.Token
 
@@ -109,6 +106,5 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error {
 		Container:      prm.Container,
 		ContainerOwner: prm.ContainerOwner,
 		BearerToken:    prm.BearerToken,
-		SoftAPECheck:   prm.SoftAPECheck,
 	})
 }

pkg/services/object/ape/service.go

@@ -84,8 +84,6 @@ type getStreamBasicChecker struct {
 
 	role string
 
-	softAPECheck bool
-
 	bearerToken *bearer.Token
 }
 
@@ -105,7 +103,6 @@ func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error {
 			SenderKey:      hex.EncodeToString(g.senderKey),
 			ContainerOwner: g.containerOwner,
 			Role:           g.role,
-			SoftAPECheck:   g.softAPECheck,
 			BearerToken:    g.bearerToken,
 			XHeaders:       resp.GetMetaHeader().GetXHeaders(),
 		}
@@ -142,7 +139,6 @@ func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectSt
 		senderKey:       reqCtx.SenderKey,
 		containerOwner:  reqCtx.ContainerOwner,
 		role:            nativeSchemaRole(reqCtx.Role),
-		softAPECheck:    reqCtx.SoftAPECheck,
 		bearerToken:     reqCtx.BearerToken,
 	})
 }
@@ -174,7 +170,6 @@ func (p *putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutR
 			SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 			ContainerOwner: reqCtx.ContainerOwner,
 			Role:           nativeSchemaRole(reqCtx.Role),
-			SoftAPECheck:   reqCtx.SoftAPECheck,
 			BearerToken:    reqCtx.BearerToken,
 			XHeaders:       request.GetMetaHeader().GetXHeaders(),
 		}
@@ -230,7 +225,6 @@ func (p *patchStreamBasicChecker) Send(ctx context.Context, request *objectV2.Pa
 			SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 			ContainerOwner: reqCtx.ContainerOwner,
 			Role:           nativeSchemaRole(reqCtx.Role),
-			SoftAPECheck:   reqCtx.SoftAPECheck,
 			BearerToken:    reqCtx.BearerToken,
 			XHeaders:       request.GetMetaHeader().GetXHeaders(),
 		}
@@ -300,7 +294,6 @@ func (c *Service) Head(ctx context.Context, request *objectV2.HeadRequest) (*obj
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	})
@@ -330,7 +323,6 @@ func (c *Service) Search(request *objectV2.SearchRequest, stream objectSvc.Searc
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	})
@@ -360,7 +352,6 @@ func (c *Service) Delete(ctx context.Context, request *objectV2.DeleteRequest) (
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	})
@@ -395,7 +386,6 @@ func (c *Service) GetRange(request *objectV2.GetRangeRequest, stream objectSvc.G
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	})
@@ -425,7 +415,6 @@ func (c *Service) GetRangeHash(ctx context.Context, request *objectV2.GetRangeHa
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	}
@@ -461,7 +450,6 @@ func (c *Service) PutSingle(ctx context.Context, request *objectV2.PutSingleRequ
 		Role:           nativeSchemaRole(reqCtx.Role),
 		SenderKey:      hex.EncodeToString(reqCtx.SenderKey),
 		ContainerOwner: reqCtx.ContainerOwner,
-		SoftAPECheck:   reqCtx.SoftAPECheck,
 		BearerToken:    reqCtx.BearerToken,
 		XHeaders:       request.GetMetaHeader().GetXHeaders(),
 	}

pkg/services/object/request_context.go

@@ -20,7 +20,5 @@ type RequestContext struct {
 
 	Role acl.Role
 
-	SoftAPECheck bool
-
 	BearerToken *bearer.Token
 }

pkg/services/tree/ape.go

@@ -81,7 +81,6 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token,
 		ContainerOwner: container.Value.Owner(),
 		PublicKey:      publicKey,
 		BearerToken:    bt,
-		SoftAPECheck:   false,
 	})
 }