Skip to main content
Group

Security.stripInaccessible

For feedback and discussion of the Security.stripInaccessible Apex method for FLS/CRUD enforcement.

@Chris Peterson I want to use security.stripInaccessable in a batch on query string that's getting passed to Querylocator. Is this possible?

1 answer
  1. Dec 15, 2021, 11:33 AM

    stripInaccessible is used on a collection of records, not on a SOQL query

     

    Your Batch Apex start() method would return a QueryLocator just like normal.

     

    Within your execute() method, you can run stripInaccessible against the chunk of records passed in.

0/9000

@Chris Peterson  here are 'idea' links for items that were talked about below:

 

Have stripInaccessible work with SOQL in the defnition of a FOR loop:

https://success.salesforce.com/ideaView?id=0873A0000003gEyQAI

 

Have WITH SECURITY_ENFORCED use a run-time boolean parameter

https://success.salesforce.com/ideaView?id=0873A0000003gEtQAI

1 comment
0/9000

@Chris Peterson  It would be a big plus to have similar functionality available for FOR loops that use SOQL (eg:  https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/langCon_apex_loops_for_SOQL.htm)

 

That construction is recommended by Salesforce to minimize memory consumption, but applying FLS protection is very manual and cumbersome. 

1 comment
  1. Jan 23, 2020, 6:57 PM
    Good point. StripInaccessible internally should make use of caching for checking FLS/CRUD access on the same types of Objects, but I agree that the for loop chunking feature of Apex is something we should make sure to keep as a first class feature, and better integration with stripInaccessible would indeed help there.
0/9000

@Chris Peterson  The stripInaccessible method is a nice feature, and a step in a good direction.  However the syntax feels a little cumbersome to me.  I would strongly prefer a flag built into the query syntax, similar to how "WITH SECURITY_ENFORCED" was delivered.  eg:

list<Account> = [select id, name, aaa__c, bbb__c from Account WITH STRIP_INACCESSIBLE];

 

and, what would be even better is if those query flags were variablized , so that the behavior could be changed at run-time.  The most important use-case here is a query in public utility function that needs to strip fields for a VF page but needs to return all fields when executed by a trigger.  eg roughly:

 

public List<account> getAccounts () {   

        List<account> a = 

             [select id, name, aaa__c, bbb__c

             FROM Account

                WITH STRIP_INACCESSIBLE=:(Trigger.IsExecuting==false)];

        return a;

 }

1 comment
  1. Jan 23, 2020, 6:56 PM

    Good feedback. Something we'll absolutely take a look into in the future, although with the way bind variables work today I'm not sure making them variablized will be possible, we'll have to check into that.

    I'd love it if you posted an idea, since idea votes do help quantify demand and help justify a higher position on the backlog.

0/9000

@Chris Peterson I know this feature is in Pilot, but are can we still use it in a managed package that's scheduled for security review in the next week or so?

 

@David Esposito Love to see that you're already on top of integrating into UoW; can't wait to leverage this enhancement!

