From 89037c3dfbf973bd7b157ee50b599a0819b830ba Mon Sep 17 00:00:00 2001 From: ronanclancy <macuser@Macs-MacBook-Pro.local> Date: Tue, 26 Mar 2019 10:01:07 +0000 Subject: [PATCH 1/5] Fim simple email assignment typo --- .../java/org/owasp/webgoat/plugin/SimpleMailAssignment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webgoat-lessons/password-reset/src/main/java/org/owasp/webgoat/plugin/SimpleMailAssignment.java b/webgoat-lessons/password-reset/src/main/java/org/owasp/webgoat/plugin/SimpleMailAssignment.java index a2c551687..e32815d66 100644 --- a/webgoat-lessons/password-reset/src/main/java/org/owasp/webgoat/plugin/SimpleMailAssignment.java +++ b/webgoat-lessons/password-reset/src/main/java/org/owasp/webgoat/plugin/SimpleMailAssignment.java @@ -63,7 +63,7 @@ public class SimpleMailAssignment extends AssignmentEndpoint { .recipient(username) .title("Simple e-mail assignment") .time(LocalDateTime.now()) - .contents("Thanks your resetting your password, your new password is: " + StringUtils.reverse(username)) + .contents("Thanks for resetting your password, your new password is: " + StringUtils.reverse(username)) .sender("webgoat@owasp.org") .build(); try { From a242347ee0bb0107ce8919768ba2967c56305ee8 Mon Sep 17 00:00:00 2001 From: rjclancy <ronan.clancy@gmail.com> Date: Tue, 26 Mar 2019 12:05:42 +0000 Subject: [PATCH 2/5] add UserService unit test, modify UserService --- .../org/owasp/webwolf/user/UserService.java | 19 +++--- .../owasp/webwolf/user/UserServiceTest.java | 66 +++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java diff --git a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java index 319a9a355..85c1576ff 100644 --- a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java +++ b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java @@ -1,6 +1,6 @@ package org.owasp.webwolf.user; -import lombok.AllArgsConstructor; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.stereotype.Service; @@ -10,26 +10,27 @@ import org.springframework.stereotype.Service; * @since 3/19/17. */ @Service -@AllArgsConstructor public class UserService implements UserDetailsService { - private final UserRepository userRepository; + private UserRepository userRepository; + + @Autowired + public UserService(final UserRepository userRepository) { + this.userRepository = userRepository; + } @Override - public WebGoatUser loadUserByUsername(String username) throws UsernameNotFoundException { + public WebGoatUser loadUserByUsername(final String username) throws UsernameNotFoundException { WebGoatUser webGoatUser = userRepository.findByUsername(username); if (webGoatUser == null) { throw new UsernameNotFoundException("User not found"); - } else { - webGoatUser.createUser(); } + webGoatUser.createUser(); return webGoatUser; } - public void addUser(String username, String password) { + public void addUser(final String username, final String password) { userRepository.save(new WebGoatUser(username, password)); } - - } diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java new file mode 100644 index 000000000..63da863bf --- /dev/null +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java @@ -0,0 +1,66 @@ +package org.owasp.webwolf.user; + + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.springframework.security.core.userdetails.UsernameNotFoundException; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.mockito.Mockito.*; + +@RunWith(SpringJUnit4ClassRunner.class) +public class UserServiceTest { + + @Mock + private UserRepository mockUserRepository; + + private UserService cut; + + @Before + public void setup(){ + cut = new UserService(mockUserRepository); + } + + @Test + public void testLoadUserByUsername(){ + // setup + final String username = "guest"; + final String password = "123"; + + WebGoatUser user = new WebGoatUser(username, password); + when(mockUserRepository.findByUsername(username)).thenReturn(user); + + // execute + final WebGoatUser webGoatUser = cut.loadUserByUsername(username); + + // verify + Assert.assertEquals(username, webGoatUser.getUsername()); + Assert.assertEquals(password, webGoatUser.getPassword()); + } + + @Test(expected = UsernameNotFoundException.class) + public void testLoadUserByUsername_NULL(){ + // setup + final String username = "guest"; + when(mockUserRepository.findByUsername(username)).thenReturn(null); + + // execute + cut.loadUserByUsername(username); + } + + @Test + public void testAddUser(){ + // setup + final String username = "guest"; + final String password = "guest"; + + // execute + cut.addUser(username, password); + + // verify + verify(mockUserRepository, times(1)).save(any(WebGoatUser.class)); + } +} From c6c0cc60f94712503e2eeb68217db1d1e07ce146 Mon Sep 17 00:00:00 2001 From: rjclancy <ronan.clancy@gmail.com> Date: Tue, 26 Mar 2019 20:23:28 +0000 Subject: [PATCH 3/5] Add UserValidator tests + minor code clean up --- .../org/owasp/webwolf/user/UserService.java | 8 +- .../owasp/webwolf/user/UserServiceTest.java | 7 +- .../owasp/webwolf/user/UserValidatorTest.java | 85 +++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java diff --git a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java index 85c1576ff..90a430a1c 100644 --- a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java +++ b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java @@ -1,5 +1,6 @@ package org.owasp.webwolf.user; +import lombok.AllArgsConstructor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; @@ -10,14 +11,11 @@ import org.springframework.stereotype.Service; * @since 3/19/17. */ @Service +@AllArgsConstructor public class UserService implements UserDetailsService { - private UserRepository userRepository; - @Autowired - public UserService(final UserRepository userRepository) { - this.userRepository = userRepository; - } + private UserRepository userRepository; @Override public WebGoatUser loadUserByUsername(final String username) throws UsernameNotFoundException { diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java index 63da863bf..ad53e36ec 100644 --- a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java @@ -5,6 +5,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -17,13 +18,9 @@ public class UserServiceTest { @Mock private UserRepository mockUserRepository; + @InjectMocks private UserService cut; - @Before - public void setup(){ - cut = new UserService(mockUserRepository); - } - @Test public void testLoadUserByUsername(){ // setup diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java new file mode 100644 index 000000000..60e614a58 --- /dev/null +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java @@ -0,0 +1,85 @@ +package org.owasp.webwolf.user; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.validation.BindException; + +import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertFalse; +import static org.mockito.Mockito.when; + +/** + * @author rjclancy + * @since 3/26/19. + */ +@RunWith(SpringJUnit4ClassRunner.class) +public class UserValidatorTest { + + @Mock + private UserRepository mockUserRepository; + + @InjectMocks + private UserValidator userValidator; + + @Test + public void testValidUserForm() { + UserForm validUserForm = new UserForm(); + validUserForm.setUsername("guest"); + validUserForm.setMatchingPassword("123"); + validUserForm.setPassword("123"); + BindException errors = new BindException(validUserForm, "validUserForm"); + + userValidator.validate(validUserForm, errors); + + assertFalse(errors.hasErrors()); + } + + @Test + public void testValidUserForm_INVALID_PASSWORD() { + UserForm validUserForm = new UserForm(); + validUserForm.setUsername("guest"); + validUserForm.setMatchingPassword("123"); + validUserForm.setPassword("124"); + BindException errors = new BindException(validUserForm, "validUserForm"); + + userValidator.validate(validUserForm, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void testValidUserForm_DUPLICATE_USER() { + // setup + final String username = "guest"; + final String password = "123"; + + UserForm validUserForm = new UserForm(); + validUserForm.setUsername(username); + validUserForm.setMatchingPassword(password); + validUserForm.setPassword("124"); + BindException errors = new BindException(validUserForm, "validUserForm"); + + WebGoatUser webGoatUser = new WebGoatUser(username, password); + when(mockUserRepository.findByUsername(validUserForm.getUsername())).thenReturn(webGoatUser); + + // execute + userValidator.validate(validUserForm, errors); + + // verify + assertTrue(errors.hasErrors()); + } + + @Test + public void testSupports(){ + Assert.assertTrue(userValidator.supports(UserForm.class)); + } + + @Test + public void testSupports_false(){ + Assert.assertFalse(userValidator.supports(UserService.class)); + } +} \ No newline at end of file From 331d9c8dd4b7f20563e456dc0e3cdc2c9373d59c Mon Sep 17 00:00:00 2001 From: rjclancy <ronan.clancy@gmail.com> Date: Tue, 26 Mar 2019 20:33:11 +0000 Subject: [PATCH 4/5] add authur tag to test class --- .../src/test/java/org/owasp/webwolf/user/UserServiceTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java index ad53e36ec..3d611c30a 100644 --- a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java @@ -12,6 +12,10 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import static org.mockito.Mockito.*; +/** + * @author rjclancy + * @since 3/26/19. + */ @RunWith(SpringJUnit4ClassRunner.class) public class UserServiceTest { From 43c25dc3bb321b2acd582df837c696aabe386cbb Mon Sep 17 00:00:00 2001 From: Nanne Baars <nanne.baars@owasp.org> Date: Tue, 10 Sep 2019 13:53:30 +0200 Subject: [PATCH 5/5] Modified PR to reflect coding style --- .../org/owasp/webwolf/user/UserService.java | 5 ++- .../owasp/webwolf/user/UserServiceTest.java | 39 ++++++----------- .../owasp/webwolf/user/UserValidatorTest.java | 42 ++++++++----------- 3 files changed, 33 insertions(+), 53 deletions(-) diff --git a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java index 5acc52f92..b8cd0c9c3 100644 --- a/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java +++ b/webwolf/src/main/java/org/owasp/webwolf/user/UserService.java @@ -14,9 +14,12 @@ import org.springframework.stereotype.Service; @Service public class UserService implements UserDetailsService { - @Autowired private UserRepository userRepository; + public UserService(UserRepository userRepository) { + this.userRepository = userRepository; + } + @Override public WebGoatUser loadUserByUsername(final String username) throws UsernameNotFoundException { WebGoatUser webGoatUser = userRepository.findByUsername(username); diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java index 3d611c30a..e05048841 100644 --- a/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserServiceTest.java @@ -1,8 +1,6 @@ package org.owasp.webwolf.user; - -import org.junit.Assert; -import org.junit.Before; +import org.assertj.core.api.Assertions; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -12,10 +10,6 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import static org.mockito.Mockito.*; -/** - * @author rjclancy - * @since 3/26/19. - */ @RunWith(SpringJUnit4ClassRunner.class) public class UserServiceTest { @@ -23,45 +17,36 @@ public class UserServiceTest { private UserRepository mockUserRepository; @InjectMocks - private UserService cut; + private UserService sut; @Test public void testLoadUserByUsername(){ - // setup - final String username = "guest"; - final String password = "123"; - + var username = "guest"; + var password = "123"; WebGoatUser user = new WebGoatUser(username, password); when(mockUserRepository.findByUsername(username)).thenReturn(user); - // execute - final WebGoatUser webGoatUser = cut.loadUserByUsername(username); + var webGoatUser = sut.loadUserByUsername(username); - // verify - Assert.assertEquals(username, webGoatUser.getUsername()); - Assert.assertEquals(password, webGoatUser.getPassword()); + Assertions.assertThat(username).isEqualTo(webGoatUser.getUsername()); + Assertions.assertThat(password).isEqualTo(webGoatUser.getPassword()); } @Test(expected = UsernameNotFoundException.class) public void testLoadUserByUsername_NULL(){ - // setup - final String username = "guest"; + var username = "guest"; when(mockUserRepository.findByUsername(username)).thenReturn(null); - // execute - cut.loadUserByUsername(username); + sut.loadUserByUsername(username); } @Test public void testAddUser(){ - // setup - final String username = "guest"; - final String password = "guest"; + var username = "guest"; + var password = "guest"; - // execute - cut.addUser(username, password); + sut.addUser(username, password); - // verify verify(mockUserRepository, times(1)).save(any(WebGoatUser.class)); } } diff --git a/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java b/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java index 60e614a58..5c160f5e1 100644 --- a/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java +++ b/webwolf/src/test/java/org/owasp/webwolf/user/UserValidatorTest.java @@ -1,5 +1,6 @@ package org.owasp.webwolf.user; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -12,10 +13,6 @@ import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.when; -/** - * @author rjclancy - * @since 3/26/19. - */ @RunWith(SpringJUnit4ClassRunner.class) public class UserValidatorTest { @@ -26,8 +23,8 @@ public class UserValidatorTest { private UserValidator userValidator; @Test - public void testValidUserForm() { - UserForm validUserForm = new UserForm(); + public void validUserFormShouldNotHaveErrors() { + var validUserForm = new UserForm(); validUserForm.setUsername("guest"); validUserForm.setMatchingPassword("123"); validUserForm.setPassword("123"); @@ -35,12 +32,12 @@ public class UserValidatorTest { userValidator.validate(validUserForm, errors); - assertFalse(errors.hasErrors()); + Assertions.assertThat(errors.hasErrors()).isFalse(); } @Test - public void testValidUserForm_INVALID_PASSWORD() { - UserForm validUserForm = new UserForm(); + public void whenPasswordDoNotMatchShouldFail() { + var validUserForm = new UserForm(); validUserForm.setUsername("guest"); validUserForm.setMatchingPassword("123"); validUserForm.setPassword("124"); @@ -48,38 +45,33 @@ public class UserValidatorTest { userValidator.validate(validUserForm, errors); - assertTrue(errors.hasErrors()); + Assertions.assertThat(errors.hasErrors()).isTrue(); } @Test - public void testValidUserForm_DUPLICATE_USER() { - // setup - final String username = "guest"; - final String password = "123"; - - UserForm validUserForm = new UserForm(); + public void registerExistingUserAgainShouldFail() { + var username = "guest"; + var password = "123"; + var validUserForm = new UserForm(); validUserForm.setUsername(username); validUserForm.setMatchingPassword(password); validUserForm.setPassword("124"); BindException errors = new BindException(validUserForm, "validUserForm"); - - WebGoatUser webGoatUser = new WebGoatUser(username, password); + var webGoatUser = new WebGoatUser(username, password); when(mockUserRepository.findByUsername(validUserForm.getUsername())).thenReturn(webGoatUser); - // execute userValidator.validate(validUserForm, errors); - // verify - assertTrue(errors.hasErrors()); + Assertions.assertThat(errors.hasErrors()).isTrue(); } @Test - public void testSupports(){ - Assert.assertTrue(userValidator.supports(UserForm.class)); + public void testSupports() { + Assertions.assertThat(userValidator.supports(UserForm.class)).isTrue(); } @Test - public void testSupports_false(){ - Assert.assertFalse(userValidator.supports(UserService.class)); + public void testSupports_false() { + Assertions.assertThat(userValidator.supports(UserService.class)).isFalse(); } } \ No newline at end of file