Better error messages for realm exceptions

adrienlauer · adrienlauer · commit f42466858b5d · 2020-04-09T10:09:11.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * [chg] Moved `web.server.sessions` configuration options to `web.sessions`. 
 * [chg] Default session timeout with embedded servers is now defined by the `web.server.` configuration options to `web.sessions`. 
 * [chg] Renamed `web.staticResources` configuration options to `web.static`.
+* [chg] Better error messages for security realm exceptions.
 * [fix] Ensure that JVM-wide base configuration is refreshed between tests.
 * [fix] With Jersey 2, allow JAX-RS components to be instantiated without Guice as a fallback.
 * [fix] Default session timeout for Undertow was incorrect. It is now 20 minutes.
diff --git a/security/core/src/main/java/org/seedstack/seed/security/internal/SecurityConfigurer.java b/security/core/src/main/java/org/seedstack/seed/security/internal/SecurityConfigurer.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.security.internal;
 
 import java.util.ArrayList;
@@ -22,8 +23,11 @@
 import org.seedstack.seed.security.internal.authorization.EmptyRolePermissionResolver;
 import org.seedstack.seed.security.internal.authorization.SameRoleMapping;
 import org.seedstack.seed.security.internal.realms.ConfigurationRealm;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 class SecurityConfigurer {
+    private static final Logger LOGGER = LoggerFactory.getLogger(SecurityConfigurer.class);
     private static final Class<? extends Realm> DEFAULT_REALM = ConfigurationRealm.class;
     private static final Class<? extends RoleMapping> DEFAULT_ROLE_MAPPING = SameRoleMapping.class;
     private static final Class<? extends RoleMapping> CONFIGURATION_ROLE_MAPPING = ConfigurationRoleMapping.class;
@@ -69,6 +73,7 @@ private void buildRealms() {
             confRealm.setRolePermissionResolverClass(findRolePermissionResolver(null, confRealm));
             confRealm.setRoleMappingClass(findRoleMapping(null, confRealm));
             configurationRealms.add(confRealm);
+            LOGGER.info("No security realm configured, using ConfigurationRealm by default");
         } else {
             for (SecurityConfig.RealmConfig realmConfig : securityConfig.getRealms()) {
                 Class<? extends Realm> realmClass = (Class<? extends Realm>) findClass(realmConfig.getName(),
@@ -81,6 +86,7 @@ private void buildRealms() {
                 confRealm.setRoleMappingClass(findRoleMapping(realmConfig, confRealm));
                 configurationRealms.add(confRealm);
             }
+            LOGGER.info("Configured security realms: {}", configurationRealms);
         }
     }
 
diff --git a/security/core/src/main/java/org/seedstack/seed/security/internal/SecurityInternalModule.java b/security/core/src/main/java/org/seedstack/seed/security/internal/SecurityInternalModule.java
@@ -52,7 +52,7 @@ public void configure() {
     private void bindPrincipalCustomizers() {
         Multibinder<PrincipalCustomizer> principalCustomizers = Multibinder.newSetBinder(binder(),
                 PrincipalCustomizer.class);
-        for (Class<? extends PrincipalCustomizer> customizerClass : securityConfigurer.getPrincipalCustomizers()) {
+        for (Class<? extends PrincipalCustomizer<?>> customizerClass : securityConfigurer.getPrincipalCustomizers()) {
             principalCustomizers.addBinding().to(customizerClass);
         }
         expose(new PrincipalCustomizersTypeLiteral());
diff --git a/security/core/src/main/java/org/seedstack/seed/security/internal/ShiroRealmAdapter.java b/security/core/src/main/java/org/seedstack/seed/security/internal/ShiroRealmAdapter.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.security.internal;
 
 import java.util.ArrayList;
@@ -113,7 +114,7 @@ public boolean supports(AuthenticationToken token) {
     protected Object getAuthenticationCacheKey(AuthenticationToken token) {
         Object authenticationCacheKey = super.getAuthenticationCacheKey(token);
         if (authenticationCacheKey instanceof PrincipalProvider) {
-            return ((PrincipalProvider) authenticationCacheKey).get();
+            return ((PrincipalProvider<?>) authenticationCacheKey).get();
         } else {
             return authenticationCacheKey;
         }
@@ -123,7 +124,7 @@ protected Object getAuthenticationCacheKey(AuthenticationToken token) {
     protected Object getAuthenticationCacheKey(PrincipalCollection principals) {
         Object authenticationCacheKey = super.getAuthenticationCacheKey(principals);
         if (authenticationCacheKey instanceof PrincipalProvider) {
-            return ((PrincipalProvider) authenticationCacheKey).get();
+            return ((PrincipalProvider<?>) authenticationCacheKey).get();
         } else {
             return authenticationCacheKey;
         }
@@ -133,7 +134,7 @@ protected Object getAuthenticationCacheKey(PrincipalCollection principals) {
     protected Object getAuthorizationCacheKey(PrincipalCollection principals) {
         Object primaryPrincipal = principals.getPrimaryPrincipal();
         if (primaryPrincipal instanceof PrincipalProvider) {
-            return ((PrincipalProvider) primaryPrincipal).get();
+            return ((PrincipalProvider<?>) primaryPrincipal).get();
         } else {
             return primaryPrincipal;
         }
@@ -157,6 +158,11 @@ void setRealm(Realm realm) {
         this.realm = realm;
     }
 
+    @Override
+    public String toString() {
+        return realm.name();
+    }
+
     private org.seedstack.seed.security.AuthenticationToken convertToken(AuthenticationToken token) {
         if (token instanceof org.seedstack.seed.security.AuthenticationToken) {
             return (org.seedstack.seed.security.AuthenticationToken) token;
diff --git a/security/core/src/main/java/org/seedstack/seed/security/internal/realms/AuthenticationTokenWrapper.java b/security/core/src/main/java/org/seedstack/seed/security/internal/realms/AuthenticationTokenWrapper.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.security.internal.realms;
 
 import org.apache.shiro.authc.AuthenticationToken;
@@ -35,6 +36,11 @@ public Object getCredentials() {
         return seedToken.getCredentials();
     }
 
+    @Override
+    public String toString() {
+        return seedToken.name();
+    }
+
     /**
      * Gives the seed authentication token.
      *
diff --git a/security/specs/src/main/java/org/seedstack/seed/security/AuthenticationToken.java b/security/specs/src/main/java/org/seedstack/seed/security/AuthenticationToken.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.security;
 
 import java.io.Serializable;
@@ -26,6 +27,12 @@
  * sufficient for your needs.
  */
 public interface AuthenticationToken extends Serializable {
+    /**
+     * @return the name of the token type
+     */
+    default String name() {
+        return getClass().getSimpleName();
+    }
 
     /**
      * Returns the account identity submitted during the authentication process.
diff --git a/security/specs/src/main/java/org/seedstack/seed/security/Realm.java b/security/specs/src/main/java/org/seedstack/seed/security/Realm.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.security;
 
 import java.util.Collection;
@@ -15,6 +16,12 @@
  * A realm is used to authenticate and retrieve authorization for a user.
  */
 public interface Realm {
+    /**
+     * @return the name of the realm
+     */
+    default String name() {
+        return getClass().getSimpleName();
+    }
 
     /**
      * Get the roles
diff --git a/web/security/src/main/java/org/seedstack/seed/web/security/internal/AntiXsrfFilter.java b/web/security/src/main/java/org/seedstack/seed/web/security/internal/AntiXsrfFilter.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.web.security.internal;
 
 import java.security.SecureRandom;
@@ -18,8 +19,11 @@
 import org.apache.shiro.web.filter.PathMatchingFilter;
 import org.seedstack.seed.Configuration;
 import org.seedstack.seed.web.security.WebSecurityConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class AntiXsrfFilter extends PathMatchingFilter {
+    private static final Logger LOGGER = LoggerFactory.getLogger(AntiXsrfFilter.class);
     private final static char[] CHARSET = new char[]{'a', 'b', 'c', 'd', 'e',
             'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r',
             's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4',
@@ -31,7 +35,7 @@ public class AntiXsrfFilter extends PathMatchingFilter {
 
     @Override
     protected boolean onPreHandle(ServletRequest request, ServletResponse response,
-            Object mappedValue) throws Exception {
+            Object mappedValue) {
         HttpServletRequest httpServletRequest = (HttpServletRequest) request;
         HttpServletResponse httpServletResponse = (HttpServletResponse) response;
         final HttpSession session = httpServletRequest.getSession(false);
@@ -56,8 +60,11 @@ protected boolean onPreHandle(ServletRequest request, ServletResponse response,
 
                     // If no cookie is available, send an error
                     if (cookieToken == null) {
-                        ((HttpServletResponse) response).sendError(HttpServletResponse.SC_FORBIDDEN,
-                                "Missing CSRF protection token cookie");
+                        WebSecurityPlugin.sendErrorToClient((HttpServletResponse) response,
+                                LOGGER,
+                                HttpServletResponse.SC_FORBIDDEN,
+                                "Missing CSRF protection token cookie",
+                                null);
                         return false;
                     }
 
@@ -71,15 +78,21 @@ protected boolean onPreHandle(ServletRequest request, ServletResponse response,
 
                     // If no request token available, send an error
                     if (requestToken == null) {
-                        ((HttpServletResponse) response).sendError(HttpServletResponse.SC_FORBIDDEN,
-                                "Missing CSRF protection token in the request headers");
+                        WebSecurityPlugin.sendErrorToClient((HttpServletResponse) response,
+                                LOGGER,
+                                HttpServletResponse.SC_FORBIDDEN,
+                                "Missing CSRF protection token in the request headers",
+                                null);
                         return false;
                     }
 
                     // If tokens don't match, send an error
                     if (!cookieToken.equals(requestToken)) {
-                        ((HttpServletResponse) response).sendError(HttpServletResponse.SC_FORBIDDEN,
-                                "Request token does not match session token");
+                        WebSecurityPlugin.sendErrorToClient((HttpServletResponse) response,
+                                LOGGER,
+                                HttpServletResponse.SC_FORBIDDEN,
+                                "Request token does not match session token",
+                                null);
                         return false;
                     }
 
diff --git a/web/security/src/main/java/org/seedstack/seed/web/security/internal/WebSecurityPlugin.java b/web/security/src/main/java/org/seedstack/seed/web/security/internal/WebSecurityPlugin.java
@@ -5,6 +5,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.web.security.internal;
 
 import com.google.common.collect.Lists;
@@ -13,11 +14,14 @@
 import io.nuun.kernel.api.plugin.PluginException;
 import io.nuun.kernel.api.plugin.context.InitContext;
 import io.nuun.kernel.api.plugin.request.ClasspathScanRequest;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.UUID;
 import javax.servlet.Filter;
 import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletResponse;
 import org.apache.shiro.guice.web.GuiceShiroFilter;
 import org.seedstack.seed.core.SeedRuntime;
 import org.seedstack.seed.core.internal.AbstractSeedPlugin;
@@ -31,6 +35,7 @@
 import org.seedstack.seed.web.spi.SeedFilterPriority;
 import org.seedstack.seed.web.spi.ServletDefinition;
 import org.seedstack.seed.web.spi.WebProvider;
+import org.slf4j.Logger;
 
 /**
  * This plugins adds web security.
@@ -116,4 +121,25 @@ public List<FilterDefinition> filters() {
     public List<ListenerDefinition> listeners() {
         return null;
     }
-}
+
+    static void sendErrorToClient(HttpServletResponse resp, Logger logger, int code, String message, Exception e) {
+        String uuid = UUID.randomUUID().toString();
+        try {
+            resp.sendError(code, message + " [" + uuid + "]");
+            if (e != null) {
+                logger.warn("Sent {} HTTP error to client [{}]: {}. Server-side exception message: {}",
+                        code,
+                        uuid,
+                        message,
+                        e.getMessage());
+            } else {
+                logger.warn("Sent {} HTTP error to client [{}]: {}. No server-side exception occurred.",
+                        code,
+                        uuid,
+                        message);
+            }
+        } catch (IOException e1) {
+            logger.warn("Unable to send {} error to client [{}]: {}", code, uuid, message, e1);
+        }
+    }
+}
diff --git a/web/security/src/main/java/org/seedstack/seed/web/security/internal/X509CertificateFilter.java b/web/security/src/main/java/org/seedstack/seed/web/security/internal/X509CertificateFilter.java
@@ -5,9 +5,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
+
 package org.seedstack.seed.web.security.internal;
 
-import java.io.IOException;
 import java.security.cert.X509Certificate;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -18,11 +18,14 @@
 import org.apache.shiro.web.filter.authc.AuthenticatingFilter;
 import org.seedstack.seed.security.X509CertificateToken;
 import org.seedstack.seed.security.internal.realms.AuthenticationTokenWrapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A security filter that extracts the certificate from the request for later use
  */
 public class X509CertificateFilter extends AuthenticatingFilter {
+    private static final Logger LOGGER = LoggerFactory.getLogger(X509CertificateFilter.class);
     private static final String OPTIONAL = "optional";
     private boolean optional;
 
@@ -45,14 +48,14 @@ protected boolean onLoginFailure(AuthenticationToken token, AuthenticationExcept
             ServletResponse response) {
         if (optional) {
             return true;
+        } else {
+            WebSecurityPlugin.sendErrorToClient((HttpServletResponse) response,
+                    LOGGER,
+                    HttpServletResponse.SC_UNAUTHORIZED,
+                    "A valid certificate is required to gain access",
+                    e);
+            return false;
         }
-        try {
-            ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED,
-                    "A valid certificate is required to gain access");
-        } catch (IOException e1) {
-            throw new IllegalStateException(e1);
-        }
-        return false;
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ public void configure() {`
`52`	`52`	`private void bindPrincipalCustomizers() {`
`53`	`53`	`Multibinder<PrincipalCustomizer> principalCustomizers = Multibinder.newSetBinder(binder(),`
`54`	`54`	`PrincipalCustomizer.class);`
`55`		`- for (Class<? extends PrincipalCustomizer> customizerClass : securityConfigurer.getPrincipalCustomizers()) {`
	`55`	`+ for (Class<? extends PrincipalCustomizer<?>> customizerClass : securityConfigurer.getPrincipalCustomizers()) {`
`56`	`56`	`principalCustomizers.addBinding().to(customizerClass);`
`57`	`57`	`}`
`58`	`58`	`expose(new PrincipalCustomizersTypeLiteral());`