IDEMPIERE-4268 Web Services : Read miss cross-tenant check (#468)

Extracting from @hengsin pull request 465
Fixes cross tenant readings in MUser MUserDefWin and MWorkflow
This commit is contained in:
Carlos Ruiz 2020-12-16 02:08:11 +01:00 committed by GitHub
parent 03b4a1e878
commit 2ab2211f81
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 65 additions and 53 deletions

View File

@ -383,6 +383,8 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
private MRole[] m_roles = null;
/** Roles of User with Org */
private int m_rolesAD_Org_ID = -1;
/** AD_Client_ID for m_roles above */
private int m_rolesAD_Client_ID = -1;
/** Is Administrator */
private Boolean m_isAdministrator = null;
/** User Access Rights */
@ -787,7 +789,7 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
*/
public MRole[] getRoles (int AD_Org_ID)
{
if (m_roles != null && m_rolesAD_Org_ID == AD_Org_ID)
if (m_roles != null && m_rolesAD_Org_ID == AD_Org_ID && m_rolesAD_Client_ID == Env.getAD_Client_ID(Env.getCtx()))
return m_roles;
ArrayList<MRole> list = new ArrayList<MRole>();
@ -796,7 +798,7 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
// are found but also roles which delegate org access to the user level where
// this user has access to the org in question
String sql = "SELECT * FROM AD_Role r "
+ "WHERE r.IsActive='Y'"
+ "WHERE r.IsActive='Y' AND r.AD_Client_ID IN (0, ?) "
+ " AND EXISTS (SELECT * FROM AD_User_Roles ur"
+ " WHERE r.AD_Role_ID=ur.AD_Role_ID AND ur.IsActive='Y' AND ur.AD_User_ID=?) "
+ " AND ( ( r.isaccessallorgs = 'Y' ) OR "
@ -817,18 +819,14 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
try
{
pstmt = DB.prepareStatement (sql, get_TrxName());
pstmt.setInt (1, getAD_User_ID());
pstmt.setInt (2, AD_Org_ID);
pstmt.setInt (3, getAD_User_ID());
pstmt.setInt (4, AD_Org_ID);
pstmt.setInt (1, Env.getAD_Client_ID(Env.getCtx()));
pstmt.setInt (2, getAD_User_ID());
pstmt.setInt (3, AD_Org_ID);
pstmt.setInt (4, getAD_User_ID());
pstmt.setInt (5, AD_Org_ID);
rs = pstmt.executeQuery ();
try {
PO.setCrossTenantSafe();
while (rs.next ())
list.add (new MRole(Env.getCtx(), rs, get_TrxName()));
} finally {
PO.clearCrossTenantSafe();
}
while (rs.next ())
list.add (new MRole(Env.getCtx(), rs, get_TrxName()));
}
catch (Exception e)
{
@ -844,6 +842,7 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
list.stream().forEach(e -> e.markImmutable());
m_rolesAD_Org_ID = AD_Org_ID;
m_rolesAD_Client_ID = Env.getAD_Client_ID(Env.getCtx());
m_roles = new MRole[list.size()];
list.toArray (m_roles);
return m_roles;

View File

@ -16,7 +16,9 @@ package org.compiere.model;
import java.sql.ResultSet;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import org.compiere.util.Env;
@ -35,8 +37,7 @@ public class MUserDefWin extends X_AD_UserDef_Win implements ImmutablePOSupport
*
*/
private static final long serialVersionUID = -7542708120229671875L;
private volatile static List<MUserDefWin> m_fullList = null;
private static final Object m_fullListLock = new Object();
private static final Map<Integer, List<MUserDefWin>> m_fullMap = new HashMap<Integer, List<MUserDefWin>>();
/**
* Standard constructor.
@ -104,26 +105,25 @@ public class MUserDefWin extends X_AD_UserDef_Win implements ImmutablePOSupport
*/
private static MUserDefWin[] getAll (Properties ctx, int window_ID )
{
synchronized (m_fullListLock) {
if (m_fullList == null) {
try {
PO.setCrossTenantSafe();
m_fullList = new Query(ctx, MUserDefWin.Table_Name, null, null)
.setOnlyActiveRecords(true)
.list();
} finally {
PO.clearCrossTenantSafe();
}
List<MUserDefWin> fullList = null;
synchronized (m_fullMap) {
fullList = m_fullMap.get(Env.getAD_Client_ID(ctx));
if (fullList == null) {
fullList = new Query(ctx, MUserDefWin.Table_Name, null, null)
.setOnlyActiveRecords(true)
.setClient_ID()
.list();
m_fullMap.put(Env.getAD_Client_ID(ctx), fullList);
}
}
if (m_fullList.size() == 0) {
if (fullList.size() == 0) {
return null;
}
List<MUserDefWin> list = new ArrayList<MUserDefWin>();
for (MUserDefWin udw : m_fullList) {
for (MUserDefWin udw : fullList) {
if (udw.getAD_Window_ID() == window_ID
&& udw.getAD_Client_ID() == Env.getAD_Client_ID(ctx)
&& (udw.getAD_Language() == null || udw.getAD_Language().equals(Env.getAD_Language(ctx)))
@ -240,13 +240,17 @@ public class MUserDefWin extends X_AD_UserDef_Win implements ImmutablePOSupport
@Override
protected boolean beforeSave(boolean newRecord) {
m_fullList = null;
synchronized (m_fullMap) {
m_fullMap.remove(getAD_Client_ID());
}
return true;
}
@Override
protected boolean beforeDelete() {
m_fullList = null;
synchronized (m_fullMap) {
m_fullMap.remove(getAD_Client_ID());
}
return true;
}

View File

@ -23,7 +23,9 @@ import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.logging.Level;
import java.util.stream.Collectors;
@ -125,46 +127,43 @@ public class MWorkflow extends X_AD_Workflow implements ImmutablePOSupport
, String trxName //Bug 1568766 Trx should be kept all along the road
)
{
String key = "C" + AD_Client_ID + "T" + AD_Table_ID;
// Reload
if (s_cacheDocValue.isReset())
Map<Integer, MWorkflow[]> cachedMap = s_cacheDocValue.get(AD_Client_ID);
if (cachedMap == null)
{
final String whereClause = "WorkflowType=? AND IsValid=?";
final String whereClause = "WorkflowType=? AND IsValid=? AND AD_Client_ID=?";
List<MWorkflow> workflows;
try {
PO.setCrossTenantSafe();
workflows = new Query(ctx, Table_Name, whereClause, trxName)
.setParameters(new Object[]{WORKFLOWTYPE_DocumentValue, true})
.setOnlyActiveRecords(true)
.setOrderBy("AD_Client_ID, AD_Table_ID")
.list();
} finally {
PO.clearCrossTenantSafe();
}
workflows = new Query(ctx, Table_Name, whereClause, trxName)
.setParameters(new Object[]{WORKFLOWTYPE_DocumentValue, true, Env.getAD_Client_ID(ctx)})
.setOnlyActiveRecords(true)
.setOrderBy("AD_Table_ID")
.list();
cachedMap = new HashMap<Integer, MWorkflow[]>();
s_cacheDocValue.put(AD_Client_ID, cachedMap);
ArrayList<MWorkflow> list = new ArrayList<MWorkflow>();
String oldKey = "";
String newKey = null;
int previousTableId = -1;
int currentTableId = -1;
for (MWorkflow wf : workflows)
{
newKey = "C" + wf.getAD_Client_ID() + "T" + wf.getAD_Table_ID();
if (!newKey.equals(oldKey) && list.size() > 0)
currentTableId = wf.getAD_Table_ID();
if (currentTableId != previousTableId && list.size() > 0)
{
s_cacheDocValue.put (oldKey, list.stream().map(e -> {return new MWorkflow(Env.getCtx(), e);}).toArray(MWorkflow[]::new));
cachedMap.put (previousTableId, list.stream().map(e -> {return new MWorkflow(Env.getCtx(), e);}).toArray(MWorkflow[]::new));
list = new ArrayList<MWorkflow>();
}
oldKey = newKey;
previousTableId = currentTableId;
list.add(wf);
}
// Last one
if (list.size() > 0)
{
s_cacheDocValue.put (oldKey, list.stream().map(e -> {return new MWorkflow(Env.getCtx(), e);}).toArray(MWorkflow[]::new));
cachedMap.put (previousTableId, list.stream().map(e -> {return new MWorkflow(Env.getCtx(), e);}).toArray(MWorkflow[]::new));
}
if (s_log.isLoggable(Level.CONFIG)) s_log.config("#" + s_cacheDocValue.size());
if (s_log.isLoggable(Level.CONFIG)) s_log.config("#" + cachedMap.size());
}
// Look for Entry
MWorkflow[] retValue = (MWorkflow[])s_cacheDocValue.get(key);
MWorkflow[] retValue = (MWorkflow[])cachedMap.get(AD_Table_ID);
//hengsin: this is not threadsafe
/*
//set trxName to all workflow instance
@ -180,9 +179,19 @@ public class MWorkflow extends X_AD_Workflow implements ImmutablePOSupport
/** Single Cache */
private static ImmutablePOCache<String,MWorkflow> s_cache = new ImmutablePOCache<String,MWorkflow>(Table_Name, Table_Name+"|Language_Workflow", 20);
private static ImmutablePOCache<String,MWorkflow> s_cache = new ImmutablePOCache<String,MWorkflow>(Table_Name, Table_Name, 20);
/** Document Value Cache */
private static CCache<String,MWorkflow[]> s_cacheDocValue = new CCache<String,MWorkflow[]> (Table_Name, Table_Name+"|AD_Client_Table", 5);
private static final CCache<Integer,Map<Integer, MWorkflow[]>> s_cacheDocValue = new CCache<> (Table_Name, Table_Name+"|DocumentValue", 5) {
/**
* generated serial id
*/
private static final long serialVersionUID = 2548097748351277269L;
@Override
public int reset(int recordId) {
return reset();
}
};
/** Static Logger */
private static CLogger s_log = CLogger.getCLogger (MWorkflow.class);