mobile icon indicating copy to clipboard operation
mobile copied to clipboard

Requiring providing master password on application restart is not respected when a pin is set together with biometrics unlock

Open mfronczyk opened this issue 3 years ago • 10 comments
trafficstars

Steps To Reproduce

  1. Go to 'settings'
  2. Enable 'unlock using Face Id'
  3. Click on 'unlock using pin'
  4. Set a pin
  5. On the question 'do you want to require providing master password after application restart', click 'yes'.
  6. Shutdown the application
  7. Open new instance

(These might not be the exact English labels because my app renders in Polish, but I think it's close enough to reproduce)

Expected Result

Application should require the master password in order to unlock

Actual Result

It can be unlocked using master password or biometrics

Screenshots or Videos

No response

Additional Context

I'd basically like Bitwarden to require providing the master password from time to time in order to help me remembering it (not forgetting). I couldn't find such option, but it seemed the setting a pin and then requiring the master pass to be provided on restart would help he to mimic such behavior, but it doesn't seem to work as I expected.

I'd also be grateful if you could let me know if there's somewhere an option to request providing the master key from time to time.

Operating System

iOS

Operating System Version

15.6

Device

iPhone 13 Pro

Build Version

2022.6.2 (1951)

Beta

  • [ ] Using a pre-release version of the application.

mfronczyk avatar Aug 08 '22 14:08 mfronczyk

Thinking about it bit more, this setting was probably meant to require the master pass instead of pin, but if pin is combined with biometrics, then you can still unlock with biometrics and you don't have to provide the master pass.

Should the question about requiring master pass to be provided when application restarts be actually changed to a checkbox at the top level so that it affects both pin unlocking and biometrics?

mfronczyk avatar Aug 08 '22 15:08 mfronczyk

@mfronczyk @fedemkr Hi I would like to contribute to this issue can you let me know how I can solve this issue.

Mayuresh0072 avatar Jan 21 '23 09:01 Mayuresh0072

In my opinion it should be like this:

  1. Add a new checkbox in the security settings section "require providing master password after application restart"
  2. Remove the "do you want to require providing master password after application restart" question which appears after setting a pin
  3. When the box is checked, on restart, the application should ask for the master password and disable using faceid or pin to unlock. This should be only on restart, not when the application is brought to the front from the background.

mfronczyk avatar Jan 23 '23 09:01 mfronczyk

Hi there, thank you very much for the report, the feedback and the interest in contributing to this project 😄 . I talked with the team and we think the simplest approach here would be to have the same flow as in "Unlock with PIN" but on "Unlock with Biometrics".

So in essence, the flow in Settings would be:

  1. Tap "Unlock with Biometrics"
  2. After setting up biometrics, show same prompt as in "Unlock with PIN", i.e. "Do you want to require unlocking with your master password when the application is restarted?"
  3. If user confirms, then when app is restarted the master password will be asked instead of Biometrics.

With this approach the user has more flexibility to manage the settings separately.

So if you would like to tackle this @Mayuresh0072, you would have to add the new prompt to settings (following the same approach as it's done with PIN) and update the logic on app restart.

App Restart Flow

MPB: Master password on app reset for Biometrics MPP: Master password on app reset for PIN

flowchart TD
    A[App Starts on Lock view] --> B{Is Biometrics enabled?}
    B -- Yes --> MPB{Is MPB enabled?}
    B -- No --> PE
    MPB -- Yes --> MP[Ask Master Password]
    MPB -- No --> S[Scan biometrics]
    S --> BS{Scan successful?}
    BS -- Yes --> E[Access allowed]
    BS -- No --> PE{Is PIN enabled?}
    PE -- Yes --> MPP{Is MPP enabled?}
    PE -- No --> MP
    MPP -- Yes --> MP
    MPP -- No --> P[Ask PIN]
    MP -- Valid --> E
    P -- Valid --> E

fedemkr avatar Jan 24 '23 21:01 fedemkr

@fedemkr Hey, because there is no recent contribution to this issue: Is it still open? If so, I would like to try and solve this issue.

mariemllr avatar May 16 '23 10:05 mariemllr

Hi @mariemllr thank you so much for the interest and yes, you can contribute solving this issue 😄

fedemkr avatar May 17 '23 14:05 fedemkr

Hi @fedemkr, I noticed that there hadn't been any updates regarding this issue, so I decided to take the initiative and work on it. I have just submitted a PR.

abarghoud avatar Jul 16 '23 02:07 abarghoud

Hi @abarghoud thank you so much for taking the initiative and contributing!

fedemkr avatar Jul 17 '23 13:07 fedemkr

I see that PR #2621 still haven't been touched by anyone. @abarghoud - have you met Bitwarden's Contributing Docs in order for this PR to be reviewed (and hopefully merged)? I'd love to see this feature and it's a shame it's still not on production!

komidawi avatar Feb 12 '24 20:02 komidawi

I've created this issue as my primary driver was to type in the password from time to time in order not to forget it, but I must say I don't need it anymore on iPhone. I use Bitwarden on Mac also, and I type in the password there.

That said, the change might be good anyway :-) However, I'm happy with just closing it if that's what community prefers.

mfronczyk avatar Feb 22 '24 09:02 mfronczyk

Hello @fedemkr, I was wondering if the issue is still relevant. I noticed that the PR has been there for a while without any updates. If it's still needed, I'd be happy to rebase it to resolve conflicts and move things forward.

abarghoud avatar Mar 16 '24 21:03 abarghoud