5

Can I write CRUD & FLS Enforecement codes in following : If not in any of these - then Why ?

Apex Class - YES

Apex Trigger - ?

Apex BatchClasses - ?

Utility Classes - ?

1
  • 1
    What would be the use case for a trigger that needed to check field security? It would help to know what you're trying to do and why?
    – pchittum
    Dec 15, 2014 at 14:24

2 Answers 2

8

Generally, code you've written is already CRUD- and FLS-safe, as long as you follow the basic guidelines regarding Visualforce elements (e.g. check FLS when not using apex:inputField, etc). You can always check FLS and CRUD in Apex Classes, Triggers, Batches, and Utility classes.

FLS is almost never relevant in your code-- you generally want to perform whatever update you wanted to perform regardless of user security. The exception here is that you should check FLS, or at least validate input, if you'll be copying data from a field a user can edit or an input element to a field that the user can't see or edit. This should be a rare scenario, so you'll usually know in advance if it's necessary.

CRUD is automatic in most cases, assuming you're using the correct "with sharing" or "without sharing" words; you really have to go out of your way to cause problems with CRUD, and it's almost never necessary to explicitly check the UserRecordAccess in regards to a specific CRUD operation.

When in doubt, try running a cloud security scan, and see what the scanner flags as potential problems. It will alert you of possible risks and what you can do to mitigate those risks.

I almost never need to validate FLS in code I write, so this leaves me with needing to check CRUD. Generally, I rely on the "with sharing" and "without sharing" keywords to force a sharing model check as necessary without needing to consult UserRecordAccess.

As far as the sharing model I use, I generally follow these guidelines:

Controllers, Extensions: "with sharing"
Batchable, Schedulable, Queueable: "without sharing"
Trigger Classes, Utility Classes: unspecified (let the calling context determine access)

However, there are exceptions to every rule. If a batchable class needs to process user-supplied input, I'd check CRUD at minimum, and possibly FLS as well. For example, we have a system where the users can bulk load data into the system. We give them this utility because the API is disabled for them, so they can't just use the Excel Connector or Data Loader. This framework validates each field and record's access to make sure the user doesn't violate our business rules. Both CRUD and FLS are enforced to prevent violations of business rules.

Utility classes generally have no context of their own, and probably only rarely perform DML operations (but queries would be more common), so the default is to prefer whatever context they're being called from. I would be hesitant to enforce CRUD or FLS in most utility classes, but discretion is the order of the day. If you feel that there is a risk, enforce it. If not, don't waste time checking, as it will only slow down your transactions unnecessarily.

You should always avoid making security blunders, but you should also avoid checking security unnecessarily. For example, a top-level trigger context was probably called from a sharing-enabled context, so there's no need to check CRUD within the trigger, because they would have been denied before even reaching your trigger. However, if the trigger will need to update related records, you may or may not want to check CRUD. In my experience, triggers that update related records are generally performing "system" updates and don't need to worry about CRUD.

Occasionally, you'll need to update a lookup field with a record the user may not have access to. This can cause failures, so you should check CRUD in those cases. This is a rare situation, but when it does occur, you'll need to account for this possibility.

TL;DR: Don't bother checking in Apex Code at all unless you have very specific circumstances. Generally Visualforce's FLS and CRUD enforcements are sufficient for about 99% of use cases. Use the security scanner if you're concerned about possible security violations.

8
  • 1
    Worth noting is that for managed packages to pass security review, they have to explicitly check CRUD and FLS prior to DML statements since Apex runs in system mode and doesn't respect CRUD or FLS. The online scanner doesn't pick that up (or at least didn't the last time I ran it a month or two ago). Dec 16, 2014 at 3:41
  • @PeterKnolle That's not true. You only need to perform checks when you violate the Visualforce element rules. I started using that scanner when it first came out, and I spent two years working on a project that was released as a managed app, and we only had a handful of FLS violations, all of which were fixed in Visualforce, without any use of Apex Code.
    – sfdcfox
    Dec 16, 2014 at 4:51
  • Interesting. My scans didn't pick it up w/ their online scanner, then they sent some output of another scan that seemed to be done with a different tool (had references to a local file system and output was different) that reported I needed to check CRUD/FLS before every DML statement. My DML was done in a service layer, although it came from a VF page, so perhaps I got false information or it had to do with not performing my DML directly in the controller. The offenders were all insert myListOfRecs, upsert myListOfRecs, update myListOfRecs, and every one of the statements was flagged. Dec 16, 2014 at 11:20
  • Additionally, I sent email for clarification of their scan results, which included the CRUD/FLS violations as opposed to the online tool I ran which didn't, and they said I would pass if I checked CRUD/FLS explicitly before each DML statement. Dec 16, 2014 at 12:01
  • 1
    @sfdcfox the CRUD/FLS checks were introduced as a pass/fail condition for security review this April. Now you need to check all of the context fields before doing a DML, otherwise you fail security review. I know this for a fact since i experienced this a month ago.
    – zokito
    Dec 18, 2014 at 15:46
0

Visualforce inputfield/outputfield components take care of CRUD/FLS checks assuming that you are only passing sObjects directly to controllers from VF input fields and only passing sObjects back to VF pages, without any side effects on other objects that don't appear in the VF input fields, including DB queries with WHERE clauses that access fields that the user may not have permission to view.

For apps of any complexity that is never the case, as these apps:

  1. De-reference fields in the controller and pass the de-referenced fields to the page, causing the CRUD/FLS checks to not be done on the de-referenced field

  2. Make use of composite objects built up from sObjects (e.g. via wrapper classes) in which case the VF layer doesn't enforce CRUD/FLS on the composite objects.

  3. Make use of side effects that modify or read objects that never reach the visualforce layer (e.g. in a trigger to modify a new object or field other than the one being added)

  4. Use Lightning, Javascript Remoting, Web Services, or the many other dataflows to Apex that don't involve visualforce input/output fields.

  5. Make heavy use of complex DB queries that reference fields in WHERE clauses that may not appear on the visualforce page.

Therefore you really do need to do these checks yourself, which unfortunately means bulking up your Apex code to perform these checks everywhere you handle objects that you don't own. This is why there is an idea exchange topic to make this easier.

In addition to this, managed packages are under stricter requirements because they are installed in organizations that can have diverse and changing profiles/perm sets of which the package author is not aware of. Moreover, global classes are entry points to these packages and the developer has no information about whether the data passed into the global class originally came from a visualforce input field or not.

Bottom line is that even if you are constantly on the phone with the admin, making sure that your code never does anything that the admin doesn't want for any user assigned to the app, after your code ships, the admin may change the permissions of a profile or inadvertently assign to your app a user that doesn't have the right permissions. This is especially understandable for managed packages that are blackboxes to the admin, so for large apps it may not be obvious which fields you are accessing.

In the case of such conflict, the correct response is to fail gracefully rather than overriding the organization's policy. Tell the user that they don't have sufficient permissions to perform the operation. The only way to find this out is to make CRUD/FLS checks yourself in your apex classes.

To specifically answer your question:

Apex Class - YES, unless you are building a hello world style app that only handles fields in VF input pages and has no side effects.

Apex Trigger - YES, you can and should. Just because you can't set a sharing policy for a trigger doesn't mean that there is no associated user context for CRUD/FLS queries.

Apex BatchClasses - YES, you can but in most cases these will run under the admin profile.

Utility Classes - YES, you can and should, because FLS is not inherited from the caller like sharing.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .