Push most endpoint and extractor logic into functoins of `App`.

This is, again, groundwork for logic that requires more than just a database connection.

The login process has been changed to be more conventional, attempting login _before_ account creation rather than after it. This was not previously possible, because the data access methods used to perform these steps did not return enough information to carry out the workflow in that order. Separating storage from password validation and hashing forces the issue, and makes it clearer _at the App_ whether an account exists or not.

This does introduce the possibility of two racing inserts trying to lay claim to the same username. Transaction isolation should ensure that only one of them "wins," which is what you get before this change anyways.

15 files changed, 307 insertions, 251 deletions

diff --git a/src/app.rs b/src/app.rs
index eff1b5e..4195fdc 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -1,8 +1,10 @@
 use sqlx::sqlite::SqlitePool;
 
+use crate::{channel::app::Channels, index::app::Index, login::app::Logins};
+
 #[derive(Clone)]
 pub struct App {
-    pub db: SqlitePool,
+    db: SqlitePool,
 }
 
 impl App {
@@ -10,3 +12,17 @@ impl App {
         Self { db }
     }
 }
+
+impl App {
+    pub fn index(&self) -> Index {
+        Index::new(&self.db)
+    }
+
+    pub fn logins(&self) -> Logins {
+        Logins::new(&self.db)
+    }
+
+    pub fn channels(&self) -> Channels {
+        Channels::new(&self.db)
+    }
+}
diff --git a/src/channel/app.rs b/src/channel/app.rs
new file mode 100644
index 0000000..84822cb
--- /dev/null
+++ b/src/channel/app.rs
@@ -0,0 +1,21 @@
+use sqlx::sqlite::SqlitePool;
+
+use super::repo::Provider as _;
+use crate::error::BoxedError;
+
+pub struct Channels<'a> {
+    db: &'a SqlitePool,
+}
+
+impl<'a> Channels<'a> {
+    pub fn new(db: &'a SqlitePool) -> Self {
+        Self { db }
+    }
+
+    pub async fn create(&self, name: &str) -> Result<(), BoxedError> {
+        let mut tx = self.db.begin().await?;
+        tx.channels().create(name).await?;
+        tx.commit().await?;
+        Ok(())
+    }
+}
diff --git a/src/channel/mod.rs b/src/channel/mod.rs
index 238e116..f67ea04 100644
--- a/src/channel/mod.rs
+++ b/src/channel/mod.rs
@@ -1,3 +1,4 @@
+pub mod app;
 pub mod repo;
 mod routes;
 
diff --git a/src/channel/routes.rs b/src/channel/routes.rs
index 6e06cc9..864f1b3 100644
--- a/src/channel/routes.rs
+++ b/src/channel/routes.rs
@@ -5,7 +5,6 @@ use axum::{
     Router,
 };
 
