nextcloud / collectives

Collectives is a Nextcloud App for activist and community projects to organize together.
GNU Affero General Public License v3.0
98 stars 16 forks source link

Sharing: password policy breaks using password protection #1269

Closed estux closed 2 months ago

estux commented 4 months ago

Describe the bug I just upgraded Collectives to 2.11 to try the new password protection feature (as it's very useful for me) but when I load the Collective as a visitor (link opened in anonymous browsing mode) the password protection is not enforced. It seems like it doesn't save the password when setted.

To Reproduce Steps to reproduce the behavior:

  1. Go to one Collective
  2. Open the Share tab on the right
  3. Click on + sign to add a sharing link
  4. Click on the 3 dots
  5. Click on Advanced Settings
  6. check "set password" checkmark
  7. delete the proposed password and insert a new one
  8. click on Update sharing
  9. copy the link
  10. open in another anonymous browser tab
  11. I can see the Collective and there's no password field to fill out

Expected behavior The password should be asked before showing the Collective.

Screenshots If applicable, add screenshots to help explain your problem.

Server details:

Client details:

Logs #### Nextcloud log (data/nextcloud.log) ``` I will add it later, thank you! ``` #### Browser log ``` Insert your browser log here, this could for example include: a) The javascript console log b) The network log c) ... ```
Solar-Simon commented 4 months ago

There are some further Problems with passwords:

where can I express my appreciation about collective and still show my lack of understanding, how such an feature can go live?

estux commented 4 months ago

There are some further Problems with passwords:

* It is not possible to see the password later on.

* It is only possible to switch to "can edit" when there is an password set

* it is not possible to change the password

* when you share a link, open it in another window. Then you set to password and "can edit", the password is active, "can edit" is not

* it is not possible to remove password

* Workaround for @estux

  * create a link
  * set password, "can edit"
  * update share
  * set "vie only"
  * update share

where can I express my appreciation about collective and still show my lack of understanding, how such an feature can go live?

Thanks @Solar-Simon I will try your workaround but as you told it seems strange something so unreliable went live. I appreciate too all the work on Nextcloud and the modules and I'm a big supporter of open source but sometimes this nasty bugs become really annoying.

mejo- commented 4 months ago

Interesting, thanks all for your reports. So far I'm unable to reproduce any of the problems you describe, so let's try to get a better understanding:

when I load the Collective as a visitor (link opened in anonymous browsing mode) the password protection is not enforced. It seems like it doesn't save the password when setted.

@estux please open the developer tools of your browser (Ctrl-Shift-I on Firefox/Chrome) and there the "Network" tab. Then create a new share and add a password to it. When clicking on "Update share", you should see a PUT request to /apps/collectives/_api/<collective_id>/share/<share_token. Does this happen? Also, are there any unsuccessful requests? And in the developer console, do you see any errors being logged?

It is not possible to see the password later on.

@Solar-Simon this is intended behaviour. The password is stored in a hashed format on the server. The server doesn't know about the plaintext password, so it's impossible to display it later on. It's the same e.g. for password protected shares of files.

It is only possible to switch to "can edit" when there is an password set

@Solar-Simon I'm unable to reproduce this. Could you please also open the developer tools of your browser (Ctrl-Shift-I on Firefox/Chrome) and see if any errors are logged there? Please also describe in detail what happens if you select "Can edit" on a share without a password being set.

it is not possible to change the password it is not possible to remove password

Could you provide a screencast of this? In my tests on different instances this works as expected. Again: any errors logged? Any further details you can share?

estux commented 4 months ago

Interesting, thanks all for your reports. So far I'm unable to reproduce any of the problems you describe, so let's try to get a better understanding:

when I load the Collective as a visitor (link opened in anonymous browsing mode) the password protection is not enforced. It seems like it doesn't save the password when setted.

@estux please open the developer tools of your browser (Ctrl-Shift-I on Firefox/Chrome) and there the "Network" tab. Then create a new share and add a password to it. When clicking on "Update share", you should see a PUT request to /apps/collectives/_api/<collective_id>/share/<share_token. Does this happen? Also, are there any unsuccessful requests? And in the developer console, do you see any errors being logged?

It is not possible to see the password later on.

@Solar-Simon this is intended behaviour. The password is stored in a hashed format on the server. The server doesn't know about the plaintext password, so it's impossible to display it later on. It's the same e.g. for password protected shares of files.

It is only possible to switch to "can edit" when there is an password set

@Solar-Simon I'm unable to reproduce this. Could you please also open the developer tools of your browser (Ctrl-Shift-I on Firefox/Chrome) and see if any errors are logged there? Please also describe in detail what happens if you select "Can edit" on a share without a password being set.

it is not possible to change the password it is not possible to remove password

Could you provide a screencast of this? In my tests on different instances this works as expected. Again: any errors logged? Any further details you can share?

Hello @mejo- just did what you asked and:

Should I share privately all the stack content?

Thanks for your reply!

mejo- commented 4 months ago

there was a PUT request but with a 500 status

Ok, so here's the culprit :wink: Can you once again reproduce the issue, have a look into the server side nextcloud.log logfile and post the error that is logged here?

Solar-Simon commented 4 months ago

it is not possible to change the password it is not possible to remove password

Could you provide a screencast of this? In my tests on different instances this works as expected. Again: any errors logged? Any further details you can share?

When I try to remove password:

{"reqId":"UO4dweZpBpZzIkZs2DVf","level":3,"time":"2024-05-27T20:36:16+00:00","remoteAddr":"217.....","user":"...","app":"collectives","method":"PUT","url":"/apps/collectives/_api/17/_pages/624905/share/7MPRA847RTbwM9a","message":"Collectives App Error: Password needs to be at least 7 characters long.","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:126.0) Gecko/20100101 Firefox/126.0","version":"28.0.4.1","exception":{"Exception":"Exception","Message":"Password needs to be at least 7 characters long.","Code":0,"Trace":[{"file":"/app/code/lib/private/Share20/Manager.php","line":1158,"function":"verifyPassword","class":"OC\\Share20\\Manager","type":"->"},{"file":"/app/code/lib/private/Share20/Manager.php","line":1028,"function":"updateSharePasswordIfNeeded","class":"OC\\Share20\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Service/CollectiveShareService.php","line":240,"function":"updateShare","class":"OC\\Share20\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":103,"function":"updateShare","class":"OCA\\Collectives\\Service\\CollectiveShareService","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ErrorHelper.php","line":23,"function":"OCA\\Collectives\\Controller\\{closure}","class":"OCA\\Collectives\\Controller\\ShareController","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":36,"function":"handleErrorResponse","class":"OCA\\Collectives\\Controller\\ShareController","type":"->"},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":107,"function":"prepareResponse","class":"OCA\\Collectives\\Controller\\ShareController","type":"->"},{"file":"/app/code/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"updatePageShare","class":"OCA\\Collectives\\Controller\\ShareController","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/code/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/app/code/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/app/code/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/app/code/lib/base.php","line":1069,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/app/code/index.php","line":39,"function":"handleRequest","class":"OC","type":"::"}],"File":"/app/code/lib/private/Share20/Manager.php","Line":201,"message":"Collectives App Error: Password needs to be at least 7 characters long.","exception":[],"CustomMessage":"Collectives App Error: Password needs to be at least 7 characters long."},"id":"6654eec8b92c2"}

Solar-Simon commented 4 months ago

It is only possible to switch to "can edit" when there is an password set

@Solar-Simon I'm unable to reproduce this. Could you please also open the developer tools of your browser (Ctrl-Shift-I on Firefox/Chrome) and see if any errors are logged there? Please also describe in detail what happens if you select "Can edit" on a share without a password being set.

{"reqId":"BuzIp67sPpHguRdO7byY","level":3,"time":"2024-05-27T20:48:40+00:00","remoteAddr":"217...","user":"...","app":"collectives","method":"PUT","url":"/apps/collectives/_api/17/_pages/624905/share/WfBDWYAiK3XPGRE","message":"Collectives App Error: Password needs to be at least 7 characters long.","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:126.0) Gecko/20100101 Firefox/126.0","version":"28.0.4.1","exception":{"Exception":"Exception","Message":"Password needs to be at least 7 characters long.","Code":0,"Trace":[{"file":"/app/code/lib/private/Share20/Manager.php","line":1158,"function":"verifyPassword","class":"OC\\Share20\\Manager","type":"->"},{"file":"/app/code/lib/private/Share20/Manager.php","line":1028,"function":"updateSharePasswordIfNeeded","class":"OC\\Share20\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Service/CollectiveShareService.php","line":240,"function":"updateShare","class":"OC\\Share20\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":103,"function":"updateShare","class":"OCA\\Collectives\\Service\\CollectiveShareService","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ErrorHelper.php","line":23,"function":"OCA\\Collectives\\Controller\\{closure}","class":"OCA\\Collectives\\Controller\\ShareController","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":36,"function":"handleErrorResponse","class":"OCA\\Collectives\\Controller\\ShareController","type":"->"},{"file":"/app/data/apps/collectives/lib/Controller/ShareController.php","line":107,"function":"prepareResponse","class":"OCA\\Collectives\\Controller\\ShareController","type":"->"},{"file":"/app/code/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"updatePageShare","class":"OCA\\Collectives\\Controller\\ShareController","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/app/code/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/app/code/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/app/code/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/app/code/lib/base.php","line":1069,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/app/code/index.php","line":39,"function":"handleRequest","class":"OC","type":"::"}],"File":"/app/code/lib/private/Share20/Manager.php","Line":201,"message":"Collectives App Error: Password needs to be at least 7 characters long.","exception":[],"CustomMessage":"Collectives App Error: Password needs to be at least 7 characters long."},"id":"6654f1a955118"}

mejo- commented 4 months ago

Aha, thanks so much! So you have a password policy in place an the provided password doesn't match this policy. For now, can you try to create/update the share with a password that matches your password policy?

Solar-Simon commented 4 months ago

So you have a password policy in place

ya true, but in files if I dont set a password its no problem. files does not enforce a password in general. Only if i set one and it does not meet requirements, there is an error.

For now, can you try to create/update the share with a password that matches your password policy?

Like I alread said with password its possible. But now that I explore further in this direction. I don't get an error when password is to short instead the old password is still active

Solar-Simon commented 4 months ago

It is not possible to see the password later on.

@Solar-Simon this is intended behaviour. The password is stored in a hashed format on the server. The server doesn't know about the plaintext password, so it's impossible to display it later on. It's the same e.g. for password protected shares of files.

is this really neccassary? As soon someone has access to this layer, one already has access to remove/change the password or remove the hole link. What is the risk of seeing the password an being able to change it?

For me this means, when I create a link and tell the password to person a and person b. Now Person b is losing the password and I also don't write it done somewhere: I have to create a new Passwoerd and also Person a now needs to be contacted again. Or I save the password somewhere, but this is extra work and I dont wont to create a database of passwords.

estux commented 4 months ago

there was a PUT request but with a 500 status

Ok, so here's the culprit 😉 Can you once again reproduce the issue, have a look into the server side nextcloud.log logfile and post the error that is logged here?

Hello @mejo- this time I did my homework completely and checked everything that is possible to help you.

  1. I didn't know/remember this sharing password must respect the password policy of the server :) I'm sorry about this!
  2. if I choose a good password (that respect the policy) then the Collective sharing works as expected (there's a PUT request, the password is saved, the shared Collective ask for a password before opening)
  3. problems arise if I choose a bad password (not respecting the policy):
    • I'm seeing that PUT request with 500 status
    • only in nextcloud.log it's written I chose a bad password and that I must choose a better one
    • the worst one: Collective is shared publicly without password enforced (the server should wait for a good password before sharing and not remove the password requirement)

So, as a user I would expect a prompt remembering me there's a password policy to respect when I enter the password. Then the server should not proceed until I type a good password instead of sharing the Collective without password enforced.

Thanks for your help and patience!

mejo- commented 4 months ago

Dear @estux, thanks for your thorough testing. I fully agree that the behaviour you experience is a bug. If you set a password, you expect the password protection to be in place unless the app shows a clear error message.

I'll look into a fix for this as soon as I find time to do so.

estux commented 4 months ago

@mejo- Glad to help when I can :) Sadly I cannot contribute code.

Thank you so much for considering this and looking into it!