max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+223 insertions,
-7 deletions
| @@ -8,13 +8,17 @@ | |||
| 8 | 8 | ||
| 9 | 9 | /// Insert a new email signup, ignoring duplicates. | |
| 10 | 10 | /// Returns the signup ID (new or existing). | |
| 11 | + | /// | |
| 12 | + | /// Re-signing up clears a previous unsubscribe: submitting the form again is a | |
| 13 | + | /// fresh act of consent, and refusing to honour it would leave someone unable | |
| 14 | + | /// to opt back in through the only interface that exists. | |
| 11 | 15 | #[tracing::instrument(skip_all)] | |
| 12 | 16 | pub(crate) async fn insert_email_signup(pool: &PgPool, email: &str, source: &str) -> Result<Uuid> { | |
| 13 | 17 | let id = sqlx::query_scalar!( | |
| 14 | 18 | r#" | |
| 15 | 19 | INSERT INTO email_signups (email, source) | |
| 16 | 20 | VALUES ($1, $2) | |
| 17 | - | ON CONFLICT (email) DO UPDATE SET email = EXCLUDED.email | |
| 21 | + | ON CONFLICT (email) DO UPDATE SET email = EXCLUDED.email, unsubscribed_at = NULL | |
| 18 | 22 | RETURNING id | |
| 19 | 23 | "#, | |
| 20 | 24 | email, | |
| @@ -25,6 +29,25 @@ | |||
| 25 | 29 | Ok(id) | |
| 26 | 30 | } | |
| 27 | 31 | ||
| 32 | + | /// Mark an address unsubscribed. Returns whether a subscribed row was found. | |
| 33 | + | /// | |
| 34 | + | /// The row is marked, not deleted. A deleted address is one the next import | |
| 35 | + | /// would happily re-add, so the record of the opt-out is what honours it. | |
| 36 | + | /// Idempotent: unsubscribing twice reports `false` the second time and is not | |
| 37 | + | /// an error, which matters because RFC 8058 one-click POSTs get retried. | |
| 38 | + | #[tracing::instrument(skip_all)] | |
| 39 | + | pub(crate) async fn unsubscribe_email_signup(pool: &PgPool, email: &str) -> Result<bool> { | |
| 40 | + | let affected = sqlx::query!( | |
| 41 | + | "UPDATE email_signups SET unsubscribed_at = NOW() \ | |
| 42 | + | WHERE LOWER(email) = LOWER($1) AND unsubscribed_at IS NULL", | |
| 43 | + | email, | |
| 44 | + | ) | |
| 45 | + | .execute(pool) | |
| 46 | + | .await? | |
| 47 | + | .rows_affected(); | |
| 48 | + | Ok(affected > 0) | |
| 49 | + | } | |
| 50 | + | ||
| 28 | 51 | /// Row returned by the admin email signups query. | |
| 29 | 52 | pub(crate) struct DbEmailSignup { | |
| 30 | 53 | pub email: String, | |
| @@ -32,7 +55,12 @@ | |||
| 32 | 55 | pub created_at: chrono::DateTime<chrono::Utc>, | |
| 33 | 56 | } | |
| 34 | 57 | ||
| 35 | - | /// Get email signups ordered by newest first (capped at 500). | |
| 58 | + | /// Get mailable email signups, newest first (capped at 500). | |
| 59 | + | /// | |
| 60 | + | /// Unsubscribed rows are excluded here rather than filtered by the caller. This | |
| 61 | + | /// is the query a send would draw its recipients from, so the opt-out belongs | |
| 62 | + | /// inside it: a caller that has to remember to filter is a caller that will | |
| 63 | + | /// eventually forget. | |
| 36 | 64 | #[tracing::instrument(skip_all)] | |
| 37 | 65 | pub(crate) async fn get_all_email_signups(pool: &PgPool) -> Result<Vec<DbEmailSignup>> { | |
| 38 | 66 | let rows = sqlx::query_as!( | |
| @@ -41,6 +69,7 @@ | |||
| 41 | 69 | SELECT email, source, | |
| 42 | 70 | created_at as "created_at: chrono::DateTime<chrono::Utc>" | |
| 43 | 71 | FROM email_signups | |
| 72 | + | WHERE unsubscribed_at IS NULL | |
| 44 | 73 | ORDER BY created_at DESC | |
| 45 | 74 | LIMIT 500 | |
| 46 | 75 | "#, | |
| @@ -50,12 +79,15 @@ | |||
| 50 | 79 | Ok(rows) | |
| 51 | 80 | } | |
| 52 | 81 | ||
| 53 | - | /// Count total email signups. | |
| 82 | + | /// Count mailable email signups. Excludes unsubscribed, matching | |
| 83 | + | /// [`get_all_email_signups`], so the admin count is the size of the list that | |
| 84 | + | /// would actually receive a send. | |
| 54 | 85 | #[tracing::instrument(skip_all)] | |
| 55 | 86 | pub(crate) async fn count_email_signups(pool: &PgPool) -> Result<i64> { | |
| 56 | - | let count = sqlx::query_scalar!("SELECT COUNT(*) FROM email_signups") | |
| 57 | - | .fetch_one(pool) | |
| 58 | - | .await? | |
| 59 | - | .unwrap_or(0); | |
| 87 | + | let count = | |
| 88 | + | sqlx::query_scalar!("SELECT COUNT(*) FROM email_signups WHERE unsubscribed_at IS NULL") | |
| 89 | + | .fetch_one(pool) | |
| 90 | + | .await? | |
| 91 | + | .unwrap_or(0); | |
| 60 | 92 | Ok(count) | |
| 61 | 93 | } |
| @@ -16,6 +16,9 @@ | |||
| 16 | 16 | Status, | |
| 17 | 17 | MailingList, | |
| 18 | 18 | NotifyTip, | |
| 19 | + | /// The landing page "notify me" list (`email_signups`). Email-keyed only: | |
| 20 | + | /// a signup carries no account, so the user-id form cannot represent it. | |
| 21 | + | Signup, | |
| 19 | 22 | } | |
| 20 | 23 | ||
| 21 | 24 | impl std::fmt::Display for UnsubscribeAction { | |
| @@ -30,6 +33,7 @@ | |||
| 30 | 33 | Self::Status => "status", | |
| 31 | 34 | Self::MailingList => "mailing_list", | |
| 32 | 35 | Self::NotifyTip => "notify_tip", | |
| 36 | + | Self::Signup => "signup", | |
| 33 | 37 | }; | |
| 34 | 38 | f.write_str(s) | |
| 35 | 39 | } | |
| @@ -49,6 +53,7 @@ | |||
| 49 | 53 | "status" => Ok(Self::Status), | |
| 50 | 54 | "mailing_list" => Ok(Self::MailingList), | |
| 51 | 55 | "notify_tip" => Ok(Self::NotifyTip), | |
| 56 | + | "signup" => Ok(Self::Signup), | |
| 52 | 57 | other => Err(format!("invalid UnsubscribeAction: {other}")), | |
| 53 | 58 | } | |
| 54 | 59 | } | |
| @@ -247,6 +252,17 @@ | |||
| 247 | 252 | ) | |
| 248 | 253 | } | |
| 249 | 254 | ||
| 255 | + | /// Unsubscribe URL for the landing "notify me" list. | |
| 256 | + | /// | |
| 257 | + | /// Thin wrapper over [`generate_unsubscribe_url_for_email`] that pins the | |
| 258 | + | /// action and the empty target, so the first thing that sends to this list | |
| 259 | + | /// cannot get either wrong. Pass the result to `send_with_unsubscribe`, which | |
| 260 | + | /// attaches the `List-Unsubscribe` and `List-Unsubscribe-Post: One-Click` | |
| 261 | + | /// headers Gmail and Yahoo require of bulk senders. | |
| 262 | + | pub fn generate_signup_unsubscribe_url(host_url: &str, email: &str, secret: &str) -> String { | |
| 263 | + | generate_unsubscribe_url_for_email(host_url, email, UnsubscribeAction::Signup, "", secret) | |
| 264 | + | } | |
| 265 | + | ||
| 250 | 266 | /// Verify an email-keyed unsubscribe URL signature. | |
| 251 | 267 | pub fn verify_email_unsubscribe_signature( | |
| 252 | 268 | email: &str, |
| @@ -569,3 +569,130 @@ | |||
| 569 | 569 | let after = h.client.get("/?notify=ok").await; | |
| 570 | 570 | assert!(after.text.contains("You're on the list.")); | |
| 571 | 571 | } | |
| 572 | + | ||
| 573 | + | // ── Landing signup unsubscribe ── | |
| 574 | + | // | |
| 575 | + | // Step 1 of wiki [[mnw-mailing-lists]]. The table had insert, admin read and | |
| 576 | + | // count, and no way out. Withdrawal has to be as easy as consent, and consent | |
| 577 | + | // is one form field. | |
| 578 | + | ||
| 579 | + | fn signup_unsub_url(email: &str) -> String { | |
| 580 | + | makenotwork::email::generate_signup_unsubscribe_url( | |
| 581 | + | "", | |
| 582 | + | email, | |
| 583 | + | "test-signing-secret-for-integration-tests", | |
| 584 | + | ) | |
| 585 | + | } | |
| 586 | + | ||
| 587 | + | /// The signed link unsubscribes, and the address stops being mailable. | |
| 588 | + | #[tokio::test] | |
| 589 | + | async fn signup_unsubscribe_link_removes_the_address_from_the_mailable_list() { | |
| 590 | + | let mut h = TestHarness::new().await; | |
| 591 | + | h.client.fetch_csrf_token().await; | |
| 592 | + | h.client | |
| 593 | + | .post_form("/notify", "email=leaving@example.com") | |
| 594 | + | .await; | |
| 595 | + | ||
| 596 | + | let before: i64 = sqlx::query_scalar( | |
| 597 | + | "SELECT COUNT(*) FROM email_signups WHERE email = $1 AND unsubscribed_at IS NULL", | |
| 598 | + | ) | |
| 599 | + | .bind("leaving@example.com") | |
| 600 | + | .fetch_one(&h.db) | |
| 601 | + | .await | |
| 602 | + | .expect("count"); | |
| 603 | + | assert_eq!(before, 1, "signup did not land"); | |
| 604 | + | ||
| 605 | + | let resp = h.client.get(&signup_unsub_url("leaving@example.com")).await; | |
| 606 | + | assert_eq!(resp.status, 200); | |
| 607 | + | ||
| 608 | + | let after: i64 = sqlx::query_scalar( | |
| 609 | + | "SELECT COUNT(*) FROM email_signups WHERE email = $1 AND unsubscribed_at IS NULL", | |
| 610 | + | ) | |
| 611 | + | .bind("leaving@example.com") | |
| 612 | + | .fetch_one(&h.db) | |
| 613 | + | .await | |
| 614 | + | .expect("count"); | |
| 615 | + | assert_eq!(after, 0, "still mailable after unsubscribing"); | |
| 616 | + | ||
| 617 | + | // Marked, not deleted: a deleted address is one the next import re-adds. | |
| 618 | + | let retained: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM email_signups WHERE email = $1") | |
| 619 | + | .bind("leaving@example.com") | |
| 620 | + | .fetch_one(&h.db) | |
| 621 | + | .await | |
| 622 | + | .expect("count"); | |
| 623 | + | assert_eq!(retained, 1, "the opt-out record was thrown away"); | |
| 624 | + | } | |
| 625 | + | ||
| 626 | + | /// RFC 8058: a POST to the same URL unsubscribes with no confirmation step, | |
| 627 | + | /// which is what the List-Unsubscribe-Post header promises Gmail and Yahoo. | |
| 628 | + | /// Retries must not fail, so the second POST answers the same as the first. | |
| 629 | + | #[tokio::test] | |
| 630 | + | async fn signup_unsubscribe_one_click_post_is_idempotent() { | |
| 631 | + | let mut h = TestHarness::new().await; | |
| 632 | + | h.client.fetch_csrf_token().await; | |
| 633 | + | h.client | |
| 634 | + | .post_form("/notify", "email=oneclick@example.com") | |
| 635 | + | .await; | |
| 636 | + | ||
| 637 | + | let url = signup_unsub_url("oneclick@example.com"); | |
| 638 | + | let first = h.client.post_form(&url, "List-Unsubscribe=One-Click").await; | |
| 639 | + | assert_eq!(first.status, 200); | |
| 640 | + | let second = h.client.post_form(&url, "List-Unsubscribe=One-Click").await; | |
| 641 | + | assert_eq!(second.status, 200, "a retried one-click POST must not fail"); | |
| 642 | + | } | |
| 643 | + | ||
| 644 | + | /// A tampered signature does nothing. The address is signed over, so a token | |
| 645 | + | /// for one address cannot unsubscribe another. | |
| 646 | + | #[tokio::test] | |
| 647 | + | async fn signup_unsubscribe_rejects_a_forged_link() { | |
| 648 | + | let mut h = TestHarness::new().await; | |
| 649 | + | h.client.fetch_csrf_token().await; | |
| 650 | + | h.client | |
| 651 | + | .post_form("/notify", "email=victim@example.com") | |
| 652 | + | .await; | |
| 653 | + | ||
| 654 | + | // Take a valid token for one address and point it at another. | |
| 655 | + | let forged = signup_unsub_url("attacker@example.com") | |
| 656 | + | .replace("attacker%40example.com", "victim%40example.com"); | |
| 657 | + | let resp = h.client.get(&forged).await; | |
| 658 | + | assert!( | |
| 659 | + | resp.text.contains("Invalid Link"), | |
| 660 | + | "a forged link was accepted" | |
| 661 | + | ); | |
| 662 | + | ||
| 663 | + | let still_mailable: i64 = sqlx::query_scalar( | |
| 664 | + | "SELECT COUNT(*) FROM email_signups WHERE email = $1 AND unsubscribed_at IS NULL", | |
| 665 | + | ) | |
| 666 | + | .bind("victim@example.com") | |
| 667 | + | .fetch_one(&h.db) | |
| 668 | + | .await | |
| 669 | + | .expect("count"); | |
| 670 | + | assert_eq!(still_mailable, 1, "a forged link unsubscribed someone"); | |
| 671 | + | } | |
| 672 | + | ||
| 673 | + | /// Signing up again after unsubscribing re-subscribes. The form is the only | |
| 674 | + | /// interface, so refusing would leave someone unable to opt back in. | |
| 675 | + | #[tokio::test] | |
| 676 | + | async fn signing_up_again_after_unsubscribing_restores_the_subscription() { | |
| 677 | + | let mut h = TestHarness::new().await; | |
| 678 | + | h.client.fetch_csrf_token().await; | |
| 679 | + | h.client | |
| 680 | + | .post_form("/notify", "email=returning@example.com") | |
| 681 | + | .await; | |
| 682 | + | h.client | |
| 683 | + | .get(&signup_unsub_url("returning@example.com")) | |
| 684 | + | .await; | |
| 685 | + | ||
| 686 | + | h.client | |
| 687 | + | .post_form("/notify", "email=returning@example.com") | |
| 688 | + | .await; | |
| 689 | + | ||
| 690 | + | let mailable: i64 = sqlx::query_scalar( | |
| 691 | + | "SELECT COUNT(*) FROM email_signups WHERE email = $1 AND unsubscribed_at IS NULL", | |
| 692 | + | ) | |
| 693 | + | .bind("returning@example.com") | |
| 694 | + | .fetch_one(&h.db) | |
| 695 | + | .await | |
| 696 | + | .expect("count"); | |
| 697 | + | assert_eq!(mailable, 1, "could not opt back in"); | |
| 698 | + | } |
| @@ -315,6 +315,19 @@ | |||
| 315 | 315 | db::mailing_lists::unsubscribe_by_email(db, list_id, email_addr).await?; | |
| 316 | 316 | Ok("You have been unsubscribed from this mailing list.".to_string()) | |
| 317 | 317 | } | |
| 318 | + | // The landing "notify me" list. `target` is unused: there is one such | |
| 319 | + | // list, so the address alone identifies the subscription. It is still | |
| 320 | + | // signed over, which keeps this token from being replayed as any other | |
| 321 | + | // action for the same address. | |
| 322 | + | // | |
| 323 | + | // An address that was already unsubscribed reports the same message | |
| 324 | + | // rather than an error. The person asked not to be mailed and is not | |
| 325 | + | // being mailed; telling them the link failed would be both untrue and | |
| 326 | + | // alarming, and one-click POSTs get retried. | |
| 327 | + | email::UnsubscribeAction::Signup => { | |
| 328 | + | db::email_signups::unsubscribe_email_signup(db, email_addr).await?; | |
| 329 | + | Ok("You will not receive further updates from Makenotwork.".to_string()) | |
| 330 | + | } | |
| 318 | 331 | _ => Err(AppError::BadRequest( | |
| 319 | 332 | "This unsubscribe link is invalid.".to_string(), | |
| 320 | 333 | )), | |
| @@ -389,5 +402,13 @@ | |||
| 389 | 402 | db::users::disable_notification(db, user_id, "notify_tip").await?; | |
| 390 | 403 | Ok("You will no longer receive email notifications for tips.".to_string()) | |
| 391 | 404 | } | |
| 405 | + | // Signup is keyed on the address, not the account: the landing list | |
| 406 | + | // holds addresses that mostly have no user behind them, and an account | |
| 407 | + | // sharing an address with a signup row is a coincidence rather than a | |
| 408 | + | // link. Honouring a user-keyed token here would unsubscribe by an | |
| 409 | + | // association we never established. | |
| 410 | + | UnsubscribeAction::Signup => Err(AppError::BadRequest( | |
| 411 | + | "This unsubscribe link is invalid.".to_string(), | |
| 412 | + | )), | |
| 392 | 413 | } | |
| 393 | 414 | } |
| @@ -1,0 +1,20 @@ | |||
| 1 | + | -- Landing signups gain an unsubscribe state. | |
| 2 | + | -- | |
| 3 | + | -- The table had insert, admin read and count, and no way out. Under GDPR, | |
| 4 | + | -- withdrawing consent has to be as easy as giving it, and giving it is one form | |
| 5 | + | -- field on the landing page. | |
| 6 | + | -- | |
| 7 | + | -- The row is marked rather than deleted. A deleted address is one we would | |
| 8 | + | -- happily re-add on the next import, so the record of the opt-out IS the thing | |
| 9 | + | -- that honours it. Reads that pick recipients filter on unsubscribed_at IS NULL. | |
| 10 | + | -- | |
| 11 | + | -- This is step 1 of the plan in wiki [[mnw-mailing-lists]] and deliberately does | |
| 12 | + | -- not anticipate the lists/list_subscriptions/consent_events tables from step 2. | |
| 13 | + | -- When those land, this column backfills into a subscription state. | |
| 14 | + | ALTER TABLE email_signups ADD COLUMN IF NOT EXISTS unsubscribed_at TIMESTAMPTZ; | |
| 15 | + | ||
| 16 | + | -- Partial index: every recipient query filters on this, and the unsubscribed | |
| 17 | + | -- rows are the ones we never scan. | |
| 18 | + | CREATE INDEX IF NOT EXISTS idx_email_signups_subscribed | |
| 19 | + | ON email_signups (created_at DESC) | |
| 20 | + | WHERE unsubscribed_at IS NULL; |