Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: 30.0.0 share by email - no email is sent #48012

Closed
5 of 8 tasks
Tracked by #5249
himpierre opened this issue Sep 15, 2024 · 32 comments · Fixed by #48381
Closed
5 of 8 tasks
Tracked by #5249

[Bug]: 30.0.0 share by email - no email is sent #48012

himpierre opened this issue Sep 15, 2024 · 32 comments · Fixed by #48381

Comments

@himpierre
Copy link

⚠️ This issue respects the following points: ⚠️

Bug description

Hello Devs.

After updating to 30.0.0 sharing files via email does not work as it is supposed to I guess. There is no mail being sent to the sharee. So the one I try to share the files with is not getting notified. Nothing in the logs.
Email in general works. Invitations for talk go out.

Steps to reproduce

  1. Share file via Email
  2. Save
  3. No email to the sharee

Expected behavior

Sharee gets notified.

Nextcloud Server version

master

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.3

Web server

Apache (supported)

Database engine version

MySQL

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

Upgraded to a MAJOR version (ex. 28 to 29)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "30.0.0.14",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "maintenance": false,
        "maintenance_window_start": 3,
        "theme": "",
        "trusted_domains": [
            "fluffycloud.de"
        ],
        "mail_smtpmode": "sendmail",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "loglevel": 2,
        "log_rotate_size": 104857600,
        "filelocking.enabled": true,
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0
        },
        "htaccess.RewriteBase": "\/",
        "updater.release.channel": "stable",
        "preview_max_filesize_image": -1,
        "overwrite.cli.url": "https:\/\/fluffycloud.de\/",
        "mysql.utf8mb4": true,
        "default_phone_region": "DE",
        "defaultapp": "files",
        "app_install_overwrite": [
            "calendar",
            "groupfolders",
            "mail",
            "spreed",
            "passman",
            "audioplayer",
            "polls",
            "passwords",
            "breezedark"
        ]
    }
}

List of activated Apps

Enabled:
  - activity: 3.0.0
  - admin_audit: 1.20.0
  - calendar: 5.0.0
  - cloud_federation_api: 1.13.0
  - comments: 1.20.1
  - contacts: 6.1.0
  - contactsinteraction: 1.11.0
  - dashboard: 7.10.0
  - dav: 1.31.1
  - federatedfilesharing: 1.20.0
  - files: 2.2.0
  - files_downloadlimit: 3.0.0
  - files_external: 1.22.0
  - files_pdfviewer: 3.0.0
  - files_reminders: 1.3.0
  - files_sharing: 1.22.0
  - files_trashbin: 1.20.1
  - files_versions: 1.23.0
  - firstrunwizard: 3.0.0
  - logreader: 3.0.0
  - lookup_server_connector: 1.18.0
  - nextcloud_announcements: 2.0.0
  - notes: 4.10.1
  - notifications: 3.0.0
  - oauth2: 1.18.1
  - oidc: 0.9.4
  - password_policy: 2.0.0
  - passwords: 2024.7.20
  - photos: 3.0.2
  - polls: 7.2.3
  - privacy: 2.0.0
  - provisioning_api: 1.20.0
  - recommendations: 3.0.0
  - related_resources: 1.5.0
  - richdocuments: 8.5.0
  - serverinfo: 2.0.0
  - settings: 1.13.0
  - sharebymail: 1.20.0
  - spreed: 20.0.0
  - support: 2.0.0
  - survey_client: 2.0.0
  - systemtags: 1.20.0
  - text: 4.1.0
  - theming: 2.5.0
  - twofactor_backupcodes: 1.19.0
  - updatenotification: 1.20.0
  - user_status: 1.10.0
  - viewer: 3.0.0
  - weather_status: 1.10.0
  - webhook_listeners: 1.1.0-dev
  - workflowengine: 2.12.0
Disabled:
  - bruteforcesettings: 3.0.0 (installed 2.4.0)
  - circles: 30.0.0-dev (installed 27.0.0)
  - encryption: 2.18.0
  - federation: 1.20.0 (installed 1.17.0)
  - files_rightclick: 0.15.1 (installed 1.6.0)
  - groupfolders: 18.0.1 (installed 18.0.1)
  - secrets: 2.0.3 (installed 2.0.3)
  - suspicious_login: 8.0.0
  - twofactor_nextcloud_notification: 4.0.0
  - twofactor_totp: 12.0.0-dev (installed 5.0.0)
  - user_ldap: 1.21.0
  - whiteboard: 1.0.1 (installed 1.0.1)

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

