Support multiple authentication methods

[fmeum]

📢 Type of change

• Bugfix
• New feature
• Enhancement
• Refactoring

📜 Description

Add support for multiple authentication methods by offering password authentication after public key authentication.

Along the way, refactor GitOperation to use a (somewhat) proper builder
pattern. This uncovered that we were previously not using SSHJ for the
case of no authentication, which is now rectified.

💡 Motivation and Context

Fixes #333, fixes #548 and fixes #782 if it works.

💚 How did you test it?

I lack the setup to test this properly and only verified that SSH public key authentication by itself still works as expected. Password authentication, public key followed by password authentication and no authentication still need testing.

📝 Checklist

• I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
• I reviewed submitted code

🔮 Next steps

📸 Screenshots / GIFs

[msfjarvis]

I can't get this to work for me. After setting AuthenticationMethods publickey,password in my sshd_config, trying to clone my repository with the SSH Key connection mode fails here

https://github.com/android-password-store/Android-Password-Store/pull/825/files#diff-efe0564ca4779571d8e3340b0d88a26dR43 Ugh, didn't work. GitOperation.kt, line 43.

[fmeum]

I can't get this to work for me. After setting AuthenticationMethods publickey,password in my sshd_config, trying to clone my repository with the SSH Key connection mode fails here

https://github.com/android-password-store/Android-Password-Store/pull/825/files#diff-efe0564ca4779571d8e3340b0d88a26dR43 Ugh, didn't work. GitOperation.kt, line 43.

Sorry, that require shouldn't have made it in in that form: #826.

[msfjarvis]

Doesn't throw anymore but still doesn't seem to work, tried both my SSH key as well as password and SSHJ always fails saying all authentication methods were exhausted. I'll try to debug some more on my end...

[msfjarvis]

uhh

[msfjarvis]

@FabianHenneke can you rebase this and #807 when you get a chance? I have the weekend off so I'm going to try and create a docker setup to allow me to test this.

[fmeum]

@FabianHenneke can you rebase this and #807 when you get a chance? I have the weekend off so I'm going to try and create a docker setup to allow me to test this.

Done, but EDCSA keys remain broken for now. Everything else should work as before.

[msfjarvis]

@FabianHenneke would you be able to rebase this some time over the next week? Should leave us enough time to be able to review and test this before the September release.

[fmeum]

@FabianHenneke would you be able to rebase this some time over the next week? Should leave us enough time to be able to review and test this before the September release.

Please review and merge #1084 first.
We are currently offering password authentication implicitly after publickey authentication in both possible cases: As a follow-up authentication mode when publickey auth succeeded and as an alternative authentication mode when it failed. I don't think there is an easy way to differntiate between the two without delving into SSHJ internals, so we should first agree on whether this behavior is okay or not.

[msfjarvis]

@FabianHenneke would you be able to rebase this some time over the next week? Should leave us enough time to be able to review and test this before the September release.

Please review and merge #1084 first.
We are currently offering password authentication implicitly after publickey authentication in both possible cases: As a follow-up authentication mode when publickey auth succeeded and as an alternative authentication mode when it failed. I don't think there is an easy way to differntiate between the two without delving into SSHJ internals, so we should first agree on whether this behavior is okay or not.

I think it's acceptable. I assume we'd only be offering password authentication for servers that do advertise password or keyboard-interactive authentication methods alongside publickey?

[fmeum]

@FabianHenneke would you be able to rebase this some time over the next week? Should leave us enough time to be able to review and test this before the September release.

Please review and merge #1084 first.
We are currently offering password authentication implicitly after publickey authentication in both possible cases: As a follow-up authentication mode when publickey auth succeeded and as an alternative authentication mode when it failed. I don't think there is an easy way to differntiate between the two without delving into SSHJ internals, so we should first agree on whether this behavior is okay or not.

I think it's acceptable. I assume we'd only be offering password authentication for servers that do advertise password or keyboard-interactive authentication methods alongside publickey?

That should be true based on my understanding of the SSHJ code, but I don't have the setup to test it properly.

[fmeum]

Maybe we should rethink our AuthMode switches though. If we could make the switches shown depend on whether or not the URL starts with http:// or https://, we could show either a choice between "Password" and "None" (HTTP) or "Public key", "Public key (via OpenKeychain)" or "Password only" (SSH).

[msfjarvis]

Maybe we should rethink our AuthMode switches though. If we could make the switches shown depend on whether or not the URL starts with http:// or https://, we could show either a choice between "Password" and "None" (HTTP) or "Public key", "Public key (via OpenKeychain)" or "Password only" (SSH).

Should certainly not be impossible, I can take a stab at it. Do you want it to go in with this PR?

[fmeum]

Maybe we should rethink our AuthMode switches though. If we could make the switches shown depend on whether or not the URL starts with http:// or https://, we could show either a choice between "Password" and "None" (HTTP) or "Public key", "Public key (via OpenKeychain)" or "Password only" (SSH).

Should certainly not be impossible, I can take a stab at it. Do you want it to go in with this PR?

I would prefer to have this go in as is. The other change would be almost purely UI-related while this does not touch the UI at all, so I would prefer them to remain separate.

[msfjarvis]

I tried cloning from a server which only had password authentication after setting authentication mode to SSH key in the app and did not get prompted for a password, but from my understanding of the changes here, that shouldn't have been the case?

[fmeum]

I tried cloning from a server which only had password authentication after setting authentication mode to SSH key in the app and did not get prompted for a password, but from my understanding of the changes here, that shouldn't have been the case?

Yes, if that doesn't work and there is nothing else that's wrong here, then I fear sshj doesn't properly deal with multiple auth methods.

Could you send me the sshj log output?

[msfjarvis]

I tried cloning from a server which only had password authentication after setting authentication mode to SSH key in the app and did not get prompted for a password, but from my understanding of the changes here, that shouldn't have been the case?

Yes, if that doesn't work and there is nothing else that's wrong here, then I fear sshj doesn't properly deal with multiple auth methods.

Could you send me the sshj log output?

Obviously simply taking a log made it behave 😑