Skip to content

Commit 028489b

Browse files
committed
Use encrypted GitHub OAuth access tokens
1 parent 78ec64d commit 028489b

File tree

9 files changed

+38
-18
lines changed

9 files changed

+38
-18
lines changed

src/controllers/crate_owner_invitation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ async fn prepare_list(
166166
// Only allow crate owners to query pending invitations for their crate.
167167
let krate: Crate = Crate::by_name(&crate_name).first(conn).await?;
168168
let owners = krate.owners(conn).await?;
169-
if Rights::get(user, &*state.github, &owners).await? != Rights::Full {
169+
if Rights::get(user, &*state.github, &owners, &state.config.gh_token_encryption).await? != Rights::Full {
170170
let detail = "only crate owners can query pending invitations for their crate";
171171
return Err(forbidden(detail));
172172
}

src/controllers/helpers/authorization.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use crate::models::{Owner, User};
2+
use crate::util::gh_token_encryption::GitHubTokenEncryption;
23
use crates_io_github::{GitHubClient, GitHubError};
3-
use oauth2::AccessToken;
4-
use secrecy::ExposeSecret;
54

65
/// Access rights to the crate (publishing and ownership management)
76
/// NOTE: The order of these variants matters!
@@ -25,8 +24,11 @@ impl Rights {
2524
user: &User,
2625
gh_client: &dyn GitHubClient,
2726
owners: &[Owner],
27+
encryption: &GitHubTokenEncryption,
2828
) -> Result<Self, GitHubError> {
29-
let token = AccessToken::new(user.gh_access_token.expose_secret().to_string());
29+
let token = encryption
30+
.decrypt(&user.gh_encrypted_token)
31+
.map_err(GitHubError::Other)?;
3032

3133
let mut best = Self::None;
3234
for owner in owners {

src/controllers/krate/delete.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ pub async fn delete_crate(
7272
// Check that the user is an owner of the crate (team owners are not allowed to delete crates)
7373
let user = auth.user();
7474
let owners = krate.owners(&mut conn).await?;
75-
match Rights::get(user, &*app.github, &owners).await? {
75+
match Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? {
7676
Rights::Full => {}
7777
Rights::Publish => {
7878
let msg = "team members don't have permission to delete crates";

src/controllers/krate/owners.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::models::{
99
krate::NewOwnerInvite, token::EndpointScope,
1010
};
1111
use crate::util::errors::{AppResult, BoxedAppError, bad_request, crate_not_found, custom};
12+
use crate::util::gh_token_encryption::GitHubTokenEncryption;
1213
use crate::views::EncodableOwner;
1314
use crate::{App, app::AppState};
1415
use crate::{auth::AuthCheck, email::EmailMessage};
@@ -199,7 +200,7 @@ async fn modify_owners(
199200

200201
let owners = krate.owners(conn).await?;
201202

202-
match Rights::get(user, &*app.github, &owners).await? {
203+
match Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? {
203204
Rights::Full => {}
204205
// Yes!
205206
Rights::Publish => {
@@ -320,7 +321,7 @@ async fn add_owner(
320321
login: &str,
321322
) -> Result<NewOwnerInvite, OwnerAddError> {
322323
if login.contains(':') {
323-
add_team_owner(&*app.github, conn, req_user, krate, login).await
324+
add_team_owner(&*app.github, conn, req_user, krate, login, &app.config.gh_token_encryption).await
324325
} else {
325326
invite_user_owner(app, conn, req_user, krate, login).await
326327
}
@@ -363,6 +364,7 @@ async fn add_team_owner(
363364
req_user: &User,
364365
krate: &Crate,
365366
login: &str,
367+
encryption: &GitHubTokenEncryption,
366368
) -> Result<NewOwnerInvite, OwnerAddError> {
367369
// github:rust-lang:owners
368370
let mut chunks = login.split(':');
@@ -382,7 +384,7 @@ async fn add_team_owner(
382384

383385
// Always recreate teams to get the most up-to-date GitHub ID
384386
let team =
385-
create_or_update_github_team(gh_client, conn, &login.to_lowercase(), org, team, req_user)
387+
create_or_update_github_team(gh_client, conn, &login.to_lowercase(), org, team, req_user, encryption)
386388
.await?;
387389

388390
// Teams are added as owners immediately, since the above call ensures
@@ -408,6 +410,7 @@ pub async fn create_or_update_github_team(
408410
org_name: &str,
409411
team_name: &str,
410412
req_user: &User,
413+
encryption: &GitHubTokenEncryption,
411414
) -> AppResult<Team> {
412415
// GET orgs/:org/teams
413416
// check that `team` is the `slug` in results, and grab its data
@@ -424,7 +427,9 @@ pub async fn create_or_update_github_team(
424427
)));
425428
}
426429

427-
let token = AccessToken::new(req_user.gh_access_token.expose_secret().to_string());
430+
let token = encryption
431+
.decrypt(&req_user.gh_encrypted_token)
432+
.map_err(|err| custom(StatusCode::INTERNAL_SERVER_ERROR, format!("Failed to decrypt GitHub token: {err}")))?;
428433
let team = gh_client.team_by_name(org_name, team_name, &token).await
429434
.map_err(|_| {
430435
bad_request(format_args!(

src/controllers/krate/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
450450
};
451451

452452
let owners = krate.owners(conn).await?;
453-
if Rights::get(user, &*app.github, &owners).await? < Rights::Publish {
453+
if Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? < Rights::Publish {
454454
return Err(custom(StatusCode::FORBIDDEN, MISSING_RIGHTS_ERROR_MESSAGE));
455455
}
456456

src/controllers/trustpub/github_configs/create/mod.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::auth::AuthCheck;
33
use crate::controllers::krate::load_crate;
44
use crate::controllers::trustpub::github_configs::json;
55
use crate::email::EmailMessage;
6-
use crate::util::errors::{AppResult, bad_request, forbidden};
6+
use crate::util::errors::{AppResult, bad_request, forbidden, server_error};
77
use anyhow::Context;
88
use axum::Json;
99
use crates_io_database::models::OwnerKind;
@@ -17,8 +17,6 @@ use diesel::prelude::*;
1717
use diesel_async::RunQueryDsl;
1818
use http::request::Parts;
1919
use minijinja::context;
20-
use oauth2::AccessToken;
21-
use secrecy::ExposeSecret;
2220
use tracing::warn;
2321

2422
#[cfg(test)]
@@ -77,8 +75,15 @@ pub async fn create_trustpub_github_config(
7775
// Lookup `repository_owner_id` via GitHub API
7876

7977
let owner = &json_config.repository_owner;
80-
let gh_auth = &auth_user.gh_access_token;
81-
let gh_auth = AccessToken::new(gh_auth.expose_secret().to_string());
78+
79+
let encryption = &state.config.gh_token_encryption;
80+
let gh_auth = &auth_user.gh_encrypted_token;
81+
let gh_auth = encryption.decrypt(gh_auth).map_err(|err| {
82+
let login = &auth_user.gh_login;
83+
warn!("Failed to decrypt GitHub token for user {login}: {err}");
84+
server_error("Internal server error")
85+
})?;
86+
8287
let github_user = match state.github.get_user(owner, &gh_auth).await {
8388
Ok(user) => user,
8489
Err(GitHubError::NotFound(_)) => Err(bad_request("Unknown GitHub user or organization"))?,

src/controllers/version/docs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub async fn rebuild_version_docs(
3636
// Check that the user is an owner of the crate, or a team member (= publish rights)
3737
let user = auth.user();
3838
let owners = krate.owners(&mut conn).await?;
39-
if Rights::get(user, &*app.github, &owners).await? < Rights::Publish {
39+
if Rights::get(user, &*app.github, &owners, &app.config.gh_token_encryption).await? < Rights::Publish {
4040
return Err(custom(
4141
StatusCode::FORBIDDEN,
4242
"user doesn't have permission to trigger a docs rebuild",

src/controllers/version/update.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub async fn perform_version_yank_update(
125125

126126
let yanked = yanked.unwrap_or(version.yanked);
127127

128-
if Rights::get(user, &*state.github, &owners).await? < Rights::Publish {
128+
if Rights::get(user, &*state.github, &owners, &state.config.gh_token_encryption).await? < Rights::Publish {
129129
if user.is_admin {
130130
let action = if yanked { "yanking" } else { "unyanking" };
131131
warn!(

src/tests/mod.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ use crate::views::{
66
};
77

88
use crate::tests::util::github::next_gh_id;
9+
use crate::util::gh_token_encryption::GitHubTokenEncryption;
910
use diesel::prelude::*;
1011
use diesel_async::AsyncPgConnection;
1112
use serde::{Deserialize, Serialize};
13+
use std::sync::LazyLock;
1214

1315
mod account_lock;
1416
mod authentication;
@@ -92,11 +94,17 @@ pub struct OwnerResp {
9294
}
9395

9496
fn new_user(login: &str) -> NewUser<'_> {
97+
static ENCRYPTED_TOKEN: LazyLock<Vec<u8>> = LazyLock::new(|| {
98+
GitHubTokenEncryption::for_testing()
99+
.encrypt("some random token")
100+
.unwrap()
101+
});
102+
95103
NewUser::builder()
96104
.gh_id(next_gh_id())
97105
.gh_login(login)
98106
.gh_access_token("some random token")
99-
.gh_encrypted_token(&[])
107+
.gh_encrypted_token(&ENCRYPTED_TOKEN)
100108
.build()
101109
}
102110

0 commit comments

Comments
 (0)