Question: How and where to enforce CRUD and FLS in selectors in ISV apps?

[eyavorovenko]

I have question on StackExchange https://salesforce.stackexchange.com/questions/266322 about options that a developer can choose to enforce security on their selectors. Since I am working on an application that we are planning to list on AppExchange, I need to make sure that all security checks are in place.

fflib is a great library that provides a lot of tools, and this variety can also confuse sometimes. I now stuck at a problem of how and where to enforce CRUD and FLS in selectors. This is because there are several ways to do that and I don't know which one is better, or considered as best practice.

First off let's review the default behavior. By default, the base class methods in the fflib_SObjectSelector base class automatically perform Object read security. It's obvious that to pass the security review I must enforce FLS read security. So let's explore what options we have to address this problem:

Approach 1 (Override the default behavior) As Andrew Fawcett goes in his book (Force.com Enterprise Architecture) it's possible to override the default behavior by utilizing the constructor parameters provided by the fflib_SObjectSelector

public with sharing class MyObjectsSelector extends fflib_SObjectSelector
{
    public MyObjectsSelector(Boolean includeFieldSetFields, Boolean enforceCRUD, Boolean enforceFLS)
    {
        super(includeFieldSetFields, enforceCRUD, enforceFLS);
    }
}

This will allow for dynamic CRUD and FLS enforcement. In controller this selector would be called with read checks enforced

public with sharing class MyController
{ 
    List<MyObject__c> records = new MyObjectsSelector(true, true, true).selectById(recIds);
}

And other system-related classes could disable security checks

public with sharing class SomeSystemService
{ 
    List<MyObject__c> records = new MyObjectsSelector(true, false, false).selectById(recIds);
}

While all this looks good, we will loose dynamic benefit if we use the Application class and run-time instantiation of selector instances

public with sharing class MyObjectsSelector extends fflib_SObjectSelector
{
    public static IMyObjectsSelector newInstance(){
        return (IMyObjectsSelector) Application.Selector.newInstance(MyObject__c.SObjectType);
    }
}

public with sharing class MyController
{ 
    List<MyObject__c> records = MyObjectsSelector.newInstance().selectById(recIds);
}

Approach 2 (use fflib_QueryFactory constructors)

This one is easy.

public with sharing class MyObjectsSelector extends fflib_SObjectSelector
{

    public List<MyObject__c> selectAll() {
       // newQueryFactory(Boolean assertCRUD, Boolean enforceFLS, Boolean includeSelectorFields)
       fflib_QueryFactory query = newQueryFactory(true, true, true); 
       return (List<MyObject__c>) Database.query(query.toSOQL());
    }
}

However it doesn't provide dynamic support for security enforcement. So if need the same query to run in user and system contexts, I need to pass a flag to my custom select method. But this extra parameter in the method signature seems to break SOC a bit.

 public List<MyObject__c> selectAll(Boolean enforceReadSecurity) {
       // newQueryFactory(Boolean assertCRUD, Boolean enforceFLS, Boolean includeSelectorFields)
       fflib_QueryFactory query = newQueryFactory(enforceReadSecurity, enforceReadSecurity, true); 
       return (List<MyObject__c>) Database.query(query.toSOQL());
    }

Approach 3 (disable all read security checks done by fflib_SObjectSelector by default)

In the aforementioned book in the Overriding the default behavior section from Chapter 7 it is stated that Another motivation for disabling the selector default behavior would be a preference to code these checks in your controller logic more explicitly. This brings up another question: shouldn't we always perform security checks in controllers/batch classes/web services etc.?

To wrap up, it would be interesting to know which approach should a developer consider as a best practice, or best use cases for each of these options.

[cropredyHelix]

My take on this is included in this stackexchange answer

[afawcett]

@cropredyHelix i love your answer and one of the comments "i wish it was included in the book" .... having just released edition 3.... so do I! ;-) Flagging this issue as a wiki page action.