apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
914 stars 518 forks source link

Using Selector Layer with Guest User License #355

Closed devair closed 2 years ago

devair commented 3 years ago

Hi,

I have a webservice to receive notifications from external systems and I need to run a SOQL using a Selector class, but when the Selector method is called it doesn't return the records if the user who called it is from the profile with the Guest User License ( I use the Salesforce Sites feature). In the site's profile I already enabled access to the SObject and the webservice class and even so it doesn't return the records.

Is there any way I can return the records?

tfuda commented 3 years ago

This sounds like a record sharing issue. Does your Selector class have a sharing declaration? If it is declared as "with sharing", then records that are not explicitly shared with the Site Guest won't be included in the query result. You might consider setting the sharing on the Selector class to "inherited sharing" and then invoke Selector method from a class that is declared "without sharing".

devair commented 3 years ago

The Selector class doesn't have sharing declaration.

It looks like this:

`public class TransacoesPagamentoSelector extends fflib_SObjectSelector {

}`

stohn777 commented 3 years ago

Hi @devair

Expounding upon @tfuda's comment, please consider the following Salesforce Apex Dev. Guide page [1] which details class-level sharing, within stating, "Classes inherit sharing setting from a parent class when one class extends or implements another." The fflib_SObjectSelector class has with sharing, so I'd expect your implementing selector to execute with sharing as well which, as I believe you're saying, should see the records defined by Salesforce's sharing functionality.

Please conduct some research along that line and let's see what the results are.

With some definitive proofs, we can decide upon the next steps.

Thanks!

[1] https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_keywords_sharing.htm

daveespo commented 3 years ago

Also note the discussion on https://github.com/apex-enterprise-patterns/fflib-apex-common/pull/262 and https://github.com/apex-enterprise-patterns/fflib-apex-common/issues/199 and how it relates to using selectSObjectsById

tfuda commented 3 years ago

@devair Does TranscoesPagamentoSelector define its own public methods? Methods in your selector class will run with the sharing setting of the class where they are first declared, and in your case, your class doesn't specify a sharing mode. If you declare a public method called "selectAllTranscoesPagamentos" in your class, that method is going to run with the sharing mode of the class that is calling "selectAllTranscoesPagamentos" (I'm assuming a TranscoesPagamento is a custom SObject here). You need to specify "without sharing" on your class that extends fflib_SObjectSelector and define a public method that returns the records you want. Here's an example of what I mean:

public without sharing class TranscoesPagamentoSelector extends fflib_SObjectSelector {
        .
        .
        .
        // This method will run "without sharing"
    public List<TranscoesPagamento__c> selectAllTranscoesPagamentos() {
        fflib_QueryFactory qf = newQueryFactory();
        return (List<TranscoesPagamento__c>) Database.query(qf.toSOQL());
    }
}

If you invoke the "selectAllTranscoesPagamentos" method on this class, it will run "without sharing", because the class in which the method is first defined is "without sharing" (TranscoesPagamentoSelector ). However, if you invoke the "selectSObjectsById" method, it will run "with sharing", since this method is first declared in the base class (fflib_SObjectSelector) and that class is declared "with sharing".

devair commented 3 years ago

Hi,

I've read both issues and noticed the following situations in my project:

In my project, I have fflib_SObjectSelector "with sharing" declaration. It is the version of the master branch.

I've put "with sharing" in the TransacoesPagamentoSelector class and I've enabled its access in the profile but it still didn't work.

The only way it works is to put without sharing in the base class.

tfuda commented 3 years ago

Can you post the code of your TranscoesPagamentoSelector class? It's a bit hard to diagnose without seeing your code.

devair commented 3 years ago

@tfuda

public class TransacoesPagamentoSelector extends fflib_SObjectSelector {

