Fix user ssh key exporting and tests (#37256)

1. Make sure OmitEmail won't panic
2. SSH principal keys are not for signing or authentication

models/asymkey/ssh_key.go

@@ -15,6 +15,7 @@ import (
 	"code.gitea.io/gitea/models/perm"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
 
@@ -64,7 +65,12 @@ func (key *PublicKey) AfterLoad() {
 
 // OmitEmail returns content of public key without email address.
 func (key *PublicKey) OmitEmail() string {
-	return strings.Join(strings.Split(key.Content, " ")[:2], " ")
+	fields := strings.Split(key.Content, " ") // format: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC... comment
+	if len(fields) < 2 {
+		setting.PanicInDevOrTesting("invalid public key %d content: %s", key.ID, key.Content)
+		return "" // not a valid public key, it shouldn't really happen, the value is managed internally
+	}
+	return strings.Join(fields[:2], " ")
 }
 
 func addKey(ctx context.Context, key *PublicKey) (err error) {

routers/web/user/home.go

@@ -655,6 +655,9 @@ func ShowSSHKeys(ctx *context.Context) {
 	// "authorized_keys" file format: "#" followed by comment line per key
 	buf.WriteString("# Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.\n")
 	for i := range keys {
+		if keys[i].Type == asymkey_model.KeyTypePrincipal {
+			continue // SSH principal keys are not for signing or authentication
+		}
 		buf.WriteString(keys[i].OmitEmail())
 		buf.WriteString("\n")
 	}

tests/integration/api_private_serv_test.go

@@ -84,7 +84,7 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Empty(t, results)
 
 		// Add reading deploy key
-		deployKey, err := asymkey_model.AddDeployKey(ctx, 19, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", true)
+		deployKey, err := asymkey_model.AddDeployKey(ctx, 19 /* repo id */, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", true)
 		assert.NoError(t, err)
 
 		// Can pull from repo we're a deploy key for
@@ -106,17 +106,17 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Empty(t, results)
 
 		// Cannot pull from a private repo we're not associated with
-		results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "")
 		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Cannot pull from a public repo we're not associated with
-		results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
+		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "")
 		assert.Error(t, extra.Error)
 		assert.Empty(t, results)
 
 		// Add writing deploy key
-		deployKey, err = asymkey_model.AddDeployKey(ctx, 20, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", false)
+		deployKey, err = asymkey_model.AddDeployKey(ctx, 20 /* repo id */, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", false)
 		assert.NoError(t, err)
 
 		// Cannot push to a private repo with reading key

tests/integration/user_test.go

@@ -8,7 +8,9 @@ import (
 	"strings"
 	"testing"
 
+	asymkey_model "code.gitea.io/gitea/models/asymkey"
 	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
@@ -22,9 +24,20 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestViewUser(t *testing.T) {
+func TestUser(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
+	t.Run("ViewUser", testViewUser)
+	t.Run("RenameInvalidUsername", testRenameInvalidUsername)
+	t.Run("RenameReservedUsername", testRenameReservedUsername)
+	t.Run("ViewLimitedAndPrivateUserAndRename", testViewLimitedAndPrivateUserAndRename)
+	t.Run("ExportUserGPGKeys", testExportUserGPGKeys)
+	t.Run("GetUserRss", testGetUserRss)
+	t.Run("ListStopWatches", testUserListStopWatches)
+	t.Run("LocationMapLink", testUserLocationMapLink)
+	t.Run("RenameUsername", testRenameUsername)
+}
 
+func testViewUser(t *testing.T) {
 	req := NewRequest(t, "GET", "/user2")
 	MakeRequest(t, req, http.StatusOK)
 
@@ -32,12 +45,28 @@ func TestViewUser(t *testing.T) {
 	resp := MakeRequest(t, req, http.StatusOK)
 	assert.Equal(t, `# Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.
 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM=
+`, resp.Body.String())
+
+	_ = db.TruncateBeans(t.Context(), &asymkey_model.PublicKey{})
+	_ = db.Insert(t.Context(), &asymkey_model.PublicKey{
+		OwnerID: 2,
+		Name:    "key-1",
+		Content: "ssh-rsa AAAA",
+		Type:    asymkey_model.KeyTypeUser,
+	}, &asymkey_model.PublicKey{
+		OwnerID: 2,
+		Name:    "key-2",
+		Content: "principal",
+		Type:    asymkey_model.KeyTypePrincipal,
+	})
+	req = NewRequest(t, "GET", "/user2.keys")
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, `# Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.
+ssh-rsa AAAA
 `, resp.Body.String())
 }
 
-func TestRenameUsername(t *testing.T) {
+func testRenameUsername(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	session := loginUser(t, "user2")
 	req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
 		"name":     "newUsername",
@@ -50,9 +79,7 @@ func TestRenameUsername(t *testing.T) {
 	unittest.AssertNotExistsBean(t, &user_model.User{Name: "user2"})
 }
 
-func TestViewLimitedAndPrivateUserAndRename(t *testing.T) {
+func testViewLimitedAndPrivateUserAndRename(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	// user 22 is a limited visibility org
 	org22 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22})
 	req := NewRequest(t, "GET", "/"+org22.Name)
@@ -119,9 +146,7 @@ func TestViewLimitedAndPrivateUserAndRename(t *testing.T) {
 	session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user2 can visit private visibility user via old name
 }
 
-func TestRenameInvalidUsername(t *testing.T) {
+func testRenameInvalidUsername(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	invalidUsernames := []string{
 		"%2f*",
 		"%2f.",
@@ -166,9 +191,7 @@ func TestRenameInvalidUsername(t *testing.T) {
 	}
 }
 
-func TestRenameReservedUsername(t *testing.T) {
+func testRenameReservedUsername(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	reservedUsernames := []string{
 		// ".", "..", ".well-known", // The names are not only reserved but also invalid
 		"api",
@@ -198,8 +221,7 @@ func TestRenameReservedUsername(t *testing.T) {
 	}
 }
 
-func TestExportUserGPGKeys(t *testing.T) {
+func testExportUserGPGKeys(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
 	testExportUserGPGKeys := func(t *testing.T, user, expected string) {
 		session := loginUser(t, user)
 		t.Logf("Testing username %s export gpg keys", user)
@@ -284,9 +306,7 @@ GrE0MHOxUbc9tbtyk0F1SuzREUBH
 -----END PGP PUBLIC KEY BLOCK-----`)
 }
 
-func TestGetUserRss(t *testing.T) {
+func testGetUserRss(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	user34 := "the_34-user.with.all.allowedChars"
 	req := NewRequestf(t, "GET", "/%s.rss", user34)
 	resp := MakeRequest(t, req, http.StatusOK)
@@ -306,9 +326,7 @@ func TestGetUserRss(t *testing.T) {
 	session.MakeRequest(t, req, http.StatusNotFound)
 }
 
-func TestListStopWatches(t *testing.T) {
+func testUserListStopWatches(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 	owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 
@@ -329,8 +347,7 @@ func TestListStopWatches(t *testing.T) {
 	}
 }
 
-func TestUserLocationMapLink(t *testing.T) {
+func testUserLocationMapLink(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
 	defer test.MockVariableValue(&setting.Service.UserLocationMapURL, "https://example/foo/")()
 
 	session := loginUser(t, "user2")