-use super::repo::Provider as _;
 use crate::{app::App, error::InternalError, login::repo::logins::Login};
 
 pub fn router() -> Router<App> {
@@ -22,9 +21,7 @@ async fn on_create(
     _: Login, // requires auth, but doesn't actually care who you are
     Form(form): Form<CreateRequest>,
 ) -> Result<impl IntoResponse, InternalError> {
-    let mut tx = app.db.begin().await?;
-    tx.channels().create(&form.name).await?;
-    tx.commit().await?;
+    app.channels().create(&form.name).await?;
 
     Ok(Redirect::to("/"))
 }
diff --git a/src/cli.rs b/src/cli.rs
index 6b24b65..e374834 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -65,7 +65,7 @@ impl Args {
 fn routers() -> Router<App> {
     [channel::router(), login::router()]
         .into_iter()
-        .fold(index::router(), Router::merge)
+        .fold(index::routes::router(), Router::merge)
 }
 
 fn started_msg(listener: &net::TcpListener) -> io::Result<String> {
diff --git a/src/index.rs b/src/index.rs
deleted file mode 100644
index 843cb77..0000000
--- a/src/index.rs
+++ /dev/null
@@ -1,120 +0,0 @@
-use axum::{extract::State, routing::get, Router};
-use maud::Markup;
-
-use crate::{
-    app::App, channel::repo::Provider as _, error::InternalError, login::repo::logins::Login,
-};
-
-async fn index(State(app): State<App>, login: Option<Login>) -> Result<Markup, InternalError> {
-    match login {
-        None => Ok(templates::unauthenticated()),
-        Some(login) => index_authenticated(app, login).await,
-    }
-}
-
-async fn index_authenticated(app: App, login: Login) -> Result<Markup, InternalError> {
-    let mut tx = app.db.begin().await?;
-    let channels = tx.channels().all().await?;
-    tx.commit().await?;
-
-    Ok(templates::authenticated(login, &channels))
-}
-
-pub fn router() -> Router<App> {
-    Router::new().route("/", get(index))
-}
-
-mod templates {
-    use maud::{html, Markup, DOCTYPE};
-
-    use crate::{channel::repo::Channel, login::repo::logins::Login};
-
-    pub fn authenticated<'c>(
-        login: Login,
-        channels: impl IntoIterator<Item = &'c Channel>,
-    ) -> Markup {
-        html! {
-            (DOCTYPE)
-            head {
-                title { "hi" }
-            }
-            body {
-                section {
-                    (channel_list(channels))
-                    (create_channel())
-                }
-                section {
-                    (logout_form(&login.name))
-                }
-            }
-        }
-    }
-
-    fn channel_list<'c>(channels: impl IntoIterator<Item = &'c Channel>) -> Markup {
-        html! {
-            ul {
-                @for channel in channels {
-                    (channel_list_entry(&channel))
-                }
-            }
-        }
-    }
-
-    fn channel_list_entry(channel: &Channel) -> Markup {
-        html! {
-            li {
-                (channel.name) " (" (channel.id) ")"
-            }
-        }
-    }
-
-    fn create_channel() -> Markup {
-        html! {
-            form action="/create" method="post" {
-                label {
-                    "name"
-                    input name="name" type="text" {}
-                }
-                button {
-                    "start channel"
-                }
-            }
-        }
-    }
-
-    fn logout_form(name: &str) -> Markup {
-        html! {
-            form action="/logout" method="post" {
-                button { "bye, " (name) }
-            }
-        }
-    }
-
-    pub fn unauthenticated() -> Markup {
-        html! {
-            (DOCTYPE)
-            head {
-                title { "hi" }
-            }
-            body {
-                (login_form())
-            }
-        }
-    }
-
-    fn login_form() -> Markup {
-        html! {
-            form action="/login" method="post" {
-                label {
-                    "login"
-                    input name="name" type="text" {}
-                }
-                label {
-                    "password"
-                    input name="password" type="password" {}
-                }
-                button { "hi" }
-            }
-        }
-    }
-}
diff --git a/src/index/app.rs b/src/index/app.rs
new file mode 100644
index 0000000..79f5a9a
--- /dev/null
+++ b/src/index/app.rs
@@ -0,0 +1,24 @@
+use sqlx::sqlite::SqlitePool;
+
+use crate::{
+    channel::repo::{Channel, Provider as _},
+    error::BoxedError,
+};
+
+pub struct Index<'a> {
+    db: &'a SqlitePool,
+}
+
+impl<'a> Index<'a> {
+    pub fn new(db: &'a SqlitePool) -> Self {
+        Self { db }
+    }
+
+    pub async fn for_authenticated(&self) -> Result<Vec<Channel>, BoxedError> {
+        let mut tx = self.db.begin().await?;
+        let channels = tx.channels().all().await?;
+        tx.commit().await?;
+
+        Ok(channels)
+    }
+}
diff --git a/src/index/mod.rs b/src/index/mod.rs
new file mode 100644
index 0000000..0d89a50
--- /dev/null
+++ b/src/index/mod.rs
@@ -0,0 +1,3 @@
+pub mod app;
+pub mod routes;
+mod templates;
diff --git a/src/index/routes.rs b/src/index/routes.rs
new file mode 100644
index 0000000..c57278f
--- /dev/null
+++ b/src/index/routes.rs
@@ -0,0 +1,22 @@
+use axum::{extract::State, routing::get, Router};
+use maud::Markup;
+
+use super::templates;
+use crate::{app::App, error::InternalError, login::repo::logins::Login};
+
+async fn index(State(app): State<App>, login: Option<Login>) -> Result<Markup, InternalError> {
+    match login {
+        None => Ok(templates::unauthenticated()),
+        Some(login) => index_authenticated(app, login).await,
+    }
+}
+
+async fn index_authenticated(app: App, login: Login) -> Result<Markup, InternalError> {
+    let channels = app.index().for_authenticated().await?;
+
+    Ok(templates::authenticated(login, &channels))
+}
+
+pub fn router() -> Router<App> {
+    Router::new().route("/", get(index))
+}
diff --git a/src/index/templates.rs b/src/index/templates.rs
new file mode 100644
index 0000000..fdb750b
--- /dev/null
+++ b/src/index/templates.rs
@@ -0,0 +1,89 @@
+use maud::{html, Markup, DOCTYPE};
+
+use crate::{channel::repo::Channel, login::repo::logins::Login};
+
+pub fn authenticated<'c>(login: Login, channels: impl IntoIterator<Item = &'c Channel>) -> Markup {
+    html! {
+        (DOCTYPE)
+        head {
+            title { "hi" }
+        }
+        body {
+            section {
+                (channel_list(channels))
+                (create_channel())
+            }
+            section {
+                (logout_form(&login.name))
+            }
+        }
+    }
+}
+
+fn channel_list<'c>(channels: impl IntoIterator<Item = &'c Channel>) -> Markup {
+    html! {
+        ul {
+            @for channel in channels {
+                (channel_list_entry(&channel))
+            }
+        }
+    }
+}
+
+fn channel_list_entry(channel: &Channel) -> Markup {
+    html! {
+        li {
+            (channel.name) " (" (channel.id) ")"
+        }
+    }
+}
+
+fn create_channel() -> Markup {
+    html! {
+        form action="/create" method="post" {
+            label {
+                "name"
+                input name="name" type="text" {}
+            }
+            button {
+                "start channel"
+            }
+        }
+    }
+}
+
+fn logout_form(name: &str) -> Markup {
+    html! {
+        form action="/logout" method="post" {
+            button { "bye, " (name) }
+        }
+    }
+}
+
+pub fn unauthenticated() -> Markup {
+    html! {
+        (DOCTYPE)
+        head {
+            title { "hi" }
+        }
+        body {
+            (login_form())
+        }
+    }
+}
+
+fn login_form() -> Markup {
+    html! {
+        form action="/login" method="post" {
+            label {
+                "login"
+                input name="name" type="text" {}
+            }
+            label {
+                "password"
+                input name="password" type="password" {}
+            }
+            button { "hi" }
+        }
+    }
+}
diff --git a/src/login/app.rs b/src/login/app.rs
new file mode 100644
index 0000000..ced76d6
--- /dev/null
+++ b/src/login/app.rs
@@ -0,0 +1,107 @@
+use argon2::Argon2;
+use password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
+use rand_core::OsRng;
+use sqlx::sqlite::SqlitePool;
+
+use super::repo::{
+    logins::{Login, Provider as _},
+    tokens::Provider as _,
+};
+use crate::error::BoxedError;
+
+type DateTime = chrono::DateTime<chrono::Utc>;
+
+pub struct Logins<'a> {
+    db: &'a SqlitePool,
+}
+
+impl<'a> Logins<'a> {
+    pub fn new(db: &'a SqlitePool) -> Self {
+        Self { db }
+    }
+
+    pub async fn login(
+        &self,
+        name: &str,
+        password: &str,
+        login_at: DateTime,
+    ) -> Result<Option<String>, BoxedError> {
+        let mut tx = self.db.begin().await?;
+
+        let login = if let Some((login, stored_hash)) = tx.logins().for_login(name).await? {
+            if stored_hash.verify(password)? {
+                // Password verified; use the login.
+                Some(login)
+            } else {
+                // Password NOT verified.
+                None
+            }
+        } else {
+            let password_hash = StoredHash::new(password)?;
+            Some(tx.logins().create(name, &password_hash).await?)
+        };
+
+        // If `login` is Some, then we have an identity and can issue a token.
+        // If `login` is None, then neither creating a new login nor
+        // authenticating an existing one succeeded, and we must reject the
+        // login attempt.
+        let token = if let Some(login) = login {
+            Some(tx.tokens().issue(&login.id, login_at).await?)
+        } else {
+            None
+        };
+
+        tx.commit().await?;
+
+        Ok(token)
+    }
+
+    pub async fn validate(
+        &self,
+        secret: &str,
+        used_at: DateTime,
+    ) -> Result<Option<Login>, BoxedError> {
+        let mut tx = self.db.begin().await?;
+        tx.tokens().expire(used_at).await?;
+        let login = tx.tokens().validate(secret, used_at).await?;
+        tx.commit().await?;
+
+        Ok(login)
+    }
+
+    pub async fn logout(&self, secret: &str) -> Result<(), BoxedError> {
+        let mut tx = self.db.begin().await?;
+        tx.tokens().revoke(secret).await?;
+        tx.commit().await?;
+
+        Ok(())
+    }
+}
+
+#[derive(Debug, sqlx::Type)]
+#[sqlx(transparent)]
+pub struct StoredHash(String);
+
+impl StoredHash {
+    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))
+    }
+
+    fn verify(&self, password: &str) -> Result<bool, password_hash::Error> {
+        let hash = PasswordHash::new(&self.0)?;
+
+        match Argon2::default().verify_password(password.as_bytes(), &hash) {
+            // Successful authentication, not an error
+            Ok(()) => Ok(true),
+            // Unsuccessful authentication, also not an error
+            Err(password_hash::errors::Error::Password) => Ok(false),
+            // Password validation failed for some other reason, treat as an error
+            Err(err) => Err(err),
+        }
+    }
+}
diff --git a/src/login/extract/login.rs b/src/login/extract/login.rs
index 4155ec2..a5f648b 100644
--- a/src/login/extract/login.rs
+++ b/src/login/extract/login.rs
@@ -8,10 +8,7 @@ use crate::{
     app::App,
     clock::RequestedAt,
     error::InternalError,
-    login::{
-        extract::IdentityToken,
-        repo::{logins::Login, tokens::Provider as _},
-    },
+    login::{extract::IdentityToken, repo::logins::Login},
 };
 
 #[async_trait::async_trait]
