From e31107eb49dc812f415dda7f72ef28850bef6757 Mon Sep 17 00:00:00 2001
From: eikek <eike.kettner@posteo.de>
Date: Wed, 22 Sep 2021 23:06:59 +0200
Subject: [PATCH] Require a otp to disable 2fa

---
 .../scala/docspell/backend/ops/OTotp.scala    | 39 ++++++++++++++--
 .../src/main/resources/docspell-openapi.yml   | 11 ++++-
 .../restserver/conv/Conversions.scala         |  2 +-
 .../restserver/routes/TotpRoutes.scala        | 10 ++--
 modules/webapp/src/main/elm/Api.elm           |  6 +--
 modules/webapp/src/main/elm/Comp/OtpSetup.elm | 46 ++++++++++---------
 .../src/main/elm/Messages/Comp/OtpSetup.elm   | 11 ++---
 7 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/modules/backend/src/main/scala/docspell/backend/ops/OTotp.scala b/modules/backend/src/main/scala/docspell/backend/ops/OTotp.scala
index e0842b94..a872fa53 100644
--- a/modules/backend/src/main/scala/docspell/backend/ops/OTotp.scala
+++ b/modules/backend/src/main/scala/docspell/backend/ops/OTotp.scala
@@ -18,13 +18,23 @@ import org.log4s.getLogger
 
 trait OTotp[F[_]] {
 
+  /** Return whether TOTP is enabled for this account or not. */
   def state(accountId: AccountId): F[OtpState]
 
+  /** Initializes TOTP by generating a secret and storing it in the database. TOTP is
+    * still disabled, it must be confirmed in order to be active.
+    */
   def initialize(accountId: AccountId): F[InitResult]
 
+  /** Confirms and finishes initialization. TOTP is active after this for the given
+    * account.
+    */
   def confirmInit(accountId: AccountId, otp: OnetimePassword): F[ConfirmResult]
 
-  def disable(accountId: AccountId): F[UpdateResult]
+  /** Disables TOTP and removes the shared secret. If a otp is specified, it must be
+    * valid.
+    */
+  def disable(accountId: AccountId, otp: Option[OnetimePassword]): F[UpdateResult]
 }
 
 object OTotp {
@@ -133,8 +143,31 @@ object OTotp {
           }
         } yield res
 
-      def disable(accountId: AccountId): F[UpdateResult] =
-        UpdateResult.fromUpdate(store.transact(RTotp.setEnabled(accountId, false)))
+      def disable(accountId: AccountId, otp: Option[OnetimePassword]): F[UpdateResult] =
+        otp match {
+          case Some(pw) =>
+            for {
+              _ <- log.info(s"Validating TOTP, because it is requested to disable it.")
+              key <- store.transact(RTotp.findEnabledByLogin(accountId, true))
+              now <- Timestamp.current[F]
+              res <- key match {
+                case None =>
+                  UpdateResult.failure(new Exception("TOTP not enabled.")).pure[F]
+                case Some(r) =>
+                  val check = totp.checkPassword(r.secret, pw, now.value)
+                  if (check)
+                    UpdateResult.fromUpdate(
+                      store.transact(RTotp.setEnabled(accountId, false))
+                    )
+                  else
+                    log.info(s"TOTP code was invalid. Not disabling it.") *> UpdateResult
+                      .failure(new Exception("Code invalid!"))
+                      .pure[F]
+              }
+            } yield res
+          case None =>
+            UpdateResult.fromUpdate(store.transact(RTotp.setEnabled(accountId, false)))
+        }
 
       def state(accountId: AccountId): F[OtpState] =
         for {
diff --git a/modules/restapi/src/main/resources/docspell-openapi.yml b/modules/restapi/src/main/resources/docspell-openapi.yml
index 34e60298..3ebfaa27 100644
--- a/modules/restapi/src/main/resources/docspell-openapi.yml
+++ b/modules/restapi/src/main/resources/docspell-openapi.yml
@@ -1442,11 +1442,18 @@ paths:
       summary: Disables two factor authentication.
       description: |
         Disables two factor authentication for the current user. If
-        the user has no two factor authentication enabled, this
-        returns success, too.
+        the user has no two factor authentication enabled, an error is
+        returned.
+
+        It requires to specify a valid otp.
 
         After this completes successfully, two factor auth can be
         enabled again by initializing it anew.
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: "#/components/schemas/OtpConfirm"
       responses:
         200:
           description: Ok
diff --git a/modules/restserver/src/main/scala/docspell/restserver/conv/Conversions.scala b/modules/restserver/src/main/scala/docspell/restserver/conv/Conversions.scala
index 48ea6526..a893dd1d 100644
--- a/modules/restserver/src/main/scala/docspell/restserver/conv/Conversions.scala
+++ b/modules/restserver/src/main/scala/docspell/restserver/conv/Conversions.scala
@@ -695,7 +695,7 @@ trait Conversions {
       case UpdateResult.Success  => BasicResult(true, successMsg)
       case UpdateResult.NotFound => BasicResult(false, "Not found")
       case UpdateResult.Failure(ex) =>
-        BasicResult(false, s"Internal error: ${ex.getMessage}")
+        BasicResult(false, s"Error: ${ex.getMessage}")
     }
 
   def basicResult(ur: OUpload.UploadResult): BasicResult =
diff --git a/modules/restserver/src/main/scala/docspell/restserver/routes/TotpRoutes.scala b/modules/restserver/src/main/scala/docspell/restserver/routes/TotpRoutes.scala
index 255ce2c2..9b1838eb 100644
--- a/modules/restserver/src/main/scala/docspell/restserver/routes/TotpRoutes.scala
+++ b/modules/restserver/src/main/scala/docspell/restserver/routes/TotpRoutes.scala
@@ -68,9 +68,13 @@ object TotpRoutes {
           }
         } yield resp
 
