Salesforce Dev

Software development with Salesforce tools

Remote Actions: Leveraging class hierarchies to safely pass data

Valerio Sevilla Salesforce Dev , , , , , , ,

Sometimes, we need our @RemoteAction to be very flexible. This can lead to the necessity of allowing the caller to pass in some arbitrary information. But by doing so, we could be opening a security breach. In this post, I will show an example of this, as well as a possible solution to the security problem. Some knowledge about VisualForce components and remote actions will be needed to follow this.

Say, we want to implement a VF component that takes some data from the org and shows it in a custom picklist. We’d like this component to retrieve data very quickly as the user interacts with it, so we will use a remote action for this.

 
[code language=”xml”]
<apex:component controller="CustomPicklistController">
<apex:attribute name="objectName" description="Object to query from" type="string" required="true"/>

<script type="text/javascript">
function getRemoteData() {
Visualforce.remoting.Manager.invokeAction(
‘{!$RemoteAction.CustomPicklistController.getData}’,
‘{!objectName}’,
function(result, event){
…
},
{escape: true}
);
}
</script>

</apex:component>
[/code]

 
This is the controller:
[code language=”java”]
public with sharing class CustomPicklistController {

@RemoteAction
public static CustomPicklistData getData(String objectName) {
List<SObject> records = retrieveData(objectName);
CustomPicklistData data = convertData(records);
return data;
}

private static List<SObject> retrieveData(
String objectName
) {
String safeObjectName = getSafeSOQLString(objectName);
String dbQuery = ‘SELECT Id, Name FROM ‘ + safeObjectName;
return Database.query(dbQuery);
}
}
[/code]

 
… where getSafeSOQLString() makes sure no malicious string breaks our query by injecting SOQL code. In the example, there is no filter for the query. We are retrieving all records for the given object.

We want this component to be flexible enough to allow us to set custom specific filters wherever we instantiate it. A first approach would consist of passing the filter text as attribute to the component. This is the definition:

[code language=”xml”]
<apex:attribute name="filterText" type="String" required="false"
description="Filter to append to the query"/>
[/code]

 
The controller would be updated as follows:

[code language=”java”]
public with sharing class CustomPicklistController {

@RemoteAction
public static CustomPicklistData getData(String objectName, String filterText) {
List<SObject> records = retrieveData(objectName, filterText);
CustomPicklistData data = convertData(records);
return data;
}

private static List<SObject> retrieveData(
String objectName,
String filterText
) {
String safeObjectName = getSafeSOQLString(objectName);
String whereClause = String.isBlank(filterText) ? ” : ‘ WHERE ‘ + filterText;
String dbQuery = ‘SELECT Id, Name FROM ‘ + safeObjectName + whereClause;
return Database.query(dbQuery);
}
}
[/code]

 
An example of instantiation for this component can look like:

[code language=”xml”]
<CustomPicklistController objectName="Account" filterText="{!queryCondition}"/>
[/code]

 
… where queryCondition would be something like:

[code language=”java”]
public String getQueryCondition() {
return ‘Name LIKE \’A%\”;
}
[/code]

 
Unfortunately, we cannot apply getSafeSOQLString to filterText, as it is a piece of SOQL code. If we escaped it in order to prevent SOQL injection, it would stop working as well. We control front-end too in this case, so we could think that taking care of making front-end secure (so the user is not able to provide a fully free input for this attribute) is enough. But it isn’t: data is passed to back-end as a JSON string, which is not secure. We need an alternative way of passing the filter information from the front-end to the back-end.

 

Using precompiled, back-end data

 
So we want a component that gives us flexibility to pass whatever filter we need, but also, we don’t want to pass that filter in plain SOQL. We could think of creating a class which instances store the filter somehow codified, and then allows us to retrieve the SOQL string for that filter. A specific instance of that class, with the correct information, would be created from the VF page’s controller and passed to the component via attribute, which, in turn, would then pass the info to the @RemoteAction method. But in the end, it would not be secure either as the filter information is travelling from front-end to back-end, one way or another.

Instead, the flexibility will be moved to the back-end, while the front-end will be restricted to the options implemented in back-end. Let me explain this.

The component will now expect an instance of a class that implements this interface:

[code language=”java”]
public interface ICondition {
String getCondition();
}
[/code]

 
Such instance will not contain any data. It will only give us the information about which specific class implements the condition, so we can call the getCondition() method and get the information we need. This way the query condition is not passed through front end.

For each specific filter, a custom class will be implemented, defining the condition for each component instance:

[code language=”java”]
public class SpecificCondition implements ICondition {
public String getCondition() {
return ‘Name LIKE \’A%\”;
}
}
[/code]

 
The component will now receive the name of the class that implements the condition:

