From af999345a669ccf5427eeafd828eac05b3bbf28e Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:29:38 +0200 Subject: [PATCH] refactor: the public link reduction lives once, in reva publicshare.ReduceResourceInfo/ReducePermissions replace both copies: the publicstorageprovider's private filterPermissions plus its augment path rewrite, and graph's reimplementation. Graph resolves the link of the request (share root and grant, one GetPublicShare and one Stat) and applies the same reduction the provider applies to its own responses. Paths below the public drive are share-root relative now, on every route. Vendored reva change, goes into the reva PR later. --- services/graph/pkg/service/v0/driveitems.go | 76 +++++++------------ tests/acceptance/docker/src/publink-local.yml | 4 + .../publicstorageprovider.go | 36 +-------- .../reva/v2/pkg/publicshare/reduce.go | 73 ++++++++++++++++++ 4 files changed, 109 insertions(+), 80 deletions(-) create mode 100644 tests/acceptance/docker/src/publink-local.yml create mode 100644 vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 7b5fc2b0d6..6c06be893d 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -83,73 +83,55 @@ func publicDriveRequest(r *http.Request) bool { driveID.GetSpaceId() == utils.PublicStorageSpaceID } -// sanitizePublicDriveInfos prepares resource infos for a public link response. -// Navigating by id bypasses the publicstorageprovider, so the infos are the -// owner's: paths are anchored at the owner's space root (everything above the -// share root must not leak) and the permission sets are the owner's, not the -// link's. Paths are cut to their base name and permissions intersected with -// what the link grants; when the link cannot be resolved nothing is advertised. +// sanitizePublicDriveInfos prepares resource infos for a public link +// response. Navigating by id bypasses the publicstorageprovider, so the infos +// are the owner's; publicshare.ReduceResourceInfo is the provider's own +// reduction: paths relative to the share root, permissions cut to the link +// grant. When the link cannot be resolved nothing is advertised. func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) { - linkPermissions := g.publicLinkPermissions(ctx, r) + shareRoot, grant := g.publicLinkOfRequest(ctx, r) for _, info := range infos { if info == nil { continue } - if info.Path != "" { - info.Path = path.Base(info.Path) - } - if info.PermissionSet != nil { - intersectPermissions(info.PermissionSet, linkPermissions) + if shareRoot == nil { + info.Path = path.Base(info.GetPath()) + info.PermissionSet = &storageprovider.ResourcePermissions{} + continue } + publicshare.ReduceResourceInfo(info, shareRoot, grant) } } -// publicLinkPermissions resolves the permissions the public link grants. The -// link token is the opaque id of the public drive; the token scope permits -// reading exactly this one share. -func (g Graph) publicLinkPermissions(ctx context.Context, r *http.Request) *storageprovider.ResourcePermissions { +// publicLinkOfRequest resolves the public link the request runs in: the share +// root info and the granted permissions. The link token is the opaque id of +// the public drive; the token scope permits reading exactly this one share. +func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions) { driveID, err := parseIDParam(r, "driveID") if err != nil { - return nil + return nil, nil } gatewayClient, err := g.gatewaySelector.Next() if err != nil { - return nil + return nil, nil } - resp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ + shareResp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ Ref: &link.PublicShareReference{ Spec: &link.PublicShareReference_Token{Token: driveID.GetOpaqueId()}, }, }) - if err != nil || resp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { - g.logger.Error().Err(err).Str("status", resp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") - return nil + if err != nil || shareResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { + g.logger.Error().Err(err).Str("status", shareResp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") + return nil, nil } - return resp.GetShare().GetPermissions().GetPermissions() -} - -// intersectPermissions reduces l to what r also grants, the same reduction the -// publicstorageprovider applies on its own responses. A nil r clears l. -func intersectPermissions(l, r *storageprovider.ResourcePermissions) { - l.AddGrant = l.AddGrant && r.GetAddGrant() - l.CreateContainer = l.CreateContainer && r.GetCreateContainer() - l.Delete = l.Delete && r.GetDelete() - l.GetPath = l.GetPath && r.GetGetPath() - l.GetQuota = l.GetQuota && r.GetGetQuota() - l.InitiateFileDownload = l.InitiateFileDownload && r.GetInitiateFileDownload() - l.InitiateFileUpload = l.InitiateFileUpload && r.GetInitiateFileUpload() - l.ListContainer = l.ListContainer && r.GetListContainer() - l.ListFileVersions = l.ListFileVersions && r.GetListFileVersions() - l.ListGrants = l.ListGrants && r.GetListGrants() - l.ListRecycle = l.ListRecycle && r.GetListRecycle() - l.Move = l.Move && r.GetMove() - l.PurgeRecycle = l.PurgeRecycle && r.GetPurgeRecycle() - l.RemoveGrant = l.RemoveGrant && r.GetRemoveGrant() - l.RestoreFileVersion = l.RestoreFileVersion && r.GetRestoreFileVersion() - l.RestoreRecycleItem = l.RestoreRecycleItem && r.GetRestoreRecycleItem() - l.Stat = l.Stat && r.GetStat() - l.UpdateGrant = l.UpdateGrant && r.GetUpdateGrant() - l.DenyGrant = l.DenyGrant && r.GetDenyGrant() + statResp, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{ + Ref: &storageprovider.Reference{ResourceId: shareResp.GetShare().GetResourceId()}, + }) + if err != nil || statResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { + g.logger.Error().Err(err).Str("status", statResp.GetStatus().GetCode().String()).Msg("could not stat the public link root") + return nil, nil + } + return statResp.GetInfo(), shareResp.GetShare().GetPermissions().GetPermissions() } // driveItemPropertySelected reports whether the given opt-in property was requested via $select diff --git a/tests/acceptance/docker/src/publink-local.yml b/tests/acceptance/docker/src/publink-local.yml new file mode 100644 index 0000000000..d380d09b3c --- /dev/null +++ b/tests/acceptance/docker/src/publink-local.yml @@ -0,0 +1,4 @@ +services: + opencloud-server: + ports: !override + - "9402:9200" diff --git a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 1a1ce89f94..f854fff2f3 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -23,7 +23,6 @@ package publicstorageprovider import ( "context" "encoding/json" - "path" "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -37,6 +36,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/appctx" ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/errtypes" + "github.com/opencloud-eu/reva/v2/pkg/publicshare" "github.com/opencloud-eu/reva/v2/pkg/rgrpc" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -769,15 +769,7 @@ func (s *service) augmentStatResponse(ctx context.Context, statInfo *provider.Re appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", statInfo).Msg("error when adding share") } - var sharePath string - if shareInfo.Type == provider.ResourceType_RESOURCE_TYPE_FILE { - sharePath = path.Base(shareInfo.Path) - } else { - sharePath = strings.TrimPrefix(statInfo.Path, shareInfo.Path) - } - - statInfo.Path = path.Join("/", sharePath) - filterPermissions(statInfo.PermissionSet, shareInfo.PermissionSet) + publicshare.ReduceResourceInfo(statInfo, shareInfo, shareInfo.GetPermissionSet()) } } @@ -843,7 +835,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer for i := range listContainerR.Infos { // FIXME how do we reduce permissions to what is granted by the public link? // only a problem for id based access -> middleware - filterPermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) + publicshare.ReducePermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) if err := addShare(listContainerR.Infos[i], share); err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", listContainerR.Infos[i]).Msg("error when adding share") } @@ -852,28 +844,6 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer return listContainerR, nil } -func filterPermissions(l *provider.ResourcePermissions, r *provider.ResourcePermissions) { - l.AddGrant = l.AddGrant && r.AddGrant - l.CreateContainer = l.CreateContainer && r.CreateContainer - l.Delete = l.Delete && r.Delete - l.GetPath = l.GetPath && r.GetPath - l.GetQuota = l.GetQuota && r.GetQuota - l.InitiateFileDownload = l.InitiateFileDownload && r.InitiateFileDownload - l.InitiateFileUpload = l.InitiateFileUpload && r.InitiateFileUpload - l.ListContainer = l.ListContainer && r.ListContainer - l.ListFileVersions = l.ListFileVersions && r.ListFileVersions - l.ListGrants = l.ListGrants && r.ListGrants - l.ListRecycle = l.ListRecycle && r.ListRecycle - l.Move = l.Move && r.Move - l.PurgeRecycle = l.PurgeRecycle && r.PurgeRecycle - l.RemoveGrant = l.RemoveGrant && r.RemoveGrant - l.RestoreFileVersion = l.RestoreFileVersion && r.RestoreFileVersion - l.RestoreRecycleItem = l.RestoreRecycleItem && r.RestoreRecycleItem - l.Stat = l.Stat && r.Stat - l.UpdateGrant = l.UpdateGrant && r.UpdateGrant - l.DenyGrant = l.DenyGrant && r.DenyGrant -} - func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVersionsRequest) (*provider.ListFileVersionsResponse, error) { return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go new file mode 100644 index 0000000000..7e0e471af4 --- /dev/null +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go @@ -0,0 +1,73 @@ +// Copyright 2018-2026 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package publicshare + +import ( + "path" + "strings" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" +) + +// ReduceResourceInfo rewrites a resource info for a public link consumer. The +// path becomes relative to the share root, so the owner's directory structure +// above the share does not leak, and the permissions are cut to what the link +// grants. The publicstorageprovider applies it to everything it serves; +// consumers that reach resources inside a link by id bypass that provider and +// apply it themselves. +func ReduceResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { + if info == nil { + return + } + + var sharePath string + if shareRoot.GetType() == provider.ResourceType_RESOURCE_TYPE_FILE { + sharePath = path.Base(shareRoot.GetPath()) + } else { + sharePath = strings.TrimPrefix(info.GetPath(), shareRoot.GetPath()) + } + info.Path = path.Join("/", sharePath) + + if info.PermissionSet != nil { + ReducePermissions(info.PermissionSet, grant) + } +} + +// ReducePermissions reduces l to what r also grants. A nil r clears l. +func ReducePermissions(l, r *provider.ResourcePermissions) { + l.AddGrant = l.AddGrant && r.GetAddGrant() + l.CreateContainer = l.CreateContainer && r.GetCreateContainer() + l.Delete = l.Delete && r.GetDelete() + l.DenyGrant = l.DenyGrant && r.GetDenyGrant() + l.GetPath = l.GetPath && r.GetGetPath() + l.GetQuota = l.GetQuota && r.GetGetQuota() + l.InitiateFileDownload = l.InitiateFileDownload && r.GetInitiateFileDownload() + l.InitiateFileUpload = l.InitiateFileUpload && r.GetInitiateFileUpload() + l.ListContainer = l.ListContainer && r.GetListContainer() + l.ListFileVersions = l.ListFileVersions && r.GetListFileVersions() + l.ListGrants = l.ListGrants && r.GetListGrants() + l.ListRecycle = l.ListRecycle && r.GetListRecycle() + l.Move = l.Move && r.GetMove() + l.PurgeRecycle = l.PurgeRecycle && r.GetPurgeRecycle() + l.RemoveGrant = l.RemoveGrant && r.GetRemoveGrant() + l.RestoreFileVersion = l.RestoreFileVersion && r.GetRestoreFileVersion() + l.RestoreRecycleItem = l.RestoreRecycleItem && r.GetRestoreRecycleItem() + l.Stat = l.Stat && r.GetStat() + l.UpdateGrant = l.UpdateGrant && r.GetUpdateGrant() +}