Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
2f8d8dca
Commit
2f8d8dca
authored
Sep 30, 2020
by
GitLab Bot
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Add latest changes from gitlab-org/security/gitlab@13-4-stable-ee
parent
4d243f5c
Changes
8
Expand all
Hide whitespace changes
Inline
Side-by-side
Showing
8 changed files
with
273 additions
and
167 deletions
+273
-167
app/graphql/mutations/metrics/dashboard/annotations/base.rb
app/graphql/mutations/metrics/dashboard/annotations/base.rb
+1
-1
app/services/members/base_service.rb
app/services/members/base_service.rb
+5
-0
app/services/todos/destroy/entity_leave_service.rb
app/services/todos/destroy/entity_leave_service.rb
+37
-8
changelogs/unreleased/security-insufficient-type-check.yml
changelogs/unreleased/security-insufficient-type-check.yml
+5
-0
changelogs/unreleased/security-todos-redact-guests.yml
changelogs/unreleased/security-todos-redact-guests.yml
+5
-0
spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb
...ql/mutations/metrics/dashboard/annotations/delete_spec.rb
+5
-13
spec/services/members/update_service_spec.rb
spec/services/members/update_service_spec.rb
+25
-7
spec/services/todos/destroy/entity_leave_service_spec.rb
spec/services/todos/destroy/entity_leave_service_spec.rb
+190
-138
No files found.
app/graphql/mutations/metrics/dashboard/annotations/base.rb
View file @
2f8d8dca
...
...
@@ -9,7 +9,7 @@ module Mutations
# This method is defined here in order to be used by `authorized_find!` in the subclasses.
def
find_object
(
id
:)
GitlabSchema
.
object_from_id
(
id
)
GitlabSchema
.
object_from_id
(
id
,
expected_type:
::
Metrics
::
Dashboard
::
Annotation
)
end
end
end
...
...
app/services/members/base_service.rb
View file @
2f8d8dca
...
...
@@ -7,6 +7,11 @@ module Members
def
initialize
(
current_user
=
nil
,
params
=
{})
@current_user
=
current_user
@params
=
params
# could be a string, force to an integer, part of fix
# https://gitlab.com/gitlab-org/gitlab/-/issues/219496
# Allow the ArgumentError to be raised if it can't be converted to an integer.
@params
[
:access_level
]
=
Integer
(
@params
[
:access_level
])
if
@params
[
:access_level
]
end
def
after_execute
(
args
)
...
...
app/services/todos/destroy/entity_leave_service.rb
View file @
2f8d8dca
...
...
@@ -52,7 +52,14 @@ module Todos
# rubocop: disable CodeReuse/ActiveRecord
def
remove_project_todos
Todo
.
where
(
project_id:
non_authorized_projects
,
user_id:
user
.
id
).
delete_all
# Issues are viewable by guests (even in private projects), so remove those todos
# from projects without guest access
Todo
.
where
(
project_id:
non_authorized_guest_projects
,
user_id:
user
.
id
)
.
delete_all
# MRs require reporter access, so remove those todos that are not authorized
Todo
.
where
(
project_id:
non_authorized_reporter_projects
,
target_type:
MergeRequest
.
name
,
user_id:
user
.
id
)
.
delete_all
end
# rubocop: enable CodeReuse/ActiveRecord
...
...
@@ -68,7 +75,7 @@ module Todos
when
Project
{
id:
entity
.
id
}
when
Namespace
{
namespace_id:
non_
memb
er_groups
}
{
namespace_id:
non_
authorized_report
er_groups
}
end
Project
.
where
(
condition
)
...
...
@@ -76,8 +83,32 @@ module Todos
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
non_authorized_projects
projects
.
where
(
'id NOT IN (?)'
,
user
.
authorized_projects
.
select
(
:id
))
def
authorized_reporter_projects
user
.
authorized_projects
(
Gitlab
::
Access
::
REPORTER
).
select
(
:id
)
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
authorized_guest_projects
user
.
authorized_projects
(
Gitlab
::
Access
::
GUEST
).
select
(
:id
)
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
non_authorized_reporter_projects
projects
.
where
(
'id NOT IN (?)'
,
authorized_reporter_projects
)
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
non_authorized_guest_projects
projects
.
where
(
'id NOT IN (?)'
,
authorized_guest_projects
)
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
authorized_reporter_groups
GroupsFinder
.
new
(
user
,
min_access_level:
Gitlab
::
Access
::
REPORTER
).
execute
.
select
(
:id
)
end
# rubocop: enable CodeReuse/ActiveRecord
...
...
@@ -91,9 +122,9 @@ module Todos
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
non_
memb
er_groups
def
non_
authorized_report
er_groups
entity
.
self_and_descendants
.
select
(
:id
)
.
where
(
'id NOT IN (?)'
,
user
.
membership_groups
.
select
(
:id
)
)
.
where
(
'id NOT IN (?)'
,
authorized_reporter_groups
)
end
# rubocop: enable CodeReuse/ActiveRecord
...
...
@@ -106,8 +137,6 @@ module Todos
# rubocop: disable CodeReuse/ActiveRecord
def
confidential_issues
assigned_ids
=
IssueAssignee
.
select
(
:issue_id
).
where
(
user_id:
user
.
id
)
authorized_reporter_projects
=
user
.
authorized_projects
(
Gitlab
::
Access
::
REPORTER
).
select
(
:id
)
Issue
.
where
(
project_id:
projects
,
confidential:
true
)
.
where
(
'project_id NOT IN(?)'
,
authorized_reporter_projects
)
...
...
changelogs/unreleased/security-insufficient-type-check.yml
0 → 100644
View file @
2f8d8dca
---
title
:
Ensure global ID is of Annotation type in GraphQL destroy mutation
merge_request
:
author
:
type
:
security
changelogs/unreleased/security-todos-redact-guests.yml
0 → 100644
View file @
2f8d8dca
---
title
:
Fix redaction of confidential Todos
merge_request
:
author
:
type
:
security
spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb
View file @
2f8d8dca
...
...
@@ -9,13 +9,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do
let_it_be
(
:project
)
{
create
(
:project
,
:private
,
:repository
)
}
let_it_be
(
:environment
)
{
create
(
:environment
,
project:
project
)
}
let_it_be
(
:annotation
)
{
create
(
:metrics_dashboard_annotation
,
environment:
environment
)
}
let
(
:mutation
)
do
variables
=
{
id:
GitlabSchema
.
id_from_object
(
annotation
).
to_s
}
graphql_mutation
(
:delete_annotation
,
variables
)
end
let
(
:variables
)
{
{
id:
GitlabSchema
.
id_from_object
(
annotation
).
to_s
}
}
let
(
:mutation
)
{
graphql_mutation
(
:delete_annotation
,
variables
)
}
def
mutation_response
graphql_mutation_response
(
:delete_annotation
)
...
...
@@ -37,15 +33,11 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do
end
context
'with invalid params'
do
let
(
:mutation
)
do
variables
=
{
id:
'invalid_id'
}
let
(
:variables
)
{
{
id:
GitlabSchema
.
id_from_object
(
project
).
to_s
}
}
graphql_mutation
(
:delete_annotation
,
variables
)
it_behaves_like
'a mutation that returns top-level errors'
do
let
(
:match_errors
)
{
eq
([
"
#{
variables
[
:id
]
}
is not a valid ID for
#{
annotation
.
class
}
."
])
}
end
it_behaves_like
'a mutation that returns top-level errors'
,
errors:
[
'invalid_id is not a valid GitLab ID.'
]
end
context
'when the delete fails'
do
...
...
spec/services/members/update_service_spec.rb
View file @
2f8d8dca
...
...
@@ -31,17 +31,35 @@ RSpec.describe Members::UpdateService do
end
context
'when member is downgraded to guest'
do
let
(
:params
)
do
{
access_level:
Gitlab
::
Access
::
GUEST
}
shared_examples
'schedules to delete confidential todos'
do
it
do
expect
(
TodosDestroyer
::
EntityLeaveWorker
).
to
receive
(
:perform_in
).
with
(
Todo
::
WAIT_FOR_DELETE
,
member
.
user_id
,
member
.
source_id
,
source
.
class
.
name
).
once
updated_member
=
described_class
.
new
(
current_user
,
params
).
execute
(
member
,
permission:
permission
)
expect
(
updated_member
).
to
be_valid
expect
(
updated_member
.
access_level
).
to
eq
(
Gitlab
::
Access
::
GUEST
)
end
end
context
'with Gitlab::Access::GUEST level as a string'
do
let
(
:params
)
{
{
access_level:
Gitlab
::
Access
::
GUEST
.
to_s
}
}
it_behaves_like
'schedules to delete confidential todos'
end
it
'schedules to delete confidential todos'
do
expect
(
TodosDestroyer
::
EntityLeaveWorker
).
to
receive
(
:perform_in
).
with
(
Todo
::
WAIT_FOR_DELETE
,
member
.
user_id
,
member
.
source_id
,
source
.
class
.
name
).
once
context
'with Gitlab::Access::GUEST level as an integer'
do
let
(
:params
)
{
{
access_level:
Gitlab
::
Access
::
GUEST
}
}
it_behaves_like
'schedules to delete confidential todos'
end
end
updated_member
=
described_class
.
new
(
current_user
,
params
).
execute
(
member
,
permission:
permission
)
context
'when access_level is invalid'
do
let
(
:params
)
{
{
access_level:
'invalid'
}
}
expect
(
updated_member
).
to
be_valid
expect
(
updated_member
.
access_level
).
to
eq
(
Gitlab
::
Access
::
GUEST
)
it
'raises an error'
do
expect
{
described_class
.
new
(
current_user
,
params
)
}.
to
raise_error
(
ArgumentError
,
'invalid value for Integer(): "invalid"'
)
end
end
end
...
...
spec/services/todos/destroy/entity_leave_service_spec.rb
View file @
2f8d8dca
This diff is collapsed.
Click to expand it.
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