From b550bc12372a51ea66709cbdfe0f89f3e640baf4 Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Thu, 5 Jan 2023 15:59:14 +0100 Subject: [PATCH 1/6] =?UTF-8?q?Remaniement=20pour=20=C3=A9viter=20de=20fai?= =?UTF-8?q?re=20des=20effets=20de=20bords=20sur=20les=20param=C3=A8tres?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../oresing/rest/UpdateRolesOnManagement.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java b/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java index befc8a284..b687ca490 100644 --- a/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java +++ b/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java @@ -94,16 +94,8 @@ public class UpdateRolesOnManagement { private List<SqlPolicy> toDatatypePolicy(OreSiAuthorization authorization, OreSiRightOnApplicationRole oreSiRightOnApplicationRole, OperationType operation, List<SqlPolicy.Statement> statements) { - Set<String> usingExpressionElements = new LinkedHashSet<>(); SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(application); - - String dataType = authorization.getDataType(); - SqlPolicy sqlPolicy = null; - usingExpressionElements.add("application = '" + authorization.getApplication() + "'::uuid"); - usingExpressionElements.add("dataType = '" + dataType + "'"); - String expression = createExpression(authorization, usingExpressionElements, application, sqlSchemaForApplication, operation); - String usingExpression = null, checkExpression = null; - + String expression = createExpression(authorization, application, operation); return statements.stream() .map(statement -> new SqlPolicy( OreSiAuthorization.class.getSimpleName() + "_" + authorization.getId().toString() + "_data_" + statement.name().substring(0, 3), @@ -158,7 +150,14 @@ public class UpdateRolesOnManagement { return usingExpression; } - private String createExpression(OreSiAuthorization authorization, Set<String> usingExpressionElements, Application application, SqlSchemaForApplication sqlSchemaForApplication, OperationType operation) { + private String createExpression(OreSiAuthorization authorization, Application application, OperationType operation) { + + Set<String> usingExpressionElements = new LinkedHashSet<>(); + usingExpressionElements.add("application = '" + authorization.getApplication() + "'::uuid"); + usingExpressionElements.add("dataType = '" + authorization.getDataType() + "'"); + + SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(application); + if (authorization.getAuthorizations().containsKey(operation) && !authorization.getAuthorizations().get(operation).isEmpty()) { usingExpressionElements.add("\"authorization\" @> " + -- GitLab From ce55bfb9acdb1cf398fec79655d2dbfb1e6f4db8 Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Thu, 5 Jan 2023 19:35:21 +0100 Subject: [PATCH 2/6] =?UTF-8?q?G=C3=A9n=C3=A8re=20pour=20la=20table=20data?= =?UTF-8?q?=20un=20code=20pour=20les=20policy=20qui=20devrait=20=C3=AAtre?= =?UTF-8?q?=20plus=20efficace=20et=20optimisable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../fr/inra/oresing/model/Authorization.java | 41 ++++++++++++++++++ .../oresing/rest/UpdateRolesOnManagement.java | 43 +++++++++++-------- 2 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/main/java/fr/inra/oresing/model/Authorization.java b/src/main/java/fr/inra/oresing/model/Authorization.java index 75d6d52b0..04b13b2bb 100644 --- a/src/main/java/fr/inra/oresing/model/Authorization.java +++ b/src/main/java/fr/inra/oresing/model/Authorization.java @@ -6,9 +6,11 @@ import lombok.Setter; import lombok.ToString; import java.time.LocalDate; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; @Getter @@ -92,4 +94,43 @@ public class Authorization { return sql.stream() .collect(Collectors.joining(",", "(", ")::%1$s.authorization")); } + + public String toDataTablePolicyExpression() { + Set<String> authAsSqlClauses = new LinkedHashSet<>(); + if (getRequiredAuthorizations() == null || getRequiredAuthorizations().isEmpty()) { + throw new IllegalStateException("pas de contraintes d'autorisation exprimées pour " + this); + } else { + // exemple + // 'grand_lac.leman'::ltree <@ COALESCE(("authorization").requiredAuthorizations.localisation_site, ''::ltree) + // AND 'suivi_des_lacs'::ltree <@ COALESCE(("authorization").requiredAuthorizations.localisation_projet, ''::ltree) + String scopeSqlClause = getRequiredAuthorizations().entrySet().stream().map(entry -> { + String scope = entry.getKey(); + Ltree authorizedScope = entry.getValue(); + return String.format("'%s'::ltree <@ COALESCE((\"authorization\").requiredAuthorizations.%s, ''::ltree)", authorizedScope.getSql(), scope); + }).collect(Collectors.joining(" AND ")); + authAsSqlClauses.add(scopeSqlClause); + } + if (getDataGroups() == null || getDataGroups().isEmpty()) { + // pas de contrainte sur le groupe de données, on ouvre accès à tous les groupes + } else { + String dataGroupClause = getDataGroups().stream() + .map(dataGroup -> String.format(String.format("'%s'", dataGroup))) + .collect(Collectors.joining(",", "array[", "]::TEXT[] @> COALESCE((\"authorization\").datagroups, array []::TEXT[])")); + authAsSqlClauses.add(dataGroupClause); + } + if (getTimeScope() == null || getTimeScope().equals(LocalDateTimeRange.always())) { + // pas de contrainte sur la fenêtre de temps + } else { + String timeScopeAsSql = getTimeScope().toSqlExpression(); + String timeScopeClause = String.format( + "'%s'::tsrange @> COALESCE((\"authorization\").timescope, '(,)'::tsrange)", + timeScopeAsSql + ); + authAsSqlClauses.add(timeScopeClause); + } + String expression = authAsSqlClauses.stream() + .map(statement -> "(" + statement + ")") + .collect(Collectors.joining(" AND ")); + return expression; + } } \ No newline at end of file diff --git a/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java b/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java index b687ca490..27767b271 100644 --- a/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java +++ b/src/main/java/fr/inra/oresing/rest/UpdateRolesOnManagement.java @@ -2,12 +2,25 @@ package fr.inra.oresing.rest; import com.google.common.collect.Sets; import fr.inra.oresing.model.Application; +import fr.inra.oresing.model.Authorization; import fr.inra.oresing.model.Configuration; import fr.inra.oresing.model.OreSiAuthorization; -import fr.inra.oresing.persistence.*; +import fr.inra.oresing.persistence.AuthenticationService; +import fr.inra.oresing.persistence.AuthorizationRepository; +import fr.inra.oresing.persistence.OperationType; +import fr.inra.oresing.persistence.OreSiRepository; +import fr.inra.oresing.persistence.SqlPolicy; +import fr.inra.oresing.persistence.SqlSchema; +import fr.inra.oresing.persistence.SqlSchemaForApplication; +import fr.inra.oresing.persistence.SqlService; import fr.inra.oresing.persistence.roles.OreSiRightOnApplicationRole; -import java.util.*; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; public class UpdateRolesOnManagement { @@ -95,7 +108,7 @@ public class UpdateRolesOnManagement { private List<SqlPolicy> toDatatypePolicy(OreSiAuthorization authorization, OreSiRightOnApplicationRole oreSiRightOnApplicationRole, OperationType operation, List<SqlPolicy.Statement> statements) { SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(application); - String expression = createExpression(authorization, application, operation); + String expression = createExpression(authorization, operation); return statements.stream() .map(statement -> new SqlPolicy( OreSiAuthorization.class.getSimpleName() + "_" + authorization.getId().toString() + "_data_" + statement.name().substring(0, 3), @@ -150,25 +163,21 @@ public class UpdateRolesOnManagement { return usingExpression; } - private String createExpression(OreSiAuthorization authorization, Application application, OperationType operation) { + private String createExpression(OreSiAuthorization authorization, OperationType operation) { Set<String> usingExpressionElements = new LinkedHashSet<>(); usingExpressionElements.add("application = '" + authorization.getApplication() + "'::uuid"); usingExpressionElements.add("dataType = '" + authorization.getDataType() + "'"); - SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(application); - - if (authorization.getAuthorizations().containsKey(operation) && - !authorization.getAuthorizations().get(operation).isEmpty()) { - usingExpressionElements.add("\"authorization\" @> " + - authorization.getAuthorizations().get(operation).stream() - .map(auth -> auth.toSQL(application.getConfiguration().getRequiredAuthorizationsAttributes())) - .filter(auth -> auth != null) - .map(sql -> String.format(sql, sqlSchemaForApplication.getName())) - .collect(Collectors.joining(",", "ARRAY[", "]::" + sqlSchemaForApplication.getName() + ".authorization[]")) - - - ); + List<Authorization> auths = authorization.getAuthorizations().getOrDefault(operation, List.of()); + if (auths.isEmpty()) { + // pas de contrainte d'autorisation exprimée, on donne les droits sur tout le type de données + } else { + String collect = auths.stream() + .map(Authorization::toDataTablePolicyExpression) + .map(statement -> "(" + statement + ")") + .collect(Collectors.joining(" OR ")); + usingExpressionElements.add(collect); } String usingExpression = usingExpressionElements.stream() -- GitLab From a4cb2a819e024382a146b90ea25f5141a6c265fe Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Fri, 6 Jan 2023 16:07:30 +0100 Subject: [PATCH 3/6] =?UTF-8?q?Corrige=20la=20g=C3=A9n=C3=A9ration=20SQL?= =?UTF-8?q?=20du=20RLS=20pour=20la=20gestion=20des=20droits=20quand=20on?= =?UTF-8?q?=20donne=20une=20autorisation=20"vide"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/fr/inra/oresing/model/Authorization.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/fr/inra/oresing/model/Authorization.java b/src/main/java/fr/inra/oresing/model/Authorization.java index 04b13b2bb..1d581dcbc 100644 --- a/src/main/java/fr/inra/oresing/model/Authorization.java +++ b/src/main/java/fr/inra/oresing/model/Authorization.java @@ -98,7 +98,7 @@ public class Authorization { public String toDataTablePolicyExpression() { Set<String> authAsSqlClauses = new LinkedHashSet<>(); if (getRequiredAuthorizations() == null || getRequiredAuthorizations().isEmpty()) { - throw new IllegalStateException("pas de contraintes d'autorisation exprimées pour " + this); + // pas de contrainte sur le périmètre } else { // exemple // 'grand_lac.leman'::ltree <@ COALESCE(("authorization").requiredAuthorizations.localisation_site, ''::ltree) @@ -128,9 +128,14 @@ public class Authorization { ); authAsSqlClauses.add(timeScopeClause); } - String expression = authAsSqlClauses.stream() - .map(statement -> "(" + statement + ")") - .collect(Collectors.joining(" AND ")); + String expression; + if (authAsSqlClauses.isEmpty()) { + expression = "TRUE"; + } else { + expression = authAsSqlClauses.stream() + .map(statement -> "(" + statement + ")") + .collect(Collectors.joining(" AND ")); + } return expression; } } \ No newline at end of file -- GitLab From 8bcb236e0a3790d2cf38ce708fbd3aea4d7f5d57 Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Tue, 10 Jan 2023 17:08:32 +0100 Subject: [PATCH 4/6] =?UTF-8?q?Tente=20d'optimiser=20la=20requ=C3=AAte=20R?= =?UTF-8?q?LS=20sur=20le=20crit=C3=A8re=20groupe=20de=20donn=C3=A9es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/fr/inra/oresing/model/Authorization.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/fr/inra/oresing/model/Authorization.java b/src/main/java/fr/inra/oresing/model/Authorization.java index 1d581dcbc..23af3a63e 100644 --- a/src/main/java/fr/inra/oresing/model/Authorization.java +++ b/src/main/java/fr/inra/oresing/model/Authorization.java @@ -115,7 +115,7 @@ public class Authorization { } else { String dataGroupClause = getDataGroups().stream() .map(dataGroup -> String.format(String.format("'%s'", dataGroup))) - .collect(Collectors.joining(",", "array[", "]::TEXT[] @> COALESCE((\"authorization\").datagroups, array []::TEXT[])")); + .collect(Collectors.joining(",", "(\"authorization\").datagroups[1] = ANY ('{", "}'::text[])")); authAsSqlClauses.add(dataGroupClause); } if (getTimeScope() == null || getTimeScope().equals(LocalDateTimeRange.always())) { -- GitLab From 8bfe06bcd30158dbf26f8700a744b41ea531ba79 Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Tue, 10 Jan 2023 17:41:01 +0100 Subject: [PATCH 5/6] =?UTF-8?q?Corrige=20une=20syntaxe=20SQL=20g=C3=A9n?= =?UTF-8?q?=C3=A9r=C3=A9e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/fr/inra/oresing/model/Authorization.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/fr/inra/oresing/model/Authorization.java b/src/main/java/fr/inra/oresing/model/Authorization.java index 23af3a63e..acb8cc212 100644 --- a/src/main/java/fr/inra/oresing/model/Authorization.java +++ b/src/main/java/fr/inra/oresing/model/Authorization.java @@ -114,7 +114,6 @@ public class Authorization { // pas de contrainte sur le groupe de données, on ouvre accès à tous les groupes } else { String dataGroupClause = getDataGroups().stream() - .map(dataGroup -> String.format(String.format("'%s'", dataGroup))) .collect(Collectors.joining(",", "(\"authorization\").datagroups[1] = ANY ('{", "}'::text[])")); authAsSqlClauses.add(dataGroupClause); } -- GitLab From 6850391ce1d64ea21fbd7d6e76886a343f878a78 Mon Sep 17 00:00:00 2001 From: Brendan Le Ny <bleny@codelutin.com> Date: Tue, 17 Jan 2023 14:29:13 +0100 Subject: [PATCH 6/6] =?UTF-8?q?Ajoute=20un=20index=20permettant=20d'am?= =?UTF-8?q?=C3=A9liorer=20(=3F)=20les=20requ=C3=AAtes=20contenant=20des=20?= =?UTF-8?q?clauses=20RLS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../persistence/SqlSchemaForApplication.java | 7 ++++ .../fr/inra/oresing/rest/OreSiService.java | 36 +++++++++++++------ .../V2__add_authorization_index.sql | 8 +++++ .../main/V3__add_btree_gist_extension.sql | 1 + 4 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 src/main/resources/migration/application/V2__add_authorization_index.sql create mode 100644 src/main/resources/migration/main/V3__add_btree_gist_extension.sql diff --git a/src/main/java/fr/inra/oresing/persistence/SqlSchemaForApplication.java b/src/main/java/fr/inra/oresing/persistence/SqlSchemaForApplication.java index b2de768c7..a913f4e30 100644 --- a/src/main/java/fr/inra/oresing/persistence/SqlSchemaForApplication.java +++ b/src/main/java/fr/inra/oresing/persistence/SqlSchemaForApplication.java @@ -78,4 +78,11 @@ public class SqlSchemaForApplication implements SqlSchema { .collect(Collectors.joining("\n AND ")); return requiredAuthorizationsAttributesComparing + (Strings.isNullOrEmpty(requiredAuthorizationsAttributesComparing)?"":" AND\n "); } + + public String requiredAuthorizationsAttributesIndex(Application app) { + String requiredAuthorizationsAttributesIndex = app.getConfiguration().getRequiredAuthorizationsAttributes().stream() + .map(attribute -> String.format("COALESCE((data.\"authorization\").requiredauthorizations.%s, ''::ltree),", attribute)) + .collect(Collectors.joining()); + return requiredAuthorizationsAttributesIndex; + } } \ No newline at end of file diff --git a/src/main/java/fr/inra/oresing/rest/OreSiService.java b/src/main/java/fr/inra/oresing/rest/OreSiService.java index add8ea0ef..96e13dfcc 100644 --- a/src/main/java/fr/inra/oresing/rest/OreSiService.java +++ b/src/main/java/fr/inra/oresing/rest/OreSiService.java @@ -36,6 +36,7 @@ import org.assertj.core.util.Streams; import org.flywaydb.core.Flyway; import org.flywaydb.core.api.Location; import org.flywaydb.core.api.configuration.ClassicConfiguration; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.jdbc.BadSqlGrammarException; import org.springframework.stereotype.Component; @@ -66,7 +67,7 @@ import java.util.stream.Stream; @Slf4j @Component @Transactional -public class OreSiService { +public class OreSiService implements InitializingBean { public static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneOffset.UTC); public static final DateTimeFormatter DATE_FORMATTER_DDMMYYYY = DateTimeFormatter.ofPattern("dd/MM/yyyy"); @@ -153,16 +154,7 @@ public class OreSiService { public Application initApplication(Application app) { authenticationService.resetRole(); - SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(app); - org.flywaydb.core.api.configuration.ClassicConfiguration flywayConfiguration = new ClassicConfiguration(); - flywayConfiguration.setDataSource(dataSource); - flywayConfiguration.setSchemas(sqlSchemaForApplication.getName()); - flywayConfiguration.setLocations(new Location("classpath:migration/application")); - flywayConfiguration.getPlaceholders().put("applicationSchema", sqlSchemaForApplication.getSqlIdentifier()); - flywayConfiguration.getPlaceholders().put("requiredAuthorizations", sqlSchemaForApplication.requiredAuthorizationsAttributes(app)); - flywayConfiguration.getPlaceholders().put("requiredAuthorizationscomparing", sqlSchemaForApplication.requiredAuthorizationsAttributesComparing(app)); - Flyway flyway = new Flyway(flywayConfiguration); - flyway.migrate(); + migrateApplicationSchema(app); OreSiRightOnApplicationRole adminOnApplicationRole = OreSiRightOnApplicationRole.adminOn(app); OreSiRightOnApplicationRole readerOnApplicationRole = OreSiRightOnApplicationRole.readerOn(app); @@ -204,6 +196,8 @@ public class OreSiService { "name = '" + app.getName() + "'" )); + SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(app); + db.setSchemaOwner(sqlSchemaForApplication, adminOnApplicationRole); db.grantUsage(sqlSchemaForApplication, readerOnApplicationRole); @@ -219,6 +213,26 @@ public class OreSiService { return app; } + private void migrateApplicationSchema(Application app) { + SqlSchemaForApplication sqlSchemaForApplication = SqlSchema.forApplication(app); + ClassicConfiguration flywayConfiguration = new ClassicConfiguration(); + flywayConfiguration.setDataSource(dataSource); + flywayConfiguration.setSchemas(sqlSchemaForApplication.getName()); + flywayConfiguration.setLocations(new Location("classpath:migration/application")); + flywayConfiguration.getPlaceholders().put("applicationSchema", sqlSchemaForApplication.getSqlIdentifier()); + flywayConfiguration.getPlaceholders().put("requiredAuthorizations", sqlSchemaForApplication.requiredAuthorizationsAttributes(app)); + flywayConfiguration.getPlaceholders().put("requiredAuthorizationscomparing", sqlSchemaForApplication.requiredAuthorizationsAttributesComparing(app)); + flywayConfiguration.getPlaceholders().put("requiredAuthorizationsAttributesIndex", sqlSchemaForApplication.requiredAuthorizationsAttributesIndex(app)); + Flyway flyway = new Flyway(flywayConfiguration); + flyway.migrate(); + } + + @Override + public void afterPropertiesSet() { + List<Application> applications = repo.application().findAll(); + applications.forEach(this::migrateApplicationSchema); + } + public UUID changeApplicationConfiguration(String nameOrId, MultipartFile configurationFile, String comment) throws IOException, BadApplicationConfigurationException { relationalService.dropViews(nameOrId); authenticationService.setRoleForClient(); diff --git a/src/main/resources/migration/application/V2__add_authorization_index.sql b/src/main/resources/migration/application/V2__add_authorization_index.sql new file mode 100644 index 000000000..4dc178912 --- /dev/null +++ b/src/main/resources/migration/application/V2__add_authorization_index.sql @@ -0,0 +1,8 @@ + +CREATE INDEX helping_rls ON ${applicationSchema}.data USING gist ( + ${requiredAuthorizationsAttributesIndex} + ((data."authorization").datagroups[1]), + COALESCE((data."authorization").timescope, '(,)'::tsrange) +); + +ANALYSE ${applicationSchema}.data; diff --git a/src/main/resources/migration/main/V3__add_btree_gist_extension.sql b/src/main/resources/migration/main/V3__add_btree_gist_extension.sql new file mode 100644 index 000000000..afe053468 --- /dev/null +++ b/src/main/resources/migration/main/V3__add_btree_gist_extension.sql @@ -0,0 +1 @@ +CREATE EXTENSION btree_gist; -- GitLab