plumbing: transport/ssh allow different AuthMethod basd on user/host #1509
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
plumbing/transport/ssh/auth_method.go:41
- [nitpick] The naming between AuthMethod and LegacyAuthMethod is very similar and may cause confusion. Consider using a more distinct name (e.g., LegacyToAuthAdapter) to clearly differentiate the legacy conversion adapter.
type AuthMethodFromLegacy struct {
plumbing/transport/ssh/auth_method.go:76
- [nitpick] It appears there is no test covering the new ClientConfig behavior for KeyboardInteractive. Consider adding tests to verify that the user/host parameters are appropriately handled or documented as intentionally unused.
func (a *KeyboardInteractive) ClientConfig(string, string) (*ssh.ClientConfig, error) {
|
I have tested this change with the auth method which allowed me to consider both ssh agents and individual identity files for a given host. import (
// ssh-client-config is a WIP project to create a working ssh config
// as configured in the various ssh config files
sshclientconfig "github.com/tjamet/ssh-client-config"
)
type GitAuth struct {
}
func (a *GitAuth) SetAuth(r *http.Request) {
}
func (a *GitAuth) Name() string {
return "GitAuth"
}
func (a *GitAuth) String() string {
return "GitAuth"
}
func (a *GitAuth) ClientConfig(user string, host string) (*ssh.ClientConfig, error) {
return sshclientconfig.NewSSHClientConfig("").SSHClientConfig(host, func(cc *ssh.ClientConfig) {
// go git already implements the HostKeyCallback
// so far, sshclientconfig does not at the moment
cc.HostKeyCallback = nil
cc.User = user
})
} |
When using the plain git with SSH, git reads the ssh config and is able to consider all the relevant SSH options depending on the hostname. With go-git, it is not possible to use different configurations like IdentityFiles based on the hostname or other configurations as the information is not provided to the ssh auth config. This commit provides such an information while keeping the backward compatibility with older auth methods which do not.
tjamet
force-pushed
the
feat/plumbing/ssh/config-with-user-host
branch
from
bbe6fa2 to
8255f10
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using the plain git with SSH, git reads the SSH config and can consider all the relevant SSH options depending on the hostname.
With go-git, it is impossible to use different configurations like IdentityFiles based on the hostname or other configurations as the information is not provided to the ssh auth config.
This commit provides such information while keeping the backward compatibility with older auth methods which do not.
I expect this will help fix #218 and #509