diff --git a/.gitignore b/.gitignore index 5dafe25e..4c8a78fa 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ target node_modules build +logs **/generated-sources **/npm-debug.log .DS_store diff --git a/doc/setup.md b/doc/setup.md index 504cb5d3..30de586b 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -50,3 +50,19 @@ This repository is query parameter of Form service call specified in `sgovReposi SForms service is configured in `formGenServiceUrl`, the call to the service should contain SGoV model repository as query parameter. Example call: `formGenRepositoryUrl=`http://localhost:8080/s-pipes/service?_pId=transform&sgovRepositoryUrl=https%3A%2F%2Fgraphdb.onto.fel.cvut.cz%2Frepositories%2Fkodi-slovnik-gov-cz` + +### OpenID Connect Authentication + +RecordManager can work with an external authentication service implementing the OpenID Connect protocol. To use it, +set the `security.provider` (in `config.properties` or via `SECURITY_PROVIDER` via an environment variable) configuration to `oidc` +and configure the `spring.security.oauth2.resourceserver.jwt.issuer-uri` (in `application.properties` or using an environment variable) +parameter to the URI of the OAuth2 token issuer. When using Keycloak, this corresponds to the URI of the realm through +which Record Manager users authenticate their requests. For example, the value may be `http://localhost:8080/realms/record-manager`. +A client with confidential access and the corresponding valid redirect and origin URIs should be configured in the realm. + +If needed, claim used to access user's roles can be configured via `oidc.roleClaim`. The default value corresponds to the +default role mapping in Keycloak. Record Manager will assign `ROLE_USER` to authenticated users by default, any other roles +must be available in the token. + +Note also that it is expected that user metadata corresponding to the user extracted from the access token exist in the +repository. They are paired via the `prefferred_username` claim value (see `SecurityUtils`). diff --git a/pom.xml b/pom.xml index ee610c58..9bca4c26 100644 --- a/pom.xml +++ b/pom.xml @@ -41,6 +41,10 @@ org.springframework.boot spring-boot-starter-mail + + org.springframework.boot + spring-boot-starter-oauth2-resource-server + org.apache.httpcomponents.client5 diff --git a/src/main/java/cz/cvut/kbss/study/RecordManagerApplication.java b/src/main/java/cz/cvut/kbss/study/RecordManagerApplication.java index 30871772..df496f36 100644 --- a/src/main/java/cz/cvut/kbss/study/RecordManagerApplication.java +++ b/src/main/java/cz/cvut/kbss/study/RecordManagerApplication.java @@ -2,8 +2,10 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.PropertySource; @SpringBootApplication +@PropertySource("classpath:config.properties") public class RecordManagerApplication { public static void main(String[] args) { diff --git a/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java new file mode 100644 index 00000000..c791dfcb --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java @@ -0,0 +1,86 @@ +package cz.cvut.kbss.study.config; + +import cz.cvut.kbss.study.security.AuthenticationSuccess; +import cz.cvut.kbss.study.security.SecurityConstants; +import cz.cvut.kbss.study.service.ConfigReader; +import cz.cvut.kbss.study.util.oidc.OidcGrantedAuthoritiesExtractor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.converter.Converter; +import org.springframework.http.HttpStatus; +import org.springframework.security.authentication.AbstractAuthenticationToken; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.session.SessionRegistryImpl; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; +import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.HttpStatusEntryPoint; +import org.springframework.security.web.authentication.session.RegisterSessionAuthenticationStrategy; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; +import org.springframework.web.cors.CorsConfigurationSource; + +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; + +@ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "oidc") +@Configuration +@EnableWebSecurity +@EnableMethodSecurity +public class OAuth2SecurityConfig { + + private static final Logger LOG = LoggerFactory.getLogger(OAuth2SecurityConfig.class); + + private final AuthenticationSuccess authenticationSuccess; + + private final ConfigReader config; + + @Autowired + public OAuth2SecurityConfig(AuthenticationSuccess authenticationSuccess, ConfigReader config) { + this.authenticationSuccess = authenticationSuccess; + this.config = config; + } + + @Bean + protected SessionAuthenticationStrategy sessionAuthenticationStrategy() { + return new RegisterSessionAuthenticationStrategy(new SessionRegistryImpl()); + } + + @Bean + public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { + LOG.debug("Using OAuth2/OIDC security."); + http.oauth2ResourceServer( + (auth) -> auth.jwt((jwt) -> jwt.jwtAuthenticationConverter(grantedAuthoritiesExtractor()))) + .authorizeHttpRequests((auth) -> auth.anyRequest().permitAll()) + .exceptionHandling(ehc -> ehc.authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED))) + .cors((auth) -> auth.configurationSource(corsConfigurationSource())) + .csrf(AbstractHttpConfigurer::disable) + .logout((auth) -> auth.logoutUrl(SecurityConstants.LOGOUT_URI) + .logoutSuccessHandler(authenticationSuccess)); + return http.build(); + } + + private CorsConfigurationSource corsConfigurationSource() { + return SecurityConfig.createCorsConfiguration(config); + } + + private Converter grantedAuthoritiesExtractor() { + return source -> { + final Collection extractedRoles = + new OidcGrantedAuthoritiesExtractor(config).convert(source); + assert extractedRoles != null; + final Set authorities = new HashSet<>(extractedRoles); + // Add default role if it is not present + authorities.add(new SimpleGrantedAuthority(SecurityConstants.ROLE_USER)); + return new JwtAuthenticationToken(source, authorities); + }; + } +} diff --git a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java index 32e1dabf..30653a4d 100644 --- a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java @@ -4,6 +4,9 @@ import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.ConfigReader; import cz.cvut.kbss.study.util.ConfigParam; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpHeaders; @@ -28,11 +31,14 @@ import java.util.Collections; import java.util.List; +@ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "internal", matchIfMissing = true) @Configuration @EnableWebSecurity @EnableMethodSecurity public class SecurityConfig { + private static final Logger LOG = LoggerFactory.getLogger(SecurityConfig.class); + private static final String[] COOKIES_TO_DESTROY = { SecurityConstants.SESSION_COOKIE_NAME, SecurityConstants.REMEMBER_ME_COOKIE_NAME, @@ -59,6 +65,7 @@ public SecurityConfig(AuthenticationFailureHandler authenticationFailureHandler, @Bean public SecurityFilterChain filterChain(HttpSecurity http, ConfigReader config) throws Exception { + LOG.debug("Using internal security mechanisms."); final AuthenticationManager authManager = buildAuthenticationManager(http); http.authorizeHttpRequests((auth) -> auth.anyRequest().permitAll()) .cors((auth) -> auth.configurationSource(corsConfigurationSource(config))) @@ -83,11 +90,15 @@ private AuthenticationManager buildAuthenticationManager(HttpSecurity http) thro @Bean CorsConfigurationSource corsConfigurationSource(ConfigReader config) { + return createCorsConfiguration(config); + } + + static CorsConfigurationSource createCorsConfiguration(ConfigReader configReader) { // allowCredentials requires allowed origins to be configured (* is not supported) final CorsConfiguration corsConfiguration = new CorsConfiguration().applyPermitDefaultValues(); corsConfiguration.setAllowedMethods(Collections.singletonList("*")); - if (!config.getConfig(ConfigParam.APP_CONTEXT, "").isBlank()) { - String appUrl = config.getConfig(ConfigParam.APP_CONTEXT); + if (!configReader.getConfig(ConfigParam.APP_CONTEXT, "").isBlank()) { + String appUrl = configReader.getConfig(ConfigParam.APP_CONTEXT); appUrl = appUrl.substring(0, appUrl.lastIndexOf('/')); corsConfiguration.setAllowedOrigins(List.of(appUrl)); } else { diff --git a/src/main/java/cz/cvut/kbss/study/persistence/PersistenceFactory.java b/src/main/java/cz/cvut/kbss/study/persistence/PersistenceFactory.java index 9cc8f31d..6c70eec6 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/PersistenceFactory.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/PersistenceFactory.java @@ -9,7 +9,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; -import org.springframework.context.annotation.PropertySource; import org.springframework.core.env.Environment; import java.util.Collections; @@ -30,7 +29,6 @@ * Sets up persistence and provides {@link EntityManagerFactory} as Spring bean. */ @Configuration -@PropertySource("classpath:config.properties") public class PersistenceFactory { private static final String USERNAME_PROPERTY = "username"; diff --git a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java new file mode 100644 index 00000000..3504e5b8 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java @@ -0,0 +1,34 @@ +package cz.cvut.kbss.study.rest; + +import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.security.SecurityConstants; +import cz.cvut.kbss.study.service.UserService; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.http.MediaType; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * API for getting basic user info. + *

