Fix Tenant SQLi (FINERACT-854)

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);
         }

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;

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);