crates.io icon indicating copy to clipboard operation
crates.io copied to clipboard

Send API token expiry notification emails

Open 0xPoe opened this issue 1 year ago • 5 comments

ref https://github.com/rust-lang/crates.io/issues/8154

Description

This PR adds a new background job to send the expiry notification for tokens that will expire within 3 days.

As https://github.com/rust-lang/crates.io/issues/6664#issuecomment-1952208242 states, such emails are seen as a requirement before we can change the default expiration setting on the API token creation page.

Currently, GitHub only sends notifications twice - one after 6 days and another after 12 hours, and after it has expired.

Perhaps we should consider adopting a similar approach, although it may be too aggressive. So I just picked 3 days as the default value. It should be easy to extend the current design to send multiple emails by reusing 'expiry_notification_at' multiple times.

Explanation of Changes

  1. It added a new field expiry_notification_at to the api_tokens table.
  2. It added a new background job called CheckAboutToExpireToken that will scan all the expiry tokens within 3 days and send an email to the users in batches. Currently, the batch size is 100.
  3. It added an integration test to check we sent the right email.

0xPoe avatar Mar 12 '24 12:03 0xPoe

Codecov Report

Attention: Patch coverage is 92.63804% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 88.53%. Comparing base (153a4b5) to head (394f1af).

:exclamation: Current head 394f1af differs from pull request most recent head cb19848. Consider uploading reports for the commit cb19848 to get more accurate results

Files Patch % Lines
src/worker/jobs/expiry_notification.rs 93.08% 11 Missing :warning:
src/admin/enqueue_job.rs 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8290      +/-   ##
==========================================
+ Coverage   88.45%   88.53%   +0.07%     
==========================================
  Files         275      275              
  Lines       27255    27101     -154     
==========================================
- Hits        24108    23993     -115     
+ Misses       3147     3108      -39     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 27 '24 14:03 codecov[bot]

Tested locally:

  1. Start the crates.io in the docker
crates.io on  rustin-patch-expiry_notification_job [$!] via 🐳 orbstack is 📦 v0.0.0 via  v20.12.0 via 🦀 v1.77.0 
❯ docker ps
CONTAINER ID   IMAGE               COMMAND                  CREATED          STATUS          PORTS                                       NAMES
ea2f555e8f9f   cratesio-frontend   "pnpm start"             40 minutes ago   Up 15 minutes   0.0.0.0:4200->4200/tcp, :::4200->4200/tcp   cratesio-frontend-1
a3f3d61c8730   c944ce643ba1        "cargo run --bin bac…"   40 minutes ago   Up 2 minutes                                                cratesio-worker-1
d0dc3fabda26   cratesio-backend    "/app/docker_entrypo…"   40 minutes ago   Up 14 minutes   0.0.0.0:8888->8888/tcp, :::8888->8888/tcp   cratesio-backend-1
13a8ba48f41e   postgres:13         "docker-entrypoint.s…"   40 minutes ago   Up 40 minutes   127.0.0.1:5432->5432/tcp                    cratesio-postgres-1
  1. Create a new token which will expire within one day image

  2. Trigger a background job: DATABASE_URL=postgresql://postgres:password@localhost:5432/cargo_registry ./target/debug/crates-admin enqueue-job check_about_to_expire_token

  3. Check the email from the worker's tmp dir

Message-ID: <[email protected]>
To: [email protected]
From: [email protected]
Subject: Your token is about to expire
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Fri, 05 Apr 2024 08:26:31 +0000

Hi @hi-rustin,

We noticed your token "test_token" will expire in about 3 days.

If this token is still needed, visit https://crates.io/settings/tokens/new =
to generate a new one.

Thanks,
The crates.io team
  1. trigger it again: DATABASE_URL=postgresql://postgres:password@localhost:5432/cargo_registry ./target/debug/crates-admin enqueue-job check_about_to_expire_token
  2. Check no more new emails sent by the worker

0xPoe avatar Apr 05 '24 08:04 0xPoe

A couple of comments, but overall this looks good to me! 👍

eth3lbert avatar Apr 07 '24 10:04 eth3lbert

I also noticed that since we don't have an index on expired_at, I'm not sure how longfind_tokens_expiring_within_days will take. We might need to investigate this further in production.

eth3lbert avatar Apr 07 '24 10:04 eth3lbert

:umbrella: The latest upstream changes (presumably #8484) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Apr 25 '24 12:04 bors

Tested locally:

Check the steps at: https://github.com/rust-lang/crates.io/pull/8290#issuecomment-2039243197

root@e044d8a84f05:/tmp# tail bab14f97-cf75-4b66-a90d-568fca969b2c.eml 

Hi hi-rustin,

We noticed your token "Test" will expire on 2024-05-08T13:42:47Z.

If this token is still needed, visit https://crates.io/settings/tokens/new =
to generate a new one.

Thanks,
The crates.io team

0xPoe avatar May 07 '24 13:05 0xPoe