notes icon indicating copy to clipboard operation
notes copied to clipboard

Cannot use shared folder for notes anymore.

Open SimeonEhrig opened this issue 1 year ago • 19 comments

I share the Notes between two accounts. The first account is my main account. The second account is a dedicated account for my smartphone. Up to version 4.9.3, everything works fine. I could create and edit markdown files on my PC and sync it via Nextcloud client to the main account, use the web interface of my main account for editing notices and read and write notices on my smartphone with the nextcloud notes app syncing over the dedicated android account.

Since version 4.9.4. The android app and the notice app of the android account is not working anymore. The notice app on the main account is still working.

I think, this is related to this PR: https://github.com/nextcloud/notes/pull/1263

Steps to reproduce

  1. Create two accounts on the same nextcloud instance
  2. share the Notes folder of the account A with account B
  3. try to create a note in the web interface of account B and check if it is available on account A

Expected behaviour

The notices should be shared and the web interface and the android app should work with account A and B in the same way.

Actual behaviour

Account B creates an new notes folder (Notes (2)) and stores the notes there.

Server

Please complete the following information.

  • Notes app version: 4.9.4
  • Nextcloud version: 28.0.4
  • OS: Ubuntu 23.10 server
  • Web server: Apache (builtin in nextcloud docker image: https://github.com/nextcloud/docker)
  • PHP version: 8.2.17
  • Database: mariadb 10.6.17

Nextcloud configuration:

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "nextcloud.domain.com"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "overwrite.cli.url": "https:\/\/nextcloud.domain.com",
        "overwriteprotocol": "https",
        "dbtype": "mysql",
        "version": "28.0.4.1",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "logtimezone": "UTC",
        "installed": true,
        "maintenance": false,
        "theme": "",
        "loglevel": 0,
        "updater.release.channel": "stable",
        "app_install_overwrite": [
            "calendar",
            "drawio",
            "tasks"
        ],
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpsecure": "tls",
        "mail_smtpauthtype": "LOGIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "encryption.key_storage_migrated": false,
        "trashbin_retention_obligation": "auto, 30",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "password": "***REMOVED SENSITIVE VALUE***",
            "port": 6379
        },
        "default_phone_region": "DE",
        "mysql.utf8mb4": true
    }
}

Client

Please complete the following information.

  • Browser (incl. version): Firefox 124.0.1
  • OS: Linux Mint 21.3

SimeonEhrig avatar Mar 31 '24 11:03 SimeonEhrig

Yes, I can verify this is happening to me as well. This error message repeatedly appears in the Nextcloud logs.

LockedException "Notes" is locked
Controller failed with OCP\Lock\LockedException

areteichi avatar Mar 31 '24 19:03 areteichi

This is happening to me as well using the same setup as above. The bug was introduced in commit a5bec1a.

After commenting out the faulty code in the source code of the installed Notes App, everything works as expected again.

Unfortunately it is unclear why this fix was introduced and why no tests failed.

callmetango avatar Apr 01 '24 13:04 callmetango

Can confirm, reverting https://github.com/nextcloud/notes/commit/a5bec1a47db987313aaec793cc6c4f8ec6b48eb7 does help, just in my case the shared-to user needed to temporarily set a different folder before I could re-share the original. Might be normal behavior caused by me deleting the share before, in an attempt to fix the issue.

Similar setup as OP, Notes 4.9.4.

maranov avatar Apr 01 '24 14:04 maranov

Can confirm.. This is a regression for me :/

jaykijay avatar Apr 03 '24 16:04 jaykijay

Thanks for reporting it, got same problem, and did use the same revert of https://github.com/nextcloud/notes/commit/a5bec1a47db987313aaec793cc6c4f8ec6b48eb7 to fix it I do share my "Notes" folder with my wife, and it was not working anymore, it was creating endlessly "Notes (2)", "Notes (3)", ... , up to "Notes (19)" FYI, we are mainly using the "Nextcloud Notes" Android app where we had the problem using the "Steps To Reproduce" reported above

Best Regards

lftsy avatar Apr 11 '24 13:04 lftsy

All of my storage is external since my nextcloud docker container is ephemeral, so all storage is "shared" by group rules, even the user's home directory. So I can't use Notes any more.

The current implementation using Folder->getNonExistingName() couldn't have passed even superficial testing in a shared folder. The program flow goes like this: If the chosen notes folder exists but is shared, it may not be used and a new one is created (with an incrementing number appended). Once it has been created, the existence check confirms that it exists, which now makes it forbidden since only non-existing folders are allowed, that's the point of getNonExistingName(). Therefore it fails to use the just created folder and a new (higher numbered) folder is created, and the cycle repeats forever.