5 comments
  1. Sep 4, 2019, 11:55 PM
    Thanks @Chris Peterson I figured as much. Unfortunately Spring '20 is too far down the road for us to wait. So we're going to have to basically build or import a replica of "stripInaccessible" to tide us over :(
0/9000

I see in the Winter 20 the new method is going to Beta and we get an new UPSERTABLE access type. 

So will UPSERTABLE take into account each sObject having the ID field populated and adjust the field checks between CREATABLE and UPDATABLE as appropriate?

Enforce Field-Level Security in Apex (Beta)

0/9000

Problem integrating stripInaccessible into fflib_SObjectUnitOfWork

 

@Chris Peterson -- When thinking about how we'd integrate stripInaccessible() into our existing application that makes heavy use of fflib_SObjectUnitOfWork, I have two use cases that are not well served.

 

Use Case #1 - Using stripInaccessible for graceful degradation on insert/update

 

I created a prototype project that has a VF page that prompts the user for fields on an Opportunity then uses an SOUOW to create the Opp and OppContactRole

 

I used the IDML interface of SOUW to inject an implementation that sanitizes the object lists before insertion. You can see the code here:

 

https://github.com/patronmanager/sfdx-stripinaccessible/blob/master/force-app/main/default/classes/OpportunityCreationController.cls#L48

 

On line 48, I take the 'clean' objectList and insert it ... the problem is, the clean object list are copies of the originals and SOUOW can't resolve the relationships to the new records on the subsequent attempt to insert the OppContactRole with its lookups set properly

 

If you comment out line 18, you'll see that the Save succeeds. With the custom IDML implementation, you get 

 

Insert failed. First exception on row 0; first error: REQUIRED_FIELD_MISSING, Required fields are missing: [Opportunity]: [Opportunity]

Error is in expression '{!save}' in component <apex:commandButton> in page opportunitycreation: Class.OpportunityCreationController.MyDML.dmlInsert: line 50, column 1

 

I believe this could be 'solved' by having stripInaccessible modify the passed-in objectList in place, rather than return copies in the getRecords() of SObjectAccessDecision

 

Use Case #2 - Allowing unmodified fields to be passed to DML operations

 

I didn't work up an example of this yet in code, but the pattern is straightforward:

 

 * The User has Read access to some fields on an object, but not Edit.

 * All fields (both Read and Edit) are generally retrieved via a Selector and displayed to the user

 * The User performs an operation on the record(s) and an updated is applied to the database

 

The desirable behavior is that stripInaccessible will only identify fields that have changed since they were retrieved from the database.

 

Instead, one of two things may happen depending on how someone implements this:

 * We could take the stripped records returned from SObjectAccessDecision.getRecords and apply those to the database. That could mask a database corruption problem (the user should be halted if the operation modifies both fields the user can't modify as well as fields the user can modify) 

 * We could throw an exception when getModifiedIndexes returns a non-zero set of results. This would erroneously block a myriad of valid use cases

 

I can work up an example of these scenarios if they would help illustrate the problem

 

I believe this could be solved if stripInaccessible (or the SObject class itself) tracked which fields have been dirtied since they were retrieved from the database and didn't strip clean fields. 

 

While most of my thinking is focused around the use of fflib-apex-common, I can see how other common DML patterns would run afoul of the same problems. 

2 comments
  1. Aug 1, 2019, 9:28 PM
    @Chris Peterson

    It took me a little while to find a slot to put together the repro case for my second scenario but here goes:

    https://github.com/patronmanager/sfdx-stripinaccessible/blob/master/force-app/main/default/classes/AccountEditController.cls

    AccountEditController is a slightly contrived example of wanting to build a CRUD page around an SObject record that you continually update. The desire is to not have to re-query the record from the database after every save() operation -- you should be able to just render the same instance of the object.

    If I run the instance through stripInaccessible(), prior to performing the Update, some of the fields that are accessible by the user for Read are removed and are subsequently gone when trying to render the view again.

    Here are the screencasts:

    This is a privileged user with Read and Update perms on the Importance field -- he can update the record over and over and the UI remains consistent with expectations (the formula field recalculates based on his choices)

    https://video.drift.com/share/ab3Sr0QfM0o/

    This is a less privileged user that only has Read access to the Importance field. When I use stripInaccessible(), the value of Importance is stripped from the SObject and when the view renders again, it's blank:

    https://video.drift.com/share/abFeoH8eM0o/

    And here's the same scenario WITHOUT using stripInaccessible (i.e. I uncommented line 15 of AccountEditController and commented out lines 19-24))

    https://video.drift.com/share/abhY1tVYM0o/

    While putting together this sample, I ran into a new use case:

    Use Case #3 - Formula Fields

    When you run an SObject instance through stripInaccessible(), the values in the Formula fields are stripped out. While this is perfectly reasonable on paper, it kind of seems silly to strip them out if they're not ever going to violate FLS (i.e. you can't assign to them)

    The resulting SObjectAccessDecision contains a non-empty collection in getRemovedFields() .. and then you need to reconcile the list of fields in that collection against Field Describe information to decide whether the fields were stripped out because they weren't writable vs. because the user didn't have Update perms for that field. Lots of machinery for that ...

    To your points about heap size and dirty tracking ... I obviously don't understand the internals of Apex but an extra bit on the heap for each field for each record seems trivial .. and the cost to flip the bit seems like way less CPU expense than all of the work to consume the SObjectAccessDecision that's returned and all of the code that entails. Again, this is me outside looking in .. but if you look from the inside out, you'll see how painful this is for Apex developers .. anything you can do to help us out will make you a hero (err, more of a hero!).

0/9000

It's good to see beta/pilots of SOQL query clause "SECURITY_ENFORCED" and Security.stripInaccessible() coming up. Though my opinion is APEX should secure by default in API versions going forward, anyone looking to bypass security should use something like "WITHOUT SHARING" on their class header. Here are some more thoughts:

 

Graceful degradation in StripInaccessible() call is great, but how many developers will write code to handle this degradation? Forex.

  • You are querying and showing 10 fields on a component, 2 of them are not visible to the current user. Will the developer write a logic to show/hide rest of the fields based on the stripped out fields? 
  • If a component supports partial access user, that component will have FLS checks via standard API to hide the sections of the page as required. 
  • Same goes in DML if a form with 10 fields is submitted, and 2 fields are stripped out, we can't go ahead and commit the partial record, as it's the wrong transaction as well. Why we should show 10 fields on the screen, and strip out 2 fields which are not accessible? 

I believe most of the screens, and DML logics are never created for partial fields transaction support or showing limited fields of what is expected. It will eventually add more complexity on the developer's part to gracefully handle the issue. 

 

I believe the default behavior here should be an exception which states which permission/fields are troubled. It's correctly mentioned  by @David Esposito   in this idea: https://success.salesforce.com/ideaView?id=08730000000Lj8GAAS that Database.query() Database.delete() etc should be overridden or support SECURITY by default going forward. 

 

Keeping security review aside, any customer's implementation with compromised CRUD/FLS is not good, the idea here should be default bulletproofing, with optional "WITHOUT SHARING" type syntax to open up the security gates. 

 

Sorry for being very direct, but I am not really convinced with these new APIs. I feel Salesforce backend should keep security before anything, and enforcing it in full should auto imply from new API versions to avoid breakdowns. 

0/9000

@Chris Peterson Please post url of demo video referenced in https://youtu.be/RIqpZCi5A5Y?t=909 (Apex Security Object and Field Security Made Easy), thanks!

1 comment
0/9000