OKAPI-1081: Reject invalid tenant ids

okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java

@@ -38,6 +38,7 @@
 import org.folio.okapi.util.OkapiError;
 import org.folio.okapi.util.ProxyContext;
 import org.folio.okapi.util.TenantInstallOptions;
+import org.folio.okapi.util.TenantValidator;
 
 /**
  * Okapi's built-in module. Managing /_/ endpoints.
@@ -750,22 +751,22 @@ private String location(ProxyContext pc, String id, String baseUri, String s) {
   private Future<String> createTenant(ProxyContext pc, String body) {
     try {
       final TenantDescriptor td = JsonDecoder.decode(body, TenantDescriptor.class);
-      if (td.getId() == null || td.getId().isEmpty()) {
-        td.setId(UUID.randomUUID().toString());
-      }
-      final String tenantId = td.getId();
-      if (!tenantId.matches("^[a-z0-9_-]+$")) {
-        return Future.failedFuture(
-            new OkapiError(ErrorType.USER, messages.getMessage("11601", tenantId)));
-      }
-      Tenant t = new Tenant(td);
-      return tenantManager.insert(t).map(res ->
-        location(pc, tenantId, null, Json.encodePrettily(t.getDescriptor())));
+      return validateTenantId(td)
+          .compose(x -> tenantManager.insert(new Tenant(td)))
+          .map(x -> location(pc, td.getId(), null, Json.encodePrettily(td)));
     } catch (DecodeException ex) {
       return Future.failedFuture(new OkapiError(ErrorType.USER, ex.getMessage()));
     }
   }
 
+  static Future<Void> validateTenantId(TenantDescriptor td) {
+    if (td.getId() == null || td.getId().isEmpty()) {
+      td.setId(("t" + UUID.randomUUID().toString().replace("-", ""))
+          .substring(0, TenantValidator.MAX_LENGTH));
+    }
+    return TenantValidator.validate(td.getId());
+  }
+
   private Future<String> updateTenant(String tenantId, String body) {
     try {
       final TenantDescriptor td = JsonDecoder.decode(body, TenantDescriptor.class);

okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java

@@ -0,0 +1,56 @@
+package org.folio.okapi.util;
+
+import io.vertx.core.Future;
+import java.util.regex.Pattern;
+import org.folio.okapi.common.ErrorType;
+import org.folio.okapi.common.Messages;
+
+/**
+ * Validate a tenant ID to match ^[a-z][a-z0-9]{0,30}$ as required by
+ * <a href="https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions">
+ * https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions</a>
+ * Technical Council decision.
+ */
+public final class TenantValidator {
+  /** Maximum length of a tenant ID. */
+  public static final int MAX_LENGTH = 31;
+
+  // multi-byte sequences forbidden in pattern, so char length = byte length
+  @SuppressWarnings("java:S5867")  // we want to forbid Unicode characters, therefore we
+  // suppress warning "Unicode-aware versions of character classes should be preferred"
+  private static final String TENANT_PATTERN_STRING = "^[a-z][a-z0-9]{0,30}$";
+  private static final Pattern TENANT_PATTERN = Pattern.compile(TENANT_PATTERN_STRING);
+  private static final Pattern STARTS_WITH_DIGIT = Pattern.compile("^\\d");
+  private static final Messages MESSAGES = Messages.getInstance();
+
+  private TenantValidator() {
+    throw new UnsupportedOperationException("Cannot instantiate utility class.");
+  }
+
+  /**
+   * Validate tenantId against ^[a-z][a-z0-9]{0,30}$ and return a failed Future with
+   * clear violation explanation message on validation failure.
+   */
+  public static Future<Void> validate(String tenantId) {
+    if (TENANT_PATTERN.matcher(tenantId).matches()) {
+      return Future.succeededFuture();
+    }
+
+    String message;
+
+    if (tenantId.contains("_")) {
+      message = "11609";
+    } else if (tenantId.contains("-")) {
+      message = "11610";
+    } else if (tenantId.length() > MAX_LENGTH) {
+      message = "11611";
+    } else if (STARTS_WITH_DIGIT.matcher(tenantId).matches()) {
+      message = "11612";
+    } else {
+      message = "11601";
+    }
+
+    return Future.failedFuture(new OkapiError(ErrorType.USER,
+        MESSAGES.getMessage(message, tenantId)));
+  }
+}

okapi-core/src/main/raml/TenantDescriptor.json

