Wrap credential and credential-holding types to prevent `Debug` leaks.

The following values are considered confidential, and should never be logged, even by accident:

* `Password`, which is a durable bearer token for a specific Login;
* `IdentitySecret`, which is an ephemeral but potentially long-lived bearer token for a specific Login; or
* `IdentityToken`, which may hold cookies containing an `IdentitySecret`.

These values are now wrapped in types whose `Debug` impls output opaque values, so that they can be included in structs that `#[derive(Debug)]` without requiring any additional care. The wrappers also avoid implementing `Display`, to prevent inadvertent `to_string()`s.

We don't bother obfuscating `IdentitySecret`s in memory or in the `.hi` database. There's no point: we'd also need to store the information needed to de-obfuscate them, and they can be freely invalidated and replaced by blanking that table and asking everyone to log in again. Passwords _are_ obfuscated for storage, as they're intended to be durable.

10 files changed, 123 insertions, 48 deletions

diff --git a/src/login/app.rs b/src/login/app.rs
index 292b95f..f7fec88 100644
--- a/src/login/app.rs
+++ b/src/login/app.rs
@@ -1,10 +1,10 @@
 use chrono::TimeDelta;
 use sqlx::sqlite::SqlitePool;
 
-use super::repo::auth::Provider as _;
+use super::{extract::IdentitySecret, repo::auth::Provider as _};
 use crate::{
     clock::DateTime,
-    password::StoredHash,
+    password::Password,
     repo::{
         error::NotFound as _,
         login::{Login, Provider as _},
@@ -24,9 +24,9 @@ impl<'a> Logins<'a> {
     pub async fn login(
         &self,
         name: &str,
-        password: &str,
+        password: &Password,
         login_at: &DateTime,
-    ) -> Result<String, LoginError> {
+    ) -> Result<IdentitySecret, LoginError> {
         let mut tx = self.db.begin().await?;
 
         let login = if let Some((login, stored_hash)) = tx.auth().for_name(name).await? {
@@ -38,7 +38,7 @@ impl<'a> Logins<'a> {
                 return Err(LoginError::Rejected);
             }
         } else {
-            let password_hash = StoredHash::new(password)?;
+            let password_hash = password.hash()?;
             tx.logins().create(name, &password_hash).await?
         };
 
@@ -49,8 +49,8 @@ impl<'a> Logins<'a> {
     }
 
     #[cfg(test)]
-    pub async fn create(&self, name: &str, password: &str) -> Result<Login, CreateError> {
-        let password_hash = StoredHash::new(password)?;
+    pub async fn create(&self, name: &str, password: &Password) -> Result<Login, CreateError> {
+        let password_hash = password.hash()?;
 
         let mut tx = self.db.begin().await?;
         let login = tx.logins().create(name, &password_hash).await?;
@@ -59,7 +59,11 @@ impl<'a> Logins<'a> {
         Ok(login)
     }
 
-    pub async fn validate(&self, secret: &str, used_at: &DateTime) -> Result<Login, ValidateError> {
+    pub async fn validate(
+        &self,
+        secret: &IdentitySecret,
+        used_at: &DateTime,
+    ) -> Result<Login, ValidateError> {
         let mut tx = self.db.begin().await?;
         let login = tx
             .tokens()
@@ -82,7 +86,7 @@ impl<'a> Logins<'a> {
         Ok(())
     }
 
-    pub async fn logout(&self, secret: &str) -> Result<(), ValidateError> {
+    pub async fn logout(&self, secret: &IdentitySecret) -> Result<(), ValidateError> {
         let mut tx = self.db.begin().await?;
         tx.tokens()
             .revoke(secret)
diff --git a/src/login/extract.rs b/src/login/extract.rs
index 5ef454c..3b31d4c 100644
--- a/src/login/extract.rs
+++ b/src/login/extract.rs
@@ -1,3 +1,5 @@
+use std::fmt;
+
 use axum::{
     extract::FromRequestParts,
     http::request::Parts,
@@ -7,11 +9,22 @@ use axum_extra::extract::cookie::{Cookie, CookieJar};
 
 // The usage pattern here - receive the extractor as an argument, return it in
 // the response - is heavily modelled after CookieJar's own intended usage.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct IdentityToken {
     cookies: CookieJar,
 }
 
+impl fmt::Debug for IdentityToken {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("IdentityToken")
+            .field(
+                "identity",
+                &self.cookies.get(IDENTITY_COOKIE).map(|_| "********"),
+            )
+            .finish()
+    }
+}
+
 impl IdentityToken {
     // Creates a new, unpopulated identity token store.
     #[cfg(test)]
@@ -26,14 +39,18 @@ impl IdentityToken {
     // return [None]. If the identity has previously been [set], then this
     // will return that secret, regardless of what the request originally
     // included.
-    pub fn secret(&self) -> Option<&str> {
-        self.cookies.get(IDENTITY_COOKIE).map(Cookie::value)
+    pub fn secret(&self) -> Option<IdentitySecret> {
+        self.cookies
+            .get(IDENTITY_COOKIE)
+            .map(Cookie::value)
+            .map(IdentitySecret::from)
     }
 
     // Positively set the identity secret, and ensure that it will be sent
     // back to the client when this extractor is included in a response.
-    pub fn set(self, secret: &str) -> Self {
-        let identity_cookie = Cookie::build((IDENTITY_COOKIE, String::from(secret)))
+    pub fn set(self, secret: impl Into<IdentitySecret>) -> Self {
+        let IdentitySecret(secret) = secret.into();
+        let identity_cookie = Cookie::build((IDENTITY_COOKIE, secret))
             .http_only(true)
             .path("/api/")
             .permanent()
@@ -76,3 +93,22 @@ impl IntoResponseParts for IdentityToken {
         cookies.into_response_parts(res)
     }
 }
+
+#[derive(sqlx::Type)]
+#[sqlx(transparent)]
+pub struct IdentitySecret(String);
+
+impl fmt::Debug for IdentitySecret {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("IdentityToken").field(&"********").finish()
+    }
+}
+
+impl<S> From<S> for IdentitySecret
+where
+    S: Into<String>,
+{
+    fn from(value: S) -> Self {
+        Self(value.into())
+    }
+}
diff --git a/src/login/routes.rs b/src/login/routes.rs
index 31a68d0..4664063 100644
--- a/src/login/routes.rs
+++ b/src/login/routes.rs
@@ -6,7 +6,9 @@ use axum::{
     Router,
 };
 
-use crate::{app::App, clock::RequestedAt, error::Internal, repo::login::Login};
+use crate::{
+    app::App, clock::RequestedAt, error::Internal, password::Password, repo::login::Login,
+};
 
 use super::{app, extract::IdentityToken};
 
@@ -38,7 +40,7 @@ impl IntoResponse for Boot {
 #[derive(serde::Deserialize)]
 struct LoginRequest {
     name: String,
-    password: String,
+    password: Password,
 }
 
 async fn on_login(
@@ -52,7 +54,7 @@ async fn on_login(
         .login(&request.name, &request.password, &now)
         .await
         .map_err(LoginError)?;
-    let identity = identity.set(&token);
+    let identity = identity.set(token);
     Ok((identity, StatusCode::NO_CONTENT))
 }
 
@@ -82,7 +84,7 @@ async fn on_logout(
     Json(LogoutRequest {}): Json<LogoutRequest>,
 ) -> Result<(IdentityToken, StatusCode), LogoutError> {
     if let Some(secret) = identity.secret() {
-        app.logins().logout(secret).await.map_err(LogoutError)?;
+        app.logins().logout(&secret).await.map_err(LogoutError)?;
     }
 
     let identity = identity.clear();
diff --git a/src/login/routes/test/login.rs b/src/login/routes/test/login.rs
index 719ccca..10c17d6 100644
--- a/src/login/routes/test/login.rs
+++ b/src/login/routes/test/login.rs
@@ -38,7 +38,7 @@ async fn new_identity() {
     let validated_at = fixtures::now();
     let validated = app
         .logins()
-        .validate(secret, &validated_at)
+        .validate(&secret, &validated_at)
         .await
         .expect("identity secret is valid");
 
@@ -75,7 +75,7 @@ async fn existing_identity() {
     let validated_at = fixtures::now();
     let validated_login = app
         .logins()
-        .validate(secret, &validated_at)
+        .validate(&secret, &validated_at)
         .await
         .expect("identity secret is valid");
 
@@ -122,7 +122,7 @@ async fn token_expires() {
     let (identity, _) = routes::on_login(State(app.clone()), logged_in_at, identity, Json(request))
         .await
         .expect("logged in with valid credentials");
-    let token = identity.secret().expect("logged in with valid credentials");
+    let secret = identity.secret().expect("logged in with valid credentials");
 
     // Verify the semantics
 
@@ -135,7 +135,7 @@ async fn token_expires() {
     let verified_at = fixtures::now();
     let error = app
         .logins()
-        .validate(token, &verified_at)
+        .validate(&secret, &verified_at)
         .await
         .expect_err("validating an expired token");
 
diff --git a/src/login/routes/test/logout.rs b/src/login/routes/test/logout.rs
index 4c09a73..05594be 100644
--- a/src/login/routes/test/logout.rs
+++ b/src/login/routes/test/logout.rs
@@ -37,7 +37,7 @@ async fn successful() {
 
     let error = app
         .logins()
-        .validate(secret, &now)
+        .validate(&secret, &now)
         .await
         .expect_err("secret is invalid");
     match error {
diff --git a/src/password.rs b/src/password.rs
index b14f728..da3930f 100644
--- a/src/password.rs
+++ b/src/password.rs
@@ -1,3 +1,5 @@
+use std::fmt;
+
 use argon2::Argon2;
 use password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
 use rand_core::OsRng;
@@ -7,16 +9,7 @@ use rand_core::OsRng;
 pub struct StoredHash(String);
 
 impl StoredHash {
-    pub fn new(password: &str) -> Result<Self, password_hash::Error> {
-        let salt = SaltString::generate(&mut OsRng);
-        let argon2 = Argon2::default();
-        let hash = argon2
-            .hash_password(password.as_bytes(), &salt)?
-            .to_string();
-        Ok(Self(hash))
-    }
-
-    pub fn verify(&self, password: &str) -> Result<bool, password_hash::Error> {
+    pub fn verify(&self, password: &Password) -> Result<bool, password_hash::Error> {
         let hash = PasswordHash::new(&self.0)?;
 
         match Argon2::default().verify_password(password.as_bytes(), &hash) {
@@ -29,3 +22,37 @@ impl StoredHash {
         }
     }
 }
+
+#[derive(serde::Deserialize)]
+#[serde(transparent)]
+pub struct Password(String);
+
+impl Password {
+    pub fn hash(&self) -> Result<StoredHash, password_hash::Error> {
+        let Self(password) = self;
+        let salt = SaltString::generate(&mut OsRng);
+        let argon2 = Argon2::default();
+        let hash = argon2
+            .hash_password(password.as_bytes(), &salt)?
+            .to_string();
+        Ok(StoredHash(hash))
+    }
+
+    fn as_bytes(&self) -> &[u8] {
+        let Self(value) = self;
+        value.as_bytes()
+    }
+}
+
+impl fmt::Debug for Password {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("Password").field(&"********").finish()
+    }
+}
+
+#[cfg(test)]
+impl From<String> for Password {
+    fn from(password: String) -> Self {
+        Self(password)
+    }
+}
diff --git a/src/repo/login/extract.rs b/src/repo/login/extract.rs
index e5f96d0..c127078 100644
--- a/src/repo/login/extract.rs
+++ b/src/repo/login/extract.rs
@@ -32,7 +32,7 @@ impl FromRequestParts<App> for Login {
         let secret = identity_token.secret().ok_or(LoginError::Unauthorized)?;
 
         let app = State::<App>::from_request_parts(parts, state).await?;
-        match app.logins().validate(secret, &used_at).await {
+        match app.logins().validate(&secret, &used_at).await {
             Ok(login) => Ok(login),
             Err(ValidateError::InvalidToken) => Err(LoginError::Unauthorized),
             Err(other) => Err(other.into()),
diff --git a/src/repo/token.rs b/src/repo/token.rs
index 8276bea..15eef48 100644
--- a/src/repo/token.rs
+++ b/src/repo/token.rs
@@ -2,7 +2,7 @@ use sqlx::{sqlite::Sqlite, SqliteConnection, Transaction};
 use uuid::Uuid;
 
 use super::login::{self, Login};
-use crate::clock::DateTime;
+use crate::{clock::DateTime, login::extract::IdentitySecret};
 
 pub trait Provider {
     fn tokens(&mut self) -> Tokens;
@@ -23,7 +23,7 @@ impl<'c> Tokens<'c> {
         &mut self,
         login: &Login,
         issued_at: &DateTime,
-    ) -> Result<String, sqlx::Error> {
+    ) -> Result<IdentitySecret, sqlx::Error> {
         let secret = Uuid::new_v4().to_string();
 
         let secret = sqlx::query_scalar!(
@@ -31,7 +31,7 @@ impl<'c> Tokens<'c> {
                 insert
                 into token (secret, login, issued_at, last_used_at)
                 values ($1, $2, $3, $3)
-                returning secret as "secret!"
+                returning secret as "secret!: IdentitySecret"
             "#,
             secret,
             login.id,
@@ -44,7 +44,7 @@ impl<'c> Tokens<'c> {
     }
 
     // Revoke a token by its secret.
-    pub async fn revoke(&mut self, secret: &str) -> Result<(), sqlx::Error> {
+    pub async fn revoke(&mut self, secret: &IdentitySecret) -> Result<(), sqlx::Error> {
         sqlx::query!(
             r#"
                 delete
@@ -82,7 +82,7 @@ impl<'c> Tokens<'c> {
     // timestamp will be set to `used_at`.
     pub async fn validate(
         &mut self,
-        secret: &str,
+        secret: &IdentitySecret,
         used_at: &DateTime,
     ) -> Result<Login, sqlx::Error> {
         // I would use `update … returning` to do this in one query, but
diff --git a/src/test/fixtures/identity.rs b/src/test/fixtures/identity.rs
index 16463aa..69b5f4c 100644
--- a/src/test/fixtures/identity.rs
+++ b/src/test/fixtures/identity.rs
@@ -1,12 +1,17 @@
 use uuid::Uuid;
 
-use crate::{app::App, clock::RequestedAt, login::extract::IdentityToken};
+use crate::{
+    app::App,
+    clock::RequestedAt,
+    login::extract::{IdentitySecret, IdentityToken},
+    password::Password,
+};
 
 pub fn not_logged_in() -> IdentityToken {
     IdentityToken::new()
 }
 
-pub async fn logged_in(app: &App, login: &(String, String), now: &RequestedAt) -> IdentityToken {
+pub async fn logged_in(app: &App, login: &(String, Password), now: &RequestedAt) -> IdentityToken {
     let (name, password) = login;
     let token = app
         .logins()
@@ -14,14 +19,14 @@ pub async fn logged_in(app: &App, login: &(String, String), now: &RequestedAt) -
         .await
         .expect("should succeed given known-valid credentials");
 
-    IdentityToken::new().set(&token)
+    IdentityToken::new().set(token)
 }
 
-pub fn secret(identity: &IdentityToken) -> &str {
+pub fn secret(identity: &IdentityToken) -> IdentitySecret {
     identity.secret().expect("identity contained a secret")
 }
 
 pub fn fictitious() -> IdentityToken {
     let token = Uuid::new_v4().to_string();
-    IdentityToken::new().set(&token)
+    IdentityToken::new().set(token)
 }
diff --git a/src/test/fixtures/login.rs b/src/test/fixtures/login.rs
index f1e4b15..d6a321b 100644
--- a/src/test/fixtures/login.rs
+++ b/src/test/fixtures/login.rs
@@ -3,10 +3,11 @@ use uuid::Uuid;
 
 use crate::{
     app::App,
+    password::Password,
     repo::login::{self, Login},
 };
 
-pub async fn create_with_password(app: &App) -> (String, String) {
+pub async fn create_with_password(app: &App) -> (String, Password) {
     let (name, password) = propose();
     app.logins()
         .create(&name, &password)
@@ -31,7 +32,7 @@ pub fn fictitious() -> Login {
     }
 }
 
-pub fn propose() -> (String, String) {
+pub fn propose() -> (String, Password) {
     (name(), propose_password())
 }
 
@@ -39,6 +40,6 @@ fn name() -> String {
     rand::random::<internet::Username>().to_string()
 }
 
-pub fn propose_password() -> String {
-    Uuid::new_v4().to_string()
+pub fn propose_password() -> Password {
+    Uuid::new_v4().to_string().into()
 }