syncope-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ilgro...@apache.org
Subject [2/2] syncope git commit: [SYNCOPE-999] Adjusting Spring Security configuration and REST exception mapping
Date Fri, 20 Jan 2017 17:06:17 GMT
[SYNCOPE-999] Adjusting Spring Security configuration and REST exception mapping


Project: http://git-wip-us.apache.org/repos/asf/syncope/repo
Commit: http://git-wip-us.apache.org/repos/asf/syncope/commit/e2567e45
Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/e2567e45
Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/e2567e45

Branch: refs/heads/master
Commit: e2567e459bc0eeb8065f0f2fcdcbd17ad4dcb0c3
Parents: ef65d83
Author: Francesco Chicchiriccò <ilgrosso@apache.org>
Authored: Fri Jan 20 18:04:51 2017 +0100
Committer: Francesco Chicchiriccò <ilgrosso@apache.org>
Committed: Fri Jan 20 18:06:07 2017 +0100

----------------------------------------------------------------------
 .../client/lib/RestClientExceptionMapper.java   | 34 +++++++++-----------
 .../rest/cxf/RestServiceExceptionMapper.java    | 20 ++++--------
 .../src/main/resources/securityContext.xml      |  6 ++--
 .../syncope/fit/core/AuthenticationITCase.java  |  6 ++--
 .../apache/syncope/fit/core/GroupITCase.java    |  5 ++-
 .../syncope/fit/core/MultitenancyITCase.java    |  6 ++--
 .../apache/syncope/fit/core/UserSelfITCase.java |  7 ++--
 7 files changed, 36 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java
----------------------------------------------------------------------
diff --git a/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java
b/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java
index d1c51a4..2818c7b 100644
--- a/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java
+++ b/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java
@@ -24,9 +24,9 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import javax.ws.rs.BadRequestException;
+import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.core.GenericType;
 import javax.ws.rs.core.Response;
-import javax.ws.rs.ext.ExceptionMapper;
 import javax.ws.rs.ext.Provider;
 import javax.xml.ws.WebServiceException;
 import org.apache.commons.lang3.StringUtils;
