summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOwen Jacobson <owen@grimoire.ca>2024-09-28 01:40:22 -0400
committerOwen Jacobson <owen@grimoire.ca>2024-09-28 20:48:40 -0400
commit4d0bb0709b168a24ab6a8dbc86da45d7503596ee (patch)
tree031f2e35f07cef7305809e3a1d310bf304d15460
parent72efedf8e96ca6e159ce6146809ee6d3a9e5a0e7 (diff)
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.
-rw-r--r--.sqlx/query-c28b9bffa73d6a861e122a73a90e3912d23bf952740fe32544bc70a44e6a2744.json (renamed from .sqlx/query-df84b2afcb1493b3643a83b68a8abceb822eb5db2e7dd8e509d4f79c106f8561.json)6
-rw-r--r--src/login/app.rs22
-rw-r--r--src/login/extract.rs46
-rw-r--r--src/login/routes.rs10
-rw-r--r--src/login/routes/test/login.rs8
-rw-r--r--src/login/routes/test/logout.rs2
-rw-r--r--src/password.rs47
-rw-r--r--src/repo/login/extract.rs2
-rw-r--r--src/repo/token.rs10
-rw-r--r--src/test/fixtures/identity.rs15
-rw-r--r--src/test/fixtures/login.rs9
11 files changed, 126 insertions, 51 deletions
diff --git a/.sqlx/query-df84b2afcb1493b3643a83b68a8abceb822eb5db2e7dd8e509d4f79c106f8561.json b/.sqlx/query-c28b9bffa73d6a861e122a73a90e3912d23bf952740fe32544bc70a44e6a2744.json
index c788557..5927248 100644
--- a/.sqlx/query-df84b2afcb1493b3643a83b68a8abceb822eb5db2e7dd8e509d4f79c106f8561.json
+++ b/.sqlx/query-c28b9bffa73d6a861e122a73a90e3912d23bf952740fe32544bc70a44e6a2744.json
@@ -1,10 +1,10 @@
{
"db_name": "SQLite",
- "query": "\n insert\n into token (secret, login, issued_at, last_used_at)\n values ($1, $2, $3, $3)\n returning secret as \"secret!\"\n ",
+ "query": "\n insert\n into token (secret, login, issued_at, last_used_at)\n values ($1, $2, $3, $3)\n returning secret as \"secret!: IdentitySecret\"\n ",
"describe": {
"columns": [
{
- "name": "secret!",
+ "name": "secret!: IdentitySecret",
"ordinal": 0,
"type_info": "Text"
}
@@ -16,5 +16,5 @@
false
]
},
- "hash": "df84b2afcb1493b3643a83b68a8abceb822eb5db2e7dd8e509d4f79c106f8561"
+ "hash": "c28b9bffa73d6a861e122a73a90e3912d23bf952740fe32544bc70a44e6a2744"
}
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()
}