getClaimAsBoolean() should not be falsy

Closes gh-10148

Author: qavid

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/ClaimAccessor.java

@@ -91,13 +91,23 @@ default String getClaimAsString(String claim) {
 	}
 
 	/**
-	 * Returns the claim value as a {@code Boolean} or {@code null} if it does not exist.
+	 * Returns the claim value as a {@code Boolean} or {@code null} if the claim does not
+	 * exist.
 	 * @param claim the name of the claim
-	 * @return the claim value or {@code null} if it does not exist
+	 * @return the claim value or {@code null} if the claim does not exist
+	 * @throws IllegalArgumentException if the claim value cannot be converted to a
+	 * {@code Boolean}
+	 * @throws NullPointerException if the claim value is {@code null}
 	 */
 	default Boolean getClaimAsBoolean(String claim) {
-		return !hasClaim(claim) ? null
-				: ClaimConversionService.getSharedInstance().convert(getClaims().get(claim), Boolean.class);
+		if (!hasClaim(claim)) {
+			return null;
+		}
+		Object claimValue = getClaims().get(claim);
+		Boolean convertedValue = ClaimConversionService.getSharedInstance().convert(claimValue, Boolean.class);
+		Assert.notNull(convertedValue,
+				() -> "Unable to convert claim '" + claim + "' of type '" + claimValue.getClass() + "' to Boolean.");
+		return convertedValue;
 	}
 
 	/**

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToBooleanConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -41,7 +41,10 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 		if (source instanceof Boolean) {
 			return source;
 		}
-		return Boolean.valueOf(source.toString());
+		if (source instanceof String) {
+			return Boolean.valueOf((String) source);
+		}
+		return null;
 	}
 
 }

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/ClaimAccessorTests.java

@@ -105,6 +105,33 @@ public void getClaimAsStringWhenValueIsNullThenReturnNull() {
 		assertThat(this.claimAccessor.getClaimAsString(claimName)).isNull();
 	}
 
+	@Test
+	public void getClaimAsBooleanWhenBooleanTypeThenReturnBoolean() {
+		Boolean expectedClaimValue = Boolean.TRUE;
+		String claimName = "boolean";
+		this.claims.put(claimName, expectedClaimValue);
+		assertThat(this.claimAccessor.getClaimAsBoolean(claimName)).isEqualTo(expectedClaimValue);
+	}
+
+	@Test
+	public void getClaimAsBooleanWhenStringTypeThenReturnBoolean() {
+		Boolean expectedClaimValue = Boolean.TRUE;
+		String claimName = "boolean";
+		this.claims.put(claimName, expectedClaimValue.toString());
+		assertThat(this.claimAccessor.getClaimAsBoolean(claimName)).isEqualTo(expectedClaimValue);
+	}
+
+	// gh-10148
+	@Test
+	public void getClaimAsBooleanWhenNonBooleanTypeThenThrowIllegalArgumentException() {
+		String claimName = "boolean";
+		Map<Object, Object> claimValue = new HashMap<>();
+		this.claims.put(claimName, claimValue);
+		assertThatIllegalArgumentException().isThrownBy(() -> this.claimAccessor.getClaimAsBoolean(claimName))
+				.withMessage("Unable to convert claim '" + claimName + "' of type '" + claimValue.getClass()
+						+ "' to Boolean.");
+	}
+
 	@Test
 	public void getClaimAsMapWhenNotExistingThenReturnNull() {
 		assertThat(this.claimAccessor.getClaimAsMap("map")).isNull();

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/OAuth2TokenIntrospectionClaimAccessorTests.java

@@ -28,6 +28,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNullPointerException;
 
 /**
  * Tests for {@link OAuth2TokenIntrospectionClaimAccessor}.
@@ -51,9 +52,9 @@ public void isActiveWhenActiveClaimNotExistingThenReturnFalse() {
 	}
 
 	@Test
-	public void isActiveWhenActiveClaimValueIsNullThenReturnFalse() {
+	public void isActiveWhenActiveClaimValueIsNullThenThrowsNullPointerException() {
 		this.claims.put(OAuth2TokenIntrospectionClaimNames.ACTIVE, null);
-		assertThat(this.claimAccessor.isActive()).isFalse();
+		assertThatNullPointerException().isThrownBy(this.claimAccessor::isActive);
 	}
 
 	@Test