Prevent timed side-channel attacks. Add get user endpoint

server/src/secrets.rs

@@ -1,7 +1,7 @@
 use std::error::Error;
 use std::fmt::{Debug, Display};
 
-use serde::{Deserialize, Deserializer};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use sqlx::database::HasValueRef;
 use sqlx::{Decode, Postgres};
 
@@ -77,6 +77,20 @@ where
     }
 }
 
+impl<T> Serialize for Secret<T>
+where
+    T: Serialize + Default + Clone + Debug,
+{
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        // We want to skip serializing the secret value
+        // when serializing to JSON
+        serializer.serialize_unit()
+    }
+}
+
 impl<T> From<T> for Secret<T>
 where
     T: Default + Clone,

server/src/users/mod.rs

@@ -1,5 +1,6 @@
 pub use models::*;
 
 mod models;
+mod routes;
 pub mod selectors;
 pub mod services;

server/src/users/models.rs

@@ -10,41 +10,51 @@ use oauth2::{
     AuthorizationCode, CsrfToken, TokenResponse,
 };
 use password_auth::verify_password;
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Deserializer, Serialize};
 use sqlx::types::time;
 use sqlx::{FromRow, PgPool};
 use tokio::task;
 
 use crate::secrets::Secret;
 
-#[derive(sqlx::FromRow, Clone, Deserialize, Debug)]
+#[derive(Clone, Serialize, Deserialize, FromRow)]
+pub struct UserOut {
+    pub user_id: uuid::Uuid,
+    pub username: String,
+}
+
+#[derive(sqlx::FromRow, Clone, Deserialize, Serialize, Debug)]
 pub struct User {
     pub user_id: uuid::Uuid,
     pub email: String,
     pub username: String,
-    #[sqlx(default)]
     pub password_hash: Secret<Option<String>>,
-    #[sqlx(default)]
     pub access_token: Secret<Option<String>>,
 
     pub created_at: time::OffsetDateTime,
     pub updated_at: Option<time::OffsetDateTime>,
 }
 
