Skip to content

Commit

Permalink
Il est possible de préinscrire les utilisateurs avec l'email uniqueme…
Browse files Browse the repository at this point in the history 
…nt (#998)

* Le formulaire d'inscription marche sur mobile ! (cas préinscription)
* Le formulaire d'inscription / CGU permet l'inscription des comptes partagés dans le cas d'une préinscription
* Compatible IE11
* La partie admin préinscription montre toutes les erreurs et ajoute ce qui est valide (= workflow largement amélioré par rapport à l'import csv)
* Correction du bug : lorsque les utilisateurs rentrent un email avec des espaces avant ou après, l'email n'est pas reconnu
* Correction du bug : lorsque la blacklist d'emails est vide (comme en demo), les emails ne sont pas envoyés
* Certains messages d'erreurs lors du logins sont légèrement plus détaillés
* Les exceptions dans `TokenService` sont gérées (plus de page 500 de play pour ça)
* Certains events sont reclassés d'error vers warn pour avoir une meilleure visibilité sur les vraies erreurs
* Ajout des détails lors d'erreurs de validation sur formulaire d'inscription (dans l'optique d'avoir une visibilité sur la compréhension des champs)
  • Loading branch information
@niladic
niladic authored Apr 20, 2021
1 parent 1a2bd57 commit eae174f
Show file tree
Hide file tree
Showing 43 changed files with 2,491 additions and 230 deletions.
215 changes: 171 additions & 44 deletions app/actions/LoginAction.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import constants.Constants
import controllers.routes
import helper.BooleanHelper.not
import helper.UUIDHelper
import java.util.UUID
import javax.inject.{Inject, Singleton}
import models.EventType.{
AuthByKey,
Expand All @@ -16,11 +17,11 @@ import models.EventType.{
UserAccessDisabled
}
import models._
import play.api.Logger
import play.api.mvc.Results.TemporaryRedirect
import play.api.{Configuration, Logger}
import play.api.mvc.Results.{InternalServerError, TemporaryRedirect}
import play.api.mvc._
import serializers.Keys
import services.{EventService, TokenService, UserService}
import services.{EventService, SignupService, TokenService, UserService}

import scala.concurrent.{ExecutionContext, Future}

Expand Down Expand Up @@ -49,7 +50,8 @@ class LoginAction @Inject() (
userService: UserService,
eventService: EventService,
tokenService: TokenService,
configuration: play.api.Configuration
configuration: Configuration,
signupService: SignupService
)(implicit ec: ExecutionContext)
extends ActionBuilder[RequestWithUserData, AnyContent]
with ActionRefiner[Request, RequestWithUserData] {
Expand Down Expand Up @@ -78,21 +80,35 @@ class LoginAction @Inject() (
request.queryString - Keys.QueryParam.key - Keys.QueryParam.token
)
val url = "http" + (if (request.secure) "s" else "") + "://" + request.host + path
(userBySession, userByKey, tokenById) match {
case (Some(userSession), Some(userKey), None) if userSession.id === userKey.id =>
val signupOpt = request.session.get(Keys.Session.signupId).flatMap(UUIDHelper.fromString)
val tokenOpt = request.getQueryString(Keys.QueryParam.token)

(userBySession, userByKey, tokenOpt, signupOpt) match {
// Note: this case is deliberately put here for failing fast, if the token is invalid,
// we don't want to continue doing sensitive operations
case (_, _, Some(rawToken), _) =>
tryAuthByToken(rawToken)
// Note: user.key is used here as a way to check if the user comes from an email.
// With that key, the user won't see the warn in the last case.
// Consequently, if there is no key, the user will see the case
// userNotLogged("Vous devez vous identifier pour accéder à cette page.")
// It is not clear this is a good thing and the code is definitely confusing.
case (Some(userSession), Some(userKey), None, None) if userSession.id === userKey.id =>
Future(Left(TemporaryRedirect(Call(request.method, url).url)))
case (_, Some(user), None) =>
case (_, Some(user), None, None) =>
LoginAction.readUserRights(user).map { userRights =>
val area = user.areas.headOption
.flatMap(Area.fromId)
.getOrElse(Area.all.head)
implicit val requestWithUserData =
new RequestWithUserData(user, userRights, request)
if (areasWithLoginByKey.contains(area.id) && !user.admin) {
// areasWithLoginByKey is an insecure setting for demo usage and transition only
// areasWithLoginByKey is an insecure setting for demo usage
eventService.log(
LoginByKey,
s"Connexion par clé réussie (Transition ne doit pas être maintenu en prod)"
"Connexion par clé réussie (seulement pour la demo / " +
"CE LOG NE DOIT PAS APPARAITRE EN PROD !!! Si c'est le cas, " +
"il faut vider la variable d'environnement correspondant à areasWithLoginByKey)"
)
Left(
TemporaryRedirect(Call(request.method, url).url)
Expand All @@ -108,11 +124,9 @@ class LoginAction @Inject() (
)
}
}
case (_, None, Some(token)) =>
manageToken(token)
case (Some(user), None, None) if not(user.disabled) =>
case (Some(user), None, None, None) if not(user.disabled) =>
manageUserLogged(user)
case (Some(user), None, None) if user.disabled =>
case (Some(user), None, None, None) if user.disabled =>
LoginAction.readUserRights(user).map { userRights =>
implicit val requestWithUserData =
new RequestWithUserData(user, userRights, request)
Expand All @@ -124,32 +138,72 @@ class LoginAction @Inject() (
s"Votre compte a été désactivé. Contactez votre référent ou l'équipe d'Administration+ sur ${Constants.supportEmail} en cas de problème."
)
}
case (_, _, _, Some(signupId)) =>
manageSignup(signupId)
case _ =>
request.getQueryString(Keys.QueryParam.token) match {
case Some(token) =>
eventService.info(
User.systemUser,
request.remoteAddress,
"UNKNOWN_TOKEN",
s"Token $token est inconnu",
None,
None,
None
)
Future(
userNotLogged(
"Le lien que vous avez utilisé n'est plus valide, il a déjà été utilisé. Si cette erreur se répète, contactez l'équipe Administration+"
)
)
case None if routes.HomeController.index().url.contains(path) =>
Future(userNotLoggedOnLoginPage)
case None =>
log.warn(s"Accès à la ${request.path} non autorisé")
Future(userNotLogged("Vous devez vous identifier pour accéder à cette page."))
if (routes.HomeController.index().url.contains(path)) {
Future(userNotLoggedOnLoginPage)
} else {
log.warn(s"Accès à la ${request.path} non autorisé")
Future(userNotLogged("Vous devez vous identifier pour accéder à cette page."))
}
}
}

private def tryAuthByToken[A](
rawToken: String
)(implicit request: Request[A]): Future[Either[Result, RequestWithUserData[A]]] =
tokenService
.byTokenThenDelete(rawToken)
.flatMap(
_.fold(
e => {
eventService.logErrorNoUser(e)
if (e.eventType === EventType.TokenDoubleUsage)
// Note: a few users are confused by login errors, we provide here
// a better explanation than the default Play error page.
// A nice HTML page might be better though.
Future(
userNotLogged(
"Le même lien semble avoir été utilisé 2 fois. " +
"Nous vous invitons à aller sur vos demandes " +
"afin de vérifier si votre première tentative est valide. " +
"Si vous n’accédez pas à vos demandes automatiquement, " +
"un nouveau lien de connexion doit vous être envoyé. " +
"Dans ce cas il vous suffit de saisir votre email dans le champ prévu " +
"à cet effet sur la page d’accueil."
)
)
else
Future(generic500.asLeft)
},
{
case None =>
eventService.info(
User.systemUser,
request.remoteAddress,
"UNKNOWN_TOKEN",
s"Token $rawToken est inconnu",
None,
None,
None
)
Future(
userNotLogged(
"Le lien que vous avez utilisé n'est plus valide, il a déjà été utilisé. " +
"Vous pouvez générer un nouveau lien en saisissant votre email dans le champ " +
"prévu à cet effet."
)
)
case Some(token) =>
token.origin match {
case LoginToken.Origin.User(userId) => manageTokenWithUserId(token, userId)
case LoginToken.Origin.Signup(signupId) => manageTokenWithSignupId(token, signupId)
}
}
)
)

private def manageUserLogged[A](user: User)(implicit request: Request[A]) =
LoginAction.readUserRights(user).map { userRights =>
implicit val requestWithUserData = new RequestWithUserData(user, userRights, request)
Expand All @@ -164,11 +218,27 @@ class LoginAction @Inject() (
}
}

private def manageToken[A](token: LoginToken)(implicit request: Request[A]) = {
val userOption = userService.byId(token.userId)
private def manageSignup[A](signupId: UUID)(implicit request: Request[A]) = {
eventService.logSystem(
ToCGURedirected,
s"Redirection vers le formulaire d'inscription (préinscription $signupId)"
)
// Not an infinite redirect because `signupForm` does not uses `LoginAction`
Future(
Left(
TemporaryRedirect(routes.SignupController.signupForm().url)
.flashing("redirect" -> request.path)
)
)
}

private def manageTokenWithUserId[A](token: LoginToken, userId: UUID)(implicit
request: Request[A]
): Future[Either[Result, RequestWithUserData[A]]] = {
val userOption: Option[User] = userService.byId(userId)
userOption match {
case None =>
log.error(s"Try to login by token ${token.token} for an unknown user : ${token.userId}")
log.error(s"Try to login by token ${token.token} for an unknown user : ${userId}")
Future(userNotLogged("Une erreur s'est produite, votre utilisateur n'existe plus"))
case Some(user) =>
LoginAction.readUserRights(user).map { userRights =>
Expand All @@ -179,7 +249,7 @@ class LoginAction @Inject() (
if (token.ipAddress =!= request.remoteAddress) {
eventService.log(
AuthWithDifferentIp,
s"Utilisateur ${token.userId} à une adresse ip différente pour l'essai de connexion"
s"Utilisateur ${userId} à une adresse ip différente pour l'essai de connexion"
)
}
if (token.isActive) {
Expand All @@ -190,11 +260,12 @@ class LoginAction @Inject() (
Left(
TemporaryRedirect(Call(request.method, url).url)
.withSession(
request.session - Keys.Session.userId + (Keys.Session.userId -> user.id.toString)
request.session - Keys.Session.userId - Keys.Session.signupId +
(Keys.Session.userId -> user.id.toString)
)
)
} else {
eventService.log(ExpiredToken, s"Token expiré pour ${token.userId}")
eventService.log(ExpiredToken, s"Token expiré pour $userId")
redirectToHomeWithEmailSendbackButton(
user.email,
s"Votre lien de connexion a expiré, il est valable $tokenExpirationInMinutes minutes à réception."
Expand All @@ -204,10 +275,60 @@ class LoginAction @Inject() (
}
}

private def manageTokenWithSignupId[A](token: LoginToken, signupId: UUID)(implicit
request: Request[A]
): Future[Either[Result, RequestWithUserData[A]]] =
signupService
.byId(signupId)
.map(
_.fold(
e => {
eventService.logErrorNoUser(e)
generic500.asLeft
},
{
case None =>
eventService.logSystem(
EventType.MissingSignup,
s"Tentative de connexion par token ${token.token} " +
s"avec une préinscription inconnue : ${signupId}"
)
userNotLogged(
"Une erreur s'est produite, les données sur votre inscription n'existent plus"
)
case Some(signupRequest) =>
if (token.isActive) {
val url = request.path + queryToString(
request.queryString - Keys.QueryParam.key - Keys.QueryParam.token
)
eventService.logSystem(
EventType.AuthBySignupToken,
s"Identification par token avec la préinscription ${signupRequest.id}"
)
Left(
TemporaryRedirect(Call(request.method, url).url)
.withSession(
request.session - Keys.Session.signupId + (Keys.Session.signupId -> signupRequest.id.toString)
)
)
} else {
eventService.logSystem(
ExpiredToken,
s"Token expiré pour la préinscription ${signupRequest.id}"
)
redirectToHomeWithEmailSendbackButton(
signupRequest.email,
s"Votre lien de connexion a expiré, il est valable $tokenExpirationInMinutes minutes à réception."
)
}
}
)
)

private def userNotLogged[A](message: String)(implicit request: Request[A]) =
Left(
TemporaryRedirect(routes.LoginController.login().url)
.withSession(request.session - Keys.Session.userId)
.withSession(request.session - Keys.Session.userId - Keys.Session.signupId)
.flashing("error" -> message)
)

Expand All @@ -216,7 +337,7 @@ class LoginAction @Inject() (
) =
Left(
TemporaryRedirect(routes.HomeController.index().url)
.withSession(request.session - Keys.Session.userId)
.withSession(request.session - Keys.Session.userId - Keys.Session.signupId)
.flashing("email" -> email, "error" -> message)
)

Expand All @@ -226,8 +347,14 @@ class LoginAction @Inject() (
.withSession(request.session - Keys.Session.userId)
)

private def tokenById[A](implicit request: Request[A]): Option[LoginToken] =
request.getQueryString(Keys.QueryParam.token).flatMap(tokenService.byToken)
// Note: Instead of a blank page with a message, sending back to the home page
// similar to `redirectToHomeWithEmailSendbackButton` might be better
private def generic500 =
InternalServerError(
"Une erreur s’est produite sur le serveur. " +
"Celle-ci est possiblement temporaire, " +
"nous vous invitons à réessayer plus tard."
)

private def userByKey[A](implicit request: Request[A]): Option[User] =
request.getQueryString(Keys.QueryParam.key).flatMap(userService.byKey)
Expand Down
10 changes: 4 additions & 6 deletions app/controllers/CSVImportController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import models.EventType.{
UsersImported
}
import models.formModels.{
normalizedOptionalText,
normalizedText,
CSVRawLinesFormData,
CSVReviewUserFormData,
CSVUserFormData,
Expand Down Expand Up @@ -186,12 +188,8 @@ case class CSVImportController @Inject() (
},
Option.apply
),
"name" -> text(maxLength = UserGroup.nameMaxLength)
.transform[String](commonStringInputNormalization, commonStringInputNormalization),
"description" -> optional(text).transform[Option[String]](
_.map(commonStringInputNormalization).filter(_.nonEmpty),
_.map(commonStringInputNormalization).filter(_.nonEmpty)
),
"name" -> normalizedText.verifying(maxLength(UserGroup.nameMaxLength)),
"description" -> normalizedOptionalText,
"insee-code" -> list(text),
"creationDate" -> ignored(date),
"area-ids" -> list(uuid)
Expand Down
20 changes: 6 additions & 14 deletions app/controllers/GroupController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import actions.{LoginAction, RequestWithUserData}
import Operators._
import javax.inject.{Inject, Singleton}
import models.{Area, Organisation, UserGroup}
import models.formModels.{normalizedOptionalText, normalizedText}
import org.webjars.play.WebJarsUtil
import play.api.Configuration
import play.api.data.Form
import play.api.data.Forms.{email, ignored, list, mapping, of, optional, text, uuid}
import play.api.data.validation.Constraints.maxLength
import play.api.mvc.{Action, AnyContent, InjectedController}
import play.libs.ws.WSClient
import services._
Expand Down Expand Up @@ -209,12 +211,8 @@ case class GroupController @Inject() (
Form(
mapping(
"id" -> ignored(UUID.randomUUID()),
"name" -> text(maxLength = UserGroup.nameMaxLength)
.transform[String](commonStringInputNormalization, commonStringInputNormalization),
"description" -> optional(text).transform[Option[String]](
_.map(commonStringInputNormalization).filter(_.nonEmpty),
_.map(commonStringInputNormalization).filter(_.nonEmpty)
),
"name" -> normalizedText.verifying(maxLength(UserGroup.nameMaxLength)),
"description" -> normalizedOptionalText,
"insee-code" -> list(text),
"creationDate" -> ignored(ZonedDateTime.now(timeZone)),
"area-ids" -> list(uuid)
Expand All @@ -225,14 +223,8 @@ case class GroupController @Inject() (
.verifying("Vous devez sélectionner au moins 1 territoire", _.nonEmpty),
"organisation" -> optional(of[Organisation.Id]),
"email" -> optional(email),
"publicNote" -> optional(text).transform[Option[String]](
_.map(commonStringInputNormalization).filter(_.nonEmpty),
_.map(commonStringInputNormalization).filter(_.nonEmpty)
),
"internalSupportComment" -> optional(text).transform[Option[String]](
_.map(commonStringInputNormalization).filter(_.nonEmpty),
_.map(commonStringInputNormalization).filter(_.nonEmpty)
)
"publicNote" -> normalizedOptionalText,
"internalSupportComment" -> normalizedOptionalText
)(UserGroup.apply)(UserGroup.unapply)
)

Expand Down
Loading

0 comments on commit eae174f

Please sign in to comment.