Skip to content
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

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

Open
1 task
mfronczyk opened this issue Aug 8, 2022 · 11 comments · May be fixed by #2621

Comments

@mfronczyk
Copy link

mfronczyk commented Aug 8, 2022

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 mfronczyk added the bug label Aug 8, 2022
@mfronczyk mfronczyk changed the title Requiring providing master password on application restart is not respected Requiring providing master password on application restart is not respected when a pin is set Aug 8, 2022
@mfronczyk
Copy link
Author

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 mfronczyk changed the title Requiring providing master password on application restart is not respected when a pin is set Requiring providing master password on application restart is not respected when a pin is set together with biometrics unlock Aug 8, 2022
@Mayuresh0072
Copy link

Mayuresh0072 commented Jan 21, 2023

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

@mfronczyk
Copy link
Author

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.

@fedemkr
Copy link
Member

fedemkr commented Jan 24, 2023

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

@mariemllr
Copy link

@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.

@fedemkr
Copy link
Member

fedemkr commented May 17, 2023

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

@abarghoud
Copy link

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.

@fedemkr
Copy link
Member

fedemkr commented Jul 17, 2023

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

@komidawi
Copy link

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!

@mfronczyk
Copy link
Author

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.

@abarghoud
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants