SubmarinerApp / Submariner

A Subsonic client for macOS
https://submarinerapp.com
BSD 3-Clause "New" or "Revised" License
123 stars 3 forks source link

Password not stored in Keychain #125

Closed pierrec18 closed 1 year ago

pierrec18 commented 1 year ago

Hi all.

When I add a server (on Navidrome by the way) in Submariner, I can access my music. After closing and re-opening the application, it gives me a "Wrong username/password (code 40)" error. After some research, I believe the application is unable to store the password in the keychain.

I have tried uninstalling / reinstalling / restarting / whatever and nothing works. Can you help me? I don't have a problem with the application, but I can't use it.

Some useful information :

NattyNarwhal commented 1 year ago
pierrec18 commented 1 year ago

Hi

I don't know if it can help but my server have an address is https://subdomain.domain.me but I also try with the local IP and the result is still the same.

NattyNarwhal commented 1 year ago

Do you see anything in Console.app related to Keychain when you try saving the password?

If you have Xcode installed and don't mind building from source, try the latest commit. The Keychain handling code was rewritten in Swift, but should be functionally/logically identical. Put breakpoints on all the Keychain functions (fetch, adding, updating, updating), and try updating your password again.

pierrec18 commented 1 year ago

Here are the few lines about the Submariner and the keychain. I imagine this is the error, but I have no idea how to fix it. log.txt

NattyNarwhal commented 1 year ago

This is pretty weird looking. I think the class genp is a hint though - I noticed in the older version, I mixed up generic password with internet password, which I fixed in the latest code in Git. That might be a red herring though - if it was a mix-up, I'd expect it to be in Keychain but not able to find it the next time? Worst case, I'd recommend trying the latest from Git, either build it yourself, or try the (unsigned, so you'll need to right-click->open) CI-built one here.

FWIW, years ago, I did deal with a corrupt keychain database that prevented me from making an account in Mail.app. I wonder if this is another case of corruption. The solution for it was to reset the login and local keychains from Keychain Access. It is annoying because you'll lose the saved passwords in it and have to sign into i.e. iCloud again, but it did work for me.

NattyNarwhal commented 1 year ago

Oh, there's the error being thrown in Keychain. I suspect it is a mix-up between generic/internet passwords. path is an attribute of an internet password, but not a generic one. Trying the latest build might solve the issue if that's the case.

pierrec18 commented 1 year ago

I am trying your build but it crashes when I try to save my server and I do not have Xcode to try to compile it myself 😕 log2.txt

NattyNarwhal commented 1 year ago

Could you try this build? It's of the latest code from Git plus additional logging for Keychain. Submariner-2.2-KeychainTest.zip

pierrec18 commented 1 year ago

I'm sorry but the app is still crashing when I try to save the server.

Submariner-2023-05-01-213655.ips.zip

NattyNarwhal commented 1 year ago

I think it might be a null object made when it reconstructs the URL here, causing a crash on the next line. I've attached another build that adds some logging there. Can you show the console output for this build? It'll log what the value of the string and the constructed NSURL will be. Submariner-2.2-KeychainTest2.zip

pierrec18 commented 1 year ago

So, it works somehow 😀 I can create my server, but it has a "null" name (see log3.txt). Then I edited the server to give it a name. It still has "null" but I can access my music. Finally, I changed the name of the server in the SQLite and everything is OK and I see the password in the keychain!

NattyNarwhal commented 1 year ago

I've found and identified the crash.

The null resource name is a different regression introduced in the rewrite of that class to Swift. The setter for resourceName assumed there was always a previous resource name. I'm going to push a fix for these issues, and probably cut 2.2 soon.

Thanks for helping identify this!