mogol / flutter_secure_storage

A Flutter plugin to store data in secure storage
https://pub.dartlang.org/packages/flutter_secure_storage
BSD 3-Clause "New" or "Revised" License
1.12k stars 367 forks source link

Getting values in separate isolate fails - Code: -25308, Message: User interaction is not allowed #743

Closed JSBmanD closed 1 month ago

JSBmanD commented 3 months ago

Hello, I've a problem with ios (17.5.1) on 9.2.1 version. Probably it's for every version as this is the keychain problem. Steps:

  1. Save values to storage
  2. Lock iphone
  3. Spawn isolate (i have my logics in isolate that is spawned when push arrives)
  4. Try to access value from storage

Error will appear PlatformException(Unexpected security result code, Code: -25308, Message: User interaction is not allowed., -25308, null)

Any workarounds to this are possible?

JSBmanD commented 3 months ago

Found this property accessibility: KeychainAccessibility.first_unlock_this_device, but it doesn't have always accessibility. I think it's a feature request now. Use case: push is received when phone is locked and just restarted (after power was drained and device is on charge). Callback in push isolate is calling backend to confirm push delivery in our system. Request doesn't have access token.

btrautmann commented 2 months ago

I'm running into a similar issue, though not from an isolate. I use flutter_background_fetch to periodically update Home Widgets. Part of this update process is to fetch new data for the user from the server (requiring their access/refresh token to be pulled from storage). This seems to fail with the error mentioned in this issue and this one (which seem similar/the same).

It's worth noting that once I return to the app, I go through the same codepath to retrieve the access/refresh tokens and the same error is thrown, so it seems the initial background read borks the storage retrieval entirely until I force-quit the app (swiping up from recents) and start the app again.

In case it helps, the retrieval code looks like:

    final accessToken = _storage.read(key: _accessTokenKey);
    final refreshToken = _storage.read(key: _refreshTokenKey);
    final expiresIn = _storage.read(key: _expiresInKey);
    final createdAt = _storage.read(key: _createdAtKey);

    final results = await Future.wait(
      [accessToken, refreshToken, expiresIn, createdAt],
    );

I am using the default iOS keychain options, but it looks like my choices are to change those options to first_unlock or first_unlock_this_device OR downgrade to 9.0.0 (not sure why this would help, but there have been reports of it helping...)?

However I read in another issue that while this may allow background access on iOS, Android doesn't support it? That seems like a dealbreaker. Is this still the case @juliansteenbakker?

nblum37 commented 2 months ago

Hi,

We observe the same issue (in our case this makes the whole app unusable). Are there any other fixes except rolling back the version? Thank you.

btrautmann commented 2 months ago

Hi,

We observe the same issue (in our case this makes the whole app unusable). Are there any other fixes except rolling back the version? Thank you.

I ended up migrating from one storage type to another (specifically first_unlock_this_device) to allow background access. Note that you actually need to migrate, you can't just change the iOS keychain access type, otherwise existing users will not be able to retrieve previously saved values.

dJani97 commented 2 months ago

I ended up migrating from one storage type to another (specifically first_unlock_this_device) to allow background access. Note that you actually need to migrate, you can't just change the iOS keychain access type, otherwise existing users will not be able to retrieve previously saved values.

Hi @btrautmann, thanks for sharing this!

By "migrate", you mean reading all the keys using one access type, and writing them again with the other on a regular app launch, from the foreground? Did you do this by creating two instances of FlutterSecureStorage, or by supplying different IOSOptions to the read/write methods?

I see that the default accessibility setting is unlocked. Changing it to first_unlock will work just as fine, or is there any reason in particular why you had to go with first_unlock_this_device?

Did you have to use synchronizable: true as well, like it's mentioned in this comment? I don't yet understand why it's needed, to me it seems contradictory to use first_unlock_this_device (which prevents migration to a new device) along with synchronizable: true (which sync to iCloud, so I guess it allows sync between devices as well?)

btrautmann commented 2 months ago

