Set SameSite for cookies to lax; log errors instead of panic.

Also use a password strength crate since Forgejo itself insists on some minimum complexity for it.
This commit is contained in:
Joe Ardent 2024-03-04 17:13:15 -08:00
parent bafc93f6af
commit 8e25651cfa
6 changed files with 117 additions and 35 deletions

47
Cargo.lock generated
View file

@ -673,6 +673,15 @@ dependencies = [
"windows-targets 0.48.5", "windows-targets 0.48.5",
] ]
[[package]]
name = "passwords"
version = "3.1.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11407193a7c2bd14ec6b0ec3394da6fdcf7a4d5dcbc8c3cc38dfb17802c8d59c"
dependencies = [
"random-pick",
]
[[package]] [[package]]
name = "percent-encoding" name = "percent-encoding"
version = "2.3.1" version = "2.3.1"
@ -723,6 +732,12 @@ version = "0.2.17"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de"
[[package]]
name = "proc-macro-hack"
version = "0.5.20+deprecated"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068"
[[package]] [[package]]
name = "proc-macro2" name = "proc-macro2"
version = "1.0.78" version = "1.0.78"
@ -745,6 +760,7 @@ dependencies = [
"justerror", "justerror",
"lazy_static", "lazy_static",
"log", "log",
"passwords",
"rand", "rand",
"serde", "serde",
"serde_json", "serde_json",
@ -796,6 +812,37 @@ dependencies = [
"getrandom", "getrandom",
] ]
[[package]]
name = "random-number"
version = "0.1.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3a3da5cbb4c27c5150c03a54a7e4745437cd90f9e329ae657c0b889a144bb7be"
dependencies = [
"proc-macro-hack",
"rand",
"random-number-macro-impl",
]
[[package]]
name = "random-number-macro-impl"
version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8b86292cf41ccfc96c5de7165c1c53d5b4ac540c5bab9d1857acbe9eba5f1a0b"
dependencies = [
"proc-macro-hack",
"quote",
"syn 2.0.52",
]
[[package]]
name = "random-pick"
version = "1.2.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c179499072da789afe44127d5f4aa6012de2c2f96ef759990196b37387a2a0f8"
dependencies = [
"random-number",
]
[[package]] [[package]]
name = "redox_syscall" name = "redox_syscall"
version = "0.4.1" version = "0.4.1"

View file

