diff options
| author | ojacobson <ojacobson@noreply.codeberg.org> | 2025-08-26 23:36:40 +0200 |
|---|---|---|
| committer | ojacobson <ojacobson@noreply.codeberg.org> | 2025-08-26 23:36:40 +0200 |
| commit | 7b131e35fdea1a68aaf9230d157bafb200557ef8 (patch) | |
| tree | b0f3ee3ac604947a8866c692a080d3f6064d7d03 /src/login | |
| parent | 68f54c8904ec6ff2ac3be4c514fa4aa05a67cb68 (diff) | |
| parent | d0d5fa20200a7ad70173ba87ae47c33b60f44a3b (diff) | |
Split `user` into a chat-facing entity and an authentication-facing entity.
The taxonomy is now as follows:
* A _login_ is someone's identity for the purposes of authenticating to the service. Logins are not synchronized, and in fact are not published anywhere in the current API. They have a login ID, a name and a password.
* A _user_ is someone's identity for the purpose of participating in conversations. Users _are_ synchronized, as before. They have a user ID, a name, and a creation instant for the purposes of synchronization.
## API changes
* `GET /api/boot` method now returns a `login` key instead of a `user` key. The structure of the nested value is unchanged. This change is not backwards-compatible; the included client and the docs have been updated accordingly.
## Server implementation
* Most app methods that took a `&User` as an identity now take a `&Login` as an identity, instead. Where a `User` is needed, the new `tx.users().for_login(&login)` database access method resolves a `Login` to its corresponding `user::History`, which can then be turned into a `User` at whatever point in time is most appropriate.
This adds a few new error cases to methods that traverse the login-to-history-to-user chain. Those cases are presently unreachable, but I've fully fleshed them out so that they don't bite us later. Most of the resulting errors, however, are captured as internal server errors.
* There is a new `app.logins()` application entry point, dealing with login identities and password-based logins.
* `app.tokens()` is a bit more limited in scope to only things that work with an existing token.
That has the side effect of splitting up logging in (in `app.logins().with_password(…)`) and logging out (in `app.tokens().logout(…)`).
## Schema changes
The `user` table has been split:
* `login` holds the data needed for the user to log in - their login ID, their name, and their password.
* `user` now holds only the user ID and the event data for the user's `created` instant. Reconstructing a `User` struct requires joining in data from both `login` and `user`.
In theory, the relationship is one-way: every user has a login. In practice, it's reciprocal: every login has a user and every user has a login.
Relationships with downstream tables have been modified to suit:
* `message` still refers to `user` for authorship information.
* `invite` still refers to `user` for originator information.
* `token` refers to `login` for authentication information.
## Blimy, that's big
Yeah, I know. It's hard to avoid and I'm not sure the effort of making this in incremental steps is worth it.
Authentication logic has a way of getting into all sorts of corners, and Pilcrow is no different. In order for the new taxonomy to make sense, all of the places that previously used `User` as a representation of an authenticated identity have to be updated, and it's easier to do that all at once, so that we can retire all the code that _supports_ using a `User` that way.
Merges split-user into main.
Diffstat (limited to 'src/login')
| -rw-r--r-- | src/login/app.rs | 114 | ||||
| -rw-r--r-- | src/login/handlers/login/mod.rs | 51 | ||||
| -rw-r--r-- | src/login/handlers/login/test.rs | 114 | ||||
| -rw-r--r-- | src/login/handlers/logout/mod.rs | 51 | ||||
| -rw-r--r-- | src/login/handlers/logout/test.rs | 71 | ||||
| -rw-r--r-- | src/login/handlers/mod.rs | 7 | ||||
| -rw-r--r-- | src/login/handlers/password/mod.rs | 56 | ||||
| -rw-r--r-- | src/login/handlers/password/test.rs | 47 | ||||
| -rw-r--r-- | src/login/id.rs | 27 | ||||
| -rw-r--r-- | src/login/mod.rs | 13 | ||||
| -rw-r--r-- | src/login/repo.rs | 145 |
11 files changed, 696 insertions, 0 deletions
diff --git a/src/login/app.rs b/src/login/app.rs new file mode 100644 index 0000000..77d4ac3 --- /dev/null +++ b/src/login/app.rs @@ -0,0 +1,114 @@ +use sqlx::sqlite::SqlitePool; + +use crate::{ + clock::DateTime, + db::NotFound as _, + login::{self, Login, repo::Provider as _}, + name::{self, Name}, + password::Password, + token::{Broadcaster, Event as TokenEvent, Secret, Token, repo::Provider as _}, +}; + +pub struct Logins<'a> { + db: &'a SqlitePool, + token_events: &'a Broadcaster, +} + +impl<'a> Logins<'a> { + pub const fn new(db: &'a SqlitePool, token_events: &'a Broadcaster) -> Self { + Self { db, token_events } + } + + pub async fn with_password( + &self, + name: &Name, + candidate: &Password, + login_at: &DateTime, + ) -> Result<Secret, LoginError> { + let mut tx = self.db.begin().await?; + let (login, password) = tx + .logins() + .by_name(name) + .await + .not_found(|| LoginError::Rejected)?; + // Split the transaction here to avoid holding the tx open (potentially blocking + // other writes) while we do the fairly expensive task of verifying the + // password. It's okay if the token issuance transaction happens some notional + // amount of time after retrieving the login, as inserting the token will fail + // if the account is deleted during that time. + tx.commit().await?; + + if password.verify(candidate)? { + let mut tx = self.db.begin().await?; + let (token, secret) = Token::generate(&login, login_at); + tx.tokens().create(&token, &secret).await?; + tx.commit().await?; + Ok(secret) + } else { + Err(LoginError::Rejected) + } + } + + pub async fn change_password( + &self, + login: &Login, + from: &Password, + to: &Password, + changed_at: &DateTime, + ) -> Result<Secret, LoginError> { + let mut tx = self.db.begin().await?; + let (login, password) = tx + .logins() + .by_id(&login.id) + .await + .not_found(|| LoginError::Rejected)?; + // Split the transaction here to avoid holding the tx open (potentially blocking + // other writes) while we do the fairly expensive task of verifying the + // password. It's okay if the token issuance transaction happens some notional + // amount of time after retrieving the login, as inserting the token will fail + // if the account is deleted during that time. + tx.commit().await?; + + if password.verify(from)? { + let to_hash = to.hash()?; + let (token, secret) = Token::generate(&login, changed_at); + + let mut tx = self.db.begin().await?; + tx.logins().set_password(&login, &to_hash).await?; + + let revoked = tx.tokens().revoke_all(&login).await?; + tx.tokens().create(&token, &secret).await?; + tx.commit().await?; + + for event in revoked.into_iter().map(TokenEvent::Revoked) { + self.token_events.broadcast(event); + } + + Ok(secret) + } else { + Err(LoginError::Rejected) + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum LoginError { + #[error("invalid login")] + Rejected, + #[error(transparent)] + Database(#[from] sqlx::Error), + #[error(transparent)] + Name(#[from] name::Error), + #[error(transparent)] + PasswordHash(#[from] password_hash::Error), +} + +impl From<login::repo::LoadError> for LoginError { + fn from(error: login::repo::LoadError) -> Self { + use login::repo::LoadError; + match error { + LoadError::Database(error) => error.into(), + LoadError::Name(error) => error.into(), + } + } +} diff --git a/src/login/handlers/login/mod.rs b/src/login/handlers/login/mod.rs new file mode 100644 index 0000000..6591984 --- /dev/null +++ b/src/login/handlers/login/mod.rs @@ -0,0 +1,51 @@ +use axum::{ + extract::{Json, State}, + http::StatusCode, + response::{IntoResponse, Response}, +}; + +use crate::{ + app::App, clock::RequestedAt, empty::Empty, error::Internal, login::app, name::Name, + password::Password, token::extract::IdentityCookie, +}; + +#[cfg(test)] +mod test; + +pub async fn handler( + State(app): State<App>, + RequestedAt(now): RequestedAt, + identity: IdentityCookie, + Json(request): Json<Request>, +) -> Result<(IdentityCookie, Empty), Error> { + let secret = app + .logins() + .with_password(&request.name, &request.password, &now) + .await + .map_err(Error)?; + let identity = identity.set(secret); + Ok((identity, Empty)) +} + +#[derive(serde::Deserialize)] +pub struct Request { + pub name: Name, + pub password: Password, +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct Error(#[from] pub app::LoginError); + +impl IntoResponse for Error { + fn into_response(self) -> Response { + let Self(error) = self; + match error { + app::LoginError::Rejected => { + // not error::Unauthorized due to differing messaging + (StatusCode::UNAUTHORIZED, "invalid name or password").into_response() + } + other => Internal::from(other).into_response(), + } + } +} diff --git a/src/login/handlers/login/test.rs b/src/login/handlers/login/test.rs new file mode 100644 index 0000000..f3911d0 --- /dev/null +++ b/src/login/handlers/login/test.rs @@ -0,0 +1,114 @@ +use axum::extract::{Json, State}; + +use crate::{ + empty::Empty, + login::app::LoginError, + test::{fixtures, verify}, +}; + +#[tokio::test] +async fn correct_credentials() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let (name, password) = fixtures::user::create_with_password(&app, &fixtures::now()).await; + + // Call the endpoint + + let identity = fixtures::cookie::not_logged_in(); + let logged_in_at = fixtures::now(); + let request = super::Request { + name: name.clone(), + password, + }; + let (identity, Empty) = + super::handler(State(app.clone()), logged_in_at, identity, Json(request)) + .await + .expect("logged in with valid credentials"); + + // Verify the return value's basic structure + + let secret = identity + .secret() + .expect("logged in with valid credentials issues an identity cookie"); + + // Verify the semantics + verify::token::valid_for_name(&app, &secret, &name).await; +} + +#[tokio::test] +async fn invalid_name() { + // Set up the environment + + let app = fixtures::scratch_app().await; + + // Call the endpoint + + let identity = fixtures::cookie::not_logged_in(); + let logged_in_at = fixtures::now(); + let (name, password) = fixtures::user::propose(); + let request = super::Request { + name: name.clone(), + password, + }; + let super::Error(error) = + super::handler(State(app.clone()), logged_in_at, identity, Json(request)) + .await + .expect_err("logged in with an incorrect password fails"); + + // Verify the return value's basic structure + + assert!(matches!(error, LoginError::Rejected)); +} + +#[tokio::test] +async fn incorrect_password() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let login = fixtures::user::create(&app, &fixtures::now()).await; + + // Call the endpoint + + let logged_in_at = fixtures::now(); + let identity = fixtures::cookie::not_logged_in(); + let request = super::Request { + name: login.name, + password: fixtures::user::propose_password(), + }; + let super::Error(error) = + super::handler(State(app.clone()), logged_in_at, identity, Json(request)) + .await + .expect_err("logged in with an incorrect password"); + + // Verify the return value's basic structure + + assert!(matches!(error, LoginError::Rejected)); +} + +#[tokio::test] +async fn token_expires() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let (name, password) = fixtures::user::create_with_password(&app, &fixtures::now()).await; + + // Call the endpoint + + let logged_in_at = fixtures::ancient(); + let identity = fixtures::cookie::not_logged_in(); + let request = super::Request { name, password }; + let (identity, _) = super::handler(State(app.clone()), logged_in_at, identity, Json(request)) + .await + .expect("logged in with valid credentials"); + let secret = identity.secret().expect("logged in with valid credentials"); + + // Verify the semantics + + app.tokens() + .expire(&fixtures::now()) + .await + .expect("expiring tokens never fails"); + + verify::token::invalid(&app, &secret).await; +} diff --git a/src/login/handlers/logout/mod.rs b/src/login/handlers/logout/mod.rs new file mode 100644 index 0000000..73efe73 --- /dev/null +++ b/src/login/handlers/logout/mod.rs @@ -0,0 +1,51 @@ +use axum::{ + extract::{Json, State}, + response::{IntoResponse, Response}, +}; + +use crate::{ + app::App, + clock::RequestedAt, + empty::Empty, + error::{Internal, Unauthorized}, + token::{app, extract::IdentityCookie}, +}; + +#[cfg(test)] +mod test; + +pub async fn handler( + State(app): State<App>, + RequestedAt(now): RequestedAt, + identity: IdentityCookie, + Json(_): Json<Request>, +) -> Result<(IdentityCookie, Empty), Error> { + if let Some(secret) = identity.secret() { + let identity = app.tokens().validate(&secret, &now).await?; + app.tokens().logout(&identity.token).await?; + } + + let identity = identity.clear(); + Ok((identity, Empty)) +} + +// This forces the only valid request to be `{}`, and not the infinite +// variation allowed when there's no body extractor. +#[derive(Default, serde::Deserialize)] +pub struct Request {} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct Error(#[from] pub app::ValidateError); + +impl IntoResponse for Error { + fn into_response(self) -> Response { + let Self(error) = self; + match error { + app::ValidateError::InvalidToken => Unauthorized.into_response(), + app::ValidateError::Name(_) | app::ValidateError::Database(_) => { + Internal::from(error).into_response() + } + } + } +} diff --git a/src/login/handlers/logout/test.rs b/src/login/handlers/logout/test.rs new file mode 100644 index 0000000..e7b7dd4 --- /dev/null +++ b/src/login/handlers/logout/test.rs @@ -0,0 +1,71 @@ +use axum::extract::{Json, State}; + +use crate::{ + empty::Empty, + test::{fixtures, verify}, + token::app, +}; + +#[tokio::test] +async fn successful() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let now = fixtures::now(); + let creds = fixtures::user::create_with_password(&app, &fixtures::now()).await; + let identity = fixtures::cookie::logged_in(&app, &creds, &now).await; + + // Call the endpoint + + let (response_identity, Empty) = super::handler( + State(app.clone()), + fixtures::now(), + identity.clone(), + Json::default(), + ) + .await + .expect("logged out with a valid token"); + + // Verify the return value's basic structure + assert!(response_identity.secret().is_none()); + + // Verify the semantics + verify::identity::invalid(&app, &identity).await; +} + +#[tokio::test] +async fn no_identity() { + // Set up the environment + + let app = fixtures::scratch_app().await; + + // Call the endpoint + + let identity = fixtures::cookie::not_logged_in(); + let (identity, Empty) = super::handler(State(app), fixtures::now(), identity, Json::default()) + .await + .expect("logged out with no token succeeds"); + + // Verify the return value's basic structure + + assert!(identity.secret().is_none()); +} + +#[tokio::test] +async fn invalid_token() { + // Set up the environment + + let app = fixtures::scratch_app().await; + + // Call the endpoint + + let identity = fixtures::cookie::fictitious(); + let super::Error(error) = + super::handler(State(app), fixtures::now(), identity, Json::default()) + .await + .expect_err("logged out with an invalid token fails"); + + // Verify the return value's basic structure + + assert!(matches!(error, app::ValidateError::InvalidToken)); +} diff --git a/src/login/handlers/mod.rs b/src/login/handlers/mod.rs new file mode 100644 index 0000000..24ee7f9 --- /dev/null +++ b/src/login/handlers/mod.rs @@ -0,0 +1,7 @@ +mod login; +mod logout; +pub mod password; + +pub use login::handler as login; +pub use logout::handler as logout; +pub use password::handler as change_password; diff --git a/src/login/handlers/password/mod.rs b/src/login/handlers/password/mod.rs new file mode 100644 index 0000000..94c7fb4 --- /dev/null +++ b/src/login/handlers/password/mod.rs @@ -0,0 +1,56 @@ +use axum::{ + extract::{Json, State}, + http::StatusCode, + response::{IntoResponse, Response}, +}; + +use crate::{ + app::App, + clock::RequestedAt, + empty::Empty, + error::Internal, + login::app, + password::Password, + token::extract::{Identity, IdentityCookie}, +}; + +#[cfg(test)] +mod test; + +pub async fn handler( + State(app): State<App>, + RequestedAt(now): RequestedAt, + identity: Identity, + cookie: IdentityCookie, + Json(request): Json<Request>, +) -> Result<(IdentityCookie, Empty), Error> { + let secret = app + .logins() + .change_password(&identity.login, &request.password, &request.to, &now) + .await + .map_err(Error)?; + let cookie = cookie.set(secret); + Ok((cookie, Empty)) +} + +#[derive(serde::Deserialize)] +pub struct Request { + pub password: Password, + pub to: Password, +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct Error(#[from] pub app::LoginError); + +impl IntoResponse for Error { + fn into_response(self) -> Response { + let Self(error) = self; + match error { + app::LoginError::Rejected => { + (StatusCode::BAD_REQUEST, "invalid name or password").into_response() + } + other => Internal::from(other).into_response(), + } + } +} diff --git a/src/login/handlers/password/test.rs b/src/login/handlers/password/test.rs new file mode 100644 index 0000000..ba2f28f --- /dev/null +++ b/src/login/handlers/password/test.rs @@ -0,0 +1,47 @@ +use axum::extract::{Json, State}; + +use crate::{ + empty::Empty, + test::{fixtures, verify}, +}; + +#[tokio::test] +async fn password_change() { + // Set up the environment + let app = fixtures::scratch_app().await; + let creds = fixtures::user::create_with_password(&app, &fixtures::now()).await; + let cookie = fixtures::cookie::logged_in(&app, &creds, &fixtures::now()).await; + let identity = fixtures::identity::from_cookie(&app, &cookie, &fixtures::now()).await; + + // Call the endpoint + let (name, password) = creds; + let to = fixtures::user::propose_password(); + let request = super::Request { + password: password.clone(), + to: to.clone(), + }; + let (new_cookie, Empty) = super::handler( + State(app.clone()), + fixtures::now(), + identity.clone(), + cookie.clone(), + Json(request), + ) + .await + .expect("changing passwords succeeds"); + + // Verify that we have a new session + assert_ne!(cookie.secret(), new_cookie.secret()); + + // Verify that we're still ourselves + verify::identity::valid_for_login(&app, &new_cookie, &identity.login).await; + + // Verify that our original token is no longer valid + verify::identity::invalid(&app, &cookie).await; + + // Verify that our original password is no longer valid + verify::login::invalid_login(&app, &name, &password).await; + + // Verify that our new password is valid + verify::login::valid_login(&app, &name, &to).await; +} diff --git a/src/login/id.rs b/src/login/id.rs new file mode 100644 index 0000000..ab16a15 --- /dev/null +++ b/src/login/id.rs @@ -0,0 +1,27 @@ +use crate::user; + +pub type Id = crate::id::Id<Login>; + +// Login IDs and user IDs are typewise-distinct as they identify things in different namespaces, but +// in practice a login and its associated user _must_ have IDs that encode to the same value. The +// two ID types are made interconvertible (via `From`) for this purpose. +impl From<user::Id> for Id { + fn from(user: user::Id) -> Self { + Self::from(String::from(user)) + } +} + +impl PartialEq<user::Id> for Id { + fn eq(&self, other: &user::Id) -> bool { + self.as_str().eq(other.as_str()) + } +} + +#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct Login; + +impl crate::id::Prefix for Login { + fn prefix(&self) -> &'static str { + user::id::User.prefix() + } +} diff --git a/src/login/mod.rs b/src/login/mod.rs new file mode 100644 index 0000000..bccc2af --- /dev/null +++ b/src/login/mod.rs @@ -0,0 +1,13 @@ +pub mod app; +pub mod handlers; +mod id; +pub mod repo; + +use crate::name::Name; +pub use id::Id; + +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +pub struct Login { + pub id: Id, + pub name: Name, +} diff --git a/src/login/repo.rs b/src/login/repo.rs new file mode 100644 index 0000000..5be91ad --- /dev/null +++ b/src/login/repo.rs @@ -0,0 +1,145 @@ +use sqlx::{SqliteConnection, Transaction, sqlite::Sqlite}; + +use super::{Id, Login}; +use crate::{ + db::NotFound, + name::{self, Name}, + password::StoredHash, +}; + +pub trait Provider { + fn logins(&mut self) -> Logins<'_>; +} + +impl Provider for Transaction<'_, Sqlite> { + fn logins(&mut self) -> Logins<'_> { + Logins(self) + } +} + +pub struct Logins<'t>(&'t mut SqliteConnection); + +impl Logins<'_> { + pub async fn create( + &mut self, + login: &Login, + password: &StoredHash, + ) -> Result<(), sqlx::Error> { + let Login { id, name } = login; + let display_name = name.display(); + let canonical_name = name.canonical(); + + sqlx::query!( + r#" + insert into login (id, display_name, canonical_name, password) + values ($1, $2, $3, $4) + "#, + id, + display_name, + canonical_name, + password, + ) + .execute(&mut *self.0) + .await?; + + Ok(()) + } + + pub async fn by_id(&mut self, id: &Id) -> Result<(Login, StoredHash), LoadError> { + let user = sqlx::query!( + r#" + select + id as "id: Id", + display_name, + canonical_name, + password as "password: StoredHash" + from login + where id = $1 + "#, + id, + ) + .map(|row| { + Ok::<_, LoadError>(( + Login { + id: row.id, + name: Name::new(row.display_name, row.canonical_name)?, + }, + row.password, + )) + }) + .fetch_one(&mut *self.0) + .await??; + + Ok(user) + } + + pub async fn by_name(&mut self, name: &Name) -> Result<(Login, StoredHash), LoadError> { + let canonical_name = name.canonical(); + + let (login, password) = sqlx::query!( + r#" + select + id as "id: Id", + display_name, + canonical_name, + password as "password: StoredHash" + from login + where canonical_name = $1 + "#, + canonical_name, + ) + .map(|row| { + Ok::<_, LoadError>(( + Login { + id: row.id, + name: Name::new(row.display_name, row.canonical_name)?, + }, + row.password, + )) + }) + .fetch_one(&mut *self.0) + .await??; + + Ok((login, password)) + } + + pub async fn set_password( + &mut self, + login: &Login, + password: &StoredHash, + ) -> Result<(), sqlx::Error> { + sqlx::query!( + r#" + update login + set password = $1 + where id = $2 + "#, + password, + login.id, + ) + .execute(&mut *self.0) + .await?; + + Ok(()) + } +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub enum LoadError { + Database(#[from] sqlx::Error), + Name(#[from] name::Error), +} + +impl<T> NotFound for Result<T, LoadError> { + type Ok = T; + type Error = LoadError; + + fn optional(self) -> Result<Option<T>, LoadError> { + match self { + Ok(value) => Ok(Some(value)), + Err(LoadError::Database(sqlx::Error::RowNotFound)) => Ok(None), + Err(other) => Err(other), + } + } +} |