    public List<Schema.SObjectField> getSObjectFieldList(){
        return new List<Schema.SObjectField> {
            BR_TransacaoPagamento__c.Id,
            BR_TransacaoPagamento__c.BR_Adquirente__c,
            BR_TransacaoPagamento__c.BR_Autorizacao__c,
            BR_TransacaoPagamento__c.BR_DataCriacao__c,
            BR_TransacaoPagamento__c.BR_DataRecebimento__c,
            BR_TransacaoPagamento__c.BR_DataValidade__c,
            BR_TransacaoPagamento__c.BR_Estabelecimento__c,
            BR_TransacaoPagamento__c.BR_IdLinkPagamento__c,
            BR_TransacaoPagamento__c.BR_IdTransacao__c,
            BR_TransacaoPagamento__c.BR_LinkAdquirente__c,
            BR_TransacaoPagamento__c.BR_MetodoPagamento__c,
            BR_TransacaoPagamento__c.BR_NumeroParcelas__c,
            BR_TransacaoPagamento__c.BR_Oportunidade__c,
            BR_TransacaoPagamento__c.BR_Referencia__c,
            BR_TransacaoPagamento__c.BR_ValorReceber__c,
            BR_TransacaoPagamento__c.BR_ValorRecebido__c
        };
    }

    public Schema.SObjectType getSObjectType(){
        return BR_TransacaoPagamento__c.sObjectType;
    } 

    public List<BR_TransacaoPagamento__c> selectById(Set<Id> idSet)
    {
        return (List<BR_TransacaoPagamento__c>) selectSObjectsById(idSet);
    }

    public static TransacoesPagamentoSelector newInstance()
    {
        return (TransacoesPagamentoSelector) Application.Selector.newInstance(BR_TransacaoPagamento__c.SObjectType);
    }

    public TransacoesPagamentoSelector(){
        super(false,false,false);
    }

    public List<BR_TransacaoPagamento__c> selectByIdLinkPagamento(Set<String> linkIdSet){

        List<BR_TransacaoPagamento__c> retorno =  (List<BR_TransacaoPagamento__c>) Database.query( 
            newQueryFactory()
            .assertIsAccessible()
            .setCondition('BR_IdLinkPagamento__c IN :linkIdSet')
            .toSOQL() );

        System.debug(JSON.serialize(retorno) );

        return retorno;
    }

    public List<BR_TransacaoPagamento__c> selectByIdTransacao(Set<String> transIdSet){
        return (List<BR_TransacaoPagamento__c>) Database.query( 
            newQueryFactory()
            .setCondition('BR_IdTransacao__c IN :transIdSet')
            .toSOQL() );
    }

    public List<BR_TransacaoPagamento__c> selectByReference(Set<String> refIdSet){
        return (List<BR_TransacaoPagamento__c>) Database.query( 
            newQueryFactory()
            .setCondition('BR_Referencia__c IN :refIdSet')
            .toSOQL() );
    }

}
tfuda commented 3 years ago

So, in looking at this, I believe if you declare this class "without sharing", then the selectByIdLinkPagamento, selectByIdTransacao and selectByReference methods should execute "without sharing", however, the selectById method will execute "with sharing" because it calls the fflib_SObjectSelector.selectSObjectsById method and fflib_SObjectSelector is "with sharing".

If you want a selectById method that executes "without sharing" then change it to this, so that it no longer delegates to fflib_SObjectSelector.selectSObjectsById:

public List<BR_TransacaoPagamento__c> selectById(Set<Id> idSet)
{
    fflib_QueryFactory qf = newQueryFactory();
    qf.setCondition('Id IN :idSet');
    return (List<BR_TransacaoPagamento__c>) Database.query(qf.toSOQL());
}
foxysolutions commented 2 years ago

@devair Valid point on sharing that this could be sometimes requiring without sharing. At my current client we're having quite some logic and objects that are ran both internal (with sharing) and for Community Users or Guest Users which don't have Sharing on some objects.

For this we've implemented the following structure to enforce best practice of running with sharing by default, but allow deviation for specific scenarios. We are still working on an improved version of this (seeing if we could extend a class, and then wrap it with or without sharing; as that would avoid multiple methods with all different kind of variables being passed as input). If we've come passed the testing phase with a final approach, I'll let you know.

The crucial part here is that Apex requires all binded variables to be present at the moment of Database.query(). Binded variables are best-practice and should be the standard to mitigate SOQL Injections and such.

