Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Closed
GuyPaddock opened this issue Oct 25, 2017 · 2 comments
Labels

Comments

@GuyPaddock
Copy link

GuyPaddock commented Oct 25, 2017

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

@GuyPaddock
Copy link
Author

GuyPaddock commented Oct 25, 2017

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.

@GuyPaddock
Copy link
Author

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

GuyPaddock pushed a commit to GuyPaddock/wrends that referenced this issue Oct 26, 2017
Corrects three obscure defects in the Rest2LDAP implementation of CREST descriptors:
- Read-only sub-resources were not appearing at all in the CREST API description JSON.
- When read-only sub-resources appeared in the API description alongside writable sub-resources for the same models, the generated service name for the sub-resources was the same, leading to an IllegalStateException. Now, we generate a unique service name for each sub-resource based on its writability.
- A top-level `create` request was still being rendered in the API description for read-only sub-resources.
Kortanul added a commit that referenced this issue Oct 26, 2017
…y-desc

Fix read-only sub-resource CREST API descr (#6)
@GuyPaddock GuyPaddock changed the title Rest2LDAP CREST API descriptors ignore read-only collection sub-resources Bug: Rest2LDAP CREST API descriptors ignore read-only collection sub-resources Oct 29, 2017
@Kortanul Kortanul changed the title Bug: Rest2LDAP CREST API descriptors ignore read-only collection sub-resources Rest2LDAP CREST API descriptors ignore read-only collection sub-resources Oct 29, 2017
pavelhoral added a commit to orchitech/wrends that referenced this issue Apr 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants