Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip write permission checks for read-only RPC calls #141

Open
3 tasks done
grishy opened this issue Feb 23, 2025 · 1 comment
Open
3 tasks done

Skip write permission checks for read-only RPC calls #141

grishy opened this issue Feb 23, 2025 · 1 comment
Labels
bug Something isn't working

Comments

@grishy
Copy link

grishy commented Feb 23, 2025

Have you read a contributing guide?

  • I have read CONTRIBUTING.md
  • I have searched the existing issues and didn't find any that were similar
  • I have considered creating a pull request with fixes instead of a bug report and want to proceed

Current Behavior

Thank you for the project ❤️
I'm developing a lightweight server for Anytype, reimplementing RPC for self-hosting.

I've observed that user verification currently requires confirming that a user has access not only to the Space but also to individual records. This seems redundant for read-only operations RPC.

Example

For the RPC method SpaceInfo, which returns the status of a Space:

func (r rpcHandler) SpaceInfo(ctx context.Context, req *fileproto.SpaceInfoRequest) (resp *fileproto.SpaceInfoResponse, err error) {
st := time.Now()
defer func() {
r.f.metric.RequestLog(ctx,
"file.spaceInfo",
metric.TotalDur(time.Since(st)),
metric.SpaceId(req.SpaceId),
zap.Error(err),
)
}()
if resp, err = r.f.SpaceInfo(ctx, req.SpaceId); err != nil {
return
}
return
}

There's a call to StoreKey that always checks permissions.CanWrite():

if identity.Account() != storageKey.GroupId {
permissions, err := fn.acl.Permissions(ctx, identity, spaceId)
if err != nil {
log.WarnCtx(ctx, "acl permissions error", zap.Error(err))
return storageKey, fileprotoerr.ErrForbidden
}
if !permissions.CanWrite() {
return storageKey, fileprotoerr.ErrForbidden
}
}

Expected Behavior

Consider verifying only that an account has access to the space without checking permissions.CanWrite() in all RPC calls. It would be beneficial to differentiate between CanRead and CanWrite permissions for all RPC operations.

Steps To Reproduce

N/A

Environment

N/A

Anything else?

No response

@grishy grishy added the bug Something isn't working label Feb 23, 2025
@grishy
Copy link
Author

grishy commented Feb 23, 2025

@cheggaaa @requilence is here any problem, if we will change it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant