Skip to content

Commit

Permalink
OKAPI-1081: Reject invalid tenant ids
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
julianladisch committed Feb 21, 2024
1 parent 7101d6f commit b40a3d2
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -750,22 +751,21 @@ 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(2));
}
return TenantValidator.validate(td.getId());
}

private Future<String> updateTenant(String tenantId, String body) {
try {
final TenantDescriptor td = JsonDecoder.decode(body, TenantDescriptor.class);
Expand Down
58 changes: 58 additions & 0 deletions okapi-core/src/main/java/org/folio/okapi/util/TenantValidator.java
Original file line number Diff line number Diff line change
@@ -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
* <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 {

// 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<Void> 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<Void> failure(String errorMessage, String tenantId) {
return Future.failedFuture(new OkapiError(ErrorType.USER,
MESSAGES.getMessage(errorMessage, tenantId)));
}
}
2 changes: 1 addition & 1 deletion okapi-core/src/main/raml/TenantDescriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
8 changes: 4 additions & 4 deletions okapi-core/src/test/java/org/folio/okapi/ProxyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

0 comments on commit b40a3d2

Please sign in to comment.