Skip to content

Commit cc14c72

Browse files
authored
Merge pull request #11567 from Turbo87/trustpub-errors
trustpub: Improve token exchange error messages
2 parents a895d9b + c939c47 commit cc14c72

File tree

2 files changed

+59
-21
lines changed

2 files changed

+59
-21
lines changed

src/controllers/trustpub/tokens/exchange/mod.rs

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::json;
22
use crate::app::AppState;
33
use crate::util::errors::{AppResult, bad_request, server_error};
44
use axum::Json;
5-
use crates_io_database::models::trustpub::{NewToken, NewUsedJti, TrustpubData};
5+
use crates_io_database::models::trustpub::{GitHubConfig, NewToken, NewUsedJti, TrustpubData};
66
use crates_io_database::schema::trustpub_configs_github;
77
use crates_io_diesel_helpers::lower;
88
use crates_io_trustpub::access_token::AccessToken;
@@ -38,7 +38,8 @@ pub async fn exchange_trustpub_token(
3838

3939
let unverified_issuer = unverified_token_data.claims.iss;
4040
let Some(keystore) = state.oidc_key_stores.get(&unverified_issuer) else {
41-
return Err(bad_request("Unsupported JWT issuer"));
41+
let error = format!("Unsupported JWT issuer: {unverified_issuer}");
42+
return Err(bad_request(error));
4243
};
4344

4445
let Some(unverified_key_id) = unverified_token_data.header.kid else {
@@ -60,7 +61,8 @@ pub async fn exchange_trustpub_token(
6061
// The following code is only supporting GitHub Actions for now, so let's
6162
// drop out if the issuer is not GitHub.
6263
if unverified_issuer != GITHUB_ISSUER_URL {
63-
return Err(bad_request("Unsupported JWT issuer"));
64+
let error = format!("Unsupported JWT issuer: {unverified_issuer}");
65+
return Err(bad_request(error));
6466
}
6567

6668
let audience = &state.config.trustpub_audience;
@@ -105,29 +107,65 @@ pub async fn exchange_trustpub_token(
105107
return Err(bad_request(message));
106108
};
107109

108-
let crate_ids = trustpub_configs_github::table
109-
.select(trustpub_configs_github::crate_id)
110-
.filter(trustpub_configs_github::repository_owner_id.eq(&repository_owner_id))
110+
let mut repo_configs = trustpub_configs_github::table
111+
.select(GitHubConfig::as_select())
111112
.filter(
112113
lower(trustpub_configs_github::repository_owner).eq(lower(&repository_owner)),
113114
)
114115
.filter(lower(trustpub_configs_github::repository_name).eq(lower(&repository_name)))
115-
.filter(trustpub_configs_github::workflow_filename.eq(&workflow_filename))
116-
.filter(
117-
trustpub_configs_github::environment
118-
.is_null()
119-
.or(lower(trustpub_configs_github::environment)
120-
.eq(lower(&signed_claims.environment))),
121-
)
122-
.load::<i32>(conn)
116+
.load(conn)
123117
.await?;
124118

125-
if crate_ids.is_empty() {
126-
warn!("No matching Trusted Publishing config found");
127-
let message = "No matching Trusted Publishing config found";
119+
if repo_configs.is_empty() {
120+
let message = format!("No Trusted Publishing config found for repository `{repo}`.");
121+
return Err(bad_request(message));
122+
}
123+
124+
let mismatched_owner_ids: Vec<String> = repo_configs
125+
.extract_if(.., |config| config.repository_owner_id != repository_owner_id)
126+
.map(|config| config.repository_owner_id.to_string())
127+
.collect();
128+
129+
if repo_configs.is_empty() {
130+
let message = format!("The Trusted Publishing config for repository `{repo}` does not match the repository owner ID ({repository_owner_id}) in the JWT. Expected owner IDs: {}. Please recreate the Trusted Publishing config to update the repository owner ID.", mismatched_owner_ids.join(", "));
131+
return Err(bad_request(message));
132+
}
133+
134+
let mismatched_workflows: Vec<String> = repo_configs
135+
.extract_if(.., |config| config.workflow_filename != workflow_filename)
136+
.map(|config| format!("`{}`", config.workflow_filename))
137+
.collect();
138+
139+
if repo_configs.is_empty() {
140+
let message = format!("The Trusted Publishing config for repository `{repo}` does not match the workflow filename `{workflow_filename}` in the JWT. Expected workflow filenames: {}", mismatched_workflows.join(", "));
128141
return Err(bad_request(message));
129142
}
130143

144+
let mismatched_environments: Vec<String> = repo_configs
145+
.extract_if(.., |config| {
146+
match (&config.environment, &signed_claims.environment) {
147+
// Keep configs with no environment requirement
148+
(None, _) => false,
149+
// Remove configs requiring environment when JWT has none
150+
(Some(_), None) => true,
151+
// Remove non-matching environments
152+
(Some(config_env), Some(signed_env)) => config_env.to_lowercase() != signed_env.to_lowercase(),
153+
}
154+
})
155+
.filter_map(|config| config.environment.map(|env| format!("`{env}`")))
156+
.collect();
157+
158+
if repo_configs.is_empty() {
159+
let message = if let Some(signed_environment) = &signed_claims.environment {
160+
format!("The Trusted Publishing config for repository `{repo}` does not match the environment `{signed_environment}` in the JWT. Expected environments: {}", mismatched_environments.join(", "))
161+
} else {
162+
format!("The Trusted Publishing config for repository `{repo}` requires an environment, but the JWT does not specify one. Expected environments: {}", mismatched_environments.join(", "))
163+
};
164+
return Err(bad_request(message));
165+
}
166+
167+
let crate_ids = repo_configs.iter().map(|config| config.crate_id).collect::<Vec<_>>();
168+
131169
let new_token = AccessToken::generate();
132170

133171
let trustpub_data = TrustpubData::GitHub {

src/controllers/trustpub/tokens/exchange/tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ async fn test_unsupported_issuer() -> anyhow::Result<()> {
157157
let body = default_claims().as_exchange_body()?;
158158
let response = client.post::<()>(URL, body).await;
159159
assert_snapshot!(response.status(), @"400 Bad Request");
160-
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"Unsupported JWT issuer"}]}"#);
160+
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"Unsupported JWT issuer: https://token.actions.githubusercontent.com"}]}"#);
161161

