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

Fix issues with Rest2LDAP feature #79

Merged
merged 4 commits into from
Apr 18, 2023
Merged

Fix issues with Rest2LDAP feature #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
342f2ea
Fix NPE for empty resources. #78
pavelhoral Apr 17, 2023
44c6b3a
Fix startup issues. #78
pavelhoral Apr 17, 2023
253ed71
Revert "Clarifies a JavaDoc comment"
pavelhoral Apr 18, 2023
1058ece
Revert "Fix read-only sub-resource CREST API descr (#6)"
pavelhoral Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion opendj-cli/src/main/java/com/forgerock/opendj/cli/CliConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public final class CliConstants {

/** The minimum java specification supported string version. */
public static final float MINIMUM_JAVA_VERSION = 1.8F;
public static final float MINIMUM_JAVA_VERSION = 8f;

/** Default value for LDAP connection timeout. */
public static final int DEFAULT_LDAP_CONNECT_TIMEOUT = 30000;
Expand Down
2 changes: 1 addition & 1 deletion opendj-cli/src/main/java/com/forgerock/opendj/cli/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public static String wrapText(final String text, int width, final int indent) {
* If the java version we are running on is not compatible.
*/
public static void checkJavaVersion() throws ClientException {
final String version = System.getProperty("java.specification.version");
final String version = System.getProperty("java.specification.version").replaceAll("^1\\.", "");
if (Float.valueOf(version) < CliConstants.MINIMUM_JAVA_VERSION) {
final String javaBin = System.getProperty("java.home") + File.separator + "bin" + File.separator + "java";
throw new ClientException(ReturnCode.JAVA_VERSION_INCOMPATIBLE,
Expand Down
12 changes: 0 additions & 12 deletions ...rest2ldap-servlet/src/main/webapp/WEB-INF/classes/rest2ldap/endpoints/api/example-v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,6 @@
"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
4 changes: 2 additions & 2 deletions opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ObjectPropertyMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ Promise<JsonValue, ResourceException> read(final Context context, final Resource
@Override
public JsonValue apply(final List<Pair<String, JsonValue>> value) {
if (value.isEmpty()) {
// No subordinate attributes, so omit the entire JSON object from the resource.
return null;
// No subordinate attributes, we can return empty object.
return new JsonValue(Collections.emptyMap());
} else {
// Combine the sub-attributes into a single JSON object.
final Map<String, Object> result = new LinkedHashMap<>(value.size());
Expand Down
13 changes: 0 additions & 13 deletions opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReadOnlyRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

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 @@ -30,7 +28,6 @@
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 @@ -59,14 +56,4 @@ 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: 11 additions & 46 deletions opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Resource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,31 +470,6 @@ String getResourceId() {
return id;
}

/**
* Gets a unique name for the configuration of this resource as a service 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 @@ -547,7 +522,7 @@ ApiDescription instanceApi(boolean isReadOnly) {

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

resource.items(buildItems(isReadOnly));

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

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

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

private Services services(org.forgerock.api.models.Resource.Builder resource) {
return Services.services()
.put(serviceId, resource.build())
.put(id, resource.build())
.build();
}

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

private Paths paths() {
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, resource).build())
.put("", versionedPath().put(UNVERSIONED, resourceRef("#/services/" + id)).build())
.build();
}

Expand Down
88 changes: 10 additions & 78 deletions ...rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/Rest2LdapJsonConfiguratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,13 @@
import static org.forgerock.util.Options.defaultOptions;

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 Down Expand Up @@ -64,115 +60,51 @@ public class Rest2LdapJsonConfiguratorTest extends ForgeRockTestCase {

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

final DescribableRequestHandler handler = configureEndpoints(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/read-only-users",
"/api/groups");

assertThat(api.getPaths().getNames()).containsOnly("/api/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 createDescribableHandler(final File endpointsDir)
throws Exception {
final RequestHandler rh = createRequestHandler(endpointsDir);
final DescribableRequestHandler handler = new DescribableRequestHandler(rh);

private DescribableRequestHandler configureEndpoints(final File endpointsDir) throws Exception {
final RequestHandler rh = Rest2LdapJsonConfigurator.configureEndpoints(endpointsDir, Options.defaultOptions());
DescribableRequestHandler handler = new DescribableRequestHandler(rh);
handler.api(new CrestApiProducer(ID, VERSION));

return handler;
}

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

private ApiDescription requestApi(final DescribableRequestHandler handler, String uriPath) {
Context context = newRouterContext(uriPath);
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);
}

private static JsonValue parseJson(final String json) throws Exception {
static JsonValue parseJson(final String json) throws Exception {
try (StringReader r = new StringReader(json)) {
return new JsonValue(readJsonLenient(r));
}
Expand Down
12 changes: 0 additions & 12 deletions opendj-server-legacy/resource/config/rest2ldap/endpoints/api/example-v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,6 @@
"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
2 changes: 1 addition & 1 deletion opendj-server-legacy/src/main/java/org/opends/server/util/Platform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public final class Platform
private static final PlatformIMPL IMPL;

/** The minimum java supported version. */
public static final String JAVA_MINIMUM_VERSION_NUMBER = "7.0";
public static final String JAVA_MINIMUM_VERSION_NUMBER = "8";

static
{
Expand Down