clients
clients copied to clipboard
[PS-275] Allow auto-filling TOTP codes
Type of change
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
This PR allows Bitwarden to auto-fill TOTP fields in a login form, reducing hassle from having to click on another input box and paste in the code.
Most websites that have a TOTP requirement in the sign-in process do so on another page, while some have a box on the page alongside the username and password. While this change works for both scenarios, currently you would need to auto-fill twice if there is a separate TOTP page (including sites like GitHub, AWS, Google, Cloudflare, and most other big-names). However, if the "Enable Auto-fill on Page Load" option is enabled, the TOTP will be automatically filled after being redirected to the TOTP page.
Code changes
src/services/autofill.service.ts:
- Added a new array of TotpFieldNames
- Switched several synchronous methods to
asyncin order to facilitate generating the TOTP code (which must be done asynchronously) - Added a new
findTotpFieldfunction, which uses similar logic to the existingfindUsernameField
Testing requirements
- Ensure TOTP autofill works on a range of websites
Websites tested
- [x] AWS
- [x] Cloudflare
- [x] GitHub
- [x] Steam
Before you submit
- [x] I have checked for linting errors (
npm run lint) (required) - [ ] This change requires a documentation update (notify the documentation team)
- [ ] This change has particular deployment requirements (notify the DevOps team)
Agreed, or maybe allow us to use variables in custom fields so we could reference the totp code as a variable and have it autofill that way. This would be more reliable in the event it cannot find the totp field.
One small thing: there is actully autocomplete="one-time-code" attribute for such cases. https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete
Could you please all try to find the filed with the attribute as well?
@infodusha good idea! I haven't had time to work on this in a while, but I'm going to try to find some time to finish this up in the next couple weeks.
I think it would also make sense to have a keyboard shortcut to copy the TOTP code to the clipboard, so on websites the autofill might not work, you can still quickly access your code.
This should now be ready for review!
Agreed, or maybe allow us to use variables in custom fields so we could reference the totp code as a variable and have it autofill that way. This would be more reliable in the event it cannot find the totp field.
Variables in custom fields would be great, but probably fall out of the scope of this PR. Would love to start working on something like this next, though :)
Thank you @andrewda! I've added this to our internal board for review by the team.
This is great! Is the totp input field determined by the name or id attribute?
I would like to add support for Google Services, which would probably be totpPin.
Hey @djsmith85, do you have an update on this before I go ahead and resolve the conflicts?
This would be a great addition!
Is this still happening? Would love to have autofill for TOTP from the browser. Thanks!
Conflicts resolved, so this should be ready for review again :+1:
Any updates on the time horizon for this feature?
I was certainly surprised to discover that this was not a feature when I recently put in my first TOTP code. Hoping for this to be added soon :) though autofill for pass + paste TOTP is not so bad.
Hey @djsmith85, just another ping on this when you have some time! Please let me know if there's anything else I can do to make your reviewing easier.
Really hope this gets merged. Andrew has put a lot of work into this PR.
Several people have said a big feature lacking from Bitwarden present in other services is the TOTP auto-fill, here's one example https://community.bitwarden.com/t/please-add-option-to-autofill-totp-code-while-autofill-option-on/33526/17
You can't officially use an unsigned extension on the normal Firefox build either, which is why a functionality like this should be added into the public extension.
If for whatever reason the team dislikes Andrew's method, another one could be adding TOTP code as an option to custom fields.
@djsmith85 any updates on this?
@gbubemismith please let us know if there's been any updates
Very interesting feature! Is there a reason why it was not merged?
Extremely useful feature! When it's going to be merged??
@vincentsalucci please let us know if there's been any updates
Very interesting feature! Is there a reason why it was not merged? Extremely useful feature! When it's going to be merged??
We're not sure, we need the Bitwarden team to be more communicative about this.
@trmartin4 please let us know if there's been any updates
@wnelson03 @andrewda
Hello, I'm a Front End Engineer with Bitwarden. Todd asked me to take a look at this community PR and help identify a path forward for merging the work done by Andrew.
In reviewing the history of this PR, and the associated ticket we have in Jira, I saw that @djsmith85 had identified a significant flaw where autofill of the TOTP code could potentially lock a user out of an account. The scenario in which this would happen can be seen within Github's login workflow.
If a user turns on "Auto-fill on page load" within the extension, the TOTP value for a vault item will be autofilled anytime a MFA field is found on page load. Github's login workflow initially has the user authenticate using their username/password, then redirects the user to a separate page to insert their TOTP. Github automatically submits the TOTP value on change of the input element, so as a result Bitwarden's autofill logic triggers a submission of the inserted TOTP value immediately.
If this TOTP value is invalid for whatever reason, Github refreshes the MFA page and notifies the user that the MFA attempt failed. Of course, on page refresh this triggers Bitwarden's extension to re-attempt an autofill of the MFA field that is loaded. This causes a loop until Github temporarily locks the account for failing MFA repeatedly.
I've looked into a way to handle storing recently inserted TOTP values using a Set and checking if that value has recently been inserted before attempting an autofill of the TOTP. This works but it's a bit of a hacky solution, and currently we're discussing if there's a better approach for resolving this issue. I'll reach out soon with a suggestion on how best to approach resolving the problem.
thank you very much for the update. I feel like someone would close the tab before the extension can attempt it too much, though I see that's not an ideal situation.
giving the user control though to enable it as an optional feature is a great thing. that's what's so awesome about Bitwarden, I see so many controls such as KDF iterations count that Bitwarden allows its users to customize while other platforms don't "for your safety". That freedom is a great thing.
appreciate you giving us an update, wish there was more of a Trello board or GitHub projects style page for us users to see what's going on live with suggestions, bugs, etc.
Of course, on page refresh this triggers Bitwarden's extension to re-attempt an autofill of the MFA field that is loaded. This causes a loop until Github temporarily locks the account for failing MFA repeatedly.
Wouldn't this work fine though? I just tested and GitHub doesn't seem to have a rate limit for entering a 2FA code right after an invalid one is entered. So, if the page is refreshed, wouldn't the extension load the up-to-date 2FA code which would succeed when checked? I'm not seeing the problem exactly
Unless the stored TOTP secret was completely invalid, which must be an edge case, it should work fine on the 2nd attempt if the only problem was that the TOTP time ran out before the form was submitted.
@wnelson03
Unless the stored TOTP secret was completely invalid...
Yeah, this is the scenario that I found was causing issues. You're right in saying that if for some reason the autofill misses a timing window on the TOTP, that on refresh of the page the next autofill SHOULD attempt the subsequent timing window's TOTP value. In that case, it's less of a problem.
There are some other thoughts I had yesterday after work regarding this feature that bring to light security concerns with autofilling on page load in such a manner. I'll be discussing those today with the team, hoping to provide some further feedback and code change suggestions today before EOD.
Thanks for the feedback! I'll echo what @wnelson03 mentioned previously and say it would have been great for those internal Jira discussions to have been done more publicly, or at least had a little more visibility into the review process. Either way, I'm happy things are moving along now and I appreciate your efforts!
Autofilling vault items on page load already introduces concerns with regards to security, as noted by the warnings for the setting provided in our browser extension and documentation. Allowing TOTP to autofill on page load exacerbates the concerns brought forth with autofilling vault items on load and, under the right circumstances, completely removes the benefits of having MFA in place.
I fully agree with the issues brought on by autofilling on page load, and I agree that doing so isn't really necessary for TOTP. I have been using a manual build of Bitwarden since submitting this PR, and have only used the Ctrl-Shift-L keyboard shortcut to fill credentials + TOTP (though I think the BEST UX would be some kind of input popup).
I've merged your suggestions and will give it some real world testing throughout the week. Please let me know if there are any other changes you'd like to see! :slightly_smiling_face:
@andrewda
Thank you once again for your contribution! QA has tested and approved your changes. We will be merging this work into the Browser v2023.7.0 release, and the updated behavior should be present once the browser extension has been updated to that release.
@cagonzalezcs I don't see TOTP autofill in the changelog for v2023.7.0 on the apple app store etc
is this PR in for the v2023.7.0 release?
@cagonzalezcs I don't see TOTP autofill in the changelog for v2023.7.0 on the apple app store etc
is this PR in for the v2023.7.0 release?
He said Browser v2023.7.0 so it likely doesn't apply to mobile at the moment.
The mobile apps are in fact a different repository, so yeah this PR will have no bearing on mobile. You're less likely to have important data in your clipboard on mobile though.