Adding images to album from a folder shared with me fails

[BWibo]

Describe the bug
Adding images to an album from a folder that is shared with me fails.

To Reproduce

1. Create two Nextcloud users. Let's assume thair name are giver and receiver :-)
2. Using Nextcould files, create a folder e.g. called theShare with user giver upload some images and share it with user receiver with all rights (ediing, creating, resharing, deleting, download).
3. With user receiver, move the shared folder theShare to into you Memories folder.
4. With user reciever upload some more images to theShare.
5. Finally, try adding some files from theShare to an album. See it failing on the screenshot below.

Note:

• When giver tries to add the same image to an album everthing is fine.
• I tried this vice-versa for both users with the same result.

Screenshots

Platform:

• OS: Windows 10, Android 11
• Browser: Tested with Firefox and Brave (on both mobile and Desktop verssions)
• Memories Version: 5.1.0
• Nextcloud Version: 26.0.1
• PHP Version: PhP version used in official Nextcloud 26.0.0.1-fpm Docker image

Additional context

• When looking into the the docker-compose logs and the logging of the admin of the instance nothing appears.
• I'm running the docker compose stack: https://github.com/BWibo/nextcloud-compose.

• Any errors in the JS console?
  I'm getting this in the browser (Brave) console, when I open Memories. No idea if this is related to the issue.

mories-main.js?v=650e4c45-30:2     COPY https://XXX/remote.php/dav/files/XXX/Bilder/testshare/20220929_183040.heic 403
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
u.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.execute @ memories-main.js?v=650e4c45-30:2
e.patchInline @ memories-main.js?v=650e4c45-30:2
l @ memories-main.js?v=650e4c45-30:2
t.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
o @ memories-main.js?v=650e4c45-30:2
t.copyFile @ memories-main.js?v=650e4c45-30:2
copyFile @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
t.<computed> @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
c @ memories-main.js?v=650e4c45-30:2
Promise.then (async)
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
selectAlbum @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
pickAlbum @ memories-main.js?v=650e4c45-30:2
click @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
onClick @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
a._wrapper @ memories-main.js?v=650e4c45-30:2
memories-main.js?v=650e4c45-30:2 DAV COPY error <?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Can't add file to album, only files from bruno can be added</s:message>
</d:error>

(anonymous) @ memories-main.js?v=650e4c45-30:2
await in (anonymous) (async)
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
t.<computed> @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
c @ memories-main.js?v=650e4c45-30:2
Promise.then (async)
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
selectAlbum @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
pickAlbum @ memories-main.js?v=650e4c45-30:2
click @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
onClick @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
a._wrapper @ memories-main.js?v=650e4c45-30:2
memories-main.js?v=650e4c45-30:2     COPY https://XXX/remote.php/dav/files/XXX/Bilder/testshare/20220929_184157.heic 403
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
u.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.execute @ memories-main.js?v=650e4c45-30:2
e.patchInline @ memories-main.js?v=650e4c45-30:2
l @ memories-main.js?v=650e4c45-30:2
t.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
o @ memories-main.js?v=650e4c45-30:2
t.copyFile @ memories-main.js?v=650e4c45-30:2
copyFile @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
t.<computed> @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
c @ memories-main.js?v=650e4c45-30:2
Promise.then (async)
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
selectAlbum @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
pickAlbum @ memories-main.js?v=650e4c45-30:2
click @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
onClick @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
a._wrapper @ memories-main.js?v=650e4c45-30:2
memories-main.js?v=650e4c45-30:2 DAV COPY error <?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Can't add file to album, only files from bruno can be added</s:message>
</d:error>

