4

I see more and more information about the importance of performing FLS checks before SOQL and DML operations, using the Security.stripInaccessible method, e.g. in the Winter '21 PD1 certification maintenance exam.

There are several posts, like here, here and here that discuss the issue.

It makes sense that this is important in managed packages, where Apex in a package could perform operations on a field that a user does not have access to according to FLS, or to sanitize sObjects that have been deserialized from an untrusted source.

What I cannot get clear is whether it is really necessary to perform these checks on code developed by a company's internal developers, where the FLS model has been put in place by admins/developers and code written and tested with the business context in mind.

  • What risk is introduced by not performing FLS checks on all DML in internally developed code?
4
  • 3
    Field level security in apex isn't magically enforced just because it was developed by internal company devs. If you want apex to respect your org's security model in custom code, you need to enforce it with accessibility checks. Many standard UI components (VF and Lightning) as well as the UI API helpfully enforce security checks on the front end but the underlying apex in custom code does not. The risk is that your custom code can modify data that the running user may not be permitted to do otherwise based on the org config.
    – Mark Pond
    Jan 5 at 23:29
  • 1
    Sometimes that’s the desired behaviour, e.g. salesperson updates Amount, to which they have edit access, on Opportunity, a Trigger invokes Apex, and that updates some field on a finance object that the user does not have access to by design so that they can never add it to a report. If FLS was checked prior to DML, the field would be stripped. The security model and Apex operations were designed and tested by the business, so what does FLS check achieve?
    – MikeA
    Jan 6 at 0:51
  • 2
    It depends on context. Some Apex it is important that you check FLS, for others it's not. For example - Apex that returns data that is to be displayed in an LWC maybe should check FLS, for your trigger example it probably should not. This is similar to the question "with or without sharing". Salesforce docs have a habit of stating "best practice" when the reality is - it depends. You're right to question the value but also you should be asking yourself if you should be checking FLS on a case by case basis. Jan 6 at 6:57
  • 1
    @RobBaillie Can you add that as an answer please, 'it depends' should be the accepted answer.
    – MikeA
    Jan 6 at 16:30
1

It depends on context.

Some Apex it is important that you check FLS, for others it's important that you don't.

For example:

  • Apex that returns data that is to be displayed in an LWC maybe should check FLS to ensure that no data is accidentally leaked to users that should not see it.
  • However, it's possible that the LWC should return that data - maybe it's part a service console that the user is temporarily authorised to access after security questions posed to a customer on the phone ("Can I have authority to view that, and have the 3rd and 6th numbers in your passcode to confirm").
  • For your trigger example, where a non finance user is updating an Opportunity in a way that forces an update on Invoices (to which the non finance user does not have access), probably FLS shouldn't be checked. Doing so could potentially introduce a bug, as you say.

This is similar to the question "should this class use with or without sharing".

Salesforce docs have a habit of stating or implying "best practice" when the reality is - it depends.

You're right to question the value but also you should be asking yourself if you should be checking FLS on a case by case / class by class / SOQL by SOQL basis.

Sometimes you do, sometimes you don't.

1

I assume your question is more specifically about the implications of not using Security.stripInaccessible method for all DML in an internally developed code, but the last point in your post seems question the general importance of FLS (at least that is how I understood your question initially). While the comments on your post have been mostly on the importance of FLS & the related business context, I guess you already have a clear understanding about it in Salesforce context. I feel this question may get tagged as opinion-based.

I see more and more information about the importance of performing FLS checks before SOQL and DML operations, using the Security.stripInaccessible method, e.g. in the Winter '21 PD1 certification maintenance exam.

IMO, Salesforce isn't implying anything different than the importance of standard FLS check practice, in general.

where the FLS model has been put in place by admins/developers and code written and tested with the business context in mind.

I interpret this as not just having FLS in place and application tested out, but also the standard FLS check mechanism (using DescribeSObject & DescribeFieldResult) in place. Since you already have standard FLS check in place, I don't think there are any major risks involved in not having FLS check on all DML (using Security.stripInaccessible method). Not having any form of FLS check in the code is a very sensitive issue and should be avoided. FLS check has to be performed with due-diligence of business context and with a clear understanding of the apex methods being used. For example, the valid use-case you have pointed out in your comment is a good place to avoid FLS check.


Security.stripInaccessible and WITH SECURITY_ENFORCED are possibly an outcome of this idea, which you might have already seen. I see them as just better ways to code FLS check when compared to the previous standard ways. Security.stripInaccessible just makes the coding FLS check a bit easier. For example, to allow 10 different users (with different set of FLS access) to insert a SObject, one would have to do it either of the following ways:

  • Write wrapper or service layer code to handle the FLS access, create appropriate collections (stripping away inaccessible fields) & then perform DML using the collections.
  • Write a really lengthy code using DescribeSObject & DescribeFieldResult to check FLS, perform DML and handle exceptions.

Now, using Security.stripInaccessible method for the same would make the code really shorter implicitly handling the exceptions gracefully.

0

Thank you all for the valuable input here, which led me to further resources which help to clarify this:

  • the 'Access Control in Apex Controllers and Supporting Classes' section of the Secure Coding Guide: This highlights that the highest risks are around user input and output in Lightning Components due to CRUD/FLS permissions not being automatically enforced in Lightning components or controllers. CRUD/FLS checks should be implemented for such scenarios in internally developed applications.

  • Example Responses to False Positives in Checkmarx Scan Results in the ISVForce Guide: This shows that in responding to an AppExchange Security Review, the ISV can justify false positives reported by a scanning tool such as Checkmarx, which reports on DML with no FLS check. In the example given, the ISV responds with 'Are against our custom object UsageLog__c and not intended for user consumption. They are never exposed to users directly.'

My take away here is that code developed for internal business use should be security reviewed internally with the focus on CRUD/FLS checks on SOQL and DML that is related to user input/output, particularly in Lightning Components. Where SOQL/DML is performed on data that is not received from or exposed to users, a security review can assess the risk and not performing CRUD/FLS checks may be justified in such a scenario.

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.