AppExchange Security Review – CRUD Security and FLS (Field Level Security)

I will soon be submitting an application into the Force.com AppExchange Security Review process. In preparation I took a look at articles like Testing CRUD and FLS Enforcement and so realized that I needed to add some extra code to my custom UI code.

The first thing I looked at was this apparently salesforce sponsored project OWASP ESAPI implementation for Force.com. Useful to look at but I got no response to emails or issues so be warned.

In the end I added the minimum code I could think of which blocks the custom UI if any object or field level permissions are turned off as at present I do not have requirements for fine grained control. Given that some of the custom UI manipulates multiple objects at once to accomplish the required business logic, this sails close to the describe call governor limit. So don’t manipulate more than 10 types in a piece of code because you won’t be able to implement the (apparently) required security…

The biggest issue I ran into was in the testing of this code. The unit tests need to check that users running under profiles that allow access are not blocked and that users running under profiles that don’t allow access are blocked. But the bad news is that there appears to be some insidious caching that gets in the way. See my response-less post How to test CRUD Security and FLS despite implicit describe caching?. And its not that the describe caching just spans multiple test methods in a test class but it appears to span all the test classes in a test run too. So right now the “don’t allow access” tests are turned off. Not good.

Advertisements

When “Generate from WSDL” fails – hand-coding web service calls

The starting point for invoking a web service from Force.com is to see what happens when you use the “Generate From WSDL” button on the WSDL. See e.g. Apex Web Services and Callouts. This mechanism is also sometimes referred to as WSDL2Apex.

Most WSDLs will need various edits to comply with the limitations of the mechanism and it is worth trying hard to make the necessary changes. If you succeed then you will have generated Apex code that is in theory compliant with the WSDL and its embedded schemas. Most importantly you will have classes representing the request and response objects that make creating requests and processing responses simple. You will have to do some hacking of the generated classes to provide entry points for tests to achieve reasonable code coverage. Also I had one case where the code generated but was broken at run-time because of a rather unusual no-namespace element so test the generated code before declaring victory. (The API that the generated code works against is not documented which is a pity.) And as you can’t invoke a web service from an Apex test you will have to put together some UI to do the testing.

But unless the WSDL has been designed with Force.com in mind you may find that this approach is a dead end. Facilities for hand-coding in these cases have improved and this article provides an example of this. If all you need to do is make a few simple requests and process a few simple responses then this is a viable if undesirable option. But if you are working against a service that changes constantly or has a large number of requests and responses or complex requests and responses then you are in trouble…

My #1 piece of advice is to try out the service first using more capable tools. I used Axis hosted in Eclipse, with wsdl2java succeeding in generating the service client code where Force.com had failed. That then made it quick to write a JUnit test to exercise the service. And most importantly Axis includes a proxy server tcpmon that is easy to setup and lets you capture the request and response XML and also the HTTP headers. So you can establish exactly what works and use this as a reference when your Apex implementation is silently failing. (I couldn’t see a way to examine the request and response XML and headers in Force.com, and routing through something like tcpmon would require tcpmon to be running on a machine that is accessible from the internet.)

Note that I had some pain when working in Axis in that the service I was invoking was created using .Net and had very specific needs in how the addressing SOAP headers were handled. Google and the open nature of Axis came to the rescue.

So with an understanding of what is needed you can go ahead and write the code. Here is what I ended up with. This code is using DOM classes but consider using XmlStreamWriter and XmlStreamReader if they fit your data patterns better. Note this code won’t run – I’ve removed specifics like the endpoint and actual namespace.

First a base class to reduce code duplication:

public abstract class SampleBase {

    public static final String SE_NAMESPACE = 'http://www.w3.org/2003/05/soap-envelope';
    public static final String SE_PREFIX = 'se';

    public static final String TE_NAMESPACE = 'http://tempuri.org/';
    public static final String TE_PREFIX = 'te';

    public static final String MD_NAMESPACE = 'http://sample.com';
    public static final String MD_PREFIX = 'md';

    public static final String AD_NAMESPACE = 'http://www.w3.org/2005/08/addressing';
    public static final String AD_PREFIX = 'ad';

    public class SampleException extends Exception {
    }

    /**
     * Overall wrapping method. Call this to invoke the particular operation.
     */
    public virtual Object invoke() {

        HttpRequest request = createRequest();

        // Avoid unsupported media type error
        request.setHeader('Content-Type', 'application/soap+xml; charset=UTF-8');

        System.debug('request=' + request.getBody());

        Http h = new Http();
        HttpResponse response = h.send(request);
        if (response.getStatusCode() != 200) {
            throw new SampleException('Not ok;'
                    + ' statusCode='+ response.getStatusCode()
                    + ' status=' + response.getStatus()
                    );
        }
        System.debug('response=' + response.getBody());
        return parseResponse(response);
    }

    public virtual HttpRequest createRequest() {
        HttpRequest request = new HttpRequest();
        request.setEndPoint(getUrl());
        request.setMethod('POST');
        request.setBody(createRequestDocument().toXmlString());
        return request;
    }

    public abstract Dom.Document createRequestDocument();

    public virtual Object parseResponse(HttpResponse response) {
        return parseResponseDocument(response.getBodyDocument());
    }

    public abstract Object parseResponseDocument(Dom.Document response);

    protected String getUrl() {
        return 'http://sample.com/WebService/SampleService';
    }

    protected String getLicenseKey() {
        return 'a license key from e.g. custom settings';
    }
}