@@ -24,15 +21,12 @@ impl FromRequestParts<App> for Login {
         //
         // let Ok(identity_token) = IdentityToken::from_request_parts(parts, state).await;
         let identity_token = IdentityToken::from_request_parts(parts, state).await?;
-        let RequestedAt(requested_at) = RequestedAt::from_request_parts(parts, state).await?;
+        let RequestedAt(used_at) = RequestedAt::from_request_parts(parts, state).await?;
 
         let secret = identity_token.secret().ok_or(LoginError::Forbidden)?;
 
         let app = State::<App>::from_request_parts(parts, state).await?;
-        let mut tx = app.db.begin().await?;
-        tx.tokens().expire(requested_at).await?;
-        let login = tx.tokens().validate(secret, requested_at).await?;
-        tx.commit().await?;
+        let login = app.logins().validate(secret, used_at).await?;
 
         login.ok_or(LoginError::Forbidden)
     }
diff --git a/src/login/mod.rs b/src/login/mod.rs
index c2b2924..5070301 100644
--- a/src/login/mod.rs
+++ b/src/login/mod.rs
@@ -1,5 +1,6 @@
 pub use self::routes::router;
 
+pub mod app;
 mod extract;
 pub mod repo;
 mod routes;
diff --git a/src/login/repo/logins.rs b/src/login/repo/logins.rs
index 5f042fd..26a5b09 100644
--- a/src/login/repo/logins.rs
+++ b/src/login/repo/logins.rs
@@ -1,10 +1,8 @@
-use argon2::Argon2;
-use password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
-use rand_core::OsRng;
 use sqlx::{sqlite::Sqlite, SqliteConnection, Transaction};
 
 use crate::error::BoxedError;
 use crate::id::Id as BaseId;
