You are a constitutional council ranking individual git commits for ownership allocation. Compare these two commits. Decide which contributed more lasting value to the project. Judge substance, not spectacle: - Prefer correct, lasting design and real bugfixes over churn, formatting, renames, or generated noise. - Prefer clarity and necessity over sheer line count. A small precise change can beat a large diffuse one. - Do not favor a side merely because its patch is longer or noisier. - Weight what the change does for the project, not the contributor's name. Return ONLY a JSON object: {"winner": "A" or "B", "ratio": "N:M", "explanation": "..."} The explanation must cite concrete differences in the patches (1-3 sentences). Side A — contributor: tommy-mor Side A — commit message: [8f69c309] Require Reddit OAuth when credentials are set and refresh on 401/403. Avoid falling back to the public www.reddit.com API from cloud IPs, which returns Reddit's network-security block page. Also pin SORTER2_BASE_URL in fly.toml. Co-authored-by: Cursor Side A — unified diff (full patch): diff --git a/fly.toml b/fly.toml index f0c6a39f643c204987debc177234d95a8ef44b65..ca7e0088a7efd7d58d29d808f8d89080b2bff233 100644 --- a/fly.toml +++ b/fly.toml @@ -5,6 +5,7 @@ primary_region = "iad" dockerfile = "Dockerfile" [env] + SORTER2_BASE_URL = "https://reddit.sorter.social" SORTER2_DATA_DIR = "/data" SORTER2_EVENT_LOG = "/data/events.jsonl" PORT = "8080" diff --git a/server/src/reddit.rs b/server/src/reddit.rs index a874814f8927192ee62cab2d0db1efd27dcd57b7..f409764c1e1f36216f1b08107043c2eab905694c 100644 --- a/server/src/reddit.rs +++ b/server/src/reddit.rs @@ -283,26 +283,35 @@ async fn reddit_worker( ); tokio::time::sleep(current_delay).await; - if let Some(c) = &creds { - oauth = ensure_oauth_token(&client, &oauth_token_base, c, oauth.take()).await; - } - - let token = oauth.as_ref().map(|t| t.access_token.as_str()); - let fetch_base = if token.is_some() { - tracing::debug!( - item = %fetch_id, - base = %oauth_api_base, - "reddit fetch using OAuth bearer" - ); - &oauth_api_base - } else { - &api_base - }; - let url = match kind { - FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, fetch_base), - FetchKind::Children => map_children_url(&fetch_id, fetch_base), + let outcome = match &creds { + Some(c) => { + // OAuth is required when credentials are configured — never fall + // back to the public www.reddit.com JSON endpoints (cloud IPs + // get blocked with a 403 HTML interstitial). + fetch_with_oauth( + &client, + &oauth_token_base, + &oauth_api_base, + c, + &mut oauth, + &fetch_id, + kind, + ) + .await + } + None => { + let url = match kind { + FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, &api_base), + FetchKind::Children => map_children_url(&fetch_id, &api_base), + }; + match do_fetch(&client, &url, &fetch_id, None).await { + Ok(FetchOutcome::AuthRejected { status, detail }) => { + Err(format!("Reddit API {status}: {detail}")) + } + other => other, + } + } }; - let outcome = do_fetch(&client, &url, &fetch_id, token).await; match outcome { Ok(FetchOutcome::Payload(payload)) => { @@ -342,6 +351,12 @@ async fn reddit_worker( current_delay = (current_delay * 2).min(Duration::from_secs(60)); notify(done, FetchJobResult::RateLimited { reset_secs }); } + Ok(FetchOutcome::AuthRejected { status, detail }) => { + let e = format!("Reddit API {status}: {detail}"); + tracing::warn!(item = %fetch_id, err = %e, "reddit fetch auth rejected"); + current_delay = (current_delay * 2).min(Duration::from_secs(60)); + notify(done, FetchJobResult::Failed(e)); + } Err(e) => { tracing::warn!(item = %fetch_id, err = %e, "reddit fetch failed"); current_delay = (current_delay * 2).min(Duration::from_secs(60)); @@ -357,6 +372,60 @@ enum FetchOutcome { Payload(Value), NotFound, RateLimited { reset_secs: u64 }, + /// Bearer rejected — caller should drop the cached token and retry once. + AuthRejected { status: StatusCode, detail: String }, +} + +async fn fetch_with_oauth( + client: &Client, + oauth_token_base: &str, + oauth_api_base: &str, + creds: &RedditCredentials, + oauth: &mut Option, + fetch_id: &ItemId, + kind: FetchKind, +) -> Result { + for attempt in 0..2 { + let force_refresh = attempt > 0; + *oauth = Some( + ensure_oauth_token(client, oauth_token_base, creds, oauth.take(), force_refresh) + .await?, + ); + let token = oauth + .as_ref() + .expect("token set above") + .access_token + .clone(); + + tracing::debug!( + item = %fetch_id, + base = %oauth_api_base, + attempt, + "reddit fetch using OAuth bearer" + ); + + let url = match kind { + FetchKind::SelfEntity => map_item_to_reddit_api(fetch_id, oauth_api_base), + FetchKind::Children => map_children_url(fetch_id, oauth_api_base), + }; + match do_fetch(client, &url, fetch_id, Some(&token)).await? { + FetchOutcome::AuthRejected { status, detail } if attempt == 0 => { + tracing::warn!( + item = %fetch_id, + %status, + %detail, + "reddit OAuth rejected; refreshing token and retrying" + ); + *oauth = None; + continue; + } + FetchOutcome::AuthRejected { status, detail } => { + return Err(format!("Reddit API {status}: {detail}")); + } + other => return Ok(other), + } + } + unreachable!("loop always returns") } async fn ensure_oauth_token( @@ -364,35 +433,35 @@ async fn ensure_oauth_token( oauth_base: &str, creds: &RedditCredentials, existing: Option, -) -> Option { - if let Some(t) = existing { - if Instant::now() < t.expires_at - Duration::from_secs(60) { - tracing::debug!("reddit OAuth token still valid"); - return Some(t); + force_refresh: bool, +) -> Result { + if !force_refresh { + if let Some(t) = existing { + if Instant::now() < t.expires_at - Duration::from_secs(60) { + tracing::debug!("reddit OAuth token still valid"); + return Ok(t); + } } } let url = format!("{}/api/v1/access_token", oauth_base.trim_end_matches('/')); - tracing::debug!(%url, "reddit OAuth token request"); + tracing::debug!(%url, force_refresh, "reddit OAuth token request"); let resp = client .post(&url) .basic_auth(&creds.client_id, Some(&creds.client_secret)) .form(&[("grant_type", "client_credentials")]) .send() - .await; - - let resp = match resp { - Ok(r) => r, - Err(e) => { - tracing::warn!("reddit OAuth token request failed: {e}"); - return None; - } - }; + .await + .map_err(|e| format!("Reddit OAuth token request failed: {e}"))?; if !resp.status().is_success() { - tracing::warn!("reddit OAuth token HTTP {}", resp.status()); - return None; + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + return Err(format!( + "Reddit OAuth token HTTP {status}: {}", + truncate_for_error(&body) + )); } #[derive(Deserialize)] @@ -401,21 +470,40 @@ async fn ensure_oauth_token( expires_in: u64, } - let body: TokenResponse = match resp.json().await { - Ok(b) => b, - Err(e) => { - tracing::warn!("reddit OAuth token parse failed: {e}"); - return None; - } - }; + let body: TokenResponse = resp + .json() + .await + .map_err(|e| format!("Reddit OAuth token parse failed: {e}"))?; - tracing::debug!(expires_in = body.expires_in, "reddit OAuth token acquired"); - Some(OAuthToken { + tracing::info!(expires_in = body.expires_in, "reddit OAuth token acquired"); + Ok(OAuthToken { access_token: body.access_token, expires_at: Instant::now() + Duration::from_secs(body.expires_in), }) } +fn truncate_for_error(body: &str) -> String { + let compact: String = body.split_whitespace().collect::>().join(" "); + if compact.is_empty() { + return "(empty body)".into(); + } + // Prefer the human-readable block message over dumping Reddit's CSS. + if let Some(idx) = compact.find("You've been blocked") { + let slice: String = compact.chars().skip(idx).take(160).collect(); + return if compact.chars().count() > idx + 160 { + format!("{slice}…") + } else { + slice + }; + } + let chars: String = compact.chars().take(200).collect(); + if compact.chars().count() > 200 { + format!("{chars}…") + } else { + chars + } +} + async fn do_fetch( client: &Client, url: &str, @@ -460,15 +548,16 @@ async fn do_fetch( if !status.is_success() { let body = resp.text().await.unwrap_or_default(); + let detail = truncate_for_error(&body); tracing::debug!( item = %id, %status, body_len = body.len(), - body_prefix = %body.chars().take(240).collect::(), + %detail, "reddit non-success body" ); if status == StatusCode::FORBIDDEN || status == StatusCode::UNAUTHORIZED { - return Err(format!("Reddit API {status}: {body}")); + return Ok(FetchOutcome::AuthRejected { status, detail }); } return Ok(FetchOutcome::NotFound); } @@ -716,6 +805,15 @@ fn reddit_direct_image_url(url: &str) -> bool { mod tests { use super::*; + #[test] + fn truncate_error_prefers_block_message() { + let html = r#"
You've been blocked by network security. To continue, log in
"#; + let msg = truncate_for_error(html); + assert!(msg.starts_with("You've been blocked")); + assert!(msg.len() < 200); + assert!(!msg.contains(".x{color")); + } + #[test] fn map_subreddit_about_url() { let id = ItemId::from_url("https://reddit.com/r/rust").unwrap(); Side B — contributor: tommy-mor Side B — commit message: [880eb778] Harden auth: fail-closed votes, mock OAuth gate, Secure cookies. Also show the current alias in the top nav and pin durable by rev. Co-authored-by: Cursor Side B — unified diff (full patch): diff --git a/AGENTS.md b/AGENTS.md index babb889d6fbfb1fa7176c9e6b7544ae17b61dd2e..6e0fd8ebb65d665c9c1438e3275971d62b98fd95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,11 +10,11 @@ Single Rust web app **`sorter2-server`**: pairwise voting, rank-centrality ranki - **Bootstrap script**: `./scripts/cursor-env-install.sh` (also run via `.cursor/environment.json` on Cloud Agent boot) installs Playwright Chromium, Babashka, bbin, `clj-paren-repair`, and warms the RocksDB build. - **Rust 1.88+** is required (`rust-toolchain.toml`). The Cloud Dockerfile and `cursor-env-install.sh` install **rustup** 1.88.0 first so `cargo` works while Playwright/Clojure bootstrap continues. Do not rely on `/usr/local/cargo` (often missing or stale). -- **RocksDB / `durable`**: Ubuntu’s default `c++` is often **clang** without libc++ headers. Set **`CXX=g++`** and **`RUSTFLAGS="-C linker=g++"`** (or `CC=gcc`) before `cargo build` / `cargo test` — both are set in the bootstrap script and `.cursor/environment.json`. +- **RocksDB / `durable`**: `durable` is an external git dependency (`tommy-mor/durable`, pinned by rev in `server/Cargo.toml`). Ubuntu’s default `c++` is often **clang** without libc++ headers. Set **`CXX=g++`** and **`RUSTFLAGS="-C linker=g++"`** (or `CC=gcc`) before `cargo build` / `cargo test` — both are set in the bootstrap script and `.cursor/environment.json`. - **System packages** for builds: `build-essential`, `g++`, `clang`, `libclang-dev`, `pkg-config`, `libssl-dev`, `openjdk-21-jre-headless` (for `reqwest` / OpenSSL, `librocksdb-sys`, `zstd-sys` / bindgen, and **bbin** / Clojure JVM). The bootstrap sets **`JAVA_HOME`** when Java is present. - **Clojure CLI 1.12.0.1530** (used in CI): install from https://clojure.org/guides/install_clojure — needed for `./scripts/clj-test.sh` / Kaocha tests. - **Babashka / bbin / clj-paren-repair**: installed by `cursor-env-install.sh` into `~/.local/bin` (bb tasks in `bb.edn`, delimiter repair for Clojure edits). -- **Playwright** (Spel browser tests in `test/vote_compare.clj`): Chromium via `clojure -M -e "(com.microsoft.playwright.CLI/main ...)"` — run once after clone or use the bootstrap script. +- **Playwright** (Spel browser tests in `test/vote_compare.clj` / `test/auth_login.clj`): Chromium via `clojure -M -e "(com.microsoft.playwright.CLI/main ...)"` — run once after clone or use the bootstrap script. ### Commands (see also `TEST.sh`) @@ -34,13 +34,17 @@ Environment variables (defaults in `server/src/state.rs`): - `PORT` — default `8080` - `SORTER2_DATA_DIR` — default `./data` (created on startup) - `SORTER2_EVENT_LOG` — default `{data_dir}/events.jsonl` +- `SORTER2_BASE_URL` — public origin (also drives Secure cookies when `https://`) +- `GITHUB_CLIENT_ID` / `GITHUB_CLIENT_SECRET` — GitHub OAuth (optional; login disabled if unset) +- `SORTER2_ALLOW_MOCK_OAUTH=1` — allow `mock_user` on `/auth/github` (tests only) Health check: `GET /healthz` → `ok`. -Core UI flow: `POST /ui` with form field `__rpc__` (JSON). Example vote: +Core UI flow: `POST /ui` with form field `__rpc__` (JSON). Votes require a session cookie (sign in via `/login`). Example vote: ```bash curl -sf -X POST http://127.0.0.1:8080/ui \ + --cookie "sorter2_session=..." \ --data-urlencode '__rpc__={"action":"record_vote","a":"alpha","b":"beta","ratio_left":2,"ratio_right":1}' ``` diff --git a/Cargo.lock b/Cargo.lock index aa02997ad85777195f135bfd9456bcee0fc9a590..1f8690f3e486d099577a32c2ece48caf57ea7160 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -414,7 +414,7 @@ dependencies = [ [[package]] name = "durable" version = "0.2.0" -source = "git+https://github.com/tommy-mor/durable.git?branch=main#a6c14eaa809693140eea0c22b07ef24d8e74adaf" +source = "git+https://github.com/tommy-mor/durable.git?rev=a6c14eaa809693140eea0c22b07ef24d8e74adaf#a6c14eaa809693140eea0c22b07ef24d8e74adaf" dependencies = [ "ciborium", "durable-derive", @@ -426,7 +426,7 @@ dependencies = [ [[package]] name = "durable-derive" version = "0.2.0" -source = "git+https://github.com/tommy-mor/durable.git?branch=main#a6c14eaa809693140eea0c22b07ef24d8e74adaf" +source = "git+https://github.com/tommy-mor/durable.git?rev=a6c14eaa809693140eea0c22b07ef24d8e74adaf#a6c14eaa809693140eea0c22b07ef24d8e74adaf" dependencies = [ "proc-macro2", "quote", diff --git a/server/Cargo.toml b/server/Cargo.toml index dfa39beddecfa37dcdeaa602cb30f4b547528fbb..bd88687fb0ba47d68f2c08eb5e11d0e08b7c4398 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -25,7 +25,7 @@ futures-util = { version = "0.3", default-features = false, features = ["std"] } rand = "0.8" urlencoding = "2" url = "2" -durable = { git = "https://github.com/tommy-mor/durable.git", branch = "main" } +durable = { git = "https://github.com/tommy-mor/durable.git", rev = "a6c14eaa809693140eea0c22b07ef24d8e74adaf" } [dev-dependencies] reqwest = { version = "0.12", features = ["json"] } diff --git a/server/src/api/ui_html.rs b/server/src/api/ui_html.rs index b86581b1f337650564274254d840e8a75b49524d..9da62ffbed07eb28729aa3160bf33b07ce0d7945 100644 --- a/server/src/api/ui_html.rs +++ b/server/src/api/ui_html.rs @@ -71,10 +71,16 @@ pub async fn post_ui_html( return resp; } let parent = parent_from_scope(&scope); - let actor = resolve_vote_actor( + let actor = match resolve_vote_actor( state.projection_store.db(), session_id_from_jar(&jar).as_deref(), - ); + ) { + Ok(actor) => actor, + Err(_) => { + return vote_auth_redirect(&state, &jar) + .unwrap_or_else(|| login_redirect_js().into_response()); + } + }; if let Err(e) = state .record_vote(&parent, &a, &b, ratio_left, ratio_right, &actor) .await diff --git a/server/src/auth/config.rs b/server/src/auth/config.rs index a1f042c655bf3e5234eeb87a7d889f64592807fb..a5976af9a52ea207b35ae87bd1fe927c47a477ca 100644 --- a/server/src/auth/config.rs +++ b/server/src/auth/config.rs @@ -1,9 +1,42 @@ pub const AUTH_RETURN_COOKIE: &str = "sorter2_auth_return"; +/// Allow `mock_user` on `/auth/github` (test harness only). +pub fn mock_oauth_allowed() -> bool { + matches!( + std::env::var("SORTER2_ALLOW_MOCK_OAUTH").as_deref(), + Ok("1") | Ok("true") | Ok("TRUE") + ) +} + +/// Set the Secure flag on auth cookies when serving over HTTPS. +pub fn cookies_secure() -> bool { + std::env::var("SORTER2_BASE_URL") + .map(|u| u.starts_with("https://")) + .unwrap_or(false) +} + pub fn sanitize_return_to(raw: &str) -> String { let s = raw.trim(); - if s.is_empty() || !s.starts_with('/') || s.starts_with("//") { + if s.is_empty() || !s.starts_with('/') || s.starts_with("//") || s.starts_with("/\\") { + return "/".to_string(); + } + // Reject scheme-relative and protocol-smuggling forms. + if s.contains("://") || s.contains('\\') { return "/".to_string(); } s.to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sanitize_return_to_blocks_open_redirects() { + assert_eq!(sanitize_return_to(""), "/"); + assert_eq!(sanitize_return_to("//evil.com"), "/"); + assert_eq!(sanitize_return_to("/\\evil.com"), "/"); + assert_eq!(sanitize_return_to("https://evil.com"), "/"); + assert_eq!(sanitize_return_to("/vote?parent=x"), "/vote?parent=x"); + } +} diff --git a/server/src/auth/mod.rs b/server/src/auth/mod.rs index 5ed535ba199fa736f0048c32623b14c3b1e5de2d..d4a85ef52c15dc35148e4c743f0d646cbbdb056d 100644 --- a/server/src/auth/mod.rs +++ b/server/src/auth/mod.rs @@ -26,7 +26,7 @@ use crate::{ ui_action::UI_RPC_FIELD, }; -pub use session::{resolve_vote_actor, session_id_from_jar, VoteActor}; +pub use session::{nav_pseudonym, resolve_vote_actor, session_id_from_jar, VoteActor}; pub fn base_url_from_env(port: u16) -> String { std::env::var("SORTER2_BASE_URL") @@ -168,6 +168,10 @@ pub async fn login_page( "login · sorter2", login_body(session.as_ref(), &aliases, &providers), state.views.get_views("/login"), + session + .as_ref() + .filter(|s| !s.pseudonym.trim().is_empty()) + .map(|s| s.pseudonym.as_str()), ); (jar, Html(markup.into_string())).into_response() } @@ -222,6 +226,7 @@ pub async fn alias_page( "choose alias · sorter2", body, state.views.get_views("/login/alias"), + None, ) .into_string(), ) @@ -237,7 +242,12 @@ pub async fn github_start( .ok_or(StatusCode::SERVICE_UNAVAILABLE)?; let return_to = return_from_query_or_jar(&jar, query.return_to.as_deref()); let state_token = session::new_oauth_state(); - let url = oauth::authorize_url(&cfg, &state_token, query.mock_user.as_deref()); + let mock_user = if config::mock_oauth_allowed() { + query.mock_user.as_deref() + } else { + None + }; + let url = oauth::authorize_url(&cfg, &state_token, mock_user); let jar = jar .add(session::oauth_state_cookie_value(&state_token)) .add(session::auth_return_cookie_value(&return_to)); diff --git a/server/src/auth/session.rs b/server/src/auth/session.rs index 09659240b9455c6fca12db5652e1d31cf8c2acfc..41df030ded3abcafc0ab3887ab769adf103f9aa0 100644 --- a/server/src/auth/session.rs +++ b/server/src/auth/session.rs @@ -5,7 +5,7 @@ use durable::{Db, Durability}; use rand::Rng; use crate::{ - auth::config::AUTH_RETURN_COOKIE, + auth::config::{self, AUTH_RETURN_COOKIE}, fetch::now_ms, identity::{DEFAULT_ACTOR_UUID, DEFAULT_PSEUDONYM}, storage_dto::{SessionDataV1, SESSION_DATA_VERSION}, @@ -37,6 +37,7 @@ pub struct VoteActor { } impl VoteActor { + /// Test / bench helper: seed votes as the default pseudonym without a session. pub fn anon() -> Self { Self { pseudonym: DEFAULT_PSEUDONYM.to_string(), @@ -70,20 +71,51 @@ fn hex_encode(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } -pub fn resolve_vote_actor(db: &Db, session_id: Option<&str>) -> VoteActor { - let Some(session_id) = session_id else { - return VoteActor::anon(); - }; - let Ok(Some(session)) = load_session(db, session_id) else { - return VoteActor::anon(); - }; - if session.expires_at <= now_ms() { - return VoteActor::anon(); +fn build_cookie(name: &'static str, value: String) -> Cookie<'static> { + let mut builder = Cookie::build((name, value)) + .http_only(true) + .same_site(SameSite::Lax) + .path("/"); + if config::cookies_secure() { + builder = builder.secure(true); + } + builder.build() +} + +fn clear_cookie(name: &'static str) -> Cookie<'static> { + let mut builder = Cookie::build((name, "")) + .http_only(true) + .same_site(SameSite::Lax) + .path("/") + .removal(); + if config::cookies_secure() { + builder = builder.secure(true); + } + builder.build() +} + +/// Resolve the vote actor from a live session. Fail-closed: never falls back to anon. +pub fn resolve_vote_actor(db: &Db, session_id: Option<&str>) -> Result { + let session_id = session_id.ok_or("sign in to vote")?; + let session = load_valid_session(db, session_id).ok_or("session expired")?; + if !session_has_pseudonym(&session) { + return Err("choose an alias first"); } let trust_weight = user_trust_weight(db, &session.uuid).unwrap_or(1.0); - VoteActor { + Ok(VoteActor { pseudonym: session.current_pseudonym, trust_weight, + }) +} + +/// Display name for the top nav, if any session is active. +pub fn nav_pseudonym(db: &Db, jar: &CookieJar) -> Option { + let session_id = session_id_from_jar(jar)?; + let session = load_valid_session(db, &session_id)?; + if session_has_pseudonym(&session) { + Some(session.current_pseudonym) + } else { + None } } @@ -151,54 +183,27 @@ pub fn destroy_session(db: &Db, session_id: &str) -> Result<(), String> { } pub fn session_cookie_value(session_id: &str) -> Cookie<'static> { - Cookie::build((SESSION_COOKIE, session_id.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(SESSION_COOKIE, session_id.to_string()) } pub fn clear_session_cookie() -> Cookie<'static> { - Cookie::build((SESSION_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(SESSION_COOKIE) } pub fn oauth_state_cookie_value(state: &str) -> Cookie<'static> { - Cookie::build((OAUTH_STATE_COOKIE, state.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(OAUTH_STATE_COOKIE, state.to_string()) } pub fn clear_oauth_state_cookie() -> Cookie<'static> { - Cookie::build((OAUTH_STATE_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(OAUTH_STATE_COOKIE) } pub fn auth_return_cookie_value(return_to: &str) -> Cookie<'static> { - Cookie::build((AUTH_RETURN_COOKIE, return_to.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(AUTH_RETURN_COOKIE, return_to.to_string()) } pub fn clear_auth_return_cookie() -> Cookie<'static> { - Cookie::build((AUTH_RETURN_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(AUTH_RETURN_COOKIE) } pub fn auth_return_from_jar(jar: &CookieJar) -> Option { @@ -213,28 +218,35 @@ pub fn oauth_state_from_jar(jar: &CookieJar) -> Option { jar.get(OAUTH_STATE_COOKIE).map(|c| c.value().to_string()) } -pub fn actor_uuid_for_vote(db: &Db, session_id: Option<&str>) -> String { - let Some(session_id) = session_id else { - return DEFAULT_ACTOR_UUID.to_string(); - }; - load_session(db, session_id) - .ok() - .flatten() - .filter(|s| s.expires_at > now_ms()) - .map(|s| s.uuid) - .unwrap_or_else(|| DEFAULT_ACTOR_UUID.to_string()) -} - #[cfg(test)] mod tests { use super::*; #[test] - fn missing_session_falls_back_to_anon() { + fn missing_session_is_error() { + let dir = tempfile::tempdir().unwrap(); + let db = Db::open(dir.path()).unwrap(); + assert_eq!(resolve_vote_actor(&db, None).unwrap_err(), "sign in to vote"); + } + + #[test] + fn session_without_pseudonym_is_error() { + let dir = tempfile::tempdir().unwrap(); + let db = Db::open(dir.path()).unwrap(); + let (id, _) = create_session(&db, DEFAULT_ACTOR_UUID, "").unwrap(); + assert_eq!( + resolve_vote_actor(&db, Some(&id)).unwrap_err(), + "choose an alias first" + ); + } + + #[test] + fn session_with_pseudonym_resolves() { let dir = tempfile::tempdir().unwrap(); let db = Db::open(dir.path()).unwrap(); - let actor = resolve_vote_actor(&db, None); - assert_eq!(actor.pseudonym, DEFAULT_PSEUDONYM); + let (id, _) = create_session(&db, DEFAULT_ACTOR_UUID, "alice").unwrap(); + let actor = resolve_vote_actor(&db, Some(&id)).unwrap(); + assert_eq!(actor.pseudonym, "alice"); assert_eq!(actor.trust_weight, 1.0); } } diff --git a/server/src/html/mod.rs b/server/src/html/mod.rs index 46d18b87f1313bf0aeb29955d18f291961057509..3cc3d7bdf55b5cb5d009600f4ade1fcd201a410b 100644 --- a/server/src/html/mod.rs +++ b/server/src/html/mod.rs @@ -4,11 +4,13 @@ use axum::{ http::{header, StatusCode, Uri}, response::{IntoResponse, Response}, }; +use axum_extra::extract::cookie::CookieJar; use maud::{html, Markup, DOCTYPE}; use std::collections::HashSet; use crate::{ + auth::nav_pseudonym, fetch::html::entity_section, form_template::template_json_compact, path_types::ItemId, @@ -126,7 +128,7 @@ pub fn now_ms() -> i64 { t.as_millis() as i64 } -pub(crate) fn layout(title: &str, body: Markup, views: u64) -> Markup { +pub(crate) fn layout(title: &str, body: Markup, views: u64, nav_user: Option<&str>) -> Markup { let ver = asset_version(); let css_href = format!("/static/sorter.css?v={ver}"); let js_src = format!("/static/sorter_ui.js?v={ver}"); @@ -145,7 +147,15 @@ pub(crate) fn layout(title: &str, body: Markup, views: u64) -> Markup { span class="view-meta muted" { (views) " views" } } nav class="top-nav" { - a href="/login" { "login" } + @if let Some(name) = nav_user { + span class="top-nav-user" data-testid="nav-user" { (name) } + a href="/login" { "account" } + form class="top-nav-logout" method="post" action="/auth/logout" data-navigate="full" { + button type="submit" data-testid="nav-logout" { "log out" } + } + } @else { + a href="/login" data-testid="nav-login" { "login" } + } } div id="errors" {} (body) @@ -481,10 +491,11 @@ pub fn input_panel(query: &str, error: Option<&str>) -> Markup { } } -async fn item_page(state: AppState, uri: Uri, item: ItemId) -> Markup { +async fn item_page(state: AppState, uri: Uri, item: ItemId, jar: CookieJar) -> Markup { let path = uri.path().to_string(); state.views.increment(path.clone()); let views = state.views.get_views(&path); + let nav_user = nav_pseudonym(state.projection_store.db(), &jar); let tree = state .scope_tree(&item) @@ -513,16 +524,24 @@ async fn item_page(state: AppState, uri: Uri, item: ItemId) -> Markup { (ranking_panel(&item, node, &tree)) } }; - layout("sorter2", body, views) + layout("sorter2", body, views, nav_user.as_deref()) } -pub async fn home(State(state): State, uri: Uri) -> impl IntoResponse { - item_page(state, uri, ItemId::root()).await +pub async fn home( + State(state): State, + jar: CookieJar, + uri: Uri, +) -> impl IntoResponse { + item_page(state, uri, ItemId::root(), jar).await } -pub async fn browse(State(state): State, uri: Uri) -> impl IntoResponse { +pub async fn browse( + State(state): State, + jar: CookieJar, + uri: Uri, +) -> impl IntoResponse { let item = ItemId::from_browse_uri(uri.path()).unwrap_or(ItemId::root()); - item_page(state, uri, item).await + item_page(state, uri, item, jar).await } #[cfg(test)] diff --git a/server/src/html/vote.rs b/server/src/html/vote.rs index 3aa00c417c89a9cab3417c650b50ed7c73f08e20..cadbec188b17a48a63b269c0e2fa2ea8ffedd7ed 100644 --- a/server/src/html/vote.rs +++ b/server/src/html/vote.rs @@ -4,11 +4,13 @@ use axum::{ extract::{Query, State}, response::{Html, IntoResponse}, }; +use axum_extra::extract::cookie::CookieJar; use maud::{html, Markup}; use serde::Deserialize; use std::collections::HashSet; use crate::{ + auth::nav_pseudonym, fetch::html::entity_section, form_template::template_json_compact, html::{ranking_panel_with_highlights, scope_theme_style, JsBuilder}, @@ -262,6 +264,7 @@ fn suggest_next( pub async fn vote_page( State(state): State, + jar: CookieJar, Query(q): Query, ) -> impl IntoResponse { let parent = parse_item_param(&q.parent); @@ -329,8 +332,9 @@ pub async fn vote_page( let path = format!("/vote?parent={}", urlencoding::encode(parent.as_str())); state.views.increment(path.clone()); let views = state.views.get_views(&path); + let nav_user = nav_pseudonym(state.projection_store.db(), &jar); - Html(layout(&title, body, views).into_string()).into_response() + Html(layout(&title, body, views, nav_user.as_deref()).into_string()).into_response() } #[cfg(test)] diff --git a/server/static/sorter.css b/server/static/sorter.css index e66a1e6c1acc473c8ff1ddb1e16e75a82d33741c..257280b6b490c65222e580a325b77351cac6cc6b 100644 --- a/server/static/sorter.css +++ b/server/static/sorter.css @@ -34,6 +34,47 @@ body { font-size: 0.75rem; } +.top-nav { + display: flex; + align-items: center; + justify-content: flex-end; + gap: 0.75rem; + padding: 0.5rem 1rem; + font-size: 0.875rem; +} + +.top-nav a { + color: var(--muted); + text-decoration: none; +} + +.top-nav a:hover { + color: var(--fg); +} + +.top-nav-user { + color: var(--fg); + font-weight: 600; +} + +.top-nav-logout { + display: inline; + margin: 0; +} + +.top-nav-logout button { + background: none; + border: none; + padding: 0; + color: var(--muted); + font: inherit; + cursor: pointer; +} + +.top-nav-logout button:hover { + color: var(--fg); +} + .btn-primary { background: var(--accent); color: var(--accent-fg, #0f1115); diff --git a/test/support/harness.clj b/test/support/harness.clj index 3f05951f418258642dcacb4a10ccccc8bfbe8748..4505ece5aa193e826ca61c52f1467d252080d66b 100644 --- a/test/support/harness.clj +++ b/test/support/harness.clj @@ -43,6 +43,7 @@ "SORTER2_VIEWS_LOG" (str data-dir "/views.jsonl") "PORT" (str app-port) "SORTER2_BASE_URL" (str "http://127.0.0.1:" app-port) + "SORTER2_ALLOW_MOCK_OAUTH" "1" "GITHUB_CLIENT_ID" "test-client" "GITHUB_CLIENT_SECRET" "test-secret" "GITHUB_OAUTH_BASE" (str "http://127.0.0.1:" oauth-port)