-      case POST -> Root / "disable" =>
+      case req @ POST -> Root / "disable" =>
         for {
-          result <- backend.totp.disable(user.account)
+          data <- req.as[OtpConfirm]
+          result <- backend.totp.disable(
+            user.account,
+            OnetimePassword(data.otp.pass).some
+          )
           resp <- Ok(Conversions.basicResult(result, "TOTP setup disabled."))
         } yield resp
     }
@@ -83,7 +87,7 @@ object TotpRoutes {
     HttpRoutes.of { case req @ POST -> Root / "resetOTP" =>
       for {
         data <- req.as[ResetPassword]
-        result <- backend.totp.disable(data.account)
+        result <- backend.totp.disable(data.account, None)
         resp <- Ok(Conversions.basicResult(result, "TOTP setup disabled."))
       } yield resp
     }
diff --git a/modules/webapp/src/main/elm/Api.elm b/modules/webapp/src/main/elm/Api.elm
index 531125a6..5ae8945d 100644
--- a/modules/webapp/src/main/elm/Api.elm
+++ b/modules/webapp/src/main/elm/Api.elm
@@ -2195,12 +2195,12 @@ confirmOtp flags confirm receive =
         }
 
 
-disableOtp : Flags -> (Result Http.Error BasicResult -> msg) -> Cmd msg
-disableOtp flags receive =
+disableOtp : Flags -> OtpConfirm -> (Result Http.Error BasicResult -> msg) -> Cmd msg
+disableOtp flags otp receive =
     Http2.authPost
         { url = flags.config.baseUrl ++ "/api/v1/sec/user/otp/disable"
         , account = getAccount flags
-        , body = Http.emptyBody
+        , body = Http.jsonBody (Api.Model.OtpConfirm.encode otp)
         , expect = Http.expectJson receive Api.Model.BasicResult.decoder
         }
 
diff --git a/modules/webapp/src/main/elm/Comp/OtpSetup.elm b/modules/webapp/src/main/elm/Comp/OtpSetup.elm
index b453b096..46db0f75 100644
--- a/modules/webapp/src/main/elm/Comp/OtpSetup.elm
+++ b/modules/webapp/src/main/elm/Comp/OtpSetup.elm
@@ -58,8 +58,8 @@ initDisabledModel =
 type alias EnabledModel =
     { created : Int
     , loading : Bool
-    , confirmText : String
-    , confirmTextWrong : Bool
+    , confirmCode : String
+    , serverErrorMsg : String
     }
 
 
