diff --git a/src/main/java/net/geant/nmaas/portal/api/auth/OIDCAuthController.java b/src/main/java/net/geant/nmaas/portal/api/auth/OIDCAuthController.java index 16512e8291e034e9b39aeef5aed26967c123bef8..df0071568b9ec3cac5f4b67f0b067124021d5ea1 100644 --- a/src/main/java/net/geant/nmaas/portal/api/auth/OIDCAuthController.java +++ b/src/main/java/net/geant/nmaas/portal/api/auth/OIDCAuthController.java @@ -41,29 +41,20 @@ import static java.lang.String.format; public class OIDCAuthController { private final OidcUserService oidcUserService; - private final JWTTokenService jwtTokenService; - private final UserLoginRegisterService loginRegisterService; - private final UserService userService; - private final PasswordEncoder passwordEncoder; - private final DomainService domains; - private final ConfigurationManager configurationManager; - @Value("${portal.address}") private String portalAddress; @Value("${spring.security.oauth2.client.provider.my-oidc.issuer-uri:http://localhost:8080/realms/geant}") private String oidcAddress; - @PostMapping("api/oidc/link") public UserOidcToken oidcLinkedSuccess(@RequestBody final OidcLogin oidcLogin, HttpServletRequest request) { - User user = userService.findByEmail(oidcLogin.email()); try { validate( @@ -81,14 +72,12 @@ public class OIDCAuthController { throw new AuthenticationException(ae.getMessage()); } checkUserApprovals(user); - if ( - configurationManager.getConfiguration().isMaintenance() - && user.getRoles().stream().noneMatch( - value -> value.getRole().equals(Role.ROLE_SYSTEM_ADMIN) - ) - ) { + + if (configurationManager.getConfiguration().isMaintenance() + && user.getRoles().stream().noneMatch(value -> value.getRole().equals(Role.ROLE_SYSTEM_ADMIN))) { throw new UndergoingMaintenanceException("Application is undergoing maintenance right now"); } + this.loginRegisterService.registerNewSuccessfulLogin( user, request.getHeader(HttpHeaders.HOST), @@ -108,21 +97,17 @@ public class OIDCAuthController { jwtTokenService.getRefreshToken(linkedUser), oidcLogin.oidcToken() ); - - } @GetMapping("/api/oidc/success") public RedirectView oidcLoginSuccess(@AuthenticationPrincipal OidcUser oidcUser, HttpServletRequest request) { - - if (oidcUserService.externalUserRequiredLinking(oidcUser)) { + if (oidcUserService.externalUserRequiresLinking(oidcUser)) { String linkingRedirectUrl = portalAddress + "/login-linking?oidc_token=" + oidcUser.getIdToken().getTokenValue(); return new RedirectView(linkingRedirectUrl); } - try { User user = oidcUserService.checkUser(oidcUser); String redirectUrl = portalAddress @@ -152,13 +137,10 @@ public class OIDCAuthController { @GetMapping("/api/oidc/logout/{oidcToken}") public RedirectView logout(@PathVariable String oidcToken) { - String logoutUrl = oidcAddress + "/protocol/openid-connect/logout"; return new RedirectView(logoutUrl + "?id_token_hint=" + oidcToken); - } - void validate(String email, String providedPassword, String actualPassword, boolean isEnabled) { validateConditionAndLogMessage(email == null || providedPassword == null, format("Login failed: missing credentials%s", email != null ? (format(" (email: %s)", email)) : "")); @@ -168,7 +150,7 @@ public class OIDCAuthController { void checkUserApprovals(User user) { if (!user.isTermsOfUseAccepted() || !user.isPrivacyPolicyAccepted()) { - log.info(format("Check during login: Terms of Use or Privacy Policy were not accepted by user [%s]", user.getUsername())); + log.info("Check during login: Terms of Use or Privacy Policy were not accepted by user [{}]", user.getUsername()); user.setNewRoles(ImmutableSet.of(new UserRole(user, domains.getGlobalDomain().orElseThrow(SignupException::new), Role.ROLE_NOT_ACCEPTED))); } } diff --git a/src/main/java/net/geant/nmaas/portal/api/market/DomainController.java b/src/main/java/net/geant/nmaas/portal/api/market/DomainController.java index d86dece0d08ff1d61de559bc240568a1dfe825b5..019da0b7e1bb59fb6777f465551670b000d3d3e1 100644 --- a/src/main/java/net/geant/nmaas/portal/api/market/DomainController.java +++ b/src/main/java/net/geant/nmaas/portal/api/market/DomainController.java @@ -19,6 +19,7 @@ import net.geant.nmaas.portal.api.domain.KeyValueView; import net.geant.nmaas.portal.api.domain.UserViewMinimal; import net.geant.nmaas.portal.api.exception.MissingElementException; import net.geant.nmaas.portal.api.exception.ProcessingException; +import net.geant.nmaas.portal.exceptions.DataConflictException; import net.geant.nmaas.portal.exceptions.ObjectNotFoundException; import net.geant.nmaas.portal.persistent.entity.ApplicationStatePerDomain; import net.geant.nmaas.portal.persistent.entity.Domain; @@ -36,9 +37,11 @@ import org.apache.commons.lang3.StringUtils; import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.http.HttpStatus; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -47,6 +50,7 @@ import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; import java.nio.file.AccessDeniedException; @@ -142,7 +146,11 @@ public class DomainController extends AppBaseController { } return new Id(domain.getId()); - } catch (InvalidDomainException e) { + + } catch (IllegalArgumentException e) { + throw new DataConflictException(e.getMessage()); + } + catch (InvalidDomainException e) { throw new ProcessingException(e.getMessage()); } } @@ -365,4 +373,9 @@ public class DomainController extends AppBaseController { this.domainService.updateAnnotation(id, annotation); } + @ExceptionHandler(DataConflictException.class) + @ResponseStatus(code = HttpStatus.CONFLICT) + public String handleDataConfigException(DataConflictException e){ + return e.getMessage(); + } } \ No newline at end of file diff --git a/src/main/java/net/geant/nmaas/portal/api/security/JWTTokenService.java b/src/main/java/net/geant/nmaas/portal/api/security/JWTTokenService.java index 4397f113c9799cdb5bbd6350931d63c471adab9e..c63d712c4ca30753f48562f4a11deac2f9a44aa9 100644 --- a/src/main/java/net/geant/nmaas/portal/api/security/JWTTokenService.java +++ b/src/main/java/net/geant/nmaas/portal/api/security/JWTTokenService.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.Date; import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; @Service("jwtTokenService") @NoArgsConstructor @@ -82,7 +83,7 @@ public class JWTTokenService { role -> role.getRole().toString() ) - .toArray(String[]::new) + .collect(Collectors.toSet()) ) .claim(LANGUAGE, user.getSelectedLanguage()) .signWith(getSignInKey(jwtSettings.getSigningKey()), SignatureAlgorithm.HS512) diff --git a/src/main/java/net/geant/nmaas/portal/service/OidcUserService.java b/src/main/java/net/geant/nmaas/portal/service/OidcUserService.java index 7f94bf9f9a35d78a6769c0369ed4ec0638760887..c5829ede45d4e1f2500fababa25407df8d431801 100644 --- a/src/main/java/net/geant/nmaas/portal/service/OidcUserService.java +++ b/src/main/java/net/geant/nmaas/portal/service/OidcUserService.java @@ -7,9 +7,13 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser; public interface OidcUserService { User checkUser(OidcUser oidcUser); + User register(OidcUser user, Domain globalDomain); + User registerNewUser(OidcUser oidcUser); - boolean externalUserRequiredLinking(OidcUser oidcUser); + + boolean externalUserRequiresLinking(OidcUser oidcUser); + User linkUser(String email, String samlToken, String firstName, String lastName); } diff --git a/src/main/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImpl.java b/src/main/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImpl.java index a9eebe460474560b07b08bf8aa7eef72b60f5250..85b513fec339c7f61e545664ecde9b8190d91515 100644 --- a/src/main/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImpl.java +++ b/src/main/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImpl.java @@ -4,8 +4,6 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import net.geant.nmaas.portal.api.exception.MissingElementException; import net.geant.nmaas.portal.api.exception.SignupException; -import net.geant.nmaas.portal.api.exception.ExternalUserCanNotBeLinked; -import net.geant.nmaas.portal.api.exception.ExternalUserMatchException; import net.geant.nmaas.portal.exceptions.ObjectAlreadyExistsException; import net.geant.nmaas.portal.persistent.entity.Domain; import net.geant.nmaas.portal.persistent.entity.Role; @@ -21,7 +19,6 @@ import org.springframework.stereotype.Service; import java.security.SecureRandom; import java.util.Base64; -import java.util.Map; @Service @RequiredArgsConstructor @@ -29,17 +26,13 @@ import java.util.Map; public class OidcUserServiceImpl implements OidcUserService { private final UserService userService; - private final DomainService domains; - private final UserRepository userRepository; - private final ConfigurationManager configurationManager; @Value("${oidc.allowedLinkingUsersByEmail:false}") private boolean allowedLinkingUsersByEmail; - @Override public User checkUser(OidcUser oidcUser) { @@ -49,39 +42,29 @@ public class OidcUserServiceImpl implements OidcUserService { boolean existUserBySamlToken = userService .existsBySamlToken(oidcUserSub); - boolean existUserByUsernameAsSamlToken = userService - .existsBySamlToken(oidcUserPreferredUsername); - boolean existUserByEmail = userService - .existsByEmail(oidcUserEmail); if (existUserBySamlToken) { return userService .findBySamlToken(oidcUserSub) .orElseThrow(); - } else if (existUserByUsernameAsSamlToken) { - User user = userService - .findBySamlToken(oidcUserPreferredUsername) - .orElseThrow(); - if (user.getEmail().equals(oidcUserEmail)) { + } + if (userService.existsByEmail(oidcUserEmail)) { + User user = userService.findByEmail(oidcUserEmail); + if (user.getSamlToken().equals(oidcUserEmail) + || user.getSamlToken().equals(oidcUserPreferredUsername)) { user.setSamlToken(oidcUserSub); userService.update(user); return user; - } else { - throw new ExternalUserMatchException("External user " - + oidcUserPreferredUsername - + " does not match internal user "); } - } else { - return registerNewUser(oidcUser); } + return registerNewUser(oidcUser); + } @Override public User registerNewUser(OidcUser oidcUser) { try { - return register(oidcUser, - domains.getGlobalDomain().orElseThrow(MissingElementException::new) - ); + return register(oidcUser, domains.getGlobalDomain().orElseThrow(MissingElementException::new)); } catch (ObjectAlreadyExistsException e) { throw new SignupException("User already exists"); } catch (MissingElementException e) { @@ -92,7 +75,6 @@ public class OidcUserServiceImpl implements OidcUserService { @Override public User register(OidcUser oidcUser, Domain globalDomain) { - Map<String, Object> attributes = oidcUser.getAttributes(); byte[] array = new byte[16]; new SecureRandom().nextBytes(array); String generatedString = Base64.getEncoder().encodeToString(array); @@ -114,22 +96,16 @@ public class OidcUserServiceImpl implements OidcUserService { } @Override - public boolean externalUserRequiredLinking(OidcUser oidcUser) { + public boolean externalUserRequiresLinking(OidcUser oidcUser) { - String oidcUserSub = oidcUser.getAttribute("sub"); String oidcUserEmail = oidcUser.getAttribute("email"); - String oidcUserPreferredUsername = oidcUser.getAttribute("preferred_username"); - boolean existUserBySamlToken = userService - .existsBySamlToken(oidcUserSub); - boolean existUserByUsernameAsSamlToken = userService - .existsBySamlToken(oidcUserPreferredUsername); - boolean existUserByEmail = userService - .existsByEmail(oidcUserEmail); - - if(existUserBySamlToken || existUserByUsernameAsSamlToken) { - return false; - }else return existUserByEmail; + if (userService.existsByEmail(oidcUserEmail)) { + final User user = userService.findByEmail(oidcUserEmail); + return user.getSamlToken() == null || user.getSamlToken().isEmpty(); + } + + return false; } @Override @@ -144,5 +120,4 @@ public class OidcUserServiceImpl implements OidcUserService { return user; } - } diff --git a/src/main/resources/changelog.json b/src/main/resources/changelog.json index 8c1c9229d5bc342893c7e579ae91803ec0e156f8..ed50a597c6e79c34dcec4db0986d4ae24dfee13b 100644 --- a/src/main/resources/changelog.json +++ b/src/main/resources/changelog.json @@ -1,12 +1,23 @@ { "versions" : [ + { + "verNo" : "1.7.1", + "date" : "(2025/04/10)", + "topic" : [ + { + "title" : "Authentication and user access improvements", + "tags" : "[Enhancement]", + "description" : "JWT size reduction and account linking mechanism" + } + ] + }, { "verNo" : "1.7.0", "date" : "(2025/04/02)", "topic" : [ { "title" : "Integration with OIDC-compliant IdP", - "tags" : "[New feature]", + "tags" : "[New Feature]", "description" : "Moved away from the custom SAML-based IdP integration in favor of adding OIDC support" }, { diff --git a/src/test/java/net/geant/nmaas/portal/api/auth/OIDCAuthControllerTest.java b/src/test/java/net/geant/nmaas/portal/api/auth/OIDCAuthControllerTest.java index 90b0cdce3b969d71985ae2f40f9266854f644f11..8398cba5c28bceebdd5bfffae33a706e27ea94fb 100644 --- a/src/test/java/net/geant/nmaas/portal/api/auth/OIDCAuthControllerTest.java +++ b/src/test/java/net/geant/nmaas/portal/api/auth/OIDCAuthControllerTest.java @@ -114,7 +114,7 @@ class OIDCAuthControllerTest { when(idToken.getTokenValue()).thenReturn("oidc-token"); when(oidcUser.getIdToken()).thenReturn(idToken); - when(oidcUserService.externalUserRequiredLinking(any())).thenReturn(false); + when(oidcUserService.externalUserRequiresLinking(any())).thenReturn(false); Constructor<User> userConstructor = User.class.getDeclaredConstructor(); userConstructor.setAccessible(true); @@ -147,7 +147,7 @@ class OIDCAuthControllerTest { when(idToken.getTokenValue()).thenReturn("oidc-token"); when(oidcUser.getIdToken()).thenReturn(idToken); - when(oidcUserService.externalUserRequiredLinking(any())).thenReturn(true); + when(oidcUserService.externalUserRequiresLinking(any())).thenReturn(true); // when RedirectView result = oidcAuthController.oidcLoginSuccess(oidcUser, request); diff --git a/src/test/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImplTest.java b/src/test/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImplTest.java index bfc13ee3cefd9cee2a5e06dda70384ab1d1e3246..44860c31d125ed4f211c28dc578e5e9e0727c0ee 100644 --- a/src/test/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImplTest.java +++ b/src/test/java/net/geant/nmaas/portal/service/impl/OidcUserServiceImplTest.java @@ -66,27 +66,15 @@ class OidcUserServiceImplTest { //given User existingUser = new User("testuser"); existingUser.setEmail("test@example.com"); + existingUser.setSamlToken("test@example.com"); //when - when(userService.existsBySamlToken("test-sub")).thenReturn(false); - when(userService.existsBySamlToken("testuser")).thenReturn(true); - when(userService.findBySamlToken("testuser")).thenReturn(Optional.of(existingUser)); + when(userService.existsByEmail("test@example.com")).thenReturn(true); + when(userService.findByEmail("test@example.com")).thenReturn(existingUser); User result = oidcUserService.checkUser(oidcUser); //then assertEquals(existingUser, result); } - @Test - void shouldThrowExceptionWhenPreferredUsernameDoesNotMatchEmail() { - //given - User existingUser = new User("testuser"); - existingUser.setEmail("diffrent@example.com"); - //when - when(userService.existsBySamlToken("test-sub")).thenReturn(false); - when(userService.existsBySamlToken("testuser")).thenReturn(true); - when(userService.findBySamlToken("testuser")).thenReturn(Optional.of(existingUser)); - //then - assertThrows(ExternalUserMatchException.class, () -> oidcUserService.checkUser(oidcUser)); - } } \ No newline at end of file