Skip to content

Commit b9cf7b7

Browse files
JaredSnider-BitwardenJaredSnider-Bitwarden
authored
Update CoreClient's BwHttpClient to include new default headers (#621)
## 🎟️ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> n/a but required for #549 ## 📔 Objective <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> Adds the following new optional `ClientSettings`: 1. `device_identifier` 2. `bitwarden_package_type` In order to achieve feature parity with the web clients `ApiService`. Note: eventually `device_identifier` and `bitwarden_client_version` should be required. Added the following as new default headers: 1. `Device-Identifier` 2. `Bitwarden-Package-Type` 3. `reqwest::header::USER_AGENT` Worked with @dani-garcia to add proper clean up tickets as well. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes
1 parent fe36732 commit b9cf7b7

File tree

7 files changed

+111
-54
lines changed

7 files changed

+111
-54
lines changed

‎bitwarden_license/bitwarden-sm/src/client_secrets.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ mod tests {
137137
api_url: format!("http://{}/api", server.address()),
138138
user_agent: "Bitwarden Rust-SDK [TEST]".into(),
139139
device_type: DeviceType::SDK,
140+
device_identifier: None,
140141
bitwarden_client_version: None,
142+
bitwarden_package_type: None,
141143
};
142144

143145
(server, Client::new(Some(settings)))

‎crates/bitwarden-auth/src/send_access/client.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ mod tests {
124124
api_url: format!("http://{}/api", mock_server.address()),
125125
user_agent: "Bitwarden Rust-SDK [TEST]".into(),
126126
device_type: DeviceType::SDK,
127+
device_identifier: None,
127128
bitwarden_client_version: None,
129+
bitwarden_package_type: None,
128130
};
129131
let core_client = CoreClient::new(Some(settings));
130132
core_client.auth_new().send_access()

‎crates/bitwarden-core/src/client/client.rs‎

