Allow dots in alias and add port to canonical name (#209)

* allow numerical start and allow dots in ssh alias

We previously restricted the ssh alias to start with an
alphabetic character and did not allow dots within it. This caused
problems with users wanting to use an ip address as an alias. This lifts
that restriction so both can freely be used.

Note that while it is common to use the hostname as the alias, hostnames
are not allowed to use the underscore character. However, we do allow
the alias to contain an underscore. I do not think we can remove this
from aliases now but it is something to watch out for in the future.

* allow backslash in username

This involved converting the regexp string into a raw string to able to
use the backslash character. this is important.

* Revert "allow backslash in username"

This reverts commit cafe281241. Upon
reflection, more time is needed to evaluate that this actually corrects
the issue. It will be performed with proper diligence at a later time.

* add port to end of canoncial names when not 22

The canonical name is the identifying key in the database, so it causes
problems if another remote entry has the same canonical name. By adding
the port number to the end of this, it is possible to differentiate the
two.

* add db migrations for adding port to canonicalname

The up migration adds the port to the existing canonical id if it exists
and is not 22. The down migration strips the port off the canonical name
and deletes extra remotes with the same canonical name. If all remotes
with that canonical name have been archived, it keeps the first one
added to the database. If any have not been archived, it keeps the first
added to the database from the non-archived group.

* remove ability to edit port number via ssh import

Previously, ssh imports could edit the port number since it was possible
to change them in the config file without changin the canonical name.
Now that the canonical name contains the port, a change in the port will
simply create a new database entry. For this reason, the ability to
modify the ssh port is dead code and has been removed.

* allow backslash in username

This involved converting the regexp string into a raw string to able to
use the backslash character. this is important.

* simplify up migration logic

The previous up migration was suboptimal because it was accounting for a
corner case not found in production. That case no longer needs to be
considered, so the procedure was simplified.

wavesrv/db/migrations/000029_canonicalport.down.sql

@@ -0,0 +1,12 @@
+UPDATE remote
+SET remotecanonicalname = SUBSTR(remotecanonicalname, 1, INSTR(remotecanonicalname, ':') - 1)
+WHERE INSTR(remotecanonicalname, ':');
+
+DELETE FROM remote
+WHERE remoteid NOT IN (
+    SELECT remoteid FROM (
+        SELECT MIN(archived), remoteid, remotecanonicalname
+        FROM remote
+        GROUP BY remotecanonicalname
+    )
+);

wavesrv/db/migrations/000029_canonicalport.up.sql

@@ -0,0 +1,3 @@
+UPDATE remote
+SET remotecanonicalname = remotecanonicalname || COALESCE( ":" || json_extract(sshopts, '$.sshport'), "")
+WHERE json_extract(sshopts, '$.sshport') != 22;

wavesrv/pkg/cmdrunner/cmdrunner.go

@@ -115,8 +115,8 @@ var SetVarScopes = []SetVarScope{
 	{ScopeName: "remote", VarNames: []string{}},
 }
 
-var userHostRe = regexp.MustCompile("^(sudo@)?([a-z][a-z0-9._@-]*)@([a-z0-9][a-z0-9.-]*)(?::([0-9]+))?$")
-var remoteAliasRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_-]*$")
+var userHostRe = regexp.MustCompile(`^(sudo@)?([a-z][a-z0-9._@\\-]*)@([a-z0-9][a-z0-9.-]*)(?::([0-9]+))?$`)
+var remoteAliasRe = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9._-]*$")
 var genericNameRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_ .()<>,/\"'\\[\\]{}=+$@!*-]*$")
 var rendererRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_.:-]*$")
 var positionRe = regexp.MustCompile("^((S?\\+|E?-)?[0-9]+|(\\+|-|S|E))$")
@@ -1257,6 +1257,9 @@ func parseRemoteEditArgs(isNew bool, pk *scpacket.FeCommandPacketType, isLocal b
 		}
 		sshOpts.SSHPort = portVal
 		canonicalName = remoteUser + "@" + remoteHost
+		if portVal != 0 && portVal != 22 {
+			canonicalName = canonicalName + ":" + strconv.Itoa(portVal)
+		}
 		if isSudo {
 			canonicalName = "sudo@" + canonicalName
 		}
@@ -1528,7 +1531,8 @@ func NewHostInfo(hostName string) (*HostInfoType, error) {
 
 	portStr, _ := ssh_config.GetStrict(hostName, "Port")
 	var portVal int
-	if portStr != "" {
+	if portStr != "" && portStr != "22" {
+		canonicalName = canonicalName + ":" + portStr
 		var err error
 		portVal, err = strconv.Atoi(portStr)
 		if err != nil {
@@ -1539,7 +1543,6 @@ func NewHostInfo(hostName string) (*HostInfoType, error) {
 			return nil, fmt.Errorf("could not parse port \"%d\": number is not valid for a port\n", portVal)
 		}
 	}
-
 	identityFile, _ := ssh_config.GetStrict(hostName, "IdentityFile")
 	passwordAuth, _ := ssh_config.GetStrict(hostName, "PasswordAuthentication")
 
@@ -1626,13 +1629,9 @@ func RemoteConfigParseCommand(ctx context.Context, pk *scpacket.FeCommandPacketT
 		if previouslyImportedRemote != nil && !previouslyImportedRemote.Archived {
 			// this already existed and was created via import
 			// it needs to be updated instead of created
-
 			editMap := make(map[string]interface{})
 			editMap[sstore.RemoteField_Alias] = hostInfo.Host
 			editMap[sstore.RemoteField_ConnectMode] = hostInfo.ConnectMode
-			// changing port is unique to imports because it lets us avoid conflicts
-			// if the port is changed in the ssh config
-			editMap[sstore.RemoteField_SSHPort] = hostInfo.Port
 			if hostInfo.SshKeyFile != "" {
 				editMap[sstore.RemoteField_SSHKey] = hostInfo.SshKeyFile
 			}

wavesrv/pkg/sstore/dbops.go

@@ -1704,7 +1704,6 @@ const (
 	RemoteField_ConnectMode = "connectmode" // string
 	RemoteField_SSHKey      = "sshkey"      // string
 	RemoteField_SSHPassword = "sshpassword" // string
-	RemoteField_SSHPort     = "sshport"     // string
 	RemoteField_Color       = "color"       // string
 )
 
@@ -1736,10 +1735,6 @@ func UpdateRemote(ctx context.Context, remoteId string, editMap map[string]inter
 			query = `UPDATE remote SET sshopts = json_set(sshopts, '$.sshpassword', ?) WHERE remoteid = ?`
 			tx.Exec(query, sshPassword, remoteId)
 		}
-		if sshPort, found := editMap[RemoteField_SSHPort]; found {
-			query = `UPDATE remote SET sshopts = json_set(sshopts, '$.sshport', ?) WHERE remoteid = ?`
-			tx.Exec(query, sshPort, remoteId)
-		}
 		if color, found := editMap[RemoteField_Color]; found {
 			query = `UPDATE remote SET remoteopts = json_set(remoteopts, '$.color', ?) WHERE remoteid = ?`
 			tx.Exec(query, color, remoteId)

wavesrv/pkg/sstore/migrate.go

@@ -22,7 +22,7 @@ import (
 	"github.com/golang-migrate/migrate/v4"
 )
 
-const MaxMigration = 28
+const MaxMigration = 29
 const MigratePrimaryScreenVersion = 9
 const CmdScreenSpecialMigration = 13
 const CmdLineSpecialMigration = 20