@@ -7,7 +7,7 @@
   "additionalProperties" : false,
   "properties": {
     "id": {
-      "description": "Tenant ID",
+      "description": "Tenant ID. A new tenant ID created via the POST /_/proxy/tenants API must match the ^[a-z][a-z0-9]{0,30}$ regexp as required by https://folio-org.atlassian.net/wiki/spaces/TC/pages/5053983/DR-000002+-+Tenant+Id+and+Module+Name+Restrictions Technical Council decision. All other APIs also accept a legacy tenant ID that matches ^[a-z0-9_-]+$ regexp and may pose security issues as explained on https://folio-org.atlassian.net/wiki/spaces/DD/pages/1779867/Tenant+Id+and+Module+Name+Restrictions",
       "type": "string"
     },
     "name": {

okapi-core/src/main/resources/infra-messages/Messages_en.properties

@@ -107,12 +107,15 @@
 
 #InternalModule
 11600=Error in encoding location id {0}. {1}
-11601=Invalid tenant id {0}
+11601=Invalid tenant id, may only contain a-z and 0-9 and must match '[a-z][a-z0-9]{0,30}' but it is {0}
 11602=Tenant.id={0} id={1}
 11603=Can not delete the superTenant {0}
 11604=unknown orderBy field: {0}
 11605=invalid order value: {0}
 11606=Module.id={0} id={1}
 11607=Unhandled internal module path={0}
 11608=Bad format for parameter {0}. {1}
-
+11609=Tenant id must not contain underscore: {0}
+11610=Tenant id must not contain minus: {0}
+11611=Tenant id must not exceed 31 characters: {0}
+11612=Tenant id must not start with a digit: {0}

okapi-core/src/test/java/org/folio/okapi/ProxyTest.java

@@ -3377,7 +3377,7 @@ public void testTenantFailedUpgrade(TestContext context) {
 
   @Test
   public void testProxyClientFailure(TestContext context) {
-    String tenant = "test-tenant-permissions-tenant";
+    String tenant = "testtenantpermissionstenant";
     setupBasicTenant(tenant);
 
     String moduleId = "module-1.0.0";
@@ -3415,7 +3415,7 @@ public void testProxyClientFailure(TestContext context) {
 
   @Test
   public void testTenantPermissionsUpgrade() {
-    String tenant = "test-tenant-permissions-tenant";
+    String tenant = "testtenantpermissionstenant";
     setupBasicTenant(tenant);
 
     String moduleA0 = "moduleA-1.0.0";
@@ -3465,7 +3465,7 @@ private boolean hasPermReplaces(JsonArray permissions) {
 
   @Test
   public void testTenantPermissionsVersion() {
-    String tenant = "test-tenant-permissions-tenant";
+    String tenant = "testtenantpermissionstenant";
     String moduleId = "test-tenant-permissions-basic-module-1.0.0";
     String authModuleId = "test-tenant-permissions-auth-module-1.0.0";
     String body = new JsonObject().put("id", "test").encode();
@@ -3522,7 +3522,7 @@ public void testTenantPermissionsVersion() {
 
   @Test
   public void testDelegateCORS() {
-    String tenant = "test-tenant-delegate-cors";
+    String tenant = "testtenantdelegatecors";
     String moduleId = "test-tenant-delegate-cors-module-1.0.0";
     String authModuleId = "test-tenant-delegate-cors-auth-module-1.0.0";
     String body = new JsonObject().put("id", "test").encode();

okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java

@@ -8,11 +8,13 @@
 import io.vertx.junit5.VertxExtension;
 import io.vertx.junit5.VertxTestContext;
 import org.assertj.core.api.WithAssertions;
+import org.folio.okapi.bean.TenantDescriptor;
 import org.folio.okapi.util.ProxyContext;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @Timeout(5)
@@ -104,4 +106,13 @@ void invalidJson(HttpMethod httpMethod, String path, VertxTestContext vtc) {
       vtc.completeNow();
     }));
   }
+
+  @ParameterizedTest
+  @NullAndEmptySource
+  void validateTenantId(String tenantId) {
+    var td = new TenantDescriptor(tenantId, "foo");
+    assertThat(InternalModule.validateTenantId(td).succeeded()).isTrue();
+    assertThat(td.getId()).matches("t[0-9a-f]{30}");
+    assertThat(td.getName()).isEqualTo("foo");
+  }
 }

okapi-core/src/test/java/org/folio/okapi/util/TenantValidatorTest.java

@@ -0,0 +1,36 @@
+package org.folio.okapi.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.folio.okapi.testing.UtilityClassTester;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+class TenantValidatorTest {
+
+  @Test
+  void utilityClass() {
+    UtilityClassTester.assertUtilityClass(TenantValidator.class);
+  }
+
+  @ParameterizedTest
+  @CsvSource({
+    "a, ",
+    "a1, ",
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ",
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, Tenant id must not exceed 31 characters: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+    "1, Tenant id must not start with a digit: 1",
+    "foo_bar, Tenant id must not contain underscore: foo_bar",
+    "a-z, Tenant id must not contain minus: a-z",
+    "universität, 'Invalid tenant id, may only contain a-z and 0-9 and must match [a-z][a-z0-9]{0,30} but it is universität'",
+  })
+  void validate(String tenantId, String errorMessage) {
+    var result = TenantValidator.validate(tenantId);
+    if (errorMessage == null) {
+      assertThat(result.succeeded()).isTrue();
+    } else {
+      assertThat(result.cause().getMessage()).isEqualTo(errorMessage);
+    }
+  }
+}