From 14643ae4d1ee11485909b676311fba871939f84c Mon Sep 17 00:00:00 2001 From: eikek Date: Fri, 4 Nov 2022 20:45:30 +0100 Subject: [PATCH] Improve error messages when using oidc with an existing account If an account was created before locally (by signing up at docspell) and the same account is later tried to signin via openid, a better error message is shown in the logs to be able to act on it. The user won't see any details in the webapp. Issue: #1827 #1781 --- .../src/main/scala/docspell/backend/auth/Login.scala | 10 +++++++++- .../main/scala/docspell/restserver/auth/OpenId.scala | 6 +++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/modules/backend/src/main/scala/docspell/backend/auth/Login.scala b/modules/backend/src/main/scala/docspell/backend/auth/Login.scala index 9d265280..f32e5640 100644 --- a/modules/backend/src/main/scala/docspell/backend/auth/Login.scala +++ b/modules/backend/src/main/scala/docspell/backend/auth/Login.scala @@ -73,8 +73,13 @@ object Login { case object InvalidAuth extends Result { val toEither = Left("Authentication failed.") } + case class InvalidAccountSource(account: AccountId) extends Result { + val toEither = Left( + s"The account '${account.asString}' already exists from a different source (local vs openid)!" + ) + } case object InvalidTime extends Result { - val toEither = Left("Authentication failed.") + val toEither = Left("Authentication failed due expired authenticator.") } case object InvalidFactor extends Result { val toEither = Left("Authentication requires second factor.") @@ -85,6 +90,7 @@ object Login { def invalidAuth: Result = InvalidAuth def invalidTime: Result = InvalidTime def invalidFactor: Result = InvalidFactor + def invalidAccountSource(account: AccountId): Result = InvalidAccountSource(account) } def apply[F[_]: Async](store: Store[F], totp: Totp): Resource[F, Login[F]] = @@ -99,6 +105,8 @@ object Login { res <- data match { case Some(d) if checkNoPassword(d, Set(AccountSource.OpenId)) => doLogin(config, d.account, false) + case Some(d) if checkNoPassword(d, Set(AccountSource.Local)) => + Result.invalidAccountSource(accountId).pure[F] case _ => Result.invalidAuth.pure[F] } diff --git a/modules/restserver/src/main/scala/docspell/restserver/auth/OpenId.scala b/modules/restserver/src/main/scala/docspell/restserver/auth/OpenId.scala index b65841ef..1045a055 100644 --- a/modules/restserver/src/main/scala/docspell/restserver/auth/OpenId.scala +++ b/modules/restserver/src/main/scala/docspell/restserver/auth/OpenId.scala @@ -105,6 +105,7 @@ object OpenId { import dsl._ for { + _ <- logger.debug(s"Setting up external account: ${accountId.asString}") setup <- backend.signup.setupExternal(ExternalAccount(accountId)) res <- setup match { case SignupResult.Failure(ex) => @@ -143,6 +144,7 @@ object OpenId { import dsl._ for { + _ <- logger.debug(s"Login and verify external account: ${accountId.asString}") login <- backend.login.loginExternal(config.auth)(accountId) resp <- login match { case Login.Result.Ok(session, _) => @@ -158,7 +160,9 @@ object OpenId { .map(_.addCookie(CookieData(session).asCookie(baseUrl))) case failed => - logger.error(s"External login failed: $failed") *> + logger.error( + s"External login failed: $failed. ${failed.toEither.left.getOrElse("")}" + ) *> SeeOther(location) } } yield resp