From 3517396251505800a390274381f15e2a9ad276c0 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Tue, 30 Mar 2021 16:32:47 +0200 Subject: [PATCH] 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. --- pom.xml | 2 +- .../grouper/rest/GroupProcessor.java | 22 +++- .../connector/grouper/rest/Processor.java | 121 +++++++++++++----- .../connector/grouper/test/AbstractTest.java | 4 +- .../connector/grouper/test/GroupTest.java | 60 +++++++-- 5 files changed, 164 insertions(+), 45 deletions(-) diff --git a/pom.xml b/pom.xml index d35b7e3..06e38be 100644 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ connector-grouper-rest - 0.6 + 0.7 jar Grouper REST Connector diff --git a/src/main/java/com/evolveum/polygon/connector/grouper/rest/GroupProcessor.java b/src/main/java/com/evolveum/polygon/connector/grouper/rest/GroupProcessor.java index a33d211..cbd144a 100644 --- a/src/main/java/com/evolveum/polygon/connector/grouper/rest/GroupProcessor.java +++ b/src/main/java/com/evolveum/polygon/connector/grouper/rest/GroupProcessor.java @@ -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) { @@ -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); @@ -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) { diff --git a/src/main/java/com/evolveum/polygon/connector/grouper/rest/Processor.java b/src/main/java/com/evolveum/polygon/connector/grouper/rest/Processor.java index dd965e1..7025c29 100644 --- a/src/main/java/com/evolveum/polygon/connector/grouper/rest/Processor.java +++ b/src/main/java/com/evolveum/polygon/connector/grouper/rest/Processor.java @@ -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"; @@ -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); @@ -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) { @@ -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; } } @@ -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; + } + } } diff --git a/src/test/java/com/evolveum/polygon/connector/grouper/test/AbstractTest.java b/src/test/java/com/evolveum/polygon/connector/grouper/test/AbstractTest.java index 191def3..f69c9e0 100644 --- a/src/test/java/com/evolveum/polygon/connector/grouper/test/AbstractTest.java +++ b/src/test/java/com/evolveum/polygon/connector/grouper/test/AbstractTest.java @@ -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"; diff --git a/src/test/java/com/evolveum/polygon/connector/grouper/test/GroupTest.java b/src/test/java/com/evolveum/polygon/connector/grouper/test/GroupTest.java index 4972329..e6ac163 100644 --- a/src/test/java/com/evolveum/polygon/connector/grouper/test/GroupTest.java +++ b/src/test/java/com/evolveum/polygon/connector/grouper/test/GroupTest.java @@ -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 @@ -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 @@ -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 @@ -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 @@ -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); @@ -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()); @@ -126,7 +166,7 @@ public void testGetAllGroupsWithMembers() { } } - @Test(priority = 20) + @Test(priority = 900) public void dispose() { grouperConnector.dispose(); }