-#[derive(sqlx::Encode, Clone, Deserialize, Debug)]
-pub struct CreateUser {
+#[derive(Clone, Debug, Deserialize)]
+#[serde(remote = "Self")]
+pub struct CreateUserRequest {
     pub email: String,
     pub username: String,
-    #[sqlx(default)]
     pub password_hash: Secret<Option<String>>,
-    #[sqlx(default)]
     pub access_token: Secret<Option<String>>,
 }
 
-#[derive(Clone, Serialize, Deserialize, FromRow)]
-pub struct UserOut {
-    pub id: i64,
-    pub username: String,
+impl<'de> Deserialize<'de> for CreateUserRequest {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let s = Self::deserialize(deserializer)?;
+        if s.password_hash.expose().is_some() && s.access_token.expose().is_some() {
+            return Err(serde::de::Error::custom(
+                "should only have password or access token",
+            ));
+        }
+
+        Ok(s)
+    }
 }
 
 impl AuthUser for User {
@@ -145,21 +155,35 @@ impl AuthnBackend for PostgresBackend {
                 .await
                 .map_err(Self::Error::Sqlx)?;
 
-                // Verifying the password is blocking and potentially slow, so we'll do so via
+                // Verifying the password is blocking and potentially slow, so we have to do it via
                 // `spawn_blocking`.
                 task::spawn_blocking(move || {
-                    // We're using password-based authentication: this works by comparing our form
-                    // input with an argon2 password hash.
-                    Ok(user.filter(|user| {
-                        let Some(password) = user.password_hash.expose() else {
-                            return false;
-                        };
-                        verify_password(password_cred.password.expose(), password.as_ref()).is_ok()
-                    }))
+                    // password-based authentication. Compare our form  input with an argon2
+                    // password hash.
+                    // To prevent timed side-channel attacks, so we always compare the password
+                    // hash, even if the user doesn't exist.
+                    return match user {
+                        // If there is no user with this username we dummy verify the password.
+                        None => dummy_verify_password(password_cred.password.expose()),
+                        Some(user) => {
+                            // If the user exists, but has no password, we dummy verify the password.
+                            let Some(password) = user.password_hash.expose() else {
+                                return dummy_verify_password(password_cred.password.expose());
+                            };
+
+                            // If the user exists and has a password, we verify the password.
+                            match verify_password(
+                                password_cred.password.expose(),
+                                password.as_ref(),
+                            ) {
+                                Ok(_) => Ok(Some(user)),
+                                _ => Ok(None),
+                            }
+                        }
+                    };
                 })
                 .await?
             }
-
             Credentials::OAuth(oauth_creds) => {
                 // Ensure the CSRF state has not been tampered with.
                 if oauth_creds.old_state.secret() != oauth_creds.new_state.secret() {
@@ -220,6 +244,16 @@ impl AuthnBackend for PostgresBackend {
     }
 }
 
+// This is a dummy function to prevent timing attacks.
+fn dummy_verify_password(pw: &str) -> Result<Option<User>, BackendError> {
+    let _ = verify_password(
+        pw,
+        "smop-flurp-fjoing-Idoubtsomeonewouldeverusethispassword!",
+    );
+    // Even if the password is correct we still return `Ok(None)`.
+    Ok(None)
+}
+
 // We use a type alias for convenience.
 //
 // Note that we've supplied our concrete backend here.

server/src/users/routes.rs

@@ -0,0 +1,19 @@
+use axum::extract::State;
+use axum::routing::get;
+use axum::{Json, Router};
+use sqlx::PgPool;
+
+use crate::startup::AppState;
+use crate::users::selectors::get_user;
+use crate::users::User;
+
+async fn get_user_handler(
+    State(pool): State<PgPool>,
+    Json(user_id): Json<uuid::Uuid>,
+) -> crate::Result<Json<Option<User>>> {
+    Ok(Json(get_user(pool, user_id).await?))
+}
+
+pub fn routes() -> Router<AppState> {
+    Router::new().route("/user", get(get_user_handler))
+}

server/src/users/selectors.rs

@@ -1,5 +1,5 @@
-use sqlx::{Acquire, PgPool};
 use sqlx::types::uuid;
+use sqlx::{Acquire, PgPool};
 
 use crate::users::User;
 

server/src/users/services.rs

@@ -1,12 +1,12 @@
 use color_eyre::eyre::eyre;
 use sqlx::{Acquire, PgPool};
 
-use crate::users::{CreateUser, User};
+use crate::users::{CreateUserRequest, User};
 
 #[tracing::instrument(level = "debug", ret, err)]
 pub async fn create_user(
     pool: PgPool,
-    create_user: CreateUser,
+    create_user: CreateUserRequest,
 ) -> color_eyre::Result<Option<User>> {
     let mut conn = pool.acquire().await?;
 

server/tests/users.rs

@@ -2,18 +2,18 @@ use sqlx::PgPool;
 
 use server::users::selectors::get_user;
 use server::users::services::create_user;
-use server::users::CreateUser;
+use server::users::CreateUserRequest;
 
 #[sqlx::test]
-async fn get_users_test(pool: PgPool) -> sqlx::Result<()> {
+async fn get_and_create_users_test(pool: PgPool) -> sqlx::Result<()> {
     let user = get_user(pool.clone(), uuid::Uuid::nil()).await.unwrap();
     assert!(user.is_none());
 
     let new_user = create_user(
         pool.clone(),
-        CreateUser {
-            email: Default::default(),
-            username: Default::default(),
+        CreateUserRequest {
+            email: "test-email".to_string(),
+            username: "test-username".to_string(),
             password_hash: Some("password".to_string()).into(),
             access_token: Default::default(),
         },
@@ -22,8 +22,13 @@ async fn get_users_test(pool: PgPool) -> sqlx::Result<()> {
     .unwrap()
     .unwrap();
 
-    let user = get_user(pool.clone(), new_user.user_id).await.unwrap();
-    assert!(user.is_some());
+    let user = get_user(pool.clone(), new_user.user_id)
+        .await
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(user.user_id, new_user.user_id);
+    assert_eq!(user.email, new_user.email);
 
     Ok(())
 }