Allow re-sending verification mail. Never verify expired mail

5 files changed, 102 insertions, 21 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
index 76f786a3e9b..4807c1b59ab 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
@@ -58,6 +58,7 @@ enum PathGroup {
                "/application/v4/tenant/{tenant}/info/profile",
                "/application/v4/tenant/{tenant}/info/billing",
                "/application/v4/tenant/{tenant}/info/contacts",
+               "/application/v4/tenant/{tenant}/info/resend-mail-verification",
                "/application/v4/tenant/{tenant}/notifications",
                "/routing/v1/status/tenant/{tenant}/{*}"),
 
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<PendingMailVerification> 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<TenantContacts.Audience> 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