Skip to content

Commit

Permalink
Fix save_credential failing when the selected cipher doesn't have a P…
Browse files Browse the repository at this point in the history
…asskey (#831)

## 🎟️ Tracking

<!-- Paste the link to the Jira or GitHub issue or otherwise describe /
point to where this change is coming from. -->

## 📔 Objective

The current call to `get_selected_credential` inside `save_credential`
will try to fetch and decrypt the ciphers FIDO2 credentials, and error
if they are not there.

This can only happen when creating a new Passkey, so instead of calling
`get_selected_credential` we just get the value from the lock. The other
places where `get_selected_credential` is used is in `update_credential`
and right at the end of the `assertion`, `register` and `authenticate`
operations, so those should be safe.

## ⏰ 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: Andreas Coroiu <andreas.coroiu@gmail.com>
  • Loading branch information
dani-garcia and coroiu authored Jun 11, 2024
1 parent 85bfa59 commit dfd0ba8
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crates/bitwarden/src/platform/fido2/authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,14 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
let cred = try_from_credential_full(cred, user, rp)?;

// Get the previously selected cipher and add the new credential to it
let mut selected: CipherView = this.authenticator.get_selected_credential()?.cipher;
let mut selected: CipherView = this
.authenticator
.selected_credential
.lock()
.expect("Mutex is not poisoned")
.clone()
.ok_or("No selected cipher available")?;

selected.set_new_fido2_credentials(enc, vec![cred])?;

// Store the updated credential for later use
Expand Down

0 comments on commit dfd0ba8

Please sign in to comment.