diff options
author | Nick Thomas <nick@gitlab.com> | 2019-03-05 19:12:27 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-03-06 12:05:03 +0300 |
commit | e05a86cecdf52a0ec1f0f4ce4f30287f881b8ea2 (patch) | |
tree | bf11e94a8cd21c43affadcd8fd00f9f5d23d0d6e | |
parent | 42d3117f9c3371e07e8b0aafab6f504e87251c2a (diff) |
Allow all personal snippets to be accessed by API
Previously, you could only access personal snippets in the API if you
had authored them. The documentation doesn't state that this is the
case, and it's quite surprising.
-rw-r--r-- | app/finders/snippets_finder.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/41888-access-personal-snippets-by-api.yml | 5 | ||||
-rw-r--r-- | lib/api/snippets.rb | 25 | ||||
-rw-r--r-- | spec/requests/api/snippets_spec.rb | 76 |
4 files changed, 83 insertions, 25 deletions
diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index d3774746cb8..bf29f15642d 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -69,6 +69,8 @@ class SnippetsFinder < UnionFinder base.with_optional_visibility(visibility_from_scope).fresh end + private + # Produces a query that retrieves snippets from multiple projects. # # The resulting query will, depending on the user's permissions, include the diff --git a/changelogs/unreleased/41888-access-personal-snippets-by-api.yml b/changelogs/unreleased/41888-access-personal-snippets-by-api.yml new file mode 100644 index 00000000000..3561a01ec5f --- /dev/null +++ b/changelogs/unreleased/41888-access-personal-snippets-by-api.yml @@ -0,0 +1,5 @@ +--- +title: Allow all snippets to be accessed by API +merge_request: 25772 +author: +type: added diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index 326d55afd0e..f8b37b33348 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -16,6 +16,10 @@ module API def public_snippets SnippetsFinder.new(current_user, scope: :are_public).execute end + + def snippets + SnippetsFinder.new(current_user).execute + end end desc 'Get a snippets list for authenticated user' do @@ -48,7 +52,10 @@ module API requires :id, type: Integer, desc: 'The ID of a snippet' end get ':id' do - snippet = snippets_for_current_user.find(params[:id]) + snippet = snippets.find_by_id(params[:id]) + + break not_found!('Snippet') unless snippet + present snippet, with: Entities::PersonalSnippet end @@ -94,9 +101,8 @@ module API desc: 'The visibility of the snippet' at_least_one_of :title, :file_name, :content, :visibility end - # rubocop: disable CodeReuse/ActiveRecord put ':id' do - snippet = snippets_for_current_user.find_by(id: params.delete(:id)) + snippet = snippets_for_current_user.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet authorize! :update_personal_snippet, snippet @@ -113,7 +119,6 @@ module API render_validation_error!(snippet) end end - # rubocop: enable CodeReuse/ActiveRecord desc 'Remove snippet' do detail 'This feature was introduced in GitLab 8.15.' @@ -122,16 +127,14 @@ module API params do requires :id, type: Integer, desc: 'The ID of a snippet' end - # rubocop: disable CodeReuse/ActiveRecord delete ':id' do - snippet = snippets_for_current_user.find_by(id: params.delete(:id)) + snippet = snippets_for_current_user.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet authorize! :destroy_personal_snippet, snippet destroy_conditionally!(snippet) end - # rubocop: enable CodeReuse/ActiveRecord desc 'Get a raw snippet' do detail 'This feature was introduced in GitLab 8.15.' @@ -139,9 +142,8 @@ module API params do requires :id, type: Integer, desc: 'The ID of a snippet' end - # rubocop: disable CodeReuse/ActiveRecord get ":id/raw" do - snippet = snippets_for_current_user.find_by(id: params.delete(:id)) + snippet = snippets.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet env['api.format'] = :txt @@ -149,7 +151,6 @@ module API header['Content-Disposition'] = 'attachment' present snippet.content end - # rubocop: enable CodeReuse/ActiveRecord desc 'Get the user agent details for a snippet' do success Entities::UserAgentDetail @@ -157,17 +158,15 @@ module API params do requires :id, type: Integer, desc: 'The ID of a snippet' end - # rubocop: disable CodeReuse/ActiveRecord get ":id/user_agent_detail" do authenticated_as_admin! - snippet = Snippet.find_by!(id: params[:id]) + snippet = Snippet.find_by_id!(params[:id]) break not_found!('UserAgentDetail') unless snippet.user_agent_detail present snippet.user_agent_detail, with: Entities::UserAgentDetail end - # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 7c8512f7589..d600076e9fb 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -84,10 +84,17 @@ describe API::Snippets do end describe 'GET /snippets/:id/raw' do - let(:snippet) { create(:personal_snippet, author: user) } + set(:author) { create(:user) } + set(:snippet) { create(:personal_snippet, :private, author: author) } + + it 'requires authentication' do + get api("/snippets/#{snippet.id}", nil) + + expect(response).to have_gitlab_http_status(401) + end it 'returns raw text' do - get api("/snippets/#{snippet.id}/raw", user) + get api("/snippets/#{snippet.id}/raw", author) expect(response).to have_gitlab_http_status(200) expect(response.content_type).to eq 'text/plain' @@ -95,38 +102,83 @@ describe API::Snippets do end it 'forces attachment content disposition' do - get api("/snippets/#{snippet.id}/raw", user) + get api("/snippets/#{snippet.id}/raw", author) expect(headers['Content-Disposition']).to match(/^attachment/) end it 'returns 404 for invalid snippet id' do - get api("/snippets/1234/raw", user) + snippet.destroy + + get api("/snippets/#{snippet.id}/raw", author) expect(response).to have_gitlab_http_status(404) expect(json_response['message']).to eq('404 Snippet Not Found') end + + it 'hides private snippets from ordinary users' do + get api("/snippets/#{snippet.id}/raw", user) + + expect(response).to have_gitlab_http_status(404) + end + + it 'shows internal snippets to ordinary users' do + internal_snippet = create(:personal_snippet, :internal, author: author) + + get api("/snippets/#{internal_snippet.id}/raw", user) + + expect(response).to have_gitlab_http_status(200) + end end describe 'GET /snippets/:id' do - let(:snippet) { create(:personal_snippet, author: user) } + set(:admin) { create(:user, :admin) } + set(:author) { create(:user) } + set(:private_snippet) { create(:personal_snippet, :private, author: author) } + set(:internal_snippet) { create(:personal_snippet, :internal, author: author) } + + it 'requires authentication' do + get api("/snippets/#{private_snippet.id}", nil) + + expect(response).to have_gitlab_http_status(401) + end it 'returns snippet json' do - get api("/snippets/#{snippet.id}", user) + get api("/snippets/#{private_snippet.id}", author) expect(response).to have_gitlab_http_status(200) - expect(json_response['title']).to eq(snippet.title) - expect(json_response['description']).to eq(snippet.description) - expect(json_response['file_name']).to eq(snippet.file_name) - expect(json_response['visibility']).to eq(snippet.visibility) + expect(json_response['title']).to eq(private_snippet.title) + expect(json_response['description']).to eq(private_snippet.description) + expect(json_response['file_name']).to eq(private_snippet.file_name) + expect(json_response['visibility']).to eq(private_snippet.visibility) + end + + it 'shows private snippets to an admin' do + get api("/snippets/#{private_snippet.id}", admin) + + expect(response).to have_gitlab_http_status(200) + end + + it 'hides private snippets from an ordinary user' do + get api("/snippets/#{private_snippet.id}", user) + + expect(response).to have_gitlab_http_status(404) + end + + it 'shows internal snippets to an ordinary user' do + get api("/snippets/#{internal_snippet.id}", user) + + expect(response).to have_gitlab_http_status(200) end it 'returns 404 for invalid snippet id' do - get api("/snippets/1234", user) + private_snippet.destroy + + get api("/snippets/#{private_snippet.id}", admin) expect(response).to have_gitlab_http_status(404) - expect(json_response['message']).to eq('404 Not found') + expect(json_response['message']).to eq('404 Snippet Not Found') end end |