11

We are preparing our managed package for security review. And as a part of this we need to enforce not just create/update/delete but also read (i.e. select) permissions. That means basically checking isAccessible on each field from every SOQL query in our codebase.

What is best way to do it? Is there any way of framework/approach to avoid doing it manually in each class/method separately?

Moreover, some of our queries are really complex, like the one from example below. It looks like a nightmare to enlist each field separately when calling some PermissionUtils.assertReadAllowed method and then maintain such code in sync with actual field lists in query with time passes.

Does SF security review team seriously inspect all fields in all SOQL queries? Or is it enough just to enforce create/update/delete checking?

Any thoughts would be appreciated. Thanks in advance.

    [
        SELECT
            Id,
            Name,
            OwnerId,
            Owner.Id,
            Owner.Name,
            EnforceOwnerPermissions__c,
            OfflineUniqueId__c,
            DependentOfflineIds__c,
            ItemOfflineIds__c,
            LastSyncTime__c,
            Status__c,
            IsNew__c,
            IsSuccess__c,
            IsRetry__c,
            IsFailed__c,
            IsCancelled__c,
            IsManual__c,
            CreatedMoreThan24HoursAgo__c,
            HasSyncHappenedAfterwards__c,
            IsPending__c,
            IsRetriable__c,
            IsCancellable__c,
            IsSimulatable__c,
            IsRepairable__c,
            ReadyToBeProcessed__c,
            NumberOfFailedAttempts__c,
            ExpectedItemsCount__c,
            ActualItemsCount__c,
            AllItemsCount__c,
            HasLessItemsThanExpected__c,
            HasMoreItemsThanExpected__c,
            DebugMode__c,
            OfflineTimestamp__c,
            LastRunLog__c,
            LastRunLog__r.Id,
            LastRunLog__r.Log__c,
            LastRunDebug__c,
            Comments__c,
            CreatedById,
            CreatedBy.Id,
            CreatedBy.Name,
            CreatedDate,
            LastModifiedById,
            LastModifiedBy.Id,
            LastModifiedBy.Name,
            LastModifiedDate,
            (
                SELECT
                    Id,
                    Name,
                    OfflineUniqueId__c,
                    SyncTransaction__c,
                    Status__c,
                    ReadyToBeProcessed__c,
                    EntityId__c,
                    EntityType__c,
                    Operation__c,
                    Data__c,
                    OfflineTimestamp__c,
                    Owner__c,
                    EnforceOwnerPermissions__c,
                    Order__c,
                    IsEntityIdOffline__c,
                    ProcessedRecordId__c,
                    RecordId__c,
                    IsInsert__c,
                    IsUpdate__c,
                    IsDelete__c,
                    IsRecordIdNotApplicable__c,
                    CreatedById,
                    CreatedBy.Id,
                    CreatedBy.Name,
                    CreatedDate,
                    LastModifiedById,
                    LastModifiedBy.Id,
                    LastModifiedBy.Name,
                    LastModifiedDate
                FROM
                    SyncTransactionItems__r
                WHERE
                    ReadyToBeProcessed__c = true
                ORDER BY
                    Order__c ASC
            )
        FROM
            SyncTransaction__c
0

We use the Metadataservice (thanks, Financialforce! - no affiliation) at https://github.com/financialforcedev/apex-mdapi/blob/master/apex-mdapi/src/classes/MetadataService.cls and assert that we have read rights to all the fields we need, catching the exception and redirecting to an error page when the rights are missing. Yes, Salesforce expects you to pay attention to view rights, both to records via sharing and to fields via FLS

  • That recommendation seems off base, as AssertException is not catchable. Not sure why you got a downvote, but that may be part of the reason. – Adrian Larson Dec 26 '18 at 16:44
  • 1
    It doesn't matter if you are using third party solution or your own one to check for CRUD. The same problem remains. You need to go over all your code base & apply these checkings to each query and field individually. That's a lot of routine and error-prone work to do. Moreover it's hard to force devs to follow this practice for all on-going features. Also SF has plans to implement this CRUD enforcing as a part of the platform (Beta in Spring 19, releasenotes.docs.salesforce.com/en-us/spring19/release-notes/…), so you would need to clean it up. – wesaw Dec 27 '18 at 14:57
  • The referenced class takes an Objectype and a list of fields to either just check rights, returning a boolean, or asserting rights, raising a catchable exception. We query only for the fields necessary for the mechanisms to work, and, like I said, redirect to an error page if the rights aren't there, so it's just a few lines of code. – Jochen Dec 27 '18 at 17:12
  • The beta feature seems nice. I'd still need to catch the permission error as is the case now, but it will be nice not to have to list all the fields in the check, and then again in the query, so it will save a few lines and possible errors. Looking forward to it's general release, although I'm note sure what you mean by needing to clean it up. I'd likely leave existing code as is, not least because it passes the security review. – Jochen Dec 27 '18 at 20:54
  • We have more than 10,000 places in our code base where SOQL queries are used, so even adding one simple call to helper class is sort of a problem for us. Plus it's hard to rely on developers to strictly follow this approach and update list of fields appropriately every time when they add/update/remove something from related query. And yes, when SF makes CRUD checking as a part of a platform, I would rather drop all home-made or third party solutions and switch to standard one - that's what I meant when saying about 'cleaning' - going through > 10,000 places and remove/comment stuff is tough). – wesaw Dec 31 '18 at 8:49
0

Create a new Security class and create new methods like these, for example:

public static void requireAccessible (String objectName) {
    if (!isAccessible(objectName)){
        throw new Exception('requireAccessible: ' + objectName);
    }
}

public static Boolean isAccessible(SObjectType objectType) {
    return objectType.getDescribe().isAccessible();
}

You can write similar methods to check isCreatable, isUpdatable and isDeletable properties.

After that, you can use these methods before every CRUD operation.

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.