9

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:


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.

  • 8
    Neither WITH SECURITY_ENFORCED nor Security.stripInaccessible() is GA, with one release to go in CY19. So they can't yet be used for an app that "has to pass Security Review in 2019". But they will definitely make things a lot easier when they are GA! – Thomas Taylor Jul 12 at 22:10
  • They likely go GA according to Chris Patterson. Would you mind adding as answer with code details how you envision that future apps will use them to simplify CRUD FLS safety? – Robert Sösemann Jul 13 at 12:16
5

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.

  • For the last 5 years I used the READ checks in provided by the fflib_QueryFactory as discussed in depth here: salesforce.stackexchange.com/questions/266322 What do you recommend as checks when doing DML? I rarely use the fflib Unit of Work and have insert, update statements all over my code. Having a DMLManager check before that is the best I came up with until now. – Robert Sösemann Jul 13 at 21:18
  • 1
    @RobertSösemann Have updated my answer with some more detail on what I would recommend. This does add complexity but think that is sadly unavoidable unless the product is small enough that you can sacrifice the UX goal. – Kevin Jones Jul 14 at 19:10
3

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.

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.