diff --git a/.gitignore b/.gitignore index a16143f..cc54acb 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,11 @@ /.direnv /.devenv /book + + +/static/light.css +/static/dark.css + # Devenv .devenv* devenv.local.nix diff --git a/Cargo.lock b/Cargo.lock index b159d38..c256cb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -1198,6 +1198,15 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "formbeam_derive" +version = "0.0.4" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.101", +] + [[package]] name = "forwarded-header-value" version = "0.1.1" @@ -1563,6 +1572,7 @@ dependencies = [ "async-trait", "bevy_reflect", "fluent-templates", + "formbeam", "hornbeam_grammar", "hornbeam_ir", "html-escape", @@ -1650,6 +1660,12 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "http-range-header" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9171a2ea8a68358193d15dd5d70c1c10a2afc3e7e4c5bc92bc9f025cebd7359c" + [[package]] name = "httparse" version = "1.10.1" @@ -1847,6 +1863,7 @@ dependencies = [ "axum-extra", "axum-test", "base64 0.21.7", + "bevy_reflect", "blake2", "chrono", "clap", @@ -1854,6 +1871,7 @@ dependencies = [ "confique", "eyre", "formbeam", + "formbeam_derive", "futures", "governor", "hornbeam", @@ -1866,6 +1884,7 @@ dependencies = [ "pgtemp", "rand 0.8.5", "rand_xoshiro", + "regex", "rstest", "serde", "serde_json", @@ -2319,6 +2338,16 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "mime_guess" +version = "2.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7c44f8e672c00fe5308fa235f821cb4198414e1c77935c1ab6948d3fd78550e" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -3887,6 +3916,19 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-util" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66a539a9ad6d5d281510d5bd368c973d636c02dbf8a67300bfb6b950696ad7df" +dependencies = [ + "bytes", + "futures-core", + "futures-sink", + "pin-project-lite", + "tokio", +] + [[package]] name = "toml" version = "0.8.22" @@ -3968,9 +4010,18 @@ checksum = "0fdb0c213ca27a9f57ab69ddb290fd80d970922355b83ae380b395d3986b8a2e" dependencies = [ "bitflags 2.9.0", "bytes", + "futures-util", "http 1.3.1", "http-body 1.0.1", + "http-body-util", + "http-range-header", + "httpdate", + "mime", + "mime_guess", + "percent-encoding", "pin-project-lite", + "tokio", + "tokio-util", "tower-layer", "tower-service", "tracing", @@ -4150,6 +4201,12 @@ dependencies = [ "unic-langid-impl", ] +[[package]] +name = "unicase" +version = "2.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75b844d17643ee918803943289730bec8aac480150456169e647ed0b576ba539" + [[package]] name = "unicode-bidi" version = "0.3.18" diff --git a/Cargo.toml b/Cargo.toml index 295b004..b827fa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,8 +24,10 @@ confique = { version = "0.2.4", features = ["toml"], default-features = false } eyre = "0.6.8" futures = "0.3.29" governor = "0.6.0" -hornbeam = { version = "0.0.4", path = "../hornbeam/hornbeam" } +hornbeam = { version = "0.0.4", path = "../hornbeam/hornbeam", features = ["formbeam"] } formbeam = { version = "0.0.4", path = "../hornbeam/formbeam" } +formbeam_derive = { version = "0.0.4", path = "../hornbeam/formbeam_derive" } +bevy_reflect = { version = "0.14.0" } josekit = "0.8.4" metrics = "0.21.1" metrics-exporter-prometheus = "0.12.1" @@ -41,7 +43,7 @@ subtle = "2.5.0" time = "0.3.30" tokio = { version = "1.33.0", features = ["rt", "macros"] } tower-cookies = "0.11.0" -tower-http = { version = "0.6.4", features = ["trace", "cors", "set-header"] } +tower-http = { version = "0.6.4", features = ["trace", "cors", "set-header", "fs"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } uuid = { version = "1.16.0", features = ["serde"] } @@ -54,3 +56,4 @@ maplit = "1.0.2" pgtemp = "0.3.0" rand_xoshiro = "0.6.0" rstest = "0.21.0" +regex = "1.11.1" diff --git a/README.md b/README.md index 80649b9..733c68a 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,9 @@ However, if desired, please contact me via the e-mail address found in the git c We have a Nix flake available containing all the required tools; either use direnv and `direnv allow` this repository or use `devenv shell`. +### Building the CSS + +Use `scripts-dev/watch_css.sh` to build the CSS whenever the source change. ### Database diff --git a/scripts-dev/watch_css.sh b/scripts-dev/watch_css.sh new file mode 100755 index 0000000..4a829d3 --- /dev/null +++ b/scripts-dev/watch_css.sh @@ -0,0 +1,8 @@ +#!/bin/sh +set -eu + +cd "$(dirname "$0")/.." + +while true; do + find ./src_scss | entr -d bash -c 'for theme in "dark" "light"; do grass --style compressed src_scss/$theme.scss static/$theme.css; done' +done diff --git a/src/store.rs b/src/store.rs index 43b86b6..9f5a5ae 100644 --- a/src/store.rs +++ b/src/store.rs @@ -70,7 +70,7 @@ impl IdCoopStore { } /// Representation of a user -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct User { /// The unique ID for the user. /// Should never change. diff --git a/src/tests/snapshots/idcoop__tests__test_oidc_auth_flow__2__login.snap b/src/tests/snapshots/idcoop__tests__test_oidc_auth_flow__2__login.snap index a4f9e71..9e39521 100644 --- a/src/tests/snapshots/idcoop__tests__test_oidc_auth_flow__2__login.snap +++ b/src/tests/snapshots/idcoop__tests__test_oidc_auth_flow__2__login.snap @@ -1,9 +1,9 @@ --- source: src/tests/test_oidc_auth_flow.rs -expression: "(headers, text)" +expression: "(headers, xsrf_box)" --- -- content-length: "238" +- content-length: "864" content-type: text/html; charset=utf-8 set-cookie: __Host-SessionlessXsrf=HL4qRFKUlBqkrPTvAQ6z-w; HttpOnly; SameSite=Strict; Secure; Path=/; Max-Age=43200000 x-frame-options: DENY -- "
UN PW (temporary form)
" +- "" diff --git a/src/tests/test_oidc_auth_flow.rs b/src/tests/test_oidc_auth_flow.rs index 802fd5c..3420014 100644 --- a/src/tests/test_oidc_auth_flow.rs +++ b/src/tests/test_oidc_auth_flow.rs @@ -7,6 +7,7 @@ use axum_test::{TestResponse, TestServer}; use insta::assert_yaml_snapshot; use maplit::btreemap; +use regex::Regex; use sqlx::types::Uuid; use crate::{passwords::create_password_hash, tests::basic_system}; @@ -83,9 +84,15 @@ async fn test_full_flow() { // 2. /login request let resp = client.get(&login_url).await; - let (status, headers, text) = dump_resp_text("2. /login request", resp); + let (status, mut headers, text) = dump_resp_text("2. /login request", resp); assert_eq!(status, 200); - assert_yaml_snapshot!("2/login", (headers, text)); + headers.remove("Content-Length"); // too variable, unimportant + let xsrf_box = Regex::new("<[^<>]+xsrf[^<>]+>") + .unwrap() + .find(&text) + .unwrap() + .as_str(); + assert_yaml_snapshot!("2/login", (headers, xsrf_box)); // 3. /login request with credentials let resp = client diff --git a/src/web.rs b/src/web.rs index 2aab700..ad41f45 100644 --- a/src/web.rs +++ b/src/web.rs @@ -12,6 +12,7 @@ use std::{ convert::Infallible, future::{self, Future}, net::{IpAddr, SocketAddr}, + path::PathBuf, sync::{ atomic::{AtomicU32, Ordering}, Arc, @@ -21,15 +22,18 @@ use std::{ use axum::{ extract::{ConnectInfo, FromRequestParts}, + handler::HandlerWithoutStateExt, http::{request::Parts, HeaderName, HeaderValue, StatusCode, Uri}, response::{Html, IntoResponse, Response}, - routing::{get, post}, + routing::{get, get_service, post}, Extension, Router, }; use governor::{clock::QuantaClock, state::keyed::DashMapStateStore, RateLimiter}; use hornbeam::{initialise_template_manager, make_template_manager}; use tower_cookies::CookieManagerLayer; -use tower_http::{cors::CorsLayer, set_header::SetResponseHeaderLayer, trace::TraceLayer}; +use tower_http::{ + cors::CorsLayer, services::ServeDir, set_header::SetResponseHeaderLayer, trace::TraceLayer, +}; use tracing::error; use crate::{ @@ -47,6 +51,7 @@ use crate::{ }, }; +pub mod errors; pub mod login; pub mod oauth_openid; pub mod sessionless_xsrf; @@ -57,6 +62,65 @@ make_template_manager! { }; } +struct Rendered(Result); + +impl IntoResponse for Rendered { + fn into_response(self) -> Response { + match self.0 { + Ok(html) => Html(html).into_response(), + Err(err) => { + error!("failed to render template: {err:?}"); + + ( + StatusCode::INTERNAL_SERVER_ERROR, + if cfg!(debug_assertions) { + format!("Failed to render template:\n{err}.") + } else { + // TODO use a more user-friendly page + "Failed to render template.".to_owned() + }, + ) + .into_response() + } + } + } +} + +fn find_static_directory_to_serve() -> PathBuf { + fn is_valid_base_dir(path: &std::path::Path) -> bool { + path.is_dir() && path.join("static").is_dir() + } + + let base_dir = if let Ok(base_dir_env) = + std::env::var("IDCOOP_BASE").or(std::env::var("HORNBEAM_BASE")) + { + let path = PathBuf::from(&base_dir_env); + if is_valid_base_dir(&path) { + path + } else { + panic!("Could not find statics at position of IDCOOP_BASE or HORNBEAM_BASE environment variable ({base_dir_env:?})"); + } + } else { + let try_paths = ["."]; + let mut successful = None; + for path in try_paths { + let path = PathBuf::from(path); + if is_valid_base_dir(&path) { + successful = Some(path); + break; + } + } + + successful + .unwrap_or_else(|| panic!("Could not find statics: tried looking in {try_paths:?} and no IDCOOP_BASE or HORNBEAM_BASE set!")) + }; + + base_dir + .canonicalize() + .expect("can't canonicalise statics dir") + .join("static") +} + /// Make an axum `Router` but do not bind it to a port. /// This exposition allows us to perform integration testing easily. pub(crate) async fn make_router( @@ -118,6 +182,12 @@ pub(crate) async fn make_router( get(oidc_discovery_configuration) .layer(CorsLayer::permissive().max_age(Duration::from_secs(600))), ) + .nest_service( + "/static", + get_service(ServeDir::new(find_static_directory_to_serve()).fallback( + (|| async { (StatusCode::NOT_FOUND, "404 No such resource found") }).into_service(), + )), + ) .layer(TraceLayer::new_for_http()) .layer(CookieManagerLayer::new()) .layer(Extension(config)) diff --git a/src/web/errors.rs b/src/web/errors.rs new file mode 100644 index 0000000..7c0444e --- /dev/null +++ b/src/web/errors.rs @@ -0,0 +1,33 @@ +//! Errors for web forms. + +use std::collections::BTreeMap; + +use formbeam::FieldError; + +/// Error code for error produced when an XSRF token is invalid. +pub const ERRCODE_XSRF_INVALID: &str = "xsrf_invalid"; + +/// Shorthand for producing an error when an XSRF token is invalid. +/// +/// For all legitimate users, in practice, this means the form has expired. +/// It could also mean foul play is afoot, but this is less likely and in that +/// case we don't need to display an error message. +pub fn xsrf_invalid() -> FieldError { + FieldError::Custom { + code: ERRCODE_XSRF_INVALID.to_owned(), + description: "Form expired; please try again.".to_owned(), + values: BTreeMap::new(), + } +} + +/// Error code for error produced when wrong login credentials are provided. +pub const ERRCODE_INVALID_CREDENTIALS: &str = "invalid_credentials"; + +/// Shorthand for producing an error for invalid credentials. +pub fn invalid_credentials() -> FieldError { + FieldError::Custom { + code: ERRCODE_INVALID_CREDENTIALS.to_owned(), + description: "Invalid username or password; please try again.".to_owned(), + values: BTreeMap::new(), + } +} diff --git a/src/web/login.rs b/src/web/login.rs index 5a3807d..9f44cd6 100644 --- a/src/web/login.rs +++ b/src/web/login.rs @@ -23,6 +23,7 @@ use eyre::{bail, Context, ContextCompat}; use formbeam::{traits::FormValidation, FormPartial}; use formbeam_derive::Form; use governor::Jitter; +use hornbeam::{render_template_string, ReflectedForm}; use rand::Rng; use serde::Deserialize; use sqlx::types::Uuid; @@ -265,6 +266,22 @@ where } } +/// Body parameter form for `POST /login` +#[derive(Debug, Form)] +pub struct LoginForm { + /// User-supplied username + #[form(max_chars(32), regex("^[a-z0-9]+$"))] + username: String, + + /// User-supplied password + #[form(max_chars(256))] + password: String, + + /// Hidden XSRF token, used to check this request isn't being made from another site + /// (i.e. protects against cross-site request forgery) + xsrf: String, +} + /// Query string parameters for `GET /login` #[derive(Deserialize)] pub struct LoginQuery { @@ -286,44 +303,28 @@ pub async fn get_login( Query(query): Query, cookies: Cookies, Extension(mut randgen): Extension, + DesiredLocale(locale): DesiredLocale, ) -> Response { match current_session { Ok(_session) => make_post_login_redirect(query.then), Err(_) => { let xsrf_token = sessionless_xsrf::get_token(&cookies, &mut randgen); - Html(format!("
UN PW (temporary form)
", xsrf_token)).into_response() + let form = ReflectedForm::::default(); + Rendered(render_template_string!(TEMPLATING, login, locale, { + xsrf_token, + form + })) + .into_response() } } } -/// Body parameters for `POST /login` -#[derive(Deserialize)] -pub struct PostLoginForm { - /// User-supplied username - username: String, - /// User-supplied password - password: String, - /// Hidden XSRF token, used to check this request isn't being made from another site - /// (i.e. protects against cross-site request forgery) - xsrf: String, -} - /// Perform a password hash operation, even though it's not needed. /// /// This aims to prevent side-channel attacks such as timing attacks, to discern the existence of a user. -fn dummy_password_hash( - password: String, - password_hash_config: &PasswordHashingConfig, -) -> WebResult { +fn dummy_password_hash(password: String, password_hash_config: &PasswordHashingConfig) { let _hash = create_password_hash(&password, password_hash_config) - .context("unable to hash password!")?; - - Ok(render_login_retry_form()) -} - -/// Render the form to retry a login. Currently not implemented. -fn render_login_retry_form() -> Response { - (StatusCode::UNAUTHORIZED, "Wrong username or password!").into_response() // TODO(ui): this should re-render the login form for another go + .expect("BUG: unable to hash dummy password!"); } /// `POST /login?then=...` @@ -354,18 +355,58 @@ pub async fn post_login( SecureClientIp(src_ip): SecureClientIp, Extension(ratelimiters): Extension>, Extension(mut randgen): Extension, - Form(form): Form, + DesiredLocale(locale): DesiredLocale, + Form(form_raw): Form, ) -> WebResult { ratelimiters.housekeeping(); ratelimiters .login .until_key_ready_with_jitter(&src_ip, Jitter::up_to(std::time::Duration::from_secs(1))) .await; + + let mut validation = form_raw + .validate() + .await + .context("failed to run form validator")?; + + // New XSRF token for if we need to re-render the form + let xsrf_token = sessionless_xsrf::get_token(&cookies, &mut randgen); + + if !validation.is_valid() { + let form = ReflectedForm::new(form_raw, validation); + return Ok(( + StatusCode::BAD_REQUEST, + Rendered(render_template_string!(TEMPLATING, login, locale, { + xsrf_token, + form + })), + ) + .into_response()); + } + + // INVARIANT: Form fields are syntactically valid at this point (but note XSRF not checked) + + let form = form_raw + .form() + .map_err(|e| eyre!("failed to extract a so-called valid form: {e}"))?; + if !sessionless_xsrf::check_token(&cookies, &form.xsrf) { // Invalid XSRF token: try again - return Ok(get_login(None, Query(query), cookies, Extension(randgen)).await); + validation.xsrf.push(errors::xsrf_invalid()); + + let form = ReflectedForm::new(form_raw, validation); + return Ok(( + StatusCode::BAD_REQUEST, + Rendered(render_template_string!(TEMPLATING, login, locale, { + xsrf_token, + form + })), + ) + .into_response()); } + // INVARIANT: Form validated at this point, other than user credentials + // retrieve user details // N.B. it may be that there is no user returned in this step, but we need to // do a password hash check anyway to avoid user enumeration problems. @@ -375,49 +416,59 @@ pub async fn post_login( .context("failed to look up user for login")?; // verify credentials - let user = match user_opt { - Some(user) => match &user.password_hash { - Some(password_hash) => match phil - .do_limited(src_ip, || check_hash(&form.password, password_hash)) - .await - .map_err(|_| eyre!("can't check password"))? - { - Ok(is_correct) => { - if is_correct { - user - } else { - return Ok(render_login_retry_form()); - } - } - Err(err) => { - error!("failed to check password hash: {err:?}"); - return phil - .do_limited(src_ip, || { - dummy_password_hash(form.password, &config.password_hashing) - }) - .await - .map_err(|_| eyre!("can't check password"))?; - } - }, - None => { - return phil - .do_limited(src_ip, || { - dummy_password_hash(form.password, &config.password_hashing) - }) - .await - .map_err(|_| eyre!("can't check password"))?; - } - }, - None => { - return phil - .do_limited(src_ip, || { - dummy_password_hash(form.password, &config.password_hashing) - }) - .await - .map_err(|_| eyre!("can't check password"))?; - } + let (Some(user), Some(password_hash)) = + (user_opt.clone(), user_opt.and_then(|u| u.password_hash)) + else { + // Either no user, or no password hash stored for that user + // Avoid a user enumeration attack: do a dummy password hash + // TODO(errors): convert do_limited 'too many waiters' to 429s rather than 500s? + phil.do_limited(src_ip, || { + dummy_password_hash(form.password, &config.password_hashing) + }) + .await + .map_err(|_| eyre!("can't check password"))?; + + validation.form_wide.push(errors::invalid_credentials()); + + let form = ReflectedForm::new(form_raw, validation); + return Ok(( + StatusCode::UNAUTHORIZED, + Rendered(render_template_string!(TEMPLATING, login, locale, { + xsrf_token, + form + })), + ) + .into_response()); }; + // TODO(errors): convert do_limited 'too many waiters' to 429s rather than 500s? + let password_is_correct = phil + .do_limited(src_ip, || check_hash(&form.password, &password_hash)) + .await + .map_err(|_| eyre!("can't check password"))??; + + if !password_is_correct { + validation.form_wide.push(errors::invalid_credentials()); + + let form = ReflectedForm::new(form_raw, validation); + return Ok(( + StatusCode::UNAUTHORIZED, + Rendered(render_template_string!(TEMPLATING, login, locale, { + xsrf_token, + form + })), + ) + .into_response()); + } + + // + // + // + ///// INVARIANT: User authenticated beyond this point + // + // + // + // Generate a login session token and store the hash in our database let login_session_token = randgen.gen::<[u8; LOGIN_SESSION_TOKEN_BYTES]>(); let login_session_token_b64 = BASE64_URL_SAFE_NO_PAD.encode(login_session_token); diff --git a/src/web/oauth_openid/application_session_access_token.rs b/src/web/oauth_openid/application_session_access_token.rs index 9893f37..88cdec1 100644 --- a/src/web/oauth_openid/application_session_access_token.rs +++ b/src/web/oauth_openid/application_session_access_token.rs @@ -56,15 +56,13 @@ where .await { Ok(Some(session)) => Ok(session), - Ok(None) => { - return Err((StatusCode::UNAUTHORIZED, "Invalid application session.")); - } + Ok(None) => Err((StatusCode::UNAUTHORIZED, "Invalid application session.")), Err(err) => { error!("failed to check application session: {err:?}"); - return Err(( + Err(( StatusCode::INTERNAL_SERVER_ERROR, "A fault occurred when checking your application session. If the issue persists, please contact an administrator.", - )); + )) } } } diff --git a/src_scss/dark.scss b/src_scss/dark.scss new file mode 100644 index 0000000..e69de29 diff --git a/src_scss/light.scss b/src_scss/light.scss new file mode 100644 index 0000000..e69de29 diff --git a/templates/components/BarePage.hnb b/templates/components/BarePage.hnb new file mode 100644 index 0000000..f8c2a3a --- /dev/null +++ b/templates/components/BarePage.hnb @@ -0,0 +1,11 @@ +html + head + meta {charset="UTF-8"} + + title + optional slot :title + + link {rel="stylesheet", href="/statics/light.css"} // TODO support theming + + body + slot :main diff --git a/templates/components/Text.hnb b/templates/components/Text.hnb new file mode 100644 index 0000000..fe64cdf --- /dev/null +++ b/templates/components/Text.hnb @@ -0,0 +1,37 @@ +// ReflectedForm instance +//param $form + +// The name of the form field +//param $name + +// Optional. Override the type of the textbox. +// Example values: 'password' +// For email, prefer instead to have an Email validator on the field +// and this will be set automatically. +//TODO param $type = "text" + + +set $minlength = None +set $maxlength = None +set $required = None +set $email = None +set $pattern = None +set $type = "text" + +for $validator in $form.info.field_validators($name) + match $validator + MinLength($l) => + set $minlength = $l + MaxLength($l) => + set $maxlength = $l + Required => + set $required = "" + Email => + set $type = "email" + Regex($r) => + set $pattern = $r + _ => + "$validator" + + +input {$type, $name, $minlength?, $maxlength?, $required?, $pattern?} diff --git a/templates/pages/login.hnb b/templates/pages/login.hnb new file mode 100644 index 0000000..d3d9daa --- /dev/null +++ b/templates/pages/login.hnb @@ -0,0 +1,12 @@ +BarePage + form {method = "POST"} + Text {$form, name = "username"} + "UN" + input {type = "text", name = "username"} + " PW" + input {type = "password", name = "password"} + " " + input {type = "hidden", name = "xsrf", value = $xsrf_token} + button {type = "submit"} + "click here to login" + " (temporary form)"