use share id to identify ocm shares

[butonic]

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

[butonic]

Now, while our webdav client will send the shared secret when making requests as the ocm share receiver:

func (d *driver) webdavClient(ctx context.Context, forUser *userpb.UserId, ref *provider.Reference) (*gowebdav.Client, *ocmpb.ReceivedShare, string, error) {
	id, rel := shareInfoFromReference(ref)

	share, endpoint, secret, err := d.getWebDAVFromShare(ctx, forUser, id)
	if err != nil {
		return nil, nil, "", err
	}

	endpoint, err = url.PathUnescape(endpoint)
	if err != nil {
		return nil, nil, "", err
	}

	// FIXME: it's still not clear from the OCM APIs how to use the shared secret
	// will use as a token in the bearer authentication as this is the reva implementation
	c := gowebdav.NewAuthClient(endpoint, gowebdav.NewPreemptiveAuth(BearerAuthenticator{token: secret}))
	if d.c.Insecure {
		c.SetTransport(&http.Transport{
			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
		})
	}

	return c, share, rel, nil
}

reva, when acting as the ocm share provider still needs to actually read the bearer auth ...

[butonic]

I think some of the confusion comes from comments like cs3org/OCM-API#57 (comment) which do not have a path property in the url that can identify the ocm share. In fact the sharedSecret / token is required to determine what content is to be served under /remote.php/webdav, which violates URLs as resource identitifiers in the first place. I thought we had moved away from that with /dav/files and /dav/public-files in oc10 ...

Well, with spaces at least we can finally provide actual URIs ... /dav/ocm/{ocm-share-id} is a proper URI ... we could even migrate that to /dav/spaces/{ocmproviderid}!{ocmspaceid}${ocm-shareid} ... maybe even use more than one ocmproviderid ... to directly route to the correct instance ... well ... maybe in the future. let us not go there now and stick to /dav/ocm.

Anyway, our /dav/ocm/ endpoint needs to learn bearer auth. The ocmshares auth manager currently uses the shareid to make a GetOCMShareByToken call, which is why this worked. It should actually use GetOCMShare with the ocm share id and then verify the bearer token with the shared secret.

Actually, GetOCMShare requires a user to be in the context, so we need to use the shared secret as the token to look up the share.

[butonic]

@aduffeck I think this clarifies some of the confusion around authenticating OCM webdav requests. Do you remember how you set this up to test this ... The unit tests don't fully cover this. Needs to be added ...

[butonic]

@aduffeck shouldn't this condition be inverted:

func (s *service) GetOCMShare(ctx context.Context, req *ocm.GetOCMShareRequest) (*ocm.GetOCMShareResponse, error) {
	// if the request is by token, the user does not need to be in the ctx
	var user *userpb.User
	if req.Ref.GetToken() == "" {
		user = ctxpkg.ContextMustGetUser(ctx)
	}

[aduffeck]

@aduffeck shouldn't this condition be inverted:

func (s *service) GetOCMShare(ctx context.Context, req *ocm.GetOCMShareRequest) (*ocm.GetOCMShareResponse, error) {
	// if the request is by token, the user does not need to be in the ctx
	var user *userpb.User
	if req.Ref.GetToken() == "" {
		user = ctxpkg.ContextMustGetUser(ctx)
	}

I don't think so. If no token is set (req.Ref.GetToken() == "") we require a user because we end up at https://github.com/cs3org/reva/blob/edge/pkg/ocm/share/repository/json/json.go#L309, otherwise we don't because we return at https://github.com/cs3org/reva/blob/edge/pkg/ocm/share/repository/json/json.go#L299 already.

[aduffeck]

👍

[butonic]

To fix the ci I had to set up a debuggable environment ... and then I ran into owncloud/ocis#8564: sharingNG does not support OCM, yet.

Some ocis graph progress is in owncloud/ocis#8909, but it needs more work.

I'll commit the changes I made while dabbling with the setup to continue on it next week...