Adding Github login feature. Also adding condition to limit domain access in Google OAuth2

The domain restriction has been done by adding parameter `hd` in the function CustomServerOAuth2AuthorizationRequestResolver#authorizationRequest. We still verify if the OAuth2 response has the parameter `hd` to ensure that no client side manipulation has been performed.
This commit is contained in:
Arpit Mohan 2020-01-13 12:13:53 +05:30
parent 7622d76f32
commit db27e7c86c
10 changed files with 77 additions and 24 deletions

View File

@ -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 // 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. // login page again with an error message shown to the user.
String originHeader = exchange.getRequest().getHeaders().getOrigin(); String state = exchange.getRequest().getQueryParams().getFirst(Security.QUERY_PARAMETER_STATE);
if (originHeader == null || originHeader.isEmpty()) { String originHeader = "";
// Check the referer header if the origin is not available if (state != null && !state.isEmpty()) {
String refererHeader = exchange.getRequest().getHeaders().getFirst(Security.REFERER_HEADER); String[] stateArray = state.split(",");
if (refererHeader != null && !refererHeader.isBlank()) { for (int i = 0; i < stateArray.length; i++) {
URI uri = null; String stateVar = stateArray[i];
try { if (stateVar != null && stateVar.startsWith(Security.STATE_PARAMETER_ORIGIN) && stateVar.contains("=")) {
uri = new URI(refererHeader); // This is the origin of the request that we want to redirect to
String authority = uri.getAuthority(); originHeader = stateVar.split("=")[1];
String scheme = uri.getScheme();
originHeader = scheme + "://" + authority;
} catch (URISyntaxException e) {
originHeader = "/";
} }
} else {
originHeader = "/";
} }
} }
URI defaultRedirectLocation = URI.create(originHeader + "/user/login?error=true"); URI defaultRedirectLocation = URI.create(originHeader + "/user/login?error=true");
return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation);

View File

@ -106,7 +106,7 @@ public class AuthenticationSuccessHandler implements ServerAuthenticationSuccess
User newUser = new User(); User newUser = new User();
newUser.setName((String) userAttributes.get("name")); newUser.setName((String) userAttributes.get("name"));
newUser.setEmail((String) userAttributes.get("email")); newUser.setEmail((String) userAttributes.get("email"));
newUser.setSource(LoginSource.GOOGLE); newUser.setSource(LoginSource.fromString(authToken.getAuthorizedClientRegistrationId()));
newUser.setState(UserState.ACTIVATED); newUser.setState(UserState.ACTIVATED);
newUser.setIsEnabled(true); newUser.setIsEnabled(true);
// TODO: Check if this is a valid permission available in the DB // TODO: Check if this is a valid permission available in the DB

View File

@ -1,6 +1,9 @@
package com.appsmith.server.authentication.handlers; package com.appsmith.server.authentication.handlers;
import com.appsmith.server.configurations.CommonConfig;
import com.appsmith.server.constants.Security; import com.appsmith.server.constants.Security;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus; 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.endpoint.PkceParameterNames;
import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.core.oidc.OidcScopes;
import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames; 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.PathPatternParserServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
import org.springframework.util.Assert; 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 StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
private final CommonConfig commonConfig;
/** /**
* Creates a new instance * Creates a new instance
* *
* @param clientRegistrationRepository the repository to resolve the {@link ClientRegistration} * @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( 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. * {@link #DEFAULT_REGISTRATION_ID_URI_VARIABLE_NAME} from the path variables.
*/ */
public CustomServerOAuth2AuthorizationRequestResolver(ReactiveClientRegistrationRepository clientRegistrationRepository, public CustomServerOAuth2AuthorizationRequestResolver(ReactiveClientRegistrationRepository clientRegistrationRepository,
ServerWebExchangeMatcher authorizationRequestMatcher) { ServerWebExchangeMatcher authorizationRequestMatcher, CommonConfig commonConfig) {
Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null"); Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null");
Assert.notNull(authorizationRequestMatcher, "authorizationRequestMatcher cannot be null"); Assert.notNull(authorizationRequestMatcher, "authorizationRequestMatcher cannot be null");
this.clientRegistrationRepository = clientRegistrationRepository; this.clientRegistrationRepository = clientRegistrationRepository;
this.authorizationRequestMatcher = authorizationRequestMatcher; this.authorizationRequestMatcher = authorizationRequestMatcher;
this.commonConfig = commonConfig;
} }
@Override @Override
@ -135,6 +143,17 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu
if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
addPkceParameters(attributes, additionalParameters); 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); builder.additionalParameters(additionalParameters);
} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) { } else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
builder = OAuth2AuthorizationRequest.implicit(); builder = OAuth2AuthorizationRequest.implicit();
@ -143,6 +162,8 @@ public class CustomServerOAuth2AuthorizationRequestResolver implements ServerOAu
"Invalid Authorization Grant Type (" + clientRegistration.getAuthorizationGrantType().getValue() "Invalid Authorization Grant Type (" + clientRegistration.getAuthorizationGrantType().getValue()
+ ") for Client Registration with Id: " + clientRegistration.getRegistrationId()); + ") for Client Registration with Id: " + clientRegistration.getRegistrationId());
} }
return builder return builder
.clientId(clientRegistration.getClientId()) .clientId(clientRegistration.getClientId())
.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri()) .authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri())

