Show error message to OAuth2 user when a matching local account alre…

…ady exists (#116)

Added error message on OAuth2 email collision

gateway/src/main/java/org/georchestra/gateway/accounts/admin/AbstractAccountsManager.java

@@ -23,6 +23,7 @@
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Consumer;
 
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
 import org.georchestra.security.model.GeorchestraUser;
 
 import lombok.NonNull;
@@ -37,7 +38,7 @@ public abstract class AbstractAccountsManager implements AccountManager {
  protected final ReadWriteLock lock = new ReentrantReadWriteLock();
 
  @Override
- public GeorchestraUser getOrCreate(@NonNull GeorchestraUser mappedUser) {
+ public GeorchestraUser getOrCreate(@NonNull GeorchestraUser mappedUser) throws DuplicatedEmailFoundException {
  return find(mappedUser).orElseGet(() -> createIfMissing(mappedUser));
  }
 
@@ -57,7 +58,7 @@ protected Optional<GeorchestraUser> findInternal(GeorchestraUser mappedUser) {
  return findByUsername(mappedUser.getUsername());
  }
 
- GeorchestraUser createIfMissing(GeorchestraUser mapped) {
+ GeorchestraUser createIfMissing(GeorchestraUser mapped) throws DuplicatedEmailFoundException {
  lock.writeLock().lock();
  try {
  GeorchestraUser existing = findInternal(mapped).orElse(null);

gateway/src/main/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizer.java

@@ -22,7 +22,9 @@
 import java.util.Optional;
 import java.util.WeakHashMap;
 
+import org.georchestra.ds.users.DuplicatedEmailException;
 import org.georchestra.gateway.security.GeorchestraUserCustomizerExtension;
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
 import org.georchestra.security.model.GeorchestraUser;
 import org.springframework.core.Ordered;
 import org.springframework.security.core.Authentication;
@@ -61,7 +63,8 @@ public class CreateAccountUserCustomizer implements GeorchestraUserCustomizerExt
  * otherwise.
  */
  @Override
- public @NonNull GeorchestraUser apply(@NonNull Authentication auth, @NonNull GeorchestraUser mappedUser) {
+ public @NonNull GeorchestraUser apply(@NonNull Authentication auth, @NonNull GeorchestraUser mappedUser)
+ throws DuplicatedEmailFoundException {
  final boolean isOauth2 = auth instanceof OAuth2AuthenticationToken;
  final boolean isPreAuth = auth instanceof PreAuthenticatedAuthenticationToken;
  if (isOauth2) {

gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java

@@ -36,8 +36,10 @@
 import org.georchestra.ds.users.AccountFactory;
 import org.georchestra.ds.users.DuplicatedEmailException;
 import org.georchestra.ds.users.DuplicatedUidException;
-import org.georchestra.gateway.accounts.admin.AbstractAccountsManager;;
+import org.georchestra.gateway.accounts.admin.AbstractAccountsManager;
 import org.georchestra.gateway.accounts.admin.AccountManager;
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
+import org.georchestra.gateway.security.exceptions.DuplicatedUsernameFoundException;
 import org.georchestra.security.api.UsersApi;
 import org.georchestra.security.model.GeorchestraUser;
 import org.springframework.beans.factory.annotation.Value;
@@ -89,12 +91,16 @@ private GeorchestraUser ensureRolesPrefixed(GeorchestraUser user) {
  }
 
  @Override
- protected void createInternal(GeorchestraUser mapped) {
+ protected void createInternal(GeorchestraUser mapped) throws DuplicatedEmailFoundException {
  Account newAccount = mapToAccountBrief(mapped);
  try {
  accountDao.insert(newAccount);
- } catch (DataServiceException | DuplicatedUidException | DuplicatedEmailException accountError) {
+ } catch (DataServiceException accountError) {
  throw new IllegalStateException(accountError);
+ } catch (DuplicatedEmailException accountError) {
+ throw new DuplicatedEmailFoundException(accountError.getMessage());
+ } catch (DuplicatedUidException accountError) {
+ throw new DuplicatedUsernameFoundException(accountError.getMessage());
  }
 
  ensureOrgExists(newAccount);

gateway/src/main/java/org/georchestra/gateway/app/GeorchestraGatewayApplication.java

@@ -28,6 +28,7 @@
 import javax.annotation.PostConstruct;
 
 import org.georchestra.gateway.security.GeorchestraUserMapper;
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
 import org.georchestra.gateway.security.ldap.LdapConfigProperties;
 import org.georchestra.security.model.GeorchestraUser;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -87,7 +88,12 @@ void initialize() {
  @GetMapping(path = "/whoami", produces = "application/json")
  @ResponseBody
  public Mono<Map<String, Object>> whoami(Authentication principal, ServerWebExchange exchange) {
- GeorchestraUser user = Optional.ofNullable(principal).flatMap(userMapper::resolve).orElse(null);
+ GeorchestraUser user = null;
+ try {
+ user = Optional.ofNullable(principal).flatMap(userMapper::resolve).orElse(null);
+ } catch (DuplicatedEmailFoundException e) {
+ }
+
  Map<String, Object> ret = new LinkedHashMap<>();
  ret.put("GeorchestraUser", user);
  if (principal == null) {
@@ -96,8 +102,6 @@ public Mono<Map<String, Object>> whoami(Authentication principal, ServerWebExcha
  ret.put(principal.getClass().getCanonicalName(), principal);
  }
  return Mono.just(ret);
- // return principal == null ? Mono.empty() :
- // Mono.just(Map.of(principal.getClass().getCanonicalName(), principal));
  }
 
  @GetMapping(path = "/logout")
@@ -123,6 +127,8 @@ public String loginPage(@RequestParam Map<String, String> allRequestParams, Mode
  mdl.addAttribute("passwordExpired", expired);
  boolean invalidCredentials = "invalid_credentials".equals(allRequestParams.get("error"));
  mdl.addAttribute("invalidCredentials", invalidCredentials);
+ boolean duplicateAccount = "duplicate_account".equals(allRequestParams.get("error"));
+ mdl.addAttribute("duplicateAccount", duplicateAccount);
  return "login";
  }
 

gateway/src/main/java/org/georchestra/gateway/security/GeorchestraUserMapper.java

@@ -21,7 +21,9 @@
 import java.util.List;
 import java.util.Optional;
 
+import org.georchestra.ds.users.DuplicatedEmailException;
 import org.georchestra.gateway.model.GeorchestraUsers;
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
 import org.georchestra.security.model.GeorchestraUser;
 import org.springframework.core.Ordered;
 import org.springframework.security.core.Authentication;
@@ -77,15 +79,16 @@ public class GeorchestraUserMapper {
  * {@link Optional#empty()} if no extension point implementation can
  * handle the auth token.
  */
- public Optional<GeorchestraUser> resolve(@NonNull Authentication authToken) {
+ public Optional<GeorchestraUser> resolve(@NonNull Authentication authToken) throws DuplicatedEmailFoundException {
  return resolvers.stream()//
  .map(resolver -> resolver.resolve(authToken))//
  .filter(Optional::isPresent)//
  .map(Optional::orElseThrow)//
  .map(mapped -> customize(authToken, mapped)).findFirst();
  }
 
- private GeorchestraUser customize(@NonNull Authentication authToken, GeorchestraUser mapped) {
+ private GeorchestraUser customize(@NonNull Authentication authToken, GeorchestraUser mapped)
+ throws DuplicatedEmailFoundException {
  GeorchestraUser customized = mapped;
  for (GeorchestraUserCustomizerExtension customizer : customizers) {
  customized = customizer.apply(authToken, customized);

gateway/src/main/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilter.java

@@ -18,25 +18,36 @@
  */
 package org.georchestra.gateway.security;
 
+import org.apache.commons.lang3.tuple.Pair;
 import org.georchestra.gateway.model.GeorchestraOrganizations;
 import org.georchestra.gateway.model.GeorchestraTargetConfig;
 import org.georchestra.gateway.model.GeorchestraUsers;
+import org.georchestra.gateway.security.exceptions.DuplicatedEmailFoundException;
 import org.georchestra.gateway.security.ldap.extended.ExtendedGeorchestraUser;
 import org.georchestra.security.model.GeorchestraUser;
-import org.georchestra.security.model.Organization;
 import org.springframework.cloud.gateway.filter.GatewayFilterChain;
+import org.georchestra.security.model.Organization;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.cloud.gateway.filter.GlobalFilter;
 import org.springframework.cloud.gateway.filter.RouteToRequestUrlFilter;
 import org.springframework.cloud.gateway.route.Route;
 import org.springframework.core.Ordered;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.web.server.DefaultServerRedirectStrategy;
+import org.springframework.security.web.server.ServerRedirectStrategy;
 import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.WebSession;
 
 import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import reactor.core.publisher.Mono;
 
+import java.net.URI;
+import java.util.Optional;
+
 /**
  * A {@link GlobalFilter} that resolves the {@link GeorchestraUser} from the
  * request's {@link Authentication} so it can be {@link GeorchestraUsers#resolve
@@ -56,6 +67,10 @@ public class ResolveGeorchestraUserGlobalFilter implements GlobalFilter, Ordered
 
  private final @NonNull GeorchestraUserMapper resolver;
 
+ private ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy();
+
+ private static String DUPLICATE_ACCOUNT = "duplicate_account";
+
  /**
  * @return a lower precedence than {@link RouteToRequestUrlFilter}'s, in order
  * to make sure the matched {@link Route} has been set as a
@@ -72,7 +87,6 @@ public class ResolveGeorchestraUserGlobalFilter implements GlobalFilter, Ordered
  * chain.
  */
  public @Override Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
-
  return exchange.getPrincipal()//
  .doOnNext(p -> log.debug("resolving user from {}", p.getClass().getName()))//
  .filter(Authentication.class::isInstance)//
@@ -89,7 +103,11 @@ public class ResolveGeorchestraUserGlobalFilter implements GlobalFilter, Ordered
  return exchange;
  })//
  .defaultIfEmpty(exchange)//
- .flatMap(chain::filter);
+ .flatMap(chain::filter)//
+ .onErrorResume(DuplicatedEmailFoundException.class,
+ exp -> this.redirectStrategy
+ .sendRedirect(exchange, URI.create("/login?error=" + DUPLICATE_ACCOUNT))
+ .then(exchange.getSession().flatMap(WebSession::invalidate)));
  }
 
 }

gateway/src/main/java/org/georchestra/gateway/security/exceptions/DuplicatedEmailFoundException.java

@@ -0,0 +1,13 @@
+package org.georchestra.gateway.security.exceptions;
+
+public class DuplicatedEmailFoundException extends RuntimeException {
+ private String message;
+
+ public DuplicatedEmailFoundException(String message) {
+ super(message);
+ this.message = message;
+ }
+
+ public DuplicatedEmailFoundException() {
+ }
+}

gateway/src/main/java/org/georchestra/gateway/security/exceptions/DuplicatedUsernameFoundException.java

@@ -0,0 +1,13 @@
+package org.georchestra.gateway.security.exceptions;
+
+public class DuplicatedUsernameFoundException extends RuntimeException {
+ private String message;
+
+ public DuplicatedUsernameFoundException(String message) {
+ super(message);
+ this.message = message;
+ }
+
+ public DuplicatedUsernameFoundException() {
+ }
+}

gateway/src/main/java/org/georchestra/gateway/security/preauth/PreauthAuthenticationManager.java

@@ -38,16 +38,16 @@
 
 import reactor.core.publisher.Mono;
 
-class PreauthAuthenticationManager implements ReactiveAuthenticationManager, ServerAuthenticationConverter {
+public class PreauthAuthenticationManager implements ReactiveAuthenticationManager, ServerAuthenticationConverter {
 
- static final String PREAUTH_HEADER_NAME = "sec-georchestra-preauthenticated";
+ public static final String PREAUTH_HEADER_NAME = "sec-georchestra-preauthenticated";
 
- private static final String PREAUTH_USERNAME = "preauth-username";
- private static final String PREAUTH_EMAIL = "preauth-email";
- private static final String PREAUTH_FIRSTNAME = "preauth-firstname";
- private static final String PREAUTH_LASTNAME = "preauth-lastname";
- private static final String PREAUTH_ORG = "preauth-org";
- private static final String PREAUTH_ROLES = "preauth-roles";
+ public static final String PREAUTH_USERNAME = "preauth-username";
+ public static final String PREAUTH_EMAIL = "preauth-email";
+ public static final String PREAUTH_FIRSTNAME = "preauth-firstname";
+ public static final String PREAUTH_LASTNAME = "preauth-lastname";
+ public static final String PREAUTH_ORG = "preauth-org";
+ public static final String PREAUTH_ROLES = "preauth-roles";
 
  /**
  * @return {@code Mono.empty()} if the pre-auth request headers are not

gateway/src/main/resources/messages/login.properties

@@ -8,4 +8,5 @@ forget_password = Forgot password ?
 identity_provider_title = Log in with an identity provider
 expired_password = Your password has been expired
 expired_password_link = and should be changed
-invalid_credentials = Invalid username or password
+invalid_credentials = Invalid username or password
+duplicate_account = An account already exists using this email address

gateway/src/main/resources/messages/login_de.properties

@@ -8,4 +8,5 @@ forget_password = Passwort vergessen?
 identity_provider_title = Melden Sie sich bei einem Identitätsanbieter an
 expired_password = Ihr Passwort ist abgelaufen
 expired_password_link = und sollte geändert werden
-invalid_credentials = Ungültiger Benutzername oder Passwort
+invalid_credentials = Ungültiger Benutzername oder Passwort
+duplicate_account = Es existiert bereits ein Konto mit dieser E-Mail-Adresse

gateway/src/main/resources/messages/login_en.properties

@@ -8,4 +8,5 @@ forget_password = Forgot password ?
 identity_provider_title = Log in with an identity provider
 expired_password = Your password has been expired
 expired_password_link = and should be changed
-invalid_credentials = Invalid username or password
+invalid_credentials = Invalid username or password
+duplicate_account = An account already exists using this email address

gateway/src/main/resources/messages/login_es.properties

@@ -8,4 +8,5 @@ forget_password = Contraseña olvidada ?
 identity_provider_title = Iniciar sesión con un proveedor de identidad
 expired_password = Su contraseña ha caducado
 expired_password_link = y debería ser cambiado
-invalid_credentials = Nombre de usuario o contraseña invalido
+invalid_credentials = Nombre de usuario o contraseña invalido
+duplicate_account = Ya existe una cuenta usando esta dirección de correo electrónico

gateway/src/main/resources/messages/login_fr.properties

@@ -8,4 +8,5 @@ forget_password = Mot de passe oublié ?
 identity_provider_title = Se connecter depuis un fournisseur d'identité
 expired_password = Votre mot de passe a expiré
 expired_password_link = et doit être changé
-invalid_credentials = Nom d'utilisateur ou mot de passe non valide
+invalid_credentials = Nom d'utilisateur ou mot de passe non valide
+duplicate_account = Il existe déjà un compte utilisant cette adresse e-mail

gateway/src/main/resources/messages/login_nl.properties

@@ -8,4 +8,5 @@ forget_password = Wachtwoord vergeten ?
 identity_provider_title = Log in met een identiteitsprovider
 expired_password = Uw wachtwoord is verlopen
 expired_password_link = en moet worden veranderd
-invalid_credentials = ongeldige gebruikersnaam of wachtwoord
+invalid_credentials = ongeldige gebruikersnaam of wachtwoord
+duplicate_account = Er bestaat al een account met dit e-mailadres

gateway/src/main/resources/messages/login_ru.properties

@@ -8,4 +8,5 @@ forget_password = Забыли пароль ?
 identity_provider_title = Войдите в систему с помощью поставщика удостоверений
 expired_password = Срок действия вашего пароля истек,
 expired_password_link = и следует изменить
-invalid_credentials = неправильное имя пользователя или пароль
+invalid_credentials = неправильное имя пользователя или пароль
+duplicate_account = Учетная запись уже существует с использованием этого адреса электронной почты

gateway/src/main/resources/templates/login.html

@@ -39,6 +39,7 @@ <h2 class="form-signin-heading"><span th:text="#{login_message_title}"></span></
  <div style="text-align: center; font-size: 18px; color: #ff0033;" th:if="${passwordExpired}"> <span th:text="#{expired_password}" ></span>
  <a href="/console/account/passwordRecovery" > <span th:text="#{expired_password_link}" ></span> </a>
  </div>
+ <div style="text-align: center; font-size: 18px; color: #ff0033;" th:if="${duplicateAccount}"> <span th:text="#{duplicate_account}"></span> </div>
  </div>
  </form>
  <div th:if="${oauth2LoginLinks.size() != 0}" class="container"><h2 class="form-signin-heading">Login with OAuth 2.0</h2>

gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java

@@ -1,9 +1,11 @@
 package org.georchestra.gateway.security;
 
+import org.georchestra.gateway.accounts.admin.CreateAccountUserCustomizer;
 import org.georchestra.gateway.app.GeorchestraGatewayApplication;
 import org.georchestra.gateway.filter.headers.providers.JsonPayloadHeadersContributor;
 import org.georchestra.gateway.model.GatewayConfigProperties;
 import org.georchestra.gateway.model.HeaderMappings;
+import org.georchestra.gateway.security.preauth.PreauthAuthenticationManager;
 import org.georchestra.testcontainers.ldap.GeorchestraLdapContainer;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
@@ -12,6 +14,7 @@
 import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.context.ApplicationContext;
+import org.springframework.http.MediaType;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.web.reactive.server.WebTestClient;
 import org.testcontainers.containers.GenericContainer;
@@ -23,7 +26,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 @SpringBootTest(classes = GeorchestraGatewayApplication.class)
-@AutoConfigureWebTestClient(timeout = "PT20S")
+@AutoConfigureWebTestClient(timeout = "PT200S")
 @ActiveProfiles("georheaders")
 public class ResolveGeorchestraUserGlobalFilterIT {
 
@@ -101,4 +104,32 @@ protected void doStart() {
  .jsonPath(".request.headers.sec-organization").exists();
 
  }
+
+ /**
+ * Show error message to OAuth2 user when a matching local account already
+ * exists: i.e. it tries to create a user with an email address for which a user
+ * already exists.
+ * <p>
+ * {@link GeorchestraUserMapper} calls the
+ * {@link GeorchestraUserCustomizerExtension}s.
+ * {@link CreateAccountUserCustomizer} will try to create an account with email
+ * {@literal psc+testadmin@georchestra.org}, which already exists (for user
+ * {@literal testadmin})
+ */
+ @Test
+ void testRedirectIfOauth2UserExists() {
+ final String email = "psc+testadmin@georchestra.org";
+ final String expected = "/login?error=duplicate_account";
+
+ testClient.get().uri("/echo/")//
+ .header(PreauthAuthenticationManager.PREAUTH_HEADER_NAME, "true")
+ .header(PreauthAuthenticationManager.PREAUTH_EMAIL, email)
+ .header(PreauthAuthenticationManager.PREAUTH_FIRSTNAME, "bob")
+ .header(PreauthAuthenticationManager.PREAUTH_LASTNAME, "sponge")
+ .header(PreauthAuthenticationManager.PREAUTH_USERNAME, "bobsponge").accept(MediaType.APPLICATION_JSON)
+ .exchange()//
+ .expectStatus()//
+ .is3xxRedirection().expectHeader().location(expected);
+ }
+
 }