From bebb00aa2e416c21f37bab9bedc4a1bb2c271936 Mon Sep 17 00:00:00 2001
From: Valentin Tolmer <valentin@tolmer.fr>
Date: Wed, 15 Feb 2023 14:21:51 +0100
Subject: [PATCH] app: improve error message for wrong/expired reset token

---
 app/src/components/app.rs                  | 120 ++++++++++-----------
 app/src/components/reset_password_step2.rs |  16 ++-
 server/src/infra/auth_service.rs           |   8 +-
 server/src/infra/tcp_server.rs             |   3 +
 4 files changed, 78 insertions(+), 69 deletions(-)

diff --git a/app/src/components/app.rs b/app/src/components/app.rs
index a41346f..e20fed7 100644
--- a/app/src/components/app.rs
+++ b/app/src/components/app.rs
@@ -32,7 +32,7 @@ pub struct App {
     user_info: Option<(String, bool)>,
     redirect_to: Option<AppRoute>,
     route_dispatcher: RouteAgentDispatcher,
-    password_reset_enabled: bool,
+    password_reset_enabled: Option<bool>,
     task: Option<FetchTask>,
 }
 
@@ -64,7 +64,7 @@ impl Component for App {
                 }),
             redirect_to: Self::get_redirect_route(),
             route_dispatcher: RouteAgentDispatcher::new(),
-            password_reset_enabled: false,
+            password_reset_enabled: None,
             task: None,
         };
         app.task = Some(
@@ -95,22 +95,21 @@ impl Component for App {
             Msg::Logout => {
                 self.user_info = None;
                 self.redirect_to = None;
+                self.route_dispatcher
+                    .send(RouteRequest::ReplaceRoute(Route::from(AppRoute::Login)));
             }
             Msg::PasswordResetProbeFinished(Ok(enabled)) => {
                 self.task = None;
-                self.password_reset_enabled = enabled;
+                self.password_reset_enabled = Some(enabled);
             }
             Msg::PasswordResetProbeFinished(Err(err)) => {
                 self.task = None;
+                self.password_reset_enabled = Some(false);
                 ConsoleService::error(&format!(
                     "Could not probe for password reset support: {err:#}"
                 ));
             }
         }
-        if self.user_info.is_none() {
-            self.route_dispatcher
-                .send(RouteRequest::ReplaceRoute(Route::from(AppRoute::Login)));
-        }
         true
     }
 
