iOS: Breaking Change from 9.1.x forward, causing loss of data

[dballance]

On May 8th 2024, a version of this package with BREAKING CHANGES was published as a feature release on pub.dev.

Breaking Change

The package was changed from applying kSecAttrAccessible only on SecItemAdd calls to on all calls.

Previously, this value was only applied on writes (SecItemAdd) calls, and not on any other calls.

This change causes unnecessary narrowing of the search scope when reading (and possibly removing) values, as providing kSecAttrAccessible on reads will only match against keys stored with a matching kSecAttrAccessible.

For deletes, I also believe this would cause issues, as the query is still used to search the Keychain for a matching key before removing.

Compounding Factors

There are a few compounding factors that make this very difficult for consumers of this package to deal with.

No path back

It is impossible, as of the current latest release (9.2.2), to specify nil for kSecAttrAccessible, which would return the query for reads back to it's pre 9.1.0 state. Accessibility is defaulted to KeychainAccessibility.unlocked in Dart,

To illustrate, if I stored a value before 9.1.0 with an kSecAttrAccessible value other than the default (say KeychainAccessibility.first_unlock_this_device), I could read this value by simply calling read without specifying the accessibility.

After 9.1.0, I MUST call read and specify the same accessibility used to store the value, otherwise I get null as the response.

Apple only takes kSecAttrAccessible into account on the first write

Considering kSecAttrAccessible is only considered on the first write of the value to the keychain by Apple. If users of this package changed accessibility defaults after releasing a version into the field, they would likely lose all keychain data without realizing what changed, as keys stored with the default (kSecAttrAccessibleWhenUnlocked) would potentially be trying to fetch their keys with a different IOSOptions value for accessiblity, and this would ALWAYS fail in 9.1.x forward.

What should change

kSecAttrAccessible should be opt-in for all APIs, so that developers can use much like they would with the native APIs directly.

#751 is likely the path to fixing this - but even it does not allow setting a null value for accessibility in IOSOptions on the Dart side.

Other OSS packages for keychain manipulation do NOT apply kSecAttr attributes on reads, only on writes. I would suggest that this package follow their lead.

I would encourage the package maintainers to publish a 10.x release, properly recognizing this as a breaking change. Further, I would potentially support some potential to unpublish or mark as deprecated 9.1.0, 9.1.1, 9.2.0, 9.2.1, and 9.2.2.

If I had publishing rights, I'd publish a 10.0.0 version from the 9.0.0 commit, then work forward from there to clean up the breaking change.

What can consumers do?

I would encourage all consumers of this package to pin to 9.0.0 in your pubspec files, and do not upgrade (especially don't upgrade and release an app version 😬 ) until this is resolved.

Related Changesets

This change was merged for #602 and #628 for iOS and MacOS.

Related Issues

This change caused a flurry of issues, and related changesets to fix them, without recognizing the underlying cause.

#709, #711, #720, #727 (Likely), #752, #760

Important comments from the community

The first instance of this issue being noted by the community is here: #711 (comment)

This was noted 7 days after release, and correctly identified the issue with the package.

Package Maintenance

@juliansteenbakker - If you're unable to maintain this package - can the publishing rights be given over to another that could?

[paulking86]

It's worth mentioning that due to the lack of iOS Privacy Manifest in version 9.0.0, it would be impossible to release an app on the App Store using your suggestion to downgrade. The manifest was added in 9.1.0, which is the same release which introduces the breaking change.

[dballance]

It's worth mentioning that due to the lack of iOS Privacy Manifest in version 9.0.0, it would be impossible to release an app on the App Store using your suggestion to downgrade. The manifest was added in 9.1.0, which is the same release which introduces the breaking change.

This makes it even worse. I was under the understanding that they're not aggressively enforcing the privacy manifests yet (As the warnings suddenly disappeared in April), but wouldn't surprise me.

I guess the only real path forward is a fork.

[dballance]

If anyone needs a functioning branch and doesn't want to fork themselves, add the below to your pubspec file.

This pulls in #751 and adds another changeset to make accessibility nullable on the dart side.

dependency_overrides:
  flutter_secure_storage:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678
  flutter_secure_storage_linux:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage_linux
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678
  flutter_secure_storage_macos:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage_macos
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678
  flutter_secure_storage_platform_interface:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage_platform_interface
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678
  flutter_secure_storage_web:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage_web
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678
  flutter_secure_storage_windows:
    git: 
      url: https://github.com/Tango-Tango/flutter_secure_storage.git
      path: flutter_secure_storage_windows
      ref: 400f6b4eb6ebaea6936fdbaa0d2f16ef36576678

[juliansteenbakker]

I would like to apologize for my disappearance the pas months, i have however checked and merged all outstanding PR's, and will release a new version after some testing tomorrow. I think this will fix all issues mentioned in this issue, since most of the code from the fork is merged into the main repository.

[faaqi]

@juliansteenbakker when will the new version gonna release? Facing this as well

[gonzalogauto]

Any updates on this?

[mark8044]

@juliansteenbakker I think we need a fix, the problem seems to be increasing for whatever reason

[eldarkk]

Guys any workarounds to fix this ?

Except downgrading to 8.0.0

[dballance]

There is also now a dep issue with win32 that should be fixed, as the package is unusable in it's current state in the latest released Flutter version.

Would encourage a release of 10.x ASAP, including the win32 fix. There is a PR open already: #780

[mark8044]

@dballance Is there any suggested way forward at this point? seems like alot of things are breaking all at once

[dballance]

The only suggestion I have for now is to ship your own fork. There could be some community support for shipping a fork to pub.dev.

[dballance]

I have updated our fork to pull in the win32 change, just update the ref in #762 (comment) to:
c54719f367d836d6f643176296468e9c0a70157e

[dballance]

There is now another blocker potentially for some, since this package (flutter_secure_storage_web) pins web versions to <1.0.0.

web 1.0.0 was released 2 months ago, and other packages have upgraded, but because this pins <1.0.0 it will potentially clash with other packages that have upgraded and prevent resolving deps.

This is also mitigated in the fork we're using, with hash: 92cd044dbad231e52902568d3b5ae1a5e470bb0a

[amrLLSE]

Any updates on this?

[doshpin]

the only solution for me is setting exactly version 9.0.0:

  flutter_secure_storage: 9.0.0

[acacioveit]

@juliansteenbakker it passed 2 months since your last comment about it, do you have any news about it? This package is used a lot from the flutter community, we need assistance!