mirror of
				https://github.com/go-gitea/gitea
				synced 2025-11-04 05:18:25 +00:00 
			
		
		
		
	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 <wxiaoguang@gmail.com>
This commit is contained in:
		@@ -69,6 +69,29 @@ func signingModeFromStrings(modeStrings []string) []signingMode {
 | 
				
			|||||||
	return returnable
 | 
						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
 | 
					// ErrWontSign explains the first reason why a commit would not be signed
 | 
				
			||||||
// There may be other reasons - this is just the first reason found
 | 
					// There may be other reasons - this is just the first reason found
 | 
				
			||||||
type ErrWontSign struct {
 | 
					type ErrWontSign struct {
 | 
				
			||||||
@@ -170,14 +193,11 @@ Loop:
 | 
				
			|||||||
		case always:
 | 
							case always:
 | 
				
			||||||
			break Loop
 | 
								break Loop
 | 
				
			||||||
		case pubkey:
 | 
							case pubkey:
 | 
				
			||||||
			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
 | 
								hasKeys, err := userHasPubkeys(ctx, u.ID)
 | 
				
			||||||
				OwnerID:        u.ID,
 | 
					 | 
				
			||||||
				IncludeSubKeys: true,
 | 
					 | 
				
			||||||
			})
 | 
					 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				return false, nil, nil, err
 | 
									return false, nil, nil, err
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if len(keys) == 0 {
 | 
								if !hasKeys {
 | 
				
			||||||
				return false, nil, nil, &ErrWontSign{pubkey}
 | 
									return false, nil, nil, &ErrWontSign{pubkey}
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		case twofa:
 | 
							case twofa:
 | 
				
			||||||
@@ -210,14 +230,11 @@ Loop:
 | 
				
			|||||||
		case always:
 | 
							case always:
 | 
				
			||||||
			break Loop
 | 
								break Loop
 | 
				
			||||||
		case pubkey:
 | 
							case pubkey:
 | 
				
			||||||
			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
 | 
								hasKeys, err := userHasPubkeys(ctx, u.ID)
 | 
				
			||||||
				OwnerID:        u.ID,
 | 
					 | 
				
			||||||
				IncludeSubKeys: true,
 | 
					 | 
				
			||||||
			})
 | 
					 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				return false, nil, nil, err
 | 
									return false, nil, nil, err
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if len(keys) == 0 {
 | 
								if !hasKeys {
 | 
				
			||||||
				return false, nil, nil, &ErrWontSign{pubkey}
 | 
									return false, nil, nil, &ErrWontSign{pubkey}
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		case twofa:
 | 
							case twofa:
 | 
				
			||||||
@@ -266,14 +283,11 @@ Loop:
 | 
				
			|||||||
		case always:
 | 
							case always:
 | 
				
			||||||
			break Loop
 | 
								break Loop
 | 
				
			||||||
		case pubkey:
 | 
							case pubkey:
 | 
				
			||||||
			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
 | 
								hasKeys, err := userHasPubkeys(ctx, u.ID)
 | 
				
			||||||
				OwnerID:        u.ID,
 | 
					 | 
				
			||||||
				IncludeSubKeys: true,
 | 
					 | 
				
			||||||
			})
 | 
					 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				return false, nil, nil, err
 | 
									return false, nil, nil, err
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if len(keys) == 0 {
 | 
								if !hasKeys {
 | 
				
			||||||
				return false, nil, nil, &ErrWontSign{pubkey}
 | 
									return false, nil, nil, &ErrWontSign{pubkey}
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		case twofa:
 | 
							case twofa:
 | 
				
			||||||
@@ -337,14 +351,11 @@ Loop:
 | 
				
			|||||||
		case always:
 | 
							case always:
 | 
				
			||||||
			break Loop
 | 
								break Loop
 | 
				
			||||||
		case pubkey:
 | 
							case pubkey:
 | 
				
			||||||
			keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{
 | 
								hasKeys, err := userHasPubkeys(ctx, u.ID)
 | 
				
			||||||
				OwnerID:        u.ID,
 | 
					 | 
				
			||||||
				IncludeSubKeys: true,
 | 
					 | 
				
			||||||
			})
 | 
					 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				return false, nil, nil, err
 | 
									return false, nil, nil, err
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if len(keys) == 0 {
 | 
								if !hasKeys {
 | 
				
			||||||
				return false, nil, nil, &ErrWontSign{pubkey}
 | 
									return false, nil, nil, &ErrWontSign{pubkey}
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		case twofa:
 | 
							case twofa:
 | 
				
			||||||
 
 | 
				
			|||||||
							
								
								
									
										39
									
								
								services/asymkey/sign_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										39
									
								
								services/asymkey/sign_test.go
									
									
									
									
									
										Normal file
									
								
							@@ -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
 | 
				
			||||||
 | 
						})
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
		Reference in New Issue
	
	Block a user