From b40a3d2dc849790c351e7077cda3faf2a24ce97a Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Wed, 21 Feb 2024 01:18:59 +0100 Subject: [PATCH 1/6] OKAPI-1081: Reject invalid tenant ids https://folio-org.atlassian.net/browse/OKAPI-1081 Implement https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions so that creating a new tenant via POST /_/proxy/tenants API is rejected unless the tenant id matches ^[a-z][a-z0-9]{0,30}$ Legacy tenant ids still work when used in any other API. --- .../folio/okapi/managers/InternalModule.java | 22 +++---- .../org/folio/okapi/util/TenantValidator.java | 58 +++++++++++++++++++ .../src/main/raml/TenantDescriptor.json | 2 +- .../infra-messages/Messages_en.properties | 7 ++- .../test/java/org/folio/okapi/ProxyTest.java | 8 +-- .../okapi/managers/InternalModuleTest.java | 11 ++++ .../folio/okapi/util/TenantValidatorTest.java | 36 ++++++++++++ 7 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java create mode 100644 okapi-core/src/test/java/org/folio/okapi/util/TenantValidatorTest.java diff --git a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java b/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java index 094a93988..d9bbaf5b3 100644 --- a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java +++ b/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,21 @@ private String location(ProxyContext pc, String id, String baseUri, String s) { private Future 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 validateTenantId(TenantDescriptor td) { + if (td.getId() == null || td.getId().isEmpty()) { + td.setId("t" + UUID.randomUUID().toString().replace("-", "").substring(2)); + } + return TenantValidator.validate(td.getId()); + } + private Future updateTenant(String tenantId, String body) { try { final TenantDescriptor td = JsonDecoder.decode(body, TenantDescriptor.class); diff --git a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java new file mode 100644 index 000000000..6e237b631 --- /dev/null +++ b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java @@ -0,0 +1,58 @@ +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 + * + * https://folio-org.atlassian.net/wiki/display/TC/ADR-000002+-+Tenant+Id+and+Module+Name+Restrictions + * Technical Council decision. + */ +public final class TenantValidator { + + // multi-byte sequences forbidden in pattern, so char length = byte length + 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("^[0-9]"); + 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 validate(String tenantId) { + if (TENANT_PATTERN.matcher(tenantId).matches()) { + return Future.succeededFuture(); + } + + if (tenantId.contains("_")) { + return failure("11609", tenantId); + } + + if (tenantId.contains("-")) { + return failure("11610", tenantId); + } + + if (tenantId.length() > 31) { + return failure("11611", tenantId); + } + + if (STARTS_WITH_DIGIT.matcher(tenantId).matches()) { + return failure("11612", tenantId); + } + + return failure("11601", tenantId); + } + + private static Future failure(String errorMessage, String tenantId) { + return Future.failedFuture(new OkapiError(ErrorType.USER, + MESSAGES.getMessage(errorMessage, tenantId))); + } +} diff --git a/okapi-core/src/main/raml/TenantDescriptor.json b/okapi-core/src/main/raml/TenantDescriptor.json index 08dcd0c06..bc3b09080 100644 --- a/okapi-core/src/main/raml/TenantDescriptor.json +++ b/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": { diff --git a/okapi-core/src/main/resources/infra-messages/Messages_en.properties b/okapi-core/src/main/resources/infra-messages/Messages_en.properties index 4829b12db..9fe83bb63 100644 --- a/okapi-core/src/main/resources/infra-messages/Messages_en.properties +++ b/okapi-core/src/main/resources/infra-messages/Messages_en.properties @@ -107,7 +107,7 @@ #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} @@ -115,4 +115,7 @@ 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} diff --git a/okapi-core/src/test/java/org/folio/okapi/ProxyTest.java b/okapi-core/src/test/java/org/folio/okapi/ProxyTest.java index 454551c9b..68338258a 100644 --- a/okapi-core/src/test/java/org/folio/okapi/ProxyTest.java +++ b/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(); diff --git a/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java b/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java index 143f3202b..f6d8049ab 100644 --- a/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java +++ b/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"); + } } diff --git a/okapi-core/src/test/java/org/folio/okapi/util/TenantValidatorTest.java b/okapi-core/src/test/java/org/folio/okapi/util/TenantValidatorTest.java new file mode 100644 index 000000000..fcdc839cc --- /dev/null +++ b/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); + } + } +} From 538cccdc9319a350ba483862645542c47ff5a82f Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Wed, 21 Feb 2024 23:42:42 +0100 Subject: [PATCH 2/6] Fix code smells --- .../folio/okapi/managers/InternalModule.java | 3 +- .../org/folio/okapi/util/TenantValidator.java | 34 ++++++++----------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java b/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java index d9bbaf5b3..da8a2fd99 100644 --- a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java +++ b/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java @@ -761,7 +761,8 @@ private Future createTenant(ProxyContext pc, String body) { static Future validateTenantId(TenantDescriptor td) { if (td.getId() == null || td.getId().isEmpty()) { - td.setId("t" + UUID.randomUUID().toString().replace("-", "").substring(2)); + td.setId(("t" + UUID.randomUUID().toString().replace("-", "")) + .substring(0, TenantValidator.MAX_LENGTH)); } return TenantValidator.validate(td.getId()); } diff --git a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java index 6e237b631..428d5b6bd 100644 --- a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java +++ b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java @@ -12,11 +12,15 @@ * 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("^[0-9]"); + private static final Pattern STARTS_WITH_DIGIT = Pattern.compile("^\\d"); private static final Messages MESSAGES = Messages.getInstance(); private TenantValidator() { @@ -32,27 +36,19 @@ public static Future validate(String tenantId) { return Future.succeededFuture(); } - if (tenantId.contains("_")) { - return failure("11609", tenantId); - } + String message = "11601"; - if (tenantId.contains("-")) { - return failure("11610", tenantId); - } - - if (tenantId.length() > 31) { - return failure("11611", tenantId); - } - - if (STARTS_WITH_DIGIT.matcher(tenantId).matches()) { - return failure("11612", tenantId); + 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"; } - return failure("11601", tenantId); - } - - private static Future failure(String errorMessage, String tenantId) { return Future.failedFuture(new OkapiError(ErrorType.USER, - MESSAGES.getMessage(errorMessage, tenantId))); + MESSAGES.getMessage(message, tenantId))); } } From be100e948e09bf287742cd727c7704802be2c1fd Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Wed, 21 Feb 2024 23:47:51 +0100 Subject: [PATCH 3/6] Add ending period to first sentence of Javadoc. --- .../src/main/java/org/folio/okapi/util/TenantValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java index 428d5b6bd..7b672246f 100644 --- a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java +++ b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java @@ -12,7 +12,7 @@ * Technical Council decision. */ public final class TenantValidator { - /** Maximum length of a tenant ID */ + /** Maximum length of a tenant ID. */ public static final int MAX_LENGTH = 31; // multi-byte sequences forbidden in pattern, so char length = byte length From 47b442c4ad90c0a589324fa8043afb874f2ceec0 Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Thu, 22 Feb 2024 02:24:34 +0100 Subject: [PATCH 4/6] Fix sonar code smell of missing else --- .../src/main/java/org/folio/okapi/util/TenantValidator.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java index 7b672246f..0a816646d 100644 --- a/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java +++ b/okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java @@ -36,7 +36,7 @@ public static Future validate(String tenantId) { return Future.succeededFuture(); } - String message = "11601"; + String message; if (tenantId.contains("_")) { message = "11609"; @@ -46,6 +46,8 @@ public static Future validate(String tenantId) { message = "11611"; } else if (STARTS_WITH_DIGIT.matcher(tenantId).matches()) { message = "11612"; + } else { + message = "11601"; } return Future.failedFuture(new OkapiError(ErrorType.USER, From 652c403e9b4d83601254946ae6a7a40ae88700c6 Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Fri, 23 Feb 2024 14:59:09 +0100 Subject: [PATCH 5/6] test that legacy tenant id with underscore still works --- .../java/org/folio/okapi/MainVerticle.java | 4 + .../infra-messages/Messages_en.properties | 2 +- .../org/folio/okapi/ModuleTenantsTest.java | 164 ++---------------- .../java/org/folio/okapi/TenantRATest.java | 121 +++++++++++-- 4 files changed, 125 insertions(+), 166 deletions(-) diff --git a/okapi-core/src/main/java/org/folio/okapi/MainVerticle.java b/okapi-core/src/main/java/org/folio/okapi/MainVerticle.java index 9d6e6d7f2..f56f73e30 100644 --- a/okapi-core/src/main/java/org/folio/okapi/MainVerticle.java +++ b/okapi-core/src/main/java/org/folio/okapi/MainVerticle.java @@ -237,6 +237,10 @@ private Future startModuleManager() { return moduleManager.init(vertx); } + TenantManager getTenantManager() { + return tenantManager; + } + private Future startTenants() { logger.info("startTenants"); return tenantManager.init(vertx); diff --git a/okapi-core/src/main/resources/infra-messages/Messages_en.properties b/okapi-core/src/main/resources/infra-messages/Messages_en.properties index 9fe83bb63..0d66901c2 100644 --- a/okapi-core/src/main/resources/infra-messages/Messages_en.properties +++ b/okapi-core/src/main/resources/infra-messages/Messages_en.properties @@ -108,7 +108,7 @@ #InternalModule 11600=Error in encoding location id {0}. {1} 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} +11602=The tenant id must not be changed, old=''{1}'', new=''{0}'' 11603=Can not delete the superTenant {0} 11604=unknown orderBy field: {0} 11605=invalid order value: {0} diff --git a/okapi-core/src/test/java/org/folio/okapi/ModuleTenantsTest.java b/okapi-core/src/test/java/org/folio/okapi/ModuleTenantsTest.java index ef0be0402..234047ee7 100644 --- a/okapi-core/src/test/java/org/folio/okapi/ModuleTenantsTest.java +++ b/okapi-core/src/test/java/org/folio/okapi/ModuleTenantsTest.java @@ -20,7 +20,11 @@ import io.vertx.ext.unit.junit.VertxUnitRunner; import org.apache.logging.log4j.Logger; import org.folio.okapi.bean.ModuleDescriptor; +import org.folio.okapi.bean.Tenant; +import org.folio.okapi.bean.TenantDescriptor; import org.folio.okapi.common.OkapiLogger; +import org.folio.okapi.managers.TenantManager; + import static org.hamcrest.Matchers.*; import org.junit.Assert; import org.junit.BeforeClass; @@ -36,6 +40,7 @@ public class ModuleTenantsTest { private static final String LS = System.lineSeparator(); private final int port = 9230; private static RamlDefinition api; + private final String okapiTenant = "ros_kilde"; @BeforeClass public static void setUpBeforeClass() throws Exception { @@ -56,7 +61,12 @@ public void setUp(TestContext context) { DeploymentOptions opt = new DeploymentOptions() .setConfig(conf); - vertx.deployVerticle(MainVerticle.class.getName(), opt, context.asyncAssertSuccess()); + var mainVerticle = new MainVerticle(); + vertx.deployVerticle(mainVerticle, opt, context.asyncAssertSuccess(x -> { + // directly create a legacy tenant id with underscore, POST /_/proxy/tenants rejects it + var tenantManager = mainVerticle.getTenantManager(); + tenantManager.insert(new Tenant(new TenantDescriptor(okapiTenant, "the name"))); + })); } @After @@ -79,7 +89,6 @@ public void tearDown(TestContext context) { @Test public void test1() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; @@ -226,24 +235,6 @@ public void test1() { Assert.assertTrue("raml: " + c.getLastReport().toString(), c.getLastReport().isEmpty()); - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - // associate tenant for module final String docEnableSampleNoSemVer = "{" + LS + " \"id\" : \"sample-module\"" + LS @@ -1067,7 +1058,6 @@ public void test1() { @Test public void test2() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; @@ -1191,24 +1181,6 @@ public void test2() { mdl = Json.decodeValue(r.asString(), ModuleDescriptor[].class); Assert.assertEquals(2, mdl.length); - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - // install with mult first c = api.createRestAssured3(); c.given() @@ -1281,29 +1253,10 @@ public void test2() { @Test public void test3() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - final String doc1 = "{" + LS + " \"id\" : \"basic-1.0.0\"," + LS + " \"name\" : \"this module\"," + LS @@ -1416,29 +1369,10 @@ public void test3() { @Test public void test4() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - final String doc1 = "{" + LS + " \"id\" : \"sample-module-1.0.0\"," + LS + " \"name\" : \"this module\"," + LS @@ -1589,7 +1523,6 @@ public void testRegisterDepCheck() { @Test public void testInstallDepCheck() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; @@ -1631,23 +1564,6 @@ public void testInstallDepCheck() { + " }" + LS + "}"; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - // register the module basic 1.0.0 c = api.createRestAssured3(); r = c.given() @@ -1739,29 +1655,10 @@ public void testInstallDepCheck() { @Test public void test641() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - final String docLevel1_1_0_0 = "{" + LS + " \"id\" : \"level1-1.0.0\"," + LS + " \"name\" : \"level1 module\"," + LS @@ -2044,29 +1941,10 @@ public void test641() { @Test public void test648() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - final String docProv_1_0_0 = "{" + LS + " \"id\" : \"prov-1.0.0\"," + LS + " \"name\" : \"prov module\"," + LS @@ -2142,29 +2020,10 @@ public void test648() { @Test public void test666() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; - // add tenant - final String docTenantRoskilde = "{" + LS - + " \"id\" : \"" + okapiTenant + "\"," + LS - + " \"name\" : \"" + okapiTenant + "\"," + LS - + " \"description\" : \"Roskilde bibliotek\"" + LS - + "}"; - c = api.createRestAssured3(); - r = c.given() - .header("Content-Type", "application/json") - .body(docTenantRoskilde).post("/_/proxy/tenants") - .then().statusCode(201) - .body(equalTo(docTenantRoskilde)) - .extract().response(); - Assert.assertTrue( - "raml: " + c.getLastReport().toString(), - c.getLastReport().isEmpty()); - final String locationTenantRoskilde = r.getHeader("Location"); - final String docProv_1_0_0 = "{" + LS + " \"id\" : \"prov-1.0.0-alpha\"," + LS + " \"name\" : \"prov module\"," + LS @@ -2236,7 +2095,6 @@ public void test666() { @Test public void testNpmRelease() { - final String okapiTenant = "roskilde"; RestAssured.port = port; RestAssuredClient c; Response r; diff --git a/okapi-core/src/test/java/org/folio/okapi/TenantRATest.java b/okapi-core/src/test/java/org/folio/okapi/TenantRATest.java index 0208f24e2..6a3a61087 100644 --- a/okapi-core/src/test/java/org/folio/okapi/TenantRATest.java +++ b/okapi-core/src/test/java/org/folio/okapi/TenantRATest.java @@ -1,24 +1,28 @@ package org.folio.okapi; -import io.vertx.core.DeploymentOptions; -import io.vertx.core.Vertx; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + import guru.nidi.ramltester.RamlDefinition; import guru.nidi.ramltester.RamlLoaders; import guru.nidi.ramltester.restassured3.RestAssuredClient; import io.restassured.RestAssured; import io.restassured.response.Response; +import io.vertx.core.DeploymentOptions; +import io.vertx.core.Vertx; import io.vertx.core.json.JsonObject; -import io.vertx.ext.unit.Async; import io.vertx.ext.unit.TestContext; import io.vertx.ext.unit.junit.VertxUnitRunner; import org.apache.logging.log4j.Logger; +import org.folio.okapi.bean.Tenant; +import org.folio.okapi.bean.TenantDescriptor; import org.folio.okapi.common.OkapiLogger; -import static org.hamcrest.Matchers.*; +import org.folio.okapi.managers.TenantManager; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Test; import org.junit.runner.RunWith; @java.lang.SuppressWarnings({"squid:S1192"}) @@ -26,9 +30,10 @@ public class TenantRATest { private final Logger logger = OkapiLogger.get(); - private int port = 9230; + private static int PORT = 9230; private Vertx vertx; + private TenantManager tenantManager; private static final String LS = System.lineSeparator(); private static RamlDefinition api; @@ -36,15 +41,19 @@ public class TenantRATest { @BeforeClass public static void setUpBeforeClass() throws Exception { api = RamlLoaders.fromFile("src/main/raml").load("okapi.raml"); + RestAssured.port = PORT; } @Before public void setUp(TestContext context) { vertx = Vertx.vertx(); + var mainVerticle = new MainVerticle(); DeploymentOptions opt = new DeploymentOptions() - .setConfig(new JsonObject().put("port", Integer.toString(port))); - vertx.deployVerticle(MainVerticle.class.getName(), opt, context.asyncAssertSuccess()); + .setConfig(new JsonObject().put("port", Integer.toString(PORT))); + vertx.deployVerticle(mainVerticle, opt, context.asyncAssertSuccess(x -> { + tenantManager = mainVerticle.getTenantManager(); + })); } @After @@ -54,8 +63,6 @@ public void tearDown(TestContext context) { @Test public void test1() { - RestAssured.port = port; - RestAssuredClient c; Response r; @@ -321,4 +328,94 @@ public void test1() { Assert.assertTrue("raml: " + c.getLastReport().toString(), c.getLastReport().isEmpty()); } + + @Test + public void rename() { + var c = api.createRestAssured3(); + c.given() + .header("Content-Type", "application/json") + .body(""" + { "id": "foo", + "name": "The foo lib", + "description": "foo and further" + } + """) + .post("/_/proxy/tenants") + .then().statusCode(201); + assertThat(c.getLastReport().toString(), c.getLastReport().isEmpty(), is(true)); + + // can change name and description + api.createRestAssured3() + .given() + .header("Content-Type", "application/json") + .body(""" + { "id": "foo", + "name": "foo & bar", + "description": "foo raising the bar" + } + """) + .put("/_/proxy/tenants/foo") + .then() + .statusCode(200) + .body("name", is("foo & bar")); + assertThat(c.getLastReport().toString(), c.getLastReport().isEmpty(), is(true)); + + var bar = """ + { "id": "bar" + } + """; + + // cannot change id from foo to bar + api.createRestAssured3() + .given() + .header("Content-Type", "application/json") + .body(bar) + .put("/_/proxy/tenants/foo") + .then().statusCode(400) + .body(is("The tenant id must not be changed, old='foo', new='bar'")); + assertThat(c.getLastReport().toString(), c.getLastReport().isEmpty(), is(true)); + + // but can create a separate bar tenant + api.createRestAssured3() + .given() + .header("Content-Type", "application/json") + .body(bar) + .post("/_/proxy/tenants") + .then().statusCode(201); + assertThat(c.getLastReport().toString(), c.getLastReport().isEmpty(), is(true)); + } + + @Test + public void legacy() { + api.createRestAssured3() + .given() + .header("Content-Type", "application/json") + .body(""" + { "id": "lib_bee" + } + """) + .post("/_/proxy/tenants") + .then().statusCode(400) + .body(is("Tenant id must not contain underscore: lib_bee")); + + // create a legacy tenant id with underscore + tenantManager.insert(new Tenant(new TenantDescriptor("lib_bee", "lib_bee"))); + + // change name + api.createRestAssured3() + .given() + .header("Content-Type", "application/json") + .body(""" + { "id": "lib_bee", + "name": "Bee the honey maker" + } + """) + .put("/_/proxy/tenants/lib_bee") + .then().statusCode(200); + + api.createRestAssured3() + .given() + .delete("/_/proxy/tenants/lib_bee") + .then().statusCode(204); + } } From fc2e8bfc5e9fc5e4a9e23572fba8584928034cc4 Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Wed, 28 Feb 2024 21:17:53 +0100 Subject: [PATCH 6/6] change validateTenantId to setDefaultId --- .../main/java/org/folio/okapi/managers/InternalModule.java | 6 +++--- .../java/org/folio/okapi/managers/InternalModuleTest.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java b/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java index da8a2fd99..0d5423777 100644 --- a/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java +++ b/okapi-core/src/main/java/org/folio/okapi/managers/InternalModule.java @@ -751,7 +751,8 @@ private String location(ProxyContext pc, String id, String baseUri, String s) { private Future createTenant(ProxyContext pc, String body) { try { final TenantDescriptor td = JsonDecoder.decode(body, TenantDescriptor.class); - return validateTenantId(td) + setDefaultId(td); + return TenantValidator.validate(td.getId()) .compose(x -> tenantManager.insert(new Tenant(td))) .map(x -> location(pc, td.getId(), null, Json.encodePrettily(td))); } catch (DecodeException ex) { @@ -759,12 +760,11 @@ private Future createTenant(ProxyContext pc, String body) { } } - static Future validateTenantId(TenantDescriptor td) { + static void setDefaultId(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 updateTenant(String tenantId, String body) { diff --git a/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java b/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java index f6d8049ab..e43cbbac2 100644 --- a/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java +++ b/okapi-core/src/test/java/org/folio/okapi/managers/InternalModuleTest.java @@ -109,9 +109,9 @@ void invalidJson(HttpMethod httpMethod, String path, VertxTestContext vtc) { @ParameterizedTest @NullAndEmptySource - void validateTenantId(String tenantId) { + void setDefaultId(String tenantId) { var td = new TenantDescriptor(tenantId, "foo"); - assertThat(InternalModule.validateTenantId(td).succeeded()).isTrue(); + InternalModule.setDefaultId(td); assertThat(td.getId()).matches("t[0-9a-f]{30}"); assertThat(td.getName()).isEqualTo("foo"); }