View File

@ -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. // 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 // 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()) { if (!commonConfig.getAllowedDomains().isEmpty()) {
DefaultOidcUser userPrincipal = (DefaultOidcUser) principal.getPrincipal(); String domain = null;
String domain = (String) userPrincipal.getAttributes().getOrDefault("hd", ""); if (principal.getPrincipal() instanceof DefaultOidcUser) {
if (!commonConfig.getAllowedDomains().contains(domain)) { 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)); return Mono.error(new AppsmithException(AppsmithError.UNAUTHORIZED_DOMAIN));
} }
} }

View File

@ -118,7 +118,7 @@ public class SecurityConfig {
.authenticationSuccessHandler(authenticationSuccessHandler) .authenticationSuccessHandler(authenticationSuccessHandler)
.authenticationFailureHandler(authenticationFailureHandler) .authenticationFailureHandler(authenticationFailureHandler)
.and().oauth2Login() .and().oauth2Login()
.authorizationRequestResolver(new CustomServerOAuth2AuthorizationRequestResolver(reactiveClientRegistrationRepository)) .authorizationRequestResolver(new CustomServerOAuth2AuthorizationRequestResolver(reactiveClientRegistrationRepository, commonConfig))
.authenticationSuccessHandler(authenticationSuccessHandler) .authenticationSuccessHandler(authenticationSuccessHandler)
.authenticationFailureHandler(authenticationFailureHandler) .authenticationFailureHandler(authenticationFailureHandler)
.authorizedClientRepository(new ClientUserRepository(userService, commonConfig)) .authorizedClientRepository(new ClientUserRepository(userService, commonConfig))

View File

@ -1,5 +1,11 @@
package com.appsmith.server.domains; package com.appsmith.server.domains;
import com.appsmith.server.helpers.EnumUtils;
public enum LoginSource { public enum LoginSource {
GOOGLE, FORM GOOGLE, FORM, GITHUB;
public static LoginSource fromString(String name) {
return EnumUtils.getEnumFromString(LoginSource.class, name);
}
} }

View File

@ -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 <T> Enum type
* @param c enum type. All enums must be all caps.
* @param string case insensitive
* @return corresponding enum, or null
*/
public static <T extends Enum<T>> T getEnumFromString(Class<T> c, String string) {
if (c != null && string != null) {
try {
return Enum.valueOf(c, string.trim().toUpperCase());
} catch (IllegalArgumentException ex) {
}
}
return null;
}
}

View File

@ -19,6 +19,8 @@ logging.pattern.console=%X - %m%n
#Spring security #Spring security
spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com 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.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 # Accounts from specific domains are allowed to login
oauth2.allowed-domains=appsmith.com oauth2.allowed-domains=appsmith.com

View File

@ -19,6 +19,8 @@ logging.pattern.console=%X - %m%n
#Spring security #Spring security
spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com 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.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 # Accounts from specific domains are allowed to login
oauth2.allowed-domains=appsmith.com oauth2.allowed-domains=appsmith.com

View File

@ -17,6 +17,8 @@ logging.pattern.console=%X - %m%n
#Spring security #Spring security
spring.security.oauth2.client.registration.google.client-id=869021686091-9b84bbf7ea683t1aaefqnmefcnmk6fq6.apps.googleusercontent.com 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.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 # Accounts from specific domains are allowed to login
oauth2.allowed-domains=appsmith.com oauth2.allowed-domains=appsmith.com