@@ -67,8 +67,8 @@ initEnabledModel : Int -> EnabledModel
 initEnabledModel created =
     { created = created
     , loading = False
-    , confirmText = ""
-    , confirmTextWrong = False
+    , confirmCode = ""
+    , serverErrorMsg = ""
     }
 
 
@@ -85,7 +85,7 @@ type Msg
     | SecretMsg Comp.PasswordInput.Msg
     | Confirm
     | ConfirmResp (Result Http.Error BasicResult)
-    | SetDisableConfirmText String
+    | SetDisableConfirmCode String
     | Disable
     | DisableResp (Result Http.Error BasicResult)
 
@@ -178,10 +178,10 @@ update flags msg model =
         ConfirmResp (Err err) ->
             ( ConfirmError err, Cmd.none )
 
-        SetDisableConfirmText str ->
+        SetDisableConfirmCode str ->
             case model of
                 StateEnabled m ->
-                    ( StateEnabled { m | confirmText = str }, Cmd.none )
+                    ( StateEnabled { m | confirmCode = str }, Cmd.none )
 
                 _ ->
                     ( model, Cmd.none )
@@ -189,13 +189,9 @@ update flags msg model =
         Disable ->
             case model of
                 StateEnabled m ->
-                    if String.toLower m.confirmText == "ok" then
-                        ( StateEnabled { m | confirmTextWrong = False, loading = True }
-                        , Api.disableOtp flags DisableResp
-                        )
-
-                    else
-                        ( StateEnabled { m | confirmTextWrong = True }, Cmd.none )
+                    ( StateEnabled { m | loading = True }
+                    , Api.disableOtp flags (OtpConfirm m.confirmCode) DisableResp
+                    )
 
                 _ ->
                     ( model, Cmd.none )
@@ -205,7 +201,12 @@ update flags msg model =
                 init flags
 
             else
-                ( model, Cmd.none )
+                case model of
+                    StateEnabled m ->
+                        ( StateEnabled { m | serverErrorMsg = result.message, loading = False }, Cmd.none )
+
+                    _ ->
+                        ( model, Cmd.none )
 
         DisableResp (Err err) ->
             ( DisableError err, Cmd.none )
@@ -253,14 +254,15 @@ viewEnabled texts model =
         , p []
             [ text texts.revert2FAText
             ]