162162
Ok(())
163163
}
@@ -323,7 +323,7 @@ async fn test_missing_config() -> anyhow::Result<()> {
323323
let body = default_claims().as_exchange_body()?;
324324
let response = client.post::<()>(URL, body).await;
325325
assert_snapshot!(response.status(), @"400 Bad Request");
326-
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"No matching Trusted Publishing config found"}]}"#);
326+
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"No Trusted Publishing config found for repository `rust-lang/foo-rs`."}]}"#);
327327

328328
Ok(())
329329
}
@@ -335,7 +335,7 @@ async fn test_missing_environment() -> anyhow::Result<()> {
335335
let body = default_claims().as_exchange_body()?;
336336
let response = client.post::<()>(URL, body).await;
337337
assert_snapshot!(response.status(), @"400 Bad Request");
338-
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"No matching Trusted Publishing config found"}]}"#);
338+
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"The Trusted Publishing config for repository `rust-lang/foo-rs` requires an environment, but the JWT does not specify one. Expected environments: `prod`"}]}"#);
339339

340340
Ok(())
341341
}
@@ -350,7 +350,7 @@ async fn test_wrong_environment() -> anyhow::Result<()> {
350350
let body = claims.as_exchange_body()?;
351351
let response = client.post::<()>(URL, body).await;
352352
assert_snapshot!(response.status(), @"400 Bad Request");
353-
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"No matching Trusted Publishing config found"}]}"#);
353+
assert_snapshot!(response.json(), @r#"{"errors":[{"detail":"The Trusted Publishing config for repository `rust-lang/foo-rs` does not match the environment `not-prod` in the JWT. Expected environments: `prod`"}]}"#);
354354

355355
Ok(())
356356
}

0 commit comments

Comments
 (0)