Skip to content

Commit

Permalink
Merge pull request #1760 from betagouv/master
Browse files Browse the repository at this point in the history
MEP remove Fingerprint check on cookies (not reliable)
  • Loading branch information
eletallbetagouv authored Oct 22, 2024
2 parents 12ba3b7 + 43a2701 commit e081a4c
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 87 deletions.
32 changes: 6 additions & 26 deletions app/authentication/CookieAuthenticator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import scala.concurrent.Future
class CookieAuthenticator(
signer: JcaSigner,
crypter: JcaCrypter,
fingerprintGenerator: FingerprintGenerator,
settings: CookieAuthenticatorSettings,
userRepository: UserRepositoryInterface
)(implicit ec: ExecutionContext)
Expand All @@ -37,26 +36,12 @@ class CookieAuthenticator(
.map(_ => BrokenAuthError("Error while extracting data from cookie"))
} yield cookiesInfos

def extract[B](request: Request[B]): Either[BrokenAuthError, CookieInfos] = {
val maybeFingerprint = if (settings.useFingerprinting) Some(fingerprintGenerator.generate(request)) else None
val maybeCookiesInfos = request.cookies.get(settings.cookieName) match {
def extract[B](request: Request[B]): Either[BrokenAuthError, CookieInfos] =
request.cookies.get(settings.cookieName) match {
case Some(cookie) => unserialize(cookie.value)
case None => Left(BrokenAuthError("Cookie not found"))
}

maybeCookiesInfos match {
case Right(a) if maybeFingerprint.isDefined && a.fingerprint != maybeFingerprint =>
Left(
BrokenAuthError(
s"Fingerprint does not match : ${a.fingerprint} vs $maybeFingerprint. Based on headers values : ${fingerprintGenerator
.readHeadersValues(request)}",
Some("Fingerprint does not match")
)
)
case v => v
}
}

