add some fields validation for tenant info

2 files changed, 67 insertions, 11 deletions

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 bcec7ca798d..6210241cc31 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
@@ -125,8 +125,10 @@ import javax.ws.rs.NotAuthorizedException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URL;
 import java.security.DigestInputStream;
 import java.security.Principal;
 import java.security.PublicKey;
@@ -501,7 +503,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
     }
 
     private String getString(Inspector field, String defaultVale) {
-        return field.valid() ? field.asString() : defaultVale;
+        return field.valid() ? field.asString().trim() : defaultVale;
     }
 
     private SlimeJsonResponse updateTenantInfo(CloudTenant tenant, HttpRequest request) {
@@ -521,15 +523,22 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
         // Assert that we have a valid tenant info
         if (mergedInfo.contactName().isBlank()) {
-            return new MessageResponse(400, "'contactName' cannot be empty");
+            throw new IllegalArgumentException("'contactName' cannot be empty");
         }
         if (mergedInfo.contactEmail().isBlank()) {
-            return new MessageResponse(400, "'contactEmail' cannot be empty");
+            throw new IllegalArgumentException("'contactEmail' cannot be empty");
         }
         if (! mergedInfo.contactEmail().contains("@")) {
             // email address validation is notoriously hard - we should probably just try to send a
             // verification email to this address.  checking for @ is a simple best-effort.
-            return new MessageResponse(400, "'contactEmail' needs to be an email address");
+            throw new IllegalArgumentException("'contactEmail' needs to be an email address");
+        }
+        if (! mergedInfo.website().isBlank()) {
+            try {
+                new URL(mergedInfo.website());
+            } catch (MalformedURLException e) {
+                throw new IllegalArgumentException("'website' needs to be a valid address");
+            }
         }
 
         // Store changes
@@ -543,19 +552,39 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
     private TenantInfoAddress updateTenantInfoAddress(Inspector insp, TenantInfoAddress oldAddress) {
         if (!insp.valid()) return oldAddress;
-        return TenantInfoAddress.EMPTY
+        TenantInfoAddress address = TenantInfoAddress.EMPTY
                 .withCountry(getString(insp.field("country"), oldAddress.country()))
                 .withStateRegionProvince(getString(insp.field("stateRegionProvince"), oldAddress.stateRegionProvince()))
                 .withCity(getString(insp.field("city"), oldAddress.city()))
                 .withPostalCodeOrZip(getString(insp.field("postalCodeOrZip"), oldAddress.postalCodeOrZip()))
                 .withAddressLines(getString(insp.field("addressLines"), oldAddress.addressLines()));
+
+        List<String> fields = List.of(address.addressLines(),
+                        address.postalCodeOrZip(),
+                        address.country(),
+                        address.city(),
+                        address.stateRegionProvince());
+
+        if (fields.stream().allMatch(String::isBlank) || fields.stream().noneMatch(String::isBlank))
+            return address;
+
+        throw new IllegalArgumentException("All address fields must be set");
     }
 
     private TenantInfoBillingContact updateTenantInfoBillingContact(Inspector insp, TenantInfoBillingContact oldContact) {
         if (!insp.valid()) return oldContact;
+
+        String email = getString(insp.field("email"), oldContact.email());
+
+        if (!email.isBlank() && !email.contains("@")) {
+            // email address validation is notoriously hard - we should probably just try to send a
+            // verification email to this address.  checking for @ is a simple best-effort.
+            throw new IllegalArgumentException("'email' needs to be an email address");
+        }
+
         return TenantInfoBillingContact.EMPTY
                 .withName(getString(insp.field("name"), oldContact.name()))
-                .withEmail(getString(insp.field("email"), oldContact.email()))
+                .withEmail(email)
                 .withPhone(getString(insp.field("phone"), oldContact.phone()))
                 .withAddress(updateTenantInfoAddress(insp.field("address"), oldContact.address()));
     }
@@ -1834,6 +1863,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
     /** Trigger deployment of the given Vespa version if a valid one is given, e.g., "7.8.9". */
     private HttpResponse deployPlatform(String tenantName, String applicationName, String instanceName, boolean pin, HttpRequest request) {
+
         String versionString = readToString(request.getData());
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
         StringBuilder response = new StringBuilder();
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 63881e4eb7b..fa1483fd90f 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
@@ -100,8 +100,8 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
         tester.assertResponse(infoRequest, "{\"name\":\"\",\"email\":\"\",\"website\":\"\",\"invoiceEmail\":\"\",\"contactName\":\"newName\",\"contactEmail\":\"foo@example.com\",\"billingContact\":{\"name\":\"billingName\",\"email\":\"\",\"phone\":\"\"}}", 200);
 
         String fullAddress = "{\"addressLines\":\"addressLines\",\"postalCodeOrZip\":\"postalCodeOrZip\",\"city\":\"city\",\"stateRegionProvince\":\"stateRegionProvince\",\"country\":\"country\"}";
-        String fullBillingContact = "{\"name\":\"name\",\"email\":\"email\",\"phone\":\"phone\",\"address\":" + fullAddress + "}";
-        String fullInfo = "{\"name\":\"name\",\"email\":\"foo@example\",\"website\":\"webSite\",\"invoiceEmail\":\"invoiceEmail\",\"contactName\":\"contactName\",\"contactEmail\":\"contact@example.com\",\"address\":" + fullAddress + ",\"billingContact\":" + fullBillingContact + "}";
+        String fullBillingContact = "{\"name\":\"name\",\"email\":\"foo@example\",\"phone\":\"phone\",\"address\":" + fullAddress + "}";
+        String fullInfo = "{\"name\":\"name\",\"email\":\"foo@example\",\"website\":\"https://yahoo.com\",\"invoiceEmail\":\"invoiceEmail\",\"contactName\":\"contactName\",\"contactEmail\":\"contact@example.com\",\"address\":" + fullAddress + ",\"billingContact\":" + fullBillingContact + "}";
 
         // Now set all fields
         var postFull =
@@ -127,20 +127,46 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
         var missingNameResponse = request("/application/v4/tenant/scoober/info", PUT)
                 .data(partialInfoMissingName)
                 .roles(Set.of(Role.administrator(tenantName)));
-        tester.assertResponse(missingNameResponse, "{\"message\":\"'contactName' cannot be empty\"}", 400);
+        tester.assertResponse(missingNameResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'contactName' cannot be empty\"}", 400);
 
         // email needs to be present, not blank, and contain an @
         var partialInfoMissingEmail = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \" \"}";
         var missingEmailResponse = request("/application/v4/tenant/scoober/info", PUT)
                 .data(partialInfoMissingEmail)
                 .roles(Set.of(Role.administrator(tenantName)));
-        tester.assertResponse(missingEmailResponse, "{\"message\":\"'contactEmail' cannot be empty\"}", 400);
+        tester.assertResponse(missingEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'contactEmail' cannot be empty\"}", 400);
 
         var partialInfoBadEmail = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \"somethingweird\"}";
         var badEmailResponse = request("/application/v4/tenant/scoober/info", PUT)
                 .data(partialInfoBadEmail)
                 .roles(Set.of(Role.administrator(tenantName)));
-        tester.assertResponse(badEmailResponse, "{\"message\":\"'contactEmail' needs to be an email address\"}", 400);
+        tester.assertResponse(badEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'contactEmail' needs to be an email address\"}", 400);
+
+        var invalidWebsite = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \"email@scoober.com\", \"website\": \"scoober\" }";
+        var badWebsiteResponse = request("/application/v4/tenant/scoober/info", PUT)
+                .data(invalidWebsite)
+                .roles(Set.of(Role.administrator(tenantName)));
+        tester.assertResponse(badWebsiteResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'website' needs to be a valid address\"}", 400);
+
+        // If any of the address field is set, all fields in address need to be present
+        var addressInfo = "{\n" +
+                "  \"name\": \"Vespa User\",\n" +
+                "  \"email\": \"user@yahooinc.com\",\n" +
+                "  \"website\": \"\",\n" +
+                "  \"contactName\": \"Vespa User\",\n" +
+                "  \"contactEmail\": \"user@yahooinc.com\",\n" +
+                "  \"address\": {\n" +
+                "    \"addressLines\": \"\",\n" +
+                "    \"postalCodeOrZip\": \"7018\",\n" +
+                "    \"city\": \"\",\n" +
+                "    \"stateRegionProvince\": \"\",\n" +
+                "    \"country\": \"\"\n" +
+                "  }\n" +
+                "}";
+        var addressInfoResponse = request("/application/v4/tenant/scoober/info", PUT)
+                .data(addressInfo)
+                .roles(Set.of(Role.administrator(tenantName)));
+        tester.assertResponse(addressInfoResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"All address fields must be set\"}", 400);
 
         // updating a tenant that already has the fields set works
         var basicInfo = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \"foo@example.com\"}";