-
Notifications
You must be signed in to change notification settings - Fork 14
PM-23639: Add CXF Import mappings #361
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.
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
base: main
Are you sure you want to change the base?
Conversation
Great job! No new security vulnerabilities introduced in this pull request |
How do I tell Sonar to relax? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #361 +/- ##
==========================================
+ Coverage 74.19% 75.71% +1.51%
==========================================
Files 253 257 +4
Lines 21897 23214 +1317
==========================================
+ Hits 16247 17577 +1330
+ Misses 5650 5637 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
{ | ||
"type": "ssh-key", | ||
"keyType": "ssh-rsa", | ||
"privateKey": "MC4CAQAwBQYDK2VwBCIEID-U9VakauO4Fsv4b_znpDHcdYg74U68siZjnWLPn7Q1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tweaked this again, let's see which ends up in the final example.
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-23649 <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective This PR maps the Note type, and adds mapping the note for existing vault items. Note: A separate task has been created to track that we're mapping notes for all other cipheritems (some are not created yet). <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> ## ⏰ 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
… Identity (#363) ## 🎟️ Tracking * https://bitwarden.atlassian.net/browse/PM-23648 * https://bitwarden.atlassian.net/browse/PM-23652 * https://bitwarden.atlassian.net/browse/PM-23651 * https://bitwarden.atlassian.net/browse/PM-23647 * https://bitwarden.atlassian.net/browse/PM-23642 <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective This PR maps multiple identity documents to Identity + custom fields. PR was submitted in a rush, forgive me if it fails tests but would be worthwhile to have early reviews on my approach. <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> ## ⏰ 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 --------- Co-authored-by: Oscar Hinton <[email protected]>
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-23654 <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective This PR maps CXF totp during import <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> ## ⏰ 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 --------- Co-authored-by: Oscar Hinton <[email protected]>
## 🎟️ 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-23645 ## 📔 Objective <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> Wrap up cards by supporting the fields not exposed in our data model as custom fields. ## ⏰ 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
## 🎟️ 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-23653 ## 📔 Objective <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> Adds support for importing ssh keys using CXF. ## ⏰ 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
|
match ¬e_cipher.r#type { | ||
CipherType::SecureNote(_) => (), // Successfully identified as SecureNote | ||
_ => panic!("Expected SecureNote for standalone note credential"), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally recommend using if let when you only care about a single branch. (This applies to most tests in this file)
match ¬e_cipher.r#type { | |
CipherType::SecureNote(_) => (), // Successfully identified as SecureNote | |
_ => panic!("Expected SecureNote for standalone note credential"), | |
} | |
let identity = if let CipherType::Identity(identity) = &address_cipher.r#type { | |
identity | |
} else { | |
panic!("Expected Identity cipher for address") | |
}; |
// Check specific custom fields | ||
let issuing_country = identity_document_cipher | ||
.fields | ||
.iter() | ||
.find(|f| f.name.as_deref() == Some("Issuing Country")) | ||
.expect("Should have Issuing Country"); | ||
assert_eq!(issuing_country.value, Some("US".to_string())); | ||
|
||
let nationality = identity_document_cipher | ||
.fields | ||
.iter() | ||
.find(|f| f.name.as_deref() == Some("Nationality")) | ||
.expect("Should have Nationality"); | ||
assert_eq!(nationality.value, Some("American".to_string())); | ||
|
||
let birth_place = identity_document_cipher | ||
.fields | ||
.iter() | ||
.find(|f| f.name.as_deref() == Some("Birth Place")) | ||
.expect("Should have Birth Place"); | ||
assert_eq!(birth_place.value, Some("New York, USA".to_string())); | ||
|
||
let issuing_authority = identity_document_cipher | ||
.fields | ||
.iter() | ||
.find(|f| f.name.as_deref() == Some("Issuing Authority")) | ||
.expect("Should have Issuing Authority"); | ||
assert_eq!( | ||
issuing_authority.value, | ||
Some("Department of State".to_string()) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Compare fields directly which checks more than just the value.
// Check specific custom fields | |
let issuing_country = identity_document_cipher | |
.fields | |
.iter() | |
.find(|f| f.name.as_deref() == Some("Issuing Country")) | |
.expect("Should have Issuing Country"); | |
assert_eq!(issuing_country.value, Some("US".to_string())); | |
let nationality = identity_document_cipher | |
.fields | |
.iter() | |
.find(|f| f.name.as_deref() == Some("Nationality")) | |
.expect("Should have Nationality"); | |
assert_eq!(nationality.value, Some("American".to_string())); | |
let birth_place = identity_document_cipher | |
.fields | |
.iter() | |
.find(|f| f.name.as_deref() == Some("Birth Place")) | |
.expect("Should have Birth Place"); | |
assert_eq!(birth_place.value, Some("New York, USA".to_string())); | |
let issuing_authority = identity_document_cipher | |
.fields | |
.iter() | |
.find(|f| f.name.as_deref() == Some("Issuing Authority")) | |
.expect("Should have Issuing Authority"); | |
assert_eq!( | |
issuing_authority.value, | |
Some("Department of State".to_string()) | |
); | |
let expected_fields = vec![ | |
Field { | |
name: Some("Issuing Country".to_string()), | |
value: Some("US".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Nationality".to_string()), | |
value: Some("American".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Birth Date".to_string()), | |
value: Some("1990-04-15".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Birth Place".to_string()), | |
value: Some("New York, USA".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Sex".to_string()), | |
value: Some("F".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Issue Date".to_string()), | |
value: Some("2020-01-01".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Expiry Date".to_string()), | |
value: Some("2030-01-01".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
Field { | |
name: Some("Issuing Authority".to_string()), | |
value: Some("Department of State".to_string()), | |
r#type: FieldType::Text as u8, | |
linked_id: None, | |
}, | |
]; | |
assert_eq!(identity_document_cipher.fields, expected_fields); |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-23639
📔 Objective
This PR adds the CXF sample from the repo so that we can use it for reference in unit tests.
It also servers as the feature branch for adding CXF Import mappings for multiple CXF data types.
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+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 confirmedissue 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