From 29eae5887e5673787622d2536b80af723abfd44e Mon Sep 17 00:00:00 2001 From: "Steffen R. Knollmann" Date: Thu, 11 Aug 2022 10:51:18 +0100 Subject: [PATCH 1/2] Simplify encoded value generation fixing lifetime issue Shadowing `encoded` makes the async block not send which prevents sharing it across threads. Just do the generation of the encoded values through temporary variables instead of shadowing. The added code in the test module will fail to compile without the accompanying code changes. --- sdk/identity/src/device_code_flow/mod.rs | 52 +++++++++++++++++++----- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/sdk/identity/src/device_code_flow/mod.rs b/sdk/identity/src/device_code_flow/mod.rs index cd96babde5..a95bc3ccd0 100644 --- a/sdk/identity/src/device_code_flow/mod.rs +++ b/sdk/identity/src/device_code_flow/mod.rs @@ -37,10 +37,10 @@ where tenant_id ); - let mut encoded = form_urlencoded::Serializer::new(String::new()); - let encoded = encoded.append_pair("client_id", client_id.as_str()); - let encoded = encoded.append_pair("scope", &scopes.join(" ")); - let encoded = encoded.finish(); + let encoded = form_urlencoded::Serializer::new(String::new()) + .append_pair("client_id", client_id.as_str()) + .append_pair("scope", &scopes.join(" ")) + .finish(); let rsp = post_form(http_client.clone(), url, encoded).await?; let rsp_status = rsp.status(); @@ -116,12 +116,11 @@ impl<'a> DeviceCodePhaseOneResponse<'a> { // last poll and wait only the delta. new_timer(Duration::from_secs(self.interval)).await; - let mut encoded = form_urlencoded::Serializer::new(String::new()); - let encoded = encoded - .append_pair("grant_type", "urn:ietf:params:oauth:grant-type:device_code"); - let encoded = encoded.append_pair("client_id", self.client_id.as_str()); - let encoded = encoded.append_pair("device_code", &self.device_code); - let encoded = encoded.finish(); + let encoded = form_urlencoded::Serializer::new(String::new()) + .append_pair("grant_type", "urn:ietf:params:oauth:grant-type:device_code") + .append_pair("client_id", self.client_id.as_str()) + .append_pair("device_code", &self.device_code) + .finish(); let http_client = self.http_client.clone().unwrap(); @@ -184,3 +183,36 @@ async fn post_form( req.set_body(form_body); http_client.execute_request(&req).await } + +#[cfg(test)] +mod tests { + use super::*; + use async_trait::async_trait; + use std::error::Error; + + #[async_trait] + trait LoginHelper { + async fn get_secret(&self) -> Result>; + } + + struct DeviceCodeLoginHelper { + client_id: ClientId, + tenant_id: String, + scopes: Vec, + } + + #[async_trait] + impl LoginHelper for DeviceCodeLoginHelper { + async fn get_secret(&self) -> Result> { + let scopes: Vec<_> = self.scopes.iter().map(|x| x.as_ref()).collect(); + let _phase_one = start( + azure_core::new_http_client(), + &self.tenant_id, + &self.client_id, + &scopes, + ) + .await?; + Ok("".to_string()) + } + } +} From 53622680da0573288f46a6df147f1a051678de27 Mon Sep 17 00:00:00 2001 From: "Steffen R. Knollmann" Date: Thu, 11 Aug 2022 13:01:52 +0100 Subject: [PATCH 2/2] Simplify test and be clearer what it is testing --- sdk/identity/src/device_code_flow/mod.rs | 34 +++++++----------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/sdk/identity/src/device_code_flow/mod.rs b/sdk/identity/src/device_code_flow/mod.rs index a95bc3ccd0..d8c8defa5a 100644 --- a/sdk/identity/src/device_code_flow/mod.rs +++ b/sdk/identity/src/device_code_flow/mod.rs @@ -187,32 +187,16 @@ async fn post_form( #[cfg(test)] mod tests { use super::*; - use async_trait::async_trait; - use std::error::Error; - #[async_trait] - trait LoginHelper { - async fn get_secret(&self) -> Result>; - } - - struct DeviceCodeLoginHelper { - client_id: ClientId, - tenant_id: String, - scopes: Vec, - } + fn require_send(_t: T) {} - #[async_trait] - impl LoginHelper for DeviceCodeLoginHelper { - async fn get_secret(&self) -> Result> { - let scopes: Vec<_> = self.scopes.iter().map(|x| x.as_ref()).collect(); - let _phase_one = start( - azure_core::new_http_client(), - &self.tenant_id, - &self.client_id, - &scopes, - ) - .await?; - Ok("".to_string()) - } + #[test] + fn ensure_that_start_is_send() { + require_send(start( + azure_core::new_http_client(), + "UNUSED", + &ClientId::new("UNUSED".to_owned()), + &[], + )); } }