Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Submit feedback
Contribute to GitLab
Sign in
Toggle navigation
GitLab
Project
Project
Details
Activity
Releases
Cycle Analytics
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Charts
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Charts
Commits
Open sidebar
projects.thm.de
GitLab
Commits
fbf618fc
Commit
fbf618fc
authored
Aug 13, 2018
by
Valery Sizov
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Fix: Project deletion may not log audit events during user deletion
parent
5bf9f5b6
Changes
5
Hide whitespace changes
Inline
Side-by-side
Showing
5 changed files
with
35 additions
and
26 deletions
+35
-26
destroy_service.rb
app/services/projects/destroy_service.rb
+0
-3
destroy_service.rb
app/services/users/destroy_service.rb
+4
-3
49796-project-deletion-may-not-log-audit-events-during-user-deletion.yml
...eletion-may-not-log-audit-events-during-user-deletion.yml
+5
-0
destroy_service_spec.rb
spec/services/users/destroy_service_spec.rb
+26
-13
project_destroy_worker_spec.rb
spec/workers/project_destroy_worker_spec.rb
+0
-7
No files found.
app/services/projects/destroy_service.rb
View file @
fbf618fc
...
...
@@ -83,9 +83,6 @@ module Projects
end
def
remove_repository
(
path
)
# Skip repository removal. We use this flag when remove user or group
return
true
if
params
[
:skip_repo
]
==
true
# There is a possibility project does not have repository or wiki
return
true
unless
repo_exists?
(
path
)
...
...
app/services/users/destroy_service.rb
View file @
fbf618fc
...
...
@@ -2,6 +2,8 @@
module
Users
class
DestroyService
DestroyError
=
Class
.
new
(
StandardError
)
attr_accessor
:current_user
def
initialize
(
current_user
)
...
...
@@ -46,9 +48,8 @@ module Users
namespace
.
prepare_for_destroy
user
.
personal_projects
.
each
do
|
project
|
# Skip repository removal because we remove directory with namespace
# that contain all this repositories
::
Projects
::
DestroyService
.
new
(
project
,
current_user
,
skip_repo:
project
.
legacy_storage?
).
execute
success
=
::
Projects
::
DestroyService
.
new
(
project
,
current_user
).
execute
raise
DestroyError
,
"Project
#{
project
.
id
}
can't be deleted"
unless
success
end
yield
(
user
)
if
block_given?
...
...
changelogs/unreleased/49796-project-deletion-may-not-log-audit-events-during-user-deletion.yml
0 → 100644
View file @
fbf618fc
---
title
:
'
Fix:
Project
deletion
may
not
log
audit
events
during
user
deletion'
merge_request
:
author
:
type
:
fixed
spec/services/users/destroy_service_spec.rb
View file @
fbf618fc
...
...
@@ -20,7 +20,7 @@ describe Users::DestroyService do
it
'will delete the project'
do
expect_next_instance_of
(
Projects
::
DestroyService
)
do
|
destroy_service
|
expect
(
destroy_service
).
to
receive
(
:execute
).
once
expect
(
destroy_service
).
to
receive
(
:execute
).
once
.
and_return
(
true
)
end
service
.
execute
(
user
)
...
...
@@ -35,7 +35,7 @@ describe Users::DestroyService do
it
'destroys a project in pending_delete'
do
expect_next_instance_of
(
Projects
::
DestroyService
)
do
|
destroy_service
|
expect
(
destroy_service
).
to
receive
(
:execute
).
once
expect
(
destroy_service
).
to
receive
(
:execute
).
once
.
and_return
(
true
)
end
service
.
execute
(
user
)
...
...
@@ -172,23 +172,36 @@ describe Users::DestroyService do
end
describe
"user personal's repository removal"
do
before
do
perform_enqueued_jobs
{
service
.
execute
(
user
)
}
end
context
'storages'
do
before
do
perform_enqueued_jobs
{
service
.
execute
(
user
)
}
end
context
'legacy storage'
do
let!
(
:project
)
{
create
(
:project
,
:empty_repo
,
:legacy_storage
,
namespace:
user
.
namespace
)
}
it
'removes repository'
do
expect
(
gitlab_shell
.
exists?
(
project
.
repository_storage
,
"
#{
project
.
disk_path
}
.git"
)).
to
be_falsey
end
end
context
'legacy
storage'
do
let!
(
:project
)
{
create
(
:project
,
:empty_repo
,
:legacy_storage
,
namespace:
user
.
namespace
)
}
context
'hashed
storage'
do
let!
(
:project
)
{
create
(
:project
,
:empty_repo
,
namespace:
user
.
namespace
)
}
it
'removes repository'
do
expect
(
gitlab_shell
.
exists?
(
project
.
repository_storage
,
"
#{
project
.
disk_path
}
.git"
)).
to
be_falsey
it
'removes repository'
do
expect
(
gitlab_shell
.
exists?
(
project
.
repository_storage
,
"
#{
project
.
disk_path
}
.git"
)).
to
be_falsey
end
end
end
context
'hashed storage'
do
let!
(
:project
)
{
create
(
:project
,
:empty_repo
,
namespace:
user
.
namespace
)
}
context
'repository removal status is taken into account'
do
it
'raises exception'
do
expect_next_instance_of
(
::
Projects
::
DestroyService
)
do
|
destroy_service
|
expect
(
destroy_service
).
to
receive
(
:execute
).
and_return
(
false
)
end
it
'removes repository'
do
expect
(
gitlab_shell
.
exists?
(
project
.
repository_storage
,
"
#{
project
.
disk_path
}
.git"
)).
to
be_falsey
expect
{
service
.
execute
(
user
)
}
.
to
raise_error
(
Users
::
DestroyService
::
DestroyError
,
"Project
#{
project
.
id
}
can't be deleted"
)
end
end
end
...
...
spec/workers/project_destroy_worker_spec.rb
View file @
fbf618fc
...
...
@@ -18,13 +18,6 @@ describe ProjectDestroyWorker do
expect
(
Dir
.
exist?
(
path
)).
to
be_falsey
end
it
'deletes the project but skips repo deletion'
do
subject
.
perform
(
project
.
id
,
project
.
owner
.
id
,
{
"skip_repo"
=>
true
})
expect
(
Project
.
all
).
not_to
include
(
project
)
expect
(
Dir
.
exist?
(
path
)).
to
be_truthy
end
it
'does not raise error when project could not be found'
do
expect
do
subject
.
perform
(
-
1
,
project
.
owner
.
id
,
{})
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment