Skip to content

Fix users' noreply email address could be verified when sign with ssh key #35043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions models/fixtures/public_key.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,23 @@
created_unix: 1559593109
updated_unix: 1565224552
login_source_id: 0
verified: false

- id: 2
owner_id: 2
name: [email protected]
fingerprint: "SHA256:70+Mc6lGvBwTlXkxDqJwXZYqV6XzU7jDE06+lC+gocY"
content: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILpPrMXSg9qTx04jPNPWRcHsutyxWjThIpzcaO68yWVn [email protected]"
#-----BEGIN OPENSSH PRIVATE KEY-----
#b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW
#QyNTUxOQAAACC6T6zF0oPak8dOIzzT1kXB7LrcsVo04SKc3GjuvMllZwAAAJgy08upMtPL
#qQAAAAtzc2gtZWQyNTUxOQAAACC6T6zF0oPak8dOIzzT1kXB7LrcsVo04SKc3GjuvMllZw
#AAAEDWqPHTH51xb4hy1y1f1VeWL/2A9Q0b6atOyv5fx8x5prpPrMXSg9qTx04jPNPWRcHs
#utyxWjThIpzcaO68yWVnAAAAEXVzZXIyQGV4YW1wbGUuY29tAQIDBA==
#-----END OPENSSH PRIVATE KEY-----
mode: 2
type: 1
created_unix: 1559593109
updated_unix: 1565224552
login_source_id: 0
verified: true
7 changes: 7 additions & 0 deletions services/asymkey/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,13 @@ func parseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUs
log.Error("GetEmailAddresses: %v", err)
}

// committer's noreply email address should be considered as activated
// so that user signed with that email address can be verified
committerEmailAddresses = append(committerEmailAddresses, &user_model.EmailAddress{
IsActivated: true,
Email: committerUser.GetPlaceholderEmail(),
})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the fix is good.

When parseCommitWithSSHSignature is called, we already know that committerUser belongs to git commit. The whole committerEmailAddresses logic should be removed.

activated := false
for _, e := range committerEmailAddresses {
if e.IsActivated && strings.EqualFold(e.Email, c.Committer.Email) {
Expand Down
61 changes: 61 additions & 0 deletions services/asymkey/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
package asymkey

import (
"fmt"
"strings"
"testing"

asymkey_model "code.gitea.io/gitea/models/asymkey"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -17,6 +20,8 @@
)

func TestParseCommitWithSSHSignature(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

// Here we only test the TrustedSSHKeys. The complete signing test is in tests/integration/gpg_ssh_git_test.go
t.Run("TrustedSSHKey", func(t *testing.T) {
defer test.MockVariableValue(&setting.Repository.Signing.SigningName, "gitea")()
Expand Down Expand Up @@ -51,4 +56,60 @@
assert.Equal(t, "[email protected]", ret.SigningUser.Email)
}
})

t.Run("SSH_Sign_With_UserEmail", func(t *testing.T) {
commit, err := git.CommitFromReader(nil, git.Sha1ObjectFormat.EmptyObjectID(), strings.NewReader(fmt.Sprintf(`tree a3b1fad553e0f9a2b4a58327bebde36c7da75aa2
author user2 <[email protected]> 1752194028 -0700
committer user2 <[email protected]> 1752194028 -0700
gpgsig -----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAguk+sxdKD2pPHTiM809ZFwey63L
FaNOEinNxo7rzJZWcAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
AAAAQBfX+6mcKZBnXckwHcBFqRuXMD3vTKi1yv5wgrqIxTyr2LWB97xxmO92cvjsr0POQ2
2YA7mQS510Cg2s1uU1XAk=
-----END SSH SIGNATURE-----

init project
`)))
require.NoError(t, err)
committingUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
sshKey := unittest.AssertExistsAndLoadBean(t, &asymkey_model.PublicKey{ID: 2})

ret := ParseCommitWithSSHSignature(t.Context(), commit, committingUser)

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

undefined: ParseCommitWithSSHSignature

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

undefined: ParseCommitWithSSHSignature

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-backend

undefined: ParseCommitWithSSHSignature

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / checks-backend

undefined: ParseCommitWithSSHSignature

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / checks-backend

undefined: ParseCommitWithSSHSignature

Check failure on line 77 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / test-unit

undefined: ParseCommitWithSSHSignature
require.NotNil(t, ret)
assert.True(t, ret.Verified)
assert.Equal(t, "user2 / SHA256:70+Mc6lGvBwTlXkxDqJwXZYqV6XzU7jDE06+lC+gocY", ret.Reason)
assert.False(t, ret.Warning)
assert.Equal(t, committingUser, ret.SigningUser)
assert.Equal(t, committingUser, ret.CommittingUser)
assert.Equal(t, sshKey, ret.SigningSSHKey)
})

t.Run("SSH_Sign_With_NoReplyEmailAddress", func(t *testing.T) {
defer test.MockVariableValue(&setting.Service.NoReplyAddress, "noreply.localhost")()

commit, err := git.CommitFromReader(nil, git.Sha1ObjectFormat.EmptyObjectID(), strings.NewReader(fmt.Sprintf(`tree a3b1fad553e0f9a2b4a58327bebde36c7da75aa2
author user2 <user2@%s> 1752193378 -0700
committer user2 <user2@%s> 1752193378 -0700
gpgsig -----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAguk+sxdKD2pPHTiM809ZFwey63L
FaNOEinNxo7rzJZWcAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
AAAAQEbiBh8Ad9JVk4lGUsczwI+2ISaWvzBU+Y/6Lbh3F1sTRF8Rz1ZvVkYup+dVpOLswx
1tBhGYsG8gKwvqGTWQYwI=
-----END SSH SIGNATURE-----

init project
`, setting.Service.NoReplyAddress, setting.Service.NoReplyAddress)))
require.NoError(t, err)
committingUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
sshKey := unittest.AssertExistsAndLoadBean(t, &asymkey_model.PublicKey{ID: 2})

ret := ParseCommitWithSSHSignature(t.Context(), commit, committingUser)

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

undefined: ParseCommitWithSSHSignature (typecheck)

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

undefined: ParseCommitWithSSHSignature (typecheck)

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / lint-backend

undefined: ParseCommitWithSSHSignature (typecheck)

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / checks-backend

undefined: ParseCommitWithSSHSignature

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / checks-backend

undefined: ParseCommitWithSSHSignature

Check failure on line 106 in services/asymkey/commit_test.go

View workflow job for this annotation

GitHub Actions / test-unit

undefined: ParseCommitWithSSHSignature
require.NotNil(t, ret)
assert.True(t, ret.Verified)
assert.Equal(t, "user2 / SHA256:70+Mc6lGvBwTlXkxDqJwXZYqV6XzU7jDE06+lC+gocY", ret.Reason)
assert.False(t, ret.Warning)
assert.Equal(t, committingUser, ret.SigningUser)
assert.Equal(t, committingUser, ret.CommittingUser)
assert.Equal(t, sshKey, ret.SigningSSHKey)
})
}
Loading