WrenSecurity/wrends

Rest2LDAP CREST API descriptors ignore read-only collection sub-resources

Closed this issue · 2 comments

Summary

If a collection sub-resource defined in the Rest2LDAP endpoints JSON file is marked "isReadOnly": true, that endpoint does not appear at all in the API descriptor returned by CREST.

Affected Version

master (4.0.0-SNAPSHOT)

Steps to Reproduce

  1. Alter the example-v1.json file, and add the following under resourceTypes/example-v1/subResources (ensure you don't remove the existing users and groups sub-resource definitions):
                "all-users": {
                    "type": "collection",
                    "dnTemplate": "ou=people,dc=example,dc=com",
                    "resource": "frapi:opendj:rest2ldap:user:1.0",
                    "namingStrategy": {
                        "type": "clientDnNaming",
                        "dnAttribute": "uid"
                    },
                    "isReadOnly": true

2, Alter the Rest2LdapJsonConfiguratorTest, adding the line System.out.println(prettyPrint(api)); right after the parseJson(prettyPrint(api)); line.
3. Run the test.
4. Observe the console output from the test.

Expected results

The output includes the new all-users sub-resource, even though it's read-only. Something like the attached expected.json.

Actual results

The output omits all read-only collection sub-resources. See the attached actual.json.

Attachments

actual vs. expected JSON.zip

At first, I thought this issue was just an oversight on ForgeRock's part, since org.forgerock.opendj.rest2ldap.ReadOnlyRequestHandler does not define an api() method. It actually looks like org.forgerock.opendj.rest2ldap.Resource.collectionApi() is setup to be able to handle read-only collection sub-resources. (However, it still erroneously generates a create section at the top-level, even if the sub-resource is read-only).

So, I tried adding this to the handler, to ensure that the api() method gets forwarded on to the request handler being wrapped by the ReadOnlyRequestHandler:

final class ReadOnlyRequestHandler extends AbstractRequestHandler {
    // ... lines omitted ...
    @Override
    public ApiDescription api(ApiProducer<ApiDescription> producer) {
        if (delegate instanceof Describable) {
            return ((Describable<ApiDescription, Request>)delegate).api(producer);
        }
        else {
            return super.api(producer);
        }
    }

However, this didn't work. I got this error:

java.lang.IllegalStateException: The give Resource name already exists but the Resource objects are not equal
        at org.forgerock.api.CrestApiProducer.merge(CrestApiProducer.java:129)
        at org.forgerock.api.CrestApiProducer.merge(CrestApiProducer.java:45)
        at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:276)
        at org.forgerock.services.routing.AbstractRouter.updateApi(AbstractRouter.java:251)
        at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:244)
        at org.forgerock.opendj.rest2ldap.Resource.subResourcesApi(Resource.java:652)
        at org.forgerock.opendj.rest2ldap.SubResource$SubResourceHandler.api(SubResource.java:221)
        at org.forgerock.opendj.rest2ldap.SubResource$SubResourceHandler.api(SubResource.java:141)
        at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:268)
        at org.forgerock.services.routing.AbstractRouter.updateApi(AbstractRouter.java:251)
        at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:244)
        at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:109)
        at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:40)
        at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:268)
        at org.forgerock.services.routing.AbstractRouter.updateApi(AbstractRouter.java:251)
        at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:244)
        at org.forgerock.json.resource.FilterChain.api(FilterChain.java:272)
        at org.forgerock.json.resource.FilterChain.api(FilterChain.java:38)
        at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:268)
        at org.forgerock.services.routing.AbstractRouter.updateApi(AbstractRouter.java:251)
        at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:244)
        at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:109)
        at org.forgerock.opendj.rest2ldap.Rest2LdapJsonConfiguratorTest.createDescribableHandler(Rest2LdapJsonConfiguratorTest.java:128)
        at org.forgerock.opendj.rest2ldap.Rest2LdapJsonConfiguratorTest.testConfigureEndpointsWithApiDescription(Rest2LdapJsonConfiguratorTest.java:67)

The issue is that the code tries to generate two different service descriptions, but use the same name for both; they both get created as frapi:opendj:rest2ldap:user:1.0. One of the descriptions is read-only, while the other is read-write. I think that the code needs to take into account whether or not the resource is read-only or writable, and incorporate that into the service name somehow so there is no conflict.

That's why the attached expected.json file does exactly that. Instead of frapi:opendj:rest2ldap:user:1.0, it becomes frapi:opendj:rest2ldap:user:read-write:1.0 and frapi:opendj:rest2ldap:user:read-only:1.0.

Side note: given is spelled wrong. But that's likely a bug for... https://github.com/WrenSecurity/wrensec-commons ? I think?