[code language=”xml”]
<apex:component controller="CustomPicklistController">
<apex:attribute name="objectName" type="String" required="true" description="Object to query from"/>
<apex:attribute name="conditionClass" type="String" required="false" default="CustomPicklistController.NoCondition" description="Class implementing ICondition"/>
…
<script type="text/javascript">
function getRemoteData() {
Visualforce.remoting.Manager.invokeAction(
‘{!$RemoteAction.CustomPicklistController.getData}’,
‘{!objectName}’,
{‘apexType’: ‘c.’+'{!conditionClass}’},
function(result, event){
…
},
{escape: true}
);
}
</script>
…
</apex:component>
[/code]

 
What happened here? We are basically applying what this article from Salesforce’s official documentation says about how to pass interface instances to a @RemoteAction method: the instance itself is not passed. What we pass, is a string with the class name (and nothing else in our example given the instance contains no data). The system will then, automatically, take care of instantiating the class.

As you can see, the default c namespace is being concatenated here:

[code language=”xml”]
<script type="javascript">
…
{‘apexType’: ‘c.’+'{!conditionClass}’}
…
</script>
[/code]

 
This is to simplify the caller so it does not need to know about the namespace thing. Of course, if we wanted to make this component available from outside our package, we would need to tweak this.

Another interesting detail is the default value given to the attribute: CustomPicklistController.NoCondition. This references a class that we will include with the component’s controller:

[code language=”java”]
public class NoCondition implements ICondition {
public String getCondition() {
return ”;
}
}
[/code]

 
The reason why we need this, is that we cannot pass null or inexistent classes to a @RemoteAction, so if we don’t want to apply any filter, we need a default class that actually implements no filter.

With all of this settled, the @RemoteAction method now looks like this:

[code language=”java”]
public with sharing class CustomPicklistController {

@RemoteAction
public static CustomPicklistData getData(String objectName, ICondition condition) {
List<SObject> records = retrieveData(objectName, condition);
CustomPicklistData data = convertData(records);
return data;
}

private static List<SObject> retrieveData(
String objectName,
ICondition condition
) {
String safeObjectName = getSafeSOQLString(objectName);
String filter = condition.getCondition();
String whereClause = filter.isEmpty() ? ” : ‘ WHERE ‘ + filter;
String dbQuery = ‘SELECT Id, Name FROM ‘ + safeObjectName + whereClause;
return Database.query(dbQuery);
}
}
[/code]

 
So, the Visualforce page instantiates the component this way:

[code language=”xml”]
<CustomPicklistController objectName="Account" conditionClass="{!conditionClass}"/>
[/code]

 
… and the condition class is returned from the VF page’s controller:

[code language=”java”]
public String getConditionClass() {
return ‘WhateverPageController.SpecificCondition’;
}
[/code]

 
(… where WhateverPageController is the VF page’s controller, and SpecificCondition is the specific condition class we saw before, nested within the controller class).

 

Conclusions

With the proposed solution, we have avoided having to open a door to potential SOQL injection attacks by not allowing to pass any data, but only information to access the code that actually generates the data, and all of this while leveraging @RemoteAction’s speed and keeping a great flexibility for the component’s user.

Thanks to my teammates at FinancialForce Ana Cristina López, Abel Martos and Shaun Doyle for their great contribution while looking for a solution to the problem.

Constructors and global classes in Apex

Valerio Sevilla Salesforce Dev , ,

It is well known that special care must be taken when creating global classes in Apex, because global classes, methods and member variables that are released in a package, cannot be removed in further versions of the same package. Some of the restrictions are well explained in official Salesforce docs like this one and this one, but there are some issues for which it is difficult to be prepared until we face the problem or test it ourselves. This is the case of the thrilling world of constructors in global classes.

How do constructors behave in global classes?

Pretty much the same as in non-global ones. But there are some important nuances to be considered.

Let’s say… we are creating a class with no “particular” way of instantiation. We don’t need to pass any argument, there’s no needed initialisation, and it can be freely instantiated. We can then omit the constructor:

[code language="java"]
global class InstantiateMeIfYouCan {
    global void foo() {
        System.assert(false, 'OK, so you instantiated me. Now what?');
    }
}
[/code]

Salesforce automatically generates an implicit, global constructor with no parameters. When releasing this class in a package, it can be instantiated and used from the customer’s org:

[code language="java"]
public class ISwearICanInstantiateYou {
    public static void foo() {
        new VSTest.InstantiateMeIfYouCan().foo();
    }
} [/code]

(… where VSTest is the package’s namespace).

So far, so good.

Updating our global class with a new constructor

It turns out that after releasing our class, now for some reason we want it to be instantiable only with a parameter specified in the constructor. So we add a new constructor:

[code language="java"]
global class InstantiateMeIfYouCan {
    global InstantiateMeIfYouCan(Boolean coolModeActivated) {

    }

    global void foo() {
        System.assert(false, 'OK, so you instantiated me. Now what?');
    }
}
[/code]

As with any other class, global or not, Apex automatically removes the implicit, no-param constructor. This common behaviour is detailed in the official documentation:

If you write a constructor that takes arguments, you can then use that constructor to create an object using those arguments. If you create a constructor that takes arguments, and you still want to use a no-argument constructor, you must include one in your code. Once you create a constructor for a class, you no longer have access to the default, no-argument public constructor. You must create your own.

And Salesforce allows us to package this new version. One could think that, implicit or not, the previously existing constructor with no parameters was global and had already been packaged, so the system would forbid us to perform this action. But the truth is, it doesn’t. When we release this new version, it is possible for the customer to upgrade the package in their org, despite of the fact that the new version breaks their existing code. When trying to run their code with the new version, an error similar to this arises:

Dependent class is invalid and needs recompilation: ISwearICanInstantiateYou: line 3, column 16: Constructor not defined: [VSTest.InstantiateMeIfYouCan]()

OK, so now we know this is a breaking change. Let’s try to fix this and release a new version.

Recovering the no-param constructor

We have already released our 1-param constructor, and it’s global, so there’s no way to return to the original status. Now the only thing we can do, is adding an explicit, no-param constructor:

[code language="java"]
global class InstantiateMeIfYouCan {
    global InstantiateMeIfYouCan() {

    }

    global InstantiateMeIfYouCan(Boolean coolModeActivated) {

    }

    global void foo() {
        System.assert(false, 'OK, so you instantiated me. Now what?');
    }
}
[/code]

Again, the system allows us to package the new version and release it. And again, the customer can install the new version with no issues. But when running their code…

Dependent class is invalid and needs recompilation: ISwearICanInstantiateYou: line 3, column 16: Package Visibility: Constructor is not visible: [VSTest.InstantiateMeIfYouCan].<Constructor>()

Weird, isn’t it? So we have explicitly added the constructor and set it as global. We know it is global. But the platform does not get it correctly, even though when entering the class in the customer’s org, it informs about the availability of those methods:

InstantiateMeIfYouCan

Actually, not so weird. The existing class ISwearICanInstantiateYou is linked to a previous version of our package. It’s a matter of changing the version it is using by editing the class and going to Version Settings. If we create another, new class in the customer’s org that makes use of the mysterious no-param constructor:

[code language="java"]
public class SoYouThoughtICouldNotInstantiateYou {
    public static void foo(){
        new VSTest.InstantiateMeIfYouCan().foo();
    }
} [/code]

It works directly because it is taking the latest version by default.

So, the fact that we didn’t explicitly declare the no-param constructor in the first version, caused issues in the end (our updates are throwing compile errors in customer’s org). This makes it highly advisable to always declare an explicit, global no-param constructor in our global classes in order to prevent potential issues. But it is not always possible: sometimes it does not make sense to allow customers to instantiate our class.

What if we create an explicit, private no-param constructor?

[code language="java"]
global class InstantiateMeIfYouCan {
    private InstantiateMeIfYouCan() {}

    global void foo() {
        System.assert(false, 'OK, so you instantiated me. Now what?');
    }
}
[/code]

This way we are “booking” the constructor and at the same time, preventing customers from using it unless we decide to make it global in a future version. This would work as long as we keep the restriction of not being able to instantiate the class with the default constructor from the outside. Otherwise, after releasing the first version with the private constructor, an update that changes it into a global one would obviously need an update in existing customer code to start leveraging it. We could think of an alternative solution that would work without them needing to update nothing but our package.

Implementing an explicit prohibition

Let’s create an explicit, global no-param constructor that throws a special exception:

[code language="java"]
global class InstantiateMeIfYouCan {
    global InstantiateMeIfYouCan() {
        throw new InvalidInstantiationException();
    }

    global InstantiateMeIfYouCan(Boolean coolModeActivated) {
        ...
    }

    global void foo() {
        System.assert(false, 'OK, so you instantiated me. Now what?');
    }
}
[/code]

By including the global constructor in the first version, we make sure it will be available in future releases if we need it. On the other hand, we are explicitly forbidding its usage by throwing an exception that we have created for this situation. And if we need to “unlock” the constructor and make it available in a new release, we can always change its implementation with no issues. Actually, the customer would need to update their code anyway in order to use the new constructor, unless they implemented something like this to anticipate this situation:

[code language="java"]
public class ISwearICanInstantiateYou {
    public static void foo() {
        VSTest.InstantiateMeIfYouCan instance;

        try {    // Firstly, try to instantiate with default values
            instance = new VSTest.InstantiateMeIfYouCan();
        }
        catch (VSTest.InvalidInstantiationException ex) {
            // Not available: use the constructor with an explicit argument
            instance = new VSTest.InstantiateMeIfYouCan(false);
        }

        instance.foo();
    }
} [/code]

So now, there’s no need at all for the customer to create a new version of their code or product, because it will be prepared to work with and without the no-param constructor in advance.