From cfc255d881e8b1bdc1ab5bb998779a0cf1ff82e0 Mon Sep 17 00:00:00 2001 From: Piotr Orzechowski Date: Tue, 26 Mar 2024 19:49:57 +0100 Subject: [PATCH] Add login form and verify credentials --- Cargo.lock | 11 +++++ Cargo.toml | 5 ++ configuration/base.yaml | 1 + src/app_state.rs | 2 + src/authentication.rs | 91 +++++++++++++++++++++++++++++++++++ src/configuration.rs | 1 + src/lib.rs | 1 + src/routes/login/action.rs | 77 +++++++++++++++++++++++++++++ src/routes/login/form.rs | 74 ++++++++++++++++++++++++++++ src/routes/login/mod.rs | 16 ++++++ src/routes/mod.rs | 1 + src/routes/newsletters.rs | 90 ++++------------------------------ src/startup.rs | 15 +++++- templates/web/base.html | 13 +++++ templates/web/home.html | 24 +++------ templates/web/login_form.html | 21 ++++++++ 16 files changed, 344 insertions(+), 99 deletions(-) create mode 100644 src/authentication.rs create mode 100644 src/routes/login/action.rs create mode 100644 src/routes/login/form.rs create mode 100644 src/routes/login/mod.rs create mode 100644 templates/web/base.html create mode 100644 templates/web/login_form.html diff --git a/Cargo.lock b/Cargo.lock index 19f2682..f760072 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -884,6 +884,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "htmlescape" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9025058dae765dee5070ec375f591e2ba14638c63feff74f13805a72e523163" + [[package]] name = "http" version = "0.2.12" @@ -3109,6 +3115,9 @@ dependencies = [ "claims", "config", "fake", + "hex", + "hmac", + "htmlescape", "linkify", "once_cell", "proptest", @@ -3119,6 +3128,7 @@ dependencies = [ "serde", "serde-aux", "serde_json", + "sha2", "sqlx", "thiserror", "time", @@ -3130,6 +3140,7 @@ dependencies = [ "tracing-log 0.2.0", "tracing-subscriber", "unicode-segmentation", + "urlencoding", "uuid", "validator", "wiremock", diff --git a/Cargo.toml b/Cargo.toml index 51811d1..e8bb16c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,9 @@ askama_axum = { version = "0.4.0", default-features = false } axum = "0.7.4" base64 = "0.22.0" config = "0.14.0" +hex = "0.4.3" +hmac = { version = "0.12.1", features = ["std"] } +htmlescape = "0.3.1" once_cell = "1.19.0" rand = "0.8.5" regex = "1.10.3" @@ -27,6 +30,7 @@ reqwest = { version = "0.11.24", features = ["json"], default-features = false } secrecy = { version = "0.8.0", features = ["serde"] } serde = { version = "1.0.196", features = ["derive"] } serde-aux = { version = "4.4.0", default-features = false } +sha2 = "0.10.8" sqlx = { version = "0.7.3", features = ["macros", "migrate", "postgres", "time", "runtime-tokio", "tls-native-tls", "uuid"], default-features = false } thiserror = "1.0.58" time = { version = "0.3.34", features = ["macros", "serde"] } @@ -38,6 +42,7 @@ tracing-bunyan-formatter = "0.3.9" tracing-log = "0.2.0" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } unicode-segmentation = "1.11.0" +urlencoding = "2.1.3" uuid = { version = "1.7.0", features = ["v4"] } validator = "0.16.1" diff --git a/configuration/base.yaml b/configuration/base.yaml index 7fbb048..c98d8b7 100644 --- a/configuration/base.yaml +++ b/configuration/base.yaml @@ -1,5 +1,6 @@ application: port: 8000 + hmac_secret: long-and-very-secret-random-key-needed-to-verify-message-integrity database: host: localhost port: 5432 diff --git a/src/app_state.rs b/src/app_state.rs index 2fa07b5..10a32b9 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -1,5 +1,6 @@ use crate::email_client::EmailClient; use axum::http::Uri; +use secrecy::Secret; use sqlx::PgPool; #[derive(Clone)] @@ -7,4 +8,5 @@ pub struct AppState { pub db_pool: PgPool, pub email_client: EmailClient, pub base_url: Uri, + pub hmac_secret: Secret, } diff --git a/src/authentication.rs b/src/authentication.rs new file mode 100644 index 0000000..821d0c7 --- /dev/null +++ b/src/authentication.rs @@ -0,0 +1,91 @@ +use crate::telemetry::spawn_blocking_with_tracing; +use anyhow::Context; +use argon2::{Argon2, PasswordHash, PasswordVerifier}; +use secrecy::{ExposeSecret, Secret}; +use sqlx::PgPool; +use uuid::Uuid; + +#[tracing::instrument(name = "Validate credentials", skip(db_pool, credentials))] +pub async fn validate_credentials( + db_pool: &PgPool, + credentials: Credentials, +) -> Result { + let mut user_id = None; + let mut expected_password_hash = Secret::new( + "$argon2id$v=19$m=15000,t=2,p=1$\ + 5sbY1nJpEqWJ9gQP0SvDbw$\ + ZgUSqWDG8XJozXYqOTrah9Ori8FmepJwhTHZMLradFU" + .to_string(), + ); + + if let Some((stored_user_id, stored_password_hash)) = + get_stored_credentials(db_pool, credentials.username).await? + { + user_id = Some(stored_user_id); + expected_password_hash = stored_password_hash; + } + + spawn_blocking_with_tracing(move || { + verify_password_hash(expected_password_hash, credentials.password) + }) + .await + .context("Failed to spawn blocking task")??; + + user_id + .ok_or_else(|| anyhow::anyhow!("Unknown username")) + .map_err(AuthError::InvalidCredentials) +} + +#[tracing::instrument(name = "Get stored credentials", skip(db_pool, username))] +async fn get_stored_credentials( + db_pool: &PgPool, + username: String, +) -> Result)>, anyhow::Error> { + let row = sqlx::query!( + r#" + SELECT user_id, password_hash + FROM users + WHERE username = $1 + "#, + username, + ) + .fetch_optional(db_pool) + .await + .context("Failed to perform a query to retrieve stored credentials")? + .map(|row| (row.user_id, Secret::new(row.password_hash))); + + Ok(row) +} + +#[tracing::instrument( + name = "Verify password hash", + skip(expected_password_hash, password_candidate) +)] +fn verify_password_hash( + expected_password_hash: Secret, + password_candidate: Secret, +) -> Result<(), AuthError> { + let expected_password_hash = PasswordHash::new(expected_password_hash.expose_secret()) + .context("Failed to parse hash in PHC string format")?; + + Argon2::default() + .verify_password( + password_candidate.expose_secret().as_bytes(), + &expected_password_hash, + ) + .context("Invalid password") + .map_err(AuthError::InvalidCredentials) +} + +pub struct Credentials { + pub username: String, + pub password: Secret, +} + +#[derive(Debug, thiserror::Error)] +pub enum AuthError { + #[error("Invalid credentials")] + InvalidCredentials(#[source] anyhow::Error), + #[error(transparent)] + UnexpectedError(#[from] anyhow::Error), +} diff --git a/src/configuration.rs b/src/configuration.rs index 84a9ce5..83638b3 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -22,6 +22,7 @@ pub struct ApplicationSettings { #[serde(deserialize_with = "deserialize_number_from_string")] pub port: u16, pub base_url: String, + pub hmac_secret: Secret, } #[derive(Deserialize)] diff --git a/src/lib.rs b/src/lib.rs index b06a726..40f47c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,5 @@ pub mod app_state; +pub mod authentication; pub mod configuration; pub mod domain; pub mod email_client; diff --git a/src/routes/login/action.rs b/src/routes/login/action.rs new file mode 100644 index 0000000..74aa7cc --- /dev/null +++ b/src/routes/login/action.rs @@ -0,0 +1,77 @@ +use crate::{ + app_state::AppState, + authentication::{validate_credentials, AuthError, Credentials}, +}; +use axum::{ + extract::State, + http::StatusCode, + response::{IntoResponse, Redirect, Response}, + Form, +}; +use hmac::{Hmac, Mac}; +use secrecy::{ExposeSecret, Secret}; +use serde::Deserialize; +use urlencoding::Encoded; + +#[tracing::instrument( + skip(app_state, form), + fields(username = tracing::field::Empty, user_id = tracing::field::Empty) +)] +pub(super) async fn login( + State(app_state): State, + Form(form): Form, +) -> Result { + tracing::Span::current().record("username", &tracing::field::display(&form.username)); + + let user_id = validate_credentials( + &app_state.db_pool, + Credentials { + username: form.username, + password: form.password, + }, + ) + .await + .map_err(|e| match e { + AuthError::InvalidCredentials(_) => { + let error = format!("error={}", Encoded::new(e.to_string())); + let tag = format!("tag={:x}", { + let secret: &[u8] = app_state.hmac_secret.expose_secret().as_bytes(); + let mut mac = Hmac::::new_from_slice(secret).unwrap(); + mac.update(error.as_bytes()); + mac.finalize().into_bytes() + }); + + LoginError::AuthError(e.into(), Redirect::to(&format!("/login?{error}&{tag}"))) + } + AuthError::UnexpectedError(_) => LoginError::UnexpectedError(e.into()), + })?; + + tracing::Span::current().record("user_id", &tracing::field::display(&user_id)); + + Ok(Redirect::to("/")) +} + +#[derive(Deserialize)] +pub(super) struct FormData { + username: String, + password: Secret, +} + +#[derive(Debug, thiserror::Error)] +pub(super) enum LoginError { + #[error("Authentication failed")] + AuthError(#[source] anyhow::Error, Redirect), + #[error("Something went wrong")] + UnexpectedError(#[from] anyhow::Error), +} + +impl IntoResponse for LoginError { + fn into_response(self) -> Response { + tracing::error!("{:#?}", self); + + match self { + Self::AuthError(_, redirect) => redirect.into_response(), + Self::UnexpectedError(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), + } + } +} diff --git a/src/routes/login/form.rs b/src/routes/login/form.rs new file mode 100644 index 0000000..784988f --- /dev/null +++ b/src/routes/login/form.rs @@ -0,0 +1,74 @@ +use crate::app_state::AppState; +use anyhow::Context; +use askama_axum::Template; +use axum::extract::{Query, State}; +use hmac::{Hmac, Mac}; +use secrecy::{ExposeSecret, Secret}; +use serde::Deserialize; +use urlencoding::Encoded; + +#[tracing::instrument(skip(app_state, parameters))] +pub(super) async fn login_form( + State(app_state): State, + Query(parameters): Query, +) -> LoginForm<'static> { + let error_message = match parameters.error_message(&app_state.hmac_secret) { + Ok(raw_html) => raw_html, + Err(e) => { + tracing::warn!("Failed to get error message from query parameters: {:?}", e); + None + } + } + .map(|raw_html| htmlescape::encode_minimal(&raw_html)); + + LoginForm { + title: "Login", + username_label: "Username", + username_placeholder: "Enter username", + password_label: "Password", + password_placeholder: "Enter password", + submit_label: "Login", + error_message, + action: "/login", + } +} + +#[derive(Deserialize)] +pub(super) struct Parameters { + error: Option, + tag: Option, +} + +impl Parameters { + fn error_message(self, hmac_secret: &Secret) -> Result, anyhow::Error> { + match (&self.error, self.tag) { + (Some(e), Some(t)) => { + let tag = hex::decode(t).context("Failed to decode hex hmac tag")?; + let error = format!("error={}", Encoded::new(e)); + + let mut mac = + Hmac::::new_from_slice(hmac_secret.expose_secret().as_bytes())?; + mac.update(error.as_bytes()); + mac.verify_slice(&tag)?; + + Ok(self.error) + } + (None, None) => Ok(None), + (Some(_), None) => Err(anyhow::anyhow!("Error message is missing hmac tag")), + (None, Some(_)) => Err(anyhow::anyhow!("Hmac tag is missing error message")), + } + } +} + +#[derive(Template)] +#[template(path = "web/login_form.html")] +pub(super) struct LoginForm<'a> { + title: &'a str, + username_label: &'a str, + username_placeholder: &'a str, + password_label: &'a str, + password_placeholder: &'a str, + submit_label: &'a str, + error_message: Option, + action: &'a str, +} diff --git a/src/routes/login/mod.rs b/src/routes/login/mod.rs new file mode 100644 index 0000000..6a2654a --- /dev/null +++ b/src/routes/login/mod.rs @@ -0,0 +1,16 @@ +use crate::app_state::AppState; +use action::login; +use axum::{ + routing::{get, post}, + Router, +}; +use form::login_form; + +mod action; +mod form; + +pub fn router() -> Router { + Router::new() + .route("/login", get(login_form)) + .route("/login", post(login)) +} diff --git a/src/routes/mod.rs b/src/routes/mod.rs index fa1aa9f..b37042d 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -1,5 +1,6 @@ pub mod health_check; pub mod home; +pub mod login; pub mod newsletters; pub mod subscriptions; pub mod subscriptions_confirm; diff --git a/src/routes/newsletters.rs b/src/routes/newsletters.rs index 2e2a022..804e38b 100644 --- a/src/routes/newsletters.rs +++ b/src/routes/newsletters.rs @@ -1,10 +1,9 @@ use crate::{ app_state::AppState, + authentication::{validate_credentials, AuthError, Credentials}, domain::{SubscriberEmail, SubscriptionStatus}, - telemetry::spawn_blocking_with_tracing, }; -use anyhow::{anyhow, Context}; -use argon2::{Argon2, PasswordHash, PasswordVerifier}; +use anyhow::Context; use axum::{ extract::State, http::{header::WWW_AUTHENTICATE, HeaderMap, HeaderValue, StatusCode}, @@ -13,10 +12,9 @@ use axum::{ Json, Router, }; use base64::Engine; -use secrecy::{ExposeSecret, Secret}; +use secrecy::Secret; use serde::Deserialize; use sqlx::PgPool; -use uuid::Uuid; pub fn router() -> Router { Router::new().route("/newsletters", post(publish_newsletter)) @@ -35,7 +33,12 @@ async fn publish_newsletter( 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).await?; + let user_id = validate_credentials(&app_state.db_pool, credentials) + .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? { @@ -98,76 +101,6 @@ fn basic_authentication(headers: &HeaderMap) -> Result Result { - let mut user_id = None; - let mut expected_password_hash = Secret::new( - "$argon2id$v=19$m=15000,t=2,p=1$\ - 5sbY1nJpEqWJ9gQP0SvDbw$\ - ZgUSqWDG8XJozXYqOTrah9Ori8FmepJwhTHZMLradFU" - .to_string(), - ); - - if let Some((stored_user_id, stored_password_hash)) = - get_stored_credentials(db_pool, credentials.username).await? - { - user_id = Some(stored_user_id); - expected_password_hash = stored_password_hash; - } - - spawn_blocking_with_tracing(move || { - verify_password_hash(expected_password_hash, credentials.password) - }) - .await - .context("Failed to spawn blocking task")??; - - user_id.ok_or_else(|| PublishError::AuthError(anyhow!("Unknown username"))) -} - -#[tracing::instrument(name = "Get stored credentials", skip(db_pool, username))] -async fn get_stored_credentials( - db_pool: &PgPool, - username: String, -) -> Result)>, anyhow::Error> { - let row = sqlx::query!( - r#" - SELECT user_id, password_hash - FROM users - WHERE username = $1 - "#, - username, - ) - .fetch_optional(db_pool) - .await - .context("Failed to perform a query to retrieve stored credentials")? - .map(|row| (row.user_id, Secret::new(row.password_hash))); - - Ok(row) -} - -#[tracing::instrument( - name = "Verify password hash", - skip(expected_password_hash, password_candidate) -)] -fn verify_password_hash( - expected_password_hash: Secret, - password_candidate: Secret, -) -> Result<(), PublishError> { - let expected_password_hash = PasswordHash::new(expected_password_hash.expose_secret()) - .context("Failed to parse hash in PHC string format")?; - - Argon2::default() - .verify_password( - password_candidate.expose_secret().as_bytes(), - &expected_password_hash, - ) - .context("Invalid password") - .map_err(PublishError::AuthError) -} - #[tracing::instrument(name = "Get confirmed subscribers", skip(db_pool))] async fn get_confirmed_subscribers( db_pool: &PgPool, @@ -206,11 +139,6 @@ struct Content { text: String, } -struct Credentials { - username: String, - password: Secret, -} - struct ConfirmedSubscriber { email: SubscriberEmail, } diff --git a/src/startup.rs b/src/startup.rs index caa6de5..c587cfb 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -3,10 +3,11 @@ use crate::{ configuration::{DatabaseSettings, Settings}, email_client::EmailClient, request_id::RequestUuid, - routes::{health_check, home, newsletters, subscriptions, subscriptions_confirm}, + routes::{health_check, home, login, newsletters, subscriptions, subscriptions_confirm}, telemetry::request_span, }; use axum::{http::Uri, serve::Serve, Router}; +use secrecy::Secret; use sqlx::{postgres::PgPoolOptions, PgPool}; use std::{net::SocketAddr, str::FromStr}; use tokio::net::TcpListener; @@ -49,7 +50,14 @@ impl Application { .local_addr() .expect("Failed to get local address from the listener"); - let server = run(listener, db_pool, email_client, config.application.base_url).await; + let server = run( + listener, + db_pool, + email_client, + config.application.base_url, + config.application.hmac_secret, + ) + .await; Self { local_addr, server } } @@ -73,11 +81,13 @@ async fn run( db_pool: PgPool, email_client: EmailClient, base_url: String, + hmac_secret: Secret, ) -> Serve { let app_state = AppState { db_pool, email_client, base_url: Uri::from_str(&base_url).expect("Failed to parse base url"), + hmac_secret, }; let app = Router::new() @@ -86,6 +96,7 @@ async fn run( .merge(subscriptions_confirm::router()) .merge(newsletters::router()) .merge(home::router()) + .merge(login::router()) .with_state(app_state) .layer( ServiceBuilder::new() diff --git a/templates/web/base.html b/templates/web/base.html new file mode 100644 index 0000000..bb25518 --- /dev/null +++ b/templates/web/base.html @@ -0,0 +1,13 @@ + + + + + + {% block title %}{{ title }}{% endblock %} + + + + {% block content %}Something went wrong with rendering{% endblock %} + + + diff --git a/templates/web/home.html b/templates/web/home.html index 5141282..de0b0b2 100644 --- a/templates/web/home.html +++ b/templates/web/home.html @@ -1,17 +1,9 @@ - - +{% extends "base.html" %} - - - {{ title }} - - - - {%- if let Some(username) = username %} -

Welcome to our newsletter, {{ username }}!

- {%- else %} -

Welcome to our newsletter!

- {%- endif %} - - - +{% block content %} +{%- if let Some(username) = username %} +

Welcome to our newsletter, {{ username }}!

+{% else %} +

Welcome to our newsletter!

+{% endif -%} +{% endblock %} diff --git a/templates/web/login_form.html b/templates/web/login_form.html new file mode 100644 index 0000000..bba1153 --- /dev/null +++ b/templates/web/login_form.html @@ -0,0 +1,21 @@ +{% extends "base.html" %} + +{% block content %} +{%- if let Some(message) = error_message %} +

{{ message }}

+{%- endif %} + +
+ + + + + +
+{% endblock %}