@@ -40,43 +40,39 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 @Provider
-public class RestClientExceptionMapper implements ExceptionMapper<Exception>, ResponseExceptionMapper<Exception>
{
+public class RestClientExceptionMapper implements ResponseExceptionMapper<Exception>
{
 
     private static final Logger LOG = LoggerFactory.getLogger(RestClientExceptionMapper.class);
 
     @Override
-    public Response toResponse(final Exception exception) {
-        throw new UnsupportedOperationException(
-                "Call of toResponse() method is not expected in " + RestClientExceptionMapper.class.getSimpleName());
-    }
-
-    @Override
     public Exception fromResponse(final Response response) {
-        final int statusCode = response.getStatus();
-        Exception ex;
+        int statusCode = response.getStatus();
+        String message = response.getHeaderString(RESTHeaders.ERROR_INFO);
 
+        Exception ex;
         SyncopeClientCompositeException scce = checkSyncopeClientCompositeException(response);
         if (scce != null) {
             // 1. Check for client (possibly composite) exception in HTTP header
             ex = scce.getExceptions().size() == 1
                     ? scce.getExceptions().iterator().next()
                     : scce;
-        } else if (statusCode == Response.Status.UNAUTHORIZED.getStatusCode()
-                || statusCode == Response.Status.FORBIDDEN.getStatusCode()) {
-
-            // 2. Map SC_UNAUTHORIZED and SC_FORBIDDEN
-            String message = response.getHeaderString(RESTHeaders.ERROR_INFO);
+        } else if (statusCode == Response.Status.UNAUTHORIZED.getStatusCode()) {
+            // 2. Map SC_UNAUTHORIZED
             ex = new AccessControlException(StringUtils.isBlank(message)
-                    ? "Remote authorization exception"
+                    ? "Remote unauthorized exception"
+                    : message);
+        } else if (statusCode == Response.Status.FORBIDDEN.getStatusCode()) {
+            // 3. Map SC_FORBIDDEN
+            ex = new ForbiddenException(StringUtils.isBlank(message)
+                    ? "Remote forbidden exception"
                     : message);
         } else if (statusCode == Response.Status.BAD_REQUEST.getStatusCode()) {
-            // 3. Map SC_BAD_REQUEST
-            String message = response.getHeaderString(RESTHeaders.ERROR_INFO);
+            // 4. Map SC_BAD_REQUEST
             ex = StringUtils.isBlank(message)
                     ? new BadRequestException()
                     : new BadRequestException(message);
         } else {
-            // 4. All other codes are mapped to runtime exception with HTTP code information
+            // 5. All other codes are mapped to runtime exception with HTTP code information
             ex = new WebServiceException(String.format("Remote exception with status code:
%s",
                     Response.Status.fromStatusCode(statusCode).name()));
         }

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java
----------------------------------------------------------------------
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java
b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java
index 39aad09..1ff733b 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java
@@ -34,7 +34,6 @@ import javax.ws.rs.core.Response.ResponseBuilder;
 import javax.ws.rs.ext.ExceptionMapper;
 import javax.ws.rs.ext.Provider;
 import org.apache.commons.lang3.exception.ExceptionUtils;
-import org.apache.cxf.jaxrs.client.ResponseExceptionMapper;
 import org.apache.cxf.jaxrs.utils.JAXRSUtils;
 import org.apache.cxf.jaxrs.validation.ValidationExceptionMapper;
 import org.apache.syncope.common.lib.SyncopeClientCompositeException;
@@ -67,7 +66,7 @@ import org.springframework.transaction.TransactionSystemException;
 @Configuration
 @PropertySource("classpath:errorMessages.properties")
 @Provider
-public class RestServiceExceptionMapper implements ExceptionMapper<Exception>, ResponseExceptionMapper<Exception>
{
+public class RestServiceExceptionMapper implements ExceptionMapper<Exception> {
 
     private static final Logger LOG = LoggerFactory.getLogger(RestServiceExceptionMapper.class);
 
@@ -92,15 +91,14 @@ public class RestServiceExceptionMapper implements ExceptionMapper<Exception>,
R
 
         ResponseBuilder builder;
 
-        if (ex instanceof SyncopeClientException) {
+        if (ex instanceof AccessDeniedException) {
+            // leaves the default exception processing to Spring Security
+            builder = null;
+        } else if (ex instanceof SyncopeClientException) {
             SyncopeClientException sce = (SyncopeClientException) ex;
             builder = sce.isComposite()
                     ? getSyncopeClientCompositeExceptionResponse(sce.asComposite())
                     : getSyncopeClientExceptionResponse(sce);
-        } else if (ex instanceof AccessDeniedException) {
-            builder = Response.status(Response.Status.UNAUTHORIZED).
-                    header(RESTHeaders.ERROR_CODE, Response.Status.UNAUTHORIZED.getReasonPhrase()).
-                    header(RESTHeaders.ERROR_INFO, ex.getMessage());
         } else if (ex instanceof DelegatedAdministrationException) {
             builder = builder(ClientExceptionType.DelegatedAdministration, ExceptionUtils.getRootCauseMessage(ex));
         } else if (ex instanceof EntityExistsException || ex instanceof DuplicateException
@@ -148,13 +146,7 @@ public class RestServiceExceptionMapper implements ExceptionMapper<Exception>,
R
             }
         }
 
-        return builder.build();
-    }
-
-    @Override
-    public Exception fromResponse(final Response response) {
-        throw new UnsupportedOperationException(
-                "Call of fromResponse() method is not expected in RestServiceExceptionMapper");
+        return builder == null ? null : builder.build();
     }
 
     private ResponseBuilder getSyncopeClientExceptionResponse(final SyncopeClientException
ex) {

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/core/spring/src/main/resources/securityContext.xml
----------------------------------------------------------------------
diff --git a/core/spring/src/main/resources/securityContext.xml b/core/spring/src/main/resources/securityContext.xml
index 6d5f4b1..a038e8f 100644
--- a/core/spring/src/main/resources/securityContext.xml
+++ b/core/spring/src/main/resources/securityContext.xml
@@ -59,7 +59,7 @@ under the License.
         class="org.apache.syncope.core.spring.security.SyncopeAuthenticationEntryPoint">
     <property name="realmName" value="Apache Syncope authentication"/>
   </bean>
-
+  
   <bean id="syncopeAccessDeniedHandler" class="org.apache.syncope.core.spring.security.SyncopeAccessDeniedHandler"/>
   
   <bean id="firewall" class="org.springframework.security.web.firewall.DefaultHttpFirewall">
@@ -68,10 +68,10 @@ under the License.
   <security:http-firewall ref="firewall"/>
 
   <security:http security-context-repository-ref="securityContextRepository"
+                 entry-point-ref="syncopeAuthenticationEntryPoint"
                  use-expressions="false" disable-url-rewriting="false">
 
-    <security:http-basic entry-point-ref="syncopeAuthenticationEntryPoint"
-                         authentication-details-source-ref="syncopeAuthenticationDetailsSource"/>
+    <security:http-basic authentication-details-source-ref="syncopeAuthenticationDetailsSource"/>
     <security:anonymous username="${anonymousUser}"/>
     <security:intercept-url pattern="/**"/>
     

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/fit/core-reference/src/test/java/org/apache/syncope/fit/core/AuthenticationITCase.java
----------------------------------------------------------------------
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/AuthenticationITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/AuthenticationITCase.java
index af6914e..05a48ba 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/AuthenticationITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/AuthenticationITCase.java
@@ -31,6 +31,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import javax.sql.DataSource;
+import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.core.GenericType;
 import javax.ws.rs.core.Response;
 import org.apache.commons.collections4.CollectionUtils;
@@ -166,9 +167,8 @@ public class AuthenticationITCase extends AbstractITCase {
         // 5. update the schema create above (as user) - failure
         try {
             schemaService2.update(SchemaType.PLAIN, schemaTO);
-            fail("Schemaupdate as user should not work");
-        } catch (AccessControlException e) {
-            // CXF Service will throw this exception
+            fail("Schema update as user should not work");
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
----------------------------------------------------------------------
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
index 9c86a8e..0e94b58 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
@@ -35,6 +35,7 @@ import javax.naming.NamingException;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
+import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.core.GenericType;
 import javax.ws.rs.core.Response;
 import org.apache.commons.collections4.IterableUtils;
@@ -297,9 +298,7 @@ public class GroupITCase extends AbstractITCase {
         try {
             groupService2.update(groupPatch);
             fail();
-        } catch (SyncopeClientException e) {
-            assertEquals(Response.Status.UNAUTHORIZED, e.getType().getResponseStatus());
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MultitenancyITCase.java
----------------------------------------------------------------------
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MultitenancyITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MultitenancyITCase.java
index ae97d2b..dcacbeb 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MultitenancyITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MultitenancyITCase.java
@@ -23,9 +23,9 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
-import java.security.AccessControlException;
 import java.util.List;
 import java.util.Locale;
+import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.core.GenericType;
 import javax.ws.rs.core.Response;
 import org.apache.commons.lang3.StringUtils;
@@ -84,14 +84,14 @@ public class MultitenancyITCase extends AbstractITCase {
         try {
             adminClient.getService(DomainService.class).read("Two");
             fail();
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 
         try {
             adminClient.getService(LoggerService.class).list(LoggerType.LOG);
             fail();
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 

http://git-wip-us.apache.org/repos/asf/syncope/blob/e2567e45/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserSelfITCase.java
----------------------------------------------------------------------
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserSelfITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserSelfITCase.java
index 15a4bbe..b0225d8 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserSelfITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/UserSelfITCase.java
@@ -32,6 +32,7 @@ import java.security.AccessControlException;
 import java.util.Map;
 import java.util.Set;
 import javax.sql.DataSource;
+import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.core.GenericType;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
@@ -83,7 +84,7 @@ public class UserSelfITCase extends AbstractITCase {
         try {
             userSelfService.create(UserITCase.getUniqueSampleTO("anonymous@syncope.apache.org"),
true);
             fail();
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 
@@ -144,7 +145,7 @@ public class UserSelfITCase extends AbstractITCase {
         try {
             userService2.read("1417acbe-cbf6-4277-9372-e75e04f97000");
             fail();
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
         }
 
@@ -378,7 +379,7 @@ public class UserSelfITCase extends AbstractITCase {
         try {
             vivaldiClient.getService(ResourceService.class).list();
             fail();
-        } catch (AccessControlException e) {
+        } catch (ForbiddenException e) {
             assertNotNull(e);
             assertEquals("Please change your password first", e.getMessage());
         }


Mime
View raw message