Commit 1e495e76 authored by Robert Speicher's avatar Robert Speicher Committed by Rémy Coutable

Merge branch 'fix-private-snippet-api' into 'master'

Prevent information disclosure via snippet API

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15580

See merge request !1958
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 55456733
...@@ -5,6 +5,7 @@ v 8.3.9 ...@@ -5,6 +5,7 @@ v 8.3.9
- Prevent XSS via custom issue tracker URL - Prevent XSS via custom issue tracker URL
- Prevent privilege escalation via "impersonate" feature - Prevent privilege escalation via "impersonate" feature
- Prevent users from deleting Webhooks via API they do not own - Prevent users from deleting Webhooks via API they do not own
- Prevent information disclosure via snippet API
v 8.3.8 v 8.3.8
- Remove persistent XSS vulnerability in `commit_person_link` helper - Remove persistent XSS vulnerability in `commit_person_link` helper
......
...@@ -51,7 +51,7 @@ class SnippetsFinder ...@@ -51,7 +51,7 @@ class SnippetsFinder
snippets = project.snippets.fresh.non_expired snippets = project.snippets.fresh.non_expired
if current_user if current_user
if project.team.member?(current_user.id) if project.team.member?(current_user.id) || current_user.admin?
snippets snippets
else else
snippets.public_and_internal snippets.public_and_internal
......
...@@ -11,6 +11,11 @@ module API ...@@ -11,6 +11,11 @@ module API
end end
not_found! not_found!
end end
def snippets_for_current_user
finder_params = { filter: :by_project, project: user_project }
SnippetsFinder.new.execute(current_user, finder_params)
end
end end
# Get a project snippets # Get a project snippets
...@@ -20,7 +25,7 @@ module API ...@@ -20,7 +25,7 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/snippets # GET /projects/:id/snippets
get ":id/snippets" do get ":id/snippets" do
present paginate(user_project.snippets), with: Entities::ProjectSnippet present paginate(snippets_for_current_user), with: Entities::ProjectSnippet
end end
# Get a project snippet # Get a project snippet
...@@ -31,7 +36,7 @@ module API ...@@ -31,7 +36,7 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/snippets/:snippet_id # GET /projects/:id/snippets/:snippet_id
get ":id/snippets/:snippet_id" do get ":id/snippets/:snippet_id" do
@snippet = user_project.snippets.find(params[:snippet_id]) @snippet = snippets_for_current_user.find(params[:snippet_id])
present @snippet, with: Entities::ProjectSnippet present @snippet, with: Entities::ProjectSnippet
end end
...@@ -73,7 +78,7 @@ module API ...@@ -73,7 +78,7 @@ module API
# Example Request: # Example Request:
# PUT /projects/:id/snippets/:snippet_id # PUT /projects/:id/snippets/:snippet_id
put ":id/snippets/:snippet_id" do put ":id/snippets/:snippet_id" do
@snippet = user_project.snippets.find(params[:snippet_id]) @snippet = snippets_for_current_user.find(params[:snippet_id])
authorize! :update_project_snippet, @snippet authorize! :update_project_snippet, @snippet
attrs = attributes_for_keys [:title, :file_name, :visibility_level] attrs = attributes_for_keys [:title, :file_name, :visibility_level]
...@@ -97,7 +102,7 @@ module API ...@@ -97,7 +102,7 @@ module API
# DELETE /projects/:id/snippets/:snippet_id # DELETE /projects/:id/snippets/:snippet_id
delete ":id/snippets/:snippet_id" do delete ":id/snippets/:snippet_id" do
begin begin
@snippet = user_project.snippets.find(params[:snippet_id]) @snippet = snippets_for_current_user.find(params[:snippet_id])
authorize! :update_project_snippet, @snippet authorize! :update_project_snippet, @snippet
@snippet.destroy @snippet.destroy
rescue rescue
...@@ -113,7 +118,7 @@ module API ...@@ -113,7 +118,7 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/snippets/:snippet_id/raw # GET /projects/:id/snippets/:snippet_id/raw
get ":id/snippets/:snippet_id/raw" do get ":id/snippets/:snippet_id/raw" do
@snippet = user_project.snippets.find(params[:snippet_id]) @snippet = snippets_for_current_user.find(params[:snippet_id])
env['api.format'] = :txt env['api.format'] = :txt
content_type 'text/plain' content_type 'text/plain'
......
require 'rails_helper'
describe API::API, api: true do
include ApiHelpers
describe 'GET /projects/:project_id/snippets/:id' do
# TODO (rspeicher): Deprecated; remove in 9.0
it 'always exposes expires_at as nil' do
admin = create(:admin)
snippet = create(:project_snippet, author: admin)
get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}", admin)
expect(json_response).to have_key('expires_at')
expect(json_response['expires_at']).to be_nil
end
end
describe 'GET /projects/:project_id/snippets/' do
it 'all snippets available to team member' do
project = create(:project, :public)
user = create(:user)
project.team << [user, :developer]
public_snippet = create(:project_snippet, visibility_level: Gitlab::VisibilityLevel::PUBLIC, project: project)
internal_snippet = create(:project_snippet, visibility_level: Gitlab::VisibilityLevel::INTERNAL, project: project)
private_snippet = create(:project_snippet, visibility_level: Gitlab::VisibilityLevel::PRIVATE, project: project)
get api("/projects/#{project.id}/snippets/", user)
expect(response.status).to eq(200)
expect(json_response.size).to eq(3)
expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id)
end
it 'hides private snippets from regular user' do
project = create(:project, :public)
user = create(:user)
create(:project_snippet, visibility_level: Gitlab::VisibilityLevel::PRIVATE, project: project)
get api("/projects/#{project.id}/snippets/", user)
expect(response.status).to eq(200)
expect(json_response.size).to eq(0)
end
end
describe 'POST /projects/:project_id/snippets/' do
it 'creates a new snippet' do
admin = create(:admin)
project = create(:project)
params = {
title: 'Test Title',
file_name: 'test.rb',
code: 'puts "hello world"',
visibility_level: Gitlab::VisibilityLevel::PUBLIC
}
post api("/projects/#{project.id}/snippets/", admin), params
expect(response.status).to eq(201)
snippet = ProjectSnippet.find(json_response['id'])
expect(snippet.content).to eq(params[:code])
expect(snippet.title).to eq(params[:title])
expect(snippet.file_name).to eq(params[:file_name])
expect(snippet.visibility_level).to eq(params[:visibility_level])
end
end
describe 'PUT /projects/:project_id/snippets/:id/' do
it 'updates snippet' do
admin = create(:admin)
snippet = create(:project_snippet, author: admin)
new_content = 'New content'
put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content
expect(response.status).to eq(200)
snippet.reload
expect(snippet.content).to eq(new_content)
end
end
describe 'DELETE /projects/:project_id/snippets/:id/' do
it 'deletes snippet' do
admin = create(:admin)
snippet = create(:project_snippet, author: admin)
delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin)
expect(response.status).to eq(200)
end
end
describe 'GET /projects/:project_id/snippets/:id/raw' do
it 'returns raw text' do
admin = create(:admin)
snippet = create(:project_snippet, author: admin)
get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin)
expect(response.status).to eq(200)
expect(response.content_type).to eq 'text/plain'
expect(response.body).to eq(snippet.content)
end
end
end
...@@ -11,9 +11,9 @@ describe API::API, api: true do ...@@ -11,9 +11,9 @@ describe API::API, api: true do
let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) } let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) }
let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) } let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) }
let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } let(:snippet) { create(:project_snippet, visibility_level: 20, author: user, project: project, title: 'example') }
let(:project_member) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } let(:project_member) { create(:project_member, access_level: ProjectMember::MASTER, user: user, project: project) }
let(:project_member2) { create(:project_member, user: user3, project: project, access_level: ProjectMember::DEVELOPER) } let(:project_member2) { create(:project_member, access_level: ProjectMember::DEVELOPER, user: user3, project: project) }
let(:user4) { create(:user) } let(:user4) { create(:user) }
let(:project3) do let(:project3) do
create(:project, create(:project,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment