Skip to content

Commit d4096ab

Browse files
6543zeripath
andauthored
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 <[email protected]>
1 parent b3c72a7 commit d4096ab

File tree

4 files changed

+13
-6
lines changed

4 files changed

+13
-6
lines changed

models/org.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,10 @@ func CountOrganizations() int64 {
256256

257257
// DeleteOrganization completely and permanently deletes everything of organization.
258258
func DeleteOrganization(org *User) (err error) {
259+
if !org.IsOrganization() {
260+
return fmt.Errorf("%s is a user not an organization", org.Name)
261+
}
262+
259263
sess := x.NewSession()
260264
defer sess.Close()
261265

@@ -275,10 +279,6 @@ func DeleteOrganization(org *User) (err error) {
275279
}
276280

277281
func deleteOrg(e *xorm.Session, u *User) error {
278-
if !u.IsOrganization() {
279-
return fmt.Errorf("You can't delete none organization user: %s", u.Name)
280-
}
281-
282282
// Check ownership of repository.
283283
count, err := getRepositoryCount(e, u)
284284
if err != nil {

models/org_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ func TestDeleteOrganization(t *testing.T) {
272272
assert.Error(t, err)
273273
assert.True(t, IsErrUserOwnRepos(err))
274274

275-
nonOrg := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
276-
assert.Error(t, DeleteOrganization(nonOrg))
275+
user := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
276+
assert.Error(t, DeleteOrganization(user))
277277
CheckConsistencyFor(t, &User{}, &Team{})
278278
}
279279

models/user.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,10 @@ func deleteUser(e *xorm.Session, u *User) error {
12441244
// DeleteUser completely and permanently deletes everything of a user,
12451245
// but issues/comments/pulls will be kept and shown as someone has been deleted.
12461246
func DeleteUser(u *User) (err error) {
1247+
if u.IsOrganization() {
1248+
return fmt.Errorf("%s is an organization not a user", u.Name)
1249+
}
1250+
12471251
sess := x.NewSession()
12481252
defer sess.Close()
12491253
if err = sess.Begin(); err != nil {

models/user_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ func TestDeleteUser(t *testing.T) {
199199
test(4)
200200
test(8)
201201
test(11)
202+
203+
org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
204+
assert.Error(t, DeleteUser(org))
202205
}
203206

204207
func TestEmailNotificationPreferences(t *testing.T) {

0 commit comments

Comments
 (0)