@@ -160,7 +159,7 @@ impl App {
         let route_service = RouteService::<()>::new();
         let current_route = route_service.get_path();
         if current_route.contains("reset-password") {
-            if !self.password_reset_enabled {
+            if self.password_reset_enabled == Some(false) {
                 self.route_dispatcher
                     .send(RouteRequest::ReplaceRoute(Route::from(AppRoute::Login)));
             }
@@ -195,11 +194,11 @@ impl App {
         switch: AppRoute,
         link: &ComponentLink<Self>,
         is_admin: bool,
-        password_reset_enabled: bool,
+        password_reset_enabled: Option<bool>,
     ) -> Html {
         match switch {
             AppRoute::Login => html! {
-                <LoginForm on_logged_in=link.callback(Msg::Login) password_reset_enabled=password_reset_enabled/>
+                <LoginForm on_logged_in=link.callback(Msg::Login) password_reset_enabled=password_reset_enabled.unwrap_or(false)/>
             },
             AppRoute::CreateUser => html! {
                 <CreateUserForm/>
@@ -234,23 +233,20 @@ impl App {
             AppRoute::ChangePassword(username) => html! {
                 <ChangePasswordForm username=username is_admin=is_admin />
             },
-            AppRoute::StartResetPassword => {
-                if password_reset_enabled {
-                    html! {
-                      <ResetPasswordStep1Form />
-                    }
-                } else {
+            AppRoute::StartResetPassword => match password_reset_enabled {
+                Some(true) => html! { <ResetPasswordStep1Form /> },
+                Some(false) => {
                     App::dispatch_route(AppRoute::Login, link, is_admin, password_reset_enabled)
                 }
-            }
-            AppRoute::FinishResetPassword(token) => html! {
-                if password_reset_enabled {
-                    html! {
-                      <ResetPasswordStep2Form token=token />
-                    }
-                } else {
+
+                None => html! {},
+            },
+            AppRoute::FinishResetPassword(token) => match password_reset_enabled {
+                Some(true) => html! { <ResetPasswordStep2Form token=token /> },
+                Some(false) => {
                     App::dispatch_route(AppRoute::Login, link, is_admin, password_reset_enabled)
                 }
+                None => html! {},
             },
         }
     }
@@ -287,50 +283,48 @@ impl App {
                   } } else { html!{} } }
                 </ul>
 
-                <div class="dropdown text-end">
-                  <a href="#"
-                    class="d-block link-dark text-decoration-none dropdown-toggle"
-                    id="dropdownUser"
-                    data-bs-toggle="dropdown"
-                    aria-expanded="false">
-                    <svg xmlns="http://www.w3.org/2000/svg"
-                      width="32"
-                      height="32"
-                      fill="currentColor"
-                      class="bi bi-person-circle"
-                      viewBox="0 0 16 16">
-                      <path d="M11 6a3 3 0 1 1-6 0 3 3 0 0 1 6 0z"/>
-                      <path fill-rule="evenodd" d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8zm8-7a7 7 0 0 0-5.468 11.37C3.242 11.226 4.805 10 8 10s4.757 1.225 5.468 2.37A7 7 0 0 0 8 1z"/>
-                    </svg>
-                    {
-                      if let Some((user_id, _)) = &self.user_info {
-                        html! {
+                {
+                  if let Some((user_id, _)) = &self.user_info {
+                    html! {
+                      <div class="dropdown text-end">
+                        <a href="#"
+                          class="d-block link-dark text-decoration-none dropdown-toggle"
+                          id="dropdownUser"
+                          data-bs-toggle="dropdown"
+                          aria-expanded="false">
+                          <svg xmlns="http://www.w3.org/2000/svg"
+                            width="32"
+                            height="32"
+                            fill="currentColor"
+                            class="bi bi-person-circle"
+                            viewBox="0 0 16 16">
+                            <path d="M11 6a3 3 0 1 1-6 0 3 3 0 0 1 6 0z"/>
+                            <path fill-rule="evenodd" d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8zm8-7a7 7 0 0 0-5.468 11.37C3.242 11.226 4.805 10 8 10s4.757 1.225 5.468 2.37A7 7 0 0 0 8 1z"/>
+                          </svg>
                           <span class="ms-2">
                             {user_id}
                           </span>
-                        }
-                      } else { html!{} }
+                        </a>
+                        <ul
+                          class="dropdown-menu text-small dropdown-menu-lg-end"
+                          aria-labelledby="dropdownUser1"
+                          style="">
+                          <li>
+                            <Link
+                              classes="dropdown-item"
+                              route=AppRoute::UserDetails(user_id.clone())>
+                              {"View details"}
+                            </Link>
+                          </li>
+                          <li><hr class="dropdown-divider" /></li>
+                          <li>
+                            <LogoutButton on_logged_out=self.link.callback(|_| Msg::Logout) />
+                          </li>
+                        </ul>
+                      </div>
                     }
-                  </a>
-                  {if let Some((user_id, _)) = &self.user_info { html! {
-                    <ul
-                      class="dropdown-menu text-small dropdown-menu-lg-end"
-                      aria-labelledby="dropdownUser1"
-                      style="">
-                      <li>
-                        <Link
-                          classes="dropdown-item"
-                          route=AppRoute::UserDetails(user_id.clone())>
-                          {"View details"}
-                        </Link>
-                      </li>
-                      <li><hr class="dropdown-divider" /></li>
-                      <li>
-                        <LogoutButton on_logged_out=self.link.callback(|_| Msg::Logout) />
-                      </li>
-                    </ul>
-                  } } else { html!{} } }
-                </div>
+                  } else { html!{} }
+                }
               </div>
             </div>
           </header>
diff --git a/app/src/components/reset_password_step2.rs b/app/src/components/reset_password_step2.rs
index a14b36b..c1cdeba 100644
--- a/app/src/components/reset_password_step2.rs
+++ b/app/src/components/reset_password_step2.rs
@@ -1,5 +1,5 @@
 use crate::{
-    components::router::AppRoute,
+    components::router::{AppRoute, NavButton},
     infra::{
         api::HostService,
         common_component::{CommonComponent, CommonComponentParts},
@@ -158,9 +158,17 @@ impl Component for ResetPasswordStep2Form {
             }
             (None, Some(e)) => {
                 return html! {
-                  <div class="alert alert-danger">
-                    {e.to_string() }
-                  </div>
+                  <>
+                    <div class="alert alert-danger">
+                      {e.to_string() }
+                    </div>
+                    <NavButton
+                      classes="btn-link btn"
+                      disabled=self.common.is_task_running()
+                      route=AppRoute::Login>
+                      {"Back"}
+                    </NavButton>
+                  </>
                 }
             }
             _ => (),
diff --git a/server/src/infra/auth_service.rs b/server/src/infra/auth_service.rs
index ce7bc1b..5dc0cc1 100644
--- a/server/src/infra/auth_service.rs
+++ b/server/src/infra/auth_service.rs
@@ -214,11 +214,15 @@ where
     let token = request
         .match_info()
         .get("token")
-        .ok_or_else(|| TcpError::BadRequest("Missing reset token".to_string()))?;
+        .ok_or_else(|| TcpError::BadRequest("Missing reset token".to_owned()))?;
     let user_id = data
         .backend_handler
         .get_user_id_for_password_reset_token(token)
-        .await?;
+        .await
+        .map_err(|e| {
+            debug!("Reset token error: {e:#}");
+            TcpError::NotFoundError("Wrong or expired reset token".to_owned())
+        })?;
     let _ = data
         .backend_handler
         .delete_password_reset_token(token)
diff --git a/server/src/infra/tcp_server.rs b/server/src/infra/tcp_server.rs
index c27846d..166b65e 100644
--- a/server/src/infra/tcp_server.rs
+++ b/server/src/infra/tcp_server.rs
@@ -37,6 +37,8 @@ pub enum TcpError {
     BadRequest(String),
     #[error("Internal server error: `{0}`")]
     InternalServerError(String),
+    #[error("Not found: `{0}`")]
+    NotFoundError(String),
     #[error("Unauthorized: `{0}`")]
     UnauthorizedError(String),
 }
@@ -57,6 +59,7 @@ pub(crate) fn error_to_http_response(error: TcpError) -> HttpResponse {
             | DomainError::EntityNotFound(_) => HttpResponse::BadRequest(),
         },
         TcpError::BadRequest(_) => HttpResponse::BadRequest(),
+        TcpError::NotFoundError(_) => HttpResponse::NotFound(),
         TcpError::InternalServerError(_) => HttpResponse::InternalServerError(),
         TcpError::UnauthorizedError(_) => HttpResponse::Unauthorized(),
     }