diff --git a/TODO.txt b/TODO.txt index 642591ef66b..4072f12bfa3 100644 --- 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 index c346bd79dae..710b6008764 100644 --- 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,7 +32,7 @@ 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 ); } /** @@ -39,7 +40,7 @@ public inherited sharing class ortoo_UnitOfWorkFactory extends fflib_Application * 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 objectTypes, fflib_SObjectUnitOfWork.IDML dml ) + public override fflib_ISObjectUnitOfWork newInstance( List 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 index d770da338e0..12761b57f7f 100644 --- 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 new file mode 100644 index 00000000000..49adee61777 --- /dev/null +++ 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 idmlBySobjectType; + + public PerSobjectTypeDmlConfiguration() + { + idmlBySobjectType = new Map(); + } + + 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 objList ) + { + getIdmlForSobjectType( objList ).dmlInsert( objList ); + } + + public void dmlUpdate( List objList ) + { + getIdmlForSobjectType( objList ).dmlUpdate( objList ); + } + + public void dmlDelete( List objList ) + { + getIdmlForSobjectType( objList ).dmlDelete( objList ); + } + + public void eventPublish( List objList ) + { + getIdmlForSobjectType( objList ).eventPublish( objList ); + } + + public void emptyRecycleBin( List objList ) + { + getIdmlForSobjectType( objList ).emptyRecycleBin( objList ); + } + + private fflib_SobjectUnitOfWork.Idml getIdmlForSobjectType( List 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 new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/main/default/classes/PerSobjectTypeDmlConfiguration.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/main/default/classes/SecureDml.cls b/framework/main/default/classes/SecureDml.cls index c5228e7c515..dcdb4c954b1 100644 --- 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 objList ); void handleUnableToPublishEvents( List 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 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 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 originalList, List 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 objList ) {} // NOPMD: intentional 'null object' that does nothing public void handleUnableToUpdateRecords( List objList ) {} // NOPMD: intentional 'null object' that does nothing public void handleUnableToDeleteRecords( List objList ) {} // NOPMD: intentional 'null object' that does nothing public void handleUnableToPublishEvents( List 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 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 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> removedFields = securityDecision.getRemovedFields(); + + throw new SecureDmlException( StringUtils.formatLabel( label, new List{ 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 new file mode 100644 index 00000000000..59f638e6e1e --- /dev/null +++ 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 new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/main/default/classes/UnsecureDml.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active +