Skip to content
This repository was archived by the owner on May 14, 2025. It is now read-only.

[PM-4081] Fix crash on export page#2808

Merged
aj-rosado merged 1 commit intomasterfrom
bugfix/PM-4081-export-crash
Oct 3, 2023
Merged

[PM-4081] Fix crash on export page#2808
aj-rosado merged 1 commit intomasterfrom
bugfix/PM-4081-export-crash

Conversation

@aj-rosado
Copy link
Contributor

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Application is crashing to users that have biometrics active, updated to the most recent version without and try to export.
This is caused by a missing MasterKey, this fix creates the missing MasterKey if necessary at export.

Code changes

  • UserVerificationService.cs: Calling method GetOrDeriveMasterKeyAsync to ensure that the MasterKey if doesn't exist is created. In case of success, use the same logic from MasterPasswordRepromptService to update the MasterKey and the UserKey
  • CryptoService.cs: Created a method that updates the MasterKey and the UserKey based on the code from MasterPasswordRepromptService so that it can be reused by UserVerificationService
  • MasterPasswordRepromptService: Replacing the code by the new created method from CryptoService

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@aj-rosado aj-rosado added android bug Something isn't working iOS labels Oct 2, 2023
@bitwarden-bot
Copy link
Mannequin

bitwarden-bot mannequin commented Oct 2, 2023

Logo
Checkmarx One – Scan Summary & Details7f6f0ca6-fa3c-45a0-877f-9d071c17f505

No New Or Fixed Issues Found

@djsmith85 djsmith85 linked an issue Oct 2, 2023 that may be closed by this pull request
1 task
Copy link
Contributor

@andrebispo5 andrebispo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT 👍

@aj-rosado aj-rosado merged commit f2be840 into master Oct 3, 2023
@aj-rosado aj-rosado deleted the bugfix/PM-4081-export-crash branch October 3, 2023 11:54
@djsmith85 djsmith85 linked an issue Oct 4, 2023 that may be closed by this pull request
@nextlevelgamesmn
Copy link
Mannequin

nextlevelgamesmn mannequin commented Oct 16, 2023

Type of change

  • [] Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • [ X] Other

Objective

Application is crashing to users that have biometrics active, updated to the most recent version without and try to export. This is caused by a missing MasterKey, this fix creates the missing MasterKey if necessary at export.

Code changes

  • UserVerificationService.cs: Calling method GetOrDeriveMasterKeyAsync to ensure that the MasterKey if doesn't exist is created. In case of success, use the same logic from MasterPasswordRepromptService to update the MasterKey and the UserKey
  • CryptoService.cs: Created a method that updates the MasterKey and the UserKey based on the code from MasterPasswordRepromptService so that it can be reused by UserVerificationService
  • MasterPasswordRepromptService: Replacing the code by the new created method from CryptoService

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

Also having issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

android bug Something isn't working iOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when trying to export vault on iOS or android [ios] Export vault not working

5 participants