(anonymous) @ memories-main.js?v=650e4c45-30:2
await in (anonymous) (async)
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
t.<computed> @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
c @ memories-main.js?v=650e4c45-30:2
Promise.then (async)
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
selectAlbum @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
pickAlbum @ memories-main.js?v=650e4c45-30:2
click @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
onClick @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
a._wrapper @ memories-main.js?v=650e4c45-30:2
memories-main.js?v=650e4c45-30:2     COPY https://XXX/remote.php/dav/files/XXX/Bilder/testshare/20220929_184210.heic 403
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
e.exports @ memories-main.js?v=650e4c45-30:2
u.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
e.execute @ memories-main.js?v=650e4c45-30:2
e.patchInline @ memories-main.js?v=650e4c45-30:2
l @ memories-main.js?v=650e4c45-30:2
t.request @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
o @ memories-main.js?v=650e4c45-30:2
t.copyFile @ memories-main.js?v=650e4c45-30:2
copyFile @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
t.<computed> @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
s @ memories-main.js?v=650e4c45-30:2
c @ memories-main.js?v=650e4c45-30:2
Promise.then (async)
s @ memories-main.js?v=650e4c45-30:2
(anonymous) @ memories-main.js?v=650e4c45-30:2
r.<computed>.o.<computed> @ memories-main.js?v=650e4c45-30:2
selectAlbum @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
pickAlbum @ memories-main.js?v=650e4c45-30:2
click @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
e.$emit @ memories-main.js?v=650e4c45-30:2
onClick @ memories-main.js?v=650e4c45-30:2
Xn @ memories-main.js?v=650e4c45-30:2
n @ memories-main.js?v=650e4c45-30:2
a._wrapper @ memories-main.js?v=650e4c45-30:2
memories-main.js?v=650e4c45-30:2 DAV COPY error <?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Can't add file to album, only files from bruno can be added</s:message>
</d:error>

• Any errors in the Nextcloud server logs?
  No, nothing there.

[pulsejet]

This is currently an upstream limitation
nextcloud/photos#1718

[BWibo]

This is really a pitty.

The pull request you mentioned above is merged, but I'm still facing this limitation with NC 27.
Does the PR resolve this issue and it's just not in the releases jet?

As long as this persists, a more informative error message would be helpful, e.g. like the one from the console:

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Can't add file to album, only files from bruno can be added</s:message>
</d:error>

Meanwhile, would this app be a possible workaround? https://apps.nextcloud.com/apps/groupfolders
As far as I understand, those group folders will always be located in the user root folders.
This would required to allow multiple root folders for Memories I guess. Users could then have e.g. one shared group folder and a private folder.

[johnhelt]

This is really a pitty.

The pull request you mentioned above is merged, but I'm still facing this limitation with NC 27. Does the PR resolve this issue and it's just not in the releases jet?

As long as this persists, a more informative error message would be helpful, e.g. like the one from the console:

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Can't add file to album, only files from bruno can be added</s:message>
</d:error>

Meanwhile, would this app be a possible workaround? https://apps.nextcloud.com/apps/groupfolders As far as I understand, those group folders will always be located in the user root folders. This would required to allow multiple root folders for Memories I guess. Users could then have e.g. one shared group folder and a private folder.

From what I could read, the pull request does not solve the isssue:

if ($ownerUID !== $uid) {
			throw new Forbidden("Can't add file to album, only files from $uid can be added");
		}

so in fact, the requirement is that ownerUID == uid. It would be great if that condition was not so strict, for example, if a user has explicitly allowed sharing, then adding files that are shared to an album should also be allowed.

[pulsejet]

From what I could read, the pull request does not solve the isssue:

Yeah, the linked PR only improves the randomness in the behavior as described there. The limitation still exists, which is why this issue is blocked.

It would be great if that condition was not so strict, for example, if a user has explicitly allowed sharing

Shares have an "allow resharing" permission so ideally we need to check for that. I didn't dive deep enough / didn't find a trivial solution to this yet.

[BWibo]

I guess part of the problem is, that this has to be re-check for each image in an album at every request, to make sure the permissions have not changed.

[pulsejet]

True. For example,

