Default UoW now is secure - both FLS and CUD

Added class to allow for ability to specify different DML security for different SObject Types
OrtooApps · Dec 8, 2021 · 7e057f4 · 7e057f4
1 parent bdba10d
commit 7e057f4
Show file tree

Hide file tree

Showing 8 changed files with 135 additions and 21 deletions.
diff --git a/TODO.txt b/TODO.txt
@@ -6,6 +6,9 @@ Licenses that are needed with the source code and binary:
 
 Look at the use of 'MockDatabase' in fflib
 
+Introduce ortoo_Selector - make it secure by default
+Look at the interface required to make Unsecured DML calls
+
 * To finalise the core architecture:
     * Decide on FLS standards
     * Do we need to have a non all-or-nothing version of commitWork?

diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_UnitOfWorkFactory.cls b/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_UnitOfWorkFactory.cls
@@ -1,5 +1,6 @@
 public inherited sharing class ortoo_UnitOfWorkFactory extends fflib_Application.UnitOfWorkFactory // NOPMD: specified a mini-namespace to differentiate from fflib versions
 {
+    private static fflib_SobjectUnitOfWork.Idml defaultIdml = new SecureDml( new SecureDml.ErrorOnInaccessibleFieldsHandler() );
     /*
     * Constructs a Unit Of Work factory with no default configuration
     *
@@ -31,15 +32,15 @@ public inherited sharing class ortoo_UnitOfWorkFactory extends fflib_Application
         {
             return m_mockUow;
         }
-        return new ortoo_SObjectUnitOfWork( m_objectTypes );
+        return new ortoo_SObjectUnitOfWork( m_objectTypes, defaultIdml );
     }
 
     /**
      * Returns a new SObjectUnitOfWork configured with the
      *   SObjectType list provided in the constructor, returns a Mock implementation
      *   if set via the setMock method
      **/
-    public override fflib_ISObjectUnitOfWork newInstance( fflib_SObjectUnitOfWork.IDML dml )
+    public override fflib_ISObjectUnitOfWork newInstance( fflib_SObjectUnitOfWork.Idml dml )
     {
         if ( m_mockUow != null )
         {
@@ -62,7 +63,7 @@ public inherited sharing class ortoo_UnitOfWorkFactory extends fflib_Application
         {
             return m_mockUow;
         }
-        return new ortoo_SObjectUnitOfWork( objectTypes );
+        return new ortoo_SObjectUnitOfWork( objectTypes, defaultIdml );
     }
 
     /**
@@ -73,7 +74,7 @@ public inherited sharing class ortoo_UnitOfWorkFactory extends fflib_Application
      * @remark If mock is set, the list of SObjectType in the mock could be different
      *         then the list of SObjectType specified in this method call
      **/
-    public override fflib_ISObjectUnitOfWork newInstance( List<SObjectType> objectTypes, fflib_SObjectUnitOfWork.IDML dml )
+    public override fflib_ISObjectUnitOfWork newInstance( List<SObjectType> objectTypes, fflib_SObjectUnitOfWork.Idml dml )
     {
         if ( m_mockUow != null )
         {

diff --git a/framework/default/sobject-fabricator/classes/ortoo_FabricatedSObjectRegister.cls b/framework/default/sobject-fabricator/classes/ortoo_FabricatedSObjectRegister.cls
@@ -104,7 +104,7 @@ public class ortoo_FabricatedSObjectRegister {
 
 		public void persist()
 		{
-			ortoo_SObjectUnitOfWork uow = (ortoo_SObjectUnitOfWork)Application.UNIT_OF_WORK.newInstance( getOrderOfInserts() );
+			ortoo_SObjectUnitOfWork uow = (ortoo_SObjectUnitOfWork)Application.UNIT_OF_WORK.newInstance( getOrderOfInserts(), new UnsecureDml() );
 
 			buildObjectsByFabricated();
 			registerInserts( uow );

diff --git a/framework/main/default/classes/PerSobjectTypeDmlConfiguration.cls b/framework/main/default/classes/PerSobjectTypeDmlConfiguration.cls
@@ -0,0 +1,63 @@
+public with sharing class PerSobjectTypeDmlConfiguration implements fflib_SobjectUnitOfWork.Idml
+{
+    fflib_SobjectUnitOfWork.Idml defaultIdml;
+    Map<SobjectType,fflib_SobjectUnitOfWork.Idml> idmlBySobjectType;
+
+    public PerSobjectTypeDmlConfiguration()
+    {
+        idmlBySobjectType = new Map<SobjectType,fflib_SobjectUnitOfWork.Idml>();
+    }
+
+    public PerSobjectTypeDmlConfiguration setDefault( fflib_SobjectUnitOfWork.Idml idml )
+    {
+        defaultIdml = idml;
+        return this;
+    }
+
+    public PerSobjectTypeDmlConfiguration addIdml( SobjectType sobjectType, fflib_SobjectUnitOfWork.Idml idml )
+    {
+        idmlBySobjectType.put( sobjectType, idml );
+        return this;
+    }
+
+    public void dmlInsert( List<SObject> objList )
+    {
+        getIdmlForSobjectType( objList ).dmlInsert( objList );
+    }
+
+    public void dmlUpdate( List<SObject> objList )
+    {
+        getIdmlForSobjectType( objList ).dmlUpdate( objList );
+    }
+
+    public void dmlDelete( List<SObject> objList )
+    {
+        getIdmlForSobjectType( objList ).dmlDelete( objList );
+    }
+
+    public void eventPublish( List<SObject> objList )
+    {
+        getIdmlForSobjectType( objList ).eventPublish( objList );
+    }
+
+    public void emptyRecycleBin( List<SObject> objList )
+    {
+        getIdmlForSobjectType( objList ).emptyRecycleBin( objList );
+    }
+
+    private fflib_SobjectUnitOfWork.Idml getIdmlForSobjectType( List<SObject> objList )
+    {
+        fflib_SobjectUnitOfWork.Idml idml;
+
+        if ( !objList.isEmpty() )
+        {
+            idml = idmlBySobjectType.get( objList[0].getSObjectType() );
+        }
+
+        if ( idml == null )
+        {
+            idml = defaultIdml;
+        }
+        return idml;
+    }
+}
diff --git a/framework/main/default/classes/PerSobjectTypeDmlConfiguration.cls-meta.xml b/framework/main/default/classes/PerSobjectTypeDmlConfiguration.cls-meta.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ApexClass xmlns="http://soap.sforce.com/2006/04/metadata">
+    <apiVersion>52.0</apiVersion>
+    <status>Active</status>
+</ApexClass>
diff --git a/framework/main/default/classes/SecureDml.cls b/framework/main/default/classes/SecureDml.cls
@@ -1,6 +1,7 @@
 // TODO: document
 // TODO: test
 // TODO: ensure the default is constructed with one - potentially configure it
+// TODO: split field level checking and object level checking?
 
 public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleDML implements Idml
 {
@@ -13,7 +14,7 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
         void handleUnableToDeleteRecords( List<SObject> objList );
         void handleUnableToPublishEvents( List<SObject> objList );
 
-        void reportInaccessibleFields( String operation, SObjectAccessDecision securityDecision );
+        void handleInaccessibleFields( String operation, SObjectAccessDecision securityDecision );
     }
 
     InaccessibleHandler inaccessibleHandler;
@@ -29,15 +30,20 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
         {
             return;
         }
-        if ( ! SobjectUtils.getSobjectDescribeResult( objList[0] ).isCreateable() )
+        if ( ! SobjectUtils.getSobjectDescribeResult( objList[0] ).isCreateable() ) // TODO: probably don't need this
         {
             inaccessibleHandler.handleUnableToInsertRecords( objList );
             return;
         }
 
         SObjectAccessDecision securityDecision = Security.stripInaccessible( AccessType.CREATABLE, objList );
-        inaccessibleHandler.reportInaccessibleFields( 'insert', securityDecision );
-        insert securityDecision.getRecords();
+        if ( ! securityDecision.getRemovedFields().isEmpty() )
+        {
+            inaccessibleHandler.handleInaccessibleFields( 'insert', securityDecision );
+            replaceList( objList, securityDecision.getRecords() );
+        }
+
+        insert objList;
     }
 
     public override void dmlUpdate( List<SObject> objList )
@@ -53,8 +59,13 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
         }
 
         SObjectAccessDecision securityDecision = Security.stripInaccessible( AccessType.UPDATABLE, objList );
-        inaccessibleHandler.reportInaccessibleFields( 'update', securityDecision );
-        update securityDecision.getRecords();
+
+        if ( ! securityDecision.getRemovedFields().isEmpty() )
+        {
+            inaccessibleHandler.handleInaccessibleFields( 'update', securityDecision );
+            replaceList( objList, securityDecision.getRecords() );
+        }
+        update objList;
     }
 
     public override void dmlDelete( List<SObject> objList )
@@ -84,22 +95,38 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
             return;
         }
         SObjectAccessDecision securityDecision = Security.stripInaccessible( AccessType.CREATABLE, objList );
-        inaccessibleHandler.reportInaccessibleFields( 'publish', securityDecision );
-        EventBus.publish(objList);
+
+        if ( ! securityDecision.getRemovedFields().isEmpty() )
+        {
+            inaccessibleHandler.handleInaccessibleFields( 'publish', securityDecision );
+            replaceList( objList, securityDecision.getRecords() );
+        }
+        EventBus.publish( objList );
     }
 
-    public inherited sharing class NullInaccessibleHandler implements InaccessibleHandler
+    private static void replaceList( List<Sobject> originalList, List<Sobject> newList )
+    {
+        Contract.requires( originalList != null, 'replaceList called with a null originalList' );
+        Contract.requires( newList != null, 'replaceList called with a null newList' );
+        Contract.requires( originalList.size() == newList.size(), 'replaceList called with lists that are different sizes' );
+
+        for ( Integer i; i < originalList.size(); i++ )
+        {
+            originalList[i] = newList[i];
+        }
+    }
+
+    public inherited sharing class SwallowErrorOnInaccessibleHandler implements InaccessibleHandler
     {
         public void handleUnableToInsertRecords( List<SObject> objList ) {} // NOPMD: intentional 'null object' that does nothing
         public void handleUnableToUpdateRecords( List<SObject> objList ) {} // NOPMD: intentional 'null object' that does nothing
         public void handleUnableToDeleteRecords( List<SObject> objList ) {} // NOPMD: intentional 'null object' that does nothing
         public void handleUnableToPublishEvents( List<SObject> objList ) {} // NOPMD: intentional 'null object' that does nothing
 
-        public void reportInaccessibleFields( String operation, SObjectAccessDecision securityDecision ) {} // NOPMD: intentional 'null object' that does nothing
+        public void handleInaccessibleFields( String operation, SObjectAccessDecision securityDecision ) {} // NOPMD: intentional 'null object' that does nothing
     }
 
-    // TODO: give this a better name
-    public inherited sharing virtual class ErrorOnUnableToHandler implements InaccessibleHandler
+    public inherited sharing virtual class ErrorOnUnableToCudHandler implements InaccessibleHandler
     {
         public void handleUnableToInsertRecords( List<SObject> objList )
         {
@@ -118,7 +145,7 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
             throwUnableException( '00004', 'Attempted to publish {0} events without the required permission', objList );
         }
 
-        public virtual void reportInaccessibleFields( String operation, SObjectAccessDecision securityDecision ) {} // NOPMD: intentionally left empty
+        public virtual void handleInaccessibleFields( String operation, SObjectAccessDecision securityDecision ) {} // NOPMD: intentionally left empty
 
         private void throwUnableException( String errorCode, String label, List<SObject> objList )
         {
@@ -130,11 +157,17 @@ public inherited sharing class SecureDml extends fflib_SobjectUnitOfWork.SimpleD
         }
     }
 
-    public inherited sharing virtual class ErrorOnInaccessibleFieldsHandler extends ErrorOnUnableToHandler
+    public inherited sharing virtual class ErrorOnInaccessibleFieldsHandler extends ErrorOnUnableToCudHandler
     {
-        public override void reportInaccessibleFields( String operation, SObjectAccessDecision securityDecision )
+        public override void handleInaccessibleFields( String operation, SObjectAccessDecision securityDecision )
         {
-            // TODO: implemednt
+            String label = 'Attempted to {0} with fields that are not accessible: {1}';
+            Map<String,Set<String>> removedFields = securityDecision.getRemovedFields();
+
+            throw new SecureDmlException( StringUtils.formatLabel( label, new List<String>{ operation, removedFields.toString() } ) )
+                        .setErrorCode( '0000' )
+                        .addContext( 'removedFields', removedFields )
+                        .addContext( 'securityDecision', securityDecision );
         }
     }
 }
diff --git a/framework/main/default/classes/UnsecureDml.cls b/framework/main/default/classes/UnsecureDml.cls
@@ -0,0 +1,4 @@
+/**
+ *  Class is a rename of FFLIB's SimpleDml to be clear about what it actually does - it provides Unsecure access to DML operations
+ */
+public inherited sharing class UnsecureDml extends fflib_SobjectUnitOfWork.SimpleDML implements Idml {} // NOPMD: intentionally left blank
diff --git a/framework/main/default/classes/UnsecureDml.cls-meta.xml b/framework/main/default/classes/UnsecureDml.cls-meta.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ApexClass xmlns="http://soap.sforce.com/2006/04/metadata">
+    <apiVersion>52.0</apiVersion>
+    <status>Active</status>
+</ApexClass>