This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch postgresql in repository https://gitbox.apache.org/repos/asf/james-project.git
commit a207a9a0fa956bfaba7e4a7a43301cb7c58121f7 Author: Tung Tran <[email protected]> AuthorDate: Mon Nov 13 11:43:50 2023 +0700 JAMES-2586 Fix row-level security implementation --- .../backends/postgres/PostgresTableManager.java | 1 + .../james/backends/postgres/PostgresExtension.java | 44 +++++++++---- .../james/backends/postgres/PostgresFixture.java | 76 +++++++++++++++++++--- .../SimpleJamesPostgresConnectionFactoryTest.java | 3 +- .../resources/postgres-rowlevelsecurity-init.sql | 5 ++ ...gresSubscriptionMapperRowLevelSecurityTest.java | 6 -- .../user/PostgresSubscriptionMapperTest.java | 5 +- 7 files changed, 108 insertions(+), 32 deletions(-) diff --git a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java index eaa4aa7994..c7b2ff1bf7 100644 --- a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java +++ b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java @@ -94,6 +94,7 @@ public class PostgresTableManager implements Startable { private String rowLevelSecurityAlterStatement(String tableName) { return "SET app.current_domain = ''; ALTER TABLE " + tableName + " ADD DOMAIN varchar(255) not null DEFAULT current_setting('app.current_domain')::text;" + "ALTER TABLE " + tableName + " ENABLE ROW LEVEL SECURITY; " + + "ALTER TABLE " + tableName + " FORCE ROW LEVEL SECURITY; " + "CREATE POLICY DOMAIN_" + tableName + "_POLICY ON " + tableName + " USING (DOMAIN = current_setting('app.current_domain')::text);"; } diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java index a1ae9b9aba..b340a5f8ac 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java @@ -19,13 +19,19 @@ package org.apache.james.backends.postgres; +import static org.apache.james.backends.postgres.PostgresFixture.Database.DEFAULT_DATABASE; +import static org.apache.james.backends.postgres.PostgresFixture.SCRIPT_ROW_LEVEL_SECURITY_INIT_PATH; + import java.net.URISyntaxException; +import java.util.Optional; import org.apache.http.client.utils.URIBuilder; import org.apache.james.GuiceModuleTestExtension; import org.apache.james.backends.postgres.utils.PostgresExecutor; import org.junit.jupiter.api.extension.ExtensionContext; +import org.testcontainers.containers.PostgreSQLContainer; +import com.github.fge.lambdas.Throwing; import com.google.inject.Module; import com.google.inject.util.Modules; @@ -50,8 +56,11 @@ public class PostgresExtension implements GuiceModuleTestExtension { return withoutRowLevelSecurity(PostgresModule.EMPTY_MODULE); } + public static PostgreSQLContainer<?> PG_CONTAINER = DockerPostgresSingleton.SINGLETON; private final PostgresModule postgresModule; private final boolean rlsEnabled; + private final Optional<String> initScriptPath; + private final PostgresFixture.Database selectedDatabase; private PostgresConfiguration postgresConfiguration; private PostgresExecutor postgresExecutor; private PostgresqlConnectionFactory connectionFactory; @@ -59,27 +68,40 @@ public class PostgresExtension implements GuiceModuleTestExtension { private PostgresExtension(PostgresModule postgresModule, boolean rlsEnabled) { this.postgresModule = postgresModule; this.rlsEnabled = rlsEnabled; + if (rlsEnabled) { + this.selectedDatabase = PostgresFixture.Database.ROW_LEVEL_SECURITY_DATABASE; + this.initScriptPath = Optional.of(SCRIPT_ROW_LEVEL_SECURITY_INIT_PATH); + } else { + this.selectedDatabase = DEFAULT_DATABASE; + this.initScriptPath = Optional.empty(); + } } @Override public void beforeAll(ExtensionContext extensionContext) throws Exception { - if (!DockerPostgresSingleton.SINGLETON.isRunning()) { - DockerPostgresSingleton.SINGLETON.start(); + if (!PG_CONTAINER.isRunning()) { + PG_CONTAINER.start(); } + runInitScriptIfNeed(); initPostgresSession(); } + + private void runInitScriptIfNeed() { + initScriptPath.ifPresent(scriptPath -> Throwing.supplier(() -> PG_CONTAINER.execInContainer("psql", "-U", DEFAULT_DATABASE.dbUser(), "-f", scriptPath)).get()); + } + private void initPostgresSession() throws URISyntaxException { postgresConfiguration = PostgresConfiguration.builder() .url(new URIBuilder() .setScheme("postgresql") .setHost(getHost()) .setPort(getMappedPort()) - .setUserInfo(PostgresFixture.Database.DB_USER, PostgresFixture.Database.DB_PASSWORD) + .setUserInfo(selectedDatabase.dbUser(), selectedDatabase.dbPassword()) .build() .toString()) - .databaseName(PostgresFixture.Database.DB_NAME) - .databaseSchema(PostgresFixture.Database.SCHEMA) + .databaseName(selectedDatabase.dbName()) + .databaseSchema(selectedDatabase.schema()) .rowLevelSecurityEnabled(rlsEnabled) .build(); @@ -117,8 +139,8 @@ public class PostgresExtension implements GuiceModuleTestExtension { } public void restartContainer() throws URISyntaxException { - DockerPostgresSingleton.SINGLETON.stop(); - DockerPostgresSingleton.SINGLETON.start(); + PG_CONTAINER.stop(); + PG_CONTAINER.start(); initPostgresSession(); } @@ -129,11 +151,11 @@ public class PostgresExtension implements GuiceModuleTestExtension { } public String getHost() { - return DockerPostgresSingleton.SINGLETON.getHost(); + return PG_CONTAINER.getHost(); } public Integer getMappedPort() { - return DockerPostgresSingleton.SINGLETON.getMappedPort(PostgresFixture.PORT); + return PG_CONTAINER.getMappedPort(PostgresFixture.PORT); } public Mono<Connection> getConnection() { @@ -156,8 +178,8 @@ public class PostgresExtension implements GuiceModuleTestExtension { private void resetSchema() { getConnection() - .flatMapMany(connection -> Mono.from(connection.createStatement("DROP SCHEMA " + PostgresFixture.Database.SCHEMA + " CASCADE").execute()) - .then(Mono.from(connection.createStatement("CREATE SCHEMA " + PostgresFixture.Database.SCHEMA).execute())) + .flatMapMany(connection -> Mono.from(connection.createStatement("DROP SCHEMA " + selectedDatabase.schema() + " CASCADE").execute()) + .then(Mono.from(connection.createStatement("CREATE SCHEMA " + selectedDatabase.schema() + " AUTHORIZATION " + selectedDatabase.dbUser()).execute())) .flatMap(result -> Mono.from(result.getRowsUpdated()))) .collectList() .block(); diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java index 813e9d73a3..b7fed73163 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java @@ -19,28 +19,84 @@ package org.apache.james.backends.postgres; +import static org.apache.james.backends.postgres.PostgresFixture.Database.DEFAULT_DATABASE; import static org.testcontainers.containers.PostgreSQLContainer.POSTGRESQL_PORT; import java.util.UUID; import java.util.function.Supplier; import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.utility.MountableFile; public interface PostgresFixture { interface Database { - String DB_USER = "james"; - String DB_PASSWORD = "secret1"; - String DB_NAME = "james"; - String SCHEMA = "public"; + + Database DEFAULT_DATABASE = new DefaultDatabase(); + Database ROW_LEVEL_SECURITY_DATABASE = new RowLevelSecurityDatabase(); + + String dbUser(); + + String dbPassword(); + + String dbName(); + + String schema(); + + + class DefaultDatabase implements Database { + @Override + public String dbUser() { + return "james"; + } + + @Override + public String dbPassword() { + return "secret1"; + } + + @Override + public String dbName() { + return "james"; + } + + @Override + public String schema() { + return "public"; + } + } + + class RowLevelSecurityDatabase implements Database { + @Override + public String dbUser() { + return "rlsuser"; + } + + @Override + public String dbPassword() { + return "secret1"; + } + + @Override + public String dbName() { + return "rlsdb"; + } + + @Override + public String schema() { + return "rlsschema"; + } + } } - String IMAGE = "postgres:16.0"; + String IMAGE = "postgres:16"; Integer PORT = POSTGRESQL_PORT; - + String POSTGRES_ROW_LEVEL_SECURITY_INIT_FILE = "postgres-rowlevelsecurity-init.sql"; + String SCRIPT_ROW_LEVEL_SECURITY_INIT_PATH = "/tmp/" + POSTGRES_ROW_LEVEL_SECURITY_INIT_FILE; Supplier<PostgreSQLContainer<?>> PG_CONTAINER = () -> new PostgreSQLContainer<>(IMAGE) - .withDatabaseName(Database.DB_NAME) - .withUsername(Database.DB_USER) - .withPassword(Database.DB_PASSWORD) - .withCreateContainerCmdModifier(cmd -> cmd.withName("james-postgres-test-" + UUID.randomUUID())); + .withDatabaseName(DEFAULT_DATABASE.dbName()) + .withUsername(DEFAULT_DATABASE.dbUser()) + .withPassword(DEFAULT_DATABASE.dbPassword()) + .withCreateContainerCmdModifier(cmd -> cmd.withName("james-postgres-test-" + UUID.randomUUID())) + .withCopyFileToContainer(MountableFile.forClasspathResource(POSTGRES_ROW_LEVEL_SECURITY_INIT_FILE), "/tmp/"); } diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/SimpleJamesPostgresConnectionFactoryTest.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/SimpleJamesPostgresConnectionFactoryTest.java index cfe457db34..10d7b8f84e 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/SimpleJamesPostgresConnectionFactoryTest.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/SimpleJamesPostgresConnectionFactoryTest.java @@ -19,6 +19,7 @@ package org.apache.james.backends.postgres; +import static org.apache.james.backends.postgres.PostgresFixture.Database.DEFAULT_DATABASE; import static org.assertj.core.api.Assertions.assertThat; import java.net.URISyntaxException; @@ -84,7 +85,7 @@ public class SimpleJamesPostgresConnectionFactoryTest extends JamesPostgresConne @Nullable private Integer getNumberOfConnections() { return Mono.from(postgresqlConnection.createStatement("SELECT count(*) from pg_stat_activity where usename = $1;") - .bind("$1", PostgresFixture.Database.DB_USER) + .bind("$1", DEFAULT_DATABASE.dbUser()) .execute()).flatMap(result -> Mono.from(result.map((row, rowMetadata) -> row.get(0, Integer.class)))).block(); } diff --git a/backends-common/postgres/src/test/resources/postgres-rowlevelsecurity-init.sql b/backends-common/postgres/src/test/resources/postgres-rowlevelsecurity-init.sql new file mode 100644 index 0000000000..7a18723041 --- /dev/null +++ b/backends-common/postgres/src/test/resources/postgres-rowlevelsecurity-init.sql @@ -0,0 +1,5 @@ +create user rlsuser WITH PASSWORD 'secret1'; +create database rlsdb; +grant all privileges on database rlsdb to rlsuser; +\c rlsdb; +create schema if not exists rlsschema authorization rlsuser; \ No newline at end of file diff --git a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperRowLevelSecurityTest.java b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperRowLevelSecurityTest.java index 9505cd473e..69dc70eddb 100644 --- a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperRowLevelSecurityTest.java +++ b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperRowLevelSecurityTest.java @@ -22,19 +22,14 @@ package org.apache.james.mailbox.postgres.user; import static org.assertj.core.api.Assertions.assertThat; import org.apache.james.backends.postgres.PostgresExtension; -import org.apache.james.backends.postgres.utils.JamesPostgresConnectionFactory; import org.apache.james.backends.postgres.utils.PostgresExecutor; import org.apache.james.backends.postgres.utils.SimpleJamesPostgresConnectionFactory; import org.apache.james.core.Username; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSessionUtil; -import org.apache.james.mailbox.exception.SubscriptionException; -import org.apache.james.mailbox.store.user.SubscriptionMapper; import org.apache.james.mailbox.store.user.SubscriptionMapperFactory; -import org.apache.james.mailbox.store.user.SubscriptionMapperTest; import org.apache.james.mailbox.store.user.model.Subscription; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -67,7 +62,6 @@ public class PostgresSubscriptionMapperRowLevelSecurityTest { .containsOnly(subscription); } - @Disabled("Row level security for subscriptions is not implemented correctly") @Test void subscriptionsShouldBeIsolatedByDomain() throws Exception { Username username = Username.of("bob@domain1"); diff --git a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperTest.java b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperTest.java index 5d05795398..ebd4c626e2 100644 --- a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperTest.java +++ b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionMapperTest.java @@ -20,9 +20,6 @@ package org.apache.james.mailbox.postgres.user; import org.apache.james.backends.postgres.PostgresExtension; -import org.apache.james.mailbox.postgres.user.PostgresSubscriptionDAO; -import org.apache.james.mailbox.postgres.user.PostgresSubscriptionMapper; -import org.apache.james.mailbox.postgres.user.PostgresSubscriptionModule; import org.apache.james.mailbox.store.user.SubscriptionMapper; import org.apache.james.mailbox.store.user.SubscriptionMapperTest; import org.junit.jupiter.api.extension.RegisterExtension; @@ -30,7 +27,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; public class PostgresSubscriptionMapperTest extends SubscriptionMapperTest { @RegisterExtension - static PostgresExtension postgresExtension = PostgresExtension.withoutRowLevelSecurity(PostgresSubscriptionModule.MODULE); + static PostgresExtension postgresExtension = PostgresExtension.withRowLevelSecurity(PostgresSubscriptionModule.MODULE); @Override protected SubscriptionMapper createSubscriptionMapper() { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
