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

Implement cross tenant check on foreign keys for insert and update

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

@@ -112,7 +112,7 @@ public abstract class PO
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = -2086165095004944867L;
+	private static final long serialVersionUID = 1556095090658747371L;
 
 	public static final String LOCAL_TRX_PREFIX = "POSave";
 
@@ -1943,6 +1943,8 @@ public abstract class PO
 
 	/**	Cache						*/
 	private static CCache<String,String> trl_cache	= new CCache<String,String>("PO_Trl", 5);
+	/** Cache for foreign keys */
+	private static CCache<Integer,List<ValueNamePair>> fks_cache	= new CCache<Integer,List<ValueNamePair>>("FKs", 5);
 
 	public String get_Translation (String columnName, String AD_Language)
 	{
@@ -5000,4 +5002,92 @@ public abstract class PO
 		}
 	}
 
+	/**
+	 * Validate Foreign keys for cross tenant
+	 * to be called programmatically before saving in programs that can receive arbitrary values in IDs
+	 * This is an expensive operation in terms of database, use it wisely
+	 * 
+	 * TODO: there is huge room for performance improvement, for example:
+	 * - caching the valid values found on foreign tables
+	 * - caching the column ID of the foreign column
+	 * - caching the systemAccess
+	 *  
+	 * @return true if all the foreign keys are valid
+	 */
+	public boolean validForeignKeys() {
+		List<ValueNamePair> fks = getForeignColumnIdxs();
+		if (fks == null) {
+			return true;
+		}
+		for (ValueNamePair vnp : fks) {
+			String fkcol = vnp.getID();
+			String fktab = vnp.getName();
+			int index = get_ColumnIndex(fkcol); 
+			if (is_new() || is_ValueChanged(index)) {
+				int fkval = get_ValueAsInt(index);
+				if (fkval > 0) {
+					MTable ft = MTable.get(getCtx(), fktab);
+					boolean systemAccess = false;
+					String accessLevel = ft.getAccessLevel();
+					if (   MTable.ACCESSLEVEL_All.equals(accessLevel)
+						|| MTable.ACCESSLEVEL_SystemOnly.equals(accessLevel)
+						|| MTable.ACCESSLEVEL_SystemPlusClient.equals(accessLevel)) {
+						systemAccess = true;
+					}
+					StringBuilder sql = new StringBuilder("SELECT AD_Client_ID FROM ")
+							.append(fktab)
+							.append(" WHERE ")
+							.append(ft.getKeyColumns()[0])
+							.append("=?");
+					int pocid = DB.getSQLValue(get_TrxName(), sql.toString(), fkval);
+					if (pocid < 0) {
+						log.saveError("Error", "Foreign ID " + fkval + " not found in " + fkcol);
+						return false;
+					}
+					if (pocid == 0 && !systemAccess) {
+						log.saveError("Error", "System ID " + fkval + " cannot be used in " + fkcol);
+						return false;
+					}
+					int curcid = Env.getAD_Client_ID(getCtx());
+					if (pocid > 0 && pocid != curcid) {
+						log.saveError("Error", "Cross tenant ID " + fkval + " not allowed in " + fkcol);
+						return false;
+					}
+				}
+			}
+		}
+		return true;
+	}
+
+	/**
+	 * Returns a list of indexes for the foreign columns, null if none
+	 * @return array of int indexes
+	 */
+	private List<ValueNamePair> getForeignColumnIdxs() {
+		List<ValueNamePair> retValue;
+		if (fks_cache.containsKey(get_Table_ID())) {
+			retValue = fks_cache.get(get_Table_ID());
+			return retValue;
+		}
+		retValue = new ArrayList<ValueNamePair>();
+		int size = get_ColumnCount();
+		for (int i = 0; i < size; i++) {
+			int dt = p_info.getColumnDisplayType(i);
+			if (dt != DisplayType.ID && DisplayType.isID(dt)) {
+				MColumn col = MColumn.get(p_info.getColumn(i).AD_Column_ID);
+				if ("AD_Client_ID".equals(col.getColumnName())) {
+					// ad_client_id is verified with checkValidClient
+					continue;
+				}
+				String refTable = col.getReferenceTableName();
+				retValue.add(new ValueNamePair(col.getColumnName(), refTable));
+			}
+		}
+		if (retValue.size() == 0) {
+			retValue = null;
+		}
+		fks_cache.put(get_Table_ID(), retValue);
+		return retValue;
+	}
+
 }   //  PO

org.idempiere.webservices/WEB-INF/src/org/idempiere/adinterface/ModelADServiceImpl.java

@@ -822,6 +822,9 @@ public class ModelADServiceImpl extends AbstractService implements ModelADServic
 			if (retResp != null)
 				return retResp;
 
+			if (!po.validForeignKeys())
+				return rollbackAndSetError(trx, resp, ret, true, "Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));
+
 	    	if (!po.save())
 	    		return rollbackAndSetError(trx, resp, ret, true, "Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));
 	
@@ -1023,6 +1026,9 @@ public class ModelADServiceImpl extends AbstractService implements ModelADServic
 			if (retResp != null)
 				return retResp;
 	
+			if (!po.validForeignKeys())
+				return rollbackAndSetError(trx, resp, ret, true, "Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));
+
 			if (!po.save())
 				return rollbackAndSetError(trx, resp, ret, true,
 						"Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));
@@ -1295,6 +1301,9 @@ public class ModelADServiceImpl extends AbstractService implements ModelADServic
 			if (retResp != null)
 				return retResp;
 
+			if (!po.validForeignKeys())
+				return rollbackAndSetError(trx, resp, ret, true, "Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));
+
 	    	if (!po.save())
 	    		return rollbackAndSetError(trx, resp, ret, true, "Cannot save record in " + tableName + ": " + CLogger.retrieveErrorString("no log message"));