Lines changed: 89 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -41,61 +41,21 @@ impl Client {
4141
fn new_internal(settings_input: Option<ClientSettings>, tokens: Tokens) -> Self {
4242
let settings = settings_input.unwrap_or_default();
4343

44-
fn new_client_builder() -> reqwest::ClientBuilder {
45-
#[allow(unused_mut)]
46-
let mut client_builder = reqwest::Client::builder();
47-
48-
#[cfg(not(target_arch = "wasm32"))]
49-
{
50-
use rustls::ClientConfig;
51-
use rustls_platform_verifier::ConfigVerifierExt;
52-
client_builder = client_builder.use_preconfigured_tls(
53-
ClientConfig::with_platform_verifier()
54-
.expect("Failed to create platform verifier"),
55-
);
56-
57-
// Enforce HTTPS for all requests in non-debug builds
58-
#[cfg(not(debug_assertions))]
59-
{
60-
client_builder = client_builder.https_only(true);
61-
}
62-
}
63-
64-
client_builder
65-
}
66-
67-
let external_client = new_client_builder().build().expect("Build should not fail");
68-
69-
let mut headers = header::HeaderMap::new();
70-
headers.append(
71-
"Device-Type",
72-
HeaderValue::from_str(&(settings.device_type as u8).to_string())
73-
.expect("All numbers are valid ASCII"),
74-
);
44+
let external_http_client = new_http_client_builder()
45+
.build()
46+
.expect("External HTTP Client build should not fail");
7547

76-
if let Some(client_type) = Into::<Option<ClientName>>::into(settings.device_type) {
77-
headers.append(
78-
"Bitwarden-Client-Name",
79-
HeaderValue::from_str(&client_type.to_string())
80-
.expect("All ASCII strings are valid header values"),
81-
);
82-
}
83-
84-
if let Some(version) = &settings.bitwarden_client_version {
85-
headers.append(
86-
"Bitwarden-Client-Version",
87-
HeaderValue::from_str(version).expect("Version should be a valid header value"),
88-
);
89-
}
48+
let headers = build_default_headers(&settings);
9049

91-
let client_builder = new_client_builder().default_headers(headers);
92-
93-
let client = client_builder.build().expect("Build should not fail");
50+
let bw_http_client = new_http_client_builder()
51+
.default_headers(headers)
52+
.build()
53+
.expect("Bw HTTP Client build should not fail");
9454

9555
let identity = bitwarden_api_identity::apis::configuration::Configuration {
9656
base_path: settings.identity_url,
9757
user_agent: Some(settings.user_agent.clone()),
98-
client: client.clone(),
58+
client: bw_http_client.clone(),
9959
basic_auth: None,
10060
oauth_access_token: None,
10161
bearer_access_token: None,
@@ -105,7 +65,7 @@ impl Client {
10565
let api = bitwarden_api_api::apis::configuration::Configuration {
10666
base_path: settings.api_url,
10767
user_agent: Some(settings.user_agent),
108-
client,
68+
client: bw_http_client,
10969
basic_auth: None,
11070
oauth_access_token: None,
11171
bearer_access_token: None,
@@ -124,7 +84,7 @@ impl Client {
12484
api,
12585
settings.device_type,
12686
)),
127-
external_client,
87+
external_http_client,
12888
key_store: KeyStore::default(),
12989
#[cfg(feature = "internal")]
13090
security_state: RwLock::new(None),
@@ -134,3 +94,81 @@ impl Client {
13494
}
13595
}
13696
}
97+
98+
fn new_http_client_builder() -> reqwest::ClientBuilder {
99+
#[allow(unused_mut)]
100+
let mut client_builder = reqwest::Client::builder();
101+
102+
#[cfg(not(target_arch = "wasm32"))]
103+
{
104+
use rustls::ClientConfig;
105+
use rustls_platform_verifier::ConfigVerifierExt;
106+
client_builder = client_builder.use_preconfigured_tls(
107+
ClientConfig::with_platform_verifier().expect("Failed to create platform verifier"),
108+
);
109+
110+
// Enforce HTTPS for all requests in non-debug builds
111+
#[cfg(not(debug_assertions))]
112+
{
113+
client_builder = client_builder.https_only(true);
114+
}
115+
}
116+
117+
client_builder
118+
}
119+
120+
/// Build default headers for Bitwarden HttpClient
121+
fn build_default_headers(settings: &ClientSettings) -> header::HeaderMap {
122+
let mut headers = header::HeaderMap::new();
123+
124+
// Handle optional headers
125+
126+
if let Some(device_identifier) = &settings.device_identifier {
127+
headers.append(
128+
"Device-Identifier",
129+
HeaderValue::from_str(device_identifier)
130+
.expect("Device identifier should be a valid header value"),
131+
);
132+
}
133+
134+
if let Some(client_type) = Into::<Option<ClientName>>::into(settings.device_type) {
135+
headers.append(
136+
"Bitwarden-Client-Name",
137+
HeaderValue::from_str(&client_type.to_string())
138+
.expect("All ASCII strings are valid header values"),
139+
);
140+
}
141+
142+
if let Some(version) = &settings.bitwarden_client_version {
143+
headers.append(
144+
"Bitwarden-Client-Version",
145+
HeaderValue::from_str(version).expect("Version should be a valid header value"),
146+
);
147+
}
148+
149+
if let Some(package_type) = &settings.bitwarden_package_type {
150+
headers.append(
151+
"Bitwarden-Package-Type",
152+
HeaderValue::from_str(package_type)
153+
.expect("Package type should be a valid header value"),
154+
);
155+
}
156+
157+
// Handle required headers
158+
159+
headers.append(
160+
"Device-Type",
161+
HeaderValue::from_str(&(settings.device_type as u8).to_string())
162+
.expect("All numbers are valid ASCII"),
163+
);
164+
165+
// TODO: PM-29938 - Since we now add this header always, we need to remove this from the
166+
// auto-generated code
167+
headers.append(
168+
reqwest::header::USER_AGENT,
169+
HeaderValue::from_str(&settings.user_agent)
170+
.expect("User agent should be a valid header value"),
171+
);
172+
173+
headers
174+
}

‎crates/bitwarden-core/src/client/client_settings.rs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use serde::{Deserialize, Serialize};
1616
/// user_agent: "Bitwarden Rust-SDK".to_string(),
1717
/// device_type: DeviceType::SDK,
1818
/// bitwarden_client_version: None,
19+
/// bitwarden_package_type: None,
20+
/// device_identifier: None,
1921
/// };
2022
/// let default = ClientSettings::default();
2123
/// ```
@@ -36,8 +38,15 @@ pub struct ClientSettings {
3638
pub user_agent: String,
3739
/// Device type to send to Bitwarden. Defaults to SDK
3840
pub device_type: DeviceType,
39-
/// Bitwarden Client Version to send to Bitwarden.
41+
42+
// TODO: PM-29939 - Remove optionality when all clients pass these values
43+
/// Device identifier to send to Bitwarden. Optional for now in transition period.
44+
pub device_identifier: Option<String>,
45+
/// Bitwarden Client Version to send to Bitwarden. Optional for now in transition period.
4046
pub bitwarden_client_version: Option<String>,
47+
/// Bitwarden Package Type to send to Bitwarden. We should evaluate this field to see if it
48+
/// should be optional later.
49+
pub bitwarden_package_type: Option<String>,
4150
}
4251

4352
impl Default for ClientSettings {
@@ -47,7 +56,9 @@ impl Default for ClientSettings {
4756
api_url: "https://api.bitwarden.com".into(),
4857
user_agent: "Bitwarden Rust-SDK".into(),
4958
device_type: DeviceType::SDK,
59+
device_identifier: None,
5060
bitwarden_client_version: None,
61+
bitwarden_package_type: None,
5162
}
5263
}
5364
}

‎crates/bitwarden-core/src/client/internal.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub struct InternalClient {
125125

126126
/// Reqwest client useable for external integrations like email forwarders, HIBP.
127127
#[allow(unused)]
128-
pub(crate) external_client: reqwest::Client,
128+
pub(crate) external_http_client: reqwest::Client,
129129

130130
pub(super) key_store: KeyStore<KeyIds>,
131131
#[cfg(feature = "internal")]
@@ -230,7 +230,7 @@ impl InternalClient {
230230
#[allow(missing_docs)]
231231
#[cfg(feature = "internal")]
232232
pub fn get_http_client(&self) -> &reqwest::Client {
233-
&self.external_client
233+
&self.external_http_client
234234
}
235235

236236
#[allow(missing_docs)]

‎crates/bitwarden-vault/src/cipher/cipher_client/share_cipher.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,9 @@ mod tests {
795795
api_url: format!("http://{}", mock_server.address()),
796796
user_agent: "Bitwarden Test".into(),
797797
device_type: DeviceType::SDK,
798+
device_identifier: None,
798799
bitwarden_client_version: None,
800+
bitwarden_package_type: None,
799801
};
800802

801803
let client = Client::new(Some(settings));

‎crates/bw/src/vault/sync.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,9 @@ mod tests {
256256
api_url: api_config.base_path,
257257
user_agent: api_config.user_agent.unwrap(),
258258
device_type: DeviceType::SDK,
259+
device_identifier: None,
259260
bitwarden_client_version: None,
261+
bitwarden_package_type: None,
260262
}));
261263

262264
client

0 commit comments

Comments
 (0)