Indirect share of s2s share isn't possible

[MorrisJobke]

• Share a file by the invite based (mjob@server2) s2s share on server1
• login as mjob at server2 and accept the share
• works also with a normal share that is received
• create a folder
• put the accepted share into this folder
• share the folder by public link

Expected behaviour

I got a link

Actual behaviour

Error message Sharing "FOLDERNAME" failed, because it contains files shared with you!

Comments

• sharing the accepted folder directly simply works and no error pops up

cc @schiesbn @PVince81

[LukasReschke]

Sure that this isn't #13490?

[MorrisJobke]

I just reproduced with

*   bd88874 - (HEAD, origin/master, origin/HEAD, master) Merge pull request #13490 from owncloud/fix_reshare_s2s_share (vor 4 Stunden) <Vincent Petry>

as head (current master) on both instance. (pulled 5 minutes ago)

[MorrisJobke]

As I wrote: It works if I share the accepted share directly, but not if I put the share into a folder and then share that parent folder ;)

[LukasReschke]

Hey, it's late – don't blame me ;-)

[DeepDiver1975]

this should be easy to debug in order to find the cause - let's analyze this and see how this can be fixed an decide afterwards if this is 8.0 or 8.1

[PVince81]

@schiesbn can you have a look ?

[schiessle]

There is code in lib/private/share/share.php which check exactly this and refuses to create such shares. That's why we have such a descriptive error message for it.

But I'm not sure why we prevent such shares... @PVince81 do you have a idea? I remember we discussed all this various cases from mount points in mount points and reshares a lot. At the moment I don't see why it is blocked, but I'm quite sure we had good reasons back than so I don't want to remove this check to fast.

[schiessle]

Btw. that's not s2s related, same should happen if you put a local share in the folder.

[MorrisJobke]

@schiesbn But why is it working when I share the share itself directly?!? It should work in both ways or in none of these.

[nickvergessen]

btw @MorrisJobke with your 4th step (moving the accepted share into a folder) I get:
Error while scanning remote share: "http://localhost/ownCloud/OnlyMaster/" in the nice yellow top-bar

[PVince81]

I don't remember exactly which exact case. I think it was mostly cases where the permissions would become ambiguous, for example moving a folder you received from someone into your own outgoing share folder. That might qualify as a reshare though...

[MorrisJobke]

@PVince81 I moved the received folder into an unshared folder and then tried to share. Let me test that again.

[MorrisJobke]

Still an issue with up to date master on both servers. Now with screen recording (left is server1 and right is server2)

Log from server1:

{"reqId":"dacf3a737f79e9ed0fe0311d9539620f","remoteAddr":"127.0.0.1","app":"PHP","message":"Uninitialized string offset: 0 at \/home\/mjob\/Projekte\/owncloud\/core\/lib\/private\/urlgenerator.php#164","level":3,"time":"2015-01-29T16:35:23+00:00"}
{"reqId":"894120f0c8adb0ec46c20c791319eb68","remoteAddr":"127.0.0.1","app":"PHP","message":"The requested uri(\/core\/ocs\/v1.php\/cloud\/shares\/1\/accept) cannot be processed by the script '\/core\/\/ocs\/v1.php') at \/home\/mjob\/Projekte\/owncloud\/core\/lib\/private\/request.php#297","level":3,"time":"2015-01-29T16:35:29+00:00"}

log from server2:

{"reqId":"5bbf9118ce70c838a62600a049c71fd0","remoteAddr":"127.0.0.1","app":"core","message":"OC\\Tags::__construct, tags: Array\n(\n)\n","level":0,"time":"2015-01-29T16:34:40+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/list.php?dir=%2F&sort=name&sortdirection=asc"}
{"reqId":"07a315a3908e00dde42a0a36fa41ad14","remoteAddr":"127.0.0.1","app":"core","message":"OC\\Tags::__construct, tags: Array\n(\n)\n","level":0,"time":"2015-01-29T16:34:51+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/list.php?dir=%2Ftest&sort=name&sortdirection=asc"}
{"reqId":"3582ae7041ff4ce22cfb99834df9e348","remoteAddr":"127.0.0.1","app":"core","message":"OC\\Tags::__construct, tags: Array\n(\n)\n","level":0,"time":"2015-01-29T16:34:52+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/list.php?dir=%2F&sort=name&sortdirection=asc"}
{"reqId":"3859713af4016d00ee0093775d82a5a4","remoteAddr":"127.0.0.1","app":"core","message":"OC\\Tags::__construct, tags: Array\n(\n)\n","level":0,"time":"2015-01-29T16:35:27+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/list.php?dir=%2F&sort=name&sortdirection=asc"}
{"reqId":"e74f36d9a6f26854534569c7921a5c71","remoteAddr":"127.0.0.1","app":"core","message":"OC\\Tags::__construct, tags: Array\n(\n)\n","level":0,"time":"2015-01-29T16:35:30+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/list.php?dir=%2F&sort=name&sortdirection=asc"}
{"reqId":"75d2fe628ec15bf7540975df61466b04","remoteAddr":"127.0.0.1","app":"core","message":"Generating preview for \"\/welcome.txt\" with \"OC\\Preview\\TXT\"","level":0,"time":"2015-01-29T16:35:37+00:00","method":"GET","url":"\/master\/index.php\/apps\/files_sharing\/ajax\/publicpreview.php?file=%2F%2Fwelcome.txt&c=d6955ce0ac71d7854acda2382224df2c&t=17T7erhAPVSfsYd"}
{"reqId":"bda6a65dea697562536cf660f54a4d1d","remoteAddr":"127.0.0.1","app":"PHP","message":"ob_end_clean(): failed to delete buffer. No buffer to delete at \/home\/mjob\/Projekte\/owncloud\/master\/lib\/private\/files\/view.php#273","level":0,"time":"2015-01-29T16:35:39+00:00","method":"GET","url":"\/master\/index.php\/s\/17T7erhAPVSfsYd\/download?path=%2F&files=welcome.txt"}
{"reqId":"5da65b4c827e26170d0b6b8a457d423c","remoteAddr":"127.0.0.1","app":"OCP\\Share","message":"Sharing \"testit\" failed, because it contains files shared with you!","level":3,"time":"2015-01-29T16:36:03+00:00","method":"POST","url":"\/master\/index.php\/core\/ajax\/share.php"}

