LukeMathWalker / zero-to-production

Code for "Zero To Production In Rust", a book on API development using Rust.
https://www.zero2prod.com
Apache License 2.0
5.78k stars 500 forks source link

Subscriptions confirm is a GET request that should not change the state but be read-only #187

Open Bodobolero opened 1 year ago

Bodobolero commented 1 year ago

See discussion on https://softwareengineering.stackexchange.com/questions/422507/email-confirmation-links-must-be-get-but-not-safe

A better way to handle the email confirmation link instead of a GET request as in your book

 .route("/subscriptions/confirm", web::get().to(confirm))

would be to:

The solution is that the GET request itself doesn't change the state, it just returns a custom form, to be submitted with POST. In an abstract description, the GET request is reading a resource which reflects the current status of a particular transaction; the form is a convenient representation of that transaction with hypermedia to transition to a new status. In a more concrete set of steps:

  1. Generate a random URL associated with the pending registration.
  2. Include that URL in the e-mail to the user.
  3. When that URL is requested, check if the registration is still pending. If not, skip to (6).
  4. Show an HTML form with a button for the user to complete the confirmation process. The target of the form can be the same URL, but with a method of POST.
  5. When the POST request for the URL is received, complete the registration process.
  6. Show the user a "thank you, your address is now confirmed" page.

Since many readers may re-implement the confirmation logic following your book those readers run the risk that their confirmation link is already invoked by an email security scanner or other software checking the mail in the user's inbox without their approval.

LukeMathWalker commented 1 year ago

This is a very good point, thanks for bringing it up.

I'm not sure I have the bandwidth to redesign this part of the book (ahead of a proper 2nd edition), but I can definitely include a footnote warning about this issue in a next revision.