This is an automated email from the ASF dual-hosted git repository. ptuomola pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/fineract.git
commit 678bc3a5f3771359e568e2b21a7e30896535c76c Author: Joseph Makara <[email protected]> AuthorDate: Sun Mar 28 22:02:12 2021 +0300 Fix Tenant SQLi (FINERACT-854) --- .../service/BasicAuthTenantDetailsServiceJdbc.java | 13 ++++----- .../security/service/JdbcTenantDetailsService.java | 31 ++++++++++++++-------- .../list_db/V6__add_unique_tenant_identifier.sql | 20 ++++++++++++++ 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java index c0783d6..a5c1596 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/BasicAuthTenantDetailsServiceJdbc.java @@ -49,7 +49,7 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails private static final class TenantMapper implements RowMapper<FineractPlatformTenant> { private final boolean isReport; - private final StringBuilder sqlBuilder = new StringBuilder(" t.id, ts.id as connectionId , ")// + private final StringBuilder sqlBuilder = new StringBuilder("SELECT t.id, ts.id as connectionId, ")// .append(" t.timezone_id as timezoneId , t.name,t.identifier, ts.schema_name as schemaName, ts.schema_server as schemaServer,")// .append(" ts.schema_server_port as schemaServerPort, ts.schema_connection_parameters as schemaConnectionParameters, ts.auto_update as autoUpdate,")// .append(" ts.schema_username as schemaUsername, ts.schema_password as schemaPassword , ts.pool_initial_size as initialSize,")// @@ -60,7 +60,7 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails .append(" ts.pool_min_evictable_idle_time_millis as poolMinEvictableIdleTimeMillis,")// .append(" ts.deadlock_max_retries as maxRetriesOnDeadlock,")// .append(" ts.deadlock_max_retry_interval as maxIntervalBetweenRetries ")// - .append(" from tenants t left join tenant_server_connections ts "); + .append("FROM tenants t LEFT JOIN tenant_server_connections ts "); TenantMapper(boolean isReport) { this.isReport = isReport; @@ -68,10 +68,11 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails public String schema() { if (this.isReport) { - this.sqlBuilder.append(" on t.report_Id = ts.id"); + this.sqlBuilder.append(" ON t.report_Id = ts.id"); } else { - this.sqlBuilder.append(" on t.oltp_Id = ts.id"); + this.sqlBuilder.append(" ON t.oltp_Id = ts.id"); } + this.sqlBuilder.append(" WHERE t.identifier = ?"); return this.sqlBuilder.toString(); } @@ -138,9 +139,9 @@ public class BasicAuthTenantDetailsServiceJdbc implements BasicAuthTenantDetails try { final TenantMapper rm = new TenantMapper(isReport); - final String sql = "select " + rm.schema() + " where t.identifier like ?"; + final String sql = rm.schema(); - return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { tenantIdentifier }); + return this.jdbcTemplate.queryForObject(sql, rm, tenantIdentifier); } catch (final EmptyResultDataAccessException e) { throw new InvalidTenantIdentiferException("The tenant identifier: " + tenantIdentifier + " is not valid.", e); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java index dd1551a..879fec9 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java @@ -49,7 +49,9 @@ public class JdbcTenantDetailsService implements TenantDetailsService { private static final class TenantMapper implements RowMapper<FineractPlatformTenant> { - private final StringBuilder sqlBuilder = new StringBuilder("t.id, ts.id as connectionId , ")// + private final String tenantIdentifier; + + private final StringBuilder sqlBuilder = new StringBuilder("SELECT t.id, ts.id as connectionId, ")// .append(" t.timezone_id as timezoneId , t.name,t.identifier, ts.schema_name as schemaName, ts.schema_server as schemaServer,")// .append(" ts.schema_server_port as schemaServerPort, ts.schema_connection_parameters as schemaConnectionParameters, ts.auto_update as autoUpdate,")// .append(" ts.schema_username as schemaUsername, ts.schema_password as schemaPassword , ts.pool_initial_size as initialSize,")// @@ -60,21 +62,28 @@ public class JdbcTenantDetailsService implements TenantDetailsService { .append(" ts.pool_min_evictable_idle_time_millis as poolMinEvictableIdleTimeMillis,")// .append(" ts.deadlock_max_retries as maxRetriesOnDeadlock,")// .append(" ts.deadlock_max_retry_interval as maxIntervalBetweenRetries ")// - .append(" from tenants t left join tenant_server_connections ts on t.oltp_Id=ts.id "); + .append("FROM tenants t LEFT JOIN tenant_server_connections ts ON t.oltp_Id=ts.id "); + + TenantMapper(String aTenantIdentifier) { + this.tenantIdentifier = aTenantIdentifier; + } public String schema() { + if (tenantIdentifier != null) { + this.sqlBuilder.append(" WHERE t.identifier = ?"); + } return this.sqlBuilder.toString(); } @Override public FineractPlatformTenant mapRow(final ResultSet rs, @SuppressWarnings("unused") final int rowNum) throws SQLException { final Long id = rs.getLong("id"); - final String tenantIdentifier = rs.getString("identifier"); + final String identifier = rs.getString("identifier"); final String name = rs.getString("name"); final String timezoneId = rs.getString("timezoneId"); final FineractPlatformTenantConnection connection = getDBConnection(rs); - return new FineractPlatformTenant(id, tenantIdentifier, name, timezoneId, connection); + return new FineractPlatformTenant(id, identifier, name, timezoneId, connection); } // gets the DB connection @@ -127,22 +136,22 @@ public class JdbcTenantDetailsService implements TenantDetailsService { @Override @Cacheable(value = "tenantsById") - public FineractPlatformTenant loadTenantById(final String tenantIdentifier) { + public FineractPlatformTenant loadTenantById(final String aTenantIdentifier) { try { - final TenantMapper rm = new TenantMapper(); - final String sql = "select " + rm.schema() + " where t.identifier like ?"; + final TenantMapper rm = new TenantMapper(aTenantIdentifier); + final String sql = rm.schema(); - return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { tenantIdentifier }); + return this.jdbcTemplate.queryForObject(sql, rm, aTenantIdentifier); } catch (final EmptyResultDataAccessException e) { - throw new InvalidTenantIdentiferException("The tenant identifier: " + tenantIdentifier + " is not valid.", e); + throw new InvalidTenantIdentiferException("The tenant identifier: " + aTenantIdentifier + " is not valid.", e); } } @Override public List<FineractPlatformTenant> findAllTenants() { - final TenantMapper rm = new TenantMapper(); - final String sql = "select " + rm.schema(); + final TenantMapper rm = new TenantMapper(null); + final String sql = rm.schema(); final List<FineractPlatformTenant> fineractPlatformTenants = this.jdbcTemplate.query(sql, rm, new Object[] {}); return fineractPlatformTenants; diff --git a/fineract-provider/src/main/resources/sql/migrations/list_db/V6__add_unique_tenant_identifier.sql b/fineract-provider/src/main/resources/sql/migrations/list_db/V6__add_unique_tenant_identifier.sql new file mode 100644 index 0000000..f94da8f --- /dev/null +++ b/fineract-provider/src/main/resources/sql/migrations/list_db/V6__add_unique_tenant_identifier.sql @@ -0,0 +1,20 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- + +ALTER TABLE tenants ADD UNIQUE (identifier);
