Use relevant SSHProxyAddr after Login

[klizhentas]

Login could change the SSH proxy address based
on the information from "ping" endpoint

The function ConnectToProxy was saving
old version of the variable and kept using
it during login procedure.

This commit fixes that so the relevant
variable is always referenced.

[russjones]

Looks fine, can you add a comment to the tc.Login line in connectToProxy that it has a side effect of updating the proxy address?

Ideally we'd factor out the applyProxySettings from Login and into connectToProxy so it's more clear, but I'm not sure if that's safe for all code paths.

[klizhentas]

@russjones updated comment line

[klizhentas]

@russjones @r0mant I had to introduce a concept of "alias" proxy names, otherwise in some cases (when proxy name does not match advertised web_proxy or ssh_proxy addr) tsh did not work as expected.

[r0mant]

Unchecked error.

[russjones]

Logic looks fine, some restructuring would help clear things up further.

[russjones]

SaveProfile should take a struct of params, SaveProfile("", "") is not clear what's being saved.

This would also simply things when no alias is needed (see other comment), because you could do things like:

tc.SaveProfile(&profile{
   path: "/path/to/profile",
})

[russjones]

Proxy alias logic is split in two different places in SaveProfile and SaveTo. However, SaveTo appears to mainly be working with files on disk where as SaveProfile is updating configuration.

Moving that configuration logic from SaveTo into SaveProfile would simply things. Something like the following.

// Set alias path if it differs from the address advertised by Teleport.
aliasPath = ""
if profilePath != profileAliasPath {
    aliasPath = profileAliasPath
}

err := SaveTo(&profile{
    path: profilePath,
    alias: aliasPath,
    options: ...,
})

Then SaveTo once again only works with files on disk:

// If a alias has been provided, update the symlink to to point to the profile
// that Teleport advertises.
if p.alias != "" {
    if err := os.Remove(profileAliasPath); err != nil {
        log.Warningf("Failed to remove symlink alias: %v", err)
    }

    err := os.Symlink(filepath.Base(filePath), profileAliasPath)
    if err != nil {
        log.Warningf("Failed to create profile alias: %v", err)
    }
}

[russjones]

Comment explaining why symlinks are skipped because it's not clear from reading this function that you are skipping over aliases.

// Skip over symlinks (aliases) to prevent double printing of profiles.

[klizhentas]

ok, I will refactor in the master, will keep backport relatively small.