Instance signing rule pubkey should allow all public keys, not just GPG (#35357)

Instance signing rule `pubkey` is described as "Only sign if the user
has a public key", however if the user only has SSH public keys, this
check will fail, as it only checks for GPG keys.

Changed the `pubkey` checks to call a helper `userHasPubkeys` which
sequentially checks for GPG, then SSH keys.

Related #34341

---------

Co-authored-by: wxiaoguang <[email protected]>

Author: lyricwulf

services/asymkey/sign.go

@@ -69,6 +69,29 @@ func signingModeFromStrings(modeStrings []string) []signingMode {
 	return returnable
 }
 
+func userHasPubkeysGPG(ctx context.Context, userID int64) (bool, error) {
+	return db.Exist[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
+		OwnerID:        userID,
+		IncludeSubKeys: true,
+	}.ToConds())
+}
+
+func userHasPubkeysSSH(ctx context.Context, userID int64) (bool, error) {
+	return db.Exist[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{
+		OwnerID:    userID,
+		NotKeytype: asymkey_model.KeyTypePrincipal,
+	}.ToConds())
+}
+
+// userHasPubkeys checks if a user has any public keys (GPG or SSH)
+func userHasPubkeys(ctx context.Context, userID int64) (bool, error) {
+	has, err := userHasPubkeysGPG(ctx, userID)
+	if has || err != nil {
+		return has, err
+	}
+	return userHasPubkeysSSH(ctx, userID)
+}
+
 // ErrWontSign explains the first reason why a commit would not be signed
 // There may be other reasons - this is just the first reason found
 type ErrWontSign struct {
@@ -170,14 +193,11 @@ Loop:
 		case always:
 			break Loop
 		case pubkey:
-			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
-				OwnerID:        u.ID,
-				IncludeSubKeys: true,
-			})
+			hasKeys, err := userHasPubkeys(ctx, u.ID)
 			if err != nil {
 				return false, nil, nil, err
 			}
-			if len(keys) == 0 {
+			if !hasKeys {
 				return false, nil, nil, &ErrWontSign{pubkey}
 			}
 		case twofa:
@@ -210,14 +230,11 @@ Loop:
 		case always:
 			break Loop
 		case pubkey:
-			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
-				OwnerID:        u.ID,
-				IncludeSubKeys: true,
-			})
+			hasKeys, err := userHasPubkeys(ctx, u.ID)
 			if err != nil {
 				return false, nil, nil, err
 			}
-			if len(keys) == 0 {
+			if !hasKeys {
 				return false, nil, nil, &ErrWontSign{pubkey}
 			}
 		case twofa:
@@ -266,14 +283,11 @@ Loop:
 		case always:
 			break Loop
 		case pubkey:
-			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
-				OwnerID:        u.ID,
-				IncludeSubKeys: true,
-			})
+			hasKeys, err := userHasPubkeys(ctx, u.ID)
 			if err != nil {
 				return false, nil, nil, err
 			}
-			if len(keys) == 0 {
+			if !hasKeys {
 				return false, nil, nil, &ErrWontSign{pubkey}
 			}
 		case twofa:
@@ -337,14 +351,11 @@ Loop:
 		case always:
 			break Loop
 		case pubkey:
-			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
-				OwnerID:        u.ID,
-				IncludeSubKeys: true,
-			})
+			hasKeys, err := userHasPubkeys(ctx, u.ID)
 			if err != nil {
 				return false, nil, nil, err
 			}
-			if len(keys) == 0 {
+			if !hasKeys {
 				return false, nil, nil, &ErrWontSign{pubkey}
 			}
 		case twofa:

services/asymkey/sign_test.go

@@ -0,0 +1,39 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package asymkey
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUserHasPubkeys(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	test := func(t *testing.T, userID int64, expectedHasGPG, expectedHasSSH bool) {
+		ctx := t.Context()
+		hasGPG, err := userHasPubkeysGPG(ctx, userID)
+		require.NoError(t, err)
+		hasSSH, err := userHasPubkeysSSH(ctx, userID)
+		require.NoError(t, err)
+		hasPubkeys, err := userHasPubkeys(ctx, userID)
+		require.NoError(t, err)
+		assert.Equal(t, expectedHasGPG, hasGPG)
+		assert.Equal(t, expectedHasSSH, hasSSH)
+		assert.Equal(t, expectedHasGPG || expectedHasSSH, hasPubkeys)
+	}
+
+	t.Run("AllowUserWithGPGKey", func(t *testing.T) {
+		test(t, 36, true, false) // has gpg
+	})
+	t.Run("AllowUserWithSSHKey", func(t *testing.T) {
+		test(t, 2, false, true) // has ssh
+	})
+	t.Run("DenyUserWithNoKeys", func(t *testing.T) {
+		test(t, 1, false, false) // no pubkey
+	})
+}