+use crate::login::app::StoredHash;
 
 pub trait Provider {
     fn logins(&mut self) -> Logins;
@@ -30,77 +28,40 @@ pub struct Login {
 }
 
 impl<'c> Logins<'c> {
-    /// Create a new login, if the name is not already taken. Returns a [Login]
-    /// if a new login has actually been created, or `None` if an existing login
-    /// was found.
     pub async fn create(
         &mut self,
         name: &str,
-        password: &str,
-    ) -> Result<Option<Login>, BoxedError> {
+        password_hash: &StoredHash,
+    ) -> Result<Login, BoxedError> {
         let id = Id::generate();
-        let password_hash = StoredHash::new(password)?;
 
-        let insert_res = sqlx::query_as!(
+        let login = sqlx::query_as!(
             Login,
             r#"
                 insert or fail
                 into login (id, name, password_hash)
                 values ($1, $2, $3)
-                returning id as "id: Id", name
+                returning
+                    id as "id: Id",
+                    name
             "#,
             id,
             name,
             password_hash,
         )
         .fetch_one(&mut *self.0)
-        .await;
+        .await?;
 
-        let result = match insert_res {
-            Ok(id) => Ok(Some(id)),
-            Err(err) => {
-                if let Some(true) = err
-                    .as_database_error()
-                    .map(|db_err| db_err.is_unique_violation())
-                {
-                    // Login with the same username (or, very rarely, same ID) already
-                    // exists.
-                    Ok(None)
-                } else {
-                    Err(err)
-                }
-            }
-        }?;
-        Ok(result)
+        Ok(login)
     }
 
-    /// Authenticates `name` and `password` against an existing [Login]. Returns
-    /// that [Login] if one was found and the password was correct, or `None` if
-    /// either condition does not hold.
-    pub async fn authenticate(
+    /// Retrieves a login by name, plus its stored password hash for
+    /// verification. If there's no login with the requested name, this will
+    /// return [None].
+    pub async fn for_login(
         &mut self,
         name: &str,
-        password: &str,
-    ) -> Result<Option<Login>, BoxedError> {
-        let found = self.for_name(name).await?;
-
-        let login = if let Some((login, stored_hash)) = found {
-            if stored_hash.verify(password)? {
-                // User found and password validation succeeded.
-                Some(login)
-            } else {
-                // Password validation failed.
-                None
-            }
-        } else {
-            // User not found.
-            None
-        };
-
-        Ok(login)
-    }
-
-    async fn for_name(&mut self, name: &str) -> Result<Option<(Login, StoredHash)>, BoxedError> {
+    ) -> Result<Option<(Login, StoredHash)>, BoxedError> {
         let found = sqlx::query!(
             r#"
                 select
@@ -144,31 +105,3 @@ impl Id {
         BaseId::generate("L")
     }
 }
-
-#[derive(Debug, sqlx::Type)]
-#[sqlx(transparent)]
-struct StoredHash(String);
-
-impl StoredHash {
-    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))
-    }
-
-    fn verify(&self, password: &str) -> Result<bool, password_hash::Error> {
-        let hash = PasswordHash::new(&self.0)?;
-
-        match Argon2::default().verify_password(password.as_bytes(), &hash) {
-            // Successful authentication, not an error
-            Ok(()) => Ok(true),
-            // Unsuccessful authentication, also not an error
-            Err(password_hash::errors::Error::Password) => Ok(false),
-            // Password validation failed for some other reason, treat as an error
-            Err(err) => Err(err),
-        }
-    }
-}
diff --git a/src/login/routes.rs b/src/login/routes.rs
index 9cefe38..816926e 100644
--- a/src/login/routes.rs
+++ b/src/login/routes.rs
@@ -8,10 +8,7 @@ use axum::{
 
 use crate::{app::App, clock::RequestedAt, error::InternalError};
 
-use super::{
-    extract::IdentityToken,
-    repo::{logins::Provider as _, tokens::Provider as _},
-};
+use super::extract::IdentityToken;
 
 pub fn router() -> Router<App> {
     Router::new()
@@ -31,34 +28,7 @@ async fn on_login(
     identity: IdentityToken,
     Form(form): Form<LoginRequest>,
 ) -> Result<impl IntoResponse, InternalError> {
-    let mut tx = app.db.begin().await?;
-
-    // Spelling the following in the more conventional form,
-    //     if let Some(…) = create().await? {}
-    //     else if let Some(…) = validate().await? {}
-    //     else {}
-    // pushes the specifics of whether the returned error types are Send or not
-    // (they aren't) into the type of this function's generated Futures, which
-    // in turn makes this function unusable as an Axum handler.
-    let login = tx.logins().create(&form.name, &form.password).await?;
-    let login = if login.is_some() {
-        login
-    } else {
-        tx.logins().authenticate(&form.name, &form.password).await?
-    };
-
-    // If `login` is Some, then we have an identity and can issue an identity
-    // token. If `login` is None, then neither creating a new login nor authenticating
-    // an existing one succeeded, and we must reject the attempt.
-    //
-    // These properties will be transferred to `token`, as well.
-    let token = if let Some(login) = login {
-        Some(tx.tokens().issue(&login.id, now).await?)
-    } else {
-        None
-    };
-
-    tx.commit().await?;
+    let token = app.logins().login(&form.name, &form.password, now).await?;
 
     let resp = if let Some(token) = token {
         let identity = identity.set(&token);
@@ -91,9 +61,7 @@ async fn on_logout(
     identity: IdentityToken,
 ) -> Result<impl IntoResponse, InternalError> {
     if let Some(secret) = identity.secret() {
-        let mut tx = app.db.begin().await?;
-        tx.tokens().revoke(secret).await?;
-        tx.commit().await?;
+        app.logins().logout(secret).await?;
     }
 
     let identity = identity.clear();