From f3bd6dd41f524f9fb6065e044274c880264007ee Mon Sep 17 00:00:00 2001 From: Miguel Oller Date: Sun, 11 Feb 2024 12:00:50 -0500 Subject: [PATCH] feat: add admin newsletter publish form Also moved POST /newsletters to POST /admin/newsletters. --- Cargo.lock | 1 - Cargo.toml | 1 - src/routes/admin/newsletters/get.rs | 49 +++++- src/routes/admin/newsletters/mod.rs | 4 +- .../newsletters/post.rs} | 73 ++------- src/routes/mod.rs | 2 - src/startup.rs | 6 +- tests/api/helpers.rs | 10 +- tests/api/newsletter.rs | 150 ++++++------------ 9 files changed, 123 insertions(+), 173 deletions(-) rename src/routes/{newsletters.rs => admin/newsletters/post.rs} (59%) diff --git a/Cargo.lock b/Cargo.lock index 67e86ca..b28f144 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3718,7 +3718,6 @@ dependencies = [ "actix-web-lab", "anyhow", "argon2", - "base64 0.21.4", "chrono", "claims", "config", diff --git a/Cargo.toml b/Cargo.toml index d9787bc..d33dbad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,6 @@ actix-web-flash-messages = { version = "0.4", features = ["cookies"] } actix-web-lab = "0.18" anyhow = "1" argon2 = { version = "0.4", features = ["std"] } -base64 = "0.21" chrono = { version = "0.4.22", default-features = false, features = ["clock"] } config = "0.13" rand = { version = "0.8", features = ["std_rng"] } diff --git a/src/routes/admin/newsletters/get.rs b/src/routes/admin/newsletters/get.rs index f68b5db..f8c24a1 100644 --- a/src/routes/admin/newsletters/get.rs +++ b/src/routes/admin/newsletters/get.rs @@ -1,5 +1,50 @@ +use actix_web::http::header::ContentType; +use actix_web::http::StatusCode; use actix_web::HttpResponse; +use actix_web_flash_messages::IncomingFlashMessages; +use std::fmt::Write; -pub async fn submit_newsletter_form() -> HttpResponse { - HttpResponse::Ok().finish() +pub async fn publish_newsletter_form(flash_messages: IncomingFlashMessages) -> HttpResponse { + let mut msg_html = String::new(); + for m in flash_messages.iter() { + write!(msg_html, "

{}

", m.content()).unwrap(); + } + + let html_body = format!( + r#" + + + + Submit Newsletter + + + {msg_html} +
+ +
+ +
+ +
+ +
+ +"# + ); + + HttpResponse::build(StatusCode::OK) + .content_type(ContentType::html()) + .body(html_body) } diff --git a/src/routes/admin/newsletters/mod.rs b/src/routes/admin/newsletters/mod.rs index c3116d1..146f7bc 100644 --- a/src/routes/admin/newsletters/mod.rs +++ b/src/routes/admin/newsletters/mod.rs @@ -1,3 +1,5 @@ mod get; +mod post; -pub use get::*; +pub use get::publish_newsletter_form; +pub use post::publish_newsletter; diff --git a/src/routes/newsletters.rs b/src/routes/admin/newsletters/post.rs similarity index 59% rename from src/routes/newsletters.rs rename to src/routes/admin/newsletters/post.rs index 998c33a..2a9d2d1 100644 --- a/src/routes/newsletters.rs +++ b/src/routes/admin/newsletters/post.rs @@ -1,26 +1,19 @@ -use crate::authentication::{validate_credentials, AuthError, Credentials}; +use crate::authentication::UserId; use crate::domain::SubscriberEmail; use crate::email_client::EmailClient; +use crate::routes::admin::dashboard::get_username; use crate::routes::error_chain_fmt; -use actix_web::http::header::HeaderMap; use actix_web::http::header::HeaderValue; use actix_web::http::{header, StatusCode}; -use actix_web::{web, HttpRequest, HttpResponse, ResponseError}; +use actix_web::{web, HttpResponse, ResponseError}; use anyhow::Context; -use base64::Engine; -use secrecy::Secret; use sqlx::PgPool; #[derive(serde::Deserialize)] -pub struct BodyData { +pub struct FormData { title: String, - content: Content, -} - -#[derive(serde::Deserialize)] -pub struct Content { - html: String, - text: String, + html_content: String, + text_content: String, } #[derive(thiserror::Error)] @@ -59,26 +52,19 @@ impl ResponseError for PublishError { #[tracing::instrument( name = "Publish a newsletter issue", - skip(body, pool, email_client, request), + skip(body, pool, email_client), fields(username=tracing::field::Empty, user_id=tracing::field::Empty) )] pub async fn publish_newsletter( - body: web::Json, + body: web::Form, pool: web::Data, email_client: web::Data, - request: HttpRequest, + user_id: web::ReqData, ) -> Result { - let credentials = basic_authentication(request.headers()).map_err(PublishError::AuthError)?; - - tracing::Span::current().record("username", &tracing::field::display(&credentials.username)); - - let user_id = validate_credentials(credentials, &pool) - .await - .map_err(|e| match e { - AuthError::InvalidCredentials(_) => PublishError::AuthError(e.into()), - AuthError::UnexpectedError(_) => PublishError::UnexpectedError(e.into()), - })?; + let user_id = user_id.into_inner(); + let username = get_username(*user_id, &pool).await?; + tracing::Span::current().record("username", &tracing::field::display(&username)); tracing::Span::current().record("user_id", &tracing::field::display(&user_id)); let subscribers = get_confirmed_subscribers(&pool).await?; @@ -90,8 +76,8 @@ pub async fn publish_newsletter( .send_email( &subscriber.email, &body.title, - &body.content.html, - &body.content.text, + &body.html_content, + &body.text_content, ) .await .with_context(|| { @@ -111,37 +97,6 @@ pub async fn publish_newsletter( Ok(HttpResponse::Ok().finish()) } -fn basic_authentication(headers: &HeaderMap) -> Result { - let header_value = headers - .get("Authorization") - .context("The 'Authorization' header was missing.")? - .to_str() - .context("The 'Authorization' header was not a valid UTF8 string.")?; - let base64encoded_segment = header_value - .strip_prefix("Basic ") - .context("The authorization scheme was not 'Basic'.")?; - let decoded_bytes = base64::engine::general_purpose::STANDARD - .decode(base64encoded_segment) - .context("Failed to base64-decode 'Basic' credentials.")?; - let decoded_credentials = String::from_utf8(decoded_bytes) - .context("The decoded credential string is not valid UTF8.")?; - - let mut credentials = decoded_credentials.splitn(2, ':'); - let username = credentials - .next() - .ok_or_else(|| anyhow::anyhow!("A username must be provided in 'Basic' auth."))? - .to_string(); - let password = credentials - .next() - .ok_or_else(|| anyhow::anyhow!("A password must be provided in 'Basic' auth."))? - .to_string(); - - Ok(Credentials { - username, - password: Secret::new(password), - }) -} - struct ConfirmedSubscriber { email: SubscriberEmail, } diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 671805f..772876f 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -2,7 +2,6 @@ mod admin; mod health_check; mod home; mod login; -mod newsletters; mod subscriptions; mod subscriptions_confirm; @@ -10,6 +9,5 @@ pub use admin::*; pub use health_check::*; pub use home::*; pub use login::*; -pub use newsletters::*; pub use subscriptions::*; pub use subscriptions_confirm::*; diff --git a/src/startup.rs b/src/startup.rs index 65b223a..6dd381b 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -4,7 +4,7 @@ use crate::{ email_client::EmailClient, routes::{ admin_dashboard, change_password, change_password_form, confirm, health_check, home, - log_out, login, login_form, publish_newsletter, submit_newsletter_form, subscribe, + log_out, login, login_form, publish_newsletter, publish_newsletter_form, subscribe, }, }; use actix_session::storage::RedisSessionStore; @@ -101,14 +101,14 @@ async fn run( .route("/login", web::get().to(login_form)) .route("/login", web::post().to(login)) .route("/health_check", web::get().to(health_check)) - .route("/newsletters", web::post().to(publish_newsletter)) .route("/subscriptions", web::post().to(subscribe)) .route("/subscriptions/confirm", web::get().to(confirm)) .service( web::scope("/admin") .wrap(from_fn(reject_anonymous_users)) .route("/dashboard", web::get().to(admin_dashboard)) - .route("/newsletters", web::get().to(submit_newsletter_form)) + .route("/newsletters", web::get().to(publish_newsletter_form)) + .route("/newsletters", web::post().to(publish_newsletter)) .route("/password", web::get().to(change_password_form)) .route("/password", web::post().to(change_password)) .route("/logout", web::post().to(log_out)), diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index ede8ef6..fc921fb 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -155,11 +155,13 @@ impl TestApp { .expect("Failed to execute request.") } - pub async fn post_newsletters(&self, body: serde_json::Value) -> reqwest::Response { + pub async fn post_newsletters(&self, body: &Body) -> reqwest::Response + where + Body: serde::Serialize, + { self.api_client - .post(&format!("{}/newsletters", &self.address)) - .basic_auth(&self.test_user.username, Some(&self.test_user.password)) - .json(&body) + .post(&format!("{}/admin/newsletters", &self.address)) + .form(&body) .send() .await .expect("Failed to execute request.") diff --git a/tests/api/newsletter.rs b/tests/api/newsletter.rs index c4419a0..a58fa0c 100644 --- a/tests/api/newsletter.rs +++ b/tests/api/newsletter.rs @@ -1,10 +1,9 @@ -use uuid::Uuid; use wiremock::{ matchers::{any, method, path}, Mock, ResponseTemplate, }; -use crate::helpers::{spawn_app, ConfirmationLinks, TestApp}; +use crate::helpers::{assert_is_redirect_to, spawn_app, ConfirmationLinks, TestApp}; #[tokio::test] async fn newsletters_are_not_delivered_to_unconfirmed_subscribers() { @@ -19,15 +18,19 @@ async fn newsletters_are_not_delivered_to_unconfirmed_subscribers() { .mount(&app.email_server) .await; + app.post_login(&serde_json::json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + // Act let newsletter_request_body = serde_json::json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } + "title": "Newsletter Title", + "text_content": "Newsletter body as plain text", + "html_content": "

Newsletter body as HTML

" }); - let response = app.post_newsletters(newsletter_request_body).await; + let response = app.post_newsletters(&newsletter_request_body).await; // Assert assert_eq!(response.status().as_u16(), 200); @@ -49,15 +52,19 @@ async fn newsletters_are_delivered_to_confirmed_subscribers() { .mount(&app.email_server) .await; + app.post_login(&serde_json::json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + // Act let newsletter_request_body = serde_json::json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } + "title": "Newsletter Title", + "text_content": "Newsletter body as plain text", + "html_content": "

Newsletter body as HTML

" }); - let response = app.post_newsletters(newsletter_request_body).await; + let response = app.post_newsletters(&newsletter_request_body).await; // Assert assert_eq!(response.status().as_u16(), 200); @@ -72,22 +79,36 @@ async fn newsletters_returns_400_for_invalid_data() { let test_cases = vec![ ( serde_json::json!({ - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } + "text_content": "Newsletter body as plain text", + "html_content": "

Newsletter body as HTML

" }), "missing title", ), ( - serde_json::json!({"title": "Newsletter!"}), - "missing content", + serde_json::json!({ + "title": "Newsletter!", + "text_content": "Newsletter body as plain text", + }), + "missing HTML content", + ), + ( + serde_json::json!({ + "title": "Newsletter!", + "html_content": "

Newsletter body as HTML

" + }), + "missing text content", ), ]; + app.post_login(&serde_json::json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + for (invalid_body, error_message) in test_cases { // Act - let response = app.post_newsletters(invalid_body).await; + let response = app.post_newsletters(&invalid_body).await; // Assert assert_eq!( @@ -139,90 +160,19 @@ async fn create_confirmed_subscriber(app: &TestApp) { } #[tokio::test] -async fn requests_missing_authorization_are_rejected() { - // Arrange - let app = spawn_app().await; - - // Act - let response = reqwest::Client::new() - .post(&format!("{}/newsletters", &app.address)) - .json(&serde_json::json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } - })) - .send() - .await - .expect("Failed to execute request."); - - // Assert - assert_eq!(401, response.status().as_u16()); - assert_eq!( - r#"Basic realm="publish""#, - response.headers()["WWW-Authenticate"] - ); -} - -#[tokio::test] -async fn non_existing_user_is_rejected() { +async fn redirected_to_login_if_not_logged_in() { // Arrange let app = spawn_app().await; - let username = Uuid::new_v4().to_string(); - let password = Uuid::new_v4().to_string(); // Act - let response = reqwest::Client::new() - .post(&format!("{}/newsletters", &app.address)) - .basic_auth(&username, Some(&password)) - .json(&serde_json::json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } + let response = app + .post_newsletters(&serde_json::json!({ + "title": "Newsletter Title", + "text_content": "Newsletter body as plain text", + "html_content": "

Newsletter body as HTML

" })) - .send() - .await - .expect("Failed to execute request."); - - // Assert - assert_eq!(401, response.status().as_u16()); - assert_eq!( - r#"Basic realm="publish""#, - response.headers()["WWW-Authenticate"] - ); -} - -#[tokio::test] -async fn invalid_password_is_rejected() { - // Arrange - let app = spawn_app().await; - let username = &app.test_user.username; - let password = Uuid::new_v4().to_string(); - - assert_ne!(app.test_user.password, password); - - // Act - let response = reqwest::Client::new() - .post(&format!("{}/newsletters", &app.address)) - .basic_auth(username, Some(&password)) - .json(&serde_json::json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text", - "html": "

Newsletter body as HTML

" - } - })) - .send() - .await - .expect("Failed to execute request."); + .await; // Assert - assert_eq!(401, response.status().as_u16()); - assert_eq!( - r#"Basic realm="publish""#, - response.headers()["WWW-Authenticate"] - ); + assert_is_redirect_to(&response, "/login"); }