-        , div [ class "flex flex-col items-center mt-6" ]
+        , div [ class "flex flex-col mt-6" ]
             [ div [ class "flex flex-row max-w-md" ]
                 [ input
                     [ type_ "text"
-                    , value model.confirmText
-                    , onInput SetDisableConfirmText
+                    , value model.confirmCode
+                    , onInput SetDisableConfirmCode
                     , class S.textInput
-                    , class "rounded-r-none"
+                    , class "rounded-r-none pl-2 pr-10 py-2 rounded-lg max-w-xs text-center font-mono"
+                    , placeholder "123456"
                     ]
                     []
                 , B.genericButton
@@ -281,9 +283,9 @@ viewEnabled texts model =
             , div
                 [ class S.errorMessage
                 , class "my-2"
-                , classList [ ( "hidden", not model.confirmTextWrong ) ]
+                , classList [ ( "hidden", model.serverErrorMsg == "" ) ]
                 ]
-                [ text texts.disableConfirmErrorMsg
+                [ text texts.codeInvalid
                 ]
             , Markdown.toHtml [ class "mt-2" ] texts.disableConfirmBoxInfo
             ]
@@ -367,7 +369,7 @@ viewDisabled texts model =
                             , class S.errorMessage
                             , class "mt-2"
                             ]
-                            [ text texts.setupCodeInvalid ]
+                            [ text texts.codeInvalid ]
                         , div [ class "mt-6" ]
                             [ p [] [ text texts.ifNotQRCode ]
                             , div [ class "max-w-md mx-auto mt-4" ]
diff --git a/modules/webapp/src/main/elm/Messages/Comp/OtpSetup.elm b/modules/webapp/src/main/elm/Messages/Comp/OtpSetup.elm
index 2378301f..3c4112d8 100644
--- a/modules/webapp/src/main/elm/Messages/Comp/OtpSetup.elm
+++ b/modules/webapp/src/main/elm/Messages/Comp/OtpSetup.elm
@@ -29,14 +29,13 @@ type alias Texts =
     , twoFaActiveSince : String
     , revert2FAText : String
     , disableButton : String
-    , disableConfirmErrorMsg : String
     , disableConfirmBoxInfo : String
     , setupTwoFactorAuth : String
     , setupTwoFactorAuthInfo : String
     , activateButton : String
     , setupConfirmLabel : String
     , scanQRCode : String
-    , setupCodeInvalid : String
+    , codeInvalid : String
     , ifNotQRCode : String
     , reloadToTryAgain : String
     , twoFactorNowActive : String
@@ -57,14 +56,13 @@ gb =
     , twoFaActiveSince = "Two Factor Authentication is active since "
     , revert2FAText = "If you really want to revert back to password-only authentication, you can do this here. You can run the setup any time to enable the second factor again."
     , disableButton = "Disable 2FA"
-    , disableConfirmErrorMsg = "Please type OK if you really want to disable this!"
-    , disableConfirmBoxInfo = "Type `OK` into the text box and click the button to disable 2FA."
+    , disableConfirmBoxInfo = "Enter a TOTP code and click the button to disable 2FA."
     , setupTwoFactorAuth = "Setup Two Factor Authentication"
     , setupTwoFactorAuthInfo = "You can setup a second factor for authentication using a one-time password. When clicking the button a secret is generated that you can load into an app on your mobile device. The app then provides a 6 digit code that you need to pass in the field in order to confirm and finalize the setup."
     , activateButton = "Activate two-factor authentication"
     , setupConfirmLabel = "Confirm"
     , scanQRCode = "Scan this QR code with your device and enter the 6 digit code:"
-    , setupCodeInvalid = "The confirmation code was invalid!"
+    , codeInvalid = "The code was invalid!"
     , ifNotQRCode = "If you cannot use the qr code, enter this secret:"
     , reloadToTryAgain = "If you want to try again, reload the page."
     , twoFactorNowActive = "Two Factor Authentication is now active!"
@@ -85,14 +83,13 @@ de =
     , twoFaActiveSince = "Die Zwei-Faktor-Authentifizierung ist aktiv seit "
     , revert2FAText = "Die Zwei-Faktor-Authentifizierung kann hier wieder deaktiviert werden. Danach kann die Einrichtung wieder von neuem gestartet werden, um 2FA wieder zu aktivieren."
     , disableButton = "Deaktiviere 2FA"
-    , disableConfirmErrorMsg = "Bitte tippe OK ein, um die Zwei-Faktor-Authentifizierung zu deaktivieren."
     , disableConfirmBoxInfo = "Tippe `OK` in das Feld und klicke, um die Zwei-Faktor-Authentifizierung zu deaktivieren."
     , setupTwoFactorAuth = "Zwei-Faktor-Authentifizierung einrichten"
     , setupTwoFactorAuthInfo = "Ein zweiter Faktor zur Authentifizierung mittels eines Einmalkennworts kann eingerichtet werden. Beim Klicken des Button wird ein Schlüssel generiert, der an eine Authentifizierungs-App eines mobilen Gerätes übetragen werden kann. Danach präsentiert die App ein 6-stelliges Kennwort, welches zur Bestätigung und zum Abschluss angegeben werden muss."
     , activateButton = "Zwei-Faktor-Authentifizierung aktivieren"
     , setupConfirmLabel = "Bestätigung"
     , scanQRCode = "Scanne den QR Code mit der Authentifizierungs-App und gebe den 6-stelligen Code ein:"
-    , setupCodeInvalid = "Der Code war ungültig!"
+    , codeInvalid = "Der Code war ungültig!"
     , ifNotQRCode = "Wenn der QR-Code nicht möglich ist, kann der Schlüssel manuell eingegeben werden:"
     , reloadToTryAgain = "Um es noch einmal zu versuchen, bitte die Seite neu laden."
     , twoFactorNowActive = "Die Zwei-Faktor-Authentifizierung ist nun aktiv!"