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

iOS AutoFill Functionality #133

Merged
merged 15 commits into from
Jan 17, 2019
Merged

Conversation

se1exin
Copy link
Member

@se1exin se1exin commented Jan 14, 2019

This PR introduces native AutoFill functionality for iOS, as discussed in #125

Overview

  • Users must manually enable AutoFill and Buttercup in the Passwords section of iOS Settings.
  • Buttercup will replicate user credentials into the iOS AutoFill credential store everytime an Archive is opened/updated. iOS will make these available directly in the QuickBar above the keyboard for fastest possible login.
  • The user may enter Buttercup via the AutoFill functionality to further search for credentials.
  • Buttercup AutoFill will attempt to automatically unlock all touch enabled Archives on load.
  • Non-touch enabled Archives can still be accessed via manual master password entry.
  • The user can only access the Archive List and Search page. Tapping an Archive goes to the Search page with Search restricted to that Archive.
  • Search Results mimic the Buttercup Browser Extension behaviour. With domain related results shown initially and when search has not matching results.
  • Tapping a credential will complete the autofill process.
  • The user can cancel the process at any time via the 'Cancel' button in header right on both pages.

Significant changes

  • AsyncStorage replaced with react-native-secure-storage. Includes auto-migration for existing users in base App component.
  • Additional RN Entry Point created - index.ios.autofill.js.
  • Some top-level containers/components replicated to remove unneeded/incompatible functionality. See source/autofill/**
  • Additional Redux Store - autofill. Used to flag/check if the app is running in the context of the AutoFill extension
  • Update to the Xcode Project entitlements. May need some updates in the Apple Dev Account, but Automatic Signing in Xcode should take care of it.

I've tested quite a lot but there may still be bugs, would great to have some other people test it out.

@perry-mitchell
Copy link
Member

omfg

@perry-mitchell
Copy link
Member

@se1exin This is incredible! Thanks so much for pushing this through. Amazing to see a PR for it already.

By the way, do you know if the release process changes much? Do we need to release the extension separately or does it just bundle?

@sallar
Copy link
Member

sallar commented Jan 14, 2019

@se1exin thanks a lot! extraordinary job :)
this will take a while to review though haha

Copy link
Member

@perry-mitchell perry-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Huge item to go over but definitely well done. I can't immediately see any issues. Excellent stuff!

<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleVersion</key>
<string>1</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so we need to version this separately? I wonder if there's a way we can tie these to the main project's versions using macros.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build script added to BCAutoFillExtension target that sync's the Extension Build numbers with the main app's. See commit 908fdcf

@@ -98,7 +99,8 @@ export function getArchiveEncryptedContent(archive, credentials) {

export function getSharedArchiveManager() {
if (__sharedManager === null) {
__sharedManager = new ArchiveManager(new AsyncStorageInterface());
// __sharedManager = new ArchiveManager(new AsyncStorageInterface());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 908fdcf

@@ -6,7 +6,8 @@ import {
Credentials,
entryFacade
} from "./buttercupCore.js";
import AsyncStorageInterface from "../compat/AsyncStorageInterface.js";
// import AsyncStorageInterface from "../compat/AsyncStorageInterface.js";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can also be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 908fdcf

@perry-mitchell
Copy link
Member

We'll probably merge this to a mid-way branch for final testing before going to master. We should also endeavour to strip the Facebook comments form all files (new and old): #134

…er with main app. Update all iOS Copyright notices from Facebook to Buttercup (buttercup#134). Remove commented code missed in last cleanup commits
@se1exin
Copy link
Member Author

se1exin commented Jan 14, 2019

Thanks for the positive feedback @perry-mitchell and @sallar!

It turned out to be a bit bigger of a job than I first expected, but much learnt about iOS AuthenticationFramework along the way 😅. Let me know if you need any changes or explanations for anything 👍

Do we need to release the extension separately or does it just bundle?

It just bundles from the main app's build process, so you shouldn't have to change the release process at all. In 908fdcf I've also added a Build Script in the Extension Build Phases that ensures the Extension version numbers are the same as the main app. It's rumoured on Stack Overflow that Apple actually requires these to be the same for review.

We should also endeavour to strip the Facebook comments form all files (new and old)

I've attempted to do this for the iOS code files in 908fdcf. Wasn't sure what the Copyright should be changed to so just went with Buttercup and added reference to GPLv3 where Facebook had previously mentioned MIT. Also updated the organisation in the Xcode project so that future autogenerated copyright comments say Buttercup instead of Facebook 👍.

@perry-mitchell perry-mitchell changed the base branch from master to ios_autofill January 17, 2019 18:26
@perry-mitchell perry-mitchell self-assigned this Jan 17, 2019
@perry-mitchell
Copy link
Member

Thanks @se1exin! I appreciate the trouble you went to to make this easy to deploy. I'll merge now into an intermediary branch for some testing before we go to master.

@perry-mitchell perry-mitchell merged commit 1a70b22 into buttercup:ios_autofill Jan 17, 2019
@perry-mitchell perry-mitchell mentioned this pull request Jan 17, 2019
@se1exin
Copy link
Member Author

se1exin commented Jan 17, 2019

@perry-mitchell Awesome! Looking forward to having this in the official build so I can start using it on my day-to-day phone!

@se1exin se1exin deleted the issue/125 branch January 29, 2019 08:12
@se1exin se1exin mentioned this pull request Feb 9, 2019
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.

3 participants