details: https://code.openbravo.com/erp/devel/pi/rev/72f9f1727bc1 changeset: 33693:72f9f1727bc1 user: Stefan Hühner <stefan.huehner <at> openbravo.com> date: Thu Mar 15 10:41:23 2018 +0100 summary: Fixed 38137. Use-bind parameters instead of mixing data values in hql-String.
details: https://code.openbravo.com/erp/devel/pi/rev/c6e6d9b1a8f0 changeset: 33694:c6e6d9b1a8f0 user: Stefan Hühner <stefan.huehner <at> openbravo.com> date: Thu Mar 15 10:44:22 2018 +0100 summary: Fixed 38135. Use bind-parameters instead of mixing data with the HQL-String. - Modify getOne utility method to receive a Map of parameters to be applied to the query - Change all callers to use named parameters in the HQL string and a map for the param values - Remove the 2 extra copies of getOne now no longer required. - in getOrganizations function - change HQL positional parameter to named one as that is better style and avoid a future hibernate behavior change diffstat: src/org/openbravo/dal/core/OBContext.java | 61 ++++++++++------ src/org/openbravo/dal/security/EntityAccessChecker.java | 14 ++- 2 files changed, 45 insertions(+), 30 deletions(-) diffs (155 lines): diff -r 636948c66791 -r c6e6d9b1a8f0 src/org/openbravo/dal/core/OBContext.java --- a/src/org/openbravo/dal/core/OBContext.java Sun Mar 11 13:10:30 2018 +0100 +++ b/src/org/openbravo/dal/core/OBContext.java Thu Mar 15 10:44:22 2018 +0100 @@ -659,9 +659,11 @@ "select o.id from " + Organization.class.getName() + " o, " + RoleOrganization.class.getName() + " roa where o." + Organization.PROPERTY_ID + "=roa." + RoleOrganization.PROPERTY_ORGANIZATION + "." + Organization.PROPERTY_ID - + " and roa." + RoleOrganization.PROPERTY_ROLE + "." + Organization.PROPERTY_ID + "='" - + targetRole.getId() + "' and roa." + RoleOrganization.PROPERTY_ACTIVE + "='Y' and o." - + Organization.PROPERTY_ACTIVE + "='" + propertyActive + "'"); + + " and roa." + RoleOrganization.PROPERTY_ROLE + "." + Organization.PROPERTY_ID + "= :targetRoleId" + + " and roa." + RoleOrganization.PROPERTY_ACTIVE + "='Y' and o." + + Organization.PROPERTY_ACTIVE + "= :active"); + qry.setString("targetRoleId", targetRole.getId()); + qry.setString("active", propertyActive); @SuppressWarnings("unchecked") List<String> currentOrgList = qry.list(); @@ -679,8 +681,8 @@ private List<String> getOrganizations(Client client) { final Query qry = SessionHandler.getInstance().createQuery( "select o.id from " + Organization.class.getName() + " o where " + "o." - + Organization.PROPERTY_CLIENT + "=? and o." + Organization.PROPERTY_ACTIVE + "='Y'"); - qry.setParameter(0, client); + + Organization.PROPERTY_CLIENT + "=:clientId and o." + Organization.PROPERTY_ACTIVE + "='Y'"); + qry.setParameter("clientId", client); organizationList = qry.list(); return organizationList; } @@ -886,18 +888,22 @@ // to be // selected. if (roleId != null) { + Map<String, String> params = new HashMap<>(); + params.put("roleId", roleId); final Role r = getOne(Role.class, "select r from " + Role.class.getName() + " r where " - + " r." + Role.PROPERTY_ID + "='" + roleId + "'"); + + " r." + Role.PROPERTY_ID + "=:roleId", params, true); setRole(r); } else if (getUser().getDefaultRole() != null && getUser().getDefaultRole().isActive()) { setRole(getUser().getDefaultRole()); } else { + Map<String, String> params = new HashMap<>(); + params.put("userId", u.getId()); final UserRoles ur = getOne(UserRoles.class, "select ur from " + UserRoles.class.getName() + " ur where " + " ur." + UserRoles.PROPERTY_USERCONTACT + "." + User.PROPERTY_ID - + "='" + u.getId() + "' and ur." + UserRoles.PROPERTY_ACTIVE + "='Y' and ur." + + "=:userId and ur." + UserRoles.PROPERTY_ACTIVE + "='Y' and ur." + UserRoles.PROPERTY_ROLE + "." + Role.PROPERTY_ACTIVE + "='Y' order by ur." - + UserRoles.PROPERTY_ROLE + "." + Role.PROPERTY_ID + " asc", false); + + UserRoles.PROPERTY_ROLE + "." + Role.PROPERTY_ID + " asc", params, false); if (ur == null) { throw new OBSecurityException( "Your user is not assigned to a Role and it is required to login into Openbravo. Ask the Security Administrator"); @@ -909,21 +915,26 @@ Check.isNotNull(getRole(), "Role may not be null"); if (orgId != null) { - final Organization o = getOne(Organization.class, - "select r from " + Organization.class.getName() + " r where " + " r." - + Organization.PROPERTY_ID + "='" + orgId + "'"); + Map<String, String> params = new HashMap<>(); + params.put("orgId", orgId); + final Organization o = getOne(Organization.class, "select r from " + + Organization.class.getName() + " r where " + " r." + Organization.PROPERTY_ID + "=:orgId", + params, true); setCurrentOrganization(o); } else if (getUser().getDefaultOrganization() != null && getUser().getDefaultOrganization().isActive()) { setCurrentOrganization(getUser().getDefaultOrganization()); } else { - final RoleOrganization roa = getOne(RoleOrganization.class, "select roa from " - + RoleOrganization.class.getName() + " roa where roa." + RoleOrganization.PROPERTY_ROLE - + "." + Organization.PROPERTY_ID + "='" + getRole().getId() + "' and roa." - + RoleOrganization.PROPERTY_ACTIVE + "='Y' and roa." - + RoleOrganization.PROPERTY_ORGANIZATION + "." + Organization.PROPERTY_ACTIVE - + "='Y' order by roa." + RoleOrganization.PROPERTY_ORGANIZATION + "." - + Organization.PROPERTY_ID + " desc", false); + Map<String, String> params = new HashMap<>(); + params.put("roleId", getRole().getId()); + final RoleOrganization roa = getOne(RoleOrganization.class, + "select roa from " + RoleOrganization.class.getName() + " roa where roa." + + RoleOrganization.PROPERTY_ROLE + "." + Organization.PROPERTY_ID + "=:roleId and roa." + + RoleOrganization.PROPERTY_ACTIVE + "='Y' and roa." + + RoleOrganization.PROPERTY_ORGANIZATION + "." + Organization.PROPERTY_ACTIVE + + "='Y' order by roa." + RoleOrganization.PROPERTY_ORGANIZATION + "." + + Organization.PROPERTY_ID + " desc", + params, false); Hibernate.initialize(roa.getOrganization()); setCurrentOrganization(roa.getOrganization()); @@ -950,8 +961,10 @@ } if (localClientId != null) { + Map<String, String> params = new HashMap<>(); + params.put("clientId", localClientId); final Client c = getOne(Client.class, "select r from " + Client.class.getName() - + " r where " + " r." + Client.PROPERTY_ID + "='" + localClientId + "'"); + + " r where " + " r." + Client.PROPERTY_ID + "=:clientId", params, true); setCurrentClient(c); } else if (getUser().getDefaultClient() != null && getUser().getDefaultClient().isActive()) { setCurrentClient(getUser().getDefaultClient()); @@ -1038,13 +1051,11 @@ return true; } - private <T extends Object> T getOne(Class<T> clz, String qryStr) { - return getOne(clz, qryStr, true); - } - - @SuppressWarnings("unchecked") - private <T extends Object> T getOne(Class<T> clz, String qryStr, boolean doCheck) { + @SuppressWarnings({ "unchecked" }) + private <T extends Object> T getOne(Class<T> clz, String qryStr, Map<String, String> parameters, + boolean doCheck) { final Query qry = SessionHandler.getInstance().createQuery(qryStr); + qry.setProperties(parameters); qry.setMaxResults(1); final List<?> result = qry.list(); if (doCheck && result.size() != 1) { diff -r 636948c66791 -r c6e6d9b1a8f0 src/org/openbravo/dal/security/EntityAccessChecker.java --- a/src/org/openbravo/dal/security/EntityAccessChecker.java Sun Mar 11 13:10:30 2018 +0100 +++ b/src/org/openbravo/dal/security/EntityAccessChecker.java Thu Mar 15 10:44:22 2018 +0100 @@ -231,9 +231,11 @@ // and take into account table access final String tafQryStr = "select ta from " + TableAccess.class.getName() - + " ta where role.id='" + getRoleId() + "'"; + + " ta where role.id= :roleId"; + Query tafQry = SessionHandler.getInstance().createQuery(tafQryStr); + tafQry.setString("roleId", getRoleId()); @SuppressWarnings("unchecked") - final List<TableAccess> tas = SessionHandler.getInstance().createQuery(tafQryStr).list(); + final List<TableAccess> tas = tafQry.list(); for (final TableAccess ta : tas) { final String tableName = ta.getTable().getName(); final Entity e = mp.getEntity(tableName); @@ -304,10 +306,12 @@ // and take into account explicit process access final String processAccessQryStr = "select p.obuiappProcess.id from " - + ProcessAccess.class.getName() + " p where p.role.id='" + getRoleId() + "'"; + + ProcessAccess.class.getName() + " p where p.role.id= :roleId"; + Query processAccessQry = SessionHandler.getInstance() + .createQuery(processAccessQryStr); + processAccessQry.setString("roleId", getRoleId()); @SuppressWarnings("unchecked") - final List<String> processAccessQuery = SessionHandler.getInstance() - .createQuery(processAccessQryStr).list(); + final List<String> processAccessQuery = processAccessQry.list(); for (final String processAccess : processAccessQuery) { processes.add(processAccess); } ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
