OmerFlame / SwiftRant

devRant API library made in Swift.
GNU General Public License v3.0
3 stars 1 forks source link

Token not stored in keychain #1

Open WilhelmOks opened 2 years ago

WilhelmOks commented 2 years ago

After a successful login, I get the message:

STORING USERNAME AND PASSWORD TO KEYCHAIN NOT SUCCESSFUL DID SUCCEED IN WRITING TOKEN TO KEYCHAIN: false

And the token is indeed not stored. SwiftRant.shared.tokenFromKeychain returns nil.

Am I missing something?

WilhelmOks commented 2 years ago

Found the fix: Keychain sharing with the particular group "SwiftRantAccessGroup" must be enabled:

Bildschirmfoto 2022-08-24 um 21 48 42

This seems like an unintentional requirement for me. If it is intentional, you might want to document it :)

OmerFlame commented 2 years ago

Oh boy! I was so focused on making sure that it works and forgot to document the whole access group... Yes, the group is intentional and needs to be enabled. Thank you so much for bringing this to my attention! I will document this as soon as possible.

Also, thank you for using my library! I hope you have a good, easy time using it, I put quite a lot of work into properly documenting everything to a tee.

WilhelmOks commented 2 years ago

Hey, thank you for the effort. I was pleasantly surprised when I saw that you have separated the API stuff into this SwiftRant project from your AltRant project.

This will save me a lot of time and work. I’m currently playing around with a basic SwiftUI devRant app.

Can you explain to me why the keychain sharing and the group is needed?

OmerFlame commented 2 years ago

I had no idea on how Keychain APIs work while implementing them and just kept going until it worked. IIRC, I wanted to make sure that nothing else other than SwiftRant can touch SwiftRant Keychain items.

I don't have enough knowledge on implementing Keychain stuff, so if you have any improvements you can contribute to the topic, I encourage you to make a pull request with working code that doesn't require the group and I will push it after testing!

WilhelmOks commented 2 years ago

Alright. I have worked with keychains before and I don’t remember that I needed to do this sharing and group stuff. But I’m also not very experienced with keychains. I think the sharing is needed if you want different apps to share the credentials. So, you log into one app and you are then also logged in in another app, if they use the same group. It should be some setting in the keychain wrapper lib, but I’ll need to dig into it a bit deeper to know what to do.

WilhelmOks commented 2 years ago

IIRC, I wanted to make sure that nothing else other than SwiftRant can touch SwiftRant Keychain items.

The keychain is already only accessible from the app that is using SwiftRant.

I think sharing and groups is for letting other apps access this keychain, too.

OmerFlame commented 2 years ago

Alright, so we just need to get rid of the access group and it will stay private to the app.

WilhelmOks commented 2 years ago

I've looked into the SwiftRant project and I can't find any clue of where this requirement for a Group comes from. I'm confused right now. So, I can't explain why projects that are using SwiftRant as a dependency, need to use a Group.

WilhelmOks commented 2 years ago

I did a few tests: Apparently, it doesn't matter what the acual group name is. I've changed it and it still works. But when I remove the group (but keep the keychain sharing enabled), then I get the original error message again and it doesnt work: STORING USERNAME AND PASSWORD TO KEYCHAIN NOT SUCCESSFUL

Still confused about that.