database: Remove obsolete users.gh_access_token column (#11789)

Turbo87 · web-flow · commit f3d3bf03e0d7 · 2025-09-04T22:42:21.000+02:00
diff --git a/crates/crates_io_database/src/models/user.rs b/crates/crates_io_database/src/models/user.rs
@@ -5,7 +5,6 @@ use diesel::prelude::*;
 use diesel::sql_types::Integer;
 use diesel::upsert::excluded;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
-use secrecy::SecretString;
 use serde::Serialize;
 
 use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind};
@@ -20,9 +19,6 @@ pub struct User {
     pub gh_id: i32,
     pub gh_login: String,
     pub gh_avatar: Option<String>,
-    #[diesel(deserialize_as = String)]
-    #[serde(skip)]
-    pub gh_access_token: SecretString,
     #[serde(skip)]
     pub gh_encrypted_token: Vec<u8>,
     pub account_lock_reason: Option<String>,
@@ -95,7 +91,6 @@ pub struct NewUser<'a> {
     pub gh_login: &'a str,
     pub name: Option<&'a str>,
     pub gh_avatar: Option<&'a str>,
-    pub gh_access_token: &'a str,
     pub gh_encrypted_token: &'a [u8],
 }
 
@@ -127,7 +122,6 @@ impl NewUser<'_> {
                 users::gh_login.eq(excluded(users::gh_login)),
                 users::name.eq(excluded(users::name)),
                 users::gh_avatar.eq(excluded(users::gh_avatar)),
-                users::gh_access_token.eq(excluded(users::gh_access_token)),
                 users::gh_encrypted_token.eq(excluded(users::gh_encrypted_token)),
             ))
             .returning(User::as_returning())
