From 753a927d10bd760653b617ed356a6bba352add4e Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Sat, 25 Apr 2020 10:13:01 +0300 Subject: [PATCH 01/13] Added file Test that I understand is needed - this is is for understanding of the task --- .../ldap/SpringLdapFalseAuthTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java new file mode 100644 index 00000000000..edd3b62d989 --- /dev/null +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java @@ -0,0 +1,37 @@ +package org.springframework.security.ldap; + +import com.unboundid.ldap.listener.InMemoryDirectoryServer; +import com.unboundid.ldap.sdk.LDAPException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.ldap.server.ApacheDSContainer; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +@RunWith(SpringRunner.class) +@ContextConfiguration(classes = ApacheDsContainerConfig.class) +public class SpringLdapFalseAuthTest { + + @Test + public void testApacheDSContainerInvalidLdifFile(){ + + //TODO - get exception here in case of invalid root base + new ApacheDSContainer("dc=springframework,dc=org", + "classpath:missing-file.ldif"); + } + + @Test(expected = LDAPException.class) + public void testInMemoryDirectoryServerInvalidLdifFile(){ + //TODO - get exception here in case of invalid root base + try { + new InMemoryDirectoryServer("dc=springframework,dc=org", + "classpath:missing-file.ldif"); + } catch (LDAPException e) { + + } + } + + + +} From 4a6b4970fdaced1e9c6ce974f6b16db92422f008 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Sat, 2 May 2020 17:51:25 +0300 Subject: [PATCH 02/13] Following inputs I have done the followings: 1. move the tests into UnboundIdContainerTests 2. Starting phase one of the commit --- .../ldap/SpringLdapFalseAuthTest.java | 13 ++------- .../ldap/server/UnboundIdContainerTests.java | 14 ++++++++++ .../ldap/server/UnboundIdContainer.java | 27 ++++++++++++++++++- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java index edd3b62d989..f3d76b9ebd6 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java @@ -4,8 +4,6 @@ import com.unboundid.ldap.sdk.LDAPException; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.security.ldap.server.ApacheDSContainer; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; @@ -13,22 +11,15 @@ @ContextConfiguration(classes = ApacheDsContainerConfig.class) public class SpringLdapFalseAuthTest { - @Test - public void testApacheDSContainerInvalidLdifFile(){ - //TODO - get exception here in case of invalid root base - new ApacheDSContainer("dc=springframework,dc=org", - "classpath:missing-file.ldif"); - } - - @Test(expected = LDAPException.class) + @Test(expected = IllegalArgumentException.class) public void testInMemoryDirectoryServerInvalidLdifFile(){ //TODO - get exception here in case of invalid root base try { new InMemoryDirectoryServer("dc=springframework,dc=org", "classpath:missing-file.ldif"); } catch (LDAPException e) { - + throw new IllegalArgumentException(e); } } diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index 5b1a7bc59db..3c79783d4fa 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -47,6 +47,7 @@ public void startLdapServer() throws Exception { } } + @Test public void afterPropertiesSetWhenPortIsZeroThenRandomPortIsSelected() throws Exception { UnboundIdContainer server = new UnboundIdContainer("dc=springframework,dc=org", null); @@ -77,4 +78,17 @@ private List getDefaultPorts(int count) throws IOException { } } + @Test + public void testInvalidLdapFile(){ + UnboundIdContainer server = new UnboundIdContainer("dc=springframework,dc=org", "classpath:missing-file.ldif"); + server.setPort(0); + try { + server.start(); + + }catch (I){} finally { + server.destroy(); + } + } + + } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index aaa54ee4136..448143bded2 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -15,6 +15,7 @@ */ package org.springframework.security.ldap.server; +import java.io.File; import java.io.InputStream; import com.unboundid.ldap.listener.InMemoryDirectoryServer; @@ -25,6 +26,7 @@ import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; +import org.apache.commons.io.FileUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -54,9 +56,31 @@ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lif public UnboundIdContainer(String defaultPartitionSuffix, String ldif) { this.defaultPartitionSuffix = defaultPartitionSuffix; + setLdif(ldif); + } + + public String getLdif() { + return ldif; + } + + public void setLdif(String ldif) { + if(!StringUtils.hasText(ldif)){ + throw new IllegalArgumentException("ldif file value is missing"); + } + checkFilePath(ldif); this.ldif = ldif; } + private void checkFilePath(String ldif) { + File ldifFile = new File(ldif); + if( ldifFile.exists()){ + throw new IllegalArgumentException("the requested file not found within given path"); + } + if(ldifFile.isFile()){ + throw new IllegalArgumentException("the given path is not a file"); + } + } + public int getPort() { return this.port; } @@ -113,9 +137,10 @@ public void start() { } private void importLdif(InMemoryDirectoryServer directoryServer) { + //TODO - would like to remove this check I think set and get are the best place to handle these if (StringUtils.hasText(this.ldif)) { try { - Resource[] resources = this.context.getResources(this.ldif); + Resource[] resources = this.context.getResources(getLdif()); if (resources.length > 0 && resources[0].exists()) { try (InputStream inputStream = resources[0].getInputStream()) { directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); From ab67520e204144a6601f0936193a02c62c867e27 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Mon, 4 May 2020 12:44:14 +0300 Subject: [PATCH 03/13] adding formating --- .../ldap/SpringLdapFalseAuthTest.java | 28 +++++++++++---- .../ldap/server/UnboundIdContainerTests.java | 36 +++++++++++++++---- .../ldap/server/UnboundIdContainer.java | 10 +++--- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java index f3d76b9ebd6..49584016d2b 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java @@ -1,19 +1,34 @@ +/* + * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.springframework.security.ldap; import com.unboundid.ldap.listener.InMemoryDirectoryServer; import com.unboundid.ldap.sdk.LDAPException; import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringRunner; -@RunWith(SpringRunner.class) -@ContextConfiguration(classes = ApacheDsContainerConfig.class) + +/** + * @Auther - dratler + */ public class SpringLdapFalseAuthTest { @Test(expected = IllegalArgumentException.class) - public void testInMemoryDirectoryServerInvalidLdifFile(){ + public void testInMemoryDirectoryServerInvalidLdifFile() { //TODO - get exception here in case of invalid root base try { new InMemoryDirectoryServer("dc=springframework,dc=org", @@ -24,5 +39,4 @@ public void testInMemoryDirectoryServerInvalidLdifFile(){ } - } diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index 3c79783d4fa..fddf6006d61 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -31,10 +31,13 @@ */ public class UnboundIdContainerTests { + private final String validLdifClassPath = "classpath:test-server.ldif"; + private final String validRootDn = "dc=springframework,dc=org"; + @Test public void startLdapServer() throws Exception { - UnboundIdContainer server = new UnboundIdContainer("dc=springframework,dc=org", - "classpath:test-server.ldif"); + UnboundIdContainer server = new UnboundIdContainer(validLdifClassPath, + validRootDn); server.setApplicationContext(new GenericApplicationContext()); List ports = getDefaultPorts(1); server.setPort(ports.get(0)); @@ -50,7 +53,7 @@ public void startLdapServer() throws Exception { @Test public void afterPropertiesSetWhenPortIsZeroThenRandomPortIsSelected() throws Exception { - UnboundIdContainer server = new UnboundIdContainer("dc=springframework,dc=org", null); + UnboundIdContainer server = new UnboundIdContainer(validLdifClassPath, validRootDn); server.setPort(0); try { @@ -78,17 +81,38 @@ private List getDefaultPorts(int count) throws IOException { } } - @Test + @Test(expected = IllegalArgumentException.class) public void testInvalidLdapFile(){ - UnboundIdContainer server = new UnboundIdContainer("dc=springframework,dc=org", "classpath:missing-file.ldif"); + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:missing-file.ldif"); server.setPort(0); try { server.start(); - }catch (I){} finally { + }catch (Exception e){} finally { server.destroy(); } } + @Test(expected = IllegalArgumentException.class) + public void testInvalidNullLdapFile(){ + new UnboundIdContainer(validRootDn, null); + } + + @Test(expected = IllegalArgumentException.class) + public void testInvalidEmptyPathLdapFile(){ + new UnboundIdContainer(validRootDn, ""); + } + + @Test(expected = IllegalArgumentException.class) + public void testInvalidRootDnLdapFile(){ + UnboundIdContainer server = new UnboundIdContainer("dc=fake,dc=org", validLdifClassPath); + server.setPort(0); + try { + server.start(); + + }catch (Exception e){} finally { + server.destroy(); + } + } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index 448143bded2..313b0e75919 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -25,8 +25,6 @@ import com.unboundid.ldap.sdk.Entry; import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; - -import org.apache.commons.io.FileUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -64,7 +62,7 @@ public String getLdif() { } public void setLdif(String ldif) { - if(!StringUtils.hasText(ldif)){ + if (!StringUtils.hasText(ldif)) { throw new IllegalArgumentException("ldif file value is missing"); } checkFilePath(ldif); @@ -73,10 +71,10 @@ public void setLdif(String ldif) { private void checkFilePath(String ldif) { File ldifFile = new File(ldif); - if( ldifFile.exists()){ + if (ldifFile.exists()) { throw new IllegalArgumentException("the requested file not found within given path"); } - if(ldifFile.isFile()){ + if (ldifFile.isFile()) { throw new IllegalArgumentException("the given path is not a file"); } } @@ -137,7 +135,7 @@ public void start() { } private void importLdif(InMemoryDirectoryServer directoryServer) { - //TODO - would like to remove this check I think set and get are the best place to handle these + //TODO - would like to remove this check I think set and get are the best place to handle these* if (StringUtils.hasText(this.ldif)) { try { Resource[] resources = this.context.getResources(getLdif()); From 921fbab66fdf789b93e25027ff795b4f99c0209e Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Thu, 7 May 2020 01:06:23 +0300 Subject: [PATCH 04/13] Adding ldap fix to UnboundIdContainer class and adding tests to UnboundIdContainerTests and to UnboundIdContainerLdifTests classes --- .../ldap/SpringLdapFalseAuthTest.java | 42 ----------- .../server/UnboundIdContainerLdifTests.java | 2 +- .../ldap/server/UnboundIdContainerTests.java | 71 ++++++++++++++----- .../ldap/server/UnboundIdContainer.java | 47 +++++------- 4 files changed, 74 insertions(+), 88 deletions(-) delete mode 100644 ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java b/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java deleted file mode 100644 index 49584016d2b..00000000000 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/SpringLdapFalseAuthTest.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.ldap; - -import com.unboundid.ldap.listener.InMemoryDirectoryServer; -import com.unboundid.ldap.sdk.LDAPException; -import org.junit.Test; - - -/** - * @Auther - dratler - */ -public class SpringLdapFalseAuthTest { - - - @Test(expected = IllegalArgumentException.class) - public void testInMemoryDirectoryServerInvalidLdifFile() { - //TODO - get exception here in case of invalid root base - try { - new InMemoryDirectoryServer("dc=springframework,dc=org", - "classpath:missing-file.ldif"); - } catch (LDAPException e) { - throw new IllegalArgumentException(e); - } - } - - -} diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java index a407a244e80..80c6aa93902 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java @@ -120,7 +120,7 @@ public void unboundIdContainerWhenMalformedLdifThenException() { appCtx = new AnnotationConfigApplicationContext(MalformedLdifConfig.class); failBecauseExceptionWasNotThrown(IllegalStateException.class); } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); } } diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index fddf6006d61..e1e93ae2da0 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -28,11 +28,12 @@ /** * @author Eddú Meléndez + * @author Shay Dratler */ public class UnboundIdContainerTests { - private final String validLdifClassPath = "classpath:test-server.ldif"; - private final String validRootDn = "dc=springframework,dc=org"; + private final String validLdifClassPath = "classpath:test-server.ldif"; + private final String validRootDn = "dc=springframework,dc=org"; @Test public void startLdapServer() throws Exception { @@ -81,37 +82,75 @@ private List getDefaultPorts(int count) throws IOException { } } - @Test(expected = IllegalArgumentException.class) - public void testInvalidLdapFile(){ + @Test + public void testInvalidLdapFile() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:missing-file.ldif"); server.setPort(0); try { server.start(); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } + } - }catch (Exception e){} finally { - server.destroy(); + @Test + public void testInvalidNullLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, null); + try { + server.start(); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } + } + + @Test + public void testInvalidEmptyPathLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, ""); + try { + server.start(); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); } } - @Test(expected = IllegalArgumentException.class) - public void testInvalidNullLdapFile(){ - new UnboundIdContainer(validRootDn, null); + @Test + public void testInvalidDirectoryAsLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:/"); + server.setPort(0); + try { + server.start(); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } } - @Test(expected = IllegalArgumentException.class) - public void testInvalidEmptyPathLdapFile(){ - new UnboundIdContainer(validRootDn, ""); + @Test + public void testInvalidFileTextFileAsLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:test-server-malformed.txt"); + server.setPort(0); + try { + server.start(); + + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } } - @Test(expected = IllegalArgumentException.class) - public void testInvalidRootDnLdapFile(){ + @Test + public void testInvalidRootDnLdapFile() { UnboundIdContainer server = new UnboundIdContainer("dc=fake,dc=org", validLdifClassPath); server.setPort(0); try { server.start(); - }catch (Exception e){} finally { - server.destroy(); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index 313b0e75919..ac5fda59e6d 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -15,9 +15,8 @@ */ package org.springframework.security.ldap.server; -import java.io.File; -import java.io.InputStream; +import java.io.InputStream; import com.unboundid.ldap.listener.InMemoryDirectoryServer; import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig; import com.unboundid.ldap.listener.InMemoryListenerConfig; @@ -36,6 +35,7 @@ /** * @author Eddú Meléndez + * @author Shay Dratler */ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lifecycle, ApplicationContextAware { @@ -54,28 +54,15 @@ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lif public UnboundIdContainer(String defaultPartitionSuffix, String ldif) { this.defaultPartitionSuffix = defaultPartitionSuffix; - setLdif(ldif); - } - - public String getLdif() { - return ldif; - } - - public void setLdif(String ldif) { - if (!StringUtils.hasText(ldif)) { - throw new IllegalArgumentException("ldif file value is missing"); - } - checkFilePath(ldif); this.ldif = ldif; } private void checkFilePath(String ldif) { - File ldifFile = new File(ldif); - if (ldifFile.exists()) { - throw new IllegalArgumentException("the requested file not found within given path"); + if (!StringUtils.hasText(ldif)) { + throw new IllegalArgumentException("Unable to load LDIF "+ldif); } - if (ldifFile.isFile()) { - throw new IllegalArgumentException("the given path is not a file"); + if (!ldif.endsWith(".ldif")) { + throw new IllegalArgumentException("Unable to load LDIF "+ldif); } } @@ -135,19 +122,21 @@ public void start() { } private void importLdif(InMemoryDirectoryServer directoryServer) { - //TODO - would like to remove this check I think set and get are the best place to handle these* - if (StringUtils.hasText(this.ldif)) { - try { - Resource[] resources = this.context.getResources(getLdif()); - if (resources.length > 0 && resources[0].exists()) { - try (InputStream inputStream = resources[0].getInputStream()) { - directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); - } + checkFilePath(this.ldif); + try { + Resource[] resources = this.context.getResources(this.ldif); + if (resources.length > 0 + && resources[0].exists() + && resources[0].isFile() + && resources[0].isReadable()) { + try (InputStream inputStream = resources[0].getInputStream()) { + directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); } - } catch (Exception ex) { - throw new IllegalStateException("Unable to load LDIF " + this.ldif, ex); } + } catch (Exception ex) { + throw new IllegalStateException("Unable to load LDIF " + this.ldif, ex); } + } @Override From 10d34923fcf6be651c83cdab35786a1f1f854b3d Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Tue, 12 May 2020 01:06:49 +0300 Subject: [PATCH 05/13] adding some test and fix missing context on destory --- .../ldap/server/UnboundIdContainerTests.java | 89 ++++++++++++------- .../ldap/server/UnboundIdContainer.java | 9 +- 2 files changed, 62 insertions(+), 36 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index e1e93ae2da0..10a539fd6d1 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -15,6 +15,7 @@ */ package org.springframework.security.ldap.server; +import com.unboundid.ldap.sdk.LDAPException; import java.io.IOException; import java.net.ServerSocket; import java.util.ArrayList; @@ -22,6 +23,7 @@ import org.junit.Test; +import org.mockito.internal.matchers.Null; import org.springframework.context.support.GenericApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -37,8 +39,8 @@ public class UnboundIdContainerTests { @Test public void startLdapServer() throws Exception { - UnboundIdContainer server = new UnboundIdContainer(validLdifClassPath, - validRootDn); + UnboundIdContainer server = new UnboundIdContainer( + validRootDn, validLdifClassPath); server.setApplicationContext(new GenericApplicationContext()); List ports = getDefaultPorts(1); server.setPort(ports.get(0)); @@ -54,9 +56,9 @@ public void startLdapServer() throws Exception { @Test public void afterPropertiesSetWhenPortIsZeroThenRandomPortIsSelected() throws Exception { - UnboundIdContainer server = new UnboundIdContainer(validLdifClassPath, validRootDn); + UnboundIdContainer server = new UnboundIdContainer(validRootDn, validLdifClassPath); + server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); - try { server.afterPropertiesSet(); assertThat(server.getPort()).isNotEqualTo(0); @@ -82,75 +84,96 @@ private List getDefaultPorts(int count) throws IOException { } } + @Test + public void missingContextInInit() throws Exception { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, validLdifClassPath); + server.setPort(0); + try { + server.afterPropertiesSet(); + assertThat(server.getPort()).isNotEqualTo(0); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(NullPointerException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); + } finally { + server.destroy(); + } + } + @Test public void testInvalidLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:missing-file.ldif"); + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "missing-file.ldif"); + server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); try { - server.start(); - } catch (Exception e) { + server.afterPropertiesSet(); + } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); + } finally { + server.destroy(); } } @Test public void testInvalidNullLdapFile() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, null); - try { - server.start(); - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); - } + BuildInvalidServer(server); } @Test public void testInvalidEmptyPathLdapFile() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, ""); - try { - server.start(); - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); - } + BuildInvalidServer(server); } @Test public void testInvalidDirectoryAsLdapFile() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:/"); + server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); try { - server.start(); - } catch (Exception e) { + server.afterPropertiesSet(); + } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + assertThat(e.getMessage()).contains("Unable to load LDIF /"); + } finally { + server.destroy(); } } @Test public void testInvalidFileTextFileAsLdapFile() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:test-server-malformed.txt"); + BuildInvalidServer(server); + } + + @Test + public void testInvalidRootDnLdapFile() { + UnboundIdContainer server = new UnboundIdContainer("dc=fake,dc=org", validLdifClassPath); + server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); try { - server.start(); + server.afterPropertiesSet(); - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(LDAPException.class); + assertThat(e.getMessage()).contains("Unable to add entry 'ou=groups,dc=springframework,dc=org' because it is not within any of the base DNs configured in the server."); + } finally { + server.destroy(); } } - @Test - public void testInvalidRootDnLdapFile() { - UnboundIdContainer server = new UnboundIdContainer("dc=fake,dc=org", validLdifClassPath); + private void BuildInvalidServer(UnboundIdContainer server) { + server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); try { - server.start(); + server.afterPropertiesSet(); - } catch (Exception e) { + } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); + } finally { + server.destroy(); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index ac5fda59e6d..a08331d2383 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -59,10 +59,10 @@ public UnboundIdContainer(String defaultPartitionSuffix, String ldif) { private void checkFilePath(String ldif) { if (!StringUtils.hasText(ldif)) { - throw new IllegalArgumentException("Unable to load LDIF "+ldif); + throw new IllegalArgumentException("Unable to load LDIF " + ldif); } if (!ldif.endsWith(".ldif")) { - throw new IllegalArgumentException("Unable to load LDIF "+ldif); + throw new IllegalArgumentException("Unable to load LDIF " + ldif); } } @@ -76,7 +76,9 @@ public void setPort(int port) { @Override public void destroy() { - stop(); + if (null != directoryServer) { + stop(); + } } @Override @@ -124,6 +126,7 @@ public void start() { private void importLdif(InMemoryDirectoryServer directoryServer) { checkFilePath(this.ldif); try { + Resource[] resources = this.context.getResources(this.ldif); if (resources.length > 0 && resources[0].exists() From 2a8c5dd9fdd62c0380434423deec43b6476df7e0 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Sun, 17 May 2020 23:10:36 +0300 Subject: [PATCH 06/13] Fixing the tests --- .../server/UnboundIdContainerLdifTests.java | 2 +- .../ldap/server/UnboundIdContainerTests.java | 70 +------------------ .../ldap/server/UnboundIdContainer.java | 16 +++-- 3 files changed, 13 insertions(+), 75 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java index 80c6aa93902..a407a244e80 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerLdifTests.java @@ -120,7 +120,7 @@ public void unboundIdContainerWhenMalformedLdifThenException() { appCtx = new AnnotationConfigApplicationContext(MalformedLdifConfig.class); failBecauseExceptionWasNotThrown(IllegalStateException.class); } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); } } diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index 10a539fd6d1..4e77aea6ce3 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -15,18 +15,16 @@ */ package org.springframework.security.ldap.server; -import com.unboundid.ldap.sdk.LDAPException; import java.io.IOException; import java.net.ServerSocket; import java.util.ArrayList; import java.util.List; import org.junit.Test; - -import org.mockito.internal.matchers.Null; import org.springframework.context.support.GenericApplicationContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; /** * @author Eddú Meléndez @@ -91,9 +89,9 @@ public void missingContextInInit() throws Exception { try { server.afterPropertiesSet(); assertThat(server.getPort()).isNotEqualTo(0); + failBecauseExceptionWasNotThrown(IllegalArgumentException.class); } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(NullPointerException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); } finally { server.destroy(); } @@ -106,6 +104,7 @@ public void testInvalidLdapFile() { server.setPort(0); try { server.afterPropertiesSet(); + failBecauseExceptionWasNotThrown(IllegalArgumentException.class); } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); @@ -114,67 +113,4 @@ public void testInvalidLdapFile() { } } - @Test - public void testInvalidNullLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, null); - BuildInvalidServer(server); - } - - @Test - public void testInvalidEmptyPathLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, ""); - BuildInvalidServer(server); - } - - @Test - public void testInvalidDirectoryAsLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:/"); - server.setApplicationContext(new GenericApplicationContext()); - server.setPort(0); - try { - server.afterPropertiesSet(); - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF /"); - } finally { - server.destroy(); - } - } - - @Test - public void testInvalidFileTextFileAsLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:test-server-malformed.txt"); - BuildInvalidServer(server); - } - - @Test - public void testInvalidRootDnLdapFile() { - UnboundIdContainer server = new UnboundIdContainer("dc=fake,dc=org", validLdifClassPath); - server.setApplicationContext(new GenericApplicationContext()); - server.setPort(0); - try { - server.afterPropertiesSet(); - - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(LDAPException.class); - assertThat(e.getMessage()).contains("Unable to add entry 'ou=groups,dc=springframework,dc=org' because it is not within any of the base DNs configured in the server."); - } finally { - server.destroy(); - } - } - - private void BuildInvalidServer(UnboundIdContainer server) { - server.setApplicationContext(new GenericApplicationContext()); - server.setPort(0); - try { - server.afterPropertiesSet(); - - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); - } finally { - server.destroy(); - } - } - } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index a08331d2383..bb5c1fb6f25 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -61,9 +61,6 @@ private void checkFilePath(String ldif) { if (!StringUtils.hasText(ldif)) { throw new IllegalArgumentException("Unable to load LDIF " + ldif); } - if (!ldif.endsWith(".ldif")) { - throw new IllegalArgumentException("Unable to load LDIF " + ldif); - } } public int getPort() { @@ -119,6 +116,8 @@ public void start() { this.running = true; } catch (LDAPException ex) { throw new RuntimeException("Server startup failed", ex); + } catch (IllegalArgumentException ex){ + throw ex; } } @@ -130,10 +129,13 @@ private void importLdif(InMemoryDirectoryServer directoryServer) { Resource[] resources = this.context.getResources(this.ldif); if (resources.length > 0 && resources[0].exists() - && resources[0].isFile() - && resources[0].isReadable()) { - try (InputStream inputStream = resources[0].getInputStream()) { - directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); + ) { + if (resources[0].isFile() && resources[0].isReadable()) { + try (InputStream inputStream = resources[0].getInputStream()) { + directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); + } + } else { + throw new IllegalStateException("Unable to load LDIF " + this.ldif); } } } catch (Exception ex) { From c99b252a95c798878a16af71f85f63172169fe73 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Mon, 18 May 2020 22:48:20 +0300 Subject: [PATCH 07/13] Fix test --- .../ldap/server/UnboundIdContainerTests.java | 25 +++++++++++++++---- .../ldap/server/UnboundIdContainer.java | 2 ++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index 4e77aea6ce3..d4e0f31e642 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -89,7 +89,7 @@ public void missingContextInInit() throws Exception { try { server.afterPropertiesSet(); assertThat(server.getPort()).isNotEqualTo(0); - failBecauseExceptionWasNotThrown(IllegalArgumentException.class); + failBecauseExceptionWasNotThrown(NullPointerException.class); } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(NullPointerException.class); } finally { @@ -98,19 +98,34 @@ public void missingContextInInit() throws Exception { } @Test - public void testInvalidLdapFile() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, "missing-file.ldif"); + public void testMissingLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:missing-file.ldif"); server.setApplicationContext(new GenericApplicationContext()); server.setPort(0); try { server.afterPropertiesSet(); - failBecauseExceptionWasNotThrown(IllegalArgumentException.class); } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF test-server-malformed.txt"); + assertThat(e.getMessage()).contains("Unable to load LDIF missing-file.ldif"); } finally { server.destroy(); } } + + @Test + public void testInvalidLdapFile() { + UnboundIdContainer server = new UnboundIdContainer(validRootDn, "classpath:test-server-malformed.txt"); + server.setApplicationContext(new GenericApplicationContext()); + server.setPort(0); + try { + server.afterPropertiesSet(); + failBecauseExceptionWasNotThrown(IllegalStateException.class); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + } finally { + server.destroy(); + } + } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index bb5c1fb6f25..e955aff404b 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -133,6 +133,8 @@ private void importLdif(InMemoryDirectoryServer directoryServer) { if (resources[0].isFile() && resources[0].isReadable()) { try (InputStream inputStream = resources[0].getInputStream()) { directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); + } catch (Exception e){ + throw new IllegalStateException("Unable to load LDIF " + this.ldif, e); } } else { throw new IllegalStateException("Unable to load LDIF " + this.ldif); From 9fac54f84c606268dd8bb1b5dae5456e03855f51 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Wed, 27 May 2020 23:21:38 +0300 Subject: [PATCH 08/13] fixing ldif loading issue --- .../ldap/server/UnboundIdContainerTests.java | 19 +++++++- .../ldap/server/UnboundIdContainer.java | 45 ++++++++++--------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index d4e0f31e642..eb6cfe75280 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -83,7 +83,23 @@ private List getDefaultPorts(int count) throws IOException { } @Test - public void missingContextInInit() throws Exception { + public void startLdapServerWithoutLdif() throws Exception { + UnboundIdContainer server = new UnboundIdContainer( + validRootDn, null); + server.setApplicationContext(new GenericApplicationContext()); + List ports = getDefaultPorts(1); + server.setPort(ports.get(0)); + + try { + server.afterPropertiesSet(); + assertThat(server.getPort()).isEqualTo(ports.get(0)); + } finally { + server.destroy(); + } + } + + @Test + public void missingContextInInit() { UnboundIdContainer server = new UnboundIdContainer(validRootDn, validLdifClassPath); server.setPort(0); try { @@ -104,6 +120,7 @@ public void testMissingLdapFile() { server.setPort(0); try { server.afterPropertiesSet(); + failBecauseExceptionWasNotThrown(IllegalStateException.class); } catch (Exception e) { assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); assertThat(e.getMessage()).contains("Unable to load LDIF missing-file.ldif"); diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index e955aff404b..f1e3ac4ceef 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -24,6 +24,7 @@ import com.unboundid.ldap.sdk.Entry; import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; +import java.util.Arrays; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -57,11 +58,6 @@ public UnboundIdContainer(String defaultPartitionSuffix, String ldif) { this.ldif = ldif; } - private void checkFilePath(String ldif) { - if (!StringUtils.hasText(ldif)) { - throw new IllegalArgumentException("Unable to load LDIF " + ldif); - } - } public int getPort() { return this.port; @@ -116,36 +112,41 @@ public void start() { this.running = true; } catch (LDAPException ex) { throw new RuntimeException("Server startup failed", ex); - } catch (IllegalArgumentException ex){ + } catch (IllegalArgumentException ex) { throw ex; } } private void importLdif(InMemoryDirectoryServer directoryServer) { - checkFilePath(this.ldif); - try { + if (!StringUtils.hasText(this.ldif)) { + try { - Resource[] resources = this.context.getResources(this.ldif); - if (resources.length > 0 - && resources[0].exists() - ) { - if (resources[0].isFile() && resources[0].isReadable()) { - try (InputStream inputStream = resources[0].getInputStream()) { - directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); - } catch (Exception e){ - throw new IllegalStateException("Unable to load LDIF " + this.ldif, e); - } - } else { - throw new IllegalStateException("Unable to load LDIF " + this.ldif); + Resource[] resources = this.context.getResources("*.ldif"); + Resource resource = locateResource(resources); + try (InputStream inputStream = resource.getInputStream()) { + directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); } + } catch (Exception ex) { + throw new IllegalStateException("Unable to load LDIF " + this.ldif, ex); } - } catch (Exception ex) { - throw new IllegalStateException("Unable to load LDIF " + this.ldif, ex); } } + private Resource locateResource(Resource[] resources) { + if (null == resources || 0 == resources.length) { + throw new IllegalArgumentException("requested resource is not found"); + } + return Arrays.asList(resources).stream().filter(resource -> + resource.getFilename().equalsIgnoreCase(this.ldif) + && resource.isFile() + && resource.exists() + && resource.isReadable() + ) + .findFirst().orElseThrow(IllegalArgumentException::new); + } + @Override public void stop() { this.directoryServer.shutDown(true); From bc8e0999e677f02b47c9c2f48057a1505f6a3e2a Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Mon, 1 Jun 2020 22:08:24 +0300 Subject: [PATCH 09/13] Adding more tests and better validation --- .../ldap/server/UnboundIdContainerTests.java | 33 +++++++++++-------- .../resources/applicationContext-security.xml | 9 +++++ .../ldap/server/UnboundIdContainer.java | 17 ++++++---- 3 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 ldap/src/integration-test/resources/applicationContext-security.xml diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index eb6cfe75280..a3a195f93b3 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -21,6 +21,7 @@ import java.util.List; import org.junit.Test; +import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -39,16 +40,7 @@ public class UnboundIdContainerTests { public void startLdapServer() throws Exception { UnboundIdContainer server = new UnboundIdContainer( validRootDn, validLdifClassPath); - server.setApplicationContext(new GenericApplicationContext()); - List ports = getDefaultPorts(1); - server.setPort(ports.get(0)); - - try { - server.afterPropertiesSet(); - assertThat(server.getPort()).isEqualTo(ports.get(0)); - } finally { - server.destroy(); - } + createAndRunServer(server); } @@ -86,6 +78,10 @@ private List getDefaultPorts(int count) throws IOException { public void startLdapServerWithoutLdif() throws Exception { UnboundIdContainer server = new UnboundIdContainer( validRootDn, null); + createAndRunServer(server); + } + + private void createAndRunServer(UnboundIdContainer server) throws IOException { server.setApplicationContext(new GenericApplicationContext()); List ports = getDefaultPorts(1); server.setPort(ports.get(0)); @@ -105,9 +101,6 @@ public void missingContextInInit() { try { server.afterPropertiesSet(); assertThat(server.getPort()).isNotEqualTo(0); - failBecauseExceptionWasNotThrown(NullPointerException.class); - } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(NullPointerException.class); } finally { server.destroy(); } @@ -145,4 +138,18 @@ public void testInvalidLdapFile() { server.destroy(); } } + + @Test + public void loadContextFromXmlApacheDSContainer() { + ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("applicationContext-security.xml"); + String[] beanNames = context.getBeanNamesForType(ApacheDSContainer.class); + assertThat(beanNames).hasSize(2); + } + + @Test + public void loadContextFromXmlUnboundIdContainer() { + ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("applicationContext-security.xml"); + String[] beanNames = context.getBeanNamesForType(UnboundIdContainer.class); + assertThat(beanNames).hasSize(1); + } } diff --git a/ldap/src/integration-test/resources/applicationContext-security.xml b/ldap/src/integration-test/resources/applicationContext-security.xml new file mode 100644 index 00000000000..94e4e7ac469 --- /dev/null +++ b/ldap/src/integration-test/resources/applicationContext-security.xml @@ -0,0 +1,9 @@ + + + + + diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index f1e3ac4ceef..c00e0526ad5 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -16,6 +16,8 @@ package org.springframework.security.ldap.server; +import static org.springframework.core.io.support.ResourcePatternResolver.CLASSPATH_ALL_URL_PREFIX; + import java.io.InputStream; import com.unboundid.ldap.listener.InMemoryDirectoryServer; import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig; @@ -25,6 +27,7 @@ import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; import java.util.Arrays; +import java.util.Objects; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -118,11 +121,10 @@ public void start() { } - private void importLdif(InMemoryDirectoryServer directoryServer) { - if (!StringUtils.hasText(this.ldif)) { + private void importLdif(InMemoryDirectoryServer directoryServer) throws LDAPException { + if (StringUtils.hasText(this.ldif)) { try { - - Resource[] resources = this.context.getResources("*.ldif"); + Resource[] resources = this.context.getResources(CLASSPATH_ALL_URL_PREFIX); Resource resource = locateResource(resources); try (InputStream inputStream = resource.getInputStream()) { directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); @@ -138,13 +140,14 @@ private Resource locateResource(Resource[] resources) { if (null == resources || 0 == resources.length) { throw new IllegalArgumentException("requested resource is not found"); } - return Arrays.asList(resources).stream().filter(resource -> - resource.getFilename().equalsIgnoreCase(this.ldif) + return Arrays.stream(resources).filter(resource -> + Objects.requireNonNull(resource.getFilename()).equalsIgnoreCase(this.ldif) && resource.isFile() && resource.exists() && resource.isReadable() ) - .findFirst().orElseThrow(IllegalArgumentException::new); + .findFirst() + .orElseThrow(()->new IllegalArgumentException("Unable to load LDIF :"+this.ldif)); } @Override From 23ac3828ae8f40f830043ff97870652d4af4829e Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Mon, 8 Jun 2020 21:53:00 +0300 Subject: [PATCH 10/13] attempt to fix the issue + add to git ignore /lib/ and .hprof files --- .../security/ldap/server/UnboundIdContainer.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index c00e0526ad5..cf28a96b917 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -15,9 +15,7 @@ */ package org.springframework.security.ldap.server; - -import static org.springframework.core.io.support.ResourcePatternResolver.CLASSPATH_ALL_URL_PREFIX; - +import java.io.IOException; import java.io.InputStream; import com.unboundid.ldap.listener.InMemoryDirectoryServer; import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig; @@ -124,8 +122,8 @@ public void start() { private void importLdif(InMemoryDirectoryServer directoryServer) throws LDAPException { if (StringUtils.hasText(this.ldif)) { try { - Resource[] resources = this.context.getResources(CLASSPATH_ALL_URL_PREFIX); - Resource resource = locateResource(resources); + + Resource resource = locateResource(); try (InputStream inputStream = resource.getInputStream()) { directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); } @@ -136,7 +134,8 @@ private void importLdif(InMemoryDirectoryServer directoryServer) throws LDAPExce } - private Resource locateResource(Resource[] resources) { + private Resource locateResource() throws IOException , NullPointerException { + Resource[] resources = this.context.getResources(this.ldif); if (null == resources || 0 == resources.length) { throw new IllegalArgumentException("requested resource is not found"); } From 179a4668615c347107eea29f2807efb4280bbcc9 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Tue, 16 Jun 2020 23:31:44 +0300 Subject: [PATCH 11/13] attempt to resolve issue --- .../security/ldap/server/UnboundIdContainer.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index cf28a96b917..a733738c150 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -25,7 +25,6 @@ import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; import java.util.Arrays; -import java.util.Objects; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -37,7 +36,7 @@ /** * @author Eddú Meléndez - * @author Shay Dratler + * @contributer Shay Dratler */ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lifecycle, ApplicationContextAware { @@ -140,7 +139,7 @@ private Resource locateResource() throws IOException , NullPointerException { throw new IllegalArgumentException("requested resource is not found"); } return Arrays.stream(resources).filter(resource -> - Objects.requireNonNull(resource.getFilename()).equalsIgnoreCase(this.ldif) + !StringUtils.isEmpty(resource.getFilename()) && resource.isFile() && resource.exists() && resource.isReadable() From 0efb88664a79207447d17ae52d121bf20bff6555 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Sat, 20 Jun 2020 23:32:11 +0300 Subject: [PATCH 12/13] attempt fix 1 --- .../ldap/server/UnboundIdContainerTests.java | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index a3a195f93b3..1882aaaeb8b 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -15,13 +15,13 @@ */ package org.springframework.security.ldap.server; +import com.unboundid.ldap.sdk.LDAPException; import java.io.IOException; import java.net.ServerSocket; import java.util.ArrayList; import java.util.List; import org.junit.Test; -import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -96,7 +96,7 @@ private void createAndRunServer(UnboundIdContainer server) throws IOException { @Test public void missingContextInInit() { - UnboundIdContainer server = new UnboundIdContainer(validRootDn, validLdifClassPath); + UnboundIdContainer server = new UnboundIdContainer(validRootDn, null); server.setPort(0); try { server.afterPropertiesSet(); @@ -113,10 +113,10 @@ public void testMissingLdapFile() { server.setPort(0); try { server.afterPropertiesSet(); - failBecauseExceptionWasNotThrown(IllegalStateException.class); + failBecauseExceptionWasNotThrown(IllegalArgumentException.class); } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF missing-file.ldif"); + assertThat(e.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(e.getMessage()).contains("Unable to load LDIF classpath:missing-file.ldif"); } finally { server.destroy(); } @@ -130,26 +130,12 @@ public void testInvalidLdapFile() { server.setPort(0); try { server.afterPropertiesSet(); - failBecauseExceptionWasNotThrown(IllegalStateException.class); + failBecauseExceptionWasNotThrown(LDAPException.class); } catch (Exception e) { - assertThat(e.getCause()).isInstanceOf(IllegalStateException.class); - assertThat(e.getMessage()).contains("Unable to load LDIF classpath:test-server-malformed.txt"); + assertThat(e.getCause()).isInstanceOf(LDAPException.class); } finally { server.destroy(); } } - @Test - public void loadContextFromXmlApacheDSContainer() { - ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("applicationContext-security.xml"); - String[] beanNames = context.getBeanNamesForType(ApacheDSContainer.class); - assertThat(beanNames).hasSize(2); - } - - @Test - public void loadContextFromXmlUnboundIdContainer() { - ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("applicationContext-security.xml"); - String[] beanNames = context.getBeanNamesForType(UnboundIdContainer.class); - assertThat(beanNames).hasSize(1); - } } From 1fa8a4bfb4795c703639790185f5169baea17383 Mon Sep 17 00:00:00 2001 From: sdratler1 Date: Tue, 14 Jul 2020 23:54:14 +0300 Subject: [PATCH 13/13] handle requested rejects --- .../ldap/server/UnboundIdContainerTests.java | 61 ++++++++++++------- .../resources/applicationContext-security.xml | 9 --- .../ldap/server/UnboundIdContainer.java | 31 +++++----- 3 files changed, 53 insertions(+), 48 deletions(-) delete mode 100644 ldap/src/integration-test/resources/applicationContext-security.xml diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java index 1882aaaeb8b..6f264a4b01e 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/server/UnboundIdContainerTests.java @@ -35,6 +35,7 @@ public class UnboundIdContainerTests { private final String validLdifClassPath = "classpath:test-server.ldif"; private final String validRootDn = "dc=springframework,dc=org"; + private final String validLdifFileClassPathTop = "classpath:test-server-custom-attribute-types.ldif"; @Test public void startLdapServer() throws Exception { @@ -57,23 +58,6 @@ public void afterPropertiesSetWhenPortIsZeroThenRandomPortIsSelected() throws Ex } } - private List getDefaultPorts(int count) throws IOException { - List connections = new ArrayList<>(); - List availablePorts = new ArrayList<>(count); - try { - for (int i = 0; i < count; i++) { - ServerSocket socket = new ServerSocket(0); - connections.add(socket); - availablePorts.add(socket.getLocalPort()); - } - return availablePorts; - } finally { - for (ServerSocket conn : connections) { - conn.close(); - } - } - } - @Test public void startLdapServerWithoutLdif() throws Exception { UnboundIdContainer server = new UnboundIdContainer( @@ -81,14 +65,17 @@ public void startLdapServerWithoutLdif() throws Exception { createAndRunServer(server); } - private void createAndRunServer(UnboundIdContainer server) throws IOException { + @Test + public void startLdapServerMixedLdif() throws Exception { + UnboundIdContainer server = new UnboundIdContainer( + validRootDn, validLdifFileClassPathTop); server.setApplicationContext(new GenericApplicationContext()); - List ports = getDefaultPorts(1); - server.setPort(ports.get(0)); - + server.setPort(0); try { server.afterPropertiesSet(); - assertThat(server.getPort()).isEqualTo(ports.get(0)); + failBecauseExceptionWasNotThrown(LDAPException.class); + } catch (Exception e) { + assertThat(e.getCause()).isInstanceOf(LDAPException.class); } finally { server.destroy(); } @@ -138,4 +125,34 @@ public void testInvalidLdapFile() { } } + private List getDefaultPorts(int count) throws IOException { + List connections = new ArrayList<>(); + List availablePorts = new ArrayList<>(count); + try { + for (int i = 0; i < count; i++) { + ServerSocket socket = new ServerSocket(0); + connections.add(socket); + availablePorts.add(socket.getLocalPort()); + } + return availablePorts; + } finally { + for (ServerSocket conn : connections) { + conn.close(); + } + } + } + + private void createAndRunServer(UnboundIdContainer server) throws IOException { + server.setApplicationContext(new GenericApplicationContext()); + List ports = getDefaultPorts(1); + server.setPort(ports.get(0)); + + try { + server.afterPropertiesSet(); + assertThat(server.getPort()).isEqualTo(ports.get(0)); + } finally { + server.destroy(); + } + } + } diff --git a/ldap/src/integration-test/resources/applicationContext-security.xml b/ldap/src/integration-test/resources/applicationContext-security.xml deleted file mode 100644 index 94e4e7ac469..00000000000 --- a/ldap/src/integration-test/resources/applicationContext-security.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index a733738c150..c277b6ad59c 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -15,6 +15,8 @@ */ package org.springframework.security.ldap.server; +import static org.springframework.util.StringUtils.isEmpty; + import java.io.IOException; import java.io.InputStream; import com.unboundid.ldap.listener.InMemoryDirectoryServer; @@ -24,7 +26,6 @@ import com.unboundid.ldap.sdk.Entry; import com.unboundid.ldap.sdk.LDAPException; import com.unboundid.ldif.LDIFReader; -import java.util.Arrays; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -89,7 +90,6 @@ public void start() { if (isRunning()) { return; } - try { InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig(this.defaultPartitionSuffix); config.addAdditionalBindCredentials("uid=admin,ou=system", "secret"); @@ -112,16 +112,12 @@ public void start() { this.running = true; } catch (LDAPException ex) { throw new RuntimeException("Server startup failed", ex); - } catch (IllegalArgumentException ex) { - throw ex; } - } - private void importLdif(InMemoryDirectoryServer directoryServer) throws LDAPException { + private void importLdif(InMemoryDirectoryServer directoryServer) { if (StringUtils.hasText(this.ldif)) { try { - Resource resource = locateResource(); try (InputStream inputStream = resource.getInputStream()) { directoryServer.importFromLDIF(false, new LDIFReader(inputStream)); @@ -133,19 +129,20 @@ private void importLdif(InMemoryDirectoryServer directoryServer) throws LDAPExce } - private Resource locateResource() throws IOException , NullPointerException { + private Resource locateResource() throws IOException, NullPointerException { Resource[] resources = this.context.getResources(this.ldif); - if (null == resources || 0 == resources.length) { + if (0 == resources.length) { throw new IllegalArgumentException("requested resource is not found"); } - return Arrays.stream(resources).filter(resource -> - !StringUtils.isEmpty(resource.getFilename()) - && resource.isFile() - && resource.exists() - && resource.isReadable() - ) - .findFirst() - .orElseThrow(()->new IllegalArgumentException("Unable to load LDIF :"+this.ldif)); + for (Resource resource : resources) { + if (!isEmpty(resource.getFilename()) + && resource.isFile() + && resource.exists() + && resource.isReadable()) { + return resource; + } + } + throw new IllegalArgumentException("Unable to load LDIF :" + this.ldif); } @Override