From 7bf7c457c1de6b02a99a894a41290d4d12977184 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:14:35 +0200 Subject: [PATCH 1/7] Extract `AuthHeader` for `Authorization` header parsing --- src/auth.rs | 59 ++++++++++++++++++- src/controllers/trustpub/tokens/revoke/mod.rs | 19 ++---- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index a3eb7d999eb..0c4f8300cd2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -4,14 +4,69 @@ use crate::middleware::log_request::RequestLogExt; use crate::models::token::{CrateScope, EndpointScope}; use crate::models::{ApiToken, User}; use crate::util::errors::{ - AppResult, InsecurelyGeneratedTokenRevoked, account_locked, forbidden, internal, + AppResult, BoxedAppError, InsecurelyGeneratedTokenRevoked, account_locked, custom, forbidden, + internal, }; use crate::util::token::HashedToken; +use axum::extract::{FromRequestParts, OptionalFromRequestParts}; use chrono::Utc; use crates_io_session::SessionExtension; use diesel_async::AsyncPgConnection; -use http::header; use http::request::Parts; +use http::{StatusCode, header}; +use secrecy::SecretString; + +pub struct AuthHeader(SecretString); + +impl AuthHeader { + pub async fn optional_from_request_parts(parts: &Parts) -> Result, BoxedAppError> { + let Some(auth_header) = parts.headers.get(header::AUTHORIZATION) else { + return Ok(None); + }; + + let auth_header = auth_header.to_str().map_err(|_| { + let message = "Invalid authorization header"; + custom(StatusCode::UNAUTHORIZED, message) + })?; + + let (scheme, token) = auth_header.split_once(' ').unwrap_or(("", auth_header)); + if !scheme.eq_ignore_ascii_case("Bearer") { + let message = "Invalid authorization header"; + return Err(custom(StatusCode::UNAUTHORIZED, message)); + } + + let token = SecretString::from(token.trim_ascii()); + Ok(Some(AuthHeader(token))) + } + + pub async fn from_request_parts(parts: &Parts) -> Result { + let auth = Self::optional_from_request_parts(parts).await?; + auth.ok_or_else(|| { + let message = "Missing authorization header"; + custom(StatusCode::UNAUTHORIZED, message) + }) + } + + pub fn token(&self) -> &SecretString { + &self.0 + } +} + +impl OptionalFromRequestParts for AuthHeader { + type Rejection = BoxedAppError; + + async fn from_request_parts(parts: &mut Parts, _: &S) -> Result, Self::Rejection> { + Self::optional_from_request_parts(parts).await + } +} + +impl FromRequestParts for AuthHeader { + type Rejection = BoxedAppError; + + async fn from_request_parts(parts: &mut Parts, _: &S) -> Result { + Self::from_request_parts(parts).await + } +} #[derive(Debug, Clone)] pub struct AuthCheck { diff --git a/src/controllers/trustpub/tokens/revoke/mod.rs b/src/controllers/trustpub/tokens/revoke/mod.rs index 54693af4b9c..beaae9a439a 100644 --- a/src/controllers/trustpub/tokens/revoke/mod.rs +++ b/src/controllers/trustpub/tokens/revoke/mod.rs @@ -1,10 +1,12 @@ use crate::app::AppState; +use crate::auth::AuthHeader; use crate::util::errors::{AppResult, custom}; use crates_io_database::schema::trustpub_tokens; use crates_io_trustpub::access_token::AccessToken; use diesel::prelude::*; use diesel_async::RunQueryDsl; -use http::{HeaderMap, StatusCode, header}; +use http::StatusCode; +use secrecy::ExposeSecret; #[cfg(test)] mod tests; @@ -20,18 +22,9 @@ mod tests; tag = "trusted_publishing", responses((status = 204, description = "Successful Response")), )] -pub async fn revoke_trustpub_token(app: AppState, headers: HeaderMap) -> AppResult { - let Some(auth_header) = headers.get(header::AUTHORIZATION) else { - let message = "Missing authorization header"; - return Err(custom(StatusCode::UNAUTHORIZED, message)); - }; - - let Some(bearer) = auth_header.as_bytes().strip_prefix(b"Bearer ") else { - let message = "Invalid authorization header"; - return Err(custom(StatusCode::UNAUTHORIZED, message)); - }; - - let Ok(token) = AccessToken::from_byte_str(bearer) else { +pub async fn revoke_trustpub_token(app: AppState, auth: AuthHeader) -> AppResult { + let token = auth.token().expose_secret(); + let Ok(token) = AccessToken::from_byte_str(token.as_bytes()) else { let message = "Invalid authorization header"; return Err(custom(StatusCode::UNAUTHORIZED, message)); }; From 41fed8d61d197a616f49349fa383dbd084c95032 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:17:38 +0200 Subject: [PATCH 2/7] AuthHeader: Allow empty auth scheme in `Authorization` header --- src/auth.rs | 12 ++--------- .../trustpub/tokens/revoke/tests.rs | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 0c4f8300cd2..2f0c86a5072 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -8,7 +8,7 @@ use crate::util::errors::{ internal, }; use crate::util::token::HashedToken; -use axum::extract::{FromRequestParts, OptionalFromRequestParts}; +use axum::extract::FromRequestParts; use chrono::Utc; use crates_io_session::SessionExtension; use diesel_async::AsyncPgConnection; @@ -30,7 +30,7 @@ impl AuthHeader { })?; let (scheme, token) = auth_header.split_once(' ').unwrap_or(("", auth_header)); - if !scheme.eq_ignore_ascii_case("Bearer") { + if !(scheme.eq_ignore_ascii_case("Bearer") || scheme.is_empty()) { let message = "Invalid authorization header"; return Err(custom(StatusCode::UNAUTHORIZED, message)); } @@ -52,14 +52,6 @@ impl AuthHeader { } } -impl OptionalFromRequestParts for AuthHeader { - type Rejection = BoxedAppError; - - async fn from_request_parts(parts: &mut Parts, _: &S) -> Result, Self::Rejection> { - Self::optional_from_request_parts(parts).await - } -} - impl FromRequestParts for AuthHeader { type Rejection = BoxedAppError; diff --git a/src/controllers/trustpub/tokens/revoke/tests.rs b/src/controllers/trustpub/tokens/revoke/tests.rs index bac0ef954e0..989f8bb5d81 100644 --- a/src/controllers/trustpub/tokens/revoke/tests.rs +++ b/src/controllers/trustpub/tokens/revoke/tests.rs @@ -61,6 +61,27 @@ async fn test_happy_path() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn test_happy_path_without_auth_scheme() -> anyhow::Result<()> { + let (app, _client) = TestApp::full().empty().await; + let mut conn = app.db_conn().await; + + let token1 = new_token(&mut conn, 1).await?; + let _token2 = new_token(&mut conn, 2).await?; + assert_compact_debug_snapshot!(all_crate_ids(&mut conn).await?, @"[[Some(1)], [Some(2)]]"); + + let token_client = MockTokenUser::with_auth_header(token1, app.clone()); + + let response = token_client.delete::<()>(URL).await; + assert_snapshot!(response.status(), @"204 No Content"); + assert_eq!(response.text(), ""); + + // Check that the token is deleted + assert_compact_debug_snapshot!(all_crate_ids(&mut conn).await?, @"[[Some(2)]]"); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn test_missing_authorization_header() -> anyhow::Result<()> { let (_app, client) = TestApp::full().empty().await; From bd3db7e026ed2adf8aec6107c0bc092baa797785 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:19:50 +0200 Subject: [PATCH 3/7] AuthHeader: Improve `Authorization` header error messages --- src/auth.rs | 8 +++++--- src/controllers/trustpub/tokens/revoke/mod.rs | 2 +- src/controllers/trustpub/tokens/revoke/tests.rs | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 2f0c86a5072..6a2d399343e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -25,13 +25,15 @@ impl AuthHeader { }; let auth_header = auth_header.to_str().map_err(|_| { - let message = "Invalid authorization header"; + let message = "Invalid `Authorization` header: Found unexpected non-ASCII characters"; custom(StatusCode::UNAUTHORIZED, message) })?; let (scheme, token) = auth_header.split_once(' ').unwrap_or(("", auth_header)); if !(scheme.eq_ignore_ascii_case("Bearer") || scheme.is_empty()) { - let message = "Invalid authorization header"; + let message = format!( + "Invalid `Authorization` header: Found unexpected authentication scheme `{scheme}`" + ); return Err(custom(StatusCode::UNAUTHORIZED, message)); } @@ -42,7 +44,7 @@ impl AuthHeader { pub async fn from_request_parts(parts: &Parts) -> Result { let auth = Self::optional_from_request_parts(parts).await?; auth.ok_or_else(|| { - let message = "Missing authorization header"; + let message = "Missing `Authorization` header"; custom(StatusCode::UNAUTHORIZED, message) }) } diff --git a/src/controllers/trustpub/tokens/revoke/mod.rs b/src/controllers/trustpub/tokens/revoke/mod.rs index beaae9a439a..9295f99b988 100644 --- a/src/controllers/trustpub/tokens/revoke/mod.rs +++ b/src/controllers/trustpub/tokens/revoke/mod.rs @@ -25,7 +25,7 @@ mod tests; pub async fn revoke_trustpub_token(app: AppState, auth: AuthHeader) -> AppResult { let token = auth.token().expose_secret(); let Ok(token) = AccessToken::from_byte_str(token.as_bytes()) else { - let message = "Invalid authorization header"; + let message = "Invalid `Authorization` header: Failed to parse token"; return Err(custom(StatusCode::UNAUTHORIZED, message)); }; diff --git a/src/controllers/trustpub/tokens/revoke/tests.rs b/src/controllers/trustpub/tokens/revoke/tests.rs index 989f8bb5d81..4b9fd427a41 100644 --- a/src/controllers/trustpub/tokens/revoke/tests.rs +++ b/src/controllers/trustpub/tokens/revoke/tests.rs @@ -88,7 +88,7 @@ async fn test_missing_authorization_header() -> anyhow::Result<()> { let response = client.delete::<()>(URL).await; assert_snapshot!(response.status(), @"401 Unauthorized"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Missing authorization header"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Missing `Authorization` header"}]}"#); Ok(()) } @@ -103,7 +103,7 @@ async fn test_invalid_authorization_header_format() -> anyhow::Result<()> { let response = token_client.delete::<()>(URL).await; assert_snapshot!(response.status(), @"401 Unauthorized"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Invalid authorization header"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Invalid `Authorization` header: Failed to parse token"}]}"#); Ok(()) } @@ -118,7 +118,7 @@ async fn test_invalid_token_format() -> anyhow::Result<()> { let response = token_client.delete::<()>(URL).await; assert_snapshot!(response.status(), @"401 Unauthorized"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Invalid authorization header"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Invalid `Authorization` header: Failed to parse token"}]}"#); Ok(()) } From 55000d6deb98fe29ae350002a49f18be55e235f5 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:23:56 +0200 Subject: [PATCH 4/7] publish: Allow empty auth scheme for Trusted Publishing tokens ... by using the new `AuthHeader` extractor, which also slightly improves our error messages. --- src/controllers/krate/publish.rs | 32 ++++++++++++++--------------- src/tests/krate/publish/trustpub.rs | 27 ++++++++++-------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index c635b6be596..212de58d1f2 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -1,7 +1,7 @@ //! Functionality related to publishing a new crate or version of a crate. use crate::app::AppState; -use crate::auth::{AuthCheck, Authentication}; +use crate::auth::{AuthCheck, AuthHeader, Authentication}; use crate::worker::jobs::{ self, CheckTyposquat, SendPublishNotificationsJob, UpdateDefaultVersion, }; @@ -19,8 +19,9 @@ use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl}; use futures_util::TryFutureExt; use futures_util::TryStreamExt; use hex::ToHex; +use http::StatusCode; use http::request::Parts; -use http::{StatusCode, header}; +use secrecy::ExposeSecret; use sha2::{Digest, Sha256}; use std::collections::HashMap; use tokio::io::{AsyncRead, AsyncReadExt}; @@ -146,21 +147,20 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult anyhow::Result<()> { // Step 4: Publish a new version of the crate using the temporary access token - let header = format!("Bearer {}", token); - let oidc_token_client = MockTokenUser::with_auth_header(header, app.clone()); + let oidc_token_client = MockTokenUser::with_auth_header(token.to_string(), app.clone()); let pb = PublishBuilder::new(CRATE_NAME, "1.1.0"); let response = oidc_token_client.publish_crate(pb).await; @@ -186,8 +185,7 @@ async fn test_happy_path() -> anyhow::Result<()> { let token = new_token(&mut conn, krate.id).await?; - let header = format!("Bearer {}", token); - let oidc_token_client = MockTokenUser::with_auth_header(header, app); + let oidc_token_client = MockTokenUser::with_auth_header(token, app); let pb = PublishBuilder::new(&krate.name, "1.1.0"); let response = oidc_token_client.publish_crate(pb).await; @@ -226,7 +224,7 @@ async fn test_happy_path_with_fancy_auth_header() -> anyhow::Result<()> { } #[tokio::test(flavor = "multi_thread")] -async fn test_invalid_authorization_header_format() -> anyhow::Result<()> { +async fn test_invalid_token_format() -> anyhow::Result<()> { let (app, _client, cookie_client) = TestApp::full().with_user().await; let mut conn = app.db_conn().await; @@ -234,7 +232,7 @@ async fn test_invalid_authorization_header_format() -> anyhow::Result<()> { let owner_id = cookie_client.as_model().id; let krate = CrateBuilder::new("foo", owner_id).build(&mut conn).await?; - // Create a client with an invalid authorization header (missing "Bearer " prefix) + // Create a client with an invalid authorization header (missing token prefix) let header = "invalid-format".to_string(); let oidc_token_client = MockTokenUser::with_auth_header(header, app); @@ -247,7 +245,7 @@ async fn test_invalid_authorization_header_format() -> anyhow::Result<()> { } #[tokio::test(flavor = "multi_thread")] -async fn test_invalid_token_format() -> anyhow::Result<()> { +async fn test_invalid_bearer_token_format() -> anyhow::Result<()> { let (app, _client, cookie_client) = TestApp::full().with_user().await; let mut conn = app.db_conn().await; @@ -255,14 +253,14 @@ async fn test_invalid_token_format() -> anyhow::Result<()> { let owner_id = cookie_client.as_model().id; let krate = CrateBuilder::new("foo", owner_id).build(&mut conn).await?; - // Create a client with an invalid authorization header (missing "Bearer " prefix) + // Create a client with an invalid authorization header (missing token prefix) let header = "Bearer invalid-token".to_string(); let oidc_token_client = MockTokenUser::with_auth_header(header, app); let pb = PublishBuilder::new(&krate.name, "1.1.0"); let response = oidc_token_client.publish_crate(pb).await; - assert_snapshot!(response.status(), @"403 Forbidden"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Invalid authentication token"}]}"#); + assert_snapshot!(response.status(), @"401 Unauthorized"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"The given API token does not match the format used by crates.io. Tokens generated before 2020-07-14 were generated with an insecure random number generator, and have been revoked. You can generate a new token at https://crates.io/me. For more information please see https://blog.rust-lang.org/2020/07/14/crates-io-security-advisory.html. We apologize for any inconvenience."}]}"#); Ok(()) } @@ -278,8 +276,7 @@ async fn test_non_existent_token() -> anyhow::Result<()> { // Generate a valid token format, but it doesn't exist in the database let (token, _) = generate_token(); - let header = format!("Bearer {}", token); - let oidc_token_client = MockTokenUser::with_auth_header(header, app); + let oidc_token_client = MockTokenUser::with_auth_header(token, app); let pb = PublishBuilder::new(&krate.name, "1.1.0"); let response = oidc_token_client.publish_crate(pb).await; @@ -295,8 +292,7 @@ async fn test_non_existent_token_with_new_crate() -> anyhow::Result<()> { // Generate a valid token format, but it doesn't exist in the database let (token, _) = generate_token(); - let header = format!("Bearer {}", token); - let oidc_token_client = MockTokenUser::with_auth_header(header, app); + let oidc_token_client = MockTokenUser::with_auth_header(token, app); let pb = PublishBuilder::new("foo", "1.0.0"); let response = oidc_token_client.publish_crate(pb).await; @@ -316,8 +312,7 @@ async fn test_token_for_wrong_crate() -> anyhow::Result<()> { let krate = CrateBuilder::new("foo", owner_id).build(&mut conn).await?; let token = new_token(&mut conn, krate.id).await?; - let header = format!("Bearer {}", token); - let oidc_token_client = MockTokenUser::with_auth_header(header, app); + let oidc_token_client = MockTokenUser::with_auth_header(token, app); let krate = CrateBuilder::new("bar", owner_id).build(&mut conn).await?; From 4c323740048ce7d0c54cf95e22e6e701f6bdd321 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:39:10 +0200 Subject: [PATCH 5/7] Allow `Bearer` auth scheme for API tokens ... by using the new `AuthHeader` extractor, which also slightly improves our error messages. --- src/auth.rs | 13 ++---- src/tests/krate/publish/auth.rs | 28 ++++++++++++- ...__auth__new_krate_with_bearer_token-2.snap | 42 +++++++++++++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 src/tests/krate/publish/snapshots/crates_io__tests__krate__publish__auth__new_krate_with_bearer_token-2.snap diff --git a/src/auth.rs b/src/auth.rs index 6a2d399343e..53c60d5c191 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -14,7 +14,7 @@ use crates_io_session::SessionExtension; use diesel_async::AsyncPgConnection; use http::request::Parts; use http::{StatusCode, header}; -use secrecy::SecretString; +use secrecy::{ExposeSecret, SecretString}; pub struct AuthHeader(SecretString); @@ -252,17 +252,12 @@ async fn authenticate_via_token( parts: &Parts, conn: &mut AsyncPgConnection, ) -> AppResult> { - let maybe_authorization = parts - .headers() - .get(header::AUTHORIZATION) - .and_then(|h| h.to_str().ok()); - - let Some(header_value) = maybe_authorization else { + let Some(auth_header) = AuthHeader::optional_from_request_parts(parts).await? else { return Ok(None); }; - let token = - HashedToken::parse(header_value).map_err(|_| InsecurelyGeneratedTokenRevoked::boxed())?; + let token = auth_header.token().expose_secret(); + let token = HashedToken::parse(token).map_err(|_| InsecurelyGeneratedTokenRevoked::boxed())?; let token = ApiToken::find_by_api_token(conn, &token) .await diff --git a/src/tests/krate/publish/auth.rs b/src/tests/krate/publish/auth.rs index 14815909ae6..ebb6f42ccc4 100644 --- a/src/tests/krate/publish/auth.rs +++ b/src/tests/krate/publish/auth.rs @@ -1,10 +1,10 @@ use crate::schema::api_tokens; use crate::tests::builders::{CrateBuilder, PublishBuilder}; -use crate::tests::util::{RequestHelper, TestApp}; +use crate::tests::util::{MockTokenUser, RequestHelper, TestApp}; use diesel::ExpressionMethods; use diesel_async::RunQueryDsl; use googletest::prelude::*; -use insta::assert_snapshot; +use insta::{assert_json_snapshot, assert_snapshot}; #[tokio::test(flavor = "multi_thread")] async fn new_wrong_token() { @@ -54,3 +54,27 @@ async fn new_krate_wrong_user() { assert_that!(app.stored_files().await, empty()); assert_that!(app.emails().await, empty()); } + +#[tokio::test(flavor = "multi_thread")] +async fn new_krate_with_bearer_token() { + let (app, _, _, token) = TestApp::full().with_token().await; + + let token = format!("Bearer {}", token.plaintext()); + let token = MockTokenUser::with_auth_header(token, app.clone()); + + let crate_to_publish = PublishBuilder::new("foo_new", "1.0.0"); + let response = token.publish_crate(crate_to_publish).await; + assert_snapshot!(response.status(), @"200 OK"); + assert_json_snapshot!(response.json(), { + ".crate.created_at" => "[datetime]", + ".crate.updated_at" => "[datetime]", + }); + + assert_snapshot!(app.stored_files().await.join("\n"), @r" + crates/foo_new/foo_new-1.0.0.crate + index/fo/o_/foo_new + rss/crates.xml + rss/crates/foo_new.xml + rss/updates.xml + "); +} diff --git a/src/tests/krate/publish/snapshots/crates_io__tests__krate__publish__auth__new_krate_with_bearer_token-2.snap b/src/tests/krate/publish/snapshots/crates_io__tests__krate__publish__auth__new_krate_with_bearer_token-2.snap new file mode 100644 index 00000000000..88cc12fe00c --- /dev/null +++ b/src/tests/krate/publish/snapshots/crates_io__tests__krate__publish__auth__new_krate_with_bearer_token-2.snap @@ -0,0 +1,42 @@ +--- +source: src/tests/krate/publish/auth.rs +expression: response.json() +--- +{ + "crate": { + "badges": [], + "categories": null, + "created_at": "[datetime]", + "default_version": "1.0.0", + "description": "description", + "documentation": null, + "downloads": 0, + "exact_match": false, + "homepage": null, + "id": "foo_new", + "keywords": null, + "links": { + "owner_team": "/api/v1/crates/foo_new/owner_team", + "owner_user": "/api/v1/crates/foo_new/owner_user", + "owners": "/api/v1/crates/foo_new/owners", + "reverse_dependencies": "/api/v1/crates/foo_new/reverse_dependencies", + "version_downloads": "/api/v1/crates/foo_new/downloads", + "versions": "/api/v1/crates/foo_new/versions" + }, + "max_stable_version": "1.0.0", + "max_version": "1.0.0", + "name": "foo_new", + "newest_version": "1.0.0", + "num_versions": 1, + "recent_downloads": null, + "repository": null, + "updated_at": "[datetime]", + "versions": null, + "yanked": false + }, + "warnings": { + "invalid_badges": [], + "invalid_categories": [], + "other": [] + } +} From 3b694870f113dddb625b51bbf1bd59aa782e9e81 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:44:55 +0200 Subject: [PATCH 6/7] AccessToken: Implement `FromStr` trait and update parsing usages All of our users at this point have `&str` anyway, so we might as well operate on strings. --- crates/crates_io_trustpub/src/access_token.rs | 12 ++++++++++++ src/controllers/krate/publish.rs | 2 +- src/controllers/trustpub/tokens/revoke/mod.rs | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/crates_io_trustpub/src/access_token.rs b/crates/crates_io_trustpub/src/access_token.rs index bdb9c2ed9fc..83201095290 100644 --- a/crates/crates_io_trustpub/src/access_token.rs +++ b/crates/crates_io_trustpub/src/access_token.rs @@ -2,6 +2,7 @@ use rand::distr::{Alphanumeric, SampleString}; use secrecy::{ExposeSecret, SecretString}; use sha2::digest::Output; use sha2::{Digest, Sha256}; +use std::str::FromStr; /// A temporary access token used to publish crates to crates.io using /// the "Trusted Publishing" feature. @@ -77,6 +78,17 @@ impl AccessToken { } } +impl FromStr for AccessToken { + type Err = AccessTokenError; + + /// Parse a string into an access token. + /// + /// This is equivalent to `AccessToken::from_byte_str`. + fn from_str(s: &str) -> Result { + Self::from_byte_str(s.as_bytes()) + } +} + /// The error type for parsing access tokens. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AccessTokenError { diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 212de58d1f2..1aa770be521 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -155,7 +155,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult().map_err(|_| { let message = "Invalid `Authorization` header: Failed to parse token"; custom(StatusCode::UNAUTHORIZED, message) })) diff --git a/src/controllers/trustpub/tokens/revoke/mod.rs b/src/controllers/trustpub/tokens/revoke/mod.rs index 9295f99b988..b0d79b2eb45 100644 --- a/src/controllers/trustpub/tokens/revoke/mod.rs +++ b/src/controllers/trustpub/tokens/revoke/mod.rs @@ -24,7 +24,7 @@ mod tests; )] pub async fn revoke_trustpub_token(app: AppState, auth: AuthHeader) -> AppResult { let token = auth.token().expose_secret(); - let Ok(token) = AccessToken::from_byte_str(token.as_bytes()) else { + let Ok(token) = token.parse::() else { let message = "Invalid `Authorization` header: Failed to parse token"; return Err(custom(StatusCode::UNAUTHORIZED, message)); }; From cd28aa372f27ad14426652b77d8dd14c8d992de6 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Jun 2025 17:48:35 +0200 Subject: [PATCH 7/7] AccessToken: Inline `from_byte_str()` fn --- crates/crates_io_trustpub/src/access_token.rs | 93 ++++++++----------- .../trustpub/tokens/exchange/tests.rs | 2 +- 2 files changed, 38 insertions(+), 57 deletions(-) diff --git a/crates/crates_io_trustpub/src/access_token.rs b/crates/crates_io_trustpub/src/access_token.rs index 83201095290..46b2877e348 100644 --- a/crates/crates_io_trustpub/src/access_token.rs +++ b/crates/crates_io_trustpub/src/access_token.rs @@ -30,36 +30,6 @@ impl AccessToken { Self(raw.into()) } - /// Parse a byte string into an access token. - /// - /// This can be used to convert an HTTP header value into an access token. - pub fn from_byte_str(byte_str: &[u8]) -> Result { - let suffix = byte_str - .strip_prefix(Self::PREFIX.as_bytes()) - .ok_or(AccessTokenError::MissingPrefix)?; - - if suffix.len() != Self::RAW_LENGTH + 1 { - return Err(AccessTokenError::InvalidLength); - } - - let suffix = std::str::from_utf8(suffix).map_err(|_| AccessTokenError::InvalidCharacter)?; - if !suffix.chars().all(|c| char::is_ascii_alphanumeric(&c)) { - return Err(AccessTokenError::InvalidCharacter); - } - - let raw = suffix.chars().take(Self::RAW_LENGTH).collect::(); - let claimed_checksum = suffix.chars().nth(Self::RAW_LENGTH).unwrap(); - let actual_checksum = checksum(raw.as_bytes()); - if claimed_checksum != actual_checksum { - return Err(AccessTokenError::InvalidChecksum { - claimed: claimed_checksum, - actual: actual_checksum, - }); - } - - Ok(Self(raw.into())) - } - /// Wrap the raw access token with the token prefix and a checksum. /// /// This turns e.g. `ABC` into `cio_tp_ABC{checksum}`. @@ -82,10 +52,30 @@ impl FromStr for AccessToken { type Err = AccessTokenError; /// Parse a string into an access token. - /// - /// This is equivalent to `AccessToken::from_byte_str`. fn from_str(s: &str) -> Result { - Self::from_byte_str(s.as_bytes()) + let suffix = s + .strip_prefix(Self::PREFIX) + .ok_or(AccessTokenError::MissingPrefix)?; + + if suffix.len() != Self::RAW_LENGTH + 1 { + return Err(AccessTokenError::InvalidLength); + } + + if !suffix.chars().all(|c| char::is_ascii_alphanumeric(&c)) { + return Err(AccessTokenError::InvalidCharacter); + } + + let raw = suffix.chars().take(Self::RAW_LENGTH).collect::(); + let claimed_checksum = suffix.chars().nth(Self::RAW_LENGTH).unwrap(); + let actual_checksum = checksum(raw.as_bytes()); + if claimed_checksum != actual_checksum { + return Err(AccessTokenError::InvalidChecksum { + claimed: claimed_checksum, + actual: actual_checksum, + }); + } + + Ok(Self(raw.into())) } } @@ -141,42 +131,33 @@ mod tests { } #[test] - fn test_from_byte_str() { + fn test_from_str() { let token = AccessToken::generate().finalize(); let token = token.expose_secret(); - let token2 = assert_ok!(AccessToken::from_byte_str(token.as_bytes())); + let token2 = assert_ok!(token.parse::()); assert_eq!(token2.finalize().expose_secret(), token); - let bytes = b"cio_tp_0000000000000000000000000000000w"; - assert_ok!(AccessToken::from_byte_str(bytes)); + let str = "cio_tp_0000000000000000000000000000000w"; + assert_ok!(str.parse::()); - let bytes = b"invalid_token"; - assert_err_eq!( - AccessToken::from_byte_str(bytes), - AccessTokenError::MissingPrefix - ); + let str = "invalid_token"; + assert_err_eq!(str.parse::(), AccessTokenError::MissingPrefix); - let bytes = b"cio_tp_invalid_token"; - assert_err_eq!( - AccessToken::from_byte_str(bytes), - AccessTokenError::InvalidLength - ); + let str = "cio_tp_invalid_token"; + assert_err_eq!(str.parse::(), AccessTokenError::InvalidLength); - let bytes = b"cio_tp_00000000000000000000000000"; - assert_err_eq!( - AccessToken::from_byte_str(bytes), - AccessTokenError::InvalidLength - ); + let str = "cio_tp_00000000000000000000000000"; + assert_err_eq!(str.parse::(), AccessTokenError::InvalidLength); - let bytes = b"cio_tp_000000@0000000000000000000000000"; + let str = "cio_tp_000000@0000000000000000000000000"; assert_err_eq!( - AccessToken::from_byte_str(bytes), + str.parse::(), AccessTokenError::InvalidCharacter ); - let bytes = b"cio_tp_00000000000000000000000000000000"; + let str = "cio_tp_00000000000000000000000000000000"; assert_err_eq!( - AccessToken::from_byte_str(bytes), + str.parse::(), AccessTokenError::InvalidChecksum { claimed: '0', actual: 'w', diff --git a/src/controllers/trustpub/tokens/exchange/tests.rs b/src/controllers/trustpub/tokens/exchange/tests.rs index 8f5d8ac915d..61c5bc08c3b 100644 --- a/src/controllers/trustpub/tokens/exchange/tests.rs +++ b/src/controllers/trustpub/tokens/exchange/tests.rs @@ -83,7 +83,7 @@ async fn test_happy_path() -> anyhow::Result<()> { "#); let token = json["token"].as_str().unwrap(); - let token = assert_ok!(AccessToken::from_byte_str(token.as_bytes())); + let token = assert_ok!(token.parse::()); let hashed_token = token.sha256(); let mut conn = client.app().db_conn().await;