Skip to content

fix(error): map vertex/azure credential-validity errors to 401, not 500 (follow-up to #511) #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nic-6443 merged 1 commit into from
Jun 4, 2026
Merged

fix(error): map vertex/azure credential-validity errors to 401, not 500 (follow-up to #511) #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions crates/aisix-provider-azure-openai/src/aad_token_mint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl AadCredentials {
("client_secret", &self.client_secret),
] {
if value.is_empty() {
return Err(BridgeError::Config(format!(
return Err(BridgeError::InvalidUpstreamCredentials(format!(
"azure aad credentials.{name} is empty"
)));
}
Expand All @@ -119,7 +119,7 @@ impl AadCredentials {
|| value.contains('\n')
|| value.contains("..")
{
return Err(BridgeError::Config(format!(
return Err(BridgeError::InvalidUpstreamCredentials(format!(
"azure aad credentials.{name} {value:?} contains URL-control \
characters — reject `/`, `?`, `#`, whitespace, `..`"
)));
Expand All @@ -138,14 +138,14 @@ impl AadCredentials {
.filter(|s| !s.is_empty())
{
if host.contains('@') || host.contains('?') || host.contains('#') {
return Err(BridgeError::Config(
return Err(BridgeError::InvalidUpstreamConfig(
"azure aad credentials.authority_host must be a bare origin — \
reject userinfo (@), query (?), fragment (#)"
.into(),
));
}
if !(host.starts_with("https://") || host.starts_with("http://")) {
return Err(BridgeError::Config(format!(
return Err(BridgeError::InvalidUpstreamConfig(format!(
"azure aad credentials.authority_host must use http:// or https:// \
scheme, got {host:?}"
)));
Expand All @@ -165,7 +165,7 @@ impl AadCredentials {
.unwrap_or(host)
.trim_end_matches('/');
if after_scheme.contains('/') || after_scheme.contains('\\') {
return Err(BridgeError::Config(format!(
return Err(BridgeError::InvalidUpstreamConfig(format!(
"azure aad credentials.authority_host must be a bare origin \
(scheme://host[:port]) with no path, got {host:?}"
)));
Expand Down Expand Up @@ -502,10 +502,10 @@ mod tests {
};
let err = minter.get_token(&creds).await.err().unwrap();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("tenant_id is empty"));
}
other => panic!("expected Config, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand All @@ -529,10 +529,10 @@ mod tests {
};
let err = minter.get_token(&creds).await.err().unwrap();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("URL-control"));
}
other => panic!("expected Config, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand Down Expand Up @@ -632,8 +632,8 @@ mod tests {
};
let err = creds.validate().err().unwrap();
match err {
BridgeError::Config(msg) => assert!(msg.contains("http:// or https://")),
other => panic!("expected Config, got {other:?}"),
BridgeError::InvalidUpstreamConfig(msg) => assert!(msg.contains("http:// or https://")),
other => panic!("expected InvalidUpstreamConfig, got {other:?}"),
}
}

Expand All @@ -649,11 +649,11 @@ mod tests {
};
let err = creds.validate().err().unwrap();
match err {
BridgeError::Config(msg) => assert!(
BridgeError::InvalidUpstreamConfig(msg) => assert!(
msg.contains("bare origin") && msg.contains("no path"),
"expected a bare-origin/no-path rejection; got {msg}"
),
other => panic!("expected Config, got {other:?}"),
other => panic!("expected InvalidUpstreamConfig, got {other:?}"),
}
}

Expand Down Expand Up @@ -689,11 +689,11 @@ mod tests {
};
let err = creds.validate().err().unwrap();
match err {
BridgeError::Config(msg) => assert!(
BridgeError::InvalidUpstreamConfig(msg) => assert!(
msg.contains("bare origin") && msg.contains("no path"),
"expected a bare-origin/no-path rejection; got {msg}"
),
other => panic!("expected Config, got {other:?}"),
other => panic!("expected InvalidUpstreamConfig, got {other:?}"),
}
}

Expand All @@ -707,7 +707,7 @@ mod tests {
};
let err = creds.validate().err().unwrap();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamConfig(msg) => {
assert!(msg.contains("bare origin"));
// The pasted userinfo must NOT surface in the error.
assert!(!msg.contains("pass"), "error leaked userinfo: {msg}");
Expand All @@ -716,7 +716,7 @@ mod tests {
"error leaked host: {msg}"
);
}
other => panic!("expected Config, got {other:?}"),
other => panic!("expected InvalidUpstreamConfig, got {other:?}"),
}
}
}
38 changes: 19 additions & 19 deletions crates/aisix-provider-vertex/src/bridge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,15 +371,15 @@ impl VertexSecret {
/// "invalid character X at position N").
fn parse(secret: &str) -> Result<Self, BridgeError> {
if secret.trim().is_empty() {
return Err(BridgeError::Config(
return Err(BridgeError::InvalidUpstreamCredentials(
"vertex provider_key.secret is empty — \
expected JSON with project, region, and either access_token \
or service_account_json"
.into(),
));
}
let parsed: VertexSecret = serde_json::from_str(secret).map_err(|_e| {
BridgeError::Config(
BridgeError::InvalidUpstreamCredentials(
"vertex provider_key.secret must be valid JSON: \
{project, region, and either access_token or service_account_json}"
.into(),
Expand All @@ -390,7 +390,7 @@ impl VertexSecret {
// distinct error so the operator gets a clearer message than
// generic "neither set".
if parsed.access_token.as_deref().is_some_and(str::is_empty) {
return Err(BridgeError::Config(
return Err(BridgeError::InvalidUpstreamCredentials(
"vertex provider_key.secret.access_token is empty".into(),
));
}
Expand All @@ -400,14 +400,14 @@ impl VertexSecret {
.is_some_and(|t| !t.is_empty());
let has_sa = parsed.service_account_json.is_some();
if has_token && has_sa {
return Err(BridgeError::Config(
return Err(BridgeError::InvalidUpstreamCredentials(
"vertex provider_key.secret must set exactly one of access_token \
or service_account_json (both were provided)"
.into(),
));
}
if !has_token && !has_sa {
return Err(BridgeError::Config(
return Err(BridgeError::InvalidUpstreamCredentials(
"vertex provider_key.secret must set either access_token or \
service_account_json (neither was provided)"
.into(),
Expand Down Expand Up @@ -2213,21 +2213,21 @@ mod tests {
fn vertex_secret_rejects_empty() {
let err = VertexSecret::parse("").unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("secret is empty"));
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

#[test]
fn vertex_secret_rejects_non_json() {
let err = VertexSecret::parse("ya29.justatoken").unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("must be valid JSON"));
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand All @@ -2238,13 +2238,13 @@ mod tests {
let leaky = "X-DISTINCTIVE-LEAK-MARKER-Y";
let err = VertexSecret::parse(leaky).unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(
!msg.contains("DISTINCTIVE") && !msg.contains("LEAK-MARKER"),
"must NOT leak raw secret bytes; got {msg}"
);
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand Down Expand Up @@ -2282,14 +2282,14 @@ mod tests {
});
let err = VertexSecret::parse(&json.to_string()).unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(
msg.contains("exactly one of access_token or service_account_json"),
"got: {msg}"
);
assert!(msg.contains("both were provided"));
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand All @@ -2298,14 +2298,14 @@ mod tests {
let json = r#"{"project":"my-proj","region":"us-central1"}"#;
let err = VertexSecret::parse(json).unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(
msg.contains("either access_token or service_account_json"),
"got: {msg}"
);
assert!(msg.contains("neither was provided"));
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand All @@ -2318,10 +2318,10 @@ mod tests {
let json = r#"{"access_token":"","project":"my-proj","region":"us-central1"}"#;
let err = VertexSecret::parse(json).unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("access_token is empty"), "got: {msg}");
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand Down Expand Up @@ -2598,10 +2598,10 @@ mod tests {
let req = ChatFormat::new("customer-facing", vec![ChatMessage::user("hi")]);
let err = bridge.chat(&req, &ctx).await.unwrap_err();
match err {
BridgeError::Config(msg) => {
BridgeError::InvalidUpstreamCredentials(msg) => {
assert!(msg.contains("must be valid JSON"));
}
other => panic!("expected Config error, got {other:?}"),
other => panic!("expected InvalidUpstreamCredentials, got {other:?}"),
}
}

Expand Down
Loading