Merge pull request #11697 from Turbo87/use-encrypted-tokens

Turbo87 · web-flow · commit e825fd436ddb · 2025-08-23T17:37:57.000+02:00
Use encrypted GitHub OAuth access tokens
diff --git a/crates/crates_io_database/src/models/user.rs b/crates/crates_io_database/src/models/user.rs
@@ -24,7 +24,7 @@ pub struct User {
     #[serde(skip)]
     pub gh_access_token: SecretString,
     #[serde(skip)]
-    pub gh_encrypted_token: Option<Vec<u8>>,
+    pub gh_encrypted_token: Vec<u8>,
     pub account_lock_reason: Option<String>,
     pub account_lock_until: Option<DateTime<Utc>>,
     pub is_admin: bool,
@@ -96,7 +96,7 @@ pub struct NewUser<'a> {
     pub name: Option<&'a str>,
     pub gh_avatar: Option<&'a str>,
     pub gh_access_token: &'a str,
-    pub gh_encrypted_token: Option<&'a [u8]>,
+    pub gh_encrypted_token: &'a [u8],
 }
 
 impl NewUser<'_> {
diff --git a/crates/crates_io_database/src/schema.rs b/crates/crates_io_database/src/schema.rs
@@ -881,7 +881,7 @@ diesel::table! {
         /// Whether or not the user wants to receive notifications when a package they own is published
         publish_notifications -> Bool,
         /// Encrypted GitHub access token
-        gh_encrypted_token -> Nullable<Bytea>,
+        gh_encrypted_token -> Bytea,
     }
 }
 
diff --git a/crates/crates_io_database_dump/src/dump-db.toml b/crates/crates_io_database_dump/src/dump-db.toml
@@ -235,6 +235,7 @@ publish_notifications = "private"
 gh_encrypted_token = "private"
 [users.column_defaults]
 gh_access_token = "''"
+gh_encrypted_token = "''"
 
 [version_downloads]
 dependencies = ["versions"]
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
@@ -24,6 +24,7 @@ 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,6 +68,7 @@ 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-163013_make_gh_encrypted_token_not_null/down.sql b/migrations/2025-07-16-163013_make_gh_encrypted_token_not_null/down.sql
@@ -0,0 +1 @@
+ALTER TABLE users ALTER COLUMN gh_encrypted_token DROP NOT NULL;
diff --git a/migrations/2025-07-16-163013_make_gh_encrypted_token_not_null/up.sql b/migrations/2025-07-16-163013_make_gh_encrypted_token_not_null/up.sql
@@ -0,0 +1 @@
+ALTER TABLE users ALTER COLUMN gh_encrypted_token SET NOT NULL;
diff --git a/src/bin/crates-admin/encrypt_github_tokens.rs b/src/bin/crates-admin/encrypt_github_tokens.rs
diff --git a/src/bin/crates-admin/main.rs b/src/bin/crates-admin/main.rs
@@ -6,7 +6,6 @@ mod default_versions;
 mod delete_crate;
 mod delete_version;
 mod dialoguer;
-mod encrypt_github_tokens;
 mod enqueue_job;
 mod migrate;
 mod populate;
