What if reshare happens to a group that the owner belongs to?

[navid-shokri]

assume this scenario:

user1@oc1 that belongs to group1 share a file with an entity (user/group) on oc2 and then the user2@oc2 share the file again with group1@oc1? ❓

what should be our reaction?

[yasharpm]

In other words:
In order to find the remote of a reshared file, the code for the FederatedShareProvider has to look up both the share_external and share_external_group tables.
The federatedfilesharing app works closely with direct instantiation of its FederatedShareProvider. Meaning that if ShareProviderFactory returns a different class instance for SHARE_TYPE_REMOTE share type, the federatedfilesharing app is not using it anyways.

[yasharpm]

After spending a few hours investigating the reshare problem, I have come up with 3 solutions:

1. (our current direction) I have overwritten the FederatedShareProvider for sharing with a remote user in our OCM app. Our ShareProviderFactory returns this instance. federatedfilesharing app must be modified to only get an instance of FederatedShareProvider from the share manager (which gets it from the factory). I checked its others usages. It doesn't break other parts of the OwnCloud.
2. (@michielbdejong's approach that we didn't take) We completely take over the federatedfilesharing app. Meaning our FedShareManager that will be configured to be used, will handle both SHARE_TYPE_REMOTE and SHARE_TYPE_REMOTE_GROUP share types. Our FedShareManager in turn will be talking to 2 similar instances of the original FedShareManager for the 2 share types.
3. (the elegant approach, my favortie) Through out OwnCloud the remote share type has been assumed and hard coded. In a sense, federatedfilesharing app is actually spread on file_sharing app as well as the core and other apps as well. We already had to clean many of them to work dynamically based on the providers available in the share manager. federatedfilesharing app can be totally modified to work only based on the share manager and a mapping from OwnCloud remote share type constants to the OCM share type constants. The custom configuration (federatedfilesharing.fedShareManager) will no longer be needed. When we change the ProviderFactory, we are effectively changing the behavior of the federatedfilesharing app without touching it (after revolutionizing it of course!).

Solution 1 is the quickest as it is mostly already done. But as it already relies on the FedShareManager configuration, it is ugly to have a dual behavior/reliance by getting the FederatedShareProvider from the share manager. It takes 1 day to implement. Solution 2 takes 2 days to implement but @navid-shokri is against our app to take unnecessary responsibility. Solution 3 is the most seamless one but it incurs the most changes. It takes 3 days to implement not considering the many existing test codes that will fail and will need fixing.

My assessment is that since we are planning to work on a new version of the OCM, the federatedfilesharing app will be effectively deprecated and need major changes to it. At which point all our current work will be null as well. Hence, I suggest the quickest solution which is solution 1. And if we are not looking at the OwnCloud conforming to some new OCM specification any time soon, then solution 3 is my go to.

[michielbdejong]

In other words

I don't think you accurately reworded Navid's question, I think you gave an answer to it! :)

I have come up with 3 solutions

I think you listed 3 code architectures here

federatedfilesharing app can be totally modified

How does that option affect our goal of keeping the changes we make in owncloud/core to a minimum? Remember this app is part of owncloud/core, so al we're allowed to do in it is insert if-statements (or catch-statements, I think that's fine too) that divert the code path if one or both of our apps are installed, and that do nothing otherwise.

[michielbdejong]

@yasharpm regarding your discussion about code architecture, maybe you can split that out to a separate issue?

And regarding @navid-shokri 's original question, I think we should test how it is handled if this happens on a single server:
user1@oc1 that belongs to group1 share a file with an entity (user/group) that is also on oc1 and then the user2@oc1 share the file again with group1@oc1? ❓

I think that, although resharing to the resource owner is prohibited, resharing to a group of which the resource owner is a member is not. So we don't have to do any lookups regarding group memberships of user1@oc1, we can just let the reshare happen.

I think processing of reshare-request notifications is already implemented on the sender side, and our additional share type should not have much effect on that code, right? The only thing we need to add is reshare to a remote group (for @yasharpm's OCM app) and reshare to a mixed group (for @navid-shokri's FG app). In both cases, the answer to "what if reshare happens to a group that the owner belongs to?" is, I think "no special requirements, just reshare as usual"! :)

[yasharpm]

I don't think you accurately reworded Navid's question, I think you gave an answer to it!

It's getting closer to understanding the question and hence the answer.

I think processing of reshare-request notifications is already implemented on the sender side, and our additional share type should not have much effect on that code, right?

There is a fundamental problem here. Different share type implementations need to be aware of each other. If I want to share something to a user that has been shared with me as a group, the reshare to user logic has to find the share in the shared to group table (i.e. share_external_group). This is valid for all reshares from one share type to another.

I think you listed 3 code architectures here
How does that option affect our goal of keeping the changes we make in owncloud/core to a minimum

Hence we can not longer be as passive to OC managers' liking. If we want share to groups to work, we have no choice but to change federatedfilesharing in a way that respects a deeper level of intervention. Or completely steal the control from it which is solution 2.

[michielbdejong]

If I want to share something to a user that has been shared with me as a group, the reshare to user logic has to find the share in the shared to group table (i.e. share_external_group)

Ah but that is for the process at the receiver's side, that leads up to sending the reshare-request notification.
The way I understood the OP's question was strictly about how the sending side should handle a reshare-request notification, once it arrives?

[yasharpm]

Ah I think I had to create a separate issue! Apologies, just because the issue was related and recent, I assumed it is about the issue I raised.
Regarding the way it is handled, it's like a chain. When we receive a reshare request, we check if we are the owner or the file has been shared with us. If it is shared, we forward the call to whom shared it with us. The chain breaks if we can not find a share from the other type.

[michielbdejong]

This potentially affects our decision from 19 December - discussing in a video call now.

[michielbdejong]

As discussed, we are not going to work on this issue during this sprint.
Once we do have time to look into this, we should first observe, and then generalise from what we see.
So we have three things to look at:

• suppose user1@oc1 shares with user2@oc1 and user2@oc1 requests a reshare with group1@oc1. What happens? Is a local user-to-group share created.
• suppose user1@oc1 shares with user2@oc2 and user2@oc2 requests a reshare with user3@oc1. What happens? Is a local user-to-user share created?
• on Nextcloud, suppose user1@oc1 shares with group2@oc2 and user2@oc2 requests a reshare with user3@oc1. How does Nextcloud deal with reshare after user-to-group OCM?