mengw15 opened a new issue, #5161:
URL: https://github.com/apache/texera/issues/5161

   ### What happened?
   
   A three-layer false positive: when SMTP delivery fails, the UI still shows a 
green "Email sent successfully" notification.
   
   **Layer 1 — config defaults are empty** 
([`user-system.conf:31-37`](https://github.com/apache/texera/blob/a820f6727/common/config/src/main/resources/user-system.conf#L31-L37)):
   
   ```hocon
   smtp {
     gmail = ""
     gmail = ${?USER_SYS_GOOGLE_SMTP_GMAIL}
     password = ""
     password = ${?USER_SYS_GOOGLE_SMTP_PASSWORD}
   }
   ```
   
   In any deployment that doesn't set `USER_SYS_GOOGLE_SMTP_GMAIL` / 
`USER_SYS_GOOGLE_SMTP_PASSWORD` (default for local dev and many self-hosted 
setups), Texera tries to authenticate against `smtp.gmail.com:465` with empty 
credentials. Gmail rejects with `535-5.7.8 Username and Password not accepted`.
   
   **Layer 2 — backend drops the error result** 
([`GmailResource.scala:144-150`](https://github.com/apache/texera/blob/a820f6727/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala#L144-L150)):
   
   ```scala
   @PUT
   @Path("/send")
   def sendEmailRequest(emailMessage: EmailMessage, @Auth user: SessionUser): 
Unit = {
     val recipientEmail = if (emailMessage.receiver.isEmpty) user.getEmail else 
emailMessage.receiver
     sendEmail(emailMessage, recipientEmail)   // ← returns Either[String, 
Unit], discarded
   }
   ```
   
   `sendEmail` returns `Either[String, Unit]` with `Left("Failed to send email: 
...")` on `Transport.send` failure. `sendEmailRequest` ignores the return value 
and is itself typed `Unit`, so JAX-RS always responds **204 No Content** 
regardless of outcome. The error is logged, but the HTTP layer cannot tell 
success from failure.
   
   **Layer 3 — frontend assumes 2xx == success** 
([`gmail.service.ts:38-49`](https://github.com/apache/texera/blob/a820f6727/frontend/src/app/common/service/gmail/gmail.service.ts#L38-L49)):
   
   ```typescript
   public sendEmail(subject, content, receiver = "") {
     this.http.put(`${API}/gmail/send`, { receiver, subject, content 
}).subscribe({
       next: () => this.notificationService.success("Email sent successfully"), 
 // ← any 2xx triggers this
       error: ...                                                               
  // ← never fires
     });
   }
   ```
   
   The `error:` branch is correctly wired, but Layer 2 guarantees it never 
fires.
   
   **User-visible symptom:** Sharing a workflow/dataset/project via the share 
dialog shows "Email sent successfully" — the recipient never receives the 
email. No frontend logs; only visible by tailing the amber backend log for 
`Failed to send email`.
   
   **Fix:** The root cause is Layer 2. Translate `Left` into an HTTP error so 
the frontend's existing `error:` branch fires:
   
   ```scala
   sendEmail(emailMessage, recipientEmail) match {
     case Right(_) => ()
     case Left(error) =>
       throw new WebApplicationException(error, Response.Status.BAD_GATEWAY)  
// 502
   }
   ```
   
   Layer 1 (empty defaults) is not itself a bug — dev deployments commonly run 
without SMTP. Layer 3 (frontend) is correct in principle; once Layer 2 surfaces 
errors, the existing error handler will trigger.
   
   Same anti-pattern as #4725 (`DatasetFileDocument.get_presigned_url silently 
returns None`) — silent swallowing of an inner `Either`/`Option` failure mode 
by an outer caller that doesn't check the return value. Worth auditing the 
codebase for other instances.
   
   ### How to reproduce?
   
   1. Don't set `USER_SYS_GOOGLE_SMTP_GMAIL` and 
`USER_SYS_GOOGLE_SMTP_PASSWORD` (default).
   2. Start amber backend + frontend.
   3. Open the share dialog on any workflow/dataset/project, enter a real 
email, submit.
   4. Frontend: green notification *"Email sent successfully"*.
   5. Backend log: `Failed to send email: 535-5.7.8 Username and Password not 
accepted...` (or similar — depends on SMTP server).
   6. Recipient: no email arrives.
   
   A regression test (asserting `sendEmailRequest` returns a non-2xx HTTP 
status when `sendEmail` returns `Left`) is best added alongside the fix — it 
requires a JAX-RS controller test harness with `sendEmail` mocked, which the 
existing `GmailResource` does not yet have.
   
   ### Version
   
   1.1.0-incubating (Pre-release/Master)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to