Screen recording: https://cloud.morrisjobke.de/public.php?service=files&t=0b8db8c9fd8cb88fdd2db4824b869638

[schiessle]

@MorrisJobke That's because if this check: https://github.com/owncloud/core/blob/master/lib/private/share/share.php#L572

If you remove it it should work... But at the moment I don't feel to comfortable to remove it. It might be OK in your case but I'm quite sure we had a good reason to add this check.

[MorrisJobke]

So this is partly the intended behaviour. Removing the 8.0 milestone

This is also related to #13776

[nickvergessen]

@schiesbn I guess the problem is that a sharing loop can be created.
But with remote shares we can't reliantly test this anymore.

Simplest loop:

User1: folder1 share public
User2: mount public-folder1 as folder2 and share public
User1: mount public-folder2 inside folder1

Have fun FileScanner

[MorrisJobke]

@nickvergessen Yes. That is actually a big problem and nothing we can solve for 8.0 I think. I hope this gets better once all this sharing stuff is simplified (#9058) This will reduce the complexity of such constructs a lot.

[PVince81]

@MorrisJobke does this still happen on master / 8.1 ?

[MorrisJobke]

@MorrisJobke does this still happen on master / 8.1 ?

Yes. Still happens. With both being master.

[MorrisJobke]

@michaelstingl Noticed this too. What is the solution for this? Can we deny the reshare if the folder contains a share that has no reshare permission?

BTW: If you share the folder before moving the share into it, it works. After the folder is shared you can move the received share in it and voila ... it works. We definetely need to work on the permissions. Because this allows a reshare ;)

[bboule]

Guys is this a blue ticket or is this simply a feature request?

[MTRichards]

Or is this a bug?

[MorrisJobke]

Guys is this a blue ticket or is this simply a feature request?

Or is this a bug?

I rated it as bug, because I felt like it should work. Then I talked to @schiesbn and he said, that it is working as intended and the other behavior (move a share into an existing share) is the bug (and not the feature).

I guess we need to have a look at this together with all the share work that is done nowadays.

cc @rullzer

[rullzer]

I don't get from this discussion what the actual behaviour we want is?

Shares within shares are a complex thing and I think we deny them for good reasons.

Let me explain:

1. UserA shares folder 'foo' read only with UserB.
2. UserB creates a folder 'bar' and moves 'foo' into 'bar'.
3. UserB shares 'bar' with full permissions with UserC.

Now obviously this should fail since we don't allow increasing permissions. But we can't check all the files/folders in 'bar' since that would kill all performance.

Now consider the other way around. If we would allow the share, userC would see 'bar' appear. But for some reason they do not have write access to 'foo'.

And this becomes even more of a mess since we want to move to a flat sharing model. So that the effective reshare of 'foo' would show up as a regular share at UserA.

Long story short. Shares within shares is a complex beast. And I think the 'real world' use cases are rather limited.

[MorrisJobke]

Long story short. Shares within shares is a complex beast. And I think the 'real world' use cases are rather limited.

@bboule To answer your question - this is rather a feature request.
@MTRichards Do we really want this? I would vote for make the decision here towards a simpler sharing model, which excludes such a quite rare use case of having received shares inside a share.

[MorrisJobke]

This is not really security relevant - rather privacy stuff.

[MTRichards]

I would like @schiesbn 's input on this. :)

The thought here:

1. We should be preventing the placement of shares within shares.
   a) This is BAD and we don't want to do that.
   b) We also want to prevent behaviors that lead a recipient to get a share within a share. (e.g. putting a share bar in a folder foo and sharing the folder foo with another person who would get a share within a share bar inside foo)

2. Server to server shares that are mounted are just shares.
   a) See 1). No shares within shares, including server to server.
   b) If allow resharing is off, it is off for server to server shares too.
   c) If I put a server to server share bar inside a folder foo, then I should not be able to share foo, as that is a share inside a share.

Then we also need to think carefully about all of the migration for this...so that we don't get orphans or other issues. Yeah!

[michaelstingl]

@MorrisJobke What is the status here?

[MorrisJobke]

@MorrisJobke What is the status here?

I don't think this has changed much, but maybe the flattened share data has brought some benefits - cc @rullzer ?

[rullzer]

@MorrisJobke I stand by my comment in #13695 (comment)

This is by design and complicates stuff significantly.

[michaelstingl]

@DeepDiver1975 @dragotin Could you have another look? What can we do here?

[PVince81]

@michaelstingl which would be the expected behavior you think we should work towards ? So far the current behavior is by design.

[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.