Skip to content

Commit 0976884

Browse files
authored
fix(aliyun-oss): normalize custom sts endpoints (#726)
This fixes a correctness bug in Aliyun OSS STS endpoint handling: when `ALIBABA_CLOUD_STS_ENDPOINT` was set to a full URL, the AssumeRole path prefixed `https://` again and produced a malformed request URI. The change normalizes custom STS endpoints so both bare hosts and full URLs work consistently across the AK-based AssumeRole provider and the OIDC-based STS provider. It also adds regression coverage for the env-configured full-URL case. Context: found while reviewing the latest 20 commits on `main`, specifically the new AssumeRole provider added in #724.
1 parent 3b749af commit 0976884

File tree

2 files changed

+155
-6
lines changed

2 files changed

+155
-6
lines changed

services/aliyun-oss/src/provide_credential/assume_role.rs

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ static ALIYUN_RPC_QUERY_ENCODE_SET: AsciiSet = NON_ALPHANUMERIC
3636
.remove(b'.')
3737
.remove(b'_')
3838
.remove(b'~');
39+
const DEFAULT_STS_ENDPOINT: &str = "https://sts.aliyuncs.com";
3940
static SIGNATURE_NONCE_COUNTER: AtomicU64 = AtomicU64::new(0);
4041

4142
/// AssumeRoleCredentialProvider loads credentials via Alibaba Cloud STS AssumeRole.
@@ -175,12 +176,12 @@ impl AssumeRoleCredentialProvider {
175176

176177
fn get_sts_endpoint(&self, envs: &HashMap<String, String>) -> String {
177178
if let Some(endpoint) = &self.sts_endpoint {
178-
return endpoint.clone();
179+
return normalize_sts_endpoint(endpoint);
179180
}
180181

181182
match envs.get(ALIBABA_CLOUD_STS_ENDPOINT) {
182-
Some(endpoint) => format!("https://{endpoint}"),
183-
None => "https://sts.aliyuncs.com".to_string(),
183+
Some(endpoint) => normalize_sts_endpoint(endpoint),
184+
None => DEFAULT_STS_ENDPOINT.to_string(),
184185
}
185186
}
186187

@@ -308,6 +309,15 @@ fn default_base_provider_chain() -> ProvideCredentialChain<Credential> {
308309
.push(ConfigFileCredentialProvider::new())
309310
}
310311

312+
fn normalize_sts_endpoint(endpoint: &str) -> String {
313+
let endpoint = endpoint.trim().trim_end_matches('/');
314+
if endpoint.starts_with("https://") || endpoint.starts_with("http://") {
315+
endpoint.to_string()
316+
} else {
317+
format!("https://{endpoint}")
318+
}
319+
}
320+
311321
fn canonicalized_query_string(params: &BTreeMap<String, String>) -> String {
312322
params
313323
.iter()
@@ -453,6 +463,22 @@ mod tests {
453463
Ok(())
454464
}
455465

466+
#[test]
467+
fn test_normalize_sts_endpoint_accepts_bare_host_and_full_url() {
468+
assert_eq!(
469+
"https://sts.aliyuncs.com",
470+
normalize_sts_endpoint("sts.aliyuncs.com")
471+
);
472+
assert_eq!(
473+
"https://sts.example.com",
474+
normalize_sts_endpoint("https://sts.example.com/")
475+
);
476+
assert_eq!(
477+
"http://sts.example.com",
478+
normalize_sts_endpoint("http://sts.example.com/")
479+
);
480+
}
481+
456482
#[tokio::test]
457483
async fn test_assume_role_loader_without_config() {
458484
let ctx = Context::new().with_env(StaticEnv {
@@ -637,4 +663,50 @@ mod tests {
637663

638664
Ok(())
639665
}
666+
667+
#[tokio::test]
668+
async fn test_assume_role_accepts_full_url_sts_endpoint_from_env() -> Result<()> {
669+
let http_send = CaptureHttpSend::new(vec![
670+
br#"{"Credentials":{"SecurityToken":"sts-token","Expiration":"2124-05-25T11:45:17Z","AccessKeySecret":"sts-secret","AccessKeyId":"sts-ak"}}"#
671+
.to_vec(),
672+
]);
673+
let ctx = Context::new()
674+
.with_http_send(http_send.clone())
675+
.with_env(StaticEnv {
676+
home_dir: None,
677+
envs: HashMap::from([
678+
(
679+
ALIBABA_CLOUD_ROLE_ARN.to_string(),
680+
"acs:ram::123456789012:role/test-role".to_string(),
681+
),
682+
(
683+
ALIBABA_CLOUD_STS_ENDPOINT.to_string(),
684+
"https://sts.example.com".to_string(),
685+
),
686+
]),
687+
});
688+
689+
let provider = AssumeRoleCredentialProvider::new()
690+
.with_base_provider(TestBaseCredentialProvider::new(Some(Credential {
691+
access_key_id: "base-ak".to_string(),
692+
access_key_secret: "base-sk".to_string(),
693+
security_token: None,
694+
expires_in: None,
695+
})))
696+
.with_role_session_name("test-session")
697+
.with_time("2024-03-05T06:07:08Z".parse().unwrap())
698+
.with_signature_nonce("test-nonce");
699+
700+
let _ = provider
701+
.provide_credential(&ctx)
702+
.await?
703+
.expect("credential must be loaded");
704+
705+
let recorded_uri = http_send.uri().expect("request uri must be captured");
706+
let uri: http::Uri = recorded_uri.parse().expect("uri must parse");
707+
assert_eq!(Some("sts.example.com"), uri.authority().map(|v| v.as_str()));
708+
assert_eq!("/", uri.path());
709+
710+
Ok(())
711+
}
640712
}

services/aliyun-oss/src/provide_credential/assume_role_with_oidc.rs

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ use reqsign_core::time::Timestamp;
2222
use reqsign_core::{Context, ProvideCredential};
2323
use serde::Deserialize;
2424

25+
const DEFAULT_STS_ENDPOINT: &str = "https://sts.aliyuncs.com";
26+
2527
/// AssumeRoleWithOidcCredentialProvider loads credential via assume role with OIDC.
2628
///
2729
/// This provider reads configuration from environment variables at runtime:
@@ -59,12 +61,12 @@ impl AssumeRoleWithOidcCredentialProvider {
5961

6062
fn get_sts_endpoint(&self, envs: &std::collections::HashMap<String, String>) -> String {
6163
if let Some(endpoint) = &self.sts_endpoint {
62-
return endpoint.clone();
64+
return normalize_sts_endpoint(endpoint);
6365
}
6466

6567
match envs.get(ALIBABA_CLOUD_STS_ENDPOINT) {
66-
Some(endpoint) => format!("https://{endpoint}"),
67-
None => "https://sts.aliyuncs.com".to_string(),
68+
Some(endpoint) => normalize_sts_endpoint(endpoint),
69+
None => DEFAULT_STS_ENDPOINT.to_string(),
6870
}
6971
}
7072

@@ -146,6 +148,15 @@ impl ProvideCredential for AssumeRoleWithOidcCredentialProvider {
146148
}
147149
}
148150

151+
fn normalize_sts_endpoint(endpoint: &str) -> String {
152+
let endpoint = endpoint.trim().trim_end_matches('/');
153+
if endpoint.starts_with("https://") || endpoint.starts_with("http://") {
154+
endpoint.to_string()
155+
} else {
156+
format!("https://{endpoint}")
157+
}
158+
}
159+
149160
#[derive(Default, Debug, Deserialize)]
150161
#[serde(default)]
151162
struct AssumeRoleWithOidcResponse {
@@ -214,6 +225,22 @@ mod tests {
214225
Ok(())
215226
}
216227

228+
#[test]
229+
fn test_normalize_sts_endpoint_accepts_bare_host_and_full_url() {
230+
assert_eq!(
231+
"https://sts.aliyuncs.com",
232+
normalize_sts_endpoint("sts.aliyuncs.com")
233+
);
234+
assert_eq!(
235+
"https://sts.example.com",
236+
normalize_sts_endpoint("https://sts.example.com/")
237+
);
238+
assert_eq!(
239+
"http://sts.example.com",
240+
normalize_sts_endpoint("http://sts.example.com/")
241+
);
242+
}
243+
217244
#[tokio::test]
218245
async fn test_assume_role_with_oidc_loader_without_config() {
219246
let ctx = Context::new()
@@ -404,4 +431,54 @@ mod tests {
404431

405432
Ok(())
406433
}
434+
435+
#[tokio::test]
436+
async fn test_assume_role_with_oidc_accepts_full_url_sts_endpoint_from_env() -> Result<()> {
437+
let token_path = "/mock/token";
438+
let file_read = TestFileRead {
439+
expected_path: token_path.to_string(),
440+
content: b"token".to_vec(),
441+
};
442+
let http_body = r#"{"Credentials":{"SecurityToken":"security_token","Expiration":"2124-05-25T11:45:17Z","AccessKeySecret":"secret_access_key","AccessKeyId":"access_key_id"}}"#;
443+
let http_send = CaptureHttpSend::new(http_body);
444+
445+
let ctx = Context::new()
446+
.with_file_read(file_read)
447+
.with_http_send(http_send.clone())
448+
.with_env(StaticEnv {
449+
home_dir: None,
450+
envs: HashMap::from_iter([
451+
(
452+
ALIBABA_CLOUD_OIDC_TOKEN_FILE.to_string(),
453+
token_path.to_string(),
454+
),
455+
(
456+
ALIBABA_CLOUD_ROLE_ARN.to_string(),
457+
"acs:ram::123456789012:role/test-role".to_string(),
458+
),
459+
(
460+
ALIBABA_CLOUD_OIDC_PROVIDER_ARN.to_string(),
461+
"acs:ram::123456789012:oidc-provider/test-provider".to_string(),
462+
),
463+
(
464+
ALIBABA_CLOUD_STS_ENDPOINT.to_string(),
465+
"https://sts.example.com".to_string(),
466+
),
467+
]),
468+
});
469+
470+
let _ = AssumeRoleWithOidcCredentialProvider::new()
471+
.provide_credential(&ctx)
472+
.await?
473+
.expect("credential must be loaded");
474+
475+
let recorded_uri = http_send
476+
.uri()
477+
.expect("http_send must capture outgoing uri");
478+
let uri: http::Uri = recorded_uri.parse().expect("uri must parse");
479+
assert_eq!(Some("sts.example.com"), uri.authority().map(|v| v.as_str()));
480+
assert_eq!("/", uri.path());
481+
482+
Ok(())
483+
}
407484
}

0 commit comments

Comments
 (0)