search

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

refactor: Cleanup optional values #734

Closed koji-1009 closed 1 month ago

koji-1009 commented 3 months ago

https://github.com/mogol/flutter_secure_storage/blob/v9.2.2/flutter_secure_storage/lib/options/apple_options.dart https://github.com/mogol/flutter_secure_storage/blob/v9.2.2/flutter_secure_storage/lib/options/ios_options.dart

In AppleOptions and IOSOptions, synchronizable and accessibility are treated as non-null values. Also, since it is defined as Map<String, String> in Dart, it can be treated as [String : String] in Swift. Because of the above, the synchronizable and accessibility properties of the methods are non-nil. Because of the above, the code will not be able to use the ? has been removed.

dballance commented 3 months ago

This SHOULD NOT be merged. In fact, it should be moved the other direction - so that accessibility can be made nil on queries. It is simply not required for reads in most cases.

koji-1009 commented 3 months ago

https://github.com/mogol/flutter_secure_storage/issues/711#issuecomment-2184934919

About the code reading I did regarding this PR.

dballance commented 2 months ago

Yes, the issue is not on writes.

Writes have had accessibility set for several years. It's the addition of kSecAttrAccessible on all the other calls that is causing an issue.

Previously, values were read with a nil value for kSecAttrAccessible. Now, it's not possible to read with nil - you MUST provide a value.

In some cases (many, I'd argue), you can read / delete without setting kSecAttrAccessible that was used to store the value.

I want to be able to READ and DELETE without regard to the kSecAttrAccessible value set for this parameter. I COULD do that before, I CANNOT do that now, and I certainly cannot do it if this PR is merged.

koji-1009 commented 2 months ago

"PlatformException(Unexpected security result code, Code: -25299, Message: The specified item already exists in the keychain. This is a problem that occurs during write. This problem appears to be an error at write time; the read and delete cases are a different problem, no?

You may be interested in the discussion of #719 where accessibility was set to nil for read and delete. #719 addresses the issue in #692. We must address multiple issues simultaneously. This PR corresponds to the code modified in #719. Therefore, once #719 is reverted, this PR will no longer be needed.

dballance commented 2 months ago

Yeah, it's not likely to be reverted - #751 should be pulled in which will make this PR moot.

juliansteenbakker commented 1 month ago

Closing this in favor of #751