From d4096ab6a284ce2c36305428459fd75dcb2e4ee5 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 4 Feb 2020 15:27:18 +0100 Subject: [PATCH] Ensure DeleteUser is not allowed to Delete Orgs and visa versa (#10134) * add check to DeleteUser * add check to DeleteOrganization * add Test * remove redundancy (deleteOrg is only used in DeleteOrganization) * Update models/org.go Co-authored-by: zeripath --- models/org.go | 8 ++++---- models/org_test.go | 4 ++-- models/user.go | 4 ++++ models/user_test.go | 3 +++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/models/org.go b/models/org.go index 176a51ef9..4961db2ac 100644 --- a/models/org.go +++ b/models/org.go @@ -256,6 +256,10 @@ func CountOrganizations() int64 { // DeleteOrganization completely and permanently deletes everything of organization. func DeleteOrganization(org *User) (err error) { + if !org.IsOrganization() { + return fmt.Errorf("%s is a user not an organization", org.Name) + } + sess := x.NewSession() defer sess.Close() @@ -275,10 +279,6 @@ func DeleteOrganization(org *User) (err error) { } func deleteOrg(e *xorm.Session, u *User) error { - if !u.IsOrganization() { - return fmt.Errorf("You can't delete none organization user: %s", u.Name) - } - // Check ownership of repository. count, err := getRepositoryCount(e, u) if err != nil { diff --git a/models/org_test.go b/models/org_test.go index 934d5bc4a..79f8b060e 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -272,8 +272,8 @@ func TestDeleteOrganization(t *testing.T) { assert.Error(t, err) assert.True(t, IsErrUserOwnRepos(err)) - nonOrg := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) - assert.Error(t, DeleteOrganization(nonOrg)) + user := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) + assert.Error(t, DeleteOrganization(user)) CheckConsistencyFor(t, &User{}, &Team{}) } diff --git a/models/user.go b/models/user.go index fda3bb5ea..213b5f76a 100644 --- a/models/user.go +++ b/models/user.go @@ -1244,6 +1244,10 @@ func deleteUser(e *xorm.Session, u *User) error { // DeleteUser completely and permanently deletes everything of a user, // but issues/comments/pulls will be kept and shown as someone has been deleted. func DeleteUser(u *User) (err error) { + if u.IsOrganization() { + return fmt.Errorf("%s is an organization not a user", u.Name) + } + sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { diff --git a/models/user_test.go b/models/user_test.go index fa1b0048e..02b1893c4 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -199,6 +199,9 @@ func TestDeleteUser(t *testing.T) { test(4) test(8) test(11) + + org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) + assert.Error(t, DeleteUser(org)) } func TestEmailNotificationPreferences(t *testing.T) {