From 777e4281431a036eb663b5eec70f347b7425737d Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Thu, 17 Oct 2024 01:45:58 -0400 Subject: Retain deleted messages and channels temporarily, to preserve events for replay. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when a channel (message) was deleted, `hi` would send events to all _connected_ clients to inform them of the deletion, then delete all memory of the channel (message). Any disconnected client, on reconnecting, would not receive the deletion event, and would de-synch with the service. The creation events were also immediately retconned out of the event stream, as well. With this change, `hi` keeps a record of deleted channels (messages). When replaying events, these records are used to replay the deletion event. After 7 days, the retained data is deleted, both to keep storage under control and to conform to users' expectations that deleted means gone. To match users' likely intuitions about what deletion does, deleting a channel (message) _does_ immediately delete some of its associated data. Channels' names are blanked, and messages' bodies are also blanked. When the event stream is replayed, the original channel.created (message.sent) event is "tombstoned", with an additional `deleted_at` field to inform clients. The included client does not use this field, at least yet. The migration is, once again, screamingingly complicated due to sqlite's limited ALTER TABLE … ALTER COLUMN support. This change also contains capabilities that would allow the API to return 410 Gone for deleted channels or messages, instead of 404. I did experiment with this, but it's tricky to do pervasively, especially since most app-level interfaces return an `Option` or `Option`. Redesigning these to return either `Ok(Channel)` (`Ok(Message)`) or `Err(Error::NotFound)` or `Err(Error::Deleted)` is more work than I wanted to take on for this change, and the utility of 410 Gone responses is not obvious to me. We have other, more pressing API design warts to address. --- src/test/fixtures/mod.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'src/test/fixtures/mod.rs') diff --git a/src/test/fixtures/mod.rs b/src/test/fixtures/mod.rs index 41f7e13..9658831 100644 --- a/src/test/fixtures/mod.rs +++ b/src/test/fixtures/mod.rs @@ -4,7 +4,6 @@ use crate::{app::App, clock::RequestedAt, db}; pub mod channel; pub mod event; -pub mod filter; pub mod future; pub mod identity; pub mod login; -- cgit v1.2.3 From 01f9f3549c76702fd56e58d44c5180fecddb4bfa Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Tue, 22 Oct 2024 23:25:24 -0400 Subject: Sort out the naming of the various parts of an identity. * A `cookie::Identity` (`IdentityCookie`) is a specialized CookieJar for working with identities. * An `Identity` is a token/login pair. I hope for this to be a bit more legible. In service of this, `Login` is no longer extractable. You have to get an identity. --- src/boot/routes/get.rs | 9 ++-- src/boot/routes/test.rs | 6 +-- src/channel/routes/channel/delete.rs | 4 +- src/channel/routes/channel/post.rs | 6 +-- src/channel/routes/channel/test.rs | 52 +++++++++++++++++--- src/channel/routes/post.rs | 4 +- src/channel/routes/test.rs | 6 +-- src/event/routes/test.rs | 26 +++------- src/invite/routes/invite/post.rs | 6 +-- src/invite/routes/post.rs | 10 ++-- src/login/extract.rs | 15 ------ src/login/mod.rs | 1 - src/login/routes/login/post.rs | 6 +-- src/login/routes/login/test.rs | 29 ++++++----- src/login/routes/logout/post.rs | 6 +-- src/login/routes/logout/test.rs | 14 +++--- src/message/app.rs | 2 - src/message/routes/message.rs | 10 ++-- src/setup/routes/post.rs | 6 +-- src/test/fixtures/cookie.rs | 37 ++++++++++++++ src/test/fixtures/identity.rs | 48 ++++++++---------- src/test/fixtures/login.rs | 4 +- src/test/fixtures/mod.rs | 1 + src/token/extract/cookie.rs | 94 ++++++++++++++++++++++++++++++++++++ src/token/extract/identity.rs | 6 +-- src/token/extract/identity_token.rs | 94 ------------------------------------ src/token/extract/mod.rs | 4 +- src/ui/routes/ch/channel.rs | 6 +-- src/ui/routes/get.rs | 6 +-- 29 files changed, 281 insertions(+), 237 deletions(-) delete mode 100644 src/login/extract.rs create mode 100644 src/test/fixtures/cookie.rs create mode 100644 src/token/extract/cookie.rs delete mode 100644 src/token/extract/identity_token.rs (limited to 'src/test/fixtures/mod.rs') diff --git a/src/boot/routes/get.rs b/src/boot/routes/get.rs index 737b479..563fbf1 100644 --- a/src/boot/routes/get.rs +++ b/src/boot/routes/get.rs @@ -3,11 +3,14 @@ use axum::{ response::{self, IntoResponse}, }; -use crate::{app::App, boot::Snapshot, error::Internal, login::Login}; +use crate::{app::App, boot::Snapshot, error::Internal, login::Login, token::extract::Identity}; -pub async fn handler(State(app): State, login: Login) -> Result { +pub async fn handler(State(app): State, identity: Identity) -> Result { let snapshot = app.boot().snapshot().await?; - Ok(Response { login, snapshot }) + Ok(Response { + login: identity.login, + snapshot, + }) } #[derive(serde::Serialize)] diff --git a/src/boot/routes/test.rs b/src/boot/routes/test.rs index 4023753..0430854 100644 --- a/src/boot/routes/test.rs +++ b/src/boot/routes/test.rs @@ -6,10 +6,10 @@ use crate::test::fixtures; #[tokio::test] async fn returns_identity() { let app = fixtures::scratch_app().await; - let login = fixtures::login::fictitious(); - let response = get::handler(State(app), login.clone()) + let identity = fixtures::identity::fictitious(); + let response = get::handler(State(app), identity.clone()) .await .expect("boot always succeeds"); - assert_eq!(login, response.login); + assert_eq!(identity.login, response.login); } diff --git a/src/channel/routes/channel/delete.rs b/src/channel/routes/channel/delete.rs index 5f40ddf..91eb506 100644 --- a/src/channel/routes/channel/delete.rs +++ b/src/channel/routes/channel/delete.rs @@ -9,14 +9,14 @@ use crate::{ channel::app, clock::RequestedAt, error::{Internal, NotFound}, - login::Login, + token::extract::Identity, }; pub async fn handler( State(app): State, Path(channel): Path, RequestedAt(deleted_at): RequestedAt, - _: Login, + _: Identity, ) -> Result { app.channels().delete(&channel, &deleted_at).await?; diff --git a/src/channel/routes/channel/post.rs b/src/channel/routes/channel/post.rs index d0cae05..b51e691 100644 --- a/src/channel/routes/channel/post.rs +++ b/src/channel/routes/channel/post.rs @@ -8,20 +8,20 @@ use crate::{ app::App, clock::RequestedAt, error::{Internal, NotFound}, - login::Login, message::{app::SendError, Body, Message}, + token::extract::Identity, }; pub async fn handler( State(app): State, Path(channel): Path, RequestedAt(sent_at): RequestedAt, - login: Login, + identity: Identity, Json(request): Json, ) -> Result { let message = app .messages() - .send(&channel, &login, &sent_at, &request.body) + .send(&channel, &identity.login, &sent_at, &request.body) .await?; Ok(Response(message)) diff --git a/src/channel/routes/channel/test.rs b/src/channel/routes/channel/test.rs index b895b69..9a2227d 100644 --- a/src/channel/routes/channel/test.rs +++ b/src/channel/routes/channel/test.rs @@ -14,7 +14,7 @@ async fn messages_in_order() { // Set up the environment let app = fixtures::scratch_app().await; - let sender = fixtures::login::create(&app, &fixtures::now()).await; + let sender = fixtures::identity::create(&app, &fixtures::now()).await; let channel = fixtures::channel::create(&app, &fixtures::now()).await; // Call the endpoint (twice) @@ -35,7 +35,7 @@ async fn messages_in_order() { Json(request), ) .await - .expect("sending to a valid channel"); + .expect("sending to a valid channel succeeds"); } // Verify the semantics @@ -44,7 +44,7 @@ async fn messages_in_order() { .events() .subscribe(None) .await - .expect("subscribing to a valid channel") + .expect("subscribing to a valid channel succeeds") .filter_map(fixtures::message::events) .take(requests.len()); @@ -55,7 +55,7 @@ async fn messages_in_order() { assert!(matches!( event, message::Event::Sent(event) - if event.message.sender == sender.id + if event.message.sender == sender.login.id && event.message.body == message )); } @@ -66,7 +66,7 @@ async fn nonexistent_channel() { // Set up the environment let app = fixtures::scratch_app().await; - let login = fixtures::login::create(&app, &fixtures::now()).await; + let sender = fixtures::identity::create(&app, &fixtures::now()).await; // Call the endpoint @@ -79,11 +79,49 @@ async fn nonexistent_channel() { State(app), Path(channel.clone()), sent_at, - login, + sender, Json(request), ) .await - .expect_err("sending to a nonexistent channel"); + .expect_err("sending to a nonexistent channel fails"); + + // Verify the structure of the response + + assert!(matches!( + error, + SendError::ChannelNotFound(error_channel) if channel == error_channel + )); +} + +#[tokio::test] +async fn deleted_channel() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::identity::create(&app, &fixtures::now()).await; + let channel = fixtures::channel::create(&app, &fixtures::now()).await; + + app.channels() + .delete(&channel.id, &fixtures::now()) + .await + .expect("deleting a new channel succeeds"); + + // Call the endpoint + + let sent_at = fixtures::now(); + let channel = channel::Id::generate(); + let request = post::Request { + body: fixtures::message::propose(), + }; + let post::Error(error) = post::handler( + State(app), + Path(channel.clone()), + sent_at, + sender, + Json(request), + ) + .await + .expect_err("sending to a deleted channel fails"); // Verify the structure of the response diff --git a/src/channel/routes/post.rs b/src/channel/routes/post.rs index 9781dd7..810445c 100644 --- a/src/channel/routes/post.rs +++ b/src/channel/routes/post.rs @@ -9,13 +9,13 @@ use crate::{ channel::{app, Channel}, clock::RequestedAt, error::Internal, - login::Login, name::Name, + token::extract::Identity, }; pub async fn handler( State(app): State, - _: Login, // requires auth, but doesn't actually care who you are + _: Identity, // requires auth, but doesn't actually care who you are RequestedAt(created_at): RequestedAt, Json(request): Json, ) -> Result { diff --git a/src/channel/routes/test.rs b/src/channel/routes/test.rs index 7879ba0..77f283b 100644 --- a/src/channel/routes/test.rs +++ b/src/channel/routes/test.rs @@ -14,7 +14,7 @@ async fn new_channel() { // Set up the environment let app = fixtures::scratch_app().await; - let creator = fixtures::login::create(&app, &fixtures::now()).await; + let creator = fixtures::identity::create(&app, &fixtures::now()).await; // Call the endpoint @@ -65,7 +65,7 @@ async fn duplicate_name() { // Set up the environment let app = fixtures::scratch_app().await; - let creator = fixtures::login::create(&app, &fixtures::now()).await; + let creator = fixtures::identity::create(&app, &fixtures::now()).await; let channel = fixtures::channel::create(&app, &fixtures::now()).await; // Call the endpoint @@ -91,7 +91,7 @@ async fn name_reusable_after_delete() { // Set up the environment let app = fixtures::scratch_app().await; - let creator = fixtures::login::create(&app, &fixtures::now()).await; + let creator = fixtures::identity::create(&app, &fixtures::now()).await; let name = fixtures::channel::propose(); // Call the endpoint (first time) diff --git a/src/event/routes/test.rs b/src/event/routes/test.rs index e6a8b9d..49f8094 100644 --- a/src/event/routes/test.rs +++ b/src/event/routes/test.rs @@ -22,8 +22,7 @@ async fn includes_historical_message() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let get::Response(events) = get::handler(State(app), subscriber, None, Query::default()) .await .expect("subscribe never fails"); @@ -49,8 +48,7 @@ async fn includes_live_message() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let get::Response(events) = get::handler(State(app.clone()), subscriber, None, Query::default()) .await @@ -95,8 +93,7 @@ async fn includes_multiple_channels() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let get::Response(events) = get::handler(State(app), subscriber, None, Query::default()) .await .expect("subscribe never fails"); @@ -133,8 +130,7 @@ async fn sequential_messages() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let get::Response(events) = get::handler(State(app), subscriber, None, Query::default()) .await .expect("subscribe never fails"); @@ -180,8 +176,7 @@ async fn resumes_from() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let resume_at = { // First subscription @@ -258,8 +253,7 @@ async fn serial_resume() { // Call the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber = fixtures::identity::identity(&app, &subscriber_creds, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let resume_at = { let initial_messages = [ @@ -382,7 +376,7 @@ async fn terminates_on_token_expiry() { let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; let subscriber = - fixtures::identity::identity(&app, &subscriber_creds, &fixtures::ancient()).await; + fixtures::identity::logged_in(&app, &subscriber_creds, &fixtures::ancient()).await; let get::Response(events) = get::handler(State(app.clone()), subscriber, None, Query::default()) @@ -426,11 +420,7 @@ async fn terminates_on_logout() { // Subscribe via the endpoint - let subscriber_creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let subscriber_token = - fixtures::identity::logged_in(&app, &subscriber_creds, &fixtures::now()).await; - let subscriber = - fixtures::identity::from_token(&app, &subscriber_token, &fixtures::now()).await; + let subscriber = fixtures::identity::create(&app, &fixtures::now()).await; let get::Response(events) = get::handler( State(app.clone()), diff --git a/src/invite/routes/invite/post.rs b/src/invite/routes/invite/post.rs index a41207a..3ca4e6b 100644 --- a/src/invite/routes/invite/post.rs +++ b/src/invite/routes/invite/post.rs @@ -11,16 +11,16 @@ use crate::{ invite::app, login::{Login, Password}, name::Name, - token::extract::IdentityToken, + token::extract::IdentityCookie, }; pub async fn handler( State(app): State, RequestedAt(accepted_at): RequestedAt, - identity: IdentityToken, + identity: IdentityCookie, Path(invite): Path, Json(request): Json, -) -> Result<(IdentityToken, Json), Error> { +) -> Result<(IdentityCookie, Json), Error> { let (login, secret) = app .invites() .accept(&invite, &request.name, &request.password, &accepted_at) diff --git a/src/invite/routes/post.rs b/src/invite/routes/post.rs index 80b1c27..eb7d706 100644 --- a/src/invite/routes/post.rs +++ b/src/invite/routes/post.rs @@ -1,17 +1,19 @@ use axum::extract::{Json, State}; -use crate::{app::App, clock::RequestedAt, error::Internal, invite::Invite, login::Login}; +use crate::{ + app::App, clock::RequestedAt, error::Internal, invite::Invite, token::extract::Identity, +}; pub async fn handler( State(app): State, RequestedAt(issued_at): RequestedAt, - login: Login, - // Require `{}` as the only valid request for this endpoint. + identity: Identity, _: Json, ) -> Result, Internal> { - let invite = app.invites().create(&login, &issued_at).await?; + let invite = app.invites().create(&identity.login, &issued_at).await?; Ok(Json(invite)) } +// Require `{}` as the only valid request for this endpoint. #[derive(Default, serde::Deserialize)] pub struct Request {} diff --git a/src/login/extract.rs b/src/login/extract.rs deleted file mode 100644 index c2d97f2..0000000 --- a/src/login/extract.rs +++ /dev/null @@ -1,15 +0,0 @@ -use axum::{extract::FromRequestParts, http::request::Parts}; - -use super::Login; -use crate::{app::App, token::extract::Identity}; - -#[async_trait::async_trait] -impl FromRequestParts for Login { - type Rejection = >::Rejection; - - async fn from_request_parts(parts: &mut Parts, state: &App) -> Result { - let identity = Identity::from_request_parts(parts, state).await?; - - Ok(identity.login) - } -} diff --git a/src/login/mod.rs b/src/login/mod.rs index 64a3698..279e9a6 100644 --- a/src/login/mod.rs +++ b/src/login/mod.rs @@ -1,6 +1,5 @@ pub mod app; pub mod event; -pub mod extract; mod history; mod id; pub mod password; diff --git a/src/login/routes/login/post.rs b/src/login/routes/login/post.rs index 20430db..96da5c5 100644 --- a/src/login/routes/login/post.rs +++ b/src/login/routes/login/post.rs @@ -10,15 +10,15 @@ use crate::{ error::Internal, login::{Login, Password}, name::Name, - token::{app, extract::IdentityToken}, + token::{app, extract::IdentityCookie}, }; pub async fn handler( State(app): State, RequestedAt(now): RequestedAt, - identity: IdentityToken, + identity: IdentityCookie, Json(request): Json, -) -> Result<(IdentityToken, Json), Error> { +) -> Result<(IdentityCookie, Json), Error> { let (login, secret) = app .tokens() .login(&request.name, &request.password, &now) diff --git a/src/login/routes/login/test.rs b/src/login/routes/login/test.rs index c94f14c..7399796 100644 --- a/src/login/routes/login/test.rs +++ b/src/login/routes/login/test.rs @@ -8,14 +8,14 @@ async fn correct_credentials() { // Set up the environment let app = fixtures::scratch_app().await; - let (login, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let (name, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; // Call the endpoint - let identity = fixtures::identity::not_logged_in(); + let identity = fixtures::cookie::not_logged_in(); let logged_in_at = fixtures::now(); let request = post::Request { - name: login.name.clone(), + name: name.clone(), password, }; let (identity, Json(response)) = @@ -25,8 +25,10 @@ async fn correct_credentials() { // Verify the return value's basic structure - assert_eq!(login, response); - let secret = identity.secret().expect("logged in with valid credentials"); + assert_eq!(name, response.name); + let secret = identity + .secret() + .expect("logged in with valid credentials issues an identity cookie"); // Verify the semantics @@ -37,7 +39,7 @@ async fn correct_credentials() { .await .expect("identity secret is valid"); - assert_eq!(login, validated_login); + assert_eq!(response, validated_login); } #[tokio::test] @@ -48,7 +50,7 @@ async fn invalid_name() { // Call the endpoint - let identity = fixtures::identity::not_logged_in(); + let identity = fixtures::cookie::not_logged_in(); let logged_in_at = fixtures::now(); let (name, password) = fixtures::login::propose(); let request = post::Request { @@ -58,7 +60,7 @@ async fn invalid_name() { let post::Error(error) = post::handler(State(app.clone()), logged_in_at, identity, Json(request)) .await - .expect_err("logged in with an incorrect password"); + .expect_err("logged in with an incorrect password fails"); // Verify the return value's basic structure @@ -75,7 +77,7 @@ async fn incorrect_password() { // Call the endpoint let logged_in_at = fixtures::now(); - let identity = fixtures::identity::not_logged_in(); + let identity = fixtures::cookie::not_logged_in(); let request = post::Request { name: login.name, password: fixtures::login::propose_password(), @@ -95,16 +97,13 @@ async fn token_expires() { // Set up the environment let app = fixtures::scratch_app().await; - let (login, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let (name, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; // Call the endpoint let logged_in_at = fixtures::ancient(); - let identity = fixtures::identity::not_logged_in(); - let request = post::Request { - name: login.name.clone(), - password, - }; + let identity = fixtures::cookie::not_logged_in(); + let request = post::Request { name, password }; let (identity, _) = post::handler(State(app.clone()), logged_in_at, identity, Json(request)) .await .expect("logged in with valid credentials"); diff --git a/src/login/routes/logout/post.rs b/src/login/routes/logout/post.rs index 6b7a62a..bb09b9f 100644 --- a/src/login/routes/logout/post.rs +++ b/src/login/routes/logout/post.rs @@ -8,15 +8,15 @@ use crate::{ app::App, clock::RequestedAt, error::{Internal, Unauthorized}, - token::{app, extract::IdentityToken}, + token::{app, extract::IdentityCookie}, }; pub async fn handler( State(app): State, RequestedAt(now): RequestedAt, - identity: IdentityToken, + identity: IdentityCookie, Json(_): Json, -) -> Result<(IdentityToken, StatusCode), Error> { +) -> Result<(IdentityCookie, StatusCode), Error> { if let Some(secret) = identity.secret() { let (token, _) = app.tokens().validate(&secret, &now).await?; app.tokens().logout(&token).await?; diff --git a/src/login/routes/logout/test.rs b/src/login/routes/logout/test.rs index 91837fe..775fa9f 100644 --- a/src/login/routes/logout/test.rs +++ b/src/login/routes/logout/test.rs @@ -12,9 +12,9 @@ async fn successful() { let app = fixtures::scratch_app().await; let now = fixtures::now(); - let login = fixtures::login::create_with_password(&app, &fixtures::now()).await; - let identity = fixtures::identity::logged_in(&app, &login, &now).await; - let secret = fixtures::identity::secret(&identity); + let creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let identity = fixtures::cookie::logged_in(&app, &creds, &now).await; + let secret = fixtures::cookie::secret(&identity); // Call the endpoint @@ -49,10 +49,10 @@ async fn no_identity() { // Call the endpoint - let identity = fixtures::identity::not_logged_in(); + let identity = fixtures::cookie::not_logged_in(); let (identity, status) = post::handler(State(app), fixtures::now(), identity, Json::default()) .await - .expect("logged out with no token"); + .expect("logged out with no token succeeds"); // Verify the return value's basic structure @@ -68,10 +68,10 @@ async fn invalid_token() { // Call the endpoint - let identity = fixtures::identity::fictitious(); + let identity = fixtures::cookie::fictitious(); let post::Error(error) = post::handler(State(app), fixtures::now(), identity, Json::default()) .await - .expect_err("logged out with an invalid token"); + .expect_err("logged out with an invalid token fails"); // Verify the return value's basic structure diff --git a/src/message/app.rs b/src/message/app.rs index 852b958..eed6ba4 100644 --- a/src/message/app.rs +++ b/src/message/app.rs @@ -136,8 +136,6 @@ impl From for SendError { #[derive(Debug, thiserror::Error)] pub enum DeleteError { - #[error("channel {0} not found")] - ChannelNotFound(channel::Id), #[error("message {0} not found")] NotFound(Id), #[error("message {0} deleted")] diff --git a/src/message/routes/message.rs b/src/message/routes/message.rs index fbef35a..f83cb39 100644 --- a/src/message/routes/message.rs +++ b/src/message/routes/message.rs @@ -9,15 +9,15 @@ pub mod delete { app::App, clock::RequestedAt, error::{Internal, NotFound}, - login::Login, message::{self, app::DeleteError}, + token::extract::Identity, }; pub async fn handler( State(app): State, Path(message): Path, RequestedAt(deleted_at): RequestedAt, - _: Login, + _: Identity, ) -> Result { app.messages().delete(&message, &deleted_at).await?; @@ -33,9 +33,9 @@ pub mod delete { let Self(error) = self; #[allow(clippy::match_wildcard_for_single_variants)] match error { - DeleteError::ChannelNotFound(_) - | DeleteError::NotFound(_) - | DeleteError::Deleted(_) => NotFound(error).into_response(), + DeleteError::NotFound(_) | DeleteError::Deleted(_) => { + NotFound(error).into_response() + } other => Internal::from(other).into_response(), } } diff --git a/src/setup/routes/post.rs b/src/setup/routes/post.rs index fb2280a..f7b256e 100644 --- a/src/setup/routes/post.rs +++ b/src/setup/routes/post.rs @@ -11,15 +11,15 @@ use crate::{ login::{Login, Password}, name::Name, setup::app, - token::extract::IdentityToken, + token::extract::IdentityCookie, }; pub async fn handler( State(app): State, RequestedAt(setup_at): RequestedAt, - identity: IdentityToken, + identity: IdentityCookie, Json(request): Json, -) -> Result<(IdentityToken, Json), Error> { +) -> Result<(IdentityCookie, Json), Error> { let (login, secret) = app .setup() .initial(&request.name, &request.password, &setup_at) diff --git a/src/test/fixtures/cookie.rs b/src/test/fixtures/cookie.rs new file mode 100644 index 0000000..58777c8 --- /dev/null +++ b/src/test/fixtures/cookie.rs @@ -0,0 +1,37 @@ +use uuid::Uuid; + +use crate::{ + app::App, + clock::RequestedAt, + login::Password, + name::Name, + token::{extract::IdentityCookie, Secret}, +}; + +pub fn not_logged_in() -> IdentityCookie { + IdentityCookie::new() +} + +pub async fn logged_in( + app: &App, + credentials: &(Name, Password), + now: &RequestedAt, +) -> IdentityCookie { + let (name, password) = credentials; + let (_, token) = app + .tokens() + .login(name, password, now) + .await + .expect("should succeed given known-valid credentials"); + + IdentityCookie::new().set(token) +} + +pub fn secret(identity: &IdentityCookie) -> Secret { + identity.secret().expect("identity contained a secret") +} + +pub fn fictitious() -> IdentityCookie { + let token = Uuid::new_v4().to_string(); + IdentityCookie::new().set(token) +} diff --git a/src/test/fixtures/identity.rs b/src/test/fixtures/identity.rs index c434473..e438f2b 100644 --- a/src/test/fixtures/identity.rs +++ b/src/test/fixtures/identity.rs @@ -1,31 +1,21 @@ -use uuid::Uuid; - use crate::{ app::App, clock::RequestedAt, - login::{Login, Password}, + login::Password, + name::Name, + test::fixtures, token::{ - extract::{Identity, IdentityToken}, - Secret, + self, + extract::{Identity, IdentityCookie}, }, }; -pub fn not_logged_in() -> IdentityToken { - IdentityToken::new() -} - -pub async fn logged_in(app: &App, login: &(Login, Password), now: &RequestedAt) -> IdentityToken { - let (login, password) = login; - let (_, token) = app - .tokens() - .login(&login.name, password, now) - .await - .expect("should succeed given known-valid credentials"); - - IdentityToken::new().set(token) +pub async fn create(app: &App, created_at: &RequestedAt) -> Identity { + let credentials = fixtures::login::create_with_password(app, created_at).await; + logged_in(app, &credentials, created_at).await } -pub async fn from_token(app: &App, token: &IdentityToken, issued_at: &RequestedAt) -> Identity { +pub async fn from_cookie(app: &App, token: &IdentityCookie, issued_at: &RequestedAt) -> Identity { let secret = token.secret().expect("identity token has a secret"); let (token, login) = app .tokens() @@ -36,16 +26,18 @@ pub async fn from_token(app: &App, token: &IdentityToken, issued_at: &RequestedA Identity { token, login } } -pub async fn identity(app: &App, login: &(Login, Password), issued_at: &RequestedAt) -> Identity { - let secret = logged_in(app, login, issued_at).await; - from_token(app, &secret, issued_at).await +pub async fn logged_in( + app: &App, + credentials: &(Name, Password), + issued_at: &RequestedAt, +) -> Identity { + let secret = fixtures::cookie::logged_in(app, credentials, issued_at).await; + from_cookie(app, &secret, issued_at).await } -pub fn secret(identity: &IdentityToken) -> Secret { - identity.secret().expect("identity contained a secret") -} +pub fn fictitious() -> Identity { + let token = token::Id::generate(); + let login = fixtures::login::fictitious(); -pub fn fictitious() -> IdentityToken { - let token = Uuid::new_v4().to_string(); - IdentityToken::new().set(token) + Identity { token, login } } diff --git a/src/test/fixtures/login.rs b/src/test/fixtures/login.rs index 714b936..e308289 100644 --- a/src/test/fixtures/login.rs +++ b/src/test/fixtures/login.rs @@ -8,7 +8,7 @@ use crate::{ name::Name, }; -pub async fn create_with_password(app: &App, created_at: &RequestedAt) -> (Login, Password) { +pub async fn create_with_password(app: &App, created_at: &RequestedAt) -> (Name, Password) { let (name, password) = propose(); let login = app .logins() @@ -16,7 +16,7 @@ pub async fn create_with_password(app: &App, created_at: &RequestedAt) -> (Login .await .expect("should always succeed if the login is actually new"); - (login, password) + (login.name, password) } pub async fn create(app: &App, created_at: &RequestedAt) -> Login { diff --git a/src/test/fixtures/mod.rs b/src/test/fixtures/mod.rs index 9658831..5609ebc 100644 --- a/src/test/fixtures/mod.rs +++ b/src/test/fixtures/mod.rs @@ -3,6 +3,7 @@ use chrono::{TimeDelta, Utc}; use crate::{app::App, clock::RequestedAt, db}; pub mod channel; +pub mod cookie; pub mod event; pub mod future; pub mod identity; diff --git a/src/token/extract/cookie.rs b/src/token/extract/cookie.rs new file mode 100644 index 0000000..af5787d --- /dev/null +++ b/src/token/extract/cookie.rs @@ -0,0 +1,94 @@ +use std::fmt; + +use axum::{ + extract::FromRequestParts, + http::request::Parts, + response::{IntoResponseParts, ResponseParts}, +}; +use axum_extra::extract::cookie::{Cookie, CookieJar}; + +use crate::token::Secret; + +// 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)] +pub struct Identity { + cookies: CookieJar, +} + +impl fmt::Debug for Identity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("IdentityCookie") + .field("identity", &self.secret()) + .finish() + } +} + +impl Identity { + const COOKIE_NAME: &str = "identity"; + + // Creates a new, unpopulated identity token store. + #[cfg(test)] + pub fn new() -> Self { + Self { + cookies: CookieJar::new(), + } + } + + // Get the identity secret sent in the request, if any. If the identity + // was not sent, or if it has previously been [clear]ed, then this will + // 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 { + self.cookies + .get(Self::COOKIE_NAME) + .map(Cookie::value) + .map(Secret::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: impl Into) -> Self { + let secret = secret.into().reveal(); + let identity_cookie = Cookie::build((Self::COOKIE_NAME, secret)) + .http_only(true) + .path("/") + .permanent() + .build(); + + Self { + cookies: self.cookies.add(identity_cookie), + } + } + + // Remove the identity secret and ensure that it will be cleared when this + // extractor is included in a response. + pub fn clear(self) -> Self { + Self { + cookies: self.cookies.remove(Self::COOKIE_NAME), + } + } +} + +#[async_trait::async_trait] +impl FromRequestParts for Identity +where + S: Send + Sync, +{ + type Rejection = >::Rejection; + + async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { + let cookies = CookieJar::from_request_parts(parts, state).await?; + Ok(Self { cookies }) + } +} + +impl IntoResponseParts for Identity { + type Error = ::Error; + + fn into_response_parts(self, res: ResponseParts) -> Result { + let Self { cookies } = self; + cookies.into_response_parts(res) + } +} diff --git a/src/token/extract/identity.rs b/src/token/extract/identity.rs index 8b3cd94..a69f509 100644 --- a/src/token/extract/identity.rs +++ b/src/token/extract/identity.rs @@ -4,7 +4,7 @@ use axum::{ response::{IntoResponse, Response}, }; -use super::IdentityToken; +use super::IdentityCookie; use crate::{ app::App, @@ -25,10 +25,10 @@ impl FromRequestParts for Identity { type Rejection = LoginError; async fn from_request_parts(parts: &mut Parts, state: &App) -> Result { - let Ok(identity_token) = IdentityToken::from_request_parts(parts, state).await; + let Ok(cookie) = IdentityCookie::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::Unauthorized)?; + let secret = cookie.secret().ok_or(LoginError::Unauthorized)?; let app = State::::from_request_parts(parts, state).await?; match app.tokens().validate(&secret, &used_at).await { diff --git a/src/token/extract/identity_token.rs b/src/token/extract/identity_token.rs deleted file mode 100644 index a1e900e..0000000 --- a/src/token/extract/identity_token.rs +++ /dev/null @@ -1,94 +0,0 @@ -use std::fmt; - -use axum::{ - extract::FromRequestParts, - http::request::Parts, - response::{IntoResponseParts, ResponseParts}, -}; -use axum_extra::extract::cookie::{Cookie, CookieJar}; - -use crate::token::Secret; - -// 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)] -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.secret()) - .finish() - } -} - -impl IdentityToken { - // Creates a new, unpopulated identity token store. - #[cfg(test)] - pub fn new() -> Self { - Self { - cookies: CookieJar::new(), - } - } - - // Get the identity secret sent in the request, if any. If the identity - // was not sent, or if it has previously been [clear]ed, then this will - // 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 { - self.cookies - .get(IDENTITY_COOKIE) - .map(Cookie::value) - .map(Secret::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: impl Into) -> Self { - let secret = secret.into().reveal(); - let identity_cookie = Cookie::build((IDENTITY_COOKIE, secret)) - .http_only(true) - .path("/") - .permanent() - .build(); - - Self { - cookies: self.cookies.add(identity_cookie), - } - } - - // Remove the identity secret and ensure that it will be cleared when this - // extractor is included in a response. - pub fn clear(self) -> Self { - Self { - cookies: self.cookies.remove(IDENTITY_COOKIE), - } - } -} - -const IDENTITY_COOKIE: &str = "identity"; - -#[async_trait::async_trait] -impl FromRequestParts for IdentityToken -where - S: Send + Sync, -{ - type Rejection = >::Rejection; - - async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { - let cookies = CookieJar::from_request_parts(parts, state).await?; - Ok(Self { cookies }) - } -} - -impl IntoResponseParts for IdentityToken { - type Error = ::Error; - - fn into_response_parts(self, res: ResponseParts) -> Result { - let Self { cookies } = self; - cookies.into_response_parts(res) - } -} diff --git a/src/token/extract/mod.rs b/src/token/extract/mod.rs index b4800ae..fc0f52b 100644 --- a/src/token/extract/mod.rs +++ b/src/token/extract/mod.rs @@ -1,4 +1,4 @@ +mod cookie; mod identity; -mod identity_token; -pub use self::{identity::Identity, identity_token::IdentityToken}; +pub use self::{cookie::Identity as IdentityCookie, identity::Identity}; diff --git a/src/ui/routes/ch/channel.rs b/src/ui/routes/ch/channel.rs index 353d000..a338f1f 100644 --- a/src/ui/routes/ch/channel.rs +++ b/src/ui/routes/ch/channel.rs @@ -8,7 +8,7 @@ pub mod get { app::App, channel, error::Internal, - login::Login, + token::extract::Identity, ui::{ assets::{Asset, Assets}, error::NotFound, @@ -17,10 +17,10 @@ pub mod get { pub async fn handler( State(app): State, - login: Option, + identity: Option, Path(channel): Path, ) -> Result { - login.ok_or(Error::NotLoggedIn)?; + let _ = identity.ok_or(Error::NotLoggedIn)?; app.channels() .get(&channel) .await diff --git a/src/ui/routes/get.rs b/src/ui/routes/get.rs index 97737e1..2fcb51c 100644 --- a/src/ui/routes/get.rs +++ b/src/ui/routes/get.rs @@ -2,12 +2,12 @@ use axum::response::{self, IntoResponse, Redirect}; use crate::{ error::Internal, - login::Login, + token::extract::Identity, ui::assets::{Asset, Assets}, }; -pub async fn handler(login: Option) -> Result { - login.ok_or(Error::NotLoggedIn)?; +pub async fn handler(identity: Option) -> Result { + let _ = identity.ok_or(Error::NotLoggedIn)?; Assets::index().map_err(Error::Internal) } -- cgit v1.2.3 From b614d0a754b3432dac1624410f37579712decc41 Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Wed, 23 Oct 2024 12:00:30 -0400 Subject: Tests for `DELETE /api/messages/:id` --- src/message/routes/message.rs | 43 ---------- src/message/routes/message/mod.rs | 46 +++++++++++ src/message/routes/message/test.rs | 160 +++++++++++++++++++++++++++++++++++++ src/test/fixtures/message.rs | 4 + src/test/fixtures/mod.rs | 5 ++ 5 files changed, 215 insertions(+), 43 deletions(-) delete mode 100644 src/message/routes/message.rs create mode 100644 src/message/routes/message/mod.rs create mode 100644 src/message/routes/message/test.rs (limited to 'src/test/fixtures/mod.rs') diff --git a/src/message/routes/message.rs b/src/message/routes/message.rs deleted file mode 100644 index f83cb39..0000000 --- a/src/message/routes/message.rs +++ /dev/null @@ -1,43 +0,0 @@ -pub mod delete { - use axum::{ - extract::{Path, State}, - http::StatusCode, - response::{IntoResponse, Response}, - }; - - use crate::{ - app::App, - clock::RequestedAt, - error::{Internal, NotFound}, - message::{self, app::DeleteError}, - token::extract::Identity, - }; - - pub async fn handler( - State(app): State, - Path(message): Path, - RequestedAt(deleted_at): RequestedAt, - _: Identity, - ) -> Result { - app.messages().delete(&message, &deleted_at).await?; - - Ok(StatusCode::ACCEPTED) - } - - #[derive(Debug, thiserror::Error)] - #[error(transparent)] - pub struct Error(#[from] pub DeleteError); - - impl IntoResponse for Error { - fn into_response(self) -> Response { - let Self(error) = self; - #[allow(clippy::match_wildcard_for_single_variants)] - match error { - DeleteError::NotFound(_) | DeleteError::Deleted(_) => { - NotFound(error).into_response() - } - other => Internal::from(other).into_response(), - } - } - } -} diff --git a/src/message/routes/message/mod.rs b/src/message/routes/message/mod.rs new file mode 100644 index 0000000..545ad26 --- /dev/null +++ b/src/message/routes/message/mod.rs @@ -0,0 +1,46 @@ +#[cfg(test)] +mod test; + +pub mod delete { + use axum::{ + extract::{Path, State}, + http::StatusCode, + response::{IntoResponse, Response}, + }; + + use crate::{ + app::App, + clock::RequestedAt, + error::{Internal, NotFound}, + message::{self, app::DeleteError}, + token::extract::Identity, + }; + + pub async fn handler( + State(app): State, + Path(message): Path, + RequestedAt(deleted_at): RequestedAt, + _: Identity, + ) -> Result { + app.messages().delete(&message, &deleted_at).await?; + + Ok(StatusCode::ACCEPTED) + } + + #[derive(Debug, thiserror::Error)] + #[error(transparent)] + pub struct Error(#[from] pub DeleteError); + + impl IntoResponse for Error { + fn into_response(self) -> Response { + let Self(error) = self; + #[allow(clippy::match_wildcard_for_single_variants)] + match error { + DeleteError::NotFound(_) | DeleteError::Deleted(_) => { + NotFound(error).into_response() + } + other => Internal::from(other).into_response(), + } + } + } +} diff --git a/src/message/routes/message/test.rs b/src/message/routes/message/test.rs new file mode 100644 index 0000000..2016fb8 --- /dev/null +++ b/src/message/routes/message/test.rs @@ -0,0 +1,160 @@ +use axum::{ + extract::{Path, State}, + http::StatusCode, +}; + +use super::delete; +use crate::{message::app, test::fixtures}; + +#[tokio::test] +pub async fn delete_message() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::login::create(&app, &fixtures::now()).await; + let channel = fixtures::channel::create(&app, &fixtures::now()).await; + let message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let response = delete::handler( + State(app.clone()), + Path(message.id.clone()), + fixtures::now(), + deleter, + ) + .await + .expect("deleting a valid message succeeds"); + + // Verify the response + + assert_eq!(response, StatusCode::ACCEPTED); + + // Verify the semantics + + let snapshot = app.boot().snapshot().await.expect("boot always succeeds"); + assert!(!snapshot.messages.contains(&message)); +} + +#[tokio::test] +pub async fn delete_invalid_message_id() { + // Set up the environment + + let app = fixtures::scratch_app().await; + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let message = fixtures::message::fictitious(); + let delete::Error(error) = delete::handler( + State(app.clone()), + Path(message.clone()), + fixtures::now(), + deleter, + ) + .await + .expect_err("deleting a nonexistent message fails"); + + // Verify the response + + assert!(matches!(error, app::DeleteError::NotFound(id) if id == message)); +} + +#[tokio::test] +pub async fn delete_deleted() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::login::create(&app, &fixtures::now()).await; + let channel = fixtures::channel::create(&app, &fixtures::now()).await; + let message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; + + app.messages() + .delete(&message.id, &fixtures::now()) + .await + .expect("deleting a recently-sent message succeeds"); + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let delete::Error(error) = delete::handler( + State(app.clone()), + Path(message.id.clone()), + fixtures::now(), + deleter, + ) + .await + .expect_err("deleting a deleted message fails"); + + // Verify the response + + assert!(matches!(error, app::DeleteError::Deleted(id) if id == message.id)); +} + +#[tokio::test] +pub async fn delete_expired() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::login::create(&app, &fixtures::ancient()).await; + let channel = fixtures::channel::create(&app, &fixtures::ancient()).await; + let message = fixtures::message::send(&app, &channel, &sender, &fixtures::ancient()).await; + + app.messages() + .expire(&fixtures::now()) + .await + .expect("expiring messages always succeeds"); + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let delete::Error(error) = delete::handler( + State(app.clone()), + Path(message.id.clone()), + fixtures::now(), + deleter, + ) + .await + .expect_err("deleting an expired message fails"); + + // Verify the response + + assert!(matches!(error, app::DeleteError::Deleted(id) if id == message.id)); +} + +#[tokio::test] +pub async fn delete_purged() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::login::create(&app, &fixtures::ancient()).await; + let channel = fixtures::channel::create(&app, &fixtures::ancient()).await; + let message = fixtures::message::send(&app, &channel, &sender, &fixtures::ancient()).await; + + app.messages() + .expire(&fixtures::old()) + .await + .expect("expiring messages always succeeds"); + + app.messages() + .purge(&fixtures::now()) + .await + .expect("purging messages always succeeds"); + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let delete::Error(error) = delete::handler( + State(app.clone()), + Path(message.id.clone()), + fixtures::now(), + deleter, + ) + .await + .expect_err("deleting a purged message fails"); + + // Verify the response + + assert!(matches!(error, app::DeleteError::NotFound(id) if id == message.id)); +} diff --git a/src/test/fixtures/message.rs b/src/test/fixtures/message.rs index 38b511f..3aebdd9 100644 --- a/src/test/fixtures/message.rs +++ b/src/test/fixtures/message.rs @@ -30,3 +30,7 @@ pub fn events(event: Event) -> future::Ready> { _ => None, }) } + +pub fn fictitious() -> message::Id { + message::Id::generate() +} diff --git a/src/test/fixtures/mod.rs b/src/test/fixtures/mod.rs index 5609ebc..9111811 100644 --- a/src/test/fixtures/mod.rs +++ b/src/test/fixtures/mod.rs @@ -21,6 +21,11 @@ pub fn now() -> RequestedAt { Utc::now().into() } +pub fn old() -> RequestedAt { + let timestamp = Utc::now() - TimeDelta::days(95); + timestamp.into() +} + pub fn ancient() -> RequestedAt { let timestamp = Utc::now() - TimeDelta::days(365); timestamp.into() -- cgit v1.2.3