1

When checking Field-Level Security or object CRUD in Apex within a managed package, is it required to use conditional access to prevent the given operation (for the purposes of passing the security review), or would it also be acceptable to use an exception to to halt execution of the code?

The examples that I have encountered in the documentation use the former approach but does the latter approach introduce a vulnerability that would prevent the application from passing a security review?

Scenario 1: Using conditional logic:

if(Account.My_Custom_Field__c.getDescribe().isUpdateable(){
  acct.My_Custom_Field = 'X';
  update acct;
}

Scenario 2: Throwing an excpetion

  if(!Account.My_Custom_Field__c.getDescribe().isUpdateable())
    throw new MyCustomException('Example Message');

  acct.My_Custom_Field = 'X';
  update acct;
3

I very much doubt throwing an exception would prevent you from passing security review. I don't think it could possibly open up an attack vector, as it only reduces the number of valid execution paths.

What you really should worry about with such an approach is how your code interacts with user workflow. If your code prevents users from saving Account records for any length of time, it could be a full-blown emergency and cost millions of dollars.

If your code is used only in UI, it may be low enough risk to consider halting execution. If your code will ever be used inside of a trigger, I would deem it an unacceptable risk.

2
  • Graceful recovery is certainly an important consideration. For additional architectural context, the FLS checks for this particular application primarily take place in the Service Layer. I'm using the standard Apex error handling features to try to conform with the "Marshalling" guidelines for the Service Layer. Catching the error and recovering/halting/displaying an error message occurs in the application layer. – salesforce-hacker Feb 12 '18 at 19:44
  • 1
    Yeah if it's a service I would say stick with the conditional operation. Just because you know you need to catch that type of exception doesn't mean you can trust your subscribers. – Adrian Larson Feb 12 '18 at 19:56
-1

Question is also very subjective, I think it depends on your use case.

It will depend on what type of records are you populating. If it is config related records then you should create records only if you have access to all the fields of the object.

Now if you are populating any other records then if you are using it on UI then throwing an exception should be a correct way. Also you should show this exception to the user on screen.

In UI context if you are making a user to enter some data and from that data you are populating some other fields of records, then minimum that fields for which user entered information exception should be thrown if permission is missing.

For required fields permission exception should be thrown.

2
  • 1
    The specific question is whether the strategy of throwing the exception creates a vulnerability that would prevent passing the ISVForce security review, not general best practices for checking FLS/CRUD and displaying error messages. – salesforce-hacker Feb 12 '18 at 19:48
  • For security review, both will work. You can always give a false positive document if there is a false positive in security review. – Manjot Singh Feb 13 '18 at 19:28

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.