foster/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-12-21 12:44:49 -05:00
Code Issues Wiki Activity

Merge pull request '[v9.0/forgejo] Do not rewrite ssh keys files when deleting a user without one' (#6168) from bp-v9.0/forgejo-3c9b3dd into v9.0/forgejo

Browse source 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6168
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Otto 2024-12-06 00:01:49 +00:00
commit 02844b48c3
3 changed files with 81 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
services/user/TestPurgeUser/public_key.yml Normal file
View file

@ -0,0 +1,11 @@
-
id: 1001
owner_id: 2
name: user2@localhost
fingerprint: "SHA256:7s+isLFauDv7QSbhAd0Z4OGIYJlQQ4YMtOH9LdjCZL8"
content: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHAv3EOUcaK918Fk9d7mWuVS7oQamif/PNwqnAf/Z34G user2@localhost"
mode: 2
type: 3
created_unix: 1733363453
updated_unix: 1733363453
login_source_id: 0

23
services/user/user.go
View file

@ -131,6 +131,16 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
return models.ErrDeleteLastAdminUser{UID: u.ID}
}
hasSSHKey, err := db.GetEngine(ctx).Where("owner_id = ? AND type != ?", u.ID, asymkey_model.KeyTypePrincipal).Table("public_key").Exist()
if err != nil {
return err
}
hasPrincipialSSHKey, err := db.GetEngine(ctx).Where("owner_id = ? AND type = ?", u.ID, asymkey_model.KeyTypePrincipal).Table("public_key").Exist()
if err != nil {
return err
}
if purge {
// Disable the user first
// NOTE: This is deliberately not within a transaction as it must disable the user immediately to prevent any further action by the user to be purged.
@ -260,11 +270,16 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
}
committer.Close()
if err = asymkey_model.RewriteAllPublicKeys(ctx); err != nil {
return err
if hasSSHKey {
if err = asymkey_model.RewriteAllPublicKeys(ctx); err != nil {
return err
}
}
if err = asymkey_model.RewriteAllPrincipalKeys(ctx); err != nil {
return err
if hasPrincipialSSHKey {
if err = asymkey_model.RewriteAllPrincipalKeys(ctx); err != nil {
return err
}
}
// Note: There are something just cannot be roll back,

57
services/user/user_test.go
View file

@ -5,11 +5,14 @@ package user
import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"
"code.gitea.io/gitea/models"
asymkey_model "code.gitea.io/gitea/models/asymkey"
"code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
@ -17,6 +20,7 @@ import (
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/timeutil"
"github.com/stretchr/testify/assert"
@ -63,20 +67,61 @@ func TestDeleteUser(t *testing.T) {
}
func TestPurgeUser(t *testing.T) {
test := func(userID int64) {
defer unittest.OverrideFixtures(
unittest.FixturesOptions{
Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"),
Base: setting.AppWorkPath,
Dirs: []string{"services/user/TestPurgeUser/"},
},
)()
require.NoError(t, unittest.PrepareTestDatabase())
defer test.MockVariableValue(&setting.SSH.RootPath, t.TempDir())()
defer test.MockVariableValue(&setting.SSH.CreateAuthorizedKeysFile, true)()
defer test.MockVariableValue(&setting.SSH.CreateAuthorizedPrincipalsFile, true)()
defer test.MockVariableValue(&setting.SSH.StartBuiltinServer, false)()
require.NoError(t, asymkey_model.RewriteAllPublicKeys(db.DefaultContext))
require.NoError(t, asymkey_model.RewriteAllPrincipalKeys(db.DefaultContext))
test := func(userID int64, modifySSHKey bool) {
require.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
err := DeleteUser(db.DefaultContext, user, true)
fAuthorizedKeys, err := os.Open(filepath.Join(setting.SSH.RootPath, "authorized_keys"))
require.NoError(t, err)
authorizedKeysStatBefore, err := fAuthorizedKeys.Stat()
require.NoError(t, err)
fAuthorizedPrincipals, err := os.Open(filepath.Join(setting.SSH.RootPath, "authorized_principals"))
require.NoError(t, err)
authorizedPrincipalsBefore, err := fAuthorizedPrincipals.Stat()
require.NoError(t, err)
require.NoError(t, DeleteUser(db.DefaultContext, user, true))
unittest.AssertNotExistsBean(t, &user_model.User{ID: userID})
unittest.CheckConsistencyFor(t, &user_model.User{}, &repo_model.Repository{})
fAuthorizedKeys, err = os.Open(filepath.Join(setting.SSH.RootPath, "authorized_keys"))
require.NoError(t, err)
fAuthorizedPrincipals, err = os.Open(filepath.Join(setting.SSH.RootPath, "authorized_principals"))
require.NoError(t, err)
authorizedKeysStatAfter, err := fAuthorizedKeys.Stat()
require.NoError(t, err)
authorizedPrincipalsAfter, err := fAuthorizedPrincipals.Stat()
require.NoError(t, err)
if modifySSHKey {
assert.Greater(t, authorizedKeysStatAfter.ModTime(), authorizedKeysStatBefore.ModTime())
assert.Greater(t, authorizedPrincipalsAfter.ModTime(), authorizedPrincipalsBefore.ModTime())
} else {
assert.Equal(t, authorizedKeysStatAfter.ModTime(), authorizedKeysStatBefore.ModTime())
assert.Equal(t, authorizedPrincipalsAfter.ModTime(), authorizedPrincipalsBefore.ModTime())
}
}
test(2)
test(4)
test(8)
test(11)
test(2, true)
test(4, false)
test(8, false)
test(11, false)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
require.Error(t, DeleteUser(db.DefaultContext, org, false))