From 9f51d5112d21cbfb555143d1074b504648bf67f0 Mon Sep 17 00:00:00 2001 From: Piotr Orzechowski Date: Fri, 12 Apr 2024 11:33:48 +0200 Subject: [PATCH] Switch to session-based auth for newsletters endpoint --- Cargo.lock | 1 - Cargo.toml | 1 - src/routes/newsletters.rs | 99 +++++---------------------------------- tests/api/helpers.rs | 24 ---------- tests/api/newsletters.rs | 97 +++++++++++--------------------------- 5 files changed, 39 insertions(+), 183 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 01d272f..c2ed69b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3491,7 +3491,6 @@ dependencies = [ "askama_axum", "axum", "axum-messages", - "base64 0.22.0", "claims", "config", "fake", diff --git a/Cargo.toml b/Cargo.toml index 4f4828d..f09de13 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,6 @@ askama = { version = "0.12.1", features = ["with-axum"], default-features = fals askama_axum = { version = "0.4.0", default-features = false } axum = "0.7.4" axum-messages = "0.6.0" -base64 = "0.22.0" config = "0.14.0" once_cell = "1.19.0" rand = "0.8.5" diff --git a/src/routes/newsletters.rs b/src/routes/newsletters.rs index d016eb5..94e5b2d 100644 --- a/src/routes/newsletters.rs +++ b/src/routes/newsletters.rs @@ -1,47 +1,29 @@ use crate::{ app_state::AppState, - authentication::password::{validate_credentials, AuthError, Credentials}, + authentication::middleware::AuthorizedSessionLayer, domain::{SubscriberEmail, SubscriptionStatus}, + utils::{e500, HttpError}, }; use anyhow::Context; -use axum::{ - extract::State, - http::{header::WWW_AUTHENTICATE, HeaderMap, HeaderValue, StatusCode}, - response::{IntoResponse, Response}, - routing::post, - Json, Router, -}; -use base64::Engine; -use secrecy::Secret; +use axum::{extract::State, routing::post, Json, Router}; use serde::Deserialize; use sqlx::PgPool; pub fn router() -> Router { - Router::new().route("/newsletters", post(publish_newsletter)) + Router::new() + .route("/newsletters", post(publish_newsletter)) + .layer(AuthorizedSessionLayer) } -#[tracing::instrument( - name = "Publish newsletter", - skip(headers, app_state, body), - fields(username=tracing::field::Empty, user_id=tracing::field::Empty) -)] +#[tracing::instrument(name = "Publish newsletter", skip(app_state, body))] async fn publish_newsletter( - headers: HeaderMap, State(app_state): State, Json(body): Json, -) -> Result<(), PublishError> { - let credentials = basic_authentication(&headers).map_err(PublishError::AuthError)?; - tracing::Span::current().record("username", &tracing::field::display(&credentials.username)); - - let user_id = validate_credentials(&app_state.db_pool, credentials) +) -> Result<(), HttpError> { + for subscriber in get_confirmed_subscribers(&app_state.db_pool) .await - .map_err(|e| match e { - AuthError::InvalidCredentials(_) => PublishError::AuthError(e.into()), - AuthError::UnexpectedError(_) => PublishError::UnexpectedError(e.into()), - })?; - tracing::Span::current().record("user_id", &tracing::field::display(&user_id)); - - for subscriber in get_confirmed_subscribers(&app_state.db_pool).await? { + .map_err(e500)? + { match subscriber { Ok(subscriber) => app_state .email_client @@ -66,41 +48,6 @@ async fn publish_newsletter( Ok(()) } -#[tracing::instrument(name = "Extract basic auth credentials", skip(headers))] -fn basic_authentication(headers: &HeaderMap) -> Result { - let base64encoded_segment = headers - .get("Authorization") - .context("The 'Authorization' header was missing")? - .to_str() - .context("The 'Authorization' header was not a valid UTF-8 string")? - .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 a valid UTF-8")?; - - 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 username must be provided in 'Basic' auth"))? - .to_string(); - - Ok(Credentials { - username, - password: Secret::new(password), - }) -} - #[tracing::instrument(name = "Get confirmed subscribers", skip(db_pool))] async fn get_confirmed_subscribers( db_pool: &PgPool, @@ -142,27 +89,3 @@ struct Content { struct ConfirmedSubscriber { email: SubscriberEmail, } - -#[derive(Debug, thiserror::Error)] -enum PublishError { - #[error("Authentication failed")] - AuthError(#[source] anyhow::Error), - #[error(transparent)] - UnexpectedError(#[from] anyhow::Error), -} - -impl IntoResponse for PublishError { - fn into_response(self) -> Response { - tracing::error!("{:#?}", self); - - match self { - Self::AuthError(_) => { - let mut headers = HeaderMap::new(); - let header_value = HeaderValue::from_static(r#"Basic realm="publish""#); - headers.insert(WWW_AUTHENTICATE, header_value); - (StatusCode::UNAUTHORIZED, headers).into_response() - } - Self::UnexpectedError(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), - } - } -} diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index aa5c129..2cca5e8 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -109,31 +109,7 @@ impl TestApp { .expect(Self::FAILED_TO_EXECUTE_REQUEST) } - pub async fn post_newsletters_with_credentials( - &self, - body: &serde_json::Value, - username: &str, - password: &str, - ) -> reqwest::Response { - self.client - .post(self.url("/newsletters")) - .basic_auth(username, Some(password)) - .json(body) - .send() - .await - .expect(Self::FAILED_TO_EXECUTE_REQUEST) - } - pub async fn post_newsletters(&self, body: &serde_json::Value) -> reqwest::Response { - self.post_newsletters_with_credentials( - body, - &self.test_user.username, - &self.test_user.password, - ) - .await - } - - pub async fn post_newsletters_no_auth(&self, body: &serde_json::Value) -> reqwest::Response { self.client .post(self.url("/newsletters")) .json(body) diff --git a/tests/api/newsletters.rs b/tests/api/newsletters.rs index c1c32cb..93f61b1 100644 --- a/tests/api/newsletters.rs +++ b/tests/api/newsletters.rs @@ -1,6 +1,5 @@ -use crate::helpers::{ConfirmationLinks, TestApp}; +use crate::helpers::{assert_redirect_to, ConfirmationLinks, TestApp}; use serde_json::json; -use uuid::Uuid; use wiremock::{ matchers::{any, method, path}, Mock, ResponseTemplate, @@ -26,6 +25,14 @@ async fn newsletters_are_delivered_to_confirmed_subscribers() { .mount(&app.email_server) .await; + let response = app + .post_login(&json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + assert_redirect_to(&response, "/admin/dashboard"); + // when let response = app.post_newsletters(&newsletter_request_body).await; @@ -52,6 +59,14 @@ async fn newsletters_are_not_delivered_to_unconfirmed_subscribers() { .mount(&app.email_server) .await; + let response = app + .post_login(&json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + assert_redirect_to(&response, "/admin/dashboard"); + // when let response = app.post_newsletters(&newsletter_request_body).await; @@ -81,6 +96,14 @@ async fn newsletters_returns_400_for_invalid_data() { ), ]; + let response = app + .post_login(&json!({ + "username": &app.test_user.username, + "password": &app.test_user.password, + })) + .await; + assert_redirect_to(&response, "/admin/dashboard"); + for (body, error_message) in test_cases { // when let response = app.post_newsletters(&body).await; @@ -96,30 +119,7 @@ async fn newsletters_returns_400_for_invalid_data() { } #[tokio::test] -async fn requests_missing_authorization_are_rejected() { - // given - let app = TestApp::spawn().await; - let newsletter_request_body = json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text.", - "html": "

Newsletter body as html.

", - } - }); - - // when - let response = app.post_newsletters_no_auth(&newsletter_request_body).await; - - // then - assert_eq!(response.status(), 401); - assert_eq!( - response.headers()["WWW-Authenticate"], - r#"Basic realm="publish""# - ); -} - -#[tokio::test] -async fn non_existing_user_is_rejected() { +async fn requests_from_anonymous_users_are_redirected_to_login() { // given let app = TestApp::spawn().await; let newsletter_request_body = json!({ @@ -129,53 +129,12 @@ async fn non_existing_user_is_rejected() { "html": "

Newsletter body as html.

", } }); - let username = Uuid::new_v4().to_string(); // when - let response = app - .post_newsletters_with_credentials( - &newsletter_request_body, - &username, - &app.test_user.password, - ) - .await; - - // then - assert_eq!(response.status(), 401); - assert_eq!( - response.headers()["WWW-Authenticate"], - r#"Basic realm="publish""# - ); -} - -#[tokio::test] -async fn invalid_password_is_rejected() { - // given - let app = TestApp::spawn().await; - let newsletter_request_body = json!({ - "title": "Newsletter Title", - "content": { - "text": "Newsletter body as plain text.", - "html": "

Newsletter body as html.

", - } - }); - let password = Uuid::new_v4().to_string(); - - // when - let response = app - .post_newsletters_with_credentials( - &newsletter_request_body, - &app.test_user.username, - &password, - ) - .await; + let response = app.post_newsletters(&newsletter_request_body).await; // then - assert_eq!(response.status(), 401); - assert_eq!( - response.headers()["WWW-Authenticate"], - r#"Basic realm="publish""# - ); + assert_redirect_to(&response, "/login"); } async fn create_unfonfirmed_subscriber(app: &TestApp) -> ConfirmationLinks {