@@ -22,7 +21,6 @@ enum Command {
     BackfillOgImages(backfill_og_images::Opts),
     DeleteCrate(delete_crate::Opts),
     DeleteVersion(delete_version::Opts),
-    EncryptGithubTokens(encrypt_github_tokens::Opts),
     Populate(populate::Opts),
     RenderReadmes(render_readmes::Opts),
     TransferCrates(transfer_crates::Opts),
@@ -53,7 +51,6 @@ async fn main() -> anyhow::Result<()> {
         Command::BackfillOgImages(opts) => backfill_og_images::run(opts).await,
         Command::DeleteCrate(opts) => delete_crate::run(opts).await,
         Command::DeleteVersion(opts) => delete_version::run(opts).await,
-        Command::EncryptGithubTokens(opts) => encrypt_github_tokens::run(opts).await,
         Command::Populate(opts) => populate::run(opts).await,
         Command::RenderReadmes(opts) => render_readmes::run(opts).await,
         Command::TransferCrates(opts) => transfer_crates::run(opts).await,
diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs
@@ -166,7 +166,8 @@ async fn prepare_list(
                 // Only allow crate owners to query pending invitations for their crate.
                 let krate: Crate = Crate::by_name(&crate_name).first(conn).await?;
                 let owners = krate.owners(conn).await?;
-                if Rights::get(user, &*state.github, &owners).await? != Rights::Full {
+                let encryption = &state.config.gh_token_encryption;
+                if Rights::get(user, &*state.github, &owners, encryption).await? != Rights::Full {
                     let detail = "only crate owners can query pending invitations for their crate";
                     return Err(forbidden(detail));
                 }
diff --git a/src/controllers/helpers/authorization.rs b/src/controllers/helpers/authorization.rs
@@ -1,7 +1,6 @@
 use crate::models::{Owner, User};
+use crate::util::gh_token_encryption::GitHubTokenEncryption;
 use crates_io_github::{GitHubClient, GitHubError};
-use oauth2::AccessToken;
-use secrecy::ExposeSecret;
 
 /// Access rights to the crate (publishing and ownership management)
 /// NOTE: The order of these variants matters!
@@ -25,8 +24,11 @@ impl Rights {
         user: &User,
         gh_client: &dyn GitHubClient,
         owners: &[Owner],
+        encryption: &GitHubTokenEncryption,
     ) -> Result<Self, GitHubError> {
-        let token = AccessToken::new(user.gh_access_token.expose_secret().to_string());
+        let token = encryption
+            .decrypt(&user.gh_encrypted_token)
+            .map_err(GitHubError::Other)?;
 
         let mut best = Self::None;
         for owner in owners {
diff --git a/src/controllers/krate/delete.rs b/src/controllers/krate/delete.rs
@@ -72,7 +72,7 @@ pub async fn delete_crate(
     // Check that the user is an owner of the crate (team owners are not allowed to delete crates)
     let user = auth.user();
     let owners = krate.owners(&mut conn).await?;
-    match Rights::get(user, &*app.github, &owners).await? {
+    match Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? {
         Rights::Full => {}
         Rights::Publish => {
             let msg = "team members don't have permission to delete crates";
diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs
@@ -9,6 +9,7 @@ use crate::models::{
     krate::NewOwnerInvite, token::EndpointScope,
 };
 use crate::util::errors::{AppResult, BoxedAppError, bad_request, crate_not_found, custom};
+use crate::util::gh_token_encryption::GitHubTokenEncryption;
 use crate::views::EncodableOwner;
 use crate::{App, app::AppState};
 use crate::{auth::AuthCheck, email::EmailMessage};
@@ -199,7 +200,7 @@ async fn modify_owners(
 
                 let owners = krate.owners(conn).await?;
 
-                match Rights::get(user, &*app.github, &owners).await? {
+                match Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? {
                     Rights::Full => {}
                     // Yes!
                     Rights::Publish => {
@@ -320,7 +321,8 @@ async fn add_owner(
     login: &str,
 ) -> Result<NewOwnerInvite, OwnerAddError> {
     if login.contains(':') {
-        add_team_owner(&*app.github, conn, req_user, krate, login).await
+        let encryption = &app.config.gh_token_encryption;
+        add_team_owner(&*app.github, conn, req_user, krate, login, encryption).await
     } else {
         invite_user_owner(app, conn, req_user, krate, login).await
     }
@@ -363,6 +365,7 @@ async fn add_team_owner(
     req_user: &User,
     krate: &Crate,
     login: &str,
+    encryption: &GitHubTokenEncryption,
 ) -> Result<NewOwnerInvite, OwnerAddError> {
     // github:rust-lang:owners
     let mut chunks = login.split(':');
@@ -381,9 +384,16 @@ async fn add_team_owner(
     })?;
 
     // Always recreate teams to get the most up-to-date GitHub ID
-    let team =
-        create_or_update_github_team(gh_client, conn, &login.to_lowercase(), org, team, req_user)
-            .await?;
+    let team = create_or_update_github_team(
+        gh_client,
+        conn,
+        &login.to_lowercase(),
+        org,
+        team,
+        req_user,
+        encryption,
+    )
+    .await?;
 
     // Teams are added as owners immediately, since the above call ensures
     // the user is a team member.
@@ -408,6 +418,7 @@ pub async fn create_or_update_github_team(
     org_name: &str,
     team_name: &str,
     req_user: &User,
+    encryption: &GitHubTokenEncryption,
 ) -> AppResult<Team> {
     // GET orgs/:org/teams
     // check that `team` is the `slug` in results, and grab its data
@@ -424,7 +435,14 @@ pub async fn create_or_update_github_team(
         )));
     }
 
-    let token = AccessToken::new(req_user.gh_access_token.expose_secret().to_string());
+    let token = encryption
+        .decrypt(&req_user.gh_encrypted_token)
+        .map_err(|err| {
+            custom(
+                StatusCode::INTERNAL_SERVER_ERROR,
+                format!("Failed to decrypt GitHub token: {err}"),
+            )
+        })?;
     let team = gh_client.team_by_name(org_name, team_name, &token).await
         .map_err(|_| {
             bad_request(format_args!(
diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs
@@ -450,7 +450,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
             };
 
             let owners = krate.owners(conn).await?;
-            if Rights::get(user, &*app.github, &owners).await? < Rights::Publish {
+            if Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? < Rights::Publish {
                 return Err(custom(StatusCode::FORBIDDEN, MISSING_RIGHTS_ERROR_MESSAGE));
             }
 
diff --git a/src/controllers/trustpub/github_configs/create/mod.rs b/src/controllers/trustpub/github_configs/create/mod.rs
@@ -3,7 +3,7 @@ use crate::auth::AuthCheck;
 use crate::controllers::krate::load_crate;
 use crate::controllers::trustpub::github_configs::json;
 use crate::email::EmailMessage;
-use crate::util::errors::{AppResult, bad_request, forbidden};
+use crate::util::errors::{AppResult, bad_request, forbidden, server_error};
 use anyhow::Context;
 use axum::Json;
 use crates_io_database::models::OwnerKind;
@@ -17,8 +17,6 @@ use diesel::prelude::*;
 use diesel_async::RunQueryDsl;
 use http::request::Parts;
 use minijinja::context;
-use oauth2::AccessToken;
-use secrecy::ExposeSecret;
 use tracing::warn;
 
 #[cfg(test)]
@@ -77,8 +75,15 @@ pub async fn create_trustpub_github_config(
     // Lookup `repository_owner_id` via GitHub API
 
     let owner = &json_config.repository_owner;
-    let gh_auth = &auth_user.gh_access_token;
-    let gh_auth = AccessToken::new(gh_auth.expose_secret().to_string());
+
+    let encryption = &state.config.gh_token_encryption;
+    let gh_auth = &auth_user.gh_encrypted_token;
+    let gh_auth = encryption.decrypt(gh_auth).map_err(|err| {
+        let login = &auth_user.gh_login;
+        warn!("Failed to decrypt GitHub token for user {login}: {err}");
+        server_error("Internal server error")
+    })?;
+
     let github_user = match state.github.get_user(owner, &gh_auth).await {
         Ok(user) => user,
         Err(GitHubError::NotFound(_)) => Err(bad_request("Unknown GitHub user or organization"))?,
diff --git a/src/controllers/version/docs.rs b/src/controllers/version/docs.rs
@@ -36,7 +36,8 @@ pub async fn rebuild_version_docs(
     // Check that the user is an owner of the crate, or a team member (= publish rights)
     let user = auth.user();
     let owners = krate.owners(&mut conn).await?;
-    if Rights::get(user, &*app.github, &owners).await? < Rights::Publish {
+    let encryption = &app.config.gh_token_encryption;
+    if Rights::get(user, &*app.github, &owners, encryption).await? < Rights::Publish {
         return Err(custom(
             StatusCode::FORBIDDEN,
             "user doesn't have permission to trigger a docs rebuild",
@@ -107,6 +108,7 @@ mod tests {
             .gh_id(111)
             .gh_login("other_user")
             .gh_access_token("token")
+            .gh_encrypted_token(&[])
             .build()
             .insert(&mut conn)
             .await?;
diff --git a/src/controllers/version/update.rs b/src/controllers/version/update.rs
@@ -125,7 +125,8 @@ pub async fn perform_version_yank_update(
 
     let yanked = yanked.unwrap_or(version.yanked);
 
-    if Rights::get(user, &*state.github, &owners).await? < Rights::Publish {
+    let encryption = &state.config.gh_token_encryption;
+    if Rights::get(user, &*state.github, &owners, encryption).await? < Rights::Publish {
         if user.is_admin {
             let action = if yanked { "yanking" } else { "unyanking" };
             warn!(
diff --git a/src/index.rs b/src/index.rs
@@ -157,6 +157,7 @@ mod tests {
                 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)
             .get_result::<i32>(&mut conn)
diff --git a/src/rate_limiter.rs b/src/rate_limiter.rs
@@ -707,6 +707,7 @@ mod tests {
             .gh_id(0)
             .gh_login(gh_login)
             .gh_access_token("some random token")
+            .gh_encrypted_token(&[])
             .build()
             .insert(conn)
             .await
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
diff --git a/src/tests/owners.rs b/src/tests/owners.rs
diff --git a/src/tests/routes/users/read.rs b/src/tests/routes/users/read.rs
diff --git a/src/tests/worker/sync_admins.rs b/src/tests/worker/sync_admins.rs
diff --git a/src/typosquat/test_util.rs b/src/typosquat/test_util.rs
diff --git a/src/worker/jobs/downloads/update_metadata.rs b/src/worker/jobs/downloads/update_metadata.rs
diff --git a/src/worker/jobs/expiry_notification.rs b/src/worker/jobs/expiry_notification.rs

Original file line number	Diff line number	Diff line change
`@@ -881,7 +881,7 @@ diesel::table! {`
`881`	`881`	`/// Whether or not the user wants to receive notifications when a package they own is published`
`882`	`882`	`publish_notifications -> Bool,`
`883`	`883`	`/// Encrypted GitHub access token`
`884`		`- gh_encrypted_token -> Nullable<Bytea>,`
	`884`	`+ gh_encrypted_token -> Bytea,`
`885`	`885`	`}`
`886`	`886`	`}`
`887`	`887`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+ALTER TABLE users ALTER COLUMN gh_encrypted_token DROP NOT NULL;`