From 33ebd5405a79d72f8b2b60c4b3c5fd88dcd5ef7f Mon Sep 17 00:00:00 2001 From: sheheryarumair Date: Wed, 13 Mar 2024 19:32:07 +0500 Subject: [PATCH 1/2] Removed dataSource null validation Fixed data source validation --- .../JdbcUserServiceBeanDefinitionParser.java | 2 +- ...cUserServiceBeanDefinitionParserTests.java | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java index a0d7de83f6..5ed12e79c2 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java @@ -42,7 +42,7 @@ public class JdbcUserServiceBeanDefinitionParser extends AbstractUserDetailsServ @Override protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) { String dataSource = element.getAttribute(ATT_DATA_SOURCE); - if (dataSource != null) { + if (StringUtils.hasText(dataSource)) { builder.addPropertyReference("dataSource", dataSource); } else { diff --git a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java index 41395cd8b4..3b8add34c6 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java @@ -16,10 +16,12 @@ package org.springframework.security.config.authentication; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; +import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.CachingUserDetailsService; import org.springframework.security.authentication.ProviderManager; @@ -160,6 +162,44 @@ public class JdbcUserServiceBeanDefinitionParserTests { assertThat(AuthorityUtils.authorityListToSet(rod.getAuthorities())).contains("PREFIX_ROLE_SUPERVISOR"); } + @Test + public void testEmptyDataSourceRef() { + // @formatter:off + String xml = "" + + " " + + " " + + " " + + ""; + // @formatter:on + + try { + setContext(xml); + Assertions.fail("Expected exception due to empty data-source-ref"); + } + catch (BeanDefinitionStoreException ex) { + assertThat(ex.getMessage()).contains("data-source-ref is required"); + } + } + + @Test + public void testMissingDataSourceRef() { + // @formatter:off + String xml = "" + + " " + + " " + + " " + + ""; + // @formatter:on + + try { + setContext(xml); + Assertions.fail("Expected exception due to missing data-source-ref"); + } + catch (BeanDefinitionStoreException ex) { + assertThat(ex.getMessage()).contains("XML document from").contains("is invalid"); + } + } + private void setContext(String context) { this.appContext = new InMemoryXmlApplicationContext(context); } From 39dbd24dcbd5ce7ab21b5801ee6c4cc165e2b6cb Mon Sep 17 00:00:00 2001 From: Steve Riesenberg <5248162+sjohnr@users.noreply.github.com> Date: Thu, 4 Apr 2024 13:55:08 -0500 Subject: [PATCH 2/2] Polish gh-14742 --- .../JdbcUserServiceBeanDefinitionParser.java | 2 +- ...cUserServiceBeanDefinitionParserTests.java | 34 ++++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java index 5ed12e79c2..9cbe4f93cd 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2024 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. diff --git a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java index 3b8add34c6..7a07eeb69d 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -16,12 +16,13 @@ package org.springframework.security.config.authentication; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; +import org.xml.sax.SAXParseException; -import org.springframework.beans.factory.BeanDefinitionStoreException; +import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; +import org.springframework.beans.factory.xml.XmlBeanDefinitionStoreException; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.CachingUserDetailsService; import org.springframework.security.authentication.ProviderManager; @@ -35,6 +36,7 @@ import org.springframework.security.provisioning.JdbcUserDetailsManager; import org.springframework.security.util.FieldUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.Mockito.mock; /** @@ -170,15 +172,11 @@ public class JdbcUserServiceBeanDefinitionParserTests { + " " + " " + ""; + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> setContext(xml)) + .withFailMessage("Expected exception due to empty data-source-ref") + .withMessageContaining("data-source-ref is required for jdbc-user-service"); // @formatter:on - - try { - setContext(xml); - Assertions.fail("Expected exception due to empty data-source-ref"); - } - catch (BeanDefinitionStoreException ex) { - assertThat(ex.getMessage()).contains("data-source-ref is required"); - } } @Test @@ -189,15 +187,13 @@ public class JdbcUserServiceBeanDefinitionParserTests { + " " + " " + ""; + assertThatExceptionOfType(XmlBeanDefinitionStoreException.class) + .isThrownBy(() -> setContext(xml)) + .withFailMessage("Expected exception due to missing data-source-ref") + .havingRootCause() + .isInstanceOf(SAXParseException.class) + .withMessageContaining("Attribute 'data-source-ref' must appear on element 'jdbc-user-service'"); // @formatter:on - - try { - setContext(xml); - Assertions.fail("Expected exception due to missing data-source-ref"); - } - catch (BeanDefinitionStoreException ex) { - assertThat(ex.getMessage()).contains("XML document from").contains("is invalid"); - } } private void setContext(String context) {