Factor out ACL middleware in object service #1052

New issue

Open

opened 2024-03-20 12:31:28 +00:00 by aarifullin · 0 comments

aarifullin commented

2024-03-20 12:31:28 +00:00

Member

Since APE was introduced in object service, ACL is getting irrelevant to check request permissions.

For the context: if a container is created with zero-filled basic acl (--basic-acl 0), then hard ape check is performed and acl/eacl checks are skipped like here. Thus ACL/eACL checks can be disabled.

Still passing a request through ACL middleware is very important at least because of findRequestInfo. The method perfoms useful calculations that are passed within RequestContext (see how it is initialized). The formed request context is used in the next middleware (APE).
These steps should be borrowed from findRequestInfo:

Check for session token expiration
Assert verb
Get request owner and classify its role
Get container onwer
Get namespace - very important
Get sendery key
~~questionable~~: Get bearer. Note that bearer token may set impersonate flag that means it should be wiped. Bearer token was used to retrieve eACL table
These values apart from bearer token are already passsed within RequestContext.

So, it makes sense to not entirely nuke out ACL middleware but rename it to like common and refactor it:

Remove all things related to acl/eacl checks (CheckBasicACL, CheckEACL etc.)
Slightly refactor findRequestInfo method (see above)
Keep session token checks
Remove irrelevant files from the package

Since APE was introduced in object service, ACL is getting irrelevant to check request permissions. For the context: if a container is created with zero-filled basic acl (`--basic-acl 0`), then **hard** ape check is performed **and** `acl/eacl` checks are **skipped** like [here](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L460-L466). Thus ACL/eACL checks can be disabled. Still passing a request through ACL middleware is very important at least because of [findRequestInfo](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L733-L788). The method perfoms useful calculations that are passed within [RequestContext](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/request_context.go) (see how it is [initialized](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L114-L122)). The formed request context is used in the next middleware (APE). These steps should be borrowed from [findRequestInfo](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L733-L788): - Check for session token [expiration](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L739-L750) - [Assert verb](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L752-L754) - Get request owner and [classify its role](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L758-L765) - Get [container onwer](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L770) - Get [namespace](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L773-L776) - **very important** - Get [sendery key](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L780) - ~~*questionable*~~: Get [bearer](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/v2/service.go#L783). Note that bearer token may set `impersonate` flag that means it should be [wiped](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/acl/acl.go#L113-L115). *Bearer token was used to retrieve eACL table* These values apart from bearer token are already passsed within RequestContext. So, it makes sense to not entirely nuke out `ACL` middleware but rename it to like `common` and refactor it: - Remove all things related to acl/eacl checks (`CheckBasicACL`, `CheckEACL` etc.) - Slightly refactor `findRequestInfo` method (see above) - Keep session token checks - Remove irrelevant files from the package