diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java index ae5015248c..9096f2ce61 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java @@ -29,25 +29,20 @@ public class AuthenticationFailureHandler implements ServerAuthenticationFailure // On authentication failure, we send a redirect to the client's login error page. The browser will re-load the // login page again with an error message shown to the user. - String originHeader = exchange.getRequest().getHeaders().getOrigin(); - if (originHeader == null || originHeader.isEmpty()) { - // Check the referer header if the origin is not available - String refererHeader = exchange.getRequest().getHeaders().getFirst(Security.REFERER_HEADER); - if (refererHeader != null && !refererHeader.isBlank()) { - URI uri = null; - try { - uri = new URI(refererHeader); - String authority = uri.getAuthority(); - String scheme = uri.getScheme(); - originHeader = scheme + "://" + authority; - } catch (URISyntaxException e) { - originHeader = "/"; + String state = exchange.getRequest().getQueryParams().getFirst(Security.QUERY_PARAMETER_STATE); + String originHeader = ""; + if (state != null && !state.isEmpty()) { + String[] stateArray = state.split(","); + for (int i = 0; i < stateArray.length; i++) { + String stateVar = stateArray[i]; + if (stateVar != null && stateVar.startsWith(Security.STATE_PARAMETER_ORIGIN) && stateVar.contains("=")) { + // This is the origin of the request that we want to redirect to + originHeader = stateVar.split("=")[1]; } - } else { - originHeader = "/"; } } + URI defaultRedirectLocation = URI.create(originHeader + "/user/login?error=true"); return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java index 1066357eaa..d6d08895fc 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java @@ -106,7 +106,7 @@ public class AuthenticationSuccessHandler implements ServerAuthenticationSuccess User newUser = new User(); newUser.setName((String) userAttributes.get("name")); newUser.setEmail((String) userAttributes.get("email")); - newUser.setSource(LoginSource.GOOGLE); + newUser.setSource(LoginSource.fromString(authToken.getAuthorizedClientRegistrationId())); newUser.setState(UserState.ACTIVATED); newUser.setIsEnabled(true); // TODO: Check if this is a valid permission available in the DB diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomServerOAuth2AuthorizationRequestResolver.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomServerOAuth2AuthorizationRequestResolver.java index 8f1d8b9f70..0e3f92b0e4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomServerOAuth2AuthorizationRequestResolver.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomServerOAuth2AuthorizationRequestResolver.java @@ -1,6 +1,9 @@ package com.appsmith.server.authentication.handlers; +import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.constants.Security; +import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithException; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -17,6 +20,7 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.PkceParameterNames; import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames; +import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; import org.springframework.util.Assert; @@ -67,14 +71,17 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96); + private final CommonConfig commonConfig; + /** * Creates a new instance * * @param clientRegistrationRepository the repository to resolve the {@link ClientRegistration} + * @param commonConfig */ - public CustomServerOAuth2AuthorizationRequestResolver(ReactiveClientRegistrationRepository clientRegistrationRepository) { + public CustomServerOAuth2AuthorizationRequestResolver(ReactiveClientRegistrationRepository clientRegistrationRepository, CommonConfig commonConfig) { this(clientRegistrationRepository, new PathPatternParserServerWebExchangeMatcher( - DEFAULT_AUTHORIZATION_REQUEST_PATTERN)); + DEFAULT_AUTHORIZATION_REQUEST_PATTERN), commonConfig); } /** @@ -85,11 +92,12 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu * {@link #DEFAULT_REGISTRATION_ID_URI_VARIABLE_NAME} from the path variables. */ public CustomServerOAuth2AuthorizationRequestResolver(ReactiveClientRegistrationRepository clientRegistrationRepository, - ServerWebExchangeMatcher authorizationRequestMatcher) { + ServerWebExchangeMatcher authorizationRequestMatcher, CommonConfig commonConfig) { Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null"); Assert.notNull(authorizationRequestMatcher, "authorizationRequestMatcher cannot be null"); this.clientRegistrationRepository = clientRegistrationRepository; this.authorizationRequestMatcher = authorizationRequestMatcher; + this.commonConfig = commonConfig; } @Override @@ -135,6 +143,17 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { addPkceParameters(attributes, additionalParameters); } + if (!commonConfig.getAllowedDomains().isEmpty()) { + if (commonConfig.getAllowedDomains().size() == 1) { + // Incase there's only 1 domain, we can do a further optimization to let the user select a specific one + // from the list + additionalParameters.put("hd", commonConfig.getAllowedDomains().get(0)); + } else { + // Add multiple domains to the list of allowed domains + additionalParameters.put("hd", commonConfig.getAllowedDomains()); + } + } + builder.additionalParameters(additionalParameters); } else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) { builder = OAuth2AuthorizationRequest.implicit(); @@ -143,6 +162,8 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu "Invalid Authorization Grant Type (" + clientRegistration.getAuthorizationGrantType().getValue() + ") for Client Registration with Id: " + clientRegistration.getRegistrationId()); } + + return builder .clientId(clientRegistration.getClientId()) .authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri()) 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 5b8c4277f6..4603c53b7e 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 @@ -85,9 +85,12 @@ public class ClientUserRepository implements ServerOAuth2AuthorizedClientReposit // This is to provide more control over which accounts can be used to access the application. // TODO: This is not a good way to do this. Ideally, we should pass "hd=example.com" to OAuth2 provider to list relevant accounts only if (!commonConfig.getAllowedDomains().isEmpty()) { - DefaultOidcUser userPrincipal = (DefaultOidcUser) principal.getPrincipal(); - String domain = (String) userPrincipal.getAttributes().getOrDefault("hd", ""); - if (!commonConfig.getAllowedDomains().contains(domain)) { + String domain = null; + if (principal.getPrincipal() instanceof DefaultOidcUser) { + DefaultOidcUser userPrincipal = (DefaultOidcUser) principal.getPrincipal(); + domain = (String) userPrincipal.getAttributes().getOrDefault("hd", ""); + } + if (domain != null && !commonConfig.getAllowedDomains().contains(domain)) { return Mono.error(new AppsmithException(AppsmithError.UNAUTHORIZED_DOMAIN)); } } 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 ba087a069d..8cf2cdd865 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 @@ -118,7 +118,7 @@ public class SecurityConfig { .authenticationSuccessHandler(authenticationSuccessHandler) .authenticationFailureHandler(authenticationFailureHandler) .and().oauth2Login() - .authorizationRequestResolver(new CustomServerOAuth2AuthorizationRequestResolver(reactiveClientRegistrationRepository)) + .authorizationRequestResolver(new CustomServerOAuth2AuthorizationRequestResolver(reactiveClientRegistrationRepository, commonConfig)) .authenticationSuccessHandler(authenticationSuccessHandler) .authenticationFailureHandler(authenticationFailureHandler) .authorizedClientRepository(new ClientUserRepository(userService, commonConfig)) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/LoginSource.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/LoginSource.java index 7ed840d16a..87e73cd6b1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/LoginSource.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/LoginSource.java @@ -1,5 +1,11 @@ package com.appsmith.server.domains; +import com.appsmith.server.helpers.EnumUtils; + public enum LoginSource { - GOOGLE, FORM + GOOGLE, FORM, GITHUB; + + public static LoginSource fromString(String name) { + return EnumUtils.getEnumFromString(LoginSource.class, name); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/EnumUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/EnumUtils.java new file mode 100644 index 0000000000..977345065a --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/EnumUtils.java @@ -0,0 +1,22 @@ +package com.appsmith.server.helpers; + +public class EnumUtils { + + /** + * A common method for all enums since they can't have another base class + * + * @param Enum type + * @param c enum type. All enums must be all caps. + * @param string case insensitive + * @return corresponding enum, or null + */ + public static > T getEnumFromString(Class c, String string) { + if (c != null && string != null) { + try { + return Enum.valueOf(c, string.trim().toUpperCase()); + } catch (IllegalArgumentException ex) { + } + } + return null; + } +} diff --git a/app/server/appsmith-server/src/main/resources/application-docker.properties b/app/server/appsmith-server/src/main/resources/application-docker.properties index 873c03a48b..4de514334d 100644 --- a/app/server/appsmith-server/src/main/resources/application-docker.properties +++ b/app/server/appsmith-server/src/main/resources/application-docker.properties @@ -19,6 +19,8 @@ logging.pattern.console=%X - %m%n #Spring security spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com spring.security.oauth2.client.registration.google.client-secret=9dvITt4OayEY1HfeY8bHX74p +spring.security.oauth2.client.registration.github.client-id=ffa2f7468ea72758871c +spring.security.oauth2.client.registration.github.client-secret=b9c81a1a3216328b55a7df2d49fe2bbb6b1070f1 # Accounts from specific domains are allowed to login oauth2.allowed-domains=appsmith.com diff --git a/app/server/appsmith-server/src/main/resources/application-local.properties b/app/server/appsmith-server/src/main/resources/application-local.properties index 728d69785a..8d4dd491a2 100644 --- a/app/server/appsmith-server/src/main/resources/application-local.properties +++ b/app/server/appsmith-server/src/main/resources/application-local.properties @@ -19,6 +19,8 @@ logging.pattern.console=%X - %m%n #Spring security spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com spring.security.oauth2.client.registration.google.client-secret=9dvITt4OayEY1HfeY8bHX74p +spring.security.oauth2.client.registration.github.client-id=ffa2f7468ea72758871c +spring.security.oauth2.client.registration.github.client-secret=b9c81a1a3216328b55a7df2d49fe2bbb6b1070f1 # Accounts from specific domains are allowed to login oauth2.allowed-domains=appsmith.com diff --git a/app/server/appsmith-server/src/main/resources/application-staging.properties b/app/server/appsmith-server/src/main/resources/application-staging.properties index 56e995d4db..de93fc8e99 100644 --- a/app/server/appsmith-server/src/main/resources/application-staging.properties +++ b/app/server/appsmith-server/src/main/resources/application-staging.properties @@ -17,6 +17,8 @@ logging.pattern.console=%X - %m%n #Spring security spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com spring.security.oauth2.client.registration.google.client-secret=9dvITt4OayEY1HfeY8bHX74p +spring.security.oauth2.client.registration.github.client-id=ffa2f7468ea72758871c +spring.security.oauth2.client.registration.github.client-secret=b9c81a1a3216328b55a7df2d49fe2bbb6b1070f1 # Accounts from specific domains are allowed to login oauth2.allowed-domains=appsmith.com