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.

Sign up for GitHub

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

Jump to bottom

use share id to identify ocm shares #4630

Merged
merged 4 commits into from
Apr 26, 2024
Merged

use share id to identify ocm shares #4630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ec422a
use share id to identify ocm shares
butonic Apr 15, 2024
755ef50
correctly name ocm share id to clarify the code
butonic Apr 15, 2024
cdc7164
use bearer auth for ocm
butonic Apr 15, 2024
14cc2c8
fix tests
butonic Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-ocm-share-id.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: fix ocm-share-id

We now use the share id to correctly identify ocm shares.

https://github.com/cs3org/reva/pull/4630
14 changes: 12 additions & 2 deletions internal/grpc/interceptors/auth/scope.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s
return nil
}
}
log.Err(err).Msgf("error resolving reference %s under scope %+v", ref.String(), k)
log.Err(err).Interface("ref", ref).Interface("scope", k).Msg("error resolving reference under scope")
}

} else if ref, ok := extractShareRef(req); ok {
Expand Down Expand Up @@ -179,6 +179,11 @@ func resolveOCMShare(ctx context.Context, ref *provider.Reference, scope *authpb
return err
}

// for ListOCMSharesRequest, the ref resource id is empty and we set path to . to indicate the root of the share
if ref.GetResourceId() == nil && ref.Path == "." {
ref.ResourceId = share.GetResourceId()
}

if err := checkCacheForNestedResource(ctx, ref, share.ResourceId, client, mgr); err == nil {
return nil
}
Expand Down Expand Up @@ -213,7 +218,7 @@ func checkRelativeReference(ctx context.Context, requested *provider.Reference,
if sharedResource.ParentId == nil {
// Is the requested resource part of the shared space?
if requested.ResourceId.StorageId != sharedResource.Id.StorageId || requested.ResourceId.SpaceId != sharedResource.Id.SpaceId {
return errtypes.PermissionDenied("access forbidden via public link")
return errtypes.PermissionDenied("space access forbidden via public link")
}
} else {
parentID := sharedResource.ParentId
Expand Down Expand Up @@ -388,6 +393,11 @@ func extractRefForReaderRole(req interface{}) (*provider.Reference, bool) {
return v.GetRef(), true
case *provider.UnlockRequest:
return v.GetRef(), true

// OCM shares
case *ocmv1beta1.ListReceivedOCMSharesRequest:
return &provider.Reference{Path: "."}, true // we will try to stat the shared node

}

return nil, false
Expand Down
35 changes: 20 additions & 15 deletions internal/grpc/services/gateway/ocmshareprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ import (

// TODO(labkode): add multi-phase commit logic when commit share or commit ref is enabled.
func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest) (*ocm.CreateOCMShareResponse, error) {
if len(req.AccessMethods) == 0 {
return &ocm.CreateOCMShareResponse{
Status: status.NewInvalidArg(ctx, "access methods cannot be empty"),
}, nil
}
c, err := pool.GetOCMShareProviderClient(s.c.OCMShareProviderEndpoint)
if err != nil {
return &ocm.CreateOCMShareResponse{
Expand All @@ -48,7 +53,7 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest

res, err := c.CreateOCMShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling CreateShare")
return nil, errors.Wrap(err, "gateway: error calling CreateOCMShare")
}

status, err := s.addGrant(ctx, req.ResourceId, req.Grantee, req.AccessMethods[0].GetWebdavOptions().Permissions, req.Expiration, nil)
Expand Down Expand Up @@ -78,7 +83,7 @@ func (s *svc) RemoveOCMShare(ctx context.Context, req *ocm.RemoveOCMShareRequest

res, err := c.RemoveOCMShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling RemoveShare")
return nil, errors.Wrap(err, "gateway: error calling RemoveOCMShare")
}

return res, nil
Expand All @@ -102,7 +107,7 @@ func (s *svc) getOCMShare(ctx context.Context, req *ocm.GetOCMShareRequest) (*oc

res, err := c.GetOCMShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling GetShare")
return nil, errors.Wrap(err, "gateway: error calling GetOCMShare")
}

return res, nil
Expand Down Expand Up @@ -134,7 +139,7 @@ func (s *svc) ListOCMShares(ctx context.Context, req *ocm.ListOCMSharesRequest)

res, err := c.ListOCMShares(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling ListShares")
return nil, errors.Wrap(err, "gateway: error calling ListOCMShares")
}

return res, nil
Expand All @@ -151,7 +156,7 @@ func (s *svc) UpdateOCMShare(ctx context.Context, req *ocm.UpdateOCMShareRequest

res, err := c.UpdateOCMShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling UpdateShare")
return nil, errors.Wrap(err, "gateway: error calling UpdateOCMShare")
}

return res, nil
Expand All @@ -168,7 +173,7 @@ func (s *svc) ListReceivedOCMShares(ctx context.Context, req *ocm.ListReceivedOC

res, err := c.ListReceivedOCMShares(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling ListReceivedShares")
return nil, errors.Wrap(err, "gateway: error calling ListReceivedOCMShares")
}

return res, nil
Expand Down Expand Up @@ -223,7 +228,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive

res, err := c.UpdateReceivedOCMShare(ctx, req)
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
Expand Down Expand Up @@ -256,7 +261,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive
// handle transfer in case it has not already been accepted
if s.isTransferShare(share) && req.GetShare().State == ocm.ShareState_SHARE_STATE_ACCEPTED {
if share.State == ocm.ShareState_SHARE_STATE_ACCEPTED {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare, share already accepted.")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare, share already accepted.")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_FAILED_PRECONDITION,
Expand All @@ -268,7 +273,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive
transferDestinationPath, err := s.getTransferDestinationPath(ctx, req)
if err != nil {
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
Expand All @@ -279,7 +284,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive

error := s.handleTransfer(ctx, share, transferDestinationPath)
if error != nil {
log.Err(error).Msg("gateway: error handling transfer in UpdateReceivedShare")
log.Err(error).Msg("gateway: error handling transfer in UpdateReceivedOCMShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
Expand Down Expand Up @@ -309,17 +314,17 @@ func (s *svc) handleTransfer(ctx context.Context, share *ocm.ReceivedShare, tran
}
destWebdavEndpoint, err := s.getWebdavEndpoint(ctx, granteeIdp)
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare")
return err
}
destWebdavEndpointURL, err := url.Parse(destWebdavEndpoint)
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare: unable to parse webdav endpoint \"" + destWebdavEndpoint + "\" into URL structure")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare: unable to parse webdav endpoint \"" + destWebdavEndpoint + "\" into URL structure")
return err
}
destWebdavHost, err := s.getWebdavHost(ctx, granteeIdp)
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare")
return err
}
var dstWebdavURLString string
Expand All @@ -330,7 +335,7 @@ func (s *svc) handleTransfer(ctx context.Context, share *ocm.ReceivedShare, tran
}
dstWebdavHostURL, err := url.Parse(dstWebdavURLString)
if err != nil {
log.Err(err).Msg("gateway: error calling UpdateReceivedShare: unable to parse webdav service host \"" + dstWebdavURLString + "\" into URL structure")
log.Err(err).Msg("gateway: error calling UpdateReceivedOCMShare: unable to parse webdav service host \"" + dstWebdavURLString + "\" into URL structure")
return err
}
destServiceHost := dstWebdavHostURL.Host + dstWebdavHostURL.Path
Expand Down Expand Up @@ -393,7 +398,7 @@ func (s *svc) GetReceivedOCMShare(ctx context.Context, req *ocm.GetReceivedOCMSh

res, err := c.GetReceivedOCMShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling GetReceivedShare")
return nil, errors.Wrap(err, "gateway: error calling GetReceivedOCMShare")
}

return res, nil
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/ocmshareprovider/ocmshareprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ func getResourceType(info *providerpb.ResourceInfo) string {
return "unknown"
}

func (s *service) webdavURL(ctx context.Context, share *ocm.Share) string {
func (s *service) webdavURL(_ context.Context, share *ocm.Share) string {
// the url is in the form of https://cernbox.cern.ch/remote.php/dav/ocm/token
p, _ := url.JoinPath(s.conf.WebDAVEndpoint, "/dav/ocm", share.Token)
p, _ := url.JoinPath(s.conf.WebDAVEndpoint, "/dav/ocm", share.GetId().GetOpaqueId())
return p
}

Expand Down
9 changes: 6 additions & 3 deletions internal/grpc/services/publicstorageprovider/publicstorageprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
Status: &rpc.Status{Code: rpc.Code_CODE_INVALID_ARGUMENT, Message: err.Error()},
}, nil
}
if resID.SpaceId != utils.PublicStorageSpaceID {
if resID.SpaceId != utils.PublicStorageSpaceID && resID.SpaceId != utils.OCMStorageSpaceID {
return &provider.ListStorageSpacesResponse{
// a specific id was requested, return not found instead of empty list
Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND},
Expand All @@ -415,7 +415,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
}
}

info, _, grantee, token, err := s.extractLinkFromScope(ctx)
info, share, grantee, token, err := s.extractLinkFromScope(ctx)
if err != nil {
switch err.(type) {
case errtypes.NotFound:
Expand Down Expand Up @@ -468,6 +468,9 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
SpaceId: utils.PublicStorageSpaceID,
OpaqueId: token, // the link share has no id, only the token
}
if ocmShare, ok := share.(*ocm.Share); ok {
root.OpaqueId = ocmShare.GetId().GetOpaqueId()
}
if spaceID != nil {
switch {
case utils.ResourceIDEqual(spaceID, root):
Expand Down Expand Up @@ -506,7 +509,7 @@ func (s *service) extractLinkFromScope(ctx context.Context) (*provider.ResourceI
share := &ocm.Share{}
err := utils.UnmarshalJSONToProtoV1(v.Resource.Value, share)
if err != nil {
return nil, nil, nil, "", errtypes.InternalError("failed to unmarshal public share")
return nil, nil, nil, "", errtypes.InternalError("failed to unmarshal ocm share")
}

// the share is minimally populated, we need more than the token
Expand Down
33 changes: 17 additions & 16 deletions internal/http/services/owncloud/ocdav/dav.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,38 +185,38 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
}

// OC10 and Nextcloud (OCM 1.0) are using basic auth for carrying the
// shared token.
var token string
// ocm share id.
var ocmshare, sharedSecret string
username, _, ok := r.BasicAuth()
if ok {
// OCM 1.0
token = username
r.URL.Path = filepath.Join("/", token, r.URL.Path)
ctx = context.WithValue(ctx, net.CtxOCM10, true)
ocmshare = username
sharedSecret = username
r.URL.Path = filepath.Join("/", ocmshare, r.URL.Path)
} else {
token, _ = router.ShiftPath(r.URL.Path)
ctx = context.WithValue(ctx, net.CtxOCM10, false)
ocmshare, _ = router.ShiftPath(r.URL.Path)
sharedSecret = strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
}

authRes, err := handleOCMAuth(ctx, c, token)
authRes, err := handleOCMAuth(ctx, c, ocmshare, sharedSecret)
switch {
case err != nil:
log.Error().Err(err).Msg("error during ocm authentication")
w.WriteHeader(http.StatusInternalServerError)
return
case authRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED:
log.Debug().Str("token", token).Msg("permission denied")
log.Debug().Str("ocmshare", ocmshare).Msg("permission denied")
fallthrough
case authRes.Status.Code == rpc.Code_CODE_UNAUTHENTICATED:
log.Debug().Str("token", token).Msg("unauthorized")
log.Debug().Str("ocmshare", ocmshare).Msg("unauthorized")
w.WriteHeader(http.StatusUnauthorized)
return
case authRes.Status.Code == rpc.Code_CODE_NOT_FOUND:
log.Debug().Str("token", token).Msg("not found")
log.Debug().Str("ocmshare", ocmshare).Msg("not found")
w.WriteHeader(http.StatusNotFound)
return
case authRes.Status.Code != rpc.Code_CODE_OK:
log.Error().Str("token", token).Interface("status", authRes.Status).Msg("grpc auth request failed")
log.Error().Str("ocmshare", ocmshare).Interface("status", authRes.Status).Msg("grpc auth request failed")
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -225,7 +225,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
ctx = ctxpkg.ContextSetUser(ctx, authRes.User)
ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, authRes.Token)

log.Debug().Str("token", token).Interface("user", authRes.User).Msg("OCM user authenticated")
log.Debug().Str("ocmshare", ocmshare).Interface("user", authRes.User).Msg("OCM user authenticated")

r = r.WithContext(ctx)
h.OCMSharesHandler.Handler(s).ServeHTTP(w, r)
Expand Down Expand Up @@ -375,9 +375,10 @@ func handleSignatureAuth(ctx context.Context, selector pool.Selectable[gatewayv1
return c.Authenticate(ctx, &authenticateRequest)
}

func handleOCMAuth(ctx context.Context, c gatewayv1beta1.GatewayAPIClient, token string) (*gatewayv1beta1.AuthenticateResponse, error) {
func handleOCMAuth(ctx context.Context, c gatewayv1beta1.GatewayAPIClient, ocmshare, sharedSecret string) (*gatewayv1beta1.AuthenticateResponse, error) {
return c.Authenticate(ctx, &gatewayv1beta1.AuthenticateRequest{
Type: "ocmshares",
ClientId: token,
Type: "ocmshares",
ClientId: ocmshare,
ClientSecret: sharedSecret,
})
}
1 change: 0 additions & 1 deletion internal/http/services/owncloud/ocdav/net/net.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type ctxKey int
const (
// CtxKeyBaseURI is the key of the base URI context field
CtxKeyBaseURI ctxKey = iota
CtxOCM10

// NsDav is the Dav ns
NsDav = "DAV:"
Expand Down
13 changes: 10 additions & 3 deletions pkg/auth/manager/ocmshares/ocmshares.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@ func (m *manager) Configure(ml map[string]interface{}) error {
return nil
}

func (m *manager) Authenticate(ctx context.Context, token, _ string) (*userpb.User, map[string]*authpb.Scope, error) {
log := appctx.GetLogger(ctx).With().Str("token", token).Logger()
func (m *manager) Authenticate(ctx context.Context, ocmshare, sharedSecret string) (*userpb.User, map[string]*authpb.Scope, error) {
log := appctx.GetLogger(ctx).With().Str("ocmshare", ocmshare).Logger()
// We need to use GetOCMShareByToken, as GetOCMShare would require a user in the context
shareRes, err := m.gw.GetOCMShareByToken(ctx, &ocm.GetOCMShareByTokenRequest{
Token: token,
Token: sharedSecret,
})

switch {
Expand All @@ -103,6 +104,12 @@ func (m *manager) Authenticate(ctx context.Context, token, _ string) (*userpb.Us
return nil, nil, errtypes.InternalError(shareRes.Status.Message)
}

// compare ocm share id
if shareRes.GetShare().GetId().GetOpaqueId() != ocmshare {
log.Error().Str("persisted", ocmshare).Str("requested", shareRes.GetShare().GetId().GetOpaqueId()).Msg("mismatching ocm share id for existing secret")
return nil, nil, errtypes.InvalidCredentials("invalid shared secret")
}

// the user authenticated using the ocmshares authentication method
// is the recipient of the share
u := shareRes.Share.Grantee.GetUserId()
Expand Down
Loading
Loading