Sharing 2.0

[rullzer]

@schiesbn and myself spend some time at the conference coming up with a possible design for sharing 2.0. Here is basically an overview of my notes.

Note

As discussed in Berlin the sharing stuff will for now only be for files. Contact and calendar will use the old sharing code but will be moved to webdav.

Architecture

We want to move all real sharing to separate apps. Right now sharing is mostly in core and this is not really the right place. It makes it hard to have custom sharing implementations.

We propose a single sharemanager in core where apps can register themself. Furthermore in core we will provide a number of interface that apps have to implement in order to become a shareprovider.

Currently we have the following 3 categories of shares:

• user/group
• link
• federated

We will of course keep those 3 categories but each of them will be a separate app. This would mean we would get more core apps. Which will take some time but in the end we should end up with a more modular (and better maintainable) sharing setup.

We will then also need another new app that handles the OCS Sharing API. This clearly separates the API and the sharing implementation

No more sharing hierarchy

We propose to move to what we like to call a flat sharing model. This will prevent cyclic sharing or at least make sure it is no longer an issue. (see also #9058)

This effectively means that shares are a combination of the following items

• path
• share owner (owner of the path)
• sharer (initiatior of the share)
• sharee (receiver of the share)
• permissions

This means that the share owner always has the complete overview of who has access to a share.
And that we can easily find out the max permissions for a sharee.

Interfaces

sharemanager

The share manager is just a place where shareproviders can register and where they can be queried. There will be some functions to get a specific share or to get a traversal of all shares in a path etc.

shareprovider

The share provider just implements sharing for a specific type. You can of course tell the provider to share a file. Or to get a share etc.

Each share provider has a unique id.

We plan to have two share providers:

1. "internal share provider" which will be provided by the primary storage and handle (user, group and link sharing)
2. "federated share provider".

In general share provider will register them self at the share manager. This way it is easy to develop and add new share provider. For the internal share provider (which could be seen as the main share provider) the share manager will ask the primary storage directly to provide it.

We will implement two share providers as apps, the "oc internals share provider" which does sharing basically as we are doing it today and the "oc federated share provider". Bot will register them self at the share manager if the apps are activated. Additionally the share manager will ask the primary storage if he implements his own share provider. If he returns a share provider it will be used for internal shares, if it returns false the "oc default internal share provider" will be used. That's also the reason why the "oc default share provider" needs to be implemented as a app and not as part of our default storage, because the share provider needs to be also available for other storage if they don't implement their own share provider.

share

Just an object representation of the share. Where you can get and set the attributes etc.

Each share withing a shareprovider has a unique id.

A system wide unique id could be constructed as "providerID#localShareID" or something similiar.

We decided to only pass around userids/groupids/federatedshare ids etc. This makes sure that only the places that have to care about existing users etc will handle that.

Example Code

I took the liberty of already starting with some interfaces:

ShareManager
ShareProvider
Share

Limitation

We can only handle 1 share provider per type.

During the conference we had the idea to do things on a per storage level. But this brings in a lot of problems. Assume there are 2 folders dirA and dirB. Now the storage where dirA lives supports link sharing but the storage of dirB does not. Now what happens if a file with link share is moved from dirA to dirB?

In general most people will just use the apps that we provide. And in the case that people want very specific sharing behaviour for certain storages they should write their own sharing apps.

Forcing this limitations keeps things simple and understandable and we do not believe this to be a limitation a lot of people will have problems with.

Migration/deprecation path

For third party apps like calendar and contact we will move the existing sharing code to the apps and will make them work. From this point on the app delevlopers have to maintain or replace the existing code.

For file sharing we will re-use the exiting table, update the values add needed columns and remove old columns we no longer need at the end. The idea is that this process will be triggered with a occ script after the main upgrade to 9.0 while the system is live (still need to evaluate if this is possible).

Open Question:

• What happens for small installations with no access to a terminal? We probably need a fall back to trigger this update in the admin settings (similar to what we do for encryption key migration)
• When we migrate the shares step by step on a running system re-shares will not be visible until they are migrated (sync clients will potentially delete local copys and download them later again if the re-share shows up again)

Improvements

Since we kind of start over. There are a lot of things where we can improve and unify the current sharing design. Some things we came up with.

Expiration date on everything

Currently it is only possible to have expiration dates on link shares. There is no real technical reason for this and it is perfectly valid to have other shares expire as well.

Multiple link shares

Currently only 1 link share is allowed per file. Having multiple makes sense since I might want to share a file with multiple people but not for the same period of time. This could be combined with easily sharing via email so the individual link is automatically sent to the respective person. Individual links can then also be revoked easily based on email/name.

Open Issues

• decide about the right default permission for shares Sharing a file does set while to rw for short time #11707 (comment)

Checklists

• Share backend
  • Share manager ( Share2.0 sharemanager #19872)
    • Throw delete hooks (Move hook and delete children logic to share manager #20543)
    • Delete all children of share on delete (Move hook and delete children logic to share manager #20543)
    • allow to pass in a token when creating a new share
    • Allow registration of providers ([Share 2.0] register providers #21552)
    • Move interfaces to OCP ([Share 2.0] Move interfaces to OCP #21957)
  • Default share Provider
    • DB modifications ([Sharing 2.0] Add a new db column for the file owner #20703)
    • List share (Share2.0 sharemanager #19872, [Share2.0] OCS Share API getShare uses new code #20356)
    • Delete share (Share2.0 sharemanager #19872)
    • Get children (Share2.0 sharemanager #19872)
    • Update Share ([Sharing 2.0] update share #21887)
    • Create Share ([Sharing 2.0] create share #20773)
  • Federated Share Provider (implementation of the federated share provider #22080)
    • List share
    • Delete share
    • Get children
    • Update Share
    • Create Share
• Move OCS Share API to new sharing code
  • Delete Shares (Share2.0 sharemanager #19872)
  • List single shares ([Share2.0] OCS Share API getShare uses new code #20356)
  • List multiple shares ([Share 2.0] get shares #21532)
  • List shares shared with me ([Share 2.0] get shares #21532)
  • Update shares
    • User shares ([Sharing 2.0] update share #21887)
    • Group shares ([Sharing 2.0] update share #21887)
    • Link Shares ([Sharing 2.0] update share #21887)
    • Federated Shares (implementation of the federated share provider #22080)
  • Create Shares ([Sharing 2.0] create share #20773)
    • User Share ([Sharing 2.0] create share #20773)
    • Group Share ([Sharing 2.0] create share #20773)
    • Link Share ([Sharing 2.0] create share #20773)
    • Federated share (implementation of the federated share provider #22080)
• Other code
  • Move internal sharing calls to the share manager
    • Link shares (Share2 link #21761)
    • Sharedmount etc
    • Federated sharing accepting etc
  • Move webinterface over to the OCS Share API (Web interface should use the OCS Share API #21554)
  • Migration of old shares (upgrade to sharing 2.0, remove hierarchical re-shares #21660)
  • Provide more public exceptions ([Share 2.0] Move interfaces to OCP #21957 (comment))

How we implement it

The idea is to implement it in parallel to the existing sharing with small pull request merging sharing 2.0 one by one. In this transition phase it will be possible to switch between old and new implementation for testing purpose. Only if sharing 2.0 is complete and works reasonable well (including all integration tests for the OCS API) we will move the old sharing to the calendar and contacts app like explained under "Migration" and switch permanently to the new sharing.

DB implentation and considerations

In order to faciliate the new sharing we need to modify the database. We want to do this as little as possible to avoid large and huge migration steps.

Eventually we will remove the columns that are only needed for calendar/contacts sharing since they move to webdav. But this can only be done once we kill off the old code (since it is heavily wired into it).

Currently we store the initiated of the share in uid_owner. So the table uid_owner maps to the sharer (sharedBy). We can get the owner of a file via the fileid in the table. But this does not scale. Thus we plan to introduce a new column that holds the owner of the file (shareOwner). This is then allows for quick lookups of share info for the owner since the owner of a file should always have the full overview.

Related issues:

• Share API 2.0 #13014 : previous sharing 2.0 issue
• Recursive shares #17861 : Allow filtering of shares list
• Refactor/fix share permission masking mess #13202 : Share permission issues
• Re-Sharing Behavior Update #9058 : Resharing behaviour
• [Sharing] Avoid loop shares #5491 : Avoid loop shares
• Getting an specific share with a user as recipient doesn't return anything. #20582 : OCS API /shares/id should also return for shares with me

[rullzer]

FYI: @DeepDiver1975 @karlitschek @cmonteroluque @MTRichards

[DeepDiver1975]

We propose a single sharemanager in core where apps can register themself. Furthermore in core we will provide a number of interface that apps have to implement in order to become a shareprovider.

I question the use case - what would such a share provider be?
If sharing is file only - this is a pure private implementation - no app needs to interact on this level.

[schiessle]

I question the use case - what would such a share provider be?

A share provider could be a sharing implementation from a specific storage for example.
We need a manager to track the share provider (we may have more than one in the future) and redirect for example OCS share api calls to the right implementation.

[DeepDiver1975]

A share provider could be a sharing implementation from a specific storage for example.

Which we need to get from the storage interface from my pov

[schiessle]

Which we need to get from the storage interface from my pov

We will see if we need it or not if we move forward with the implementation. But if we assume that not every storage will have is own implementation of sharing we probably need some kind of manager as entry point. For example you have a OCS call to change permission to read-only for share "42", then we need a central instance (the share manager) who says OK share "42" is managed by share provider X (which might come from storage Y) and forward the request to change the permissions. How should a OCS share request for share "42" find the right storage by itself?

[rullzer]

Which we need to get from the storage interface from my pov

I think we should have a clear separation. If you have multiple storage providers there is no guarantee that they support the same set of sharing functionality.

[DeepDiver1975]

hmmm .. true ... I guess we need both mechanisms.

On the one hand there is the need to explicitly share a file/folder on storage level

$storage->getSharing()->share()

on the other hand there has to be a mechanism to resolve a share to the real physical resource

$shareManager->resolveShare('42')

[DeepDiver1975]

I think we should have a clear separation. If you have multiple storage providers there is no guarantee that they support the same set of sharing functionality.

There will be no new kinds of sharing.

And even if this is to be analysed how this might fit into the existing structures

[rullzer]

We propose a single sharemanager in core where apps can register themself. Furthermore in core we will provide a number of interface that apps have to implement in order to become a shareprovider.

I question the use case - what would such a share provider be?
If sharing is file only - this is a pure private implementation - no app needs to interact on this level.

A share provider for

• user/group sharing
• link sharing
• federated sharing

And maybe more in the future. This way if for example somebody wants to implement their own user/groups sharing on a file metadata level. But still wants to have link sharing. They only need to write their own user/group sharing app.

[schiessle]

@DeepDiver1975 Yes, we need both. I think this are two different levels of abstraction. The share manager is on a higher level and manage sharing on a system wide level no matter which kind of storage or share implementation is used. It will then resolve the storage/share implementation which has to handle the share (as said by @rullzer depending on the share type this can be even something like the "federated share provider" which will be storage independent) and call the share implementation to perform the action.

[DeepDiver1975]

@schiesbn sounds reasonable

But never the less I still question the share provider use case

[PVince81]

What happened to the similarly named ticket #13014 ?

[schiessle]

#13014 was a early brain storming about what could/should be improved. This one is far more concrete and will be the issue where we define sharing for 9.0 and track the progress. I will check the other issue and see what we can/should move over.

[karlitschek]

Looks good to me.
We have to keep the upgrade time under control as discussed in the zero upgrade discussion.
So we either make the new code also work with the old sharing tables and run the migration script in the background? Or we make sure that the migration script runs in minutes even for huge installations.

[DeepDiver1975]

So we either make the new code also work with the old sharing tables

Are there any reasons (for now) to introduce a new table?

[karlitschek]

@DeepDiver1975 Excellent point. I assumed that there will be changes in the data model. Everything would be a lot easier if this can be avoided of course.

[rullzer]

Are there any reasons (for now) to introduce a new table?

I don't know yet. We have to see when we get a bit more into detail. I think yes. since we want to move from a hierarchical sharing model to a flat sharing model (so the owner of the data always knows who has access etc). Doing this live in the same table as the current one is asking for complicated code.

But as I said I'm simply do not know yet. We probably should have a more in depth meeting at some point and maybe even have some (simple) code ready to look at so the discussion is not purely on a high level.

[jancborchardt]

Also keep in mind sharing via email. Normally this is just sending the share link to a person. You should be able to simply type in a person’s email (autocompletion from Contacts as well) and then ownCloud will generate an individual link for that person and also show them in the share list.

That way you can also remove these people from the share. In the backend it would just invalidate the custom link of that person – for others it would still work.

[schiessle]

cc @cmonteroluque as discussed I updated the issue with the latest state (all important information should be in the initial comment #19331 (comment))

@rullzer @DeepDiver1975 please also review it. I updated the sections "share provider", "migration" and added the section "how we implement it"... all based on our discussion we had last Friday

[ghost]

thanks @schiesbn!

[schiessle]

Updated the "shareprovider" section at the top to discuss the need of a way to register share providers at the manager and the rationals why both the "oc default internal share provider" and the "federated share provider" will be a app outside of the storage.

[nickvergessen]

Just a comment, we need a php internal API for shares. I just spoke to @schiesbn and he said that this was not the plan anymore. Quick example where we need it: Activities

When writing a file, we surely do not want to make a OCS request to the share API to get all users that have this file shared.

[DeepDiver1975]

When writing a file, we surely do not want to make a OCS request to the share API to get all users that have this file shared.

can you please link the current code which is taking care of this?

[nickvergessen]

Activity app listens to the following hooks:

        Util::connectHook('OC_Filesystem', 'post_create', 'OCA\Activity\FilesHooksStatic', 'fileCreate');
        Util::connectHook('OC_Filesystem', 'post_update', 'OCA\Activity\FilesHooksStatic', 'fileUpdate');
        Util::connectHook('OC_Filesystem', 'delete', 'OCA\Activity\FilesHooksStatic', 'fileDelete');
        Util::connectHook('\OCA\Files_Trashbin\Trashbin', 'post_restore', 'OCA\Activity\FilesHooksStatic', 'fileRestore');
        Util::connectHook('OCP\Share', 'post_shared', 'OCA\Activity\FilesHooksStatic', 'share');

Then I grab the original file owner for the file:

        $uidOwner = Filesystem::getOwner($path);
        $fileId = 0;
        if ($uidOwner !== $this->currentUser) {
            Filesystem::initMountPoints($uidOwner);
        }
        $info = Filesystem::getFileInfo($path);
        if ($info !== false) {
            $ownerView = new View('/' . $uidOwner . '/files');
            $fileId = (int) $info['fileid'];
            $path = $ownerView->getPath($fileId);
        }

And finally I get all users that share the file:
https://github.com/owncloud/activity/blob/master/lib/fileshooks.php#L176

[DeepDiver1975]

okay - this is something we need to consider - THX

@rullzer @schiesbn this still fits into our agreed approach to define the ocp interface in a later stage

[rullzer]

Added some text for the required db modifications (and why)

[jancborchardt]

@DeepDiver1975 yup. Actually if A removing B would automatically remove C as well that would be really confusing. After a few weeks/months no one remembers who reshared what with whom and unsharing from just one person should not cause that whole tree to collapse.

[jospoortvliet]

For what it's worth, I'd also vote with @DeepDiver1975 and @jancborchardt on this.

It is hard to figure out what people would expect and for sure some will have different needs and expectations anyway, but #19331 (comment) by @schiesbn has the answer:

The whole idea of the flat sharing list was that you don't create a re-share in the sense of a tree but I give you the permission to create shares "in my name". Once you have done this there shouldn't be a difference whether I created the share by my own or you did it. The new share is a independent share. The share is there because I allowed other to create it, I as a owner can see and control it. That's it.

It might not always be what users want but it does lead to the most predictable behavior. And the worst it will ever do is create a bit 'extra work' for the original sharer to manually remove all shares done by others he/she doesn't want.

[rullzer]

We must also make sure that the webinterface is ported to the OCS Share API in order to ensure a consistent view. As the old ajax endpoints still use the old code paths.

[moscicki]

Is this going to be part of OC9? Sharing currently is a major source of page loading slowness in current cernbox, we would like to check if this improves in OC9.

CC: @karlitschek

[DeepDiver1975]

Is this going to be part of OC9? Sharing currently is a major source of page loading slowness in current cernbox, we would like to check if this improves in OC9.

yes - definitely - parts have already been merged into master branch

[davivel]

@rullzer, I have a question

What's the need to have a READ bit in permissions of a Share? Is there any situation where a valid Share should have a value 0 for that bit?

I am writing some automatic tests for updating permissions in the Android app and I thought that updating a Share to remove the READ permission would fail, but that's not the case.

Thanks.

EDIT: asking here since I see in the list of tasks that "Update share" is not updated yet.

[rullzer]

@davivel well it is there for legacy reasons. We used to allow removing it (which resulted in broken shares). So right now it is just always added. Even when you create a share without read permissions they will be added automatically.

If you try to remove them it will thus allow the call. But if you then fetch the share again the read permissions should still be there.

[davivel]

Even when you create a share without read permissions they will be added automatically.

Nice to know :)

Thanks a lot, @rullzer

[rullzer]

@DeepDiver1975 I remember we talked about the bullet allow to pass in a token when creating a new share is that still required? I mean when update share allows you to migrate the share all should work right?

[PVince81]

@rullzer @schiesbn please move all remaining non-9.0 items to a separate ticket so we can close this one (if applicable)

[rullzer]

Ok lets move all remaining internal stuff to #22209 and then this can be closed.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.