From 5f88d15a63cec9c827cd0ae4062dbf7d33f95353 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Fri, 27 Jun 2025 16:26:11 +0200 Subject: [PATCH] [v12.0/forgejo] fix: pass doer's ID for CRUD instance signing (#8318) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/8304 - When doing CRUD actions, the commiter and author are reconstructed and do not contain the doer's ID. Make sure to pass this ID along so it can be used to verify the rules of instance signing for CRUD actions. - Regression of forgejo/forgejo#7693. It seems that previously this didn't work correctly as it would not care about a empty ID. - Resolves forgejo/forgejo#8278 Co-authored-by: Gusted Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8318 Reviewed-by: Gusted Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- services/repository/files/file.go | 51 +++++++++++----------- tests/integration/signing_git_test.go | 61 ++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index ef9a87dbcf..5b93258840 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -104,36 +104,35 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m // then we use bogus User objects for them to store their FullName and Email. // If only one of the two are provided, we set both of them to it. // If neither are provided, both are the doer. - if committer != nil && committer.Email != "" { - if doer != nil && strings.EqualFold(doer.Email, committer.Email) { - committerUser = doer // the committer is the doer, so will use their user object - if committer.Name != "" { - committerUser.FullName = committer.Name + getUser := func(identity *IdentityOptions) *user_model.User { + if identity == nil || identity.Email == "" { + return nil + } + + if doer != nil && strings.EqualFold(doer.Email, identity.Email) { + user := doer // the committer is the doer, so will use their user object + if identity.Name != "" { + user.FullName = identity.Name } // Use the provided email and not revert to placeholder mail. - committerUser.KeepEmailPrivate = false - } else { - committerUser = &user_model.User{ - FullName: committer.Name, - Email: committer.Email, - } - } - } - if author != nil && author.Email != "" { - if doer != nil && strings.EqualFold(doer.Email, author.Email) { - authorUser = doer // the author is the doer, so will use their user object - if authorUser.Name != "" { - authorUser.FullName = author.Name - } - // Use the provided email and not revert to placeholder mail. - authorUser.KeepEmailPrivate = false - } else { - authorUser = &user_model.User{ - FullName: author.Name, - Email: author.Email, - } + user.KeepEmailPrivate = false + return user + } + + var id int64 + if doer != nil { + id = doer.ID + } + return &user_model.User{ + ID: id, // Needed to ensure the doer is checked to pass rules for instance signing of CRUD actions. + FullName: identity.Name, + Email: identity.Email, } } + + committerUser = getUser(committer) + authorUser = getUser(author) + if authorUser == nil { if committerUser != nil { authorUser = committerUser // No valid author was given so use the committer diff --git a/tests/integration/signing_git_test.go b/tests/integration/signing_git_test.go index 9d69306e0a..c4759aaddb 100644 --- a/tests/integration/signing_git_test.go +++ b/tests/integration/signing_git_test.go @@ -235,7 +235,7 @@ func testCRUD(t *testing.T, u *url.URL, signingFormat string, objectFormat git.O })) }) - t.Run("No publickey", func(t *testing.T) { + t.Run("No 2fa", func(t *testing.T) { defer tests.PrintCurrentTest(t)() testCtx := NewAPITestContext(t, "user4", "initial-no-2fa"+suffix, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) @@ -287,6 +287,65 @@ func testCRUD(t *testing.T, u *url.URL, signingFormat string, objectFormat git.O })) }) + t.Run("AlwaysSign-Initial-CRUD-Pubkey", func(t *testing.T) { + setting.Repository.Signing.CRUDActions = []string{"pubkey"} + + t.Run("Has publickey", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testCtx := NewAPITestContext(t, username, "initial-always-pubkey"+suffix, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("CreateRepository", doAPICreateRepository(testCtx, false, objectFormat)) + t.Run("CreateCRUDFile-Pubkey", crudActionCreateFile( + t, testCtx, user, "master", "pubkey", "signed-pubkey.txt", func(t *testing.T, response api.FileResponse) { + assert.True(t, response.Verification.Verified) + assert.Equal(t, "fox@example.com", response.Verification.Signer.Email) + })) + }) + + t.Run("No publickey", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testCtx := NewAPITestContext(t, "user4", "initial-always-no-pubkey"+suffix, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("CreateRepository", doAPICreateRepository(testCtx, false, objectFormat)) + t.Run("CreateCRUDFile-Pubkey", crudActionCreateFile( + t, testCtx, user, "master", "pubkey", "unsigned-pubkey.txt", func(t *testing.T, response api.FileResponse) { + assert.False(t, response.Verification.Verified) + })) + }) + }) + + t.Run("AlwaysSign-Initial-CRUD-Twofa", func(t *testing.T) { + setting.Repository.Signing.CRUDActions = []string{"twofa"} + + t.Run("Has 2fa", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + t.Cleanup(func() { + unittest.AssertSuccessfulDelete(t, &auth_model.WebAuthnCredential{UserID: user.ID}) + }) + + testCtx := NewAPITestContext(t, username, "initial-always-twofa"+suffix, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + unittest.AssertSuccessfulInsert(t, &auth_model.WebAuthnCredential{UserID: user.ID}) + t.Run("CreateRepository", doAPICreateRepository(testCtx, false, objectFormat)) + t.Run("CreateCRUDFile-Twofa", crudActionCreateFile( + t, testCtx, user, "master", "twofa", "signed-twofa.txt", func(t *testing.T, response api.FileResponse) { + assert.True(t, response.Verification.Verified) + assert.Equal(t, "fox@example.com", response.Verification.Signer.Email) + })) + }) + + t.Run("No 2fa", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testCtx := NewAPITestContext(t, "user4", "initial-always-no-twofa"+suffix, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("CreateRepository", doAPICreateRepository(testCtx, false, objectFormat)) + t.Run("CreateCRUDFile-Pubkey", crudActionCreateFile( + t, testCtx, user, "master", "twofa", "unsigned-twofa.txt", func(t *testing.T, response api.FileResponse) { + assert.False(t, response.Verification.Verified) + })) + }) + }) + t.Run("AlwaysSign-Initial-CRUD-Always", func(t *testing.T) { defer tests.PrintCurrentTest(t)() setting.Repository.Signing.CRUDActions = []string{"always"}