From 7ca6d184f891e2307d1cd86de2333bafddfb3a99 Mon Sep 17 00:00:00 2001 From: Jennifer Arps Date: Mon, 24 Jan 2022 14:36:43 +0100 Subject: [PATCH] [#141] bug fix SecurityConfig and CustomOAuth2UserServiceTest + add AppUserTests --- .../de/bonndan/nivio/appuser/AppUser.java | 4 +--- .../nivio/security/CustomOAuth2User.java | 4 ++-- .../security/CustomOAuth2UserService.java | 2 +- .../nivio/security/SecurityConfig.java | 9 +------- .../de/bonndan/nivio/appuser/AppUserTest.java | 22 ++++++++++++++++++- .../security/CustomOAuth2UserServiceTest.java | 5 ++++- .../LoginControllerCaseRequiredTest.java | 2 +- 7 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/main/java/de/bonndan/nivio/appuser/AppUser.java b/src/main/java/de/bonndan/nivio/appuser/AppUser.java index d629f769b..656cf117d 100644 --- a/src/main/java/de/bonndan/nivio/appuser/AppUser.java +++ b/src/main/java/de/bonndan/nivio/appuser/AppUser.java @@ -131,9 +131,7 @@ public Long getId() { return id; } - public String getName() { - return name; - } + public String getName() { return name; } public String getAlias() { return alias; diff --git a/src/main/java/de/bonndan/nivio/security/CustomOAuth2User.java b/src/main/java/de/bonndan/nivio/security/CustomOAuth2User.java index c2a9f5c47..03920d293 100644 --- a/src/main/java/de/bonndan/nivio/security/CustomOAuth2User.java +++ b/src/main/java/de/bonndan/nivio/security/CustomOAuth2User.java @@ -14,10 +14,10 @@ */ public class CustomOAuth2User implements OAuth2User { - private final String alias; private final String name; private final String avatarUrl; @NonNull + private final String alias; private final String externalId; private final String idp; private final Map attributes; @@ -32,10 +32,10 @@ public CustomOAuth2User(@NonNull final String externalId, @NonNull final String idp ) { this.externalId = Objects.requireNonNull(externalId, "id must not be null"); - this.name = Objects.requireNonNull(name, "name must not be null"); this.alias = Objects.requireNonNull(alias, "alias must not be null"); this.attributes = Objects.requireNonNull(attributes, "attributes must not be null"); this.authorities = Objects.requireNonNull(authorities, "authorities must not be null"); + this.name = name; this.avatarUrl = avatarUrl; this.idp = idp; } diff --git a/src/main/java/de/bonndan/nivio/security/CustomOAuth2UserService.java b/src/main/java/de/bonndan/nivio/security/CustomOAuth2UserService.java index 3beb1703f..52fae5499 100644 --- a/src/main/java/de/bonndan/nivio/security/CustomOAuth2UserService.java +++ b/src/main/java/de/bonndan/nivio/security/CustomOAuth2UserService.java @@ -65,7 +65,7 @@ public static CustomOAuth2User fromGitHubUser(@NonNull final OAuth2User user, if (StringUtils.hasLength(nameAttribute)) { Object val = user.getAttribute(nameAttribute); if (val == null) { - Object login = Objects.requireNonNull(user.getAttribute("login")); + Object login = Objects.requireNonNull(user.getAttribute(aliasAttribute)); name = String.valueOf(login); } else { name = String.valueOf(val); diff --git a/src/main/java/de/bonndan/nivio/security/SecurityConfig.java b/src/main/java/de/bonndan/nivio/security/SecurityConfig.java index 51a71d1e8..361d22d18 100644 --- a/src/main/java/de/bonndan/nivio/security/SecurityConfig.java +++ b/src/main/java/de/bonndan/nivio/security/SecurityConfig.java @@ -1,6 +1,5 @@ package de.bonndan.nivio.security; -import de.bonndan.nivio.appuser.AppUserService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Configuration; @@ -24,8 +23,6 @@ @EnableWebSecurity public class SecurityConfig extends WebSecurityConfigurerAdapter { - private final AppUserService appUserService; - public static final String LOGIN_MODE_REQUIRED = "required"; public static final String LOGIN_MODE_OPTIONAL = "optional"; public static final String LOGIN_MODE_NONE = "none"; @@ -38,8 +35,7 @@ public class SecurityConfig extends WebSecurityConfigurerAdapter { private final AuthConfigProperties properties; - public SecurityConfig(AppUserService appUserService, AuthConfigProperties properties) { - this.appUserService = appUserService; + public SecurityConfig(AuthConfigProperties properties) { this.properties = properties; } @@ -96,7 +92,6 @@ private void configureForRequired(HttpSecurity http) throws Exception { .authorizeRequests() .antMatchers(LOGIN_PATH + "/**", "/icons/**", "/css/**").permitAll() .anyRequest().authenticated() - .antMatchers("/registration/**").permitAll() .and() .oauth2Login() //.clientRegistrationRepository(clientRegistrationRepository()) @@ -155,6 +150,4 @@ public void writeHeaders(HttpServletRequest request, HttpServletResponse respons } } - - } \ No newline at end of file diff --git a/src/test/java/de/bonndan/nivio/appuser/AppUserTest.java b/src/test/java/de/bonndan/nivio/appuser/AppUserTest.java index 823f24547..9c958250e 100644 --- a/src/test/java/de/bonndan/nivio/appuser/AppUserTest.java +++ b/src/test/java/de/bonndan/nivio/appuser/AppUserTest.java @@ -1,6 +1,5 @@ package de.bonndan.nivio.appuser; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -143,4 +142,25 @@ void setIdp() { appUser.setIdp(idp); assertEquals(idp, appUser.getIdp()); } + + @Test + void getPassword() { assertNull(appUser.getPassword()); } + + @Test + void getUsername() { assertNull(appUser.getUsername()); } + + @Test + void isAccountNonExpired() { assertTrue(appUser.isAccountNonExpired()); } + + @Test + void isAccountNonLocked() { + Boolean locked = false; + appUser.setLocked(locked); + assertEquals(!locked, appUser.isAccountNonLocked()); } + + @Test + void isCredentialsNonExpired() { assertTrue(appUser.isCredentialsNonExpired()); } + + @Test + void testIsEnabled() { assertTrue(appUser.isEnabled()); } } \ No newline at end of file diff --git a/src/test/java/de/bonndan/nivio/security/CustomOAuth2UserServiceTest.java b/src/test/java/de/bonndan/nivio/security/CustomOAuth2UserServiceTest.java index 74020164b..3e9da50ef 100644 --- a/src/test/java/de/bonndan/nivio/security/CustomOAuth2UserServiceTest.java +++ b/src/test/java/de/bonndan/nivio/security/CustomOAuth2UserServiceTest.java @@ -46,7 +46,6 @@ public void setup() { authorities = List.of(grantedAuthority); doReturn(authorities).when(oAuth2User).getAuthorities(); - customOAuth2User = CustomOAuth2UserService.fromGitHubUser(oAuth2User, "login", "name"); } @Test @@ -54,6 +53,7 @@ void fromGitHubUser() { //given when(oAuth2User.getAttribute("name")).thenReturn(name); + customOAuth2User = CustomOAuth2UserService.fromGitHubUser(oAuth2User, "login", "name"); //then assertThat(customOAuth2User).isNotNull(); @@ -67,6 +67,7 @@ void fromGitHubUserWithMissingNameFallsBackToLogin() { //given when(oAuth2User.getAttribute("name")).thenReturn(null); + customOAuth2User = CustomOAuth2UserService.fromGitHubUser(oAuth2User, "login", "name"); //then assertThat(customOAuth2User.getName()).isEqualTo(login); @@ -76,6 +77,7 @@ void fromGitHubUserWithMissingNameFallsBackToLogin() { void saveUser() { // given + customOAuth2User = CustomOAuth2UserService.fromGitHubUser(oAuth2User, "login", "name"); AppUser appUser = new AppUser(); appUser.setName(customOAuth2User.getName()); appUser.setAlias(customOAuth2User.getAlias()); @@ -97,4 +99,5 @@ void saveUser() { assertThat(appUser.getIdp()).isEqualTo(idp); } + } \ No newline at end of file diff --git a/src/test/java/de/bonndan/nivio/security/LoginControllerCaseRequiredTest.java b/src/test/java/de/bonndan/nivio/security/LoginControllerCaseRequiredTest.java index 635bb03fb..8938dcda6 100644 --- a/src/test/java/de/bonndan/nivio/security/LoginControllerCaseRequiredTest.java +++ b/src/test/java/de/bonndan/nivio/security/LoginControllerCaseRequiredTest.java @@ -88,4 +88,4 @@ void securedSockets() { .get(1, SECONDS); }).isInstanceOf(ExecutionException.class).hasCauseExactlyInstanceOf(DeploymentException.class); } -} +} \ No newline at end of file