So whenever NoteUtil->getOrCreateFolder is called (apparently quite frequently, on every note file save attempt), it creates a new empty copy of the forbidden existing folder. This goes on forever and creates hundreds of empty folders in minutes. Obviously, getNonExistingName is being used incorrectly.

Is there a reason for the apparent intention not to use existing shared folders?

Waltibaba avatar Apr 17 '24 06:04 Waltibaba

Indeed, it's still broken, I've tried both options, with and without the code

		if ($folder->isShared()) {
			$folderName = $this->root->getNonExistingName($path);
			$folder = $this->root->newFolder($folderName);
		}

it simply stays broken. I can't use the Notes app anymore together with my partner. At the legitimate user, who is sharing the folder, everything works fine. The one who is using the shared folder, gets these other folders 'Notes (2)' etc.

gerhardt avatar Apr 20 '24 13:04 gerhardt

Any response on this problem? Looks like a lot of people have the same problem and nobody knows, what the code should do, which causes the problem.

It is a little bit annoying, yesterday was a new release and there is still no response from a developer :-( At the moment, this means I have to fix the bug locally from time to time :-(

SimeonEhrig avatar Apr 25 '24 05:04 SimeonEhrig

I have the same problem. First my parter had an issue, then I've changed the permission, she started to share with me, and now I cannot use Notes app on my phone.

lapor-kris avatar May 02 '24 16:05 lapor-kris

Updated to 4.10.0. The issue is still the same and the workaround (reverting a5bec1a47db987313aaec793cc6c4f8ec6b48eb7) still works.

maranov avatar May 02 '24 18:05 maranov

I can confirm this too, it is really annoying when using shared folders.

@juliushaertl: What was your motivation in #1260 ? Can we revert this? There is already a PR for this: #1296

korelstar avatar May 15 '24 19:05 korelstar

I have this issue too.

The whole point of Nextcloud is private sharing. If we cannot share notes in a folder, surely this is a fundamental break in the point of Nextcloud notes existing?

Is there no reason we cannot merge in https://github.com/nextcloud/notes/pull/1296 as it is a fix?

matthewlordtech avatar Jun 02 '24 13:06 matthewlordtech

How's the current status?

Mannshoch avatar Jun 10 '24 16:06 Mannshoch

How's the current status?

Wait for response from @juliushaertl who wrote the line of code. His GitHub status mentioned, that he was in vacations until this week.

SimeonEhrig avatar Jun 13 '24 06:06 SimeonEhrig

This is an important Issue, since it breaks the posebility for guest accounts to use Notes. Guests can only write in folders shared with them. Background: https://help.nextcloud.com/t/guests-cannot-create-notes/195053/6

ernolf avatar Jun 17 '24 09:06 ernolf

@juliushaertl Can you please give us feedback?

areteichi avatar Jun 21 '24 20:06 areteichi

If we know the solution, it's just to restore a single line of removed code, and have a PR - why on earth is this being left broken?

matthewlordtech avatar Jun 21 '24 23:06 matthewlordtech

I think the "problematic" line of code is related to this security advisory: https://github.com/nextcloud/security-advisories/security/advisories/GHSA-wfqv-cx85-7rjx

So what we use as a feature (sharing notes), is probably seen as security bug for them...

bendeich avatar Jun 22 '24 15:06 bendeich

The solution isn't just to break legitimate use though without explanation and just have buggy behaviour - a solution would be to have some kind of dialog asking the user if they want to use an existing folder already shared

matthewlordtech avatar Jun 23 '24 17:06 matthewlordtech

Small comment about the bug fix. It is fixed in general but there is a edge case where the bug still happen. If an account A shares the whole notes folder with account B and account B has no own notes folder, the shared notes folder will be named also notes in account B. In this case, the note app still tries to create a new folder. The solution is to rename the notes folder in account B and set it as note folder in the app configuration. The name of the original notes folder in account A is not affected.

SimeonEhrig avatar Aug 17 '24 09:08 SimeonEhrig

WORKAROUND: Share only some Notes subfolder ( used for catogory )

FYI, I was impacted by the same problem and I did not wanted to apply old rollback patches anymore, but I have found a workaround, now each user uses its own Notes folder, but we do share some subfolder ( used for Notes Category )

Workaround

  1. Have 2 users with their own "Notes" folder
  2. On user 1, Assign a Category "Family Notes (shared)" to a note, it will create "Notes/Family Notes (shared)" folder
  3. On user 1, From nextcloud app, Share the "Notes/Family Notes (shared)" folder with user 2
  4. On user 2, drag and drop the folder "Family Notes (shared)" to "Notes/" ( because shared folders land on root folder ) => You will end up with both user able to see and use the Category "Family Notes (shared)", as soon as a note is tagged with this one, it is shared between them => I like this solution because it allows to keep some private notes and share only some of them

Best Regards

lftsy avatar Aug 18 '24 17:08 lftsy