Then here is an example of one operation. This is the simplest of the ones I needed to do, just returning a string session id that then needed to be passed into other operations:

public class SampleAuthenticate extends SampleBase {

    private static final String ACTION_URL = 'http://tempuri.org/SampleService/Authenticate';

    public override HttpRequest createRequest() {
        HttpRequest request = super.createRequest();
        request.setHeader('SOAPAction', ACTION_URL);
        return request;
    }

    public override Dom.Document createRequestDocument() {

        Dom.Document doc = new Dom.Document();
        Dom.XMLNode envelope = doc.createRootElement('Envelope', SE_NAMESPACE, SE_PREFIX);

        Dom.XMLNode header = envelope.addChildElement('Header', SE_NAMESPACE, SE_PREFIX);
        header.addChildElement('To', AD_NAMESPACE, AD_PREFIX).addTextNode(getUrl());
        header.addChildElement('Action', AD_NAMESPACE, AD_PREFIX).addTextNode(ACTION_URL);

        envelope.addChildElement('Body', SE_NAMESPACE, SE_PREFIX)
                .addChildElement('Authenticate', TE_NAMESPACE, TE_PREFIX)
                .addChildElement('licenseKey', TE_NAMESPACE, TE_PREFIX)
                .addTextNode(getLicenseKey());

        return doc;
    }

    public override Object parseResponseDocument(Dom.Document response) {

        Object sessionId = response
                .getRootElement()
                .getChildElement('Body', SE_NAMESPACE)
                .getChildElement('AuthenticateResponse', TE_NAMESPACE)
                .getChildElement('AuthenticateResult', TE_NAMESPACE)
                .getChildElement('SessionId', MD_NAMESPACE)
                .getText();
        return sessionId;
    }
}

And here is a test case that covers the request creation (to a degree) and checks that a sample response XML file (obtained using tcpmon) and stored as a static resource can be parsed and produces the right result:

@isTest
private class SampleAuthenticateTest {

    @isTest
    static void testCreateRequest() {
        String xml = new SampleAuthenticate().createRequestDocument().toXmlString();
        System.debug('xml=' + xml);
        System.assertNotEquals(null, xml);
        System.assert(xml.length() > 100);

        // Not much value in more asserts here; its what the remote service thinks that matters
    }

    @isTest
    static void testParseResponse() {
        Object sessionId = new SampleAuthenticate().parseResponseDocument(
                getDocument('TestSampleAuthenticationResponse')
                );
        System.assertEquals('session id in sample response xml', sessionId);
    }

    private static Dom.Document getDocument(String staticResourceName) {
        StaticResource sr = [select Body from StaticResource where Name = :staticResourceName];
        Dom.Document doc = new Dom.Document();
        doc.load(sr.Body.toString());
        return doc;
    }
}

In case you are thinking that this doesn’t look too bad, keep in mind that most service operations have multiple request arguments and return complex objects with repeating elements and multiple levels of nesting. And the XML Schema referenced by the WSDL can model a huge variety of structures – any decent XML Schema reference is hundreds of pages long – so the variations that your code can have to deal with if the service implementor has got fancy or clever are frightening.

Even in simple cases that leaves you manually creating multiple data holding classes. You should probably make each data holder responsible for its own parsing by passing the relevant Dom.XMLNode to it. You should be able to Google various patterns for this type of code from the bad old days before tooling like Java’s JAXB made this sort of work unnecessary in other technology stacks. And after a day of coding these data holders and a pile of test cases to make sure they work, and then another day of NPEs when you run against the real service, just remember that JAXB would do that same work in seconds…

One final oddity is that the invoke method in the base class returning an Object seemed like an obvious way to go so the caller can downcast as needed. But when you implement an operation that returns a List of objects you discover that in Apex a List is not an Object at all. Yuk.

A c:CheckAllOrNone for custom pages

The default UI allows a “Display Checkboxes (for Multi-Record Selection)” option to be specified that results in a checkbox per table row plus a “Toggle All Rows” checkbox in the column heading. The custom component below provides this “Toggle All Rows” facility for custom UI. Here is an example of it in use:

(The per-row checkbox is usually implemented by wrapping the SObjects in a class that provides a “selected” boolean; pity the platform doesn’t provide a standard mechanism for this as the wrappers add a lot of noise to the controller.)

To use the component add it to the header facet of the table column that contains the checkboxes:

<apex:column>
    <apex:facet name="header">
        <c:CheckAllOrNone/>
    </apex:facet>
    <apex:inputCheckbox value="{!row.Selected}" title="Select"/>
</apex:column>

Here is the definition of the component. It makes assumptions about the structure of the generated HTML to avoid the need for explicit id values to be be specified:

<apex:component>
<script type="text/javascript">
function cvCheckAllOrNone(allOrNoneCheckbox) {

    // Find parent table
    var container = allOrNoneCheckbox;
    while (container.tagName != "TABLE") {
        container = container.parentNode;
    }

    // Switch all checkboxes
    var inputs = container.getElementsByTagName("input");
    var checked = allOrNoneCheckbox.checked;
    for (var i = 0; i < inputs.length; i++) {
        var input = inputs.item(i);
        if (input.type == "checkbox") {
            if (input != allOrNoneCheckbox) {
                input.checked = checked;
            }
        }
    }
}
</script>
<apex:inputCheckbox onclick="cvCheckAllOrNone(this)" title="Toggle All Rows"/>
</apex:component>