From 279bf84066d5d7dde2b0258062b5f577b8bdd585 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 18 Apr 2026 00:57:20 +0800 Subject: [PATCH] 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 | 8 ++- routers/web/user/home.go | 3 ++ tests/integration/api_private_serv_test.go | 8 +-- tests/integration/user_test.go | 63 ++++++++++++++-------- 4 files changed, 54 insertions(+), 28 deletions(-) diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index 98784b36bd3..1873c30859d 100644 --- a/models/asymkey/ssh_key.go +++ b/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) { diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 21ca0fc683e..9c99a6c8ef3 100644 --- a/routers/web/user/home.go +++ b/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") } diff --git a/tests/integration/api_private_serv_test.go b/tests/integration/api_private_serv_test.go index b0dd0cf0496..43bf9c02ad9 100644 --- a/tests/integration/api_private_serv_test.go +++ b/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 diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 8981b6b3199..0f68ecfe6c1 100644 --- a/tests/integration/user_test.go +++ b/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) { - defer tests.PrepareTestEnv(t)() - +func testRenameUsername(t *testing.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) { - defer tests.PrepareTestEnv(t)() - +func testViewLimitedAndPrivateUserAndRename(t *testing.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) { - defer tests.PrepareTestEnv(t)() - +func testRenameInvalidUsername(t *testing.T) { invalidUsernames := []string{ "%2f*", "%2f.", @@ -166,9 +191,7 @@ func TestRenameInvalidUsername(t *testing.T) { } } -func TestRenameReservedUsername(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testRenameReservedUsername(t *testing.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) { - defer tests.PrepareTestEnv(t)() +func testExportUserGPGKeys(t *testing.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) { - defer tests.PrepareTestEnv(t)() - +func testGetUserRss(t *testing.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) { - defer tests.PrepareTestEnv(t)() - +func testUserListStopWatches(t *testing.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) { - defer tests.PrepareTestEnv(t)() +func testUserLocationMapLink(t *testing.T) { defer test.MockVariableValue(&setting.Service.UserLocationMapURL, "https://example/foo/")() session := loginUser(t, "user2")