IDEMPIERE-3423 MWFActivity missing displaytype validation in set variable nodes

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

@@ -51,7 +51,7 @@ public class MColumn extends X_AD_Column
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 3082823885314140209L;
+	private static final long serialVersionUID = -6914331394933196295L;
 
 	public static MColumn get (Properties ctx, int AD_Column_ID)
 	{
@@ -1133,4 +1133,19 @@ public class MColumn extends X_AD_Column
 		return "";
 	}
 
+	/**
+	 * 	Is Advanced
+	 *	@return true if the column has any field marked as advanced or part of an advanced tab
+	 */
+	public boolean isAdvanced() {
+		final String sql = ""
+				+ "SELECT COUNT(*) "
+				+ "FROM   AD_Tab t "
+				+ "       JOIN AD_Field f ON ( f.AD_Tab_ID = t.AD_Tab_ID ) "
+				+ "WHERE  f.AD_Column_ID = ? "
+				+ "       AND ( t.IsAdvancedTab = 'Y' OR f.IsAdvancedField = 'Y' )";
+		int cnt = DB.getSQLValueEx(get_TrxName(), sql, getAD_Column_ID());
+		return cnt > 0;
+	}
+
 }	//	MColumn

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

@@ -52,6 +52,7 @@ import org.adempiere.exceptions.DBException;
 import org.adempiere.process.UUIDGenerator;
 import org.compiere.Adempiere;
 import org.compiere.acct.Doc;
+import org.compiere.util.AdempiereUserError;
 import org.compiere.util.CCache;
 import org.compiere.util.CLogMgt;
 import org.compiere.util.CLogger;
@@ -1000,7 +1001,7 @@ public abstract class PO
 	{
 		int index = p_info.getColumnIndex(AD_Column_ID);
 		if (index < 0)
-			log.log(Level.SEVERE, "Not found - AD_Column_ID=" + AD_Column_ID);
+			throw new AdempiereUserError("Not found - AD_Column_ID=" + AD_Column_ID);
 		String ColumnName = p_info.getColumnName(index);
 		if (ColumnName.equals("IsApproved"))
 			return set_ValueNoCheck(ColumnName, value);

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

@@ -16,6 +16,8 @@
  *****************************************************************************/
 package org.compiere.wf;
 
+import static org.compiere.model.SystemIDs.MESSAGE_WORKFLOWRESULT;
+
 import java.io.File;
 import java.math.BigDecimal;
 import java.sql.PreparedStatement;
@@ -52,12 +54,12 @@ import org.compiere.model.MUserRoles;
 import org.compiere.model.MWFActivityApprover;
 import org.compiere.model.PO;
 import org.compiere.model.Query;
-import static org.compiere.model.SystemIDs.*;
 import org.compiere.model.X_AD_WF_Activity;
 import org.compiere.print.ReportEngine;
 import org.compiere.process.DocAction;
 import org.compiere.process.ProcessInfo;
 import org.compiere.process.StateEngine;
+import org.compiere.util.CLogger;
 import org.compiere.util.DB;
 import org.compiere.util.DisplayType;
 import org.compiere.util.Env;
@@ -1286,9 +1288,43 @@ public class MWFActivity extends X_AD_WF_Activity implements Runnable
 			dbValue = new Boolean("Y".equals(value));
 		else if (DisplayType.isNumeric(displayType))
 			dbValue = new BigDecimal (value);
+		else if (DisplayType.isID(displayType)) {
+			MColumn column = MColumn.get(Env.getCtx(), getNode().getAD_Column_ID());
+			String referenceTableName = column.getReferenceTableName();
+			if (referenceTableName != null) {
+				MTable refTable = MTable.get(Env.getCtx(), referenceTableName);
+				dbValue = Integer.valueOf(value);
+				boolean validValue = true;
+				PO po = refTable.getPO((Integer)dbValue, trx.getTrxName());
+				if (po == null || po.get_ID() == 0) {
+					// foreign key does not exist
+					validValue = false;
+				}
+				if (validValue && po.getAD_Client_ID() != Env.getAD_Client_ID(Env.getCtx())) {
+					validValue = false;
+					if (po.getAD_Client_ID() == 0) {
+						String accessLevel = refTable.getAccessLevel();
+						if (   MTable.ACCESSLEVEL_All.equals(accessLevel)
+							|| MTable.ACCESSLEVEL_SystemPlusClient.equals(accessLevel)) {
+							// client foreign keys are OK if the table has reference All or System+Client
+							validValue = true;
+						}
+					}
+				}
+				if (! validValue) {
+					throw new Exception("Persistent Object not updated - AD_Table_ID="
+							+ getAD_Table_ID() + ", Record_ID=" + getRecord_ID()
+							+ " - Value=" + value + " is not valid for a foreign key");
+				}
+			}
+		}
 		else
 			dbValue = value;
-		m_po.set_ValueOfColumn(getNode().getAD_Column_ID(), dbValue);
+		if (!m_po.set_ValueOfColumnReturningBoolean(getNode().getAD_Column_ID(), dbValue)) {
+			throw new Exception("Persistent Object not updated - AD_Table_ID="
+					+ getAD_Table_ID() + ", Record_ID=" + getRecord_ID()
+					+ " - Value=" + value + " error : " + CLogger.retrieveErrorString("check logs"));
+		}
 		m_po.saveEx();
 		if (dbValue != null && !dbValue.equals(m_po.get_ValueOfColumn(getNode().getAD_Column_ID())))
 			throw new Exception("Persistent Object not updated - AD_Table_ID="

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

@@ -30,6 +30,7 @@ import java.util.logging.Level;
 import org.adempiere.exceptions.AdempiereException;
 import org.adempiere.exceptions.DBException;
 import org.compiere.model.MColumn;
+import org.compiere.model.MRole;
 import org.compiere.model.Query;
 import org.compiere.model.X_AD_WF_Node;
 import org.compiere.util.CCache;
@@ -603,6 +604,16 @@ public class MWFNode extends X_AD_WF_Node
 				log.saveError("FillMandatory", Msg.getElement(getCtx(), "AttributeValue"));
 				return false;
 			}
+			if (getAD_Column_ID() > 0) {
+				// validate that just advanced roles can manipulate secure content via workflows
+				MColumn column = MColumn.get(getCtx(), getAD_Column_ID());
+				if (column.isSecure() || column.isAdvanced()) {
+					if (! MRole.getDefault().isAccessAdvanced()) {
+						log.saveError("AccessTableNoUpdate", Msg.getElement(getCtx(), column.getColumnName()));
+						return false;
+					}
+				}
+			}
 		}
 		else if (action.equals(ACTION_SubWorkflow))
 		{