Commit 680f4377 authored by Stan Hu's avatar Stan Hu

Fix snippets API not working with visibility level

When a restricted visibility level of `private` is set in the instance,
creating a snippet with the `visibility` level would always fail.
This happened because:

1. `params[:visibility]` was a string (e.g. "public")
2. `CreateSnippetService` and `UpdateSnippetService` only looked
   at `params[:visibility_level]`, which was `nil`.

To fix this, we:

1. Make `CreateSnippetService` look at the newly-built
   `snippet.visibility_level`, since the right value is assigned by the
   `VisibilityLevel#visibility=` method.
2. Modify `UpdateSnippetService` to handle both `visibility_level` and
`visibility` parameters.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/66050
parent 549e95b8
......@@ -44,6 +44,10 @@ class BaseService
model.errors.add(:visibility_level, "#{level_name} has been restricted by your GitLab administrator")
end
def visibility_level
params[:visibility].is_a?(String) ? Gitlab::VisibilityLevel.level_value(params[:visibility]) : params[:visibility_level]
end
private
def error(message, http_status = nil)
......
......@@ -12,7 +12,7 @@ class CreateSnippetService < BaseService
PersonalSnippet.new(params)
end
unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
unless Gitlab::VisibilityLevel.allowed_for?(current_user, snippet.visibility_level)
deny_visibility_level(snippet)
return snippet
end
......
......@@ -68,9 +68,5 @@ module Groups
true
end
def visibility_level
params[:visibility].present? ? Gitlab::VisibilityLevel.level_value(params[:visibility]) : params[:visibility_level]
end
end
end
......@@ -12,7 +12,7 @@ class UpdateSnippetService < BaseService
def execute
# check that user is allowed to set specified visibility_level
new_visibility = params[:visibility_level]
new_visibility = visibility_level
if new_visibility && new_visibility.to_i != snippet.visibility_level
unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
......
---
title: Fix snippets API not working with visibility level
merge_request: 32286
author:
type: fixed
......@@ -96,6 +96,28 @@ describe API::ProjectSnippets do
}
end
context 'with a regular user' do
let(:user) { create(:user) }
before do
project.add_developer(user)
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE])
params['visibility'] = 'internal'
end
it 'creates a new snippet' do
post api("/projects/#{project.id}/snippets/", user), params: params
expect(response).to have_gitlab_http_status(201)
snippet = ProjectSnippet.find(json_response['id'])
expect(snippet.content).to eq(params[:code])
expect(snippet.description).to eq(params[:description])
expect(snippet.title).to eq(params[:title])
expect(snippet.file_name).to eq(params[:file_name])
expect(snippet.visibility_level).to eq(Snippet::INTERNAL)
end
end
it 'creates a new snippet' do
post api("/projects/#{project.id}/snippets/", admin), params: params
......@@ -167,12 +189,13 @@ describe API::ProjectSnippets do
new_content = 'New content'
new_description = 'New description'
put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), params: { code: new_content, description: new_description }
put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), params: { code: new_content, description: new_description, visibility: 'private' }
expect(response).to have_gitlab_http_status(200)
snippet.reload
expect(snippet.content).to eq(new_content)
expect(snippet.description).to eq(new_description)
expect(snippet.visibility).to eq('private')
end
it 'returns 404 for invalid snippet id' do
......
......@@ -193,6 +193,7 @@ describe API::Snippets do
}
end
shared_examples 'snippet creation' do
it 'creates a new snippet' do
expect do
post api("/snippets/", user), params: params
......@@ -204,6 +205,19 @@ describe API::Snippets do
expect(json_response['file_name']).to eq(params[:file_name])
expect(json_response['visibility']).to eq(params[:visibility])
end
end
context 'with restricted visibility settings' do
before do
stub_application_setting(restricted_visibility_levels:
[Gitlab::VisibilityLevel::INTERNAL,
Gitlab::VisibilityLevel::PRIVATE])
end
it_behaves_like 'snippet creation'
end
it_behaves_like 'snippet creation'
it 'returns 400 for missing parameters' do
params.delete(:title)
......@@ -253,17 +267,32 @@ describe API::Snippets do
create(:personal_snippet, author: user, visibility_level: visibility_level)
end
it 'updates snippet' do
shared_examples 'snippet updates' do
it 'updates a snippet' do
new_content = 'New content'
new_description = 'New description'
put api("/snippets/#{snippet.id}", user), params: { content: new_content, description: new_description }
put api("/snippets/#{snippet.id}", user), params: { content: new_content, description: new_description, visibility: 'internal' }
expect(response).to have_gitlab_http_status(200)
snippet.reload
expect(snippet.content).to eq(new_content)
expect(snippet.description).to eq(new_description)
expect(snippet.visibility).to eq('internal')
end
end
context 'with restricted visibility settings' do
before do
stub_application_setting(restricted_visibility_levels:
[Gitlab::VisibilityLevel::PUBLIC,
Gitlab::VisibilityLevel::PRIVATE])
end
it_behaves_like 'snippet updates'
end
it_behaves_like 'snippet updates'
it 'returns 404 for invalid snippet id' do
put api("/snippets/1234", user), params: { title: 'foo' }
......
......@@ -34,6 +34,19 @@ describe CreateSnippetService do
expect(snippet.errors.any?).to be_falsey
expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
end
describe "when visibility level is passed as a string" do
before do
@opts[:visibility] = 'internal'
@opts.delete(:visibility_level)
end
it "assigns the correct visibility level" do
snippet = create_snippet(nil, @user, @opts)
expect(snippet.errors.any?).to be_falsey
expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
end
end
end
describe 'usage counter' do
......
......@@ -32,12 +32,25 @@ describe UpdateSnippetService do
expect(@snippet.visibility_level).to eq(old_visibility)
end
it 'admins should be able to update to pubic visibility' do
it 'admins should be able to update to public visibility' do
old_visibility = @snippet.visibility_level
update_snippet(@project, @admin, @snippet, @opts)
expect(@snippet.visibility_level).not_to eq(old_visibility)
expect(@snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
end
describe "when visibility level is passed as a string" do
before do
@opts[:visibility] = 'internal'
@opts.delete(:visibility_level)
end
it "assigns the correct visibility level" do
update_snippet(@project, @user, @snippet, @opts)
expect(@snippet.errors.any?).to be_falsey
expect(@snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
end
end
end
describe 'usage counter' do
......
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