nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.56k stars 4.08k forks source link

[Bug]: Email notification for share, even when activity settings is disabled #33205

Open EngelPika32 opened 2 years ago

EngelPika32 commented 2 years ago

⚠️ This issue respects the following points: ⚠️

Bug description

Similar to what was described in this ancient issue: #11216

A friend shared a folder, and I received an email. My settings under settings/user/activity for "A file or folder has been shared" are disabled for email (and push). Hence I shouldn't have received an email. – With a link to accept the share, although I've (under settings/user/sharing) "Allow shares by default" enabled. The share got accepted automatically (or at least, it appeared in my folders without me using the accept link from the mail).

Oh, and late on my friend added a note to the share, which caused a second mail; which is unwanted as well.

Steps to reproduce

  1. Admin settings: EMail configured; Sharing via mail allowed.
  2. User1 settings (activity): disable EMail for "A file or folder has been shared"
  3. User2: share a folder (or file) by entering the Username, and selecting the user from the search results/suggestions
  4. User1 receives an Email (with a link to accept the share)

Expected behavior

No EMail, if the user (receiving a share) doesn't want EMails about this. This can get quite spammy if you receive a lot of shares.

Installation method

Official Docker image

Operating system

Other

PHP engine version

PHP 8.1

Web server

Other

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

No response

What user-backends are you using?

Configuration report

No response

List of activated Apps

Don't have access to that right now.

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

Sharing via email was just recently enabled on our instance. Hence I do not know if this bug is new. But seeing the old issue with a relatively new comment about it, makes me confident, that this isn't new.

Mer0me commented 2 years ago

Bug is still present on v24.0.5. There is no way to prevent sending of sharing emails.

Ododo commented 2 years ago

Ok so it seems this option of config.php can be set to false as workaround:


 * Set to false to stop sending a mail when users receive a share
 */
'sharing.enable_share_mail' => true,

It triggers an email from within the core Mailer.php, which seems redundant with the activity app own mailer.

Note that turning this option to false, does not prevent you to create links or to actually share a file to an email. I think the option name is misleading.

also see: https://github.com/nextcloud/server/issues/26183

szaimen commented 1 year ago

Hi, please update to 24.0.9 or better 25.0.3 and report back if it fixes the issue. Thank you!

My goal is to add a label like e.g. 25-feedback to this ticket of an up-to-date major Nextcloud version where the bug could be reproduced. However this is not going to work without your help. So thanks for all your effort!

If you don't manage to reproduce the issue in time and the issue gets closed but you can reproduce the issue afterwards, feel free to create a new bug report with up-to-date information by following this link: https://github.com/nextcloud/server/issues/new?assignees=&labels=bug%2C0.+Needs+triage&template=BUG_REPORT.yml&title=%5BBug%5D%3A+

Mer0me commented 1 year ago

Tested on 25.0.3 and the problem is still present.

datenangebot commented 1 year ago

This bug is still present, tested with 24.0.9. You even get mail if you deactivated to receive mails.

I see two approaches:

  1. We should respect the activity settings for sending mails in general. (https://github.com/nextcloud/server/blob/c0e84cb512ca76cea263642918f9e045aed3c5c4/lib/private/Share20/Manager.php#L837)
  2. The description of the activities settings should make clear what it effects.

@szaimen You asked for feedback here (via label), do you need more information about that?

crzp commented 1 year ago

This bug is still present in 24.0.12 and very annoying. Please make it toggleable!

Btw, it also effects creating, deleting, changing files in a shared folder. It creates so much spam for all users who a part of the shared folder.

juliusknorr commented 1 year ago

@nextcloud/designers We currently have two possible mails when a file is shared to a user

Screenshot 2023-08-01 at 09 57 24

I'd say it makes sense to check the setting of the activity app and not sending out any email notification then, would you agree?

Then we could just add a check for this in https://github.com/nextcloud/server/blob/c0e84cb512ca76cea263642918f9e045aed3c5c4/lib/private/Share20/Manager.php#L837

One thing worth to mention is that The activity option is disabled by default, so we may need to switch this?

Also cc @artonge and @miaulalala for the activity app

jancborchardt commented 1 year ago

A mail sent by the files_sharing app to the user which can only be disabled globally

@juliushaertl what info would this contain?

But generally yes, I would agree with your assessment to check the setting of the activity app. If people say no mail regarding that we should honor that.

One thing worth to mention is that The activity option is disabled by default, so we may need to switch this?

Yes, probably also makes sense if currently the de-facto default is getting an email anyway.

juliusknorr commented 1 year ago
juliusknorr commented 1 year ago

@jancborchardt Those are the two mails:

  1. Email by the files sharing app (may contain share note)

    • Controlled by the config.php option sharing.enable_share_mail, enabled by default, one individual mail for each share
    • Screenshot 2023-10-17 at 09 28 43
  2. Email by the activity app

    • Controlled by admin defaults with user setting overwrite in activity settings UI, disabled by default
    • List of activities that might contain other entries as well
    • Might happen immediately or later depending on the user config
    • Screenshot 2023-10-17 at 09 28 54

Thinking more about it I'm not entirely sure we should couple those settings.

miaulalala commented 1 year ago

Thinking more about it I'm not entirely sure we should couple those settings.

I agree. But we could make the sharing app a per-user setting. That would work nicely and we can store that in oc_preferences.

max-nextcloud commented 11 months ago

I agree. But we could make the sharing app a per-user setting. That would work nicely and we can store that in oc_preferences.

@jancborchardt - What do you say? Would that be the way to go?

jancborchardt commented 11 months ago

@miaulalala @max-nextcloud yeah, that actually makes a lot of sense, that should be personal preference. :)