From 84b434c147615e5bf10dc3450090dcd249b81b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 17 Mar 2022 08:43:30 +0000 Subject: [PATCH 01/10] introduce scope context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/ctx/scopectx.go | 36 ++++++++++++++++++++++++++++++++++++ pkg/ctx/userctx.go | 1 + 2 files changed, 37 insertions(+) create mode 100644 pkg/ctx/scopectx.go diff --git a/pkg/ctx/scopectx.go b/pkg/ctx/scopectx.go new file mode 100644 index 0000000000..0d0dae20c4 --- /dev/null +++ b/pkg/ctx/scopectx.go @@ -0,0 +1,36 @@ +// Copyright 2018-2021 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 ctx + +import ( + "context" + + auth "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" +) + +// ContextGetScopes returns the scopes if set in the given context. +func ContextGetScopes(ctx context.Context) (map[string]*auth.Scope, bool) { + s, ok := ctx.Value(scopeKey).(map[string]*auth.Scope) + return s, ok +} + +// ContextSetScopes stores the scopes in the context. +func ContextSetScopes(ctx context.Context, s map[string]*auth.Scope) context.Context { + return context.WithValue(ctx, scopeKey, s) +} diff --git a/pkg/ctx/userctx.go b/pkg/ctx/userctx.go index 3c50df1879..4fbee7444f 100644 --- a/pkg/ctx/userctx.go +++ b/pkg/ctx/userctx.go @@ -31,6 +31,7 @@ const ( tokenKey idKey lockIDKey + scopeKey ) // ContextGetUser returns the user if set in the given context. From 6eaeb63789008d1dd80bc8f9a18bd697aef334ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 17 Mar 2022 08:44:10 +0000 Subject: [PATCH 02/10] add scopes to context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/grpc/interceptors/auth/auth.go | 35 ++++++++++++++++--------- internal/http/interceptors/auth/auth.go | 3 +++ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index 3d339290b4..3ecdb6cd1e 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -23,6 +23,7 @@ import ( "time" "github.com/bluele/gcache" + authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" @@ -96,9 +97,11 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI // to decide the storage provider. tkn, ok := ctxpkg.ContextGetToken(ctx) if ok { - u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, false) + u, tokenScope, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, false) if err == nil { + // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) + ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) } } return handler(ctx, req) @@ -112,13 +115,15 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI } // validate the token and ensure access to the resource is allowed - u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, true) + u, tokenScope, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, true) if err != nil { log.Warn().Err(err).Msg("access token is invalid") return nil, status.Errorf(codes.PermissionDenied, "auth: core access token is invalid") } + // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) + ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) return handler(ctx, req) } return interceptor, nil @@ -159,9 +164,11 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe // to decide the storage provider. tkn, ok := ctxpkg.ContextGetToken(ctx) if ok { - u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, false) + u, tokenScope, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, false) if err == nil { + // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) + ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) ss = newWrappedServerStream(ctx, ss) } } @@ -177,14 +184,15 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe } // validate the token and ensure access to the resource is allowed - u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, true) + u, tokenScope, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, true) if err != nil { log.Warn().Err(err).Msg("access token is invalid") return status.Errorf(codes.PermissionDenied, "auth: core access token is invalid") } - // store user and core access token in context. + // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) + ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) wrapped := newWrappedServerStream(ctx, ss) return handler(srv, wrapped) } @@ -204,21 +212,22 @@ func (ss *wrappedServerStream) Context() context.Context { return ss.newCtx } -func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string, fetchUserGroups bool) (*userpb.User, error) { +// dismantleToken extraclts the user and scopes from the reva access token +func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string, fetchUserGroups bool) (*userpb.User, map[string]*authpb.Scope, error) { u, tokenScope, err := mgr.DismantleToken(ctx, tkn) if err != nil { - return nil, err + return nil, nil, err } client, err := pool.GetGatewayServiceClient(gatewayAddr) if err != nil { - return nil, err + return nil, nil, err } if sharedconf.SkipUserGroupsInToken() && fetchUserGroups { groups, err := getUserGroups(ctx, u, client) if err != nil { - return nil, err + return nil, nil, err } u.Groups = groups } @@ -226,17 +235,17 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token. // Check if access to the resource is in the scope of the token ok, err := scope.VerifyScope(ctx, tokenScope, req) if err != nil { - return nil, errtypes.InternalError("error verifying scope of access token") + return nil, nil, errtypes.InternalError("error verifying scope of access token") } if ok { - return u, nil + return u, tokenScope, nil } if err = expandAndVerifyScope(ctx, req, tokenScope, gatewayAddr, mgr); err != nil { - return nil, err + return nil, nil, err } - return u, nil + return u, tokenScope, nil } func getUserGroups(ctx context.Context, u *userpb.User, client gatewayv1beta1.GatewayAPIClient) ([]string, error) { diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 102ede554b..85fb88b86f 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -284,6 +284,9 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.UserAgentHeader, r.UserAgent()) + // store scopes in context + ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) + r = r.WithContext(ctx) h.ServeHTTP(w, r) }) From 04bae06d242a43547fec0dbf17bc4fb5dec3150b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 17 Mar 2022 20:58:11 +0000 Subject: [PATCH 03/10] use mrants and mountpoints for public shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../publicstorageprovider.go | 294 +++++++++++------- internal/http/services/owncloud/ocdav/dav.go | 3 +- .../owncloud/ocdav/propfind/propfind.go | 20 +- pkg/auth/scope/publicshare.go | 27 +- 4 files changed, 216 insertions(+), 128 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 4f496143ea..1dd6e9a00c 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -25,12 +25,13 @@ import ( "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" - "github.com/cs3org/reva/v2/pkg/errtypes" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc" "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" @@ -163,12 +164,19 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider.Reference) (*provider.Reference, string, *link.PublicShare, *rpc.Status, error) { log := appctx.GetLogger(ctx) - tkn, opaqueid, relativePath, err := s.unwrap(ctx, ref) - if err != nil { - return nil, "", nil, nil, err + + share, _, ok := extractLinkAndInfo(ctx) + if !ok { + return nil, "", nil, nil, gstatus.Errorf(codes.NotFound, "share or token not found") } + /* + tkn, opaqueid, relativePath, err := s.unwrap(ctx, ref) + if err != nil { + return nil, "", nil, nil, err + } + */ - ls, shareInfo, st, err := s.resolveToken(ctx, tkn) + ls, shareInfo, st, err := s.resolveToken(ctx, share.Token) switch { case err != nil: return nil, "", nil, nil, err @@ -180,7 +188,7 @@ func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider. switch shareInfo.Type { case provider.ResourceType_RESOURCE_TYPE_CONTAINER: // folders point to the folder -> path needs to be added - path = utils.MakeRelativePath(relativePath) + path = utils.MakeRelativePath(ref.Path) case provider.ResourceType_RESOURCE_TYPE_FILE: // files already point to the correct id path = "." @@ -189,27 +197,20 @@ func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider. // path = utils.MakeRelativePath(relativePath) } - if opaqueid == "" { - opaqueid = shareInfo.Id.OpaqueId - } - cs3Ref := &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: shareInfo.Id.StorageId, - OpaqueId: opaqueid, - }, - Path: path, + ResourceId: shareInfo.Id, + Path: path, } log.Debug(). Interface("sourceRef", ref). Interface("cs3Ref", cs3Ref). Interface("share", ls). - Str("tkn", tkn). + Str("tkn", share.Token). Str("originalPath", shareInfo.Path). - Str("relativePath", relativePath). + Str("relativePath", path). Msg("translatePublicRefToCS3Ref") - return cs3Ref, tkn, ls, nil, nil + return cs3Ref, share.Token, ls, nil, nil } // Both, t.dir and tokenPath paths need to be merged: @@ -350,49 +351,160 @@ func (s *service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt // 2. look up the storage space using ListStorageSpaces. // 3. make related requests to that (spaceid, nodeid) func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) { + spaceTypes := map[string]struct{}{} + var exists = struct{}{} + appendTypes := []string{} + var spaceID *provider.ResourceId for _, f := range req.Filters { switch f.Type { case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: - switch f.GetSpaceType() { - case SpaceTypePublic: + spaceType := f.GetSpaceType() + // do we need to fetch the shares? + if spaceType == "+mountpoint" || spaceType == "+grant" { + appendTypes = append(appendTypes, strings.TrimPrefix(spaceType, "+")) continue - case "mountpoint", "+mountpoint": - continue - default: - return &provider.ListStorageSpacesResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - }, nil } + spaceTypes[spaceType] = exists case provider.ListStorageSpacesRequest_Filter_TYPE_ID: - spaceid, _, err := utils.SplitStorageSpaceID(f.GetId().OpaqueId) + spaceid, shareid, err := utils.SplitStorageSpaceID(f.GetId().OpaqueId) if err != nil { continue } if spaceid != utils.PublicStorageProviderID { return &provider.ListStorageSpacesResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + // a specific id was requested, return not found instead of empty list + Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}, }, nil } + spaceID = &provider.ResourceId{StorageId: spaceid, OpaqueId: shareid} } } - return &provider.ListStorageSpacesResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - StorageSpaces: []*provider.StorageSpace{{ - Id: &provider.StorageSpaceId{ - OpaqueId: utils.PublicStorageProviderID, - }, - SpaceType: SpaceTypePublic, - // return the actual resource id? - Root: &provider.ResourceId{ + // if there is no public scope there are no publicstorage spaces + share, _, ok := extractLinkAndInfo(ctx) + if !ok { + // TODO when the ocdav publicfile handler sends the token as opaqueid the + // public scope needs to change + // it always needs to check if the accessed path is a child of the resource identified by + // the token + return &provider.ListStorageSpacesResponse{ + Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + }, nil + } + + if len(spaceTypes) == 0 { + spaceTypes["mountpoint"] = exists + } + for _, s := range appendTypes { + spaceTypes[s] = exists + } + + res := &provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + } + for k := range spaceTypes { + switch k { + case "grant": + // when a list storage space with the resourceid of an external + // resource is made we may have a grant for it + root := share.ResourceId + // do we filter by id? + if spaceID != nil && !utils.ResourceIDEqual(spaceID, root) { + // none of our business + continue + } + /* + var opaque *typesv1beta1.Opaque + if etag, ok := shareEtags[share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + } + */ + // we know a grant for this resource + space := &provider.StorageSpace{ + //Opaque: opaque, + Id: &provider.StorageSpaceId{ + OpaqueId: root.StorageId + "!" + root.OpaqueId, + }, + SpaceType: "grant", + Owner: &userv1beta1.User{Id: share.Owner}, + // the publicstorageprovider keeps track of mount points + Root: root, + } + + res.StorageSpaces = append(res.StorageSpaces, space) + case "mountpoint": + root := &provider.ResourceId{ StorageId: utils.PublicStorageProviderID, - OpaqueId: utils.PublicStorageProviderID, - }, - Name: "Public shares", - Mtime: &typesv1beta1.Timestamp{}, // do we need to update it? - }}, - }, nil + OpaqueId: share.Token, // the link share has no id, only the token + } + // do we filter by id + if spaceID != nil { + switch { + case utils.ResourceIDEqual(spaceID, root): + // we have a virtual node + case utils.ResourceIDEqual(spaceID, share.ResourceId): + // we have a mount point + root = share.ResourceId + default: + // none of our business + continue + } + } + /* + var opaque *typesv1beta1.Opaque + if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + } + */ + space := &provider.StorageSpace{ + //Opaque: opaque, + Id: &provider.StorageSpaceId{ + OpaqueId: root.StorageId + "!" + root.OpaqueId, + }, + SpaceType: "mountpoint", + Owner: &userv1beta1.User{Id: share.Owner}, // FIXME actually, the mount point belongs to the recipient + // the publicstorageprovider keeps track of mount points + Root: root, + } + + // "Public share ~token"? nah, the spaceregistry uses the token in the name as the mountpoint + // TODO use token in root opaqueid when building path + space.Name = share.Token + // what if we don't have a name? + res.StorageSpaces = append(res.StorageSpaces, space) + } + } + return res, nil +} + +func extractLinkAndInfo(ctx context.Context) (*link.PublicShare, *provider.ResourceInfo, bool) { + scopes, ok := ctxpkg.ContextGetScopes(ctx) + if !ok { + return nil, nil, false + } + var share *link.PublicShare + var info *provider.ResourceInfo + for k, v := range scopes { + switch { + case strings.HasPrefix(k, "publicshare:") && v.Resource.Decoder == "json": + share = &link.PublicShare{} + err := utils.UnmarshalJSONToProtoV1(v.Resource.Value, share) + if err != nil { + continue + } + case strings.HasPrefix(k, "resourceinfo:") && v.Resource.Decoder == "json": + info = &provider.ResourceInfo{} + err := utils.UnmarshalJSONToProtoV1(v.Resource.Value, info) + if err != nil { + continue + } + } + } + if share == nil || info == nil || !utils.ResourceIDEqual(share.ResourceId, info.Id) { + return nil, nil, false + } + return share, info, true } func (s *service) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) { @@ -569,12 +681,16 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide Value: attribute.StringValue(req.Ref.String()), }) - tkn, opaqueid, relativePath, err := s.unwrap(ctx, req.Ref) - if err != nil { - return nil, err + share, _, ok := extractLinkAndInfo(ctx) + if !ok { + return &provider.StatResponse{ + Status: status.NewNotFound(ctx, "share or token not found"), + }, nil } - share, shareInfo, st, err := s.resolveToken(ctx, tkn) + // the share is minimally populated, we need more than the token + // look up complete share + share, shareInfo, st, err := s.resolveToken(ctx, share.Token) switch { case err != nil: return nil, err @@ -588,25 +704,18 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide }, nil } - if shareInfo.Type == provider.ResourceType_RESOURCE_TYPE_FILE || relativePath == "" { + if shareInfo.Type == provider.ResourceType_RESOURCE_TYPE_FILE || req.Ref.Path == "" { res := &provider.StatResponse{ Status: status.NewOK(ctx), Info: shareInfo, } - s.augmentStatResponse(ctx, res, shareInfo, share, tkn) + s.augmentStatResponse(ctx, res, shareInfo, share, share.Token) return res, nil } - if opaqueid == "" { - opaqueid = share.ResourceId.OpaqueId - } - ref := &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: share.ResourceId.StorageId, - OpaqueId: opaqueid, - }, - Path: utils.MakeRelativePath(relativePath), + ResourceId: share.ResourceId, + Path: utils.MakeRelativePath(req.Ref.Path), } statResponse, err := s.gateway.Stat(ctx, &provider.StatRequest{Ref: ref}) @@ -616,7 +725,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide }, nil } - s.augmentStatResponse(ctx, statResponse, shareInfo, share, tkn) + s.augmentStatResponse(ctx, statResponse, shareInfo, share, share.Token) return statResponse, nil } @@ -644,7 +753,7 @@ func (s *service) augmentStatResponse(ctx context.Context, res *provider.StatRes // setPublicStorageID encodes the actual spaceid and nodeid as an opaqueid in the publicstorageprovider space func (s *service) setPublicStorageID(info *provider.ResourceInfo, shareToken string) { info.Id.StorageId = utils.PublicStorageProviderID - info.Id.OpaqueId = shareToken + "/" + info.Id.OpaqueId + info.Id.OpaqueId = shareToken } func addShare(i *provider.ResourceInfo, ls *link.PublicShare) error { @@ -667,12 +776,16 @@ func (s *service) ListContainerStream(req *provider.ListContainerStreamRequest, } func (s *service) ListContainer(ctx context.Context, req *provider.ListContainerRequest) (*provider.ListContainerResponse, error) { - tkn, opaqueid, relativePath, err := s.unwrap(ctx, req.Ref) - if err != nil { - return nil, err - } - share, shareInfo, st, err := s.resolveToken(ctx, tkn) + share, _, ok := extractLinkAndInfo(ctx) + if !ok { + return &provider.ListContainerResponse{ + Status: status.NewNotFound(ctx, "share or token not found"), + }, nil + } + // the share is minimally populated, we need more than the token + // look up complete share + share, _, st, err := s.resolveToken(ctx, share.Token) switch { case err != nil: return nil, err @@ -687,20 +800,13 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer }, nil } - if opaqueid == "" { - opaqueid = shareInfo.Id.OpaqueId - } - listContainerR, err := s.gateway.ListContainer( ctx, &provider.ListContainerRequest{ Ref: &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: shareInfo.Id.StorageId, - OpaqueId: opaqueid, - }, + ResourceId: share.ResourceId, // prefix relative path with './' to make it a CS3 relative path - Path: utils.MakeRelativePath(relativePath), + Path: utils.MakeRelativePath(req.Ref.Path), }, }, ) @@ -711,8 +817,9 @@ 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? filterPermissions(listContainerR.Infos[i].PermissionSet, share.GetPermissions().Permissions) - s.setPublicStorageID(listContainerR.Infos[i], tkn) + //s.setPublicStorageID(listContainerR.Infos[i], tkn) 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") } @@ -742,43 +849,6 @@ func filterPermissions(l *provider.ResourcePermissions, r *provider.ResourcePerm l.UpdateGrant = l.UpdateGrant && r.UpdateGrant } -func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (token, opaqueid, relativePath string, err error) { - isValidReference := func(r *provider.Reference) bool { - return r != nil && r.ResourceId != nil && r.ResourceId.StorageId != "" && r.ResourceId.OpaqueId != "" - } - - switch { - case !isValidReference(ref): - return "", "", "", errtypes.BadRequest("resourceid required, got " + ref.String()) - case utils.ResourceIDEqual(ref.ResourceId, &provider.ResourceId{ - StorageId: utils.PublicStorageProviderID, - OpaqueId: utils.PublicStorageProviderID, - }): - // path has the form "./{token}/relative/path/" - parts := strings.SplitN(ref.Path, "/", 3) - if len(parts) < 2 { - // FIXME ... we should expose every public link as a storage space - // but do we need to list them then? - return "", "", "", errtypes.BadRequest("need at least token in ref: got " + ref.String()) - } - token = parts[1] - if len(parts) > 2 { - relativePath = parts[2] - } - default: - // id based stat - parts := strings.SplitN(ref.ResourceId.OpaqueId, "/", 2) - if len(parts) < 2 { - return "", "", "", errtypes.BadRequest("OpaqueId needs to have form {token}/{shared node id}: got " + ref.String()) - } - token = parts[0] - opaqueid = parts[1] - relativePath = ref.Path - } - - return -} - 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/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go index 86f2f80a4d..7665ffdabd 100644 --- a/internal/http/services/owncloud/ocdav/dav.go +++ b/internal/http/services/owncloud/ocdav/dav.go @@ -276,9 +276,8 @@ func getTokenStatInfo(ctx context.Context, client gatewayv1beta1.GatewayAPIClien return client.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ ResourceId: &provider.ResourceId{ StorageId: utils.PublicStorageProviderID, - OpaqueId: utils.PublicStorageProviderID, + OpaqueId: token, }, - Path: utils.MakeRelativePath(token), }}) } diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index e03ab5b8c0..9372020b35 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -274,6 +274,7 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) for i := range resourceInfos { + // the list of filters grows with every public link in a folder filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) } @@ -285,15 +286,18 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } var linkshares map[string]struct{} - listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) - if err == nil { - linkshares = make(map[string]struct{}, len(listResp.Share)) - for i := range listResp.Share { - linkshares[listResp.Share[i].ResourceId.OpaqueId] = struct{}{} + // public link access does not show share-types + if namespace != "/public" { + listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) + if err == nil { + linkshares = make(map[string]struct{}, len(listResp.Share)) + for i := range listResp.Share { + linkshares[listResp.Share[i].ResourceId.OpaqueId] = struct{}{} + } + } else { + log.Error().Err(err).Msg("propfindResponse: couldn't list public shares") + span.SetStatus(codes.Error, err.Error()) } - } else { - log.Error().Err(err).Msg("propfindResponse: couldn't list public shares") - span.SetStatus(codes.Error, err.Error()) } propRes, err := MultistatusResponse(ctx, &pf, resourceInfos, p.PublicURL, namespace, linkshares) diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 1a3cd44e6a..27f742a04b 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -28,6 +28,7 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" permissionsv1beta1 "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" + collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" @@ -112,9 +113,17 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return true, nil case *provider.ListStorageSpacesRequest: - return checkPublicListStorageSpacesFilter(v.Filters), nil + //return checkPublicListStorageSpacesFilter(v.Filters), nil + return true, nil case *link.GetPublicShareRequest: return checkPublicShareRef(&share, v.GetRef()), nil + case *link.ListPublicSharesRequest: + // public links must not leak info about other links + return false, nil + + case *collaboration.ListReceivedSharesRequest: + // public links must not leak info about collaborative shares + return false, nil case string: return checkResourcePath(v), nil } @@ -135,13 +144,14 @@ func checkStorageRef(ctx context.Context, s *link.PublicShare, r *provider.Refer return true } - // r: path:$path> + // r: path:$path> if id := r.GetResourceId(); id.GetStorageId() == PublicStorageProviderID { - if id.GetOpaqueId() == PublicStorageProviderID && strings.HasPrefix(r.Path, "./"+s.Token) { + // access to /public + if id.GetOpaqueId() == PublicStorageProviderID { return true } - // r: path:$path> - if strings.HasPrefix(id.GetOpaqueId(), s.Token+"/") { + // access relative to /public/$token + if id.GetOpaqueId() == s.Token { return true } } @@ -149,12 +159,16 @@ func checkStorageRef(ctx context.Context, s *link.PublicShare, r *provider.Refer } // public link access must send a filter with id or type +/* func checkPublicListStorageSpacesFilter(filters []*provider.ListStorageSpacesRequest_Filter) bool { // return true for _, f := range filters { switch f.Type { case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: - if f.GetSpaceType() == "public" { + switch f.GetSpaceType() { + case "mountpoint", "+mountpoint": + return true + case "grant", "+grant": return true } case provider.ListStorageSpacesRequest_Filter_TYPE_ID: @@ -165,6 +179,7 @@ func checkPublicListStorageSpacesFilter(filters []*provider.ListStorageSpacesReq } return false } +*/ func checkPublicShareRef(s *link.PublicShare, ref *link.PublicShareReference) bool { // ref: From dcdf513675f2df1637cc19bd73ddebc246c8ee44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 17 Mar 2022 21:26:19 +0000 Subject: [PATCH 04/10] add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/publicstorageprovider-rewrite.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/publicstorageprovider-rewrite.md diff --git a/changelog/unreleased/publicstorageprovider-rewrite.md b/changelog/unreleased/publicstorageprovider-rewrite.md new file mode 100644 index 0000000000..bbffddf1d4 --- /dev/null +++ b/changelog/unreleased/publicstorageprovider-rewrite.md @@ -0,0 +1,6 @@ +Bugfix: Make public link access use same fileids as authenticated access + +A resource now always hase the same fileid, regardless of how it is accessed. + +https://github.com/cs3org/reva/pull/2646 +https://github.com/cs3org/reva/issues/2635 From 151a1fc8895fa5ac08db9d01397c6d941becbb29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 18 Mar 2022 11:15:38 +0000 Subject: [PATCH 05/10] update test config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- tests/oc-integration-tests/drone/gateway.toml | 3 ++- tests/oc-integration-tests/local/gateway.toml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/oc-integration-tests/drone/gateway.toml b/tests/oc-integration-tests/drone/gateway.toml index fb8e896fcc..daf482e272 100644 --- a/tests/oc-integration-tests/drone/gateway.toml +++ b/tests/oc-integration-tests/drone/gateway.toml @@ -78,7 +78,8 @@ home_template = "/users/{{.Id.OpaqueId}}" ## While public shares are mounted at /public logged in end will should never see that path because it is only created by the spaces registry when ## a public link is accessed. [grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] -"public" = { "mount_point" = "/public", "path_template" = "/public" } +"grant" = { "mount_point" = "." } +"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Name}}" } [http] address = "0.0.0.0:19001" diff --git a/tests/oc-integration-tests/local/gateway.toml b/tests/oc-integration-tests/local/gateway.toml index 0f3d8166ee..1f65704558 100644 --- a/tests/oc-integration-tests/local/gateway.toml +++ b/tests/oc-integration-tests/local/gateway.toml @@ -84,7 +84,8 @@ home_template = "/users/{{.Id.OpaqueId}}" ## While public shares are mounted at /public logged in end will should never see that path because it is only created by the spaces registry when ## a public link is accessed. [grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] -"public" = { "mount_point" = "/public" } +"grant" = { "mount_point" = "." } +"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Name}}" } [http] address = "0.0.0.0:19001" From 83adbbd56b5307f2d99ea1b37996742a06b61af7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 18 Mar 2022 12:08:42 +0000 Subject: [PATCH 06/10] fix lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../services/publicstorageprovider/publicstorageprovider.go | 2 +- pkg/auth/scope/publicshare.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 1dd6e9a00c..98f409ade0 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -819,7 +819,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? filterPermissions(listContainerR.Infos[i].PermissionSet, share.GetPermissions().Permissions) - //s.setPublicStorageID(listContainerR.Infos[i], tkn) + // s.setPublicStorageID(listContainerR.Infos[i], tkn) 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") } diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 27f742a04b..29209d439c 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -113,7 +113,7 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return true, nil case *provider.ListStorageSpacesRequest: - //return checkPublicListStorageSpacesFilter(v.Filters), nil + // return checkPublicListStorageSpacesFilter(v.Filters), nil return true, nil case *link.GetPublicShareRequest: return checkPublicShareRef(&share, v.GetRef()), nil From c252c1ef394fb20e246bf267fabe29ffba5ac52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 21 Mar 2022 09:17:16 +0000 Subject: [PATCH 07/10] ocdav: replace public mountpoint fileid with grant fileid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind/propfind.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 9372020b35..132f7c90d7 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -721,6 +721,11 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p sublog.Debug().Interface("role", role).Str("dav-permissions", wdp).Msg("converted PermissionSet") } + // replace fileid of /public/{token} mountpoint with grant fileid + if ls != nil && md.Id != nil && md.Id.StorageId == utils.PublicStorageProviderID && md.Id.OpaqueId == ls.Token { + md.Id = ls.ResourceId + } + propstatOK := PropstatXML{ Status: "HTTP/1.1 200 OK", Prop: []*props.PropertyXML{}, From fc6e5d0674fb898c95b3c3cba06b8ffe50dc9d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 21 Mar 2022 10:18:51 +0000 Subject: [PATCH 08/10] cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../publicstorageprovider-rewrite.md | 4 +- .../publicstorageprovider.go | 69 +++++-------------- pkg/auth/scope/publicshare.go | 23 ------- tests/oc-integration-tests/drone/gateway.toml | 2 +- tests/oc-integration-tests/local/gateway.toml | 2 +- 5 files changed, 22 insertions(+), 78 deletions(-) diff --git a/changelog/unreleased/publicstorageprovider-rewrite.md b/changelog/unreleased/publicstorageprovider-rewrite.md index bbffddf1d4..f6ff7f9cbe 100644 --- a/changelog/unreleased/publicstorageprovider-rewrite.md +++ b/changelog/unreleased/publicstorageprovider-rewrite.md @@ -1,6 +1,6 @@ -Bugfix: Make public link access use same fileids as authenticated access +Bugfix: replace public mountpoint fileid with grant fileid in ocdav -A resource now always hase the same fileid, regardless of how it is accessed. +We now show the same resoucre id for resources when accessing them via a public links as when using a logged in user. This allows the web ui to start a WOPI session with the correct resource id. https://github.com/cs3org/reva/pull/2646 https://github.com/cs3org/reva/issues/2635 diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 98f409ade0..8665bdf84d 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -16,6 +16,8 @@ // granted to it by virtue of its status as an Intergovernmental Organization // or submit itself to any jurisdiction. +// Package publicstorageprovider provides a CS3 storageprovider implementation for public links. +// It will list spaces with type `grant` and `mountpoint` when a public scope is present. package publicstorageprovider import ( @@ -45,9 +47,6 @@ import ( gstatus "google.golang.org/grpc/status" ) -// SpaceTypePublic is the public space type -var SpaceTypePublic = "public" - func init() { rgrpc.Register("publicstorageprovider", New) } @@ -82,7 +81,7 @@ func parseConfig(m map[string]interface{}) (*config, error) { return c, nil } -// New creates a new IsPublic Storage Provider service. +// New creates a new publicstorageprovider service. func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { c, err := parseConfig(m) if err != nil { @@ -169,13 +168,9 @@ func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider. if !ok { return nil, "", nil, nil, gstatus.Errorf(codes.NotFound, "share or token not found") } - /* - tkn, opaqueid, relativePath, err := s.unwrap(ctx, ref) - if err != nil { - return nil, "", nil, nil, err - } - */ + // the share is minimally populated, we need more than the token + // look up complete share ls, shareInfo, st, err := s.resolveToken(ctx, share.Token) switch { case err != nil: @@ -213,13 +208,6 @@ func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider. return cs3Ref, share.Token, ls, nil, nil } -// Both, t.dir and tokenPath paths need to be merged: -// tokenPath = /oc/einstein/public-links -// t.dir = /public/ausGxuUePCOi/foldera/folderb/ -// res = /public-links/foldera/folderb/ -// this `res` will get then expanded taking into account the authenticated user and the storage: -// end = /einstein/files/public-links/foldera/folderb/ - func (s *service) initiateFileDownload(ctx context.Context, req *provider.InitiateFileDownloadRequest) (*provider.InitiateFileDownloadResponse, error) { cs3Ref, _, ls, st, err := s.translatePublicRefToCS3Ref(ctx, req.Ref) switch { @@ -344,12 +332,17 @@ func (s *service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") } -// ListStorageSpaces returns a Storage spaces of type "public" when given a filter by id with the public link token as spaceid. -// The root node of every storag space is the real (spaceid, nodeid) of the publicly shared node -// The ocdav service has to -// 1. Authenticate / Log in at the gateway using the token and can then -// 2. look up the storage space using ListStorageSpaces. -// 3. make related requests to that (spaceid, nodeid) +// ListStorageSpaces returns storage spaces when a public scope is present +// in the context. +// +// On the one hand, it lists a `mountpoint` space that can be used by the +// registry to construct a mount path. These spaces have their root +// storageid set to 7993447f-687f-490d-875c-ac95e89a62a4 and the +// opaqueid set to the link token. +// +// On the other hand, it lists a `grant` space for the shared resource id, +// so id based requests can find the correct storage provider. These spaces +// have their root set to the shared resource. func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) { spaceTypes := map[string]struct{}{} var exists = struct{}{} @@ -359,7 +352,6 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora switch f.Type { case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: spaceType := f.GetSpaceType() - // do we need to fetch the shares? if spaceType == "+mountpoint" || spaceType == "+grant" { appendTypes = append(appendTypes, strings.TrimPrefix(spaceType, "+")) continue @@ -383,10 +375,6 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora // if there is no public scope there are no publicstorage spaces share, _, ok := extractLinkAndInfo(ctx) if !ok { - // TODO when the ocdav publicfile handler sends the token as opaqueid the - // public scope needs to change - // it always needs to check if the accessed path is a child of the resource identified by - // the token return &provider.ListStorageSpacesResponse{ Status: &rpc.Status{Code: rpc.Code_CODE_OK}, }, nil @@ -408,20 +396,12 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora // when a list storage space with the resourceid of an external // resource is made we may have a grant for it root := share.ResourceId - // do we filter by id? if spaceID != nil && !utils.ResourceIDEqual(spaceID, root) { // none of our business continue } - /* - var opaque *typesv1beta1.Opaque - if etag, ok := shareEtags[share.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) - } - */ // we know a grant for this resource space := &provider.StorageSpace{ - //Opaque: opaque, Id: &provider.StorageSpaceId{ OpaqueId: root.StorageId + "!" + root.OpaqueId, }, @@ -437,7 +417,6 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora StorageId: utils.PublicStorageProviderID, OpaqueId: share.Token, // the link share has no id, only the token } - // do we filter by id if spaceID != nil { switch { case utils.ResourceIDEqual(spaceID, root): @@ -450,28 +429,16 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora continue } } - /* - var opaque *typesv1beta1.Opaque - if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) - } - */ space := &provider.StorageSpace{ - //Opaque: opaque, Id: &provider.StorageSpaceId{ OpaqueId: root.StorageId + "!" + root.OpaqueId, }, SpaceType: "mountpoint", - Owner: &userv1beta1.User{Id: share.Owner}, // FIXME actually, the mount point belongs to the recipient + Owner: &userv1beta1.User{Id: share.Owner}, // FIXME actually, the mount point belongs to no one? // the publicstorageprovider keeps track of mount points Root: root, } - // "Public share ~token"? nah, the spaceregistry uses the token in the name as the mountpoint - // TODO use token in root opaqueid when building path - space.Name = share.Token - - // what if we don't have a name? res.StorageSpaces = append(res.StorageSpaces, space) } } @@ -818,8 +785,8 @@ 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, share.GetPermissions().Permissions) - // s.setPublicStorageID(listContainerR.Infos[i], tkn) 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") } diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 29209d439c..46054cef8d 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -158,29 +158,6 @@ func checkStorageRef(ctx context.Context, s *link.PublicShare, r *provider.Refer return false } -// public link access must send a filter with id or type -/* -func checkPublicListStorageSpacesFilter(filters []*provider.ListStorageSpacesRequest_Filter) bool { - // return true - for _, f := range filters { - switch f.Type { - case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: - switch f.GetSpaceType() { - case "mountpoint", "+mountpoint": - return true - case "grant", "+grant": - return true - } - case provider.ListStorageSpacesRequest_Filter_TYPE_ID: - if f.GetId().OpaqueId != "" { - return true - } - } - } - return false -} -*/ - func checkPublicShareRef(s *link.PublicShare, ref *link.PublicShareReference) bool { // ref: return ref.GetToken() == s.Token diff --git a/tests/oc-integration-tests/drone/gateway.toml b/tests/oc-integration-tests/drone/gateway.toml index daf482e272..4f20f9e7b6 100644 --- a/tests/oc-integration-tests/drone/gateway.toml +++ b/tests/oc-integration-tests/drone/gateway.toml @@ -79,7 +79,7 @@ home_template = "/users/{{.Id.OpaqueId}}" ## a public link is accessed. [grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] "grant" = { "mount_point" = "." } -"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Name}}" } +"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Root.OpaqueId}}" } [http] address = "0.0.0.0:19001" diff --git a/tests/oc-integration-tests/local/gateway.toml b/tests/oc-integration-tests/local/gateway.toml index 1f65704558..2167d1505f 100644 --- a/tests/oc-integration-tests/local/gateway.toml +++ b/tests/oc-integration-tests/local/gateway.toml @@ -85,7 +85,7 @@ home_template = "/users/{{.Id.OpaqueId}}" ## a public link is accessed. [grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] "grant" = { "mount_point" = "." } -"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Name}}" } +"mountpoint" = { "mount_point" = "/public", "path_template" = "/public/{{.Space.Root.OpaqueId}}" } [http] address = "0.0.0.0:19001" From 2833e9b5cbb9d15f9fe77a384947baa241e8b70b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 21 Mar 2022 15:49:20 +0000 Subject: [PATCH 09/10] cleanup comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/grpc/interceptors/auth/auth.go | 2 +- pkg/auth/scope/publicshare.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index 3ecdb6cd1e..a78bd37016 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -212,7 +212,7 @@ func (ss *wrappedServerStream) Context() context.Context { return ss.newCtx } -// dismantleToken extraclts the user and scopes from the reva access token +// dismantleToken extracts the user and scopes from the reva access token func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string, fetchUserGroups bool) (*userpb.User, map[string]*authpb.Scope, error) { u, tokenScope, err := mgr.DismantleToken(ctx, tkn) if err != nil { diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 46054cef8d..7dc8372394 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -113,7 +113,6 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return true, nil case *provider.ListStorageSpacesRequest: - // return checkPublicListStorageSpacesFilter(v.Filters), nil return true, nil case *link.GetPublicShareRequest: return checkPublicShareRef(&share, v.GetRef()), nil From 20392fbce99e4c8a28f10b0d947e802214db731b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 22 Mar 2022 08:27:29 +0000 Subject: [PATCH 10/10] prevent nil by disallowing CreateHome in public scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/auth/scope/publicshare.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 7dc8372394..207cd7e2f9 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -71,6 +71,8 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa } } return checkStorageRef(ctx, &share, ref), nil + case *provider.CreateHomeRequest: + return false, nil case *provider.GetPathRequest: return checkStorageRef(ctx, &share, &provider.Reference{ResourceId: v.GetResourceId()}), nil case *provider.StatRequest: @@ -127,7 +129,7 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return checkResourcePath(v), nil } - msg := "resource type assertion failed" + msg := "public resource type assertion failed" logger.Debug().Str("scope", "publicshareScope").Interface("resource", resource).Msg(msg) return false, errtypes.InternalError(msg) }