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

Implement thread safe mechanism
https://mattermost.idempiere.org/idempiere/pl/hmpd8freutbhbgcjr9tu6ntitr

org.adempiere.base/src/org/compiere/model/MClient.java

@@ -105,19 +105,13 @@ public class MClient extends X_AD_Client implements ImmutablePOSupport
 	public static MClient[] getAll (Properties ctx, String orderBy)
 	{
 		List<MClient> list = null;
-		int cid = Env.getAD_Client_ID(Env.getCtx());
 		try {
-			if (cid > 0) {
+			PO.setCrossTenantSafe();
-				// forced potential cross tenant read - requires System client in context
-				Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-			}
 			list = new Query(ctx,I_AD_Client.Table_Name,(String)null,(String)null)
 					.setOrderBy(orderBy)
 					.list();
 		} finally {
-			if (cid > 0) {
+			PO.clearCrossTenantSafe();
-				Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-			}
 		}
 		for(MClient client:list ){
 			s_cache.put (Integer.valueOf(client.getAD_Client_ID()), client, e -> new MClient(Env.getCtx(), e));

org.adempiere.base/src/org/compiere/model/MRegion.java

@@ -54,19 +54,13 @@ public class MRegion extends X_C_Region
 	{
 		s_regions.clear();
 		List<MRegion> regions;
-		int cid = Env.getAD_Client_ID(Env.getCtx());
 		try {
-			if (cid > 0) {
+			PO.setCrossTenantSafe();
-				// forced potential cross tenant read - requires System client in context
-				Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-			}
 			regions = new Query(Env.getCtx(), Table_Name, "", null)
 					.setOnlyActiveRecords(true)
 					.list();
 		} finally {
-			if (cid > 0) {
+			PO.clearCrossTenantSafe();
-				Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-			}
 		}
 		for (MRegion r : regions) {
 			r.markImmutable();

org.adempiere.base/src/org/compiere/model/MUser.java

@@ -822,18 +822,12 @@ public class MUser extends X_AD_User implements ImmutablePOSupport
 			pstmt.setInt (3, getAD_User_ID());
 			pstmt.setInt (4, AD_Org_ID);
 			rs = pstmt.executeQuery ();
-			int cid = Env.getAD_Client_ID(Env.getCtx());
 			try {
-				if (cid > 0) {
+				PO.setCrossTenantSafe();
-					// forced potential cross tenant read - requires System client in context
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-				}
 				while (rs.next ())
 					list.add (new MRole(Env.getCtx(), rs, get_TrxName()));
 			} finally {
-				if (cid > 0) {
+				PO.clearCrossTenantSafe();
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-				}
 			}
 		}
 		catch (Exception e)

org.adempiere.base/src/org/compiere/model/MUserDefWin.java

@@ -104,19 +104,13 @@ public class MUserDefWin extends X_AD_UserDef_Win implements ImmutablePOSupport
 	private static MUserDefWin[] getAll (Properties ctx, int window_ID )
 	{
 		if (m_fullList == null) {
-			int cid = Env.getAD_Client_ID(Env.getCtx());
 			try {
-				if (cid > 0) {
+				PO.setCrossTenantSafe();
-					// forced potential cross tenant read - requires System client in context
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-				}
 				m_fullList = new Query(ctx, MUserDefWin.Table_Name, null, null)
 						.setOnlyActiveRecords(true)
 						.list();
 			} finally {
-				if (cid > 0) {
+				PO.clearCrossTenantSafe();
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-				}
 			}
 		}
 		

org.adempiere.base/src/org/compiere/model/PO.java

@@ -112,7 +112,7 @@ public abstract class PO
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 1556095090658747371L;
+	private static final long serialVersionUID = -7231417421289556724L;
 
 	public static final String LOCAL_TRX_PREFIX = "POSave";
 
@@ -4981,7 +4981,30 @@ public abstract class PO
 			throw new AdempiereException("Context lost");
 	}
 
-	private void checkValidClient(boolean writing) {		
+	/*
+	 * To force a cross tenant safe read/write the client program must write code like this:
+		try {
+			PO.setCrossTenantSafe();
+			// write here the Query.list or PO.saveEx that is cross tenant safe
+		} finally {
+			PO.clearCrossTenantSafe();
+		}
+	 */
+	private static ThreadLocal<Boolean> isSafeCrossTenant = new ThreadLocal<Boolean>() {
+		@Override protected Boolean initialValue() {
+			return Boolean.FALSE;
+		};
+	};
+	public static void setCrossTenantSafe() {
+		isSafeCrossTenant.set(Boolean.TRUE);
+	}
+	public static void clearCrossTenantSafe() {
+		isSafeCrossTenant.set(Boolean.FALSE);
+	}
+
+	private void checkValidClient(boolean writing) {
+		if (isSafeCrossTenant.get())
+			return;
 		int envClientID = Env.getAD_Client_ID(getCtx());
 		// processes running from system client can read/write always
 		if (envClientID > 0) {

org.adempiere.base/src/org/compiere/wf/MWorkflow.java

@@ -131,21 +131,15 @@ public class MWorkflow extends X_AD_Workflow implements ImmutablePOSupport
 		{
 			final String whereClause = "WorkflowType=? AND IsValid=?";
 			List<MWorkflow> workflows;
-			int cid = Env.getAD_Client_ID(Env.getCtx());
 			try {
-				if (cid > 0) {
+				PO.setCrossTenantSafe();
-					// forced potential cross tenant read - requires System client in context
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-				}
 				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 {
-				if (cid > 0) {
+				PO.clearCrossTenantSafe();
-					Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-				}
 			}
 			ArrayList<MWorkflow> list = new ArrayList<MWorkflow>();
 			String oldKey = "";

org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/util/UserPreference.java

@@ -18,6 +18,7 @@ import java.util.Properties;
 
 import org.compiere.model.I_AD_Preference;
 import org.compiere.model.MPreference;
+import org.compiere.model.PO;
 import org.compiere.model.Query;
 import org.compiere.util.CLogger;
 import org.compiere.util.Env;
@@ -120,17 +121,12 @@ public final class UserPreference implements Serializable {
 					}
 				}
 				
-				int cid = Env.getAD_Client_ID(Env.getCtx());
+				try {
-				try {				
+					PO.setCrossTenantSafe();
-					if (preference.getAD_Client_ID() == 0 && cid > 0) {
-						Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, 0);
-					}
 					preference.setValue(value);
 					preference.saveEx();
 				} finally {
-					if (preference.getAD_Client_ID() == 0 && cid > 0) {
+					PO.clearCrossTenantSafe();
-						Env.setContext(Env.getCtx(), Env.AD_CLIENT_ID, cid);
-					}
 				}
 			}
 		}