+ * Enabled when OIDC security is used. + */ +@ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "oidc") +@RestController +@RequestMapping("/users") +public class OidcUserController extends BaseController { + + private final UserService userService; + + public OidcUserController(UserService userService) { + this.userService = userService; + } + + @PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") + @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) + public User getCurrent() { + return userService.getCurrentUser(); + } +} diff --git a/src/main/java/cz/cvut/kbss/study/rest/UserController.java b/src/main/java/cz/cvut/kbss/study/rest/UserController.java index feabe4d0..42fac83e 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/UserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/UserController.java @@ -10,21 +10,33 @@ import cz.cvut.kbss.study.security.model.UserDetails; import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.UserService; -import org.springframework.beans.factory.annotation.Autowired; +import cz.cvut.kbss.study.service.security.SecurityUtils; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.context.SecurityContext; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +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.security.Principal; import java.util.List; import java.util.Map; +/** + * User management API. + * + * Enabled when internal security is used. + */ +@ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "internal", matchIfMissing = true) @RestController @RequestMapping("/users") public class UserController extends BaseController { @@ -51,9 +63,8 @@ public User getByUsername(@PathVariable("username") String username) { @PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) - public User getCurrent(Principal principal) { - final String username = principal.getName(); - return getByUsername(username); + public User getCurrent() { + return userService.getCurrentUser(); } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") @@ -73,8 +84,7 @@ public ResponseEntity create(@RequestBody User user) { "or hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { - final List users = institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); - return users; + return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); } private List getByInstitution(String institutionKey) { @@ -207,10 +217,8 @@ public void impersonate(@RequestBody String username) { if (user.getTypes().contains(Vocabulary.s_c_administrator)) { throw new BadRequestException("Cannot impersonate admin."); } - final SecurityContext context = SecurityContextHolder.getContext(); UserDetails ud = new UserDetails(user); - UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(ud, null, ud.getAuthorities()); - context.setAuthentication(auth); + SecurityUtils.setCurrentUser(ud); if (LOG.isTraceEnabled()) { LOG.trace("User {} impersonated.", user.getUsername()); } diff --git a/src/main/java/cz/cvut/kbss/study/security/AuthenticationFailure.java b/src/main/java/cz/cvut/kbss/study/security/AuthenticationFailure.java index 36c08401..63adb6b0 100644 --- a/src/main/java/cz/cvut/kbss/study/security/AuthenticationFailure.java +++ b/src/main/java/cz/cvut/kbss/study/security/AuthenticationFailure.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import cz.cvut.kbss.study.security.model.LoginStatus; -import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; diff --git a/src/main/java/cz/cvut/kbss/study/security/AuthenticationSuccess.java b/src/main/java/cz/cvut/kbss/study/security/AuthenticationSuccess.java index 7dfb3777..a7d700b2 100644 --- a/src/main/java/cz/cvut/kbss/study/security/AuthenticationSuccess.java +++ b/src/main/java/cz/cvut/kbss/study/security/AuthenticationSuccess.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import cz.cvut.kbss.study.exception.FormManagerException; import cz.cvut.kbss.study.security.model.LoginStatus; -import cz.cvut.kbss.study.security.model.UserDetails; import cz.cvut.kbss.study.service.ConfigReader; import cz.cvut.kbss.study.util.ConfigParam; import jakarta.servlet.http.HttpServletRequest; @@ -54,7 +53,7 @@ private String getUsername(Authentication authentication) { if (authentication == null) { return ""; } - return ((UserDetails) authentication.getPrincipal()).getUsername(); + return authentication.getName(); } @Override diff --git a/src/main/java/cz/cvut/kbss/study/security/OntologyAuthenticationProvider.java b/src/main/java/cz/cvut/kbss/study/security/OntologyAuthenticationProvider.java index b16f7d16..76f87b6b 100644 --- a/src/main/java/cz/cvut/kbss/study/security/OntologyAuthenticationProvider.java +++ b/src/main/java/cz/cvut/kbss/study/security/OntologyAuthenticationProvider.java @@ -1,7 +1,7 @@ package cz.cvut.kbss.study.security; -import cz.cvut.kbss.study.security.model.AuthenticationToken; import cz.cvut.kbss.study.security.model.UserDetails; +import cz.cvut.kbss.study.service.security.SecurityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.security.authentication.AuthenticationProvider; @@ -9,9 +9,6 @@ import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; -import org.springframework.security.core.context.SecurityContext; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.core.context.SecurityContextImpl; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; @@ -44,20 +41,11 @@ public Authentication authenticate(Authentication authentication) throws Authent LOG.trace("Provided password for username '" + username + "' doesn't match."); throw new BadCredentialsException("Provided password for username '" + username + "' doesn't match."); } - userDetails.eraseCredentials(); // Don't pass credentials around in the user details object - final AuthenticationToken token = new AuthenticationToken(userDetails.getAuthorities(), userDetails); - token.setAuthenticated(true); - token.setDetails(userDetails); - - final SecurityContext context = new SecurityContextImpl(); - context.setAuthentication(token); - SecurityContextHolder.setContext(context); - return token; + return SecurityUtils.setCurrentUser(userDetails); } @Override public boolean supports(Class aClass) { - return UsernamePasswordAuthenticationToken.class.isAssignableFrom(aClass) || - AuthenticationToken.class.isAssignableFrom(aClass); + return UsernamePasswordAuthenticationToken.class.isAssignableFrom(aClass); } } diff --git a/src/main/java/cz/cvut/kbss/study/security/model/AuthenticationToken.java b/src/main/java/cz/cvut/kbss/study/security/model/AuthenticationToken.java deleted file mode 100644 index e5c7b25f..00000000 --- a/src/main/java/cz/cvut/kbss/study/security/model/AuthenticationToken.java +++ /dev/null @@ -1,29 +0,0 @@ -package cz.cvut.kbss.study.security.model; - -import org.springframework.security.authentication.AbstractAuthenticationToken; -import org.springframework.security.core.GrantedAuthority; - -import java.security.Principal; -import java.util.Collection; - -public class AuthenticationToken extends AbstractAuthenticationToken implements Principal { - - private UserDetails userDetails; - - public AuthenticationToken(Collection authorities, UserDetails userDetails) { - super(authorities); - this.userDetails = userDetails; - super.setAuthenticated(true); - super.setDetails(userDetails); - } - - @Override - public Object getCredentials() { - return userDetails.getPassword(); - } - - @Override - public Object getPrincipal() { - return userDetails; - } -} diff --git a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java index f3ba899f..91a843fc 100644 --- a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java +++ b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java @@ -6,14 +6,19 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; -import java.util.*; -import java.util.stream.Collectors; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; public class UserDetails implements org.springframework.security.core.userdetails.UserDetails { private static final Map ROLE_MAPPING = initRoleMapping(); - private User user; + private final User user; private final Set authorities; @@ -42,8 +47,7 @@ public UserDetails(User user, Collection authorities) { private void resolveRoles() { authorities.addAll(ROLE_MAPPING.entrySet().stream().filter(e -> user.getTypes().contains(e.getKey())) - .map(e -> new SimpleGrantedAuthority(e.getValue())) - .collect(Collectors.toList())); + .map(e -> new SimpleGrantedAuthority(e.getValue())).toList()); authorities.add(new SimpleGrantedAuthority(SecurityConstants.ROLE_USER)); } diff --git a/src/main/java/cz/cvut/kbss/study/service/UserService.java b/src/main/java/cz/cvut/kbss/study/service/UserService.java index 19d4c75b..af744aad 100644 --- a/src/main/java/cz/cvut/kbss/study/service/UserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/UserService.java @@ -9,6 +9,8 @@ public interface UserService extends BaseService { User findByUsername(String username); + User getCurrentUser(); + User findByEmail(String email); User findByToken(String token); diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index 3de075b0..1ed305a0 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -71,6 +71,11 @@ public User findByUsername(String username) { return userDao.findByUsername(username); } + @Override + public User getCurrentUser() { + return securityUtils.getCurrentUser(); + } + @Transactional(readOnly = true) @Override public List findByInstitution(Institution institution) { diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index f9f3d77d..0322cb99 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -5,9 +5,13 @@ import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; import cz.cvut.kbss.study.persistence.dao.UserDao; import cz.cvut.kbss.study.security.model.UserDetails; -import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.authentication.AbstractAuthenticationToken; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; +import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.stereotype.Service; import java.util.List; @@ -25,31 +29,50 @@ public SecurityUtils(UserDao userDao, PatientRecordDao patientRecordDao) { } /** - * Gets the currently authenticated user. + * Sets the current security context to the user represented by the provided user details. + *

+ * Note that this method erases credentials from the provided user details for security reasons. * - * @return Current user + * This method should be used only when internal authentication is used. + * + * @param userDetails User details */ - public User getCurrentUser() { - final SecurityContext context = SecurityContextHolder.getContext(); - assert context != null; - final UserDetails userDetails = (UserDetails) context.getAuthentication().getPrincipal(); - return userDao.findByUsername(userDetails.getUser().getUsername()); + public static AbstractAuthenticationToken setCurrentUser(UserDetails userDetails) { + final UsernamePasswordAuthenticationToken + token = new UsernamePasswordAuthenticationToken(userDetails.getUsername(), userDetails.getPassword(), + userDetails.getAuthorities()); + token.setDetails(userDetails); + token.eraseCredentials(); // Do not pass credentials around + + final SecurityContext context = new SecurityContextImpl(); + context.setAuthentication(token); + SecurityContextHolder.setContext(context); + return token; } /** - * Gets details of the currently authenticated user. + * Gets the currently authenticated user. * - * @return Currently authenticated user details or null, if no one is currently authenticated + * @return Current user */ - public UserDetails getCurrentUserDetails() { + public User getCurrentUser() { final SecurityContext context = SecurityContextHolder.getContext(); - if (context.getAuthentication() != null && context.getAuthentication().getDetails() instanceof UserDetails) { - return (UserDetails) context.getAuthentication().getDetails(); + assert context != null; + final Object principal = context.getAuthentication().getPrincipal(); + if (principal instanceof Jwt) { + return resolveAccountFromOAuthPrincipal((Jwt) principal); } else { - return null; + assert principal instanceof String; + final String username = context.getAuthentication().getPrincipal().toString(); + return userDao.findByUsername(username); } } + private User resolveAccountFromOAuthPrincipal(Jwt principal) { + final OidcUserInfo userInfo = new OidcUserInfo(principal.getClaims()); + return userDao.findByUsername(userInfo.getPreferredUsername()); + } + /** * Checks whether the current user is a member of a institution with the specified key. * diff --git a/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java b/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java index 5b66bb86..4cc4c006 100644 --- a/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java +++ b/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java @@ -31,7 +31,9 @@ public enum ConfigParam { E_PASSWORD_CHANGE_CONTENT("email.passwordChangeContent"), E_PROFILE_UPDATE_SUBJECT("email.profileUpdateSubject"), - E_PROFILE_UPDATE_CONTENT("email.profileUpdateContent"); + E_PROFILE_UPDATE_CONTENT("email.profileUpdateContent"), + + OIDC_ROLE_CLAIM("oidc.roleClaim"); private final String name; diff --git a/src/main/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractor.java b/src/main/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractor.java new file mode 100644 index 00000000..4ea00937 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractor.java @@ -0,0 +1,45 @@ +package cz.cvut.kbss.study.util.oidc; + +import cz.cvut.kbss.study.service.ConfigReader; +import cz.cvut.kbss.study.util.ConfigParam; +import org.springframework.core.convert.converter.Converter; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.jwt.Jwt; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class OidcGrantedAuthoritiesExtractor implements Converter> { + + private final ConfigReader config; + + public OidcGrantedAuthoritiesExtractor(ConfigReader config) { + this.config = config; + } + + @Override + public Collection convert(Jwt source) { + final String rolesClaim = config.getConfig(ConfigParam.OIDC_ROLE_CLAIM); + final String[] parts = rolesClaim.split("\\."); + assert parts.length > 0; + final List roles; + if (parts.length == 1) { + roles = source.getClaimAsStringList(rolesClaim); + } else { + Map map = source.getClaimAsMap(parts[0]); + for (int i = 1; i < parts.length - 1; i++) { + if (map.containsKey(parts[i]) && !(map.get(parts[i]) instanceof Map)) { + throw new IllegalArgumentException("Access token does not contain roles under the expected claim '" + rolesClaim + "'."); + } + map = (Map) map.getOrDefault(parts[i], Collections.emptyMap()); + } + if (map.containsKey(parts[parts.length - 1]) && !(map.get(parts[parts.length - 1]) instanceof List)) { + throw new IllegalArgumentException("Roles claim does not contain a list."); + } + roles = (List) map.getOrDefault(parts[parts.length - 1], Collections.emptyList()); + } + return roles.stream().map(SimpleGrantedAuthority::new).toList(); + } +} diff --git a/src/main/resources/config.properties b/src/main/resources/config.properties index c5c10c45..6349a441 100644 --- a/src/main/resources/config.properties +++ b/src/main/resources/config.properties @@ -50,4 +50,12 @@ email.passwordChangeContent=

Dear user {{username}},

your password # Profile update email email.profileUpdateSubject=Profile updated by a study coordinator # PasswordReset email html content, variables: username, appContext -email.profileUpdateContent=

Dear user {{username}},

your profile at {{appContext}} has been updated by a study coordinator.

Best regards,
RecordManager

\ No newline at end of file +email.profileUpdateContent=

Dear user {{username}},

your profile at {{appContext}} has been updated by a study coordinator.

Best regards,
RecordManager

+ +# Provider of application security. Possible values are 'internal' for internally stored users and 'oidc' for using an +# OIDC-compatible authentication service. Its URL is configured via Spring Boot configuration parameters +security.provider=internal + +# Claim containing user roles in the OIDC access token (applies only when 'oidc' security provider is selected). Use +# dot notation for nested objects +oidc.roleClaim=realm_access.roles \ No newline at end of file diff --git a/src/test/java/cz/cvut/kbss/study/environment/util/Environment.java b/src/test/java/cz/cvut/kbss/study/environment/util/Environment.java index 5e167e93..d45af3da 100644 --- a/src/test/java/cz/cvut/kbss/study/environment/util/Environment.java +++ b/src/test/java/cz/cvut/kbss/study/environment/util/Environment.java @@ -3,12 +3,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import cz.cvut.kbss.study.config.WebAppConfig; import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.security.model.AuthenticationToken; import cz.cvut.kbss.study.security.model.UserDetails; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.ResourceHttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextImpl; @@ -34,7 +34,9 @@ public static void setCurrentUser(User user) { currentUser = user; final UserDetails userDetails = new UserDetails(user); SecurityContext context = new SecurityContextImpl(); - context.setAuthentication(new AuthenticationToken(userDetails.getAuthorities(), userDetails)); + context.setAuthentication( + new UsernamePasswordAuthenticationToken(userDetails.getUsername(), userDetails.getPassword(), + userDetails.getAuthorities())); SecurityContextHolder.setContext(context); } diff --git a/src/test/java/cz/cvut/kbss/study/security/OntologyAuthenticationProviderTest.java b/src/test/java/cz/cvut/kbss/study/security/OntologyAuthenticationProviderTest.java index 8e2d6612..c2ece819 100644 --- a/src/test/java/cz/cvut/kbss/study/security/OntologyAuthenticationProviderTest.java +++ b/src/test/java/cz/cvut/kbss/study/security/OntologyAuthenticationProviderTest.java @@ -1,7 +1,6 @@ package cz.cvut.kbss.study.security; import cz.cvut.kbss.study.environment.config.TestSecurityConfig; -import cz.cvut.kbss.study.security.model.UserDetails; import cz.cvut.kbss.study.service.BaseServiceTestRunner; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -49,10 +48,9 @@ public void successfulAuthenticationSetsSecurityContext() { final SecurityContext context = SecurityContextHolder.getContext(); assertNull(context.getAuthentication()); final Authentication result = provider.authenticate(auth); - assertNotNull(SecurityContextHolder.getContext()); - final UserDetails details = (UserDetails) SecurityContextHolder.getContext().getAuthentication().getDetails(); - assertEquals(BaseServiceTestRunner.USERNAME, details.getUsername()); assertTrue(result.isAuthenticated()); + assertNotNull(SecurityContextHolder.getContext()); + assertEquals(BaseServiceTestRunner.USERNAME, SecurityContextHolder.getContext().getAuthentication().getPrincipal()); } @Test diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java index 8d7ea4eb..bd88284b 100644 --- a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java @@ -5,49 +5,55 @@ import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.security.model.UserDetails; -import cz.cvut.kbss.study.service.BaseServiceTestRunner; -import cz.cvut.kbss.study.service.InstitutionService; -import cz.cvut.kbss.study.service.PatientRecordService; -import cz.cvut.kbss.study.service.UserService; +import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; +import cz.cvut.kbss.study.persistence.dao.UserDao; +import cz.cvut.kbss.study.security.SecurityConstants; +import cz.cvut.kbss.study.util.IdentificationUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; +@ExtendWith(MockitoExtension.class) +public class SecurityUtilsTest { -public class SecurityUtilsTest extends BaseServiceTestRunner { - - @Autowired - private SecurityUtils securityUtils; + @Mock + private UserDao userDao; - @Autowired - private UserService userService; + @Mock + private PatientRecordDao patientRecordDao; - @Autowired - private InstitutionService institutionService; - - @Autowired - private PatientRecordService patientRecordService; + @InjectMocks + private SecurityUtils sut; private User user; - public static final String USERNAME = "halsey"; - public static final String PASSWORD = "john117"; + + public static final String USERNAME = "username"; + public static final String PASSWORD = "pass" + Generator.randomInt(0, 1000); @BeforeEach public void setUp() { Institution institution = Generator.generateInstitution(); - institutionService.persist(institution); - this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institutionService.findByName(institution.getName())); + institution.setKey(IdentificationUtils.generateKey()); + this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institution); user.generateUri(); - userService.persist(user); } @AfterEach @@ -58,83 +64,93 @@ public void tearDown() { @Test public void getCurrentUserReturnsCurrentlyLoggedInUser() { Environment.setCurrentUser(user); - final User result = securityUtils.getCurrentUser(); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); + final User result = sut.getCurrentUser(); assertEquals(user, result); } @Test - public void getCurrentUserDetailsReturnsUserDetailsOfCurrentlyLoggedInUser() { - Environment.setCurrentUser(user); - final UserDetails result = securityUtils.getCurrentUserDetails(); - assertNotNull(result); - assertTrue(result.isEnabled()); - assertEquals(user, result.getUser()); - } - - @Test - public void getCurrentUserDetailsReturnsNullIfNoUserIsLoggedIn() { - assertNull(securityUtils.getCurrentUserDetails()); + void getCurrentUserRetrievesCurrentUserForOauthJwtAccessToken() { + final Jwt token = Jwt.withTokenValue("abcdef12345") + .header("alg", "RS256") + .header("typ", "JWT") + .claim("roles", List.of(SecurityConstants.ROLE_USER)) + .issuer("http://localhost:8080/termit") + .subject(USERNAME) + .claim("preferred_username", USERNAME) + .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) + .build(); + SecurityContext context = new SecurityContextImpl(); + context.setAuthentication(new JwtAuthenticationToken(token)); + SecurityContextHolder.setContext(context); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); + + final User result = sut.getCurrentUser(); + assertEquals(user, result); } @Test - public void isMemberOfInstitutionReturnsMembershipStatusTrue() { + public void isMemberOfInstitutionReturnsTrueForUserFromSpecifiedInstitution() { Environment.setCurrentUser(user); - assertTrue(securityUtils.isMemberOfInstitution(user.getInstitution().getKey())); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); + assertTrue(sut.isMemberOfInstitution(user.getInstitution().getKey())); } @Test - public void isMemberOfInstitutionReturnsMembershipStatusFalse() { + public void isMemberOfInstitutionReturnsFalseForDifferentInstitutionKey() { Environment.setCurrentUser(user); - assertFalse(securityUtils.isMemberOfInstitution("nonExistingInstitutionKey")); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); + assertFalse(sut.isMemberOfInstitution("nonExistingInstitutionKey")); } @Test - public void areFromSameInstitutionReturnsMembershipStatusTrue() { + public void areFromSameInstitutionReturnsTrueForUserFromSameInstitution() { Environment.setCurrentUser(user); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); User userFromSameInstitution = Generator.generateUser(user.getInstitution()); - userService.persist(userFromSameInstitution); + when(userDao.findByInstitution(user.getInstitution())).thenReturn(List.of(user, userFromSameInstitution)); - assertTrue(securityUtils.areFromSameInstitution(userFromSameInstitution.getUsername())); + assertTrue(sut.areFromSameInstitution(userFromSameInstitution.getUsername())); } @Test - public void areFromSameInstitutionReturnsMembershipStatusFalse() { + public void areFromSameInstitutionReturnsFalseForUserFromDifferentInstitution() { Environment.setCurrentUser(user); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); Institution institutionAnother = Generator.generateInstitution(); - institutionService.persist(institutionAnother); User userFromAnotherInstitution = Generator.generateUser(institutionAnother); - userService.persist(userFromAnotherInstitution); - assertFalse(securityUtils.areFromSameInstitution(userFromAnotherInstitution.getUsername())); + when(userDao.findByInstitution(user.getInstitution())).thenReturn(List.of(user)); + + assertFalse(sut.areFromSameInstitution(userFromAnotherInstitution.getUsername())); } @Test - public void isRecordInUsersInstitutionReturnsMembershipStatusTrueForUser() { + public void isRecordInUsersInstitutionReturnsTrueWhenRecordBelongsToCurrentUsersInstitution() { Environment.setCurrentUser(user); + when(userDao.findByUsername(user.getUsername())).thenReturn(user); PatientRecord record = Generator.generatePatientRecord(user); + record.setKey(IdentificationUtils.generateKey()); + when(patientRecordDao.findByKey(record.getKey())).thenReturn(record); - patientRecordService.persist(record); - assertTrue(securityUtils.isRecordInUsersInstitution(record.getKey())); + assertTrue(sut.isRecordInUsersInstitution(record.getKey())); } @Test - public void isRecordInUsersInstitutionReturnsMembershipStatusFalse() { + public void isRecordInUsersInstitutionReturnsFalseWhenRecordBelongsToInstitutionDifferentFromCurrentUsers() { Environment.setCurrentUser(user); - + when(userDao.findByUsername(user.getUsername())).thenReturn(user); Institution institutionAnother = Generator.generateInstitution(); - institutionService.persist(institutionAnother); - + institutionAnother.setKey(IdentificationUtils.generateKey()); User userFromAnotherInstitution = Generator.generateUser(institutionAnother); - userService.persist(userFromAnotherInstitution); PatientRecord record = Generator.generatePatientRecord(userFromAnotherInstitution); - patientRecordService.persist(record); - - Environment.setCurrentUser(userFromAnotherInstitution); + record.setKey(IdentificationUtils.generateKey()); + when(patientRecordDao.findByKey(record.getKey())).thenReturn(record); - assertFalse(securityUtils.isRecordInUsersInstitution(record.getKey())); + assertFalse(sut.isRecordInUsersInstitution(record.getKey())); } } diff --git a/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java new file mode 100644 index 00000000..881a3320 --- /dev/null +++ b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java @@ -0,0 +1,103 @@ +package cz.cvut.kbss.study.util.oidc; + +import cz.cvut.kbss.study.security.SecurityConstants; +import cz.cvut.kbss.study.service.ConfigReader; +import cz.cvut.kbss.study.util.ConfigParam; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.jwt.Jwt; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class OidcGrantedAuthoritiesExtractorTest { + + @Mock + private ConfigReader config; + + @Test + void convertMapsTopLevelClaimWithRolesToGrantedAuthorities() { + when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("roles"); + final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); + final Jwt token = Jwt.withTokenValue("abcdef12345") + .header("alg", "RS256") + .header("typ", "JWT") + .claim("roles", roles) + .issuer("http://localhost:8080/termit") + .subject("termit") + .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) + .build(); + + final OidcGrantedAuthoritiesExtractor sut = new OidcGrantedAuthoritiesExtractor(config); + final Collection result = sut.convert(token); + assertNotNull(result); + for (String r : roles) { + assertThat(result, hasItem(new SimpleGrantedAuthority(r))); + } + } + + @Test + void convertSupportsNestedRolesClaim() { + when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("realm_access.roles"); + final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); + final Jwt token = Jwt.withTokenValue("abcdef12345") + .header("alg", "RS256") + .header("typ", "JWT") + .claim("realm_access", Map.of("roles", roles)) + .issuer("http://localhost:8080/termit") + .subject("termit") + .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) + .build(); + + final OidcGrantedAuthoritiesExtractor sut = new OidcGrantedAuthoritiesExtractor(config); + final Collection result = sut.convert(token); + assertNotNull(result); + for (String r : roles) { + assertThat(result, hasItem(new SimpleGrantedAuthority(r))); + } + } + + @Test + void convertThrowsIllegalArgumentExceptionWhenExpectedClaimPathIsNotTraversable() { + when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("realm_access.roles.list"); + final Jwt token = Jwt.withTokenValue("abcdef12345") + .header("alg", "RS256") + .header("typ", "JWT") + .claim("realm_access", Map.of("roles", 1235)) + .issuer("http://localhost:8080/termit") + .subject("termit") + .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) + .build(); + + final OidcGrantedAuthoritiesExtractor sut = new OidcGrantedAuthoritiesExtractor(config); + assertThrows(IllegalArgumentException.class, () -> sut.convert(token)); + } + + @Test + void convertThrowsIllegalArgumentExceptionWhenNestedRolesClaimIsNotList() { + when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("realm_access.roles.notlist"); + final Jwt token = Jwt.withTokenValue("abcdef12345") + .header("alg", "RS256") + .header("typ", "JWT") + .claim("realm_access", Map.of("roles", Map.of("notlist", SecurityConstants.ROLE_USER))) + .issuer("http://localhost:8080/termit") + .subject("termit") + .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) + .build(); + + final OidcGrantedAuthoritiesExtractor sut = new OidcGrantedAuthoritiesExtractor(config); + assertThrows(IllegalArgumentException.class, () -> sut.convert(token)); + } +} \ No newline at end of file