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 <art27@cantab.net>

4 files changed, 13 insertions, 6 deletions

diff --git a/models/org.go b/models/org.go
index 176a51ef94..4961db2ac7 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 934d5bc4a2..79f8b060e7 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 fda3bb5eab..213b5f76ab 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 fa1b0048e1..02b1893c43 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) {