From af8c72f957c653ebf9e1f2f92d78d2f67bea12a5 Mon Sep 17 00:00:00 2001 From: Mushtaq Ahmed Date: Sun, 1 Aug 2021 14:14:40 +0530 Subject: [PATCH] getClaimAsBoolean() should not be falsy Closes gh-10148 Remove comment as per PR review --- .../security/oauth2/core/ClaimAccessor.java | 12 +++++++++--- .../core/converter/ObjectToBooleanConverter.java | 8 ++++++-- .../security/oauth2/core/ClaimAccessorTests.java | 12 +++++++++++- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/ClaimAccessor.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/ClaimAccessor.java index f5a7a4c3e09..07843fa7d4d 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/ClaimAccessor.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/ClaimAccessor.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. @@ -96,8 +96,14 @@ default String getClaimAsString(String claim) { * @return the claim value or {@code null} if it does not exist */ 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.isTrue(convertedValue != null, + () -> "Unable to convert claim '" + claim + "' of type '" + claimValue.getClass() + "' to Boolean."); + return convertedValue; } /** diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToBooleanConverter.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToBooleanConverter.java index 8dd12c1e218..1cd022d7291 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToBooleanConverter.java +++ b/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,11 @@ 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; } } diff --git a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/ClaimAccessorTests.java b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/ClaimAccessorTests.java index f1f50836c26..7fb18c5efb4 100644 --- a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/ClaimAccessorTests.java +++ b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/ClaimAccessorTests.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. @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatObject; /** @@ -135,4 +136,13 @@ public void getClaimWhenValueIsNotConvertedThenThrowClassCastException() { assertThatObject(this.claimAccessor.getClaim(claimName)).isNotInstanceOf(Boolean.class); } + // gh-10148 + @Test + public void getClaimAsBooleanThrowsIllegalArgumentForNonBooleanType() { + String claimName = "boolean"; + this.claims.put(claimName, new HashMap<>()); + + assertThatIllegalArgumentException().isThrownBy(() -> this.claimAccessor.getClaimAsBoolean(claimName)); + } + }