Fix snippets API not working with visibility levelsh-fix-snippet-visibility-api

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

9 files changed, 108 insertions, 25 deletions

diff --git a/app/services/base_service.rb b/app/services/base_service.rb
index 3e968c8f707..c39edd5c114 100644
--- a/app/services/base_service.rb
+++ b/app/services/base_service.rb
@@ -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)
diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb
index 6e5bf823cc7..0aa76df35ba 100644
--- a/app/services/create_snippet_service.rb
+++ b/app/services/create_snippet_service.rb
@@ -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
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index e78e5d5fc2c..1dd22d7a3ae 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -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
diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb
index 2969c360de5..a294812ef9e 100644
--- a/app/services/update_snippet_service.rb
+++ b/app/services/update_snippet_service.rb
@@ -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)
diff --git a/changelogs/unreleased/sh-fix-snippet-visibility-api.yml b/changelogs/unreleased/sh-fix-snippet-visibility-api.yml
new file mode 100644
index 00000000000..5cfb9cdedc0
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-snippet-visibility-api.yml
@@ -0,0 +1,5 @@
+---
+title: Fix snippets API not working with visibility level
+merge_request: 32286
+author:
+type: fixed
diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb
index 29f69b6ce20..a543ef8de78 100644
--- a/spec/requests/api/project_snippets_spec.rb
+++ b/spec/requests/api/project_snippets_spec.rb
@@ -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
diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb
index d600076e9fb..cc05b8d5b45 100644
--- a/spec/requests/api/snippets_spec.rb
+++ b/spec/requests/api/snippets_spec.rb
@@ -193,18 +193,32 @@ describe API::Snippets do
       }
     end
 
-    it 'creates a new snippet' do
-      expect do
-        post api("/snippets/", user), params: params
-      end.to change { PersonalSnippet.count }.by(1)
+    shared_examples 'snippet creation' do
+      it 'creates a new snippet' do
+        expect do
+          post api("/snippets/", user), params: params
+        end.to change { PersonalSnippet.count }.by(1)
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(json_response['title']).to eq(params[:title])
+        expect(json_response['description']).to eq(params[:description])
+        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
 
-      expect(response).to have_gitlab_http_status(201)
-      expect(json_response['title']).to eq(params[:title])
-      expect(json_response['description']).to eq(params[:description])
-      expect(json_response['file_name']).to eq(params[:file_name])
-      expect(json_response['visibility']).to eq(params[:visibility])
+      it_behaves_like 'snippet creation'
     end
 
+    it_behaves_like 'snippet creation'
+
     it 'returns 400 for missing parameters' do
       params.delete(:title)
 
@@ -253,18 +267,33 @@ describe API::Snippets do
       create(:personal_snippet, author: user, visibility_level: visibility_level)
     end
 
-    it 'updates snippet' do
-      new_content = 'New content'
-      new_description = 'New description'
+    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(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' }
 
diff --git a/spec/services/create_snippet_service_spec.rb b/spec/services/create_snippet_service_spec.rb
index 9b83f65a17e..7d2491b3a49 100644
--- a/spec/services/create_snippet_service_spec.rb
+++ b/spec/services/create_snippet_service_spec.rb
@@ -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
diff --git a/spec/services/update_snippet_service_spec.rb b/spec/services/update_snippet_service_spec.rb
index 0678f54c195..19b35dca6a7 100644
--- a/spec/services/update_snippet_service_spec.rb
+++ b/spec/services/update_snippet_service_spec.rb
@@ -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