Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Handle non-existing groups gracefully
Grouper returns status code of 500 if we ask specifically
for a group that does not exist. This is now handled correctly.

Resolves MID-6756 and MID-6900.
  • Loading branch information
mederly committed Mar 30, 2021
1 parent f4a934e commit 3517396
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 45 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -27,7 +27,7 @@
</parent>

<artifactId>connector-grouper-rest</artifactId>
<version>0.6</version>
<version>0.7</version>
<packaging>jar</packaging>

<name>Grouper REST Connector</name>
Expand Down
Expand Up @@ -162,7 +162,7 @@ private void getAllGroupsNoMembers(ResultsHandler handler) {
}

private boolean executeFindGroups(HttpPost request, JSONObject body, ResultsHandler handler) {
JSONObject response = callRequest(request, body);
JSONObject response = callRequest(request, body, null).getResponse();
checkSuccess(response, J_WS_FIND_GROUPS_RESULTS);
JSONArray groups = getArray(response, false, J_WS_FIND_GROUPS_RESULTS, J_GROUP_RESULTS);
if (groups != null) {
Expand All @@ -176,7 +176,23 @@ private boolean executeFindGroups(HttpPost request, JSONObject body, ResultsHand
}

private boolean executeGetMembers(HttpPost request, JSONObject body, ResultsHandler handler) {
JSONObject response = callRequest(request, body);
CallResponse callResponse = callRequest(request, body, (statusCode, responseBody) -> {
JSONObject errorResponse = new JSONObject(responseBody);
JSONObject resultMetadata = (JSONObject) getIfExists(errorResponse, J_WS_GET_MEMBERS_RESULTS, J_RESULTS, J_RESULT_METADATA);
String resultCode = resultMetadata != null ? getStringOrNull(resultMetadata, J_RESULT_CODE) : null;
boolean notFound = "GROUP_NOT_FOUND".equals(resultCode);
if (notFound) {
return CallResponse.error(responseBody);
} else {
return null;
}
});

if (!callResponse.isSuccess()) {
return true;
}

JSONObject response = callResponse.getResponse();
checkSuccess(response, J_WS_GET_MEMBERS_RESULTS);
JSONObject gObject = (JSONObject) get(response, J_WS_GET_MEMBERS_RESULTS, J_RESULTS, J_WS_GROUP);
String name = getStringOrNull(gObject, J_NAME);
Expand Down Expand Up @@ -251,7 +267,7 @@ private void checkStemExists(String stemName) {
.put(J_WS_STEM_QUERY_FILTER, new JSONObject()
.put(J_STEM_QUERY_FILTER_TYPE, VAL_FIND_BY_STEM_NAME)
.put(J_STEM_NAME, stemName)));
JSONObject response = callRequest(request, body);
JSONObject response = callRequest(request, body, null).getResponse();
checkSuccess(response, J_WS_FIND_STEMS_RESULTS);
stems = getArray(response, true, J_WS_FIND_STEMS_RESULTS, J_STEM_RESULTS);
} catch (RuntimeException | URISyntaxException e) {
Expand Down
Expand Up @@ -70,7 +70,8 @@ public class Processor {
static final String J_STEM_RESULTS = "stemResults";
static final String J_GROUP_RESULTS = "groupResults";
static final String J_WS_GROUP_LOOKUPS = "wsGroupLookups";
private static final String J_RESULT_METADATA = "resultMetadata";
static final String J_RESULT_METADATA = "resultMetadata";
static final String J_RESULT_CODE = "resultCode";
private static final String J_SUCCESS = "success";

static final String J_WS_SUBJECTS = "wsSubjects";
Expand All @@ -96,18 +97,14 @@ public class Processor {
this.configuration = configuration;
}

JSONObject callRequest(HttpEntityEnclosingRequestBase request, JSONObject payload) {
CallResponse callRequest(HttpEntityEnclosingRequestBase request, JSONObject payload, ErrorHandler errorHandler) {
request.addHeader("Content-Type", Processor.CONTENT_TYPE_JSON);
request.addHeader("Authorization", "Basic " + getAuthEncoded());
request.setEntity(new ByteArrayEntity(payload.toString().getBytes(StandardCharsets.UTF_8)));
LOG.info("Payload: {0}", payload); // we don't log the whole request, as it contains the (encoded) password
try (CloseableHttpResponse response = execute(request)) {
LOG.info("Response: {0}", response);
processResponseErrors(response);

String result = EntityUtils.toString(response.getEntity());
LOG.info("Response body: {0}", result);
return new JSONObject(result);
return processResponse(response, errorHandler);
} catch (IOException e) {
String msg = "Request failed: problem occurred during execute request with uri: " + request.getURI() + ": \n\t" + e.getLocalizedMessage();
LOG.error("{0}", msg);
Expand Down Expand Up @@ -157,11 +154,28 @@ private CloseableHttpResponse execute(HttpUriRequest request) {
* Checks HTTP response for errors. If the response is an error then the
* method throws the ConnId exception that is the most appropriate match for
* the error.
*
* @return true if the processing can continue
*/
private void processResponseErrors(CloseableHttpResponse response) {
private CallResponse processResponse(CloseableHttpResponse response, ErrorHandler errorHandler) throws IOException {

int statusCode = response.getStatusLine().getStatusCode();
LOG.info("Status code: {0}", statusCode);

String result = null;
try {
result = EntityUtils.toString(response.getEntity());
LOG.info("Response body: {0}", result);
} catch (IOException e) {
if (statusCode >= 200 && statusCode <= 299) {
throw e;
} else {
LOG.warn("cannot read response body: {0}", e, e);
}
}

if (statusCode >= 200 && statusCode <= 299) {
return;
return CallResponse.ok(result);
}

if (statusCode == 401 || statusCode == 403) {
Expand All @@ -172,29 +186,41 @@ private void processResponseErrors(CloseableHttpResponse response) {
throw new InvalidCredentialException(msg);
}

String responseBody = null;
try {
responseBody = EntityUtils.toString(response.getEntity());
} catch (IOException e) {
LOG.warn("cannot read response body: {0}", e, e);
}
String msg = "HTTP error " + statusCode + " " + response.getStatusLine().getReasonPhrase() + " : " + responseBody;
LOG.error("{0}", msg);
String msg = "HTTP error " + statusCode + " " + response.getStatusLine().getReasonPhrase() + " : " + result;
closeResponse(response);
if (statusCode == 400 || statusCode == 405 || statusCode == 406) {
throw new ConnectorIOException(msg);
} else if (statusCode == 402 || statusCode == 407) {
throw new PermissionDeniedException(msg);
} else if (statusCode == 404 || statusCode == 410) {
throw new UnknownUidException(msg);
} else if (statusCode == 408) {
throw new OperationTimeoutException(msg);
} else if (statusCode == 412) {
throw new PreconditionFailedException(msg);
} else if (statusCode == 418) {
throw new UnsupportedOperationException("Sorry, no coffee: " + msg);
} else {
try {
if (statusCode == 400 || statusCode == 405 || statusCode == 406) {
throw new ConnectorIOException(msg);
} else if (statusCode == 402 || statusCode == 407) {
throw new PermissionDeniedException(msg);
} else if (statusCode == 404 || statusCode == 410) {
throw new UnknownUidException(msg);
} else if (statusCode == 408) {
throw new OperationTimeoutException(msg);
} else if (statusCode == 412) {
throw new PreconditionFailedException(msg);
} else if (statusCode == 418) {
throw new UnsupportedOperationException("Sorry, no coffee: " + msg);
}

if (errorHandler != null) {
try {
CallResponse callResponse = errorHandler.handleError(statusCode, result);
if (callResponse != null) {
return callResponse;
}
} catch (Exception e) {
// TODO Consider improving this
throw new ConnectorException("Exception while handling error. Original message: " +
msg + ", exception: " + e.getMessage(), e);
}
}

throw new ConnectorException(msg);

} catch (Exception e) {
LOG.error("{0}", msg);
throw e;
}
}

Expand Down Expand Up @@ -348,4 +374,39 @@ private boolean groupNameMatches(String name, String[] patterns) {
}
return false;
}

@FunctionalInterface
public interface ErrorHandler {

/**
* Returns null if the error couldn't be handled
*/
CallResponse handleError(int statusCode, String responseBody);
}

static class CallResponse {
private final boolean success;
private final JSONObject response;

private CallResponse(boolean success, JSONObject response) {
this.success = success;
this.response = response;
}

static CallResponse ok(String text) {
return new CallResponse(true, new JSONObject(text));
}

static CallResponse error(String text) {
return new CallResponse(false, new JSONObject(text));
}

boolean isSuccess() {
return success;
}

JSONObject getResponse() {
return response;
}
}
}
Expand Up @@ -36,9 +36,11 @@ class AbstractTest {
// Test configuration
static final String TEST_USER = "banderson";
static final String TEST_GROUP = "etc:sysadmingroup";
static final String TEST_GROUP_NON_EXISTENT = "etc:thisGroupDoesNotExist";
static final String TEST_UUID_NON_EXISTENT = "dd089842948329438249284928289XXX";

// Connector configuration
private static final String BASE_URL = "https://192.168.56.101:9443";
private static final String BASE_URL = "https://localhost:9443";
private static final String ADMIN_USERNAME = TEST_USER;
private static final String ADMIN_PASSWORD = "password";
private static final String BASE_STEM = "etc";
Expand Down
Expand Up @@ -41,22 +41,22 @@ public class GroupTest extends AbstractTest {

private String uuid;

@Test(priority = 1)
@Test(priority = 100)
public void initialization() {
grouperConnector.init(getConfiguration());
}

@Test(priority = 2)
@Test(priority = 110)
public void testSchema() {
grouperConnector.schema();
}

@Test(priority = 3)
@Test(priority = 120)
public void testTestOperation() {
grouperConnector.test();
}

@Test(priority = 4)
@Test(priority = 200)
public void testFindByGroupName() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
Expand All @@ -69,7 +69,17 @@ public void testFindByGroupName() {
uuid = group.getUid().getUidValue();
}

@Test(priority = 10)
@Test(priority = 210)
public void testFindByGroupNameNonExistent() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
.equalTo(AttributeBuilder.build(ATTR_NAME, TEST_GROUP_NON_EXISTENT));

grouperConnector.executeQuery(OC_GROUP, filter, handler, options);
assertEquals("Wrong # of groups retrieved", results.size(), 0);
}

@Test(priority = 220)
public void testFindByGroupNameWithMembers() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
Expand All @@ -83,7 +93,17 @@ public void testFindByGroupNameWithMembers() {
assertEquals("Wrong members", Collections.singletonList(TEST_USER), members);
}

@Test(priority = 12)
@Test(priority = 230)
public void testFindByGroupNameWithMembersNonExistent() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
.equalTo(AttributeBuilder.build(ATTR_NAME, TEST_GROUP_NON_EXISTENT));

grouperConnector.executeQuery(OC_GROUP, filter, handler, getMembersOptions());
assertEquals("Wrong # of groups retrieved", results.size(), 0);
}

@Test(priority = 240)
public void testFindByGroupUuid() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
Expand All @@ -95,7 +115,17 @@ public void testFindByGroupUuid() {
System.out.println("Found group: " + group);
}

@Test(priority = 13)
@Test(priority = 250)
public void testFindByGroupUuidNonExistent() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
.equalTo(AttributeBuilder.build(ATTR_UUID, TEST_UUID_NON_EXISTENT));

grouperConnector.executeQuery(OC_GROUP, filter, handler, options);
assertEquals("Wrong # of groups retrieved", results.size(), 0);
}

@Test(priority = 260)
public void testFindByGroupUuidWihMembers() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
Expand All @@ -108,7 +138,17 @@ public void testFindByGroupUuidWihMembers() {
assertEquals("Wrong members", Collections.singletonList(TEST_USER), getMembers(group));
}

@Test(priority = 14)
@Test(priority = 250)
public void testFindByGroupUuidWihMembersNonExistent() {
results.clear();
AttributeFilter filter = (EqualsFilter) FilterBuilder
.equalTo(AttributeBuilder.build(ATTR_UUID, TEST_UUID_NON_EXISTENT));

grouperConnector.executeQuery(OC_GROUP, filter, handler, getMembersOptions());
assertEquals("Wrong # of groups retrieved", results.size(), 0);
}

@Test(priority = 280)
public void testGetAllGroups() {
results.clear();
grouperConnector.executeQuery(OC_GROUP, null, handler, options);
Expand All @@ -117,7 +157,7 @@ public void testGetAllGroups() {
}
}

@Test(priority = 16)
@Test(priority = 290)
public void testGetAllGroupsWithMembers() {
results.clear();
grouperConnector.executeQuery(OC_GROUP, null, handler, getMembersOptions());
Expand All @@ -126,7 +166,7 @@ public void testGetAllGroupsWithMembers() {
}
}

@Test(priority = 20)
@Test(priority = 900)
public void dispose() {
grouperConnector.dispose();
}
Expand Down

0 comments on commit 3517396

Please sign in to comment.