def authenticate[B](request: Request[B]): Future[Either[BrokenAuthError, Option[User]]] =
extract(request) match {
case Right(cookieInfos) =>
Expand All @@ -71,9 +56,7 @@ class CookieAuthenticator(
crypted <- crypter.encrypt(Json.toJson(cookieInfos).toString())
} yield signer.sign(crypted)

private def create(userEmail: EmailAddress, impersonator: Option[EmailAddress] = None)(implicit
request: RequestHeader
): CookieInfos = {
private def create(userEmail: EmailAddress, impersonator: Option[EmailAddress] = None): CookieInfos = {
val now = OffsetDateTime.now()
CookieInfos(
id = UUID.randomUUID().toString,
Expand All @@ -82,12 +65,11 @@ class CookieAuthenticator(
lastUsedDateTime = now,
expirationDateTime = now.plus(settings.authenticatorExpiry.toMillis, ChronoUnit.MILLIS),
idleTimeout = settings.authenticatorIdleTimeout,
cookieMaxAge = settings.cookieMaxAge,
fingerprint = if (settings.useFingerprinting) Some(fingerprintGenerator.generate(request)) else None
cookieMaxAge = settings.cookieMaxAge
)
}

def init(userEmail: EmailAddress)(implicit request: RequestHeader): Either[BrokenAuthError, Cookie] = {
def init(userEmail: EmailAddress): Either[BrokenAuthError, Cookie] = {
val cookieInfos = create(userEmail)
serialize(cookieInfos).map { value =>
Cookie(
Expand All @@ -105,9 +87,7 @@ class CookieAuthenticator(
}
}

def initImpersonated(userEmail: EmailAddress, impersonator: EmailAddress)(implicit
request: RequestHeader
): Either[BrokenAuthError, Cookie] = {
def initImpersonated(userEmail: EmailAddress, impersonator: EmailAddress): Either[BrokenAuthError, Cookie] = {
val cookieInfos = create(userEmail, Some(impersonator))
serialize(cookieInfos).map { value =>
Cookie(
Expand Down
3 changes: 1 addition & 2 deletions app/authentication/CookieInfos.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ case class CookieInfos(
lastUsedDateTime: OffsetDateTime,
expirationDateTime: OffsetDateTime,
idleTimeout: Option[FiniteDuration],
cookieMaxAge: Option[FiniteDuration],
fingerprint: Option[String]
cookieMaxAge: Option[FiniteDuration]
)

object CookieInfos {
Expand Down
45 changes: 0 additions & 45 deletions app/authentication/FingerprintGenerator.scala

This file was deleted.

4 changes: 2 additions & 2 deletions app/controllers/AuthController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class AuthController(
def authenticate: Action[JsValue] = IpRateLimitedAction2.async(parse.json) { implicit request =>
for {
userLogin <- request.parseBody[UserCredentials]()
userSession <- authOrchestrator.login(userLogin, request)
userSession <- authOrchestrator.login(userLogin)
} yield authenticator.embed(userSession.cookie, Ok(Json.toJson(userSession.user)))
}

Expand All @@ -52,7 +52,7 @@ class AuthController(
request.identity.impersonator match {
case Some(impersonator) =>
authOrchestrator
.logoutAs(impersonator, request)
.logoutAs(impersonator)
.map(userSession => authenticator.embed(userSession.cookie, Ok(Json.toJson(userSession.user))))
case None => Future.successful(authenticator.discard(NoContent))
}
Expand Down
7 changes: 3 additions & 4 deletions app/loader/SignalConsoApplicationLoader.scala
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,11 @@ class SignalConsoComponents(

val ipBlackListRepository = new IpBlackListRepository(dbConfig)

val crypter = new JcaCrypter(applicationConfiguration.crypter)
val signer = new JcaSigner(applicationConfiguration.signer)
val fingerprintGenerator = new FingerprintGenerator()
val crypter = new JcaCrypter(applicationConfiguration.crypter)
val signer = new JcaSigner(applicationConfiguration.signer)

val cookieAuthenticator =
new CookieAuthenticator(signer, crypter, fingerprintGenerator, applicationConfiguration.cookie, userRepository)
new CookieAuthenticator(signer, crypter, applicationConfiguration.cookie, userRepository)
val apiKeyAuthenticator = new APIKeyAuthenticator(passwordHasherRegistry, consumerRepository)

val credentialsProvider = new CredentialsProvider(passwordHasherRegistry, userRepository)
Expand Down
13 changes: 6 additions & 7 deletions app/orchestrators/AuthOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import orchestrators.AuthOrchestrator.MaxAllowedAuthAttempts
import orchestrators.AuthOrchestrator.authTokenExpiration
import play.api.Logger
import play.api.mvc.Cookie
import play.api.mvc.Request
import repositories.authattempt.AuthAttemptRepositoryInterface
import repositories.authtoken.AuthTokenRepositoryInterface
import repositories.user.UserRepositoryInterface
Expand Down Expand Up @@ -72,7 +71,7 @@ class AuthOrchestrator(
Future.successful(())
}

def login(userLogin: UserCredentials, request: Request[_]): Future[UserSession] = {
def login(userLogin: UserCredentials): Future[UserSession] = {
logger.debug(s"Validate auth attempts count")
val eventualUserSession: Future[UserSession] = for {
_ <- validateAuthenticationAttempts(userLogin.login)
Expand All @@ -83,7 +82,7 @@ class AuthOrchestrator(
_ = logger.debug(s"Check last validation email for DGCCRF users")
_ <- validateAgentAccountLastEmailValidation(user)
_ = logger.debug(s"Successful login for user")
cookie <- getCookie(userLogin)(request)
cookie <- getCookie(userLogin)
_ = logger.debug(s"Successful generated token for user")
} yield UserSession(cookie, user)

Expand Down Expand Up @@ -121,16 +120,16 @@ class AuthOrchestrator(
case UserRole.Professionnel => Future.unit
case _ => Future.failed(BrokenAuthError("Not a pro"))
}
cookie <- authenticator.initImpersonated(userEmail, request.identity.email)(request) match {
cookie <- authenticator.initImpersonated(userEmail, request.identity.email) match {
case Right(value) => Future.successful(value)
case Left(error) => Future.failed(error)
}
} yield UserSession(cookie, userToImpersonate.copy(impersonator = Some(request.identity.email)))

def logoutAs(userEmail: EmailAddress, request: Request[_]) = for {
def logoutAs(userEmail: EmailAddress) = for {
maybeUser <- userRepository.findByEmail(userEmail.value)
user <- maybeUser.liftTo[Future](UserNotFound(userEmail.value))
cookie <- authenticator.init(userEmail)(request) match {
cookie <- authenticator.init(userEmail) match {
case Right(value) => Future.successful(value)
case Left(error) => Future.failed(error)
}
Expand Down Expand Up @@ -182,7 +181,7 @@ class AuthOrchestrator(
_ = logger.debug(s"Password updated for user id ${user.id}")
} yield ()

private def getCookie(userLogin: UserCredentials)(implicit req: Request[_]): Future[Cookie] =
private def getCookie(userLogin: UserCredentials): Future[Cookie] =
for {
_ <- credentialsProvider.authenticate(userLogin.login, userLogin.password)
cookie <- authenticator.init(EmailAddress(userLogin.login)) match {
Expand Down
2 changes: 1 addition & 1 deletion test/utils/AuthHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import play.api.test.FakeRequest
object AuthHelpers {
implicit class FakeRequestOps[A](request: FakeRequest[A]) {
def withAuthCookie(userEmail: EmailAddress, cookieAuthenticator: CookieAuthenticator): FakeRequest[A] = {
val cookie = cookieAuthenticator.init(userEmail)(request).toOption.get
val cookie = cookieAuthenticator.init(userEmail).toOption.get
request.withCookies(cookie)
}
}
Expand Down

0 comments on commit e081a4c

Please sign in to comment.