Skip to content

Commit 069d782

Browse files
Auth/PM-28840 - SendAccessCredentials - re-order variant to fix email + OTP credential submissions (#594)
## 🎟️ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> https://bitwarden.atlassian.net/browse/PM-28840 ## 📔 Objective <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> To resolve an issue where, due to `serde(untagged)` deserialization, Send Access requests with email and OTP were losing the submitted OTP. This prevented callers from being able to use that flow entirely. ## ⏰ 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 a09e691 commit 069d782

File tree

1 file changed

+159
-20
lines changed

1 file changed

+159
-20
lines changed

crates/bitwarden-auth/src/send_access/access_token_request.rs

Lines changed: 159 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@ pub struct SendEmailOtpCredentials {
4141
pub enum SendAccessCredentials {
4242
#[allow(missing_docs)]
4343
Password(SendPasswordCredentials),
44-
#[allow(missing_docs)]
45-
Email(SendEmailCredentials),
44+
// IMPORTANT: EmailOtp must come before Email due to #[serde(untagged)] deserialization.
45+
// Serde tries variants in order and stops at the first match. Since EmailOtp has
46+
// both "email" and "otp" fields, while Email only has "email", placing Email first
47+
// would cause {"email": "...", "otp": "..."} to incorrectly match the Email variant
48+
// (ignoring the "otp" field). We always must order untagged enum variants from most specific
49+
// (most fields) to least specific (fewest fields).
4650
#[allow(missing_docs)]
4751
EmailOtp(SendEmailOtpCredentials),
52+
#[allow(missing_docs)]
53+
Email(SendEmailCredentials),
4854
}
4955

5056
/// A request structure for requesting a send access token from the API.
@@ -204,15 +210,16 @@ mod tests {
204210
use serde_json::{from_str, to_string};
205211

206212
use super::*;
213+
207214
#[test]
208-
fn deserializes_camelcase_from_ts() {
215+
fn deserialize_struct_camelcase_from_ts() {
209216
let json = r#"{ "passwordHashB64": "ha$h" }"#;
210217
let s: SendPasswordCredentials = from_str(json).unwrap();
211218
assert_eq!(s.password_hash_b64, "ha$h");
212219
}
213220

214221
#[test]
215-
fn serializes_camelcase_to_wire() {
222+
fn serialize_struct_camelcase_to_wire() {
216223
let s = SendPasswordCredentials {
217224
password_hash_b64: "ha$h".into(),
218225
};
@@ -221,31 +228,163 @@ mod tests {
221228
}
222229

223230
#[test]
224-
fn roundtrip_camel_in_to_camel_out() {
231+
fn roundtrip_struct_camel_in_to_camel_out() {
225232
let in_json = r#"{ "passwordHashB64": "ha$h" }"#;
226233
let parsed: SendPasswordCredentials = from_str(in_json).unwrap();
227234
let out_json = to_string(&parsed).unwrap();
228235
assert_eq!(out_json, r#"{"passwordHashB64":"ha$h"}"#);
229236
}
237+
238+
#[test]
239+
fn deserialize_enum_variant_from_json() {
240+
let json = r#"{"passwordHashB64":"ha$h"}"#;
241+
let creds: SendAccessCredentials = from_str(json).unwrap();
242+
match creds {
243+
SendAccessCredentials::Password(password_creds) => {
244+
assert_eq!(password_creds.password_hash_b64, "ha$h");
245+
}
246+
_ => panic!("Expected Password variant"),
247+
}
248+
}
249+
250+
#[test]
251+
fn serialize_enum_variant_to_json() {
252+
let creds = SendAccessCredentials::Password(SendPasswordCredentials {
253+
password_hash_b64: "ha$h".into(),
254+
});
255+
let json = to_string(&creds).unwrap();
256+
assert_eq!(json, r#"{"passwordHashB64":"ha$h"}"#);
257+
}
258+
259+
#[test]
260+
fn roundtrip_enum_variant_through_json() {
261+
let in_json = r#"{"passwordHashB64":"ha$h"}"#;
262+
let creds: SendAccessCredentials = from_str(in_json).unwrap();
263+
let out_json = to_string(&creds).unwrap();
264+
assert_eq!(out_json, r#"{"passwordHashB64":"ha$h"}"#);
265+
}
230266
}
231267

232-
#[test]
233-
fn serialize_email_credentials() {
234-
let creds = SendAccessCredentials::Email(SendEmailCredentials {
235-
email: "user@example.com".into(),
236-
});
237-
let json = serde_json::to_string(&creds).unwrap();
238-
assert_eq!(json, r#"{"email":"user@example.com"}"#);
268+
mod send_access_email_credentials_tests {
269+
use serde_json::{from_str, to_string};
270+
271+
use super::*;
272+
273+
#[test]
274+
fn deserialize_struct_camelcase_from_ts() {
275+
let json = r#"{ "email": "user@example.com" }"#;
276+
let s: SendEmailCredentials = from_str(json).unwrap();
277+
assert_eq!(s.email, "user@example.com");
278+
}
279+
280+
#[test]
281+
fn serialize_struct_camelcase_to_wire() {
282+
let s = SendEmailCredentials {
283+
email: "user@example.com".into(),
284+
};
285+
let json = to_string(&s).unwrap();
286+
assert_eq!(json, r#"{"email":"user@example.com"}"#);
287+
}
288+
289+
#[test]
290+
fn roundtrip_struct_camel_in_to_camel_out() {
291+
let in_json = r#"{ "email": "user@example.com" }"#;
292+
let parsed: SendEmailCredentials = from_str(in_json).unwrap();
293+
let out_json = to_string(&parsed).unwrap();
294+
assert_eq!(out_json, r#"{"email":"user@example.com"}"#);
295+
}
296+
297+
#[test]
298+
fn deserialize_enum_variant_from_json() {
299+
let json = r#"{"email":"user@example.com"}"#;
300+
let creds: SendAccessCredentials = from_str(json).unwrap();
301+
match creds {
302+
SendAccessCredentials::Email(email_creds) => {
303+
assert_eq!(email_creds.email, "user@example.com");
304+
}
305+
_ => panic!("Expected Email variant"),
306+
}
307+
}
308+
309+
#[test]
310+
fn serialize_enum_variant_to_json() {
311+
let creds = SendAccessCredentials::Email(SendEmailCredentials {
312+
email: "user@example.com".into(),
313+
});
314+
let json = to_string(&creds).unwrap();
315+
assert_eq!(json, r#"{"email":"user@example.com"}"#);
316+
}
317+
318+
#[test]
319+
fn roundtrip_enum_variant_through_json() {
320+
let in_json = r#"{"email":"user@example.com"}"#;
321+
let creds: SendAccessCredentials = from_str(in_json).unwrap();
322+
let out_json = to_string(&creds).unwrap();
323+
assert_eq!(out_json, r#"{"email":"user@example.com"}"#);
324+
}
239325
}
240326

241-
#[test]
242-
fn serialize_email_otp_credentials() {
243-
let creds = SendAccessCredentials::EmailOtp(SendEmailOtpCredentials {
244-
email: "user@example.com".into(),
245-
otp: "123456".into(),
246-
});
247-
let json = serde_json::to_string(&creds).unwrap();
248-
assert_eq!(json, r#"{"email":"user@example.com","otp":"123456"}"#);
327+
mod send_access_email_otp_credentials_tests {
328+
use serde_json::{from_str, to_string};
329+
330+
use super::*;
331+
332+
#[test]
333+
fn deserialize_struct_camelcase_from_ts() {
334+
let json = r#"{ "email": "user@example.com", "otp": "123456" }"#;
335+
let s: SendEmailOtpCredentials = from_str(json).unwrap();
336+
assert_eq!(s.email, "user@example.com");
337+
assert_eq!(s.otp, "123456");
338+
}
339+
340+
#[test]
341+
fn serialize_struct_camelcase_to_wire() {
342+
let s = SendEmailOtpCredentials {
343+
email: "user@example.com".into(),
344+
otp: "123456".into(),
345+
};
346+
let json = to_string(&s).unwrap();
347+
assert_eq!(json, r#"{"email":"user@example.com","otp":"123456"}"#);
348+
}
349+
350+
#[test]
351+
fn roundtrip_struct_camel_in_to_camel_out() {
352+
let in_json = r#"{ "email": "user@example.com", "otp": "123456" }"#;
353+
let parsed: SendEmailOtpCredentials = from_str(in_json).unwrap();
354+
let out_json = to_string(&parsed).unwrap();
355+
assert_eq!(out_json, r#"{"email":"user@example.com","otp":"123456"}"#);
356+
}
357+
358+
#[test]
359+
fn deserialize_enum_variant_from_json() {
360+
let json = r#"{"email":"user@example.com","otp":"123456"}"#;
361+
let creds: SendAccessCredentials = from_str(json).unwrap();
362+
match creds {
363+
SendAccessCredentials::EmailOtp(otp_creds) => {
364+
assert_eq!(otp_creds.email, "user@example.com");
365+
assert_eq!(otp_creds.otp, "123456");
366+
}
367+
_ => panic!("Expected EmailOtp variant"),
368+
}
369+
}
370+
371+
#[test]
372+
fn serialize_enum_variant_to_json() {
373+
let creds = SendAccessCredentials::EmailOtp(SendEmailOtpCredentials {
374+
email: "user@example.com".into(),
375+
otp: "123456".into(),
376+
});
377+
let json = to_string(&creds).unwrap();
378+
assert_eq!(json, r#"{"email":"user@example.com","otp":"123456"}"#);
379+
}
380+
381+
#[test]
382+
fn roundtrip_enum_variant_through_json() {
383+
let in_json = r#"{"email":"user@example.com","otp":"123456"}"#;
384+
let creds: SendAccessCredentials = from_str(in_json).unwrap();
385+
let out_json = to_string(&creds).unwrap();
386+
assert_eq!(out_json, r#"{"email":"user@example.com","otp":"123456"}"#);
387+
}
249388
}
250389
}
251390
}

0 commit comments

Comments
 (0)