From 82a6d96b1aea890bdcf4767790ca7b980bae4e01 Mon Sep 17 00:00:00 2001 From: Arpit Mohan Date: Mon, 16 Dec 2019 10:53:17 +0530 Subject: [PATCH] Upgrading to Spring boot 2.2.2 for features in Spring security Now, we have an authenticationSuccessHandler & authenticationFailureHandler for OAuth & Form sign ups. This makes the whole flow much easier to handle. --- app/server/appsmith-server/pom.xml | 7 +- .../configurations/ClientUserRepository.java | 6 +- .../server/configurations/RedisConfig.java | 5 - .../server/configurations/SecurityConfig.java | 15 ++- .../com/appsmith/server/domains/User.java | 2 +- ...java => AuthenticationFailureHandler.java} | 2 +- .../filters/AuthenticationSuccessHandler.java | 91 +++++++++++++++++++ .../FormAuthenticationSuccessHandler.java | 49 ---------- .../server/services/ActionServiceImpl.java | 2 +- .../server/services/UserServiceImpl.java | 7 +- app/server/pom.xml | 2 +- 11 files changed, 113 insertions(+), 75 deletions(-) rename app/server/appsmith-server/src/main/java/com/appsmith/server/filters/{FormAuthenticationFailureHandler.java => AuthenticationFailureHandler.java} (94%) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationSuccessHandler.java delete mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationSuccessHandler.java diff --git a/app/server/appsmith-server/pom.xml b/app/server/appsmith-server/pom.xml index 7ac1016746..1accde420d 100644 --- a/app/server/appsmith-server/pom.xml +++ b/app/server/appsmith-server/pom.xml @@ -47,21 +47,22 @@ org.springframework.boot spring-boot-starter-security + 2.2.2.RELEASE org.springframework.security spring-security-oauth2-client - 5.1.4.RELEASE + 5.2.1.RELEASE org.springframework.security spring-security-oauth2-jose - 5.1.4.RELEASE + 5.2.1.RELEASE org.springframework.security spring-security-config - 5.1.4.RELEASE + 5.2.1.RELEASE org.springframework.boot diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java index a542a39432..b50d33a3ab 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java @@ -57,13 +57,10 @@ public class ClientUserRepository implements ServerOAuth2AuthorizedClientReposit UserService userService; - OrganizationService organizationService; - CommonConfig commonConfig; - public ClientUserRepository(UserService userService, OrganizationService organizationService, CommonConfig commonConfig) { + public ClientUserRepository(UserService userService, CommonConfig commonConfig) { this.userService = userService; - this.organizationService = organizationService; this.commonConfig = commonConfig; } @@ -108,7 +105,6 @@ public class ClientUserRepository implements ServerOAuth2AuthorizedClientReposit * 1. Clustered environment * 2. Redis saved sessions */ - .then(checkAndCreateUser((OidcUser) principal.getPrincipal())) .then(Mono.empty()); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java index 6c129b42ec..3bb633fb4b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java @@ -17,11 +17,6 @@ import org.springframework.session.data.redis.config.annotation.web.server.Enabl @EnableRedisWebSession public class RedisConfig { - @Bean - public ReactiveRedisTemplate reactiveRedisTemplate(ReactiveRedisConnectionFactory factory) { - return new ReactiveRedisTemplate<>(factory, RedisSerializationContext.string()); - } - /** * This is the topic to which we will publish & subscribe to. We can have multiple topics based on the messages * that we wish to broadcast. Starting with a single one for now. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index 7a3db640ca..ee08bba1c9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -29,17 +29,14 @@ public class SecurityConfig { @Autowired private UserService userService; - @Autowired - private OrganizationService organizationService; - @Autowired private CommonConfig commonConfig; @Autowired - private ServerAuthenticationSuccessHandler formAuthenticationSuccessHandler; + private ServerAuthenticationSuccessHandler authenticationSuccessHandler; @Autowired - private ServerAuthenticationFailureHandler formAuthenticationFailureHandler; + private ServerAuthenticationFailureHandler authenticationFailureHandler; /** * This routerFunction is required to map /public/** endpoints to the src/main/resources/public folder @@ -92,12 +89,14 @@ public class SecurityConfig { .authenticated() .and().httpBasic() .and().oauth2Login() - .authorizedClientRepository(new ClientUserRepository(userService, organizationService, commonConfig)) + .authenticationSuccessHandler(authenticationSuccessHandler) + .authenticationFailureHandler(authenticationFailureHandler) + .authorizedClientRepository(new ClientUserRepository(userService, commonConfig)) .and().formLogin() .authenticationEntryPoint(new RedirectServerAuthenticationEntryPoint("/login")) .requiresAuthenticationMatcher(ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, "/login")) - .authenticationSuccessHandler(formAuthenticationSuccessHandler) - .authenticationFailureHandler(formAuthenticationFailureHandler) + .authenticationSuccessHandler(authenticationSuccessHandler) + .authenticationFailureHandler(authenticationFailureHandler) .and().build(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java index 2366517038..4d2954bdbb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java @@ -37,7 +37,7 @@ public class User extends BaseDomain implements UserDetails { @JsonIgnore private Boolean passwordResetInitiated = false; - private LoginSource source; + private LoginSource source = LoginSource.FORM; private UserState state; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationFailureHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationFailureHandler.java similarity index 94% rename from app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationFailureHandler.java rename to app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationFailureHandler.java index 40357338c1..10de76eea6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationFailureHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationFailureHandler.java @@ -16,7 +16,7 @@ import java.net.URI; @Slf4j @Component @RequiredArgsConstructor -public class FormAuthenticationFailureHandler implements ServerAuthenticationFailureHandler { +public class AuthenticationFailureHandler implements ServerAuthenticationFailureHandler { private ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationSuccessHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationSuccessHandler.java new file mode 100644 index 0000000000..aac389131d --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AuthenticationSuccessHandler.java @@ -0,0 +1,91 @@ +package com.appsmith.server.filters; + +import com.appsmith.server.constants.AclConstants; +import com.appsmith.server.domains.LoginSource; +import com.appsmith.server.domains.User; +import com.appsmith.server.domains.UserState; +import com.appsmith.server.services.UserService; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import org.springframework.security.web.server.DefaultServerRedirectStrategy; +import org.springframework.security.web.server.ServerRedirectStrategy; +import org.springframework.security.web.server.WebFilterExchange; +import org.springframework.security.web.server.authentication.ServerAuthenticationSuccessHandler; +import org.springframework.stereotype.Component; +import org.springframework.web.server.ServerWebExchange; +import reactor.core.publisher.Mono; + +import java.net.URI; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +@Slf4j +@Component +@RequiredArgsConstructor +public class AuthenticationSuccessHandler implements ServerAuthenticationSuccessHandler { + + @Autowired + UserService userService; + + private ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); + + /** + * On authentication success, we send a redirect to the endpoint that serve's the user's profile. + * The client browser will follow this redirect and fetch the user's profile JSON from the server. + * In the process, the client browser will also set the session ID in the cookie against the server's API domain. + * + * @param webFilterExchange + * @param authentication + * @return + */ + @Override + public Mono onAuthenticationSuccess(WebFilterExchange webFilterExchange, + Authentication authentication) { + log.debug("Login succeeded for user: {}", authentication.getPrincipal()); + if(authentication instanceof OAuth2AuthenticationToken) { + OAuth2AuthenticationToken oauthAuthentication = (OAuth2AuthenticationToken) authentication; + return checkAndCreateUser(oauthAuthentication) + .then(handleRedirect(webFilterExchange)); + } + + return handleRedirect(webFilterExchange); + } + + private Mono handleRedirect(WebFilterExchange webFilterExchange) { + ServerWebExchange exchange = webFilterExchange.getExchange(); + + // On authentication success, we send a redirect to the client's home page. This ensures that the session + // is set in the cookie on the browser. + String originHeader = exchange.getRequest().getHeaders().getOrigin(); + if(originHeader == null || originHeader.isEmpty()) { + originHeader = "/"; + } + + URI defaultRedirectLocation = URI.create(originHeader); + return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); + } + + private Mono checkAndCreateUser(OAuth2AuthenticationToken authToken) { + Map userAttributes = authToken.getPrincipal().getAttributes(); + User newUser = new User(); + newUser.setName((String) userAttributes.get("name")); + newUser.setEmail((String) userAttributes.get("email")); + newUser.setSource(LoginSource.GOOGLE); + newUser.setState(UserState.ACTIVATED); + newUser.setIsEnabled(true); + // TODO: Check if this is a valid permission available in the DB + // TODO: Check to see if this user was invited or is it a new sign up + Set permissions = new HashSet<>(); + // Adding the create organization permission because this is a new user and we will have to create an organization + // after this for the user. + permissions.addAll(AclConstants.PERMISSIONS_CRUD_ORG); + newUser.setPermissions(permissions); + + return userService.findByEmail((String) userAttributes.get("email")) + .switchIfEmpty(Mono.defer(() -> userService.create(newUser))); //In case the user doesn't exist, create and save the user. + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationSuccessHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationSuccessHandler.java deleted file mode 100644 index dd628ed1f7..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/FormAuthenticationSuccessHandler.java +++ /dev/null @@ -1,49 +0,0 @@ -package com.appsmith.server.filters; - -import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; -import org.springframework.security.core.Authentication; -import org.springframework.security.web.server.DefaultServerRedirectStrategy; -import org.springframework.security.web.server.ServerRedirectStrategy; -import org.springframework.security.web.server.WebFilterExchange; -import org.springframework.security.web.server.authentication.ServerAuthenticationSuccessHandler; -import org.springframework.stereotype.Component; -import org.springframework.web.server.ServerWebExchange; -import reactor.core.publisher.Mono; - -import java.net.URI; - -@Slf4j -@Component -@RequiredArgsConstructor -public class FormAuthenticationSuccessHandler implements ServerAuthenticationSuccessHandler { - - private ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); - - /** - * On authentication success, we send a redirect to the endpoint that serve's the user's profile. - * The client browser will follow this redirect and fetch the user's profile JSON from the server. - * In the process, the client browser will also set the session ID in the cookie against the server's API domain. - * - * @param webFilterExchange - * @param authentication - * @return - */ - @Override - public Mono onAuthenticationSuccess(WebFilterExchange webFilterExchange, - Authentication authentication) { - log.debug("Login succeeded for user: {}", authentication.getPrincipal()); - ServerWebExchange exchange = webFilterExchange.getExchange(); - - // On authentication success, we send a redirect to the client's home page. This ensures that the session - // is set in the cookie on the browser. - String originHeader = exchange.getRequest().getHeaders().getOrigin(); - if(originHeader == null || originHeader.isEmpty()) { - originHeader = "/"; - } - - URI defaultRedirectLocation = URI.create(originHeader); - return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); - } - -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index 04c706c1bf..b0bc953e9e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -441,7 +441,7 @@ public class ActionServiceImpl extends BaseService get(MultiValueMap params) { Action actionExample = new Action(); - Sort sort = new Sort(Direction.ASC, FieldName.NAME ); + Sort sort = Sort.by(FieldName.NAME ); if (params.getFirst(FieldName.NAME) != null) { actionExample.setName(params.getFirst(FieldName.NAME)); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 1c09d645c6..df3b0879b0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.services; import com.appsmith.server.domains.Group; +import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.PasswordResetToken; import com.appsmith.server.domains.User; @@ -253,7 +254,11 @@ public class UserServiceImpl extends BaseService i @Override public Mono create(User user) { - user.setPassword(this.passwordEncoder.encode(user.getPassword())); + + // Only encode the password if it's a form signup. For OAuth signups, we don't need password + if(LoginSource.FORM.equals(user.getSource())) { + user.setPassword(this.passwordEncoder.encode(user.getPassword())); + } Organization personalOrg = new Organization(); String name; diff --git a/app/server/pom.xml b/app/server/pom.xml index 6a959fcb7a..d0137c758a 100644 --- a/app/server/pom.xml +++ b/app/server/pom.xml @@ -4,7 +4,7 @@ org.springframework.boot spring-boot-starter-parent - 2.1.8.RELEASE + 2.2.2.RELEASE