diff --git a/crates/crates_io_database/src/schema.rs b/crates/crates_io_database/src/schema.rs
@@ -842,12 +842,6 @@ diesel::table! {
         ///
         /// (Automatically generated by Diesel.)
         id -> Int4,
-        /// The `gh_access_token` column of the `users` table.
-        ///
-        /// Its SQL type is `Varchar`.
-        ///
-        /// (Automatically generated by Diesel.)
-        gh_access_token -> Varchar,
         /// The `gh_login` column of the `users` table.
         ///
         /// Its SQL type is `Varchar`.
diff --git a/crates/crates_io_database_dump/src/dump-db.toml b/crates/crates_io_database_dump/src/dump-db.toml
@@ -228,7 +228,6 @@ id in (
 )"""
 [users.columns]
 id = "public"
-gh_access_token = "private"
 gh_login = "public"
 name = "public"
 gh_avatar = "public"
@@ -239,7 +238,6 @@ is_admin = "private"
 publish_notifications = "private"
 gh_encrypted_token = "private"
 [users.column_defaults]
-gh_access_token = "''"
 gh_encrypted_token = "''"
 
 [version_downloads]
diff --git a/crates/crates_io_database_dump/src/snapshots/crates_io_database_dump__tests__sql_scripts@import.sql.snap b/crates/crates_io_database_dump/src/snapshots/crates_io_database_dump__tests__sql_scripts@import.sql.snap
@@ -23,7 +23,6 @@ BEGIN;
 
     -- Set defaults for non-nullable columns not included in the dump.
 
-    ALTER TABLE "users" ALTER COLUMN "gh_access_token" SET DEFAULT '';
     ALTER TABLE "users" ALTER COLUMN "gh_encrypted_token" SET DEFAULT '';
 
     -- Truncate all tables.
@@ -67,7 +66,6 @@ BEGIN;
 
     -- Drop the defaults again.
 
-    ALTER TABLE "users" ALTER COLUMN "gh_access_token" DROP DEFAULT;
     ALTER TABLE "users" ALTER COLUMN "gh_encrypted_token" DROP DEFAULT;
 
     -- Reenable triggers on each table.
diff --git a/migrations/2025-07-16-170846_drop_gh_access_token/down.sql b/migrations/2025-07-16-170846_drop_gh_access_token/down.sql
@@ -0,0 +1 @@
+ALTER TABLE users ADD COLUMN gh_access_token VARCHAR NOT NULL DEFAULT '';
diff --git a/migrations/2025-07-16-170846_drop_gh_access_token/up.sql b/migrations/2025-07-16-170846_drop_gh_access_token/up.sql
@@ -0,0 +1 @@
+ALTER TABLE users DROP COLUMN gh_access_token;
diff --git a/src/controllers/session.rs b/src/controllers/session.rs
@@ -125,14 +125,7 @@ pub async fn authorize_session(
     let ghuser = app.github.current_user(token).await?;
 
     let mut conn = app.db_write().await?;
-    let user = save_user_to_database(
-        &ghuser,
-        token.secret(),
-        &encrypted_token,
-        &app.emails,
-        &mut conn,
-    )
-    .await?;
+    let user = save_user_to_database(&ghuser, &encrypted_token, &app.emails, &mut conn).await?;
 
     // Log in by setting a cookie and the middleware authentication
     session.insert("user_id".to_string(), user.id.to_string());
@@ -142,7 +135,6 @@ pub async fn authorize_session(
 
 pub async fn save_user_to_database(
     user: &GitHubUser,
-    access_token: &str,
     encrypted_token: &[u8],
     emails: &Emails,
     conn: &mut AsyncPgConnection,
@@ -152,7 +144,6 @@ pub async fn save_user_to_database(
         .gh_login(&user.login)
         .maybe_name(user.name.as_deref())
         .maybe_gh_avatar(user.avatar_url.as_deref())
-        .gh_access_token(access_token)
         .gh_encrypted_token(encrypted_token)
         .build();
 
@@ -259,8 +250,7 @@ mod tests {
             avatar_url: None,
         };
 
-        let result =
-            save_user_to_database(&gh_user, "arbitrary_token", &[], &emails, &mut conn).await;
+        let result = save_user_to_database(&gh_user, &[], &emails, &mut conn).await;
 
         assert!(
             result.is_ok(),
diff --git a/src/controllers/version/docs.rs b/src/controllers/version/docs.rs
@@ -107,7 +107,6 @@ mod tests {
         let other_user = NewUser::builder()
             .gh_id(111)
             .gh_login("other_user")
-            .gh_access_token("token")
             .gh_encrypted_token(&[])
             .build()
             .insert(&mut conn)
diff --git a/src/index.rs b/src/index.rs
@@ -156,7 +156,6 @@ mod tests {
                 users::name.eq("user1"),
                 users::gh_login.eq("user1"),
                 users::gh_id.eq(42),
-                users::gh_access_token.eq("some random token"),
                 users::gh_encrypted_token.eq(&[]),
             ))
             .returning(users::id)
diff --git a/src/rate_limiter.rs b/src/rate_limiter.rs
@@ -706,7 +706,6 @@ mod tests {
         NewUser::builder()
             .gh_id(0)
             .gh_login(gh_login)
-            .gh_access_token("some random token")
             .gh_encrypted_token(&[])
             .build()
             .insert(conn)
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -103,7 +103,6 @@ fn new_user(login: &str) -> NewUser<'_> {
     NewUser::builder()
         .gh_id(next_gh_id())
         .gh_login(login)
-        .gh_access_token("some random token")
         .gh_encrypted_token(&ENCRYPTED_TOKEN)
         .build()
 }
diff --git a/src/tests/owners.rs b/src/tests/owners.rs
@@ -820,7 +820,6 @@ async fn inactive_users_dont_get_invitations() {
     NewUser::builder()
         .gh_id(-1)
         .gh_login(invited_gh_login)
-        .gh_access_token("some random token")
         .gh_encrypted_token(&[])
         .build()
         .insert(&mut conn)
diff --git a/src/tests/routes/users/read.rs b/src/tests/routes/users/read.rs
@@ -41,15 +41,13 @@ async fn show_latest_user_case_insensitively() {
         .gh_id(1)
         .gh_login("foobar")
         .name("I was first then deleted my github account")
-        .gh_access_token("bar")
         .gh_encrypted_token(&[])
         .build();
 
     let user2 = NewUser::builder()
         .gh_id(2)
         .gh_login("FOOBAR")
         .name("I was second, I took the foobar username on github")
-        .gh_access_token("bar")
         .gh_encrypted_token(&[])
         .build();
 
diff --git a/src/tests/user.rs b/src/tests/user.rs
@@ -3,14 +3,14 @@ use crate::models::{ApiToken, Email, User};
 use crate::tests::TestApp;
 use crate::tests::util::github::next_gh_id;
 use crate::tests::util::{MockCookieUser, RequestHelper};
+use crate::util::gh_token_encryption::GitHubTokenEncryption;
 use crate::util::token::HashedToken;
 use chrono::{DateTime, Utc};
 use claims::assert_ok;
 use crates_io_github::GitHubUser;
 use diesel::prelude::*;
 use diesel_async::RunQueryDsl;
 use insta::assert_snapshot;
-use secrecy::ExposeSecret;
 use serde_json::json;
 
 impl crate::tests::util::MockCookieUser {
@@ -30,6 +30,8 @@ async fn updating_existing_user_doesnt_change_api_token() -> anyhow::Result<()>
     let gh_id = user.as_model().gh_id;
     let token = token.plaintext();
 
+    let encryption = GitHubTokenEncryption::for_testing();
+
     // Reuse gh_id but use new gh_login and gh_access_token
     let gh_user = GitHubUser {
         id: gh_id,
@@ -38,15 +40,17 @@ async fn updating_existing_user_doesnt_change_api_token() -> anyhow::Result<()>
         email: None,
         avatar_url: None,
     };
-    assert_ok!(session::save_user_to_database(&gh_user, "bar_token", &[], emails, &mut conn).await);
+    let encrypted_token = encryption.encrypt("bar_token")?;
+    assert_ok!(session::save_user_to_database(&gh_user, &encrypted_token, emails, &mut conn).await);
 
     // Use the original API token to find the now updated user
     let hashed_token = assert_ok!(HashedToken::parse(token));
     let api_token = assert_ok!(ApiToken::find_by_api_token(&mut conn, &hashed_token).await);
     let user = assert_ok!(User::find(&mut conn, api_token.user_id).await);
 
     assert_eq!(user.gh_login, "bar");
-    assert_eq!(user.gh_access_token.expose_secret(), "bar_token");
+    let decrypted_token = encryption.decrypt(&user.gh_encrypted_token)?;
+    assert_eq!(decrypted_token.secret(), "bar_token");
 
     Ok(())
 }
@@ -79,8 +83,7 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> {
         avatar_url: None,
     };
 
-    let u = session::save_user_to_database(&gh_user, "some random token", &[], emails, &mut conn)
-        .await?;
+    let u = session::save_user_to_database(&gh_user, &[], emails, &mut conn).await?;
 
     let user_without_github_email = MockCookieUser::new(&app, u);
 
@@ -103,8 +106,7 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> {
         avatar_url: None,
     };
 
-    let u = session::save_user_to_database(&gh_user, "some random token", &[], emails, &mut conn)
-        .await?;
+    let u = session::save_user_to_database(&gh_user, &[], emails, &mut conn).await?;
 
     let again_user_without_github_email = MockCookieUser::new(&app, u);
 
@@ -145,8 +147,7 @@ async fn github_with_email_does_not_overwrite_email() -> anyhow::Result<()> {
         avatar_url: None,
     };
 
-    let u = session::save_user_to_database(&gh_user, "some random token", &[], &emails, &mut conn)
-        .await?;
+    let u = session::save_user_to_database(&gh_user, &[], &emails, &mut conn).await?;
 
     let user_with_different_email_in_github = MockCookieUser::new(&app, u);
 
@@ -202,8 +203,7 @@ async fn test_confirm_user_email() -> anyhow::Result<()> {
         avatar_url: None,
     };
 
-    let u = session::save_user_to_database(&gh_user, "some random token", &[], emails, &mut conn)
-        .await?;
+    let u = session::save_user_to_database(&gh_user, &[], emails, &mut conn).await?;
 
     let user = MockCookieUser::new(&app, u);
     let user_model = user.as_model();
@@ -248,8 +248,7 @@ async fn test_existing_user_email() -> anyhow::Result<()> {
         avatar_url: None,
     };
 
-    let u = session::save_user_to_database(&gh_user, "some random token", &[], emails, &mut conn)
-        .await?;
+    let u = session::save_user_to_database(&gh_user, &[], emails, &mut conn).await?;
 
     update(Email::belonging_to(&u))
         // Users created before we added verification will have
diff --git a/src/tests/worker/sync_admins.rs b/src/tests/worker/sync_admins.rs
@@ -78,7 +78,6 @@ async fn create_user(
             users::name.eq(name),
             users::gh_login.eq(name),
             users::gh_id.eq(gh_id),
-            users::gh_access_token.eq("some random token"),
             users::gh_encrypted_token.eq(&[]),
             users::is_admin.eq(is_admin),
         ))
diff --git a/src/typosquat/test_util.rs b/src/typosquat/test_util.rs
@@ -41,7 +41,6 @@ pub mod faker {
         NewUser::builder()
             .gh_id(next_gh_id())
             .gh_login(login)
-            .gh_access_token("token")
             .gh_encrypted_token(&[])
             .build()
             .insert(conn)
diff --git a/src/worker/jobs/downloads/update_metadata.rs b/src/worker/jobs/downloads/update_metadata.rs
@@ -117,7 +117,6 @@ mod tests {
         NewUser::builder()
             .gh_id(2)
             .gh_login("login")
-            .gh_access_token("access_token")
             .gh_encrypted_token(&[])
             .build()
             .insert(conn)
diff --git a/src/worker/jobs/expiry_notification.rs b/src/worker/jobs/expiry_notification.rs
@@ -158,7 +158,6 @@ mod tests {
         let user = NewUser::builder()
             .gh_id(0)
             .gh_login("a")
-            .gh_access_token("token")
             .gh_encrypted_token(&[])
             .build()
             .insert(&mut conn)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+ALTER TABLE users ADD COLUMN gh_access_token VARCHAR NOT NULL DEFAULT '';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+ALTER TABLE users DROP COLUMN gh_access_token;`