From 5f836b422b91a156fc133f95dc96a8fd20af7e5a Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Wed, 12 Aug 2026 22:58:56 -0700 Subject: [PATCH] test(amber): cover the mail resource's address guard and admin fan-out GmailResource sat at 31.2% of 77 lines in isolation. Despite the name it is a javax.mail/SMTP resource, not a Google-OAuth one: there is no GoogleCredential, no token exchange, no clientId, and the only credential is an SMTP app password consumed inside an Authenticator callback that never fires unless a socket opens. So the parts worth testing were never credential-gated. Adds 4 tests, taking it to 61.0%. The recipient-format guard is pinned clause by clause, including the null short-circuit and the TLD floor, with a companion test asserting a well-formed address fails later in delivery so the malformed list cannot pass under a reject-everything regex. The admin fan-out is driven with two admins and a REGULAR decoy, so narrowing the role filter or notifying only the first admin both fail. No socket is opened. Two unreachable catch arms and the applicant acknowledgement are recorded as unpinnable rather than cemented, as is the fact that only the notification's recipient is pinned and not its payload. No production file is touched. --- .../web/resource/GmailResourceSpec.scala | 185 +++++++++++++++++- 1 file changed, 177 insertions(+), 8 deletions(-) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala index ec694f694f2..bcdb72de8d8 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala @@ -19,15 +19,31 @@ package org.apache.texera.web.resource +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.classic.{Level, Logger => LogbackLogger} +import ch.qos.logback.core.read.ListAppender import org.apache.texera.auth.SessionUser import org.apache.texera.common.config.UserSystemConfig +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.Tables.USER import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum +import org.apache.texera.dao.jooq.generated.tables.daos.UserDao import org.apache.texera.dao.jooq.generated.tables.pojos.User import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} +import org.slf4j.LoggerFactory +import java.util.UUID import javax.ws.rs.{BadRequestException, ForbiddenException, WebApplicationException} +import scala.jdk.CollectionConverters._ -class GmailResourceSpec extends AnyFlatSpec { +class GmailResourceSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with BeforeAndAfterEach + with MockTexeraDB { private def newSessionUser(): SessionUser = { val user = new User @@ -114,13 +130,166 @@ class GmailResourceSpec extends AnyFlatSpec { assert(ex.getResponse.getStatus == 502) } - "notifyUnauthorizedUser" should "reject a malformed receiver with HTTP 403 before any DB access" in { - // The format guard is the first statement, ahead of the admin lookup, so this needs no - // database: `new GmailResource()` touches none either, since the companion's context/userDao - // are defs and senderGmail is lazy. Remove the guard and `userDao.fetchByRole` throws something - // else — NoSuchElementException on a virgin JVM, or a jOOQ DataAccessException once another - // MockTexeraDB suite has initialised and closed the shared SqlServer — and either way - // `intercept[ForbiddenException]` rejects it. + // ─── isValidEmail, observed through the public sendEmail ──────────────────── + + "sendEmail" should "reject malformed recipient addresses before attempting delivery" in { + requireNoRealGmailSender() + val msg = EmailMessage(receiver = "unused@example.com", subject = "subj", content = "body") + val malformed = Seq( + null, // the `email != null` short-circuit: the regex would NPE on its own + "plainaddress", // no '@' at all + "@example.com", // empty local part + "user name@example.com", // space is outside the local-part character class + "user@localhost", // domain carries no dot, so there is no suffix to match + "a@b.c" // one-letter suffix, below the {2,} floor + ) + malformed.foreach { address => + withClue(s"address '$address': ") { + GmailResource.sendEmail(msg, address) shouldBe Left("Invalid email format") + } + } + } + + it should "get a well-formed address past the format guard and fail later in delivery" in { + requireNoRealGmailSender() + // Distinguishes the two Left kinds: this one comes from the Try around createMimeMessage + // (empty sender ⇒ AddressException), i.e. the guard let the address through. Without it the + // test above could stay green under a regex that rejects everything. + val msg = EmailMessage(receiver = "unused@example.com", subject = "subj", content = "body") + GmailResource.sendEmail(msg, "ok@example.com") match { + case Left(error) => error should startWith("Failed to send email:") + case Right(_) => + fail("delivery unexpectedly succeeded — this suite must never reach a real SMTP server") + } + } + + // ─── notifyUnauthorizedUser ───────────────────────────────────────────────── + + // Random suffix keeps this suite's rows from colliding with data other suites may + // have left in the shared (singleton) embedded DB; suites run sequentially. + private val runId = UUID.randomUUID().toString.substring(0, 8) + private var userDao: UserDao = _ + + override protected def beforeAll(): Unit = initializeDBAndReplaceDSLContext() + + override protected def afterAll(): Unit = shutdownDB() + + override protected def beforeEach(): Unit = { + userDao = new UserDao(getDSLContext.configuration()) + cleanup() + } + + override protected def afterEach(): Unit = cleanup() + + // startsWith escapes SQL LIKE wildcards, so the literal "gmailspec_" prefix is matched exactly. + private def cleanup(): Unit = + getDSLContext.deleteFrom(USER).where(USER.NAME.startsWith("gmailspec_")).execute() + + /** + * A value the `email` column happily stores but `isValidEmail` rejects. Seeding admins with such + * values is what makes the fan-out observable: `notifyUnauthorizedUser` discards sendEmail's + * Either, and sendEmail's only externally visible act for a rejected address is the warning it + * logs naming that address. It carries no '@' rather than a merely odd domain, so tightening or + * loosening the regex's domain rules cannot quietly turn these into deliverable addresses and + * make the fan-out invisible again. + */ + private def unroutable(tag: String): String = s"gmailspec-$tag-$runId-no-at-sign" + + private def seedUser(tag: String, role: UserRoleEnum, email: String): String = { + val user = new User + user.setName(s"gmailspec_${tag}_$runId") + user.setEmail(email) + user.setRole(role) + userDao.insert(user) + email + } + + private def adminEmailsInDb: Set[String] = + userDao.fetchByRole(UserRoleEnum.ADMIN).asScala.map(_.getEmail).toSet + + private val attemptedRecipient = """invalid address: (.+)$""".r.unanchored + + /** + * Runs `body` with a capturing appender on the logger `GmailResource.sendEmail` writes to (the + * companion object's own class), and returns the addresses it reported as unsendable. + */ + private def attemptedRecipientsDuring(body: => Unit): Seq[String] = { + val logger = LoggerFactory.getLogger(GmailResource.getClass).asInstanceOf[LogbackLogger] + val appender = new ListAppender[ILoggingEvent] + appender.start() + val previousLevel = logger.getLevel + logger.setLevel(Level.WARN) + logger.addAppender(appender) + try body + finally { + logger.detachAppender(appender) + appender.stop() + logger.setLevel(previousLevel) + } + appender.list.asScala.toSeq.map(_.getFormattedMessage).collect { + case attemptedRecipient(address) => address + } + } + + /* + * Two things in notifyUnauthorizedUser are deliberately left unasserted here. + * + * Both `catch { case ex: Exception => logger.warn(...) }` arms are unreachable: sendEmail wraps + * everything in a Try and returns a Left, it never throws. Do not contort a test into reaching + * them — delete them instead. + * + * The applicant acknowledgement that follows the fan-out has no observable effect offline. Its + * address must already be well-formed to get past the ForbiddenException guard, so sendEmail + * logs nothing for it, and the Either it returns is discarded. Deleting that whole block leaves + * this suite green; pinning it needs a configured sender, which means a real SMTP connection. + * + * Nor is the admin notification's PAYLOAD pinned, only its recipient. Replacing the whole + * `userRegistrationNotification(...)` construction with the incoming request leaves this suite + * green: sendEmail lives on the companion object, so there is no seam to intercept what it was + * handed, and the only offline observable is which address it rejected. The `affiliation` and + * `reason` values in the fixture below are therefore scene-setting, not assertions — the builder + * itself, including its toAdmin branch, is already pinned by EmailTemplateSpec, which is where + * that contract belongs. + */ + "notifyUnauthorizedUser" should "attempt one notification per ADMIN row and none for other roles" in { + requireNoRealGmailSender() + val adminA = seedUser("admina", UserRoleEnum.ADMIN, unroutable("admin-a")) + val adminB = seedUser("adminb", UserRoleEnum.ADMIN, unroutable("admin-b")) + // a non-admin whose address would show up just as loudly if the role filter were dropped + seedUser("regular", UserRoleEnum.REGULAR, unroutable("regular")) + adminEmailsInDb shouldBe Set(adminA, adminB) + + val msg = EmailMessage( + receiver = s"gmailspec-applicant-$runId@example.com", + subject = "subj", + content = "body", + affiliation = Some("UCI"), + reason = Some("research") + ) + val attempted = attemptedRecipientsDuring(new GmailResource().notifyUnauthorizedUser(msg)) + + // one attempt per admin (size pins the iteration, the set pins who) — the applicant's own + // acknowledgement leaves no trace here because its address is well-formed by construction + attempted should have size 2 + attempted.toSet shouldBe Set(adminA, adminB) + } + + it should "complete without error when no administrator is registered" in { + requireNoRealGmailSender() + adminEmailsInDb shouldBe empty + + val msg = EmailMessage( + receiver = s"gmailspec-applicant-$runId@example.com", + subject = "subj", + content = "body" + ) + noException should be thrownBy new GmailResource().notifyUnauthorizedUser(msg) + } + + it should "reject a malformed receiver with HTTP 403 before any DB access" in { + // The format guard is the first statement, ahead of the admin lookup. Drop it and the method + // runs to completion instead of throwing — the admin fan-out finds nothing and the applicant + // acknowledgement returns a Left that the method discards — so `intercept` fails. val resource = new GmailResource() val msg = EmailMessage( receiver = "not-a-valid-email",