nextcloud / nextcloud/server

occ user:add reports "Welcome email sent" when the mail failed; occ user:welcome exits 0

Open
#64,151 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

Bug description

occ user:add --email … prints Welcome email sent to <address> even when the mail was
never sent, because Mailer::send() swallows the transport failure instead of throwing.

occ user:welcome has the same root cause and is worse: it prints nothing on success, so a
failed send is indistinguishable from a successful one — it just exits 0.

Both commands do have an error branch. Neither branch can ever be reached.

Steps to reproduce
  1. Configure an SMTP account whose credentials are invalid (or otherwise make the transport fail).
  2. occ user:add --display-name "Test" --email test@example.org testuser
Expected behaviour

Unable to send the welcome email to test@example.org — the branch that already exists in
core/Command/User/Add.php.

Actual behaviour
Welcome email sent to test@example.org

and, in nextcloud.log:

Failed to authenticate on SMTP server with username "…" using the following
authenticators: "LOGIN", "PLAIN". … "535 Incorrect authentication data"

The account is created and the operator is told the invitation is on its way. Nothing in the
command output distinguishes this from a successful send.

Where the failure is lost

Three links, each individually reasonable, that together make the failure unreportable:

  1. lib/private/Mail/Mailer.phpsend() catches TransportExceptionInterface, logs it, and
    returns the failed-recipient list rather than re-throwing:

    try {
        $mailer->send($message->getSymfonyEmail());
    } catch (TransportExceptionInterface $e) {
        $this->logger->error($logMessage, ['app' => 'core', 'exception' => $e]);
        …
        return $failedRecipients;   // <- no throw
    }
    
  2. apps/settings/lib/Mailer/NewUserMailHelper.phpsendMail() is declared : void and
    discards that return value:

    public function sendMail(IUser $user, IEMailTemplate $emailTemplate): void {
        …
        $this->mailer->send($message);   // <- return value dropped
    }
    
  3. core/Command/User/Add.php — waits for an exception that can no longer arrive:

    try {
        $this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true));
        $output->writeln('Welcome email sent to ' . $email);
    } catch (\Exception $e) {
        $output->writeln('Unable to send the welcome email to ' . $email);
    }
    
  4. core/Command/User/Welcome.php — same shape, and no success output to contradict:

    try {
        $this->newUserMailHelper->sendMail($user, $emailTemplate);
    } catch (\Exception $e) {
        $output->writeln('<error>Failed to send email: ' . $e->getMessage() . '</error>');
        return 1;
    }
    return 0;
    

    A script that checks the exit code of occ user:welcome is therefore told the mail was
    sent, in every case where the transport fails.

Why this matters beyond the command output

A silent failure would send an administrator looking. A message asserting success stops the
search: the account is created, the invitation is believed sent, and the person waits for a mail
that will never arrive. On an instance where mail is the only channel that lets a user set their
own password, this is the difference between "mail is down" and "mail is down and nobody knows".

The same swallowed return value means NewUserMailHelper::sendMail() cannot report failure to
any of its callers, not only this command.

Suggested fix

Either of these would restore the existing error branch; the first is the smaller change:

  • have NewUserMailHelper::sendMail() return the failed-recipient list from
    IMailer::send() (or throw when it is non-empty), and have Add.php and Welcome.php
    check it; or
  • have Mailer::send() re-throw after logging, since every caller that wants to tolerate a
    failure can already catch it.
Server configuration
  • Nextcloud: 34.0.3 (nextcloud:34-apache)
  • Reproduced with an SMTP account returning 535 Incorrect authentication data

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/private/Mail/Mailer.php and apps/settings/lib/Mailer/NewUserMailHelper.php, then trace the existing error branches in core/Command/User/Add.php and core/Command/User/Welcome.php. Reproduce a failed SMTP send and verify both commands distinguish successful delivery from failure, including the expected error output and nonzero exit status for user:welcome.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend, cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.