By "migrate", you mean reading all the keys using one access type, and writing them again with the other on a regular app launch, from the foreground? Did you do this by creating two instances of FlutterSecureStorage, or by supplying different IOSOptions to the read/write methods?

I personally chose to create two instances, it made more sense in my head and made the code more clear, as I referred to the initial instance as "legacy" and the new one as the standard storage. Here's some example code from my oauth2 client I wrote:

/// The inverse of [_writeTokenToStorage].
Future<ReadTokenOutcome> _readTokenFromStorage({
  // Whether to read from the legacy storage. This is used to migrate
  // tokens from the legacy storage to the new storage for iOS users.
  // By default, try to read from new storage first.
  bool useLegacyStorage = false,
}) async {
  try {
    final $storage = useLegacyStorage ? _legacyStorage : _storage;
    final accessToken = $storage.read(key: _accessTokenKey);
    final refreshToken = $storage.read(key: _refreshTokenKey);
    final expiresIn = $storage.read(key: _expiresInKey);
    final createdAt = $storage.read(key: _createdAtKey);

    final results = await Future.wait(
      [accessToken, refreshToken, expiresIn, createdAt],
    );

    final $access = results[0];
    final $refresh = results[1];
    final $expiresIn = results[2];
    final $createdAt = results[3];

    if ($access == null || $refresh == null || $expiresIn == null || $createdAt == null) {
      if (UniversalPlatform.isIOS && !useLegacyStorage) {
        // Fallback to the legacy storage for iOS users.
        return _readTokenFromStorage(useLegacyStorage: true);
      } else {
        return TokenAvailable(null, isFromLegacyStorage: useLegacyStorage);
      }
    }

    final token = YnabAccessToken(
      accessToken: $access,
      refreshToken: $refresh,
      expiresIn: int.parse($expiresIn),
      createdAt: int.parse($createdAt),
      hasError: false,
    );

    return TokenAvailable(token, isFromLegacyStorage: useLegacyStorage);
  } on PlatformException catch (e) {
    if (UniversalPlatform.isIOS && !useLegacyStorage) {
      return _readTokenFromStorage(useLegacyStorage: true);
    } else {
      return TokenUnavailable(e);
    }
  }
}

Eventually, I'll bump the app's minimum version to a version following the introduction of this migration code and remove it after a while. Note that this wasn't necessarily triggered on "app launch" but effectively was because the token is read during the normal app launch process.

I see that the default accessibility setting is unlocked. Changing it to first_unlock will work just as fine, or is there any reason in particular why you had to go with first_unlock_this_device?

Yeah, I specifically chose this option because I didn't want the token to be synced across devices, as I wasn't sure how that would work with oauth2 (e.g a refresh is made on one device and the next API call invalidates the previous token, IIRC--I didn't want a failure to sync the new token to prevent the old device from fetching, etc. Seems reasonable to have a unique token on each device, and I think the need to authenticate on each device meets user's expectations).

Did you have to use synchronizable: true as well, like it's mentioned https://github.com/mogol/flutter_secure_storage/issues/727#issuecomment-2148027881

Here's how I set up the "new" storage:

final _storage = const FlutterSecureStorage(
  aOptions: AndroidOptions(encryptedSharedPreferences: true),
  iOptions: IOSOptions(
    accessibility: KeychainAccessibility.first_unlock_this_device,
    synchronizable: true,
  ),
);

I'm not sure that I tested without it--I vaguely recall trying it and getting an error without synchronizable: true, but it's been a while.

I hope that helps @dJani97

dJani97 commented 2 months ago

Thanks @btrautmann, this is all very useful for me, because for now I can't reproduce this error - it only happens on the client's device under very specific circumstances - so it takes forever to iterate. I'll deploy a fix based on what I know so far.

By the way, this perfectly describes my case as well:

It's worth noting that once I return to the app, I go through the same codepath to retrieve the access/refresh tokens and the same error is thrown, so it seems the initial background read borks the storage retrieval entirely until I force-quit the app (swiping up from recents) and start the app again.

So I think this must be a bug with flutter_secure_storage.

juliansteenbakker commented 1 month ago

Im closing this as duplicate of #727