Browse Source

SEC-2320: AuthenticationPrincipal can be null on invalid type

Previously a ClassCastException was thrown if the type was invalid. Now
a flag exists on AuthenticationPrincipal which indicates if a
ClassCastException should be thrown or not with the default being no error.
Rob Winch 12 years ago
parent
commit
32e9239fd2

+ 8 - 0
web/src/main/java/org/springframework/security/web/bind/annotation/AuthenticationPrincipal.java

@@ -37,4 +37,12 @@ import org.springframework.security.core.Authentication;
 @Documented
 public @interface AuthenticationPrincipal {
 
+    /**
+     * True if a {@link ClassCastException} should be thrown
+     * when the current {@link Authentication#getPrincipal()} is the incorrect
+     * type. Default is false.
+     *
+     * @return
+     */
+    boolean errorOnInvalidType() default false;
 }

+ 47 - 23
web/src/main/java/org/springframework/security/web/bind/support/AuthenticationPrincipalArgumentResolver.java

@@ -44,21 +44,30 @@ import org.springframework.web.method.support.ModelAndViewContainer;
  *     }
  * </pre>
  *
- * <p>Will resolve the CustomUser argument using
+ * <p>
+ * Will resolve the CustomUser argument using
  * {@link Authentication#getPrincipal()} from the {@link SecurityContextHolder}.
  * If the {@link Authentication} or {@link Authentication#getPrincipal()} is
- * null, it will return null. If the types do not match, a
- * {@link ClassCastException} will be thrown.</p>
+ * null, it will return null. If the types do not match, null will be returned
+ * unless {@link AuthenticationPrincipal#errorOnInvalidType()} is true in which
+ * case a {@link ClassCastException} will be thrown.
+ * </p>
+ *
+ * <p>
+ * Alternatively, users can create a custom meta annotation as shown below:
+ * </p>
  *
- * <p>Alternatively, users can create a custom meta annotation as shown below:</p>
  * <pre>
- *   @Target({ ElementType.PARAMETER})
- *   @Retention(RetentionPolicy.RUNTIME)
- *   @AuthenticationPrincipal
- *   public @interface CurrentUser { }
+ * &#064;Target({ ElementType.PARAMETER })
+ * &#064;Retention(RetentionPolicy.RUNTIME)
+ * &#064;AuthenticationPrincipal
+ * public @interface CurrentUser {
+ * }
  * </pre>
  *
- * <p>The custom annotation can then be used instead. For example:</p>
+ * <p>
+ * The custom annotation can then be used instead. For example:
+ * </p>
  *
  * <pre>
  * @Controller
@@ -80,16 +89,7 @@ public final class AuthenticationPrincipalArgumentResolver implements
      * @see org.springframework.web.method.support.HandlerMethodArgumentResolver#supportsParameter(org.springframework.core.MethodParameter)
      */
     public boolean supportsParameter(MethodParameter parameter) {
-        if(parameter.getParameterAnnotation(AuthenticationPrincipal.class) != null) {
-            return true;
-        }
-        Annotation[] annotationsToSearch = parameter.getParameterAnnotations();
-        for(Annotation toSearch : annotationsToSearch) {
-            if(AnnotationUtils.findAnnotation(toSearch.annotationType(), AuthenticationPrincipal.class) != null) {
-                return true;
-            }
-        }
-        return false;
+        return findMethodAnnotation(AuthenticationPrincipal.class, parameter) != null;
     }
 
     /* (non-Javadoc)
@@ -98,17 +98,41 @@ public final class AuthenticationPrincipalArgumentResolver implements
     public Object resolveArgument(MethodParameter parameter,
             ModelAndViewContainer mavContainer, NativeWebRequest webRequest,
             WebDataBinderFactory binderFactory) throws Exception {
-        if(!supportsParameter(parameter)) {
-            return null;
-        }
         Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
         if(authentication == null) {
             return null;
         }
         Object principal = authentication.getPrincipal();
         if(principal != null && !parameter.getParameterType().isAssignableFrom(principal.getClass())) {
-            throw new ClassCastException(principal + " is not assiable to " + parameter.getParameterType());
+            AuthenticationPrincipal authPrincipal = findMethodAnnotation(AuthenticationPrincipal.class, parameter);
+            if(authPrincipal.errorOnInvalidType()) {
+                throw new ClassCastException(principal + " is not assiable to " + parameter.getParameterType());
+            } else {
+                return null;
+            }
         }
         return principal;
     }
+
+    /**
+     * Obtains the specified {@link Annotation} on the specified {@link MethodParameter}.
+     *
+     * @param annotationClass the class of the {@link Annotation} to find on the {@link MethodParameter}
+     * @param parameter the {@link MethodParameter} to search for an {@link Annotation}
+     * @return the {@link Annotation} that was found or null.
+     */
+    private <T extends Annotation> T findMethodAnnotation(Class<T> annotationClass, MethodParameter parameter) {
+        T annotation = parameter.getParameterAnnotation(annotationClass);
+        if(annotation != null) {
+            return annotation;
+        }
+        Annotation[] annotationsToSearch = parameter.getParameterAnnotations();
+        for(Annotation toSearch : annotationsToSearch) {
+            annotation = AnnotationUtils.findAnnotation(toSearch.annotationType(), annotationClass);
+            if(annotation != null) {
+                return annotation;
+            }
+        }
+        return null;
+    }
 }

+ 45 - 8
web/src/test/java/org/springframework/security/web/bind/support/AuthenticationPrincipalArgumentResolverTests.java

@@ -54,6 +54,21 @@ public class AuthenticationPrincipalArgumentResolverTests {
         SecurityContextHolder.clearContext();
     }
 
+    @Test
+    public void supportsParameterNoAnnotation() throws Exception {
+        assertThat(resolver.supportsParameter(showUserNoAnnotation())).isFalse();
+    }
+
+    @Test
+    public void supportsParameterAnnotation() throws Exception {
+        assertThat(resolver.supportsParameter(showUserAnnotationObject())).isTrue();
+    }
+
+    @Test
+    public void supportsParameterCustomAnnotation() throws Exception {
+        assertThat(resolver.supportsParameter(showUserCustomAnnotation())).isTrue();
+    }
+
     @Test
     public void resolveArgumentNullAuthentication() throws Exception {
         assertThat(resolver.resolveArgument(showUserAnnotationString(), null, null, null)).isNull();
@@ -65,12 +80,6 @@ public class AuthenticationPrincipalArgumentResolverTests {
         assertThat(resolver.resolveArgument(showUserAnnotationString(), null, null, null)).isNull();
     }
 
-    @Test
-    public void resolveArgumentNoAnnotation() throws Exception {
-        setAuthenticationPrincipal("john");
-        assertThat(resolver.resolveArgument(showUserNoAnnotation(), null, null, null)).isNull();
-    }
-
     @Test
     public void resolveArgumentString() throws Exception {
         setAuthenticationPrincipal("john");
@@ -101,10 +110,23 @@ public class AuthenticationPrincipalArgumentResolverTests {
         assertThat(resolver.resolveArgument(showUserCustomAnnotation(), null, null, null)).isEqualTo(expectedPrincipal);
     }
 
+    @Test
+    public void resolveArgumentNullOnInvalidType() throws Exception {
+        setAuthenticationPrincipal(new CustomUserPrincipal());
+        assertThat(resolver.resolveArgument(showUserAnnotationString(), null, null, null)).isNull();
+    }
+
+    @Test(expected = ClassCastException.class)
+    public void resolveArgumentErrorOnInvalidType() throws Exception {
+        setAuthenticationPrincipal(new CustomUserPrincipal());
+        resolver.resolveArgument(showUserAnnotationErrorOnInvalidType(), null, null, null);
+    }
+
+
     @Test(expected = ClassCastException.class)
-    public void resolveArgumentClassCastException() throws Exception {
+    public void resolveArgumentCustomserErrorOnInvalidType() throws Exception {
         setAuthenticationPrincipal(new CustomUserPrincipal());
-        resolver.resolveArgument(showUserAnnotationString(), null, null, null);
+        resolver.resolveArgument(showUserAnnotationCurrentUserErrorOnInvalidType(), null, null, null);
     }
 
     @Test
@@ -121,6 +143,14 @@ public class AuthenticationPrincipalArgumentResolverTests {
         return getMethodParameter("showUserAnnotation", String.class);
     }
 
+    private MethodParameter showUserAnnotationErrorOnInvalidType() {
+        return getMethodParameter("showUserAnnotationErrorOnInvalidType", String.class);
+    }
+
+    private MethodParameter showUserAnnotationCurrentUserErrorOnInvalidType() {
+        return getMethodParameter("showUserAnnotationCurrentUserErrorOnInvalidType", String.class);
+    }
+
     private MethodParameter showUserAnnotationUserDetails() {
         return getMethodParameter("showUserAnnotation", UserDetails.class);
     }
@@ -147,9 +177,16 @@ public class AuthenticationPrincipalArgumentResolverTests {
     @AuthenticationPrincipal
     static @interface CurrentUser { }
 
+    @Target({ ElementType.PARAMETER})
+    @Retention(RetentionPolicy.RUNTIME)
+    @AuthenticationPrincipal(errorOnInvalidType = true)
+    static @interface CurrentUserErrorOnInvalidType { }
+
     public static class TestController {
         public void showUserNoAnnotation(String user) {}
         public void showUserAnnotation(@AuthenticationPrincipal String user) {}
+        public void showUserAnnotationErrorOnInvalidType(@AuthenticationPrincipal(errorOnInvalidType=true) String user) {}
+        public void showUserAnnotationCurrentUserErrorOnInvalidType(@CurrentUserErrorOnInvalidType String user) {}
         public void showUserAnnotation(@AuthenticationPrincipal UserDetails user) {}
         public void showUserAnnotation(@AuthenticationPrincipal CustomUserPrincipal user) {}
         public void showUserCustomAnnotation(@CurrentUser CustomUserPrincipal user) {}