1. Alice shares a file with Bob and allows re-sharing
2. Bob adds the file to an album
3. Bob shares the album with Cathy
4. Alice un-shares the file from Bob
5. What now? Does Cathy still see the file in the album?

[BWibo]

No, if a user decides to unshare a file/disallow resharing, it should recursively be removed from all albums it contains, which no longer have permissions to the file. In this example, the file should vanish in both the albums of Bob and Cathy.

This can, of course be frustrating for Bob and Cathy, as their precious file is suddenly gone. Worst case, both come here and report this as a bug. 😆

A possible solution from a user perspective could be a warning message when opening an album that user XY has unshared N files and is no longer part of the album.

Another possible solution could be a placeholder image containing a message like: "Cathy unshared this file; it is no longer visible to you." If Cathy would reshare that file, it could be displayed again. Such a placeholder should be removed from an album like a regular file. Additionally, there should be an album-wide option to remove all placeholders from an album users can no longer access. This is important if many placeholders for unshared files are in an album and must be dropped simultaneously.

One question regarding the checks: Is a file accessible/allowed for re-sharing? Are those done now already? I assume there is no way around this anyway, right?

[pulsejet]

No, if a user decides to unshare a file/disallow resharing, it should recursively be removed from all albums it contains, which no longer have permissions to the file. In this example, the file should vanish in both the albums of Bob and Cathy.

Exactly, which makes it quite complex. For one, now we also need to track who added the photo to the album and make this check in a hook. There's also the possibility that Alice simply revokes the re-sharing permission on the shared folder, which should also trigger this. But I'm not sure if such a hook even exists at the moment.

Another possible solution could be a placeholder image containing a message like: "Cathy unshared this file; it is no longer visible to you." If Cathy would reshare that file, it could be displayed again. Such a placeholder should be removed from an album like a regular file.

This is a great idea. It does still need tracking who shared the file though, since if Alice opened the album, we need to know that Bob added it to the album originally, and then make the check that Bob can still access and re-share it. One new caveat here would be that it may leak some information, for example the date of the photo that was un-shared throught it's position in the timeline.

Is a file accessible/allowed for re-sharing? Are those done now already? I assume there is no way around this anyway, right?

The associated machinery mostly exists already, it's just a matter of integrating it with the albums backend of Photos. That's not trivial, unfortunately, since IIRC the DAV backend doesn't "see" shares etc. so we need separate lookups to determine those things.

[BWibo]

In general, is this something to implemented in memories or rather nextcloud/photos?
If this needs to be done in Photos, we should bring this discussion to them and file a issue/PR.

[pulsejet]

There's nextcloud/photos#1905

[ToM-Korn]

I have the same issue... but what is this discussion about? → won't fix because it's complex?

What is interesting to me is: I have a folder shared with another account. The account adds a picture there.
I can add it to an album, but the uploader can not... Even though he must be the owner ;)

So how does this move on? Is someone fixing the problem?

PS: To your discussion above... If I want to share shared files without the problem that the main sharer can revoke the access to it... I just copy them.

[pulsejet]

won't fix because it's complex?

No. It's just blocked cause we can't fix it here.

I have a folder shared with another account. The account adds a picture there.
I can add it to an album, but the uploader can not... Even though he must be the owner ;)

Nope. When you upload to a shared folder, the owner of the folder assumes the ownership of the uploaded file. This also applies to quotas etc.

So how does this move on? Is someone fixing the problem?

Don't think so, but I'm sure the Photos maintainers would be happy to get a PR for this (best to check with them on the implementation first though)

If I want to share shared files without the problem that the main sharer can revoke the access to it... I just copy them.

The problem isn't the lack of possible solutions but that the behavior might be unintuitive in various cases. For example, Alice might be aware that Bob shared the album to Cathy and we can't really infer if she did intend to unshare from just Bob or both. Either way, we do need to take one path and see how it works. But the reason this is blocked is more on implementation rather than this issue at the moment.