Skip to content

Commit

Permalink
Fix read-only sub-resource CREST API descr (WrenSecurity#6)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
Guy Elsmore-Paddock committed Oct 26, 2017
1 parent a2f838c commit 40be863
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 22 deletions.
12 changes: 12 additions & 0 deletions ...rest2ldap-servlet/src/main/webapp/WEB-INF/classes/rest2ldap/endpoints/api/example-v1.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 21,18 @@
"dnAttribute": "uid"
}
},
// This resource is the same as "users", but read-only.
// Users cannot be created, modified, or deleted through this sub-resource.
"read-only-users": {
"type": "collection",
"dnTemplate": "ou=people,dc=example,dc=com",
"resource": "frapi:opendj:rest2ldap:user:1.0",
"namingStrategy": {
"type": "clientDnNaming",
"dnAttribute": "uid"
},
"isReadOnly": true
},
"groups": {
"type": "collection",
"dnTemplate": "ou=groups,dc=example,dc=com",
Expand Down
13 changes: 13 additions & 0 deletions opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReadOnlyRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 18,8 @@

import static org.forgerock.opendj.rest2ldap.Rest2ldapMessages.ERR_READ_ONLY_ENDPOINT;

import org.forgerock.api.models.ApiDescription;
import org.forgerock.http.ApiProducer;
import org.forgerock.json.resource.BadRequestException;
import org.forgerock.json.resource.QueryRequest;
import org.forgerock.json.resource.QueryResourceHandler;
Expand All @@ -28,6 30,7 @@
import org.forgerock.json.resource.ResourceException;
import org.forgerock.json.resource.ResourceResponse;
import org.forgerock.services.context.Context;
import org.forgerock.services.descriptor.Describable;
import org.forgerock.util.promise.Promise;

/**
Expand Down Expand Up @@ -56,4 59,14 @@ public Promise<ResourceResponse, ResourceException> handleRead(
protected <V> Promise<V, ResourceException> handleRequest(final Context context, final Request request) {
return new BadRequestException(ERR_READ_ONLY_ENDPOINT.get().toString()).asPromise();
}

@Override
public ApiDescription api(ApiProducer<ApiDescription> producer) {
if (delegate instanceof Describable) {
return ((Describable<ApiDescription, Request>)delegate).api(producer);
}
else {
return super.api(producer);
}
}
}
57 changes: 46 additions & 11 deletions opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Resource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 470,31 @@ String getResourceId() {
return id;
}

/**
* Gets a unique name for the configuration of this resource in CREST.
*
* The name is the combination of the resource type and the writability of the resource. For
* example, {@code frapi:opendj:rest2ldap:group:1.0:read-write} or
* {@code frapi:opendj:rest2ldap:user:1.0:read-only}. Multiple resources can share the same
* service description if they manipulate the same resource type and have the same writability.
*
* @param isReadOnly
* Whether or not this resource is read-only.
*
* @return The unique service ID for this resource, given the specified writability.
*/
String getServiceId(boolean isReadOnly) {
StringBuilder serviceId = new StringBuilder(this.getResourceId());

if (isReadOnly) {
serviceId.append(":read-only");
} else {
serviceId.append(":read-write");
}

return serviceId.toString();
}

void build(final Rest2Ldap rest2Ldap) {
// Prevent re-entrant calls.
if (isBuilt) {
Expand Down Expand Up @@ -522,7 547,7 @@ ApiDescription instanceApi(boolean isReadOnly) {

org.forgerock.api.models.Resource.Builder resource = org.forgerock.api.models.Resource.
resource()
.title(id)
.title(this.getServiceId(isReadOnly))
.description(toLS(description))
.resourceSchema(schemaRef("#/definitions/" id))
.mvccSupported(isMvccSupported());
Expand All @@ -539,8 564,8 @@ ApiDescription instanceApi(boolean isReadOnly) {
return ApiDescription.apiDescription()
.id("unused").version("unused")
.definitions(definitions())
.services(services(resource))
.paths(paths())
.services(services(resource, isReadOnly))
.paths(paths(isReadOnly))
.errors(errors())
.build();
}
Expand All @@ -555,13 580,17 @@ ApiDescription instanceApi(boolean isReadOnly) {
ApiDescription collectionApi(boolean isReadOnly) {
org.forgerock.api.models.Resource.Builder resource = org.forgerock.api.models.Resource.
resource()
.title(id)
.title(this.getServiceId(isReadOnly))
.description(toLS(description))
.resourceSchema(schemaRef("#/definitions/" id))
.mvccSupported(isMvccSupported());

resource.items(buildItems(isReadOnly));
resource.create(createOperation(CreateMode.ID_FROM_SERVER));

if (!isReadOnly) {
resource.create(createOperation(CreateMode.ID_FROM_SERVER));
}

resource.query(Query.query()
.stability(EVOLVING)
.type(QueryType.FILTER)
Expand All @@ -580,23 609,29 @@ ApiDescription collectionApi(boolean isReadOnly) {
return ApiDescription.apiDescription()
.id("unused").version("unused")
.definitions(definitions())
.services(services(resource))
.paths(paths())
.services(services(resource, isReadOnly))
.paths(paths(isReadOnly))
.errors(errors())
.build();
}

private Services services(org.forgerock.api.models.Resource.Builder resource) {
private Services services(org.forgerock.api.models.Resource.Builder resource,
boolean isReadOnly) {
final String serviceId = this.getServiceId(isReadOnly);

return Services.services()
.put(id, resource.build())
.put(serviceId, resource.build())
.build();
}

private Paths paths() {
private Paths paths(boolean isReadOnly) {
final String serviceId = this.getServiceId(isReadOnly);
final org.forgerock.api.models.Resource resource = resourceRef("#/services/" serviceId);

return Paths.paths()
// do not put anything in the path to avoid unfortunate string concatenation
// also use UNVERSIONED and rely on the router to stamp the version
.put("", versionedPath().put(UNVERSIONED, resourceRef("#/services/" id)).build())
.put("", versionedPath().put(UNVERSIONED, resource).build())
.build();
}

Expand Down
104 changes: 93 additions & 11 deletions ...rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/Rest2LdapJsonConfiguratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 23,17 @@
import static org.forgerock.util.Options.*;

import java.io.File;
import java.io.IOException;
import java.io.StringReader;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;

import org.forgerock.api.CrestApiProducer;
import org.forgerock.api.models.ApiDescription;
import org.forgerock.api.models.Items;
import org.forgerock.api.models.Resource;
import org.forgerock.api.models.Services;
import org.forgerock.http.routing.UriRouterContext;
import org.forgerock.http.util.Json;
import org.forgerock.json.JsonValue;
Expand All @@ -53,58 57,136 @@
public class Rest2LdapJsonConfiguratorTest extends ForgeRockTestCase {
private static final String ID = "frapi:opendj:rest2ldap";
private static final String VERSION = "4.0.0";

private static final Path SERVLET_MODULE_PATH =
getPathToMavenModule("opendj-rest2ldap-servlet");

private static final Path CONFIG_DIR = Paths.get(
"../opendj-rest2ldap-servlet/src/main/webapp/WEB-INF/classes/rest2ldap");
SERVLET_MODULE_PATH.toString(), "src", "main", "webapp", "WEB-INF", "classes", "rest2ldap");

@Test
public void testConfigureEndpointsWithApiDescription() throws Exception {
final DescribableRequestHandler handler = configureEndpoints(CONFIG_DIR.resolve("endpoints").toFile());
final DescribableRequestHandler handler =
createDescribableHandler(CONFIG_DIR.resolve("endpoints").toFile());

final ApiDescription api = requestApi(handler, "api/users/bjensen");

assertThat(api).isNotNull();

// Ensure we can can pretty print and parse back the generated api description
parseJson(prettyPrint(api));

assertThat(api.getId()).isEqualTo(ID);
assertThat(api.getVersion()).isEqualTo(VERSION);
assertThat(api.getPaths().getNames()).containsOnly("/api/users", "/api/groups");

assertThat(api.getPaths().getNames()).containsOnly(
"/api/users",
"/api/read-only-users",
"/api/groups");

assertThat(api.getDefinitions().getNames()).containsOnly(
"frapi:opendj:rest2ldap:object:1.0",
"frapi:opendj:rest2ldap:group:1.0",
"frapi:opendj:rest2ldap:user:1.0",
"frapi:opendj:rest2ldap:posixUser:1.0");

final Services services = api.getServices();

assertThat(services.getNames()).containsOnly(
"frapi:opendj:rest2ldap:user:1.0:read-write",
"frapi:opendj:rest2ldap:user:1.0:read-only",
"frapi:opendj:rest2ldap:group:1.0:read-write");

final String[] readOnlyServices = new String[] {
"frapi:opendj:rest2ldap:user:1.0:read-only"
};

for (String serviceName : readOnlyServices) {
final Resource service = services.get(serviceName);
final Items items = service.getItems();

assertThat(service.getCreate()).isNull();
assertThat(items.getCreate()).isNull();
assertThat(items.getUpdate()).isNull();
assertThat(items.getDelete()).isNull();
assertThat(items.getPatch()).isNull();

assertThat(items.getRead()).isNotNull();
}

final String[] writableServices = new String[] {
"frapi:opendj:rest2ldap:user:1.0:read-write",
"frapi:opendj:rest2ldap:group:1.0:read-write"
};

for (String serviceName : writableServices) {
final Resource service = services.get(serviceName);
final Items items = service.getItems();

assertThat(service.getCreate()).isNotNull();
assertThat(items.getCreate()).isNotNull();
assertThat(items.getUpdate()).isNotNull();
assertThat(items.getDelete()).isNotNull();
assertThat(items.getPatch()).isNotNull();
assertThat(items.getRead()).isNotNull();
}
}

private RequestHandler createRequestHandler(final File endpointsDir) throws IOException {
return Rest2LdapJsonConfigurator.configureEndpoints(endpointsDir, Options.defaultOptions());
}

private DescribableRequestHandler configureEndpoints(final File endpointsDir) throws Exception {
final RequestHandler rh = Rest2LdapJsonConfigurator.configureEndpoints(endpointsDir, Options.defaultOptions());
DescribableRequestHandler handler = new DescribableRequestHandler(rh);
private DescribableRequestHandler createDescribableHandler(final File endpointsDir)
throws Exception {
final RequestHandler rh = createRequestHandler(endpointsDir);
final DescribableRequestHandler handler = new DescribableRequestHandler(rh);

handler.api(new CrestApiProducer(ID, VERSION));

return handler;
}

private ApiDescription requestApi(final DescribableRequestHandler handler, String uriPath) {
Context context = newRouterContext(uriPath);
Request request = newApiRequest(resourcePath(uriPath));
private ApiDescription requestApi(final DescribableRequestHandler handler,
final String uriPath) {
final Context context = newRouterContext(uriPath);
final Request request = newApiRequest(resourcePath(uriPath));

return handler.handleApiRequest(context, request);
}

private Context newRouterContext(final String uriPath) {
Context ctx = new RootContext();

ctx = new Rest2LdapContext(ctx, rest2Ldap(defaultOptions()));
ctx = new UriRouterContext(ctx, null, uriPath, Collections.<String, String> emptyMap());

return ctx;
}

private String prettyPrint(Object o) throws Exception {
final ObjectMapper objectMapper =
new ObjectMapper().registerModules(new Json.LocalizableStringModule(), new Json.JsonValueModule());
new ObjectMapper().registerModules(
new Json.LocalizableStringModule(),
new Json.JsonValueModule());

final ObjectWriter writer = objectMapper.writer().withDefaultPrettyPrinter();

return writer.writeValueAsString(o);
}

static JsonValue parseJson(final String json) throws Exception {
private static JsonValue parseJson(final String json) throws Exception {
try (StringReader r = new StringReader(json)) {
return new JsonValue(readJsonLenient(r));
}
}

private static Path getPathToClass(Class<?> clazz) {
return Paths.get(clazz.getProtectionDomain().getCodeSource().getLocation().getPath());
}

private static Path getPathToMavenModule(String moduleName) {
final Path testClassPath = getPathToClass(Rest2LdapJsonConfiguratorTest.class);

return Paths.get(testClassPath.toString(), "..", "..", "..", moduleName);
}
}
12 changes: 12 additions & 0 deletions opendj-server-legacy/resource/config/rest2ldap/endpoints/api/example-v1.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 21,18 @@
"dnAttribute": "uid"
}
},
// This resource is the same as "users", but read-only.
// Users cannot be created, modified, or deleted through this sub-resource.
"read-only-users": {
"type": "collection",
"dnTemplate": "ou=people,dc=example,dc=com",
"resource": "frapi:opendj:rest2ldap:user:1.0",
"namingStrategy": {
"type": "clientDnNaming",
"dnAttribute": "uid"
},
"isReadOnly": true
},
"groups": {
"type": "collection",
"dnTemplate": "ou=groups,dc=example,dc=com",
Expand Down

0 comments on commit 40be863

Please sign in to comment.