From b080ca206999d489dbcf0517cdb67552a44c5522 Mon Sep 17 00:00:00 2001
From: Bruce MacDonald <brucewmacdonald@gmail.com>
Date: Wed, 6 Apr 2022 14:03:06 -0400
Subject: [PATCH] fix: validate key names dont have spaces (#1449) (#1490)

- validate key names dont have spaces
- migrate existing keys with spaces
- validate key name in CLI before create
---
 api/access_key.go                    |  2 +-
 internal/access/setup.go             |  2 +-
 internal/cmd/keys.go                 |  5 +++++
 internal/server/config.go            |  2 +-
 internal/server/data/migrations.go   | 25 +++++++++++++++++++++++++
 internal/server/models/access_key.go |  2 +-
 internal/server/server_test.go       |  2 +-
 7 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/api/access_key.go b/api/access_key.go
index ea84eed07f..700fccc200 100644
--- a/api/access_key.go
+++ b/api/access_key.go
@@ -20,7 +20,7 @@ type ListAccessKeysRequest struct {
 
 type CreateAccessKeyRequest struct {
 	IdentityID        uid.ID   `json:"identityID" validate:"required"`
-	Name              string   `json:"name" validate:"required"`
+	Name              string   `json:"name" validate:"required,excludes= "`
 	TTL               Duration `json:"ttl" validate:"required" note:"maximum time valid"`
 	ExtensionDeadline Duration `json:"extensionDeadline,omitempty" validate:"required" note:"How long the key is active for before it needs to be renewed. The access key must be used within this amount of time to renew validity"`
 }
diff --git a/internal/access/setup.go b/internal/access/setup.go
index c828eebb8d..6af8212ff1 100644
--- a/internal/access/setup.go
+++ b/internal/access/setup.go
@@ -57,7 +57,7 @@ func Setup(c *gin.Context) (string, *models.AccessKey, error) {
 	}
 
 	key := &models.AccessKey{
-		Name:      fmt.Sprintf("%s access key", name),
+		Name:      fmt.Sprintf("%s-access-key", name),
 		IssuedFor: identity.ID,
 		ExpiresAt: time.Now().Add(math.MaxInt64).UTC(),
 	}
diff --git a/internal/cmd/keys.go b/internal/cmd/keys.go
index 82b21e7ef8..b8abcc811b 100644
--- a/internal/cmd/keys.go
+++ b/internal/cmd/keys.go
@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -53,6 +54,10 @@ infra keys create main wall-e 12h --extension-deadline=1h
 			keyName := args[0]
 			machineName := args[1]
 
+			if strings.Contains(keyName, " ") {
+				return fmt.Errorf("key name cannot contain spaces")
+			}
+
 			client, err := defaultAPIClient()
 			if err != nil {
 				return err
diff --git a/internal/server/config.go b/internal/server/config.go
index 042e5b7bfa..d763f8fa6d 100644
--- a/internal/server/config.go
+++ b/internal/server/config.go
@@ -546,7 +546,7 @@ func (s *Server) importAccessKeys() error {
 			}
 		}
 
-		name := fmt.Sprintf("default %s access key", k)
+		name := fmt.Sprintf("default-%s-access-key", k)
 
 		accessKey, err := data.GetAccessKey(s.db, data.ByIssuedFor(machine.ID))
 		if err != nil {
diff --git a/internal/server/data/migrations.go b/internal/server/data/migrations.go
index fa67f66458..a8c459efdd 100644
--- a/internal/server/data/migrations.go
+++ b/internal/server/data/migrations.go
@@ -289,6 +289,31 @@ func migrate(db *gorm.DB) error {
 			},
 			// this change cannot be rolled back
 		},
+		// #1449: access key name can't have whitespace
+		{
+			ID: "202204061643",
+			Migrate: func(tx *gorm.DB) error {
+				if tx.Migrator().HasTable("access_keys") {
+					keys, err := ListAccessKeys(db)
+					if err != nil {
+						return err
+					}
+
+					for i := range keys {
+						if strings.Contains(keys[i].Name, " ") {
+							keys[i].Name = strings.ReplaceAll(keys[i].Name, " ", "-")
+							err := SaveAccessKey(db, &keys[i])
+							if err != nil {
+								return err
+							}
+						}
+					}
+				}
+
+				return nil
+			},
+			// context lost, cannot roll back
+		},
 		// next one here
 	})
 
diff --git a/internal/server/models/access_key.go b/internal/server/models/access_key.go
index 031d1770b8..6b06dd8367 100644
--- a/internal/server/models/access_key.go
+++ b/internal/server/models/access_key.go
@@ -14,7 +14,7 @@ var (
 // AccessKey is a session token presented to the Infra server as proof of authentication
 type AccessKey struct {
 	Model
-	Name      string `gorm:"uniqueIndex:,where:deleted_at is NULL"`
+	Name      string `gorm:"uniqueIndex:,where:deleted_at is NULL" validate:"excludes= "`
 	IssuedFor uid.ID `validate:"required"`
 
 	ExpiresAt         time.Time     `validate:"required"`
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
index a4f461c0d7..be43bc9bd4 100644
--- a/internal/server/server_test.go
+++ b/internal/server/server_test.go
@@ -875,7 +875,7 @@ func TestImportAccessKeysUpdate(t *testing.T) {
 	err = s.importAccessKeys()
 	assert.NilError(t, err)
 
-	accessKey, err := data.GetAccessKey(s.db, data.ByName("default admin access key"))
+	accessKey, err := data.GetAccessKey(s.db, data.ByName("default-admin-access-key"))
 	assert.NilError(t, err)
 	assert.Equal(t, accessKey.KeyID, "EKoHADINYX")
 }