[PM-2609] Allow auto-filling TOTP codes

[andrewda]

Type of change

• Bug fix
• 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 async in order to facilitate generating the TOTP code (which must be done asynchronously)
• Added a new findTotpField function, which uses similar logic to the existing findUsernameField

Testing requirements

• Ensure TOTP autofill works on a range of websites

Websites tested

• AWS
• Cloudflare
• GitHub
• Steam

Before you submit

• 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)

[CLAassistant]

All committers have signed the CLA.

[wnelson03]

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.

[infodusha]

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?

[andrewda]

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

[fischer-felix]

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.

[andrewda]

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 :)

[djsmith85]

Thank you @andrewda! I've added this to our internal board for review by the team.

[kimdre]

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.

[andrewda]

Hey @djsmith85, do you have an update on this before I go ahead and resolve the conflicts?

[vniehues]

This would be a great addition!

[jschwalbe]

Is this still happening? Would love to have autofill for TOTP from the browser. Thanks!

[andrewda]

Conflicts resolved, so this should be ready for review again 👍

[cagonzalezcs]

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

[wnelson03]

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.

[wnelson03]

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.

[cagonzalezcs]

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

[cagonzalezcs]

@andrewda

Alright, so I spoke with several members of the engineering team as well as product leadership to determine a path forward for merging your work. We have a number of concerns with allowing TOTP values to autofill on page load, and as a result are requesting that you modify your implementation to only handle autofill of TOTP values when a distinct user action attempts to autofill a form. I worked through some suggested changes which can be seen on the branch comparison below:

andrewda/browser@totp-autofill...bitwarden:clients:totp-autofill-remove-onload-suggestions

Our reasoning behind suggesting this functionality change is as follows:

• 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.
• Removing the autofill on page load functionality also resolves the issues identified by @djsmith85 with regard to potentially locking users out of their account if MFA fails on an auth workflow that reloads the page to allow a re-input of the MFA code. There are other ways to mitigate this behavior rather than removing the autofill on page load feature entirely, but our concerns with security put us in the position of holding off on moving forward in that direction.

Please let me know if you have any questions or concerns. I'll be keeping a watch on this PR for updates, and will work to help get this into our QA testing process once the requested modifications are added.

[andrewda]

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! 🙂

[cagonzalezcs]

@andrewda

Thank you for addressing those code changes. I've pulled your work locally and verified that the implementation requires a distinct user action to trigger a TOTP autofill.

I'm going to go ahead and move this set of code changes to the QA team for their review.

I do want to apologize for the issues experienced with communication from our side. I'll make sure to discuss the concerns brought up in this PR with Todd and other team members, and moving forward we will try to be more direct about our communication with contributors.

Thanks again for your contribution.

[cagonzalezcs]

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

[git-anish]

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

[wnelson03]

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

[git-anish]

Sorry I should have specified the Mac App Store*

The MacOS Safari browser extension is shared through the Mac app store

Screenshot
Appstore link showing 2023.7.0 release

[cagonzalezcs]

@git-anish

I believe that the exclusion of the feature in the Safari browser extension release notes is a typo. The changes for autofilling TOTP appear to be present and functional when updating the Safari extension to 2023.7.0.

[git-anish]

I can confirm it works 🙂

How can the community assist in adding more TOTP fields/sites that would be supported under this feature?

the verification codes being autofilled is hit and miss at the moment

It definitely works on the websites tested for this PR but not for others, ex. Fidelity etc

is there a change I can make locally to make autofilling TOTPs possible for incompatible sites?

[andrewda]

@git-anish This would probably be a good entry point for adding additional support:

https://github.com/andrewda/browser/blob/a8b4e6a036e2c26af6ba8b051ca485fc4a3989b5/apps/browser/src/autofill/services/autofill-constants.ts#L23

At the moment, there's no way to add additional supported fields through the UI, but that would be an excellent addition.

[wnelson03]

At the moment, there's no way to add additional supported fields through the UI, but that would be an excellent addition.

I believe the best way to implement this would be to add more variables in the custom fields, so people could reference things such as TOTP code