public with sharing class SEL_CampaignMembers extends fflib_SObjectSelector{

    // .. Standard SEL methodsx

    /**
     * Method to query CampaignMembers by a UserId, preventing a Community User to first query their Contact to find the CampaignMembers
     *
     * @param userId        UserId to whom the CampaignMembers should be related (via User.ContactId link)
     * @param withSharing   Whether or not to enforce sharing CRUD and FLS; Note, Community Users have no Campaign nor CampaignMember sharing accessibiltiy
     * @return              List of all CampaignMembers related to the requested User
     */
    public List<CampaignMember> selectByUserId( Id userId, Boolean withSharing ){
        fflib_QueryFactory query = newQueryFactory( withSharing, withSharing, false );
        query.selectFields( new Set<SObjectField>{
            CampaignMember.CampaignId,
            CampaignMember.ContactId
        } );
        query.setCondition( 'contactId IN ( SELECT ContactId From User WHERE Id = :userId )' );
        return ( withSharing )
            ? ( List<CampaignMember> ) Database.query( query.toSOQL() )
            : new WithoutSharing().runQueryWithUserId( query, userId );
    }

    private without sharing class WithoutSharing{
        private List<CampaignMember> runQueryWithUserId( fflib_QueryFactory query, Id userId ){
            return ( List<CampaignMember> ) Database.query( query.toSOQL() );
        }
    }
}

This way in the calling class (knowing the users context) we can decide to call either one of the following:

Note, I've left the exact SEL implementation just for practical reasons. but the most important parts are:

This way you can run any query with or without sharing, without opening up too much of your system.

In case of any questions, please feel free to reach out.

foxysolutions commented 2 years ago

@devair In addition (after reading the thread more carefully), please be aware that fflib_SObjectSelector should NOT have any sharing specified, to allow inheriting from the Selector class.

For this it is crucial to be conscious of the following behaviour in Apex:

"Classes inherit sharing setting from a parent class when one class extends or implements another." link

Since a Selector class always extends fflib_SObjectSelector , which on turn implements fflib_ISObjectSelector, if you'd specify any sharing on these two classes, you'd not be able to override the sharing performed in your selector class (unless working with inner classes).

Because of that I'd always suggest the following class-sharing specifications:

public with sharing class SEL_CampaignMembers extends fflib_SObjectSelector{
    // Default a class should be with sharing, and optionally having a without sharing inner-class, so the Developer is conscious of the sharing. Setting one Selector fully to without sharing is of course possible, but might be tricky for future extensions which shouldn't run in user context
}
public abstract class fflib_SObjectSelector implements fflib_ISObjectSelector{
    // ...
}
public interface fflib_ISObjectSelector{
    // ...
}

NOTE: When making this update, please do make sure all Selector classes have sharing applied, to prevent regression impact.

We just figured this out with some Domain structure we've built on top of fflib that all our Domain Trigger logic ran without sharing in the passed period. Thanks to the fflib structure, all queries are performed in isolated selectors applying their own sharing, so this only had impact on the UOW, but nevertheless was quite a surprise to find this documentation...

FYI @tfuda my thought would be that your approach wouldn't effectively run without sharing, but I'm more than happy to hear otherwise :)

daveespo commented 2 years ago

@foxysolutions -- As #262 indicates, the sharing clause of fflib_SObjectSelector effectively doesn't matter. The two special cases (selectSObjectsById and queryLocatorById) are the only methods affected by the sharing clause so those are the only two that should be moved up into your derived Selector class.

Changing the sharing declaration on fflib_SObjectSelector wouldn't be backwards compatible for those relying on the legacy sharing behavior for selectSObjectsById and queryLocatorById so we're not going to make that change since the workaround on #262 is satisfactory.

You're obviously welcome to make local changes to fflib_SObjectSelector but that does increase your maintenance burden as you take upstream updates from this project.

wimvelzeboer commented 2 years ago

Here is another example on how you can have one selector implementation with three sharing modes:

fflib-apex-extensions-samplecode