fix(security): replace predictable HMAC unsubscribe token with randomopaque token (#896)

services/api/Cargo.toml

@@ -46,6 +46,7 @@ hmac = "0.12"
 hex = "0.4"
 base64 = "0.22"
 subtle = "2.5"
+rand = { version = "0.8", features = ["getrandom"] }
 ipnet = "2"
 
 [dev-dependencies]

services/api/database/migrations/017_create_unsubscribe_tokens.sql

@@ -0,0 +1,37 @@
+-- Migration 017: opaque unsubscribe tokens
+--
+-- Replaces the predictable base64(email || "." || HMAC(email)) scheme with
+-- random 256-bit tokens stored in the database.
+--
+-- Schema:
+--   token_hash  SHA-256 hex of the 32-byte random token stored by the caller.
+--               Only the hash is persisted so a DB breach does not expose
+--               usable tokens.
+--   subscriber_id  FK to newsletter_subscribers(id). Cascade-delete keeps the
+--               table clean when a subscriber is hard-deleted.
+--   expires_at  Tokens expire after the configured TTL (default 7 days).
+--   used_at     Set on first use; subsequent uses are rejected (single-use).
+
+CREATE TABLE IF NOT EXISTS unsubscribe_tokens (
+    id            UUID        PRIMARY KEY DEFAULT gen_random_uuid(),
+    token_hash    CHAR(64)    NOT NULL UNIQUE,          -- SHA-256 hex, 64 chars
+    subscriber_id UUID        NOT NULL
+                  REFERENCES newsletter_subscribers(id)
+                  ON DELETE CASCADE,
+    expires_at    TIMESTAMPTZ NOT NULL,
+    used_at       TIMESTAMPTZ             DEFAULT NULL,
+    created_at    TIMESTAMPTZ NOT NULL    DEFAULT NOW()
+);
+
+-- Fast lookup by hash (primary use-case: validate incoming token)
+CREATE INDEX IF NOT EXISTS idx_unsubscribe_tokens_hash
+    ON unsubscribe_tokens (token_hash);
+
+-- Housekeeping: quickly find expired / already-used tokens for cleanup
+CREATE INDEX IF NOT EXISTS idx_unsubscribe_tokens_expires_at
+    ON unsubscribe_tokens (expires_at)
+    WHERE used_at IS NULL;
+
+COMMENT ON TABLE unsubscribe_tokens IS
+    'Single-use opaque unsubscribe tokens (256-bit random, SHA-256 hashed). '
+    'See services/api/src/newsletter.rs and issue #896.';

services/api/src/newsletter.rs

@@ -3,40 +3,67 @@ use std::time::{Duration, Instant};
 use crate::cache::RedisCache;
 
 use anyhow::Context;
-use base64::{engine::general_purpose::URL_SAFE_NO_PAD as BASE64URL, Engine};
-use hmac::{Hmac, Mac};
-use sha2::Sha256;
+use rand::RngCore;
+use sha2::{Digest, Sha256};
 use serde_json::json;
+use uuid::Uuid;
 
 use crate::config::Config;
 
-type HmacSha256 = Hmac<Sha256>;
-
-/// Generate a signed unsubscribe token for `email`.
-/// Format: `<base64url(email)>.<hmac_signature>`
-pub fn generate_unsubscribe_token(email: &str, secret: &str) -> anyhow::Result<String> {
-    let payload = BASE64URL.encode(email.as_bytes());
-    let mut mac = HmacSha256::new_from_slice(secret.as_bytes())
-        .map_err(|_| anyhow::anyhow!("invalid signing key"))?;
-    mac.update(payload.as_bytes());
-    let sig = BASE64URL.encode(mac.finalize().into_bytes());
-    Ok(format!("{payload}.{sig}"))
+// ── Opaque unsubscribe tokens ────────────────────────────────────────────────
+//
+// Token format (issue #896)
+// ─────────────────────────
+// The old scheme encoded the subscriber's email address in the token itself
+// (`base64(email) + "." + HMAC(email)`), making the structure guessable and
+// allowing an attacker who observes one valid token to enumerate subscribers
+// or attempt HMAC-key recovery.
+//
+// The new scheme:
+//  1. Generate 32 cryptographically-random bytes via `rand::OsRng`.
+//  2. Hex-encode those bytes to produce a 64-character URL-safe token.
+//  3. Store SHA-256(token) in the `unsubscribe_tokens` table alongside the
+//     subscriber_id and an expiry timestamp.
+//  4. On redemption: hash the incoming token, look up the hash in the DB,
+//     verify `expires_at > NOW()` and `used_at IS NULL`, then set `used_at`.
+//  5. The raw token is returned to the caller exactly once (to be embedded
+//     in the email). Only the hash is persisted; a DB breach exposes no
+//     usable tokens.
+
+/// Raw token length in bytes. 32 bytes = 256 bits of entropy.
+const TOKEN_BYTES: usize = 32;
+
+/// Generate a random 256-bit unsubscribe token.
+///
+/// Returns `(raw_token, token_hash)` where:
+/// - `raw_token`  — 64-character lowercase hex string sent to the subscriber.
+///                  Store nowhere; embed once in the unsubscribe URL.
+/// - `token_hash` — SHA-256 hex of `raw_token`. Persist this in the DB.
+pub fn generate_opaque_unsubscribe_token() -> (String, String) {
+    let mut bytes = [0u8; TOKEN_BYTES];
+    rand::rngs::OsRng.fill_bytes(&mut bytes);
+    let raw = hex::encode(bytes);
+    let hash = hex::encode(Sha256::digest(raw.as_bytes()));
+    (raw, hash)
 }
 
-/// Validate a signed unsubscribe token. Returns the email on success.
-pub fn validate_unsubscribe_token(token: &str, secret: &str) -> Option<String> {
-    let (payload, sig) = token.split_once('.')?;
-    let mut mac = HmacSha256::new_from_slice(secret.as_bytes()).ok()?;
-    mac.update(payload.as_bytes());
-    let expected = BASE64URL.encode(mac.finalize().into_bytes());
-    // Constant-time comparison via subtle
-    use subtle::ConstantTimeEq;
-    if bool::from(expected.as_bytes().ct_eq(sig.as_bytes())) {
-        let email_bytes = BASE64URL.decode(payload).ok()?;
-        String::from_utf8(email_bytes).ok()
-    } else {
-        None
-    }
+/// Hash an incoming raw token for database lookup.
+///
+/// Call this on the token value received from the URL query parameter before
+/// querying `unsubscribe_tokens.token_hash`.
+pub fn hash_unsubscribe_token(raw_token: &str) -> String {
+    hex::encode(Sha256::digest(raw_token.as_bytes()))
+}
+
+/// Result of attempting to redeem an unsubscribe token.
+#[derive(Debug, PartialEq)]
+pub enum UnsubscribeTokenResult {
+    /// Token is valid; `subscriber_id` identifies the subscriber to remove.
+    Valid { subscriber_id: uuid::Uuid },
+    /// Token has already been used (single-use enforcement).
+    AlreadyUsed,
+    /// Token not found or has expired.
+    InvalidOrExpired,
 }
 
 #[derive(Clone)]
@@ -68,6 +95,69 @@ impl IpRateLimiter {
     }
 }
 
+// ---------------------------------------------------------------------------
+// OpaqueTokenStore — in-memory mirror of the `unsubscribe_tokens` DB table.
+// Used by unit tests; mirrors the DB contract without requiring Postgres.
+// ---------------------------------------------------------------------------
+
+struct OpaqueEntry {
+    subscriber_id: uuid::Uuid,
+    expires_at: Instant,
+    used_at: Option<Instant>,
+}
+
+/// Minimal in-memory store that replicates the token lifecycle enforced by
+/// the `unsubscribe_tokens` table (migration 017).
+pub struct OpaqueTokenStore {
+    entries: std::collections::HashMap<String, OpaqueEntry>, // key = token_hash
+    token_ttl: Duration,
+}
+
+impl OpaqueTokenStore {
+    pub fn new(token_ttl: Duration) -> Self {
+        Self {
+            entries: std::collections::HashMap::new(),
+            token_ttl,
+        }
+    }
+
+    /// Insert a pre-hashed token associated with `subscriber_id`.
+    pub fn insert(&mut self, token_hash: String, subscriber_id: uuid::Uuid) {
+        self.entries.insert(
+            token_hash,
+            OpaqueEntry {
+                subscriber_id,
+                expires_at: Instant::now() + self.token_ttl,
+                used_at: None,
+            },
+        );
+    }
+
+    /// Attempt to redeem `raw_token`. Hashes it internally before lookup.
+    ///
+    /// On success marks the entry as used (single-use enforcement) and returns
+    /// the subscriber_id. Subsequent calls with the same token return
+    /// `AlreadyUsed`.
+    pub fn redeem(&mut self, raw_token: &str) -> UnsubscribeTokenResult {
+        let hash = hash_unsubscribe_token(raw_token);
+        match self.entries.get_mut(&hash) {
+            None => UnsubscribeTokenResult::InvalidOrExpired,
+            Some(entry) => {
+                if Instant::now() > entry.expires_at {
+                    return UnsubscribeTokenResult::InvalidOrExpired;
+                }
+                if entry.used_at.is_some() {
+                    return UnsubscribeTokenResult::AlreadyUsed;
+                }
+                entry.used_at = Some(Instant::now());
+                UnsubscribeTokenResult::Valid {
+                    subscriber_id: entry.subscriber_id,
+                }
+            }
+        }
+    }
+}
+
 // ---------------------------------------------------------------------------
 // In-memory token store — models the subscribe/confirm/expiry lifecycle.
 // Used by tests; mirrors the DB contract without requiring Postgres.
@@ -234,33 +324,77 @@ mod tests {
     }
 
     // -------------------------------------------------------------------------
-    // Unsubscribe token tests
+    // #896: Opaque unsubscribe token tests
     // -------------------------------------------------------------------------
 
     #[test]
-    fn unsubscribe_token_roundtrip() {
-        let email = "user@example.com";
-        let secret = "test-secret";
-        let token = generate_unsubscribe_token(email, secret).unwrap();
-        assert_eq!(validate_unsubscribe_token(&token, secret), Some(email.to_string()));
+    fn opaque_token_generation_produces_unique_tokens() {
+        let (raw1, hash1) = generate_opaque_unsubscribe_token();
+        let (raw2, hash2) = generate_opaque_unsubscribe_token();
+
+        // Tokens must be 64 hex chars (32 bytes)
+        assert_eq!(raw1.len(), 64);
+        assert_eq!(raw2.len(), 64);
+
+        // Two consecutive tokens must never be equal (birthday-attack probability ≈ 2^-256)
+        assert_ne!(raw1, raw2);
+        assert_ne!(hash1, hash2);
     }
 
     #[test]
-    fn unsubscribe_token_wrong_secret_rejected() {
-        let token = generate_unsubscribe_token("user@example.com", "secret-a").unwrap();
-        assert_eq!(validate_unsubscribe_token(&token, "secret-b"), None);
+    fn opaque_token_hash_is_sha256_of_raw() {
+        let (raw, hash) = generate_opaque_unsubscribe_token();
+        // Re-hash independently and compare
+        let recomputed = hash_unsubscribe_token(&raw);
+        assert_eq!(hash, recomputed);
     }
 
     #[test]
-    fn unsubscribe_token_tampered_rejected() {
-        let token = generate_unsubscribe_token("user@example.com", "secret").unwrap();
-        let tampered = format!("{token}x");
-        assert_eq!(validate_unsubscribe_token(&tampered, "secret"), None);
+    fn opaque_token_hash_does_not_contain_raw() {
+        let (raw, hash) = generate_opaque_unsubscribe_token();
+        // The stored hash must not equal or contain the raw token
+        assert_ne!(raw, hash);
+        assert!(!hash.contains(&raw));
+    }
+
+    #[test]
+    fn opaque_token_replay_detection_via_in_memory_store() {
+        // Models the DB-level single-use enforcement with the in-memory
+        // OpaqueTokenStore (see below in this file).
+        let mut store = OpaqueTokenStore::new(Duration::from_secs(3600));
+        let subscriber_id = uuid::Uuid::new_v4();
+
+        let (raw, hash) = generate_opaque_unsubscribe_token();
+        store.insert(hash.clone(), subscriber_id);
+
+        // First use: valid
+        assert_eq!(
+            store.redeem(&raw),
+            UnsubscribeTokenResult::Valid { subscriber_id }
+        );
+        // Second use: already used
+        assert_eq!(store.redeem(&raw), UnsubscribeTokenResult::AlreadyUsed);
+    }
+
+    #[test]
+    fn opaque_token_expiry_enforced() {
+        let mut store = OpaqueTokenStore::new(Duration::from_millis(1));
+        let subscriber_id = uuid::Uuid::new_v4();
+
+        let (raw, hash) = generate_opaque_unsubscribe_token();
+        store.insert(hash, subscriber_id);
+
+        std::thread::sleep(Duration::from_millis(10));
+
+        assert_eq!(store.redeem(&raw), UnsubscribeTokenResult::InvalidOrExpired);
     }
 
     #[test]
-    fn unsubscribe_token_missing_dot_rejected() {
-        assert_eq!(validate_unsubscribe_token("nodot", "secret"), None);
+    fn unknown_opaque_token_rejected() {
+        let mut store = OpaqueTokenStore::new(Duration::from_secs(3600));
+        let (raw, _) = generate_opaque_unsubscribe_token();
+        // Nothing inserted — token is unknown
+        assert_eq!(store.redeem(&raw), UnsubscribeTokenResult::InvalidOrExpired);
     }
 
     // -------------------------------------------------------------------------