josemakara2 commented on a change in pull request #1671:
URL: https://github.com/apache/fineract/pull/1671#discussion_r611043018
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/JdbcTenantDetailsService.java
##########
@@ -49,7 +49,9 @@ public
JdbcTenantDetailsService(@Qualifier("hikariTenantDataSource") final DataS
private static final class TenantMapper implements
RowMapper<FineractPlatformTenant> {
- private final StringBuilder sqlBuilder = new StringBuilder("t.id,
ts.id as connectionId , ")//
+ private final String tenantIdentifier;
Review comment:
Noted on pattern breakage.
The change was informed by the need to keep the query construct constant and
parameterize external input to the SQL query.
We cannot add ? for `select " + rm.schema()`. PreparedStatement will return
`SELECT 'the string value of rm.schema'`
With a simple example such as `SELECT firstname FROM m_appuser;`
`SELECT ?;` parameterizing `firstname FROM m_appuser` will translate into
a `SELECT 'firstname FROM m_appuser';` which returns resultSet value of
'firstname FROM m_appuser'
It depends on PreparedStatement set# methods and Since we cannot user
parameter queries for `SELECT ? FROM ?; ` or SELECT ? for now I suggest.
1. Leave this concatenation the way it is currently : `final String sql =
"select " + rm.schema();` - it is from a 'trusted' source
2. In future - where Parameterized Queries is not feasible to mitigate SQL
Injection, try ESAPI `final String sql = "select " +
EsapiUtils.encodeForMySQL(rm.schema());`
3. I am trying ESAPI in PR #1687 but still in the kitchen sink and haven't
tested things there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
