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

Add support for biometrics to Safari #1775

Merged
merged 3 commits into from Apr 7, 2021
Merged

Conversation

Hinton
Copy link
Member

@Hinton Hinton commented Apr 7, 2021

Objective

Adds support for browser biometrics to Safari. It was previously omitted due to needing some special logic to function correctly and we still used the old app extension.

Biometrics in Safari works slightly differently in that they do not require the desktop companion app to be running. (It's still necessary to enable biometrics inside the desktop app)

Code Changes

  • src/background/main.background.ts: Inject platformUtilService to NativeMessagingBackground.
  • src/background/nativeMessaging.background.ts: Add support for the slightly different Safari flow.
  • src/safari/desktop.xcodeproj/project.pbxproj: Automatic Xcode reformat, figured we might as well commit it since it keeps reformatting the file.
  • src/safari/safari/SafariWebExtensionHandler.swift: Native logic required for biometrics and retrieving the keychain secret.
  • src/services/browserPlatformUtils.service.ts: Remove Safari exception to supportsBiometrics.

Testing Considerations

We need to test the biometrics on Safari on machines without TouchId (should fail with no support error), on machines with biometrics disabled in desktop app. With biometrics enabled, and lastly logging out of desktop and logging into another account (should fail with wrong user error). We should also do some quick regression testing on other browsers to ensure nothing broke.

@Hinton Hinton requested a review from a team April 7, 2021 11:34
@Hinton Hinton merged commit ae4c1b2 into master Apr 7, 2021
@Hinton Hinton deleted the feature/biometrics-safari branch April 7, 2021 15:40
@armarti
Copy link

armarti commented May 2, 2021

works great!

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

Successfully merging this pull request may close these issues.

None yet

3 participants