EddyVerbruggen / nativescript-secure-storage

:closed_lock_with_key: NativeScript plugin for secure local storage of fi. passwords
MIT License
111 stars 26 forks source link

Provide an option to disable the fallback to NSUserDefaults on the iOS Simulator #47

Closed bearjaw closed 3 years ago

bearjaw commented 3 years ago

Hey,

Currently this plugin checks whether it's running on an iOS Simulator and then defaults to NSUserDefaults. However there's no option to disable this and it is simply hidden away in the implementation.

I suggest to provide an option to disable this check and use the keychain on the iOS Simulator as well since it can be used there and there's no real need to use NSUserDefaults on Simulators.

The change would be simple and non-breaking by letting users opt-in to disable this check.

 constructor(
    accessibilityType: string = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
    disableFallbackToUserDefaults = false
  ) {
    super();
    if (disableFallbackToUserDefaults) {
      this.isSimulator = false;
    } else {
      const isMinIOS9 = NSProcessInfo.processInfo.isOperatingSystemAtLeastVersion(
        { majorVersion: 9, minorVersion: 0, patchVersion: 0 }
      );
      if (isMinIOS9) {
        const simDeviceName = NSProcessInfo.processInfo.environment.objectForKey(
          "SIMULATOR_DEVICE_NAME"
        );
        this.isSimulator = simDeviceName !== null;
      } else {
        this.isSimulator =
          UIDevice.currentDevice.name.toLowerCase().indexOf("simulator") > -1;
      }
    }

    this.accessibilityType = accessibilityType;
  }

I open to hear your suggestions or improve upon this.

EddyVerbruggen commented 3 years ago

Please see #10 - can you confirm that problem is gone? And if so, since which version?

bearjaw commented 3 years ago

The issue you linked is not easily reproducible as it lacks information about the iOS Simulator version, iOS version, and accessibility values used.

Doing some research this seems to be related to keychain sharing which I haven't tried yet.

However simple credential storing is working fine on Simulator versions 9 though 12 (the ones I tested).

I am currently using nativescript-secure-storage version 2.6.1 and iOS 12, 13, 14.

Since this would be an opt-in change and does not change any of the methods it is still worth providing developers with the option to use the keychain on the Simulator and maybe adding a note or hint to the README.

EddyVerbruggen commented 3 years ago

I can't disagree with that and would be happy to merge a PR adding that logic.

bearjaw commented 3 years ago

I can't seem to be able to push my branch with the changes. Maybe you need to add me as a Contributor?

EddyVerbruggen commented 3 years ago

@bearjaw That's not how a PR works; fork the repo, push your changes to your fork, then go to your fork on github and press the "create pull request" button. Thanks!