Browse Source

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>
master
6543 GitHub 5 years ago
parent
commit
d4096ab6a2
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 13 additions and 6 deletions
  1. +4
    -4
      models/org.go
  2. +2
    -2
      models/org_test.go
  3. +4
    -0
      models/user.go
  4. +3
    -0
      models/user_test.go

+ 4
- 4
models/org.go View File

@@ -256,6 +256,10 @@ func CountOrganizations() int64 {


// DeleteOrganization completely and permanently deletes everything of organization. // DeleteOrganization completely and permanently deletes everything of organization.
func DeleteOrganization(org *User) (err error) { func DeleteOrganization(org *User) (err error) {
if !org.IsOrganization() {
return fmt.Errorf("%s is a user not an organization", org.Name)
}

sess := x.NewSession() sess := x.NewSession()
defer sess.Close() defer sess.Close()


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


func deleteOrg(e *xorm.Session, u *User) 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. // Check ownership of repository.
count, err := getRepositoryCount(e, u) count, err := getRepositoryCount(e, u)
if err != nil { if err != nil {


+ 2
- 2
models/org_test.go View File

@@ -272,8 +272,8 @@ func TestDeleteOrganization(t *testing.T) {
assert.Error(t, err) assert.Error(t, err)
assert.True(t, IsErrUserOwnRepos(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{}) CheckConsistencyFor(t, &User{}, &Team{})
} }




+ 4
- 0
models/user.go View File

@@ -1244,6 +1244,10 @@ func deleteUser(e *xorm.Session, u *User) error {
// DeleteUser completely and permanently deletes everything of a user, // DeleteUser completely and permanently deletes everything of a user,
// but issues/comments/pulls will be kept and shown as someone has been deleted. // but issues/comments/pulls will be kept and shown as someone has been deleted.
func DeleteUser(u *User) (err error) { func DeleteUser(u *User) (err error) {
if u.IsOrganization() {
return fmt.Errorf("%s is an organization not a user", u.Name)
}

sess := x.NewSession() sess := x.NewSession()
defer sess.Close() defer sess.Close()
if err = sess.Begin(); err != nil { if err = sess.Begin(); err != nil {


+ 3
- 0
models/user_test.go View File

@@ -199,6 +199,9 @@ func TestDeleteUser(t *testing.T) {
test(4) test(4)
test(8) test(8)
test(11) test(11)

org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
assert.Error(t, DeleteUser(org))
} }


func TestEmailNotificationPreferences(t *testing.T) { func TestEmailNotificationPreferences(t *testing.T) {


Loading…
Cancel
Save