IosCredentials is missing trait methods in v3

[tmpfs]

Showing All Messages
SEVERE:   --> /Users/muji/.cargo/registry/src/index.crates.io-6f17d22bba15001f/keyring-3.0.1/src/ios.rs:38:1
SEVERE:    |
SEVERE: 38 | impl CredentialApi for IosCredential {
SEVERE:    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `set_secret`, `get_secret` in implementation
SEVERE:    |
SEVERE:   ::: /Users/muji/.cargo/registry/src/index.crates.io-6f17d22bba15001f/keyring-3.0.1/src/credential.rs:24:5
SEVERE:    |
SEVERE: 24 |     fn set_secret(&self, password: &[u8]) -> Result<()>;
SEVERE:    |     ---------------------------------------------------- `set_secret` from trait
SEVERE: ...
SEVERE: 34 |     fn get_secret(&self) -> Result<Vec<u8>>;
SEVERE:    |     ---------------------------------------- `get_secret` from trait
SEVERE: 
SEVERE: For more information about this error, try `rustc --explain E0046`.

[brotskydotcom]

Wow! How did I miss that?! Thanks for the report. I'll get this fixed right away. Are you actually using this library on iOS?

[tmpfs]

Yes, I am, getting a PR ready now if you want it.

[tmpfs]

We can just duplicate the MacOS impl as it's all security framework under the hood anyhow.

[brotskydotcom]

Sure, I'd love that! And yes it should be the same as the MacOS code.

[tmpfs]

Except we also need to add (or expose) a get_keychain() impl but the existing one takes MacCredential - can I use MacCredential on iOS too?

[tmpfs]

Hmm, I feel like there should be only one apple module and then we can do conditional compilation on the security_framework imports?

[tmpfs]

Nope, that won't work as the keychain is only exposed on macos. Looking at the security_framework docs now...

[brotskydotcom]

Taking a look now, I was the one who added the calls to the security_framework for iOS so everything we need should be there. Probably you just need to do the same mod that I did on MacOS, which is to make get_password and set_password call get_secret and set_secret, doing the conversions in the wrapper. Then the current code for get_password and set_password become the code for get_secret MINUS the conversions.

[tmpfs]

Yep, just avoid the string conversion is fine, doing it now 👍

[brotskydotcom]

Thanks for the PR. See review comments, it's almost ready to go.

[brotskydotcom]

@tmpfs Have you successfully built an iOS app based on the new master? I can't get it to work in my normal test harness due to a missing symbol on iOS inserted by the security framework. It looks like there's been some decay in the security framework related to builds on iOS not being tested thoroughly. I don't want to release this new build until I'm sure it actually works on iOS.

[tmpfs]

Hey, yeh I use patch.crates-io to workaround the security framework problem.

See kornelski/rust-security-framework#203

Would be helpful if you can release anyhow as it's an upstream issue but I understand if you want to wait.

[brotskydotcom]

Thanks much for pointing me at that bug and the workaround! I can see what the issue is in my keyring iOS testing that's provoking the issue: I'm testing for an empty password but you can't do that on iOS (right now, due to a bug in security-framework). I'll see if I can resolve the testing issue in the keyring-rs package and, if so, put in an advisory about needing to use security-framework 2.10 for now. I'll probably also force that dependency in the Cargo.toml for keyring-rs for iOS.