No response

@himpierre himpierre added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Sep 15, 2024
@nfebe nfebe self-assigned this Sep 15, 2024
@nfebe nfebe added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 15, 2024
@Bad-and-Mad
Copy link

The same here. But the function is essential for me (and probably for others too).

@hxcde
Copy link

hxcde commented Sep 17, 2024

The same here. But the function is essential for me (and probably for others too).

Yes its the only usecase for our company. Same problem here, just restored the old version from backup and waiting for the fix.

@Bad-and-Mad
Copy link

The same here. But the function is essential for me (and probably for others too).

Yes its the only usecase for our company. Same problem here, just restored the old version from backup and waiting for the fix.

Lucky you. In our case, by the time I realized it, so much data had been changed that it no longer made sense to restore the backups for a downgrade. Unfortunately, I now have to wait for the error to be rectified. However, I cannot understand why such an important function for a version release was not tested extensively. My OpenOffice connection also only works if I change the program code. After that, however, the mail app no longer works. None of the last releases have caused so much trouble.

@daveatpinellas
Copy link

Confirmed this is broken in 30.0.0. The [ Send Email ] button to test smtp works. However when you share a file by email address, the outside user never gets email. Confirmed that email is bouncing correctly to this same smtp host at the operating system level.

@Bad-and-Mad
Copy link

Here is my log (level 0):

"Share notification not sent because mailsend is false."

{"reqId":"MVHwqqQbuNmAEozOhw5i","level":0,"time":"2024-09-24T12:54:28+02:00","remoteAddr":"x.x.x.x","user":"xyz","app":"share","method":"POST","url":"/ocs/v2.php/apps/files_sharing/api/v1/shares","message":"Share notification not sent because mailsend is false.","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36","version":"30.0.0.14","data":{"app":"share"}}

@himpierre
Copy link
Author

Hello Devs!

Is there maybe a patch or workaround we can try until this is fixed?

cheers!

@Bad-and-Mad
Copy link

Dear developers, is there anyone who can tell us how to set mailsend to true? There must be a way in the code to influence this logical state, as it is also queried and evaluated by the sharing api.

@Orobas42
Copy link

This issue deserves urgent attention. The email sharing function is a critical feature of Nextcloud, and we are now two weeks into a stable release with this functionality broken.

I've observed an additional detail that may be helpful for troubleshooting:
When creating a new share, no email is sent to the recipient. However, if we edit an existing share (e.g., change the password or add a comment), emails are successfully sent.

This behavior suggests that the email sending mechanism itself is working, but there's a specific issue with the initial share creation process. Can the development team please investigate this discrepancy?

Given the importance of this feature for many users and organizations, we urgently need an update on the status of this bug and an estimated timeline for a fix. In the meantime, are there any workarounds that can be implemented to restore this functionality? If there's a specific commit that introduced this issue, would it be possible to temporarily revert that commit as a quick fix until a proper solution is developed?

@Bad-and-Mad
Copy link

Bad-and-Mad commented Sep 26, 2024

The quick and dirty customization that works for me. But the function is essential for us.

File: NC_DIR/lib/private/Share20/Share.php

From line 590

        public function getMailSend() {
                // by M.W.
                // return $this->mailSend;
                return true;
        }

@Orobas42
Copy link

This is really dirty. You shouldn't do it in the getter function, as this is also called in other places. The “better” dirty way is to set it to true at the actual place in the code.

File: /lib/private/Share20/Manager.php
Line 740:

//if ($share->getMailSend()) {
if (true) {

Thats the place where the message “Share notification not sent because mailsend is false.” is raised.

But it's still the most dirty workaround in my opinion. What happens when you share with nc users or just want to share a link?

@Bad-and-Mad
Copy link

Better dirty is always possible. In any case, for now there is a solution that will help many people here who are dependent on the function. I don't want to wait another two weeks. And it is confirmed again and again: don't use a 0 version on productive systems.

@nfebe
Copy link
Contributor

nfebe commented Sep 26, 2024

Better dirty is always possible. In any case, for now there is a solution that will help many people here who are dependent on the function. I don't want to wait another two weeks. And it is confirmed again and again: don't use a 0 version on productive systems.

Hello @Bad-and-Mad sorry you have had to deal with this for a while, this shall be fixed by the next release.

@Bad-and-Mad
Copy link

Hello @Bad-and-Mad sorry you have had to deal with this for a while, this shall be fixed by the next release.

I assumed that the problem would be fixed in the next version, but for all those who have upgraded to version 30, a solution is needed for such an important function. Nobody knows when version 30.0.1 will be released and whether the problem will really have disappeared. I just hope that the change in the program code does not cause any further problems.

nfebe added a commit that referenced this issue Sep 26, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Sep 26, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@Bad-and-Mad
Copy link

Addendum: The program should not search for logical specifications that nobody can set...neither in the web frontend nor in the config.php.

nfebe added a commit that referenced this issue Sep 26, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Sep 26, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Sep 26, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Sep 27, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Sep 30, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@nfebe nfebe added the 2. developing Work in progress label Oct 24, 2024
nfebe added a commit that referenced this issue Oct 24, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Oct 24, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Oct 24, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
nfebe added a commit that referenced this issue Oct 24, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
backportbot bot pushed a commit that referenced this issue Oct 24, 2024
It looks like, the frontend it needs to provide the `sendMail` param
for the backend to decide wether mails would be sent.

Our UI does not have that at the moment so it should default to sending
emails always for mail shares.

Not exactly sure how this was handled earlier but this is a good starting point.

Resolves : #48012

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@nfebe nfebe removed 1. to develop Accepted and waiting to be taken care of 2. developing Work in progress labels Oct 24, 2024
@nfebe nfebe moved this to ☑️ Done in 📁 Files team Oct 24, 2024
@nfebe nfebe added this to the Nextcloud 30.0.2 milestone Oct 24, 2024
@dd4y
Copy link

dd4y commented Nov 5, 2024

Hello,

My implementation also relies heavily on this feature. I confirm that is it son functioning in 30.0.1.

@tigernero79
Copy link

I have this problem in both a 30.0.1 and 30.0.2 rc2 instance. Is there a manual fix that can be done while waiting for a solution?

@DerDP
Copy link

DerDP commented Nov 6, 2024

I have this problem in both a 30.0.1 and 30.0.2 rc2 instance. Is there a manual fix that can be done while waiting for a solution?

Workaround from @Bad-and-Mad works for me.
#48012 (comment)

@tigernero79
Copy link

I don't understand if the line should be removed or modified in the file manager.php

@DerDP
Copy link

DerDP commented Nov 6, 2024

I don't understand if the line should be removed or modified in the file manager.php

Modified. In his example he commented out the old if statement and add new one.

@nfebe
Copy link
Contributor

nfebe commented Nov 7, 2024

I have this problem in both a 30.0.1 and 30.0.2 rc2 instance. Is there a manual fix that can be done while waiting for a solution?

We do not expect to see this issue as from 30.0.2 rc1

@tigernero79
Copy link

and yet I updated from 30.0.2 rc2 and the problem continues. given the changes made to the file, perhaps the file should also be modified?

File: /lib/private/Share20/Manager.php
now Line 752:

//if ($share->getMailSend()) {
if (true) {

@LM-vb
Copy link

LM-vb commented Nov 7, 2024

Can someone else test it using 30.0.2 rc2?

I intended to do the upgrade to 30.0.2 once it is released, but if this major function is still broken I will postpone the upgrade.

@himpierre
Copy link
Author

himpierre commented Nov 7, 2024 via email

@tigernero79
Copy link

I updated to 30.0.2 final and still I only receive notifications if I rename files or folders, and nothing more

if I try to upload or delete some files or folders I don't receive any notification via email

the bug is still there

@LM-vb
Copy link

LM-vb commented Nov 7, 2024

I agreee with @tigernero79. I, too, only get an email notification when renaming a file, not if a new file gets uploaded into a shared folder. This used to work before (<= NC 29). However, the issue of the bug addressed in this bug report #48012 has been fixed.

@tigernero79: I think you should open a new bug report addressing the new issue.

@tigernero79
Copy link

#49133

ticket open

thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.