From 7df1fd635ba6bb1a713eef150cd3a635906e74e4 Mon Sep 17 00:00:00 2001 From: Ola Aunronning Date: Tue, 25 Oct 2022 14:53:58 +0200 Subject: Allow re-sending verification mail. Never verify expired mail --- .../vespa/hosted/controller/MailVerifier.java | 60 +++++++++++++++------- .../restapi/application/ApplicationApiHandler.java | 16 ++++++ .../vespa/hosted/controller/MailVerifierTest.java | 32 +++++++++++- .../application/ApplicationApiCloudTest.java | 14 +++++ 4 files changed, 101 insertions(+), 21 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java index 46cdf8d63d7..7c9c92c1e3a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import java.time.Clock; import java.time.Duration; +import java.util.Optional; import java.util.UUID; @@ -42,26 +43,47 @@ public class MailVerifier { return pendingMailVerification; } + public Optional resendMailVerification(TenantName tenantName, String email, PendingMailVerification.MailType mailType) { + var oldPendingVerification = curatorDb.listPendingMailVerifications() + .stream() + .filter(pendingMailVerification -> + pendingMailVerification.getMailAddress().equals(email) && + pendingMailVerification.getMailType().equals(mailType) && + pendingMailVerification.getTenantName().equals(tenantName) + ).findFirst(); + + if (oldPendingVerification.isEmpty()) + return Optional.empty(); + + try (var lock = curatorDb.lockPendingMailVerification(oldPendingVerification.get().getVerificationCode())) { + curatorDb.deletePendingMailVerification(oldPendingVerification.get()); + } + + return Optional.of(sendMailVerification(tenantName, email, mailType)); + } + public boolean verifyMail(String verificationCode) { - return curatorDb.getPendingMailVerification(verificationCode).map(pendingMailVerification -> { - var tenant = requireCloudTenant(pendingMailVerification.getTenantName()); - var oldTenantInfo = tenant.info(); - var updatedTenantInfo = switch (pendingMailVerification.getMailType()) { - case NOTIFICATIONS -> withTenantContacts(oldTenantInfo, pendingMailVerification); - case TENANT_CONTACT -> oldTenantInfo.withContact(oldTenantInfo.contact() - .withEmail(oldTenantInfo.contact().email().withVerification(true))); - }; - - tenantController.lockOrThrow(tenant.name(), LockedTenant.Cloud.class, lockedTenant -> { - lockedTenant = lockedTenant.withInfo(updatedTenantInfo); - tenantController.store(lockedTenant); - }); - - try (var lock = curatorDb.lockPendingMailVerification(pendingMailVerification.getVerificationCode())) { - curatorDb.deletePendingMailVerification(pendingMailVerification); - } - return true; - }).orElse(false); + return curatorDb.getPendingMailVerification(verificationCode) + .filter(pendingMailVerification -> pendingMailVerification.getVerificationDeadline().isAfter(clock.instant())) + .map(pendingMailVerification -> { + var tenant = requireCloudTenant(pendingMailVerification.getTenantName()); + var oldTenantInfo = tenant.info(); + var updatedTenantInfo = switch (pendingMailVerification.getMailType()) { + case NOTIFICATIONS -> withTenantContacts(oldTenantInfo, pendingMailVerification); + case TENANT_CONTACT -> oldTenantInfo.withContact(oldTenantInfo.contact() + .withEmail(oldTenantInfo.contact().email().withVerification(true))); + }; + + tenantController.lockOrThrow(tenant.name(), LockedTenant.Cloud.class, lockedTenant -> { + lockedTenant = lockedTenant.withInfo(updatedTenantInfo); + tenantController.store(lockedTenant); + }); + + try (var lock = curatorDb.lockPendingMailVerification(pendingMailVerification.getVerificationCode())) { + curatorDb.deletePendingMailVerification(pendingMailVerification); + } + return true; + }).orElse(false); } private TenantInfo withTenantContacts(TenantInfo oldInfo, PendingMailVerification pendingMailVerification) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 6c25d608f45..5f978a7d5f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -306,6 +306,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/info/profile")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoProfile); if (path.matches("/application/v4/tenant/{tenant}/info/billing")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoBilling); if (path.matches("/application/v4/tenant/{tenant}/info/contacts")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoContacts); + if (path.matches("/application/v4/tenant/{tenant}/info/resend-mail-verification")) return withCloudTenant(path.get("tenant"), request, this::resendEmailVerification); if (path.matches("/application/v4/tenant/{tenant}/archive-access")) return allowAwsArchiveAccess(path.get("tenant"), request); // TODO(enygaard, 2022-05-25) Remove when no longer used by console if (path.matches("/application/v4/tenant/{tenant}/archive-access/aws")) return allowAwsArchiveAccess(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/archive-access/gcp")) return allowGcpArchiveAccess(path.get("tenant"), request); @@ -1540,6 +1541,21 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return new MessageResponse(type.jobName() + " for " + id + " resumed"); } + private SlimeJsonResponse resendEmailVerification(CloudTenant tenant, Inspector inspector) { + var mail = mandatory("mail", inspector).asString(); + var type = mandatory("mailType", inspector).asString(); + + var mailType = switch (type) { + case "contact" -> PendingMailVerification.MailType.TENANT_CONTACT; + case "notifications" -> PendingMailVerification.MailType.NOTIFICATIONS; + default -> throw new IllegalArgumentException("Unknown mail type " + type); + }; + + var pendingVerification = controller.mailVerifier().resendMailVerification(tenant.name(), mail, mailType); + return pendingVerification.isPresent() ? new MessageResponse("Re-sent verification mail to " + mail) : + ErrorResponse.notFoundError("No pending mail verification found for " + mail); + } + private void toSlime(Cursor object, Application application, HttpRequest request) { object.setString("tenant", application.id().tenant().value()); object.setString("application", application.id().application().value()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java index c488d1d084e..873ab435444 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java @@ -9,12 +9,14 @@ import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.time.Duration; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -31,8 +33,8 @@ class MailVerifierTest { private static final String mail = "unverified@bar.com"; private static final List audiences = List.of(TenantContacts.Audience.NOTIFICATIONS, TenantContacts.Audience.TENANT); - @Test - public void test_new_mail_verification() { + @BeforeEach + public void setup() { tester.createTenant(tenantName.value(), Tenant.Type.cloud); tester.controller().tenants().lockOrThrow(tenantName, LockedTenant.Cloud.class, lockedTenant -> { @@ -44,7 +46,10 @@ class MailVerifierTest { lockedTenant = lockedTenant.withInfo(lockedTenant.get().info().withContacts(new TenantContacts(contacts))); tester.controller().tenants().store(lockedTenant); }); + } + @Test + public void test_new_mail_verification() { mailVerifier.sendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); // Verify mail is sent @@ -59,7 +64,13 @@ class MailVerifierTest { assertEquals(tester.clock().instant().plus(Duration.ofDays(7)), writtenMailVerification.getVerificationDeadline()); assertEquals(mail, writtenMailVerification.getMailAddress()); + // Mail verification is no-op if deadline has passed + tester.clock().advance(Duration.ofDays(14)); + assertFalse(mailVerifier.verifyMail(writtenMailVerification.getVerificationCode())); + assertFalse(tester.curator().listPendingMailVerifications().isEmpty()); + // Mail is verified + tester.clock().retreat(Duration.ofDays(14)); mailVerifier.verifyMail(writtenMailVerification.getVerificationCode()); assertTrue(tester.curator().listPendingMailVerifications().isEmpty()); var tenant = tester.controller().tenants().require(tenantName, CloudTenant.class); @@ -71,4 +82,21 @@ class MailVerifierTest { assertEquals(expectedContacts, tenant.info().contacts().all()); } + @Test + public void resending_verification_deletes_old_one() { + var pendingMailVerification = mailVerifier.sendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); + var tenant = tester.controller().tenants().require(tenantName, CloudTenant.class); + + // Unknown mail is no-op + var resentVerification = mailVerifier.resendMailVerification(tenantName, "unknown-mail", PendingMailVerification.MailType.NOTIFICATIONS); + assertTrue(resentVerification.isEmpty()); + assertTrue(tester.curator().getPendingMailVerification(pendingMailVerification.getVerificationCode()).isPresent()); + + // Verification mail is re-sent, old data is replaced + resentVerification = mailVerifier.resendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); + assertTrue(resentVerification.isPresent()); + assertTrue(tester.curator().getPendingMailVerification(pendingMailVerification.getVerificationCode()).isEmpty()); + assertTrue(tester.curator().getPendingMailVerification(resentVerification.get().getVerificationCode()).isPresent()); + } + } \ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 7938427d7b6..f713952ad7c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -163,6 +163,20 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { // Now compare the updated info with the full info we sent tester.assertResponse(infoRequest, fullInfo, 200); + + var invalidBody = "{\"mail\":\"contact1@example.com\", \"mailType\":\"blurb\"}"; + var resendMailRequest = + request("/application/v4/tenant/scoober/info/resend-mail-verification", PUT) + .data(invalidBody) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(resendMailRequest, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unknown mail type blurb\"}", 400); + + var resendMailBody = "{\"mail\":\"contact1@example.com\", \"mailType\":\"notifications\"}"; + resendMailRequest = + request("/application/v4/tenant/scoober/info/resend-mail-verification", PUT) + .data(resendMailBody) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(resendMailRequest, "{\"message\":\"Re-sent verification mail to contact1@example.com\"}", 200); } @Test -- cgit v1.2.3