@ -13,6 +13,7 @@ env_logger = { version = "0.11", default-features = false, features = ["humantim
justerror = { version = "1" } justerror = { version = "1" }
lazy_static = "1" lazy_static = "1"
log = { version = "0.4", default-features = false } log = { version = "0.4", default-features = false }
passwords = { version = "3", default-features = false }
rand = { version = "0.8", default-features = false, features = ["getrandom"] } rand = { version = "0.8", default-features = false, features = ["getrandom"] }
serde = { version = "1", default-features = false, features = ["derive"] } serde = { version = "1", default-features = false, features = ["derive"] }
serde_json = { version = "1", default-features = false } serde_json = { version = "1", default-features = false }

View file

@ -14,10 +14,11 @@ use crate::{
user::{ForgejoUser, User}, user::{ForgejoUser, User},
}; };
const PASSWORD_LEN: RangeInclusive<usize> = 4..=100; const PASSWORD_STRENGTH: f64 = 50.0;
const USERNAME_LEN: RangeInclusive<usize> = 1..=50; const USERNAME_LEN: RangeInclusive<usize> = 1..=50;
const DISPLAYNAME_LEN: RangeInclusive<usize> = 0..=100; const DISPLAYNAME_LEN: RangeInclusive<usize> = 0..=100;
const EMAIL_LEN: RangeInclusive<usize> = 4..=50; const EMAIL_LEN: RangeInclusive<usize> = 4..=50;
const CHECKOUT_TIMEOUT: i64 = 12 * 3600;
lazy_static! { lazy_static! {
static ref SIGNUP_KEY: String = format!("meow-{}", random::<u128>()); static ref SIGNUP_KEY: String = format!("meow-{}", random::<u128>());
@ -36,29 +37,48 @@ pub async fn post_signup(
Form(form): Form<SignupForm>, Form(form): Form<SignupForm>,
) -> Result<impl IntoResponse, CreateUserError> { ) -> Result<impl IntoResponse, CreateUserError> {
let user = validate_signup(&form).await?; let user = validate_signup(&form).await?;
dbg!(&*SIGNUP_KEY, &user); match session.insert(&SIGNUP_KEY, user).await {
session.insert(&SIGNUP_KEY, user).await.unwrap(); Ok(_) => {}
Err(e) => {
log::error!(
"Could not insert validated user form into session, got {}",
e
);
return Err(CreateUserErrorKind::UnknownEorr.into());
}
}
Ok(Redirect::to( match session.save().await {
"https://buy.stripe.com/test_eVa6rrb7ygjNbwk000", // TODO: pass in as env var/into a state object that the handlers can read from
)) Ok(_) => Ok(Redirect::to(
"https://buy.stripe.com/test_eVa6rrb7ygjNbwk000",
)),
Err(e) => {
log::error!("Could not save session, got {}", e);
Err(CreateUserErrorKind::UnknownEorr.into())
}
}
} }
/// Redirected from Stripe with the receipt of payment. /// Redirected from Stripe with the receipt of payment.
pub async fn payment_success(session: Session, receipt: Option<Path<String>>) -> impl IntoResponse { pub async fn payment_success(session: Session, receipt: Option<Path<String>>) -> impl IntoResponse {
let user: User = session.get(&SIGNUP_KEY).await.unwrap().unwrap_or_default(); session.load().await.unwrap_or_else(|e| {
// dbg!(&session); log::error!("Could not load the session, got {}", e);
dbg!(&*SIGNUP_KEY, &user); });
if receipt.is_none() { log::debug!("loaded the session");
let user = if let Some(user) = session.get::<User>(&SIGNUP_KEY).await.unwrap_or(None) {
user
} else {
log::warn!("Could not find user in session; got receipt {:?}", receipt);
return CreateUserError(CreateUserErrorKind::NoFormFound).into_response();
};
let receipt = if let Some(Path(receipt)) = receipt {
receipt
} else {
log::info!("Got {:?} from the session, but no receipt.", &user); log::info!("Got {:?} from the session, but no receipt.", &user);
return CreateUserError(CreateUserErrorKind::BadPayment).into_response(); return CreateUserError(CreateUserErrorKind::BadPayment).into_response();
} };
let Path(receipt) = receipt.unwrap();
if user == User::default() {
log::warn!("Could not find user in session; got receipt {}", receipt);
return CreateUserError(CreateUserErrorKind::NoFormFound).into_response();
}
if confirm_payment(&receipt) { if confirm_payment(&receipt) {
log::info!("Confirmed payment from {}", &receipt); log::info!("Confirmed payment from {}", &receipt);
@ -69,13 +89,13 @@ pub async fn payment_success(session: Session, receipt: Option<Path<String>>) ->
if create_user(&user) { if create_user(&user) {
log::info!("Created user {user:?}"); log::info!("Created user {user:?}");
} else { } else {
return CreateUserError(CreateUserErrorKind::AlreadyExists).into_response(); return CreateUserError(CreateUserErrorKind::UnknownEorr).into_response();
} }
// TODO: store the receipt into a durable store to prevent re-use after creating // TODO: store the receipt into a durable store to prevent re-use after creating
// an account // an account
session.delete().await.unwrap_or_else(|e| { session.delete().await.unwrap_or_else(|e| {
log::error!("Got error deleting {} from session, got {}", &user, e); log::error!("Got error deleting {:?} from session, got {}", &user, e);
}); });
log::info!("Added {:?}", &user); log::info!("Added {:?}", &user);
@ -91,14 +111,19 @@ fn create_user(user: &User) -> bool {
.expect("Could not find $ADD_USER_ENDPOINT in environment"); .expect("Could not find $ADD_USER_ENDPOINT in environment");
let auth_header = format!("token {token}"); let auth_header = format!("token {token}");
let user: ForgejoUser = user.into(); let user: ForgejoUser = user.into();
dbg!(&user);
let resp = ureq::post(&format!("{url}/api/v1/admin/users")) let resp = ureq::post(&format!("{url}/api/v1/admin/users"))
.set("Authorization", &auth_header) .set("Authorization", &auth_header)
.set("Content-Type", "application/json") .set("Content-Type", "application/json")
.set("accept", "application/json") .set("accept", "application/json")
.send_json(user) .send_json(user);
.unwrap();
resp.status() == 201 match resp {
Ok(resp) => resp.status() == 201,
Err(resp) => {
log::error!("Got error from user creation request: {}", resp);
false
}
}
} }
fn confirm_payment(stripe_checkout_session_id: &str) -> bool { fn confirm_payment(stripe_checkout_session_id: &str) -> bool {
@ -107,27 +132,32 @@ fn confirm_payment(stripe_checkout_session_id: &str) -> bool {
let json: serde_json::Value = ureq::get(&url) let json: serde_json::Value = ureq::get(&url)
.set("Authorization", &format!("Bearer {token}")) .set("Authorization", &format!("Bearer {token}"))
.call() .call()
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e)) .map_err(|e| {
log::error!("Error confirming payment from Stripe, got {}", e);
std::io::Error::new(std::io::ErrorKind::Other, e)
})
.and_then(|resp| resp.into_json()) .and_then(|resp| resp.into_json())
.unwrap_or_default(); .unwrap_or_default();
// see https://docs.stripe.com/api/checkout/sessions/retrieve // see https://docs.stripe.com/api/checkout/sessions/retrieve
let total = json["amount_total"].as_i64().unwrap_or(0); let total = json["amount_subtotal"].as_i64().unwrap_or(0);
let created_at = json["created"].as_i64().unwrap_or(0); let created_at = json["created"].as_i64().unwrap_or(0);
let now = chrono::Utc::now(); let now = chrono::Utc::now();
let then = chrono::DateTime::from_timestamp(created_at, 0).unwrap(); let then = chrono::DateTime::from_timestamp(created_at, 0).unwrap(); // safe to unwrap
let dur = now - then; let dur = now - then;
let max_elapsed = chrono::TimeDelta::new(12 * 3600, 0).unwrap(); let max_elapsed = chrono::TimeDelta::new(CHECKOUT_TIMEOUT, 0).unwrap();
(dur < max_elapsed) && (total > 0) (dur < max_elapsed) && (total >= 300)
} }
async fn validate_signup(form: &SignupForm) -> Result<User, CreateUserError> { async fn validate_signup(form: &SignupForm) -> Result<User, CreateUserError> {
use passwords::{analyzer::analyze, scorer::score};
let username = form.username.trim(); let username = form.username.trim();
let password = form.password.trim(); let password = form.password.trim();
let verify = form.pw_verify.trim(); let verify = form.pw_verify.trim();
let name_len = username.graphemes(true).size_hint().1.unwrap(); let name_len = username.graphemes(true).size_hint().1.unwrap_or(0);
// we are not ascii exclusivists around here // we are not ascii exclusivists around here
if !USERNAME_LEN.contains(&name_len) { if !USERNAME_LEN.contains(&name_len) {
return Err(CreateUserErrorKind::BadUsername.into()); return Err(CreateUserErrorKind::BadUsername.into());
@ -136,8 +166,8 @@ async fn validate_signup(form: &SignupForm) -> Result<User, CreateUserError> {
if password != verify { if password != verify {
return Err(CreateUserErrorKind::PasswordMismatch.into()); return Err(CreateUserErrorKind::PasswordMismatch.into());
} }
let pwlen = password.graphemes(true).size_hint().1.unwrap_or(0); let strength = score(&analyze(password));
if !PASSWORD_LEN.contains(&pwlen) { if strength < PASSWORD_STRENGTH {
return Err(CreateUserErrorKind::BadPassword.into()); return Err(CreateUserErrorKind::BadPassword.into());
} }

View file

@ -27,13 +27,13 @@ impl IntoResponse for CreateUserError {
#[Error] #[Error]
#[non_exhaustive] #[non_exhaustive]
pub enum CreateUserErrorKind { pub enum CreateUserErrorKind {
#[error(desc = "That username already exists")] #[error(desc = "An unknown error occurred")]
AlreadyExists, UnknownEorr,
#[error(desc = "Usernames must be between 1 and 50 characters long")] #[error(desc = "Usernames must be between 1 and 50 characters long")]
BadUsername, BadUsername,
#[error(desc = "Your passwords didn't match")] #[error(desc = "Your passwords didn't match")]
PasswordMismatch, PasswordMismatch,
#[error(desc = "Password must have at least 4 and at most 100 characters")] #[error(desc = "Your password is too weak")]
BadPassword, BadPassword,
#[error(desc = "Display name must be less than 100 characters long")] #[error(desc = "Display name must be less than 100 characters long")]
BadDisplayname, BadDisplayname,

View file

@ -35,6 +35,7 @@ async fn main() {
let session_store = MemoryStore::default(); let session_store = MemoryStore::default();
let session_layer = SessionManagerLayer::new(session_store) let session_layer = SessionManagerLayer::new(session_store)
.with_secure(false) .with_secure(false)
.with_same_site(tower_sessions::cookie::SameSite::Lax)
.with_expiry(Expiry::OnInactivity(time::Duration::hours(2))); .with_expiry(Expiry::OnInactivity(time::Duration::hours(2)));
// the core application, defining the routes and handlers // the core application, defining the routes and handlers

View file

@ -40,7 +40,10 @@ impl Display for User {
"" ""
}; };
let email = &self.email; let email = &self.email;
write!(f, "Username: {uname}\nDisplayname: {dname}\nEmail: {email}") write!(
f,
"Username: '{uname}', Displayname: '{dname}', Email: '{email}'"
)
} }
} }