How to best enforce CRUD/FLS and pass Security Review

Question

Since Salesforce began to require CRUD / FLS checks as part of the Security Review for AppEchange apps in 2015 there has been a lot of discussions why this cannot be done by the platform and how this can be solved in an easy and reliant way.

Most bigger ISVs since the use a mix of different frameworks and libraries among which the Financial Force Apex Commons (a.k.a. fflib) and Patron DML Manager are the most successful.

The fflib safeguards Object and field level queries in its Selector classes and its Query Factory:

public List<Account> selectById(Set<Id> ids) {
  return Database.query( newQueryFactory()
                               .assertIsAccessible(true)
                               .setEnforceFLS(true)
                               .setCondition('Id IN :ids').toSOQL() );
} 

The DML Manager enforces C(R)UD safety by wrapping the native DML operations:

List<Contact> contacts = [SELECT Id, FirstName, LastName FROM Contact];
...
DMLManager.updateAsUser(contacts); 

But as always all of this has some drawbacks (e.g. limit-intense and cluttered code) so that many developers who had not to go through Security Review skipped using them.

Apex also now has some new features to better handle this:

• the WITH_SECURITY_ENFORCED SOQL keyword
• the stripInaccessibleFields() method on SObjects

---

So how would YOU enforce CRUD and FLS if you could start from scratch and your goals are:

• small footprint library (amount of classes + concepts)
• clean and lean API (doesn't add much clutter to code)
• has proven to go smoothly through security review
• efficient regarding CPU and other Governor limitations

Note: I will leave this question open for a long time to invite many to add, vote and comment on answers by others.

Answer 1

The best option today I think is to largely avoid CRUD & FLS and enforce security via custom permissions aligned with your business processes but that is not really answering your question.

Part of the difficulty here is first understanding where to perform CRUD & FLS checks. For me the best answer to that is on service entry since it allows callers to the service to be given visibility into security questions, such as 'Can I do X on a Y?', that they might need to provide the right UX while also supporting a 'fail fast' model.

You can support this type of handling fairly easily and cheaply with some kind of Permission checking service, really no need to have this integrated into anything else in the stack or make it any more complex than needed for the types of packages you work on.

I think where this answer fails is that a lot of developers would rather not be troubled by thinking about what CRUD & FLS checks are needed and would prefer something more automated. That is likely a mistake as you will probably realise the first time a customer asks what permissions do I need to give a user to make X work and you realise not only that you don't know but you don't have any means to find out either for all the possible 'X' unless you have also done a lot of work on Permission sets.

That said, the next best place to service entry where you could automate CRUD & FLS is likely in a UnitOfWork. There have been some discussion recently on doing exactly that to the FinancialForce version of fflib just to make it easier to get some older code through security review without too much work.

Update:

This is does not fit your constraints but if you allow for some complexity then you get to a nicer place. If you accept the argument that checking CRUD & FLS late creates UX problems then you have to do it early which rules out directly using DMLManager or UnitOfWork style automated handling by requiring checks are at service entry or similar.

The problem with the service entry approach is the manual work needed. To reduce that you can use self-verification where you cross check (in test mode) what permissions were checked at service entry against what were used in DML ops and fail any transaction that causes differences forcing a fix. This just ensures your service entry checks always match which saves you a large chunk of dedicated permissions testing which is a big part of what ‘automated’ checks would help you do anyway.

How you perform the cross check is not that important, just really some Map comparing. A useful abstraction for permission checking in this kind of model is to create a Permission base class for various styles of permissions such as a ReadFieldPermission class and include in the hierarchy combining conditionals such as anyOf, not etc so you can express complex needs. This allows you to pass around permission needs between functions so you don’t break encapsulation while making it straightforward to check if some arbitrary set of permissions are being met by the current user.

Answer 2

The answer was given by Chris Peterson (PM of Apex) and MVP Daniel Ballinger during Dreamforce '19 in the session "Reducing the Cost of Enforcing CRUD and FLS in the ESAPI".

And the answer is: By using the new Force.com ESAPI implementation from Github

With Spring' 20 the Security.stripInaccessible() is GA. Peterson and Ballinger rewrote this Salesforce-preferred library using this new feature and not only dramatically reduce code size but also increased the overall performance of the code.

In the session, Chris Peterson mentioned multiple times that using this library will make you go through Security Review (at least the CRUD and FLS checks) like a charm.

Go here to find their detailed slide deck with a lot of code: https://speakerdeck.com/ca_peterson/df19-reducing-the-cost-of-enforcing-fls-and-crud-in-the-esapi

NOTE: After I discussed this with David Esposito the mastermind behind the current defacto standard DmlManager he pointed me to shortcoming of the ESAPI solution which sound very valid. Make sure to read https://success.salesforce.com/_ui/core/chatter/groups/GroupProfilePage?g=0F93A000000HWy5&fId=0D53A00004Ore61 before you make you final switch to ESAPI.

Answer 3

The most voted advice will get you failed at the security review. End-users rely on CRUD/FLS configurable from UI Setup, and they will get quite surprised that your app does not respect the standard platform access restriction functionality.

My advice is to check for CRUD/FLS inline, no libraries, as close as possible to the code that accesses the object, i.e. as close as possible to the SOQL query or DML operation, as it is semantically better. Just write WITH SECURITY_ENFORCED in every SOQL query or leave a comment why it does not have it. And call Security.stripInaccessible() before each insert, upsert, update, or delete.

Answer 4

TBH, considering that new security features are on the way but not quite GA yet, maybe your best bet is just to write your own central helper classes that will handle CRUD/FLS to exactly the extent that is needed to pass review. Later when the new tools go GA, you can then probably reduce the complexity of the helper classes by refactoring them to use the new tools.

Ultimately when building your own package, you are the one who knows which use cases justify enforcing versus bypassing CRUD/FLS. In the case of bypassing you will of course document to the SR team exactly why, and disclose to the subscriber administrator what the implications are when they set up your app.