Merge remote-tracking branch 'origin/master' into 34141-allow-unauthenticated-access-to-the-users-api

- Modify policy code to work with the `DeclarativePolicy` refactor
  in 37c401433b76170f0150d70865f1f4584db01fa8.

7 files changed, 246 insertions, 30 deletions

diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index b8ca73c321c..cdb60fc0d1a 100644
--- a/spec/requests/api/commit_statuses_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -164,25 +164,40 @@ describe API::CommitStatuses do
 
       context 'with all optional parameters' do
         context 'when creating a commit status' do
-          it 'creates commit status' do
+          subject do
             post api(post_url, developer), {
               state: 'success',
               context: 'coverage',
-              ref: 'develop',
+              ref: 'master',
               description: 'test',
               coverage: 80.0,
               target_url: 'http://gitlab.com/status'
             }
+          end
+
+          it 'creates commit status' do
+            subject
 
             expect(response).to have_http_status(201)
             expect(json_response['sha']).to eq(commit.id)
             expect(json_response['status']).to eq('success')
             expect(json_response['name']).to eq('coverage')
-            expect(json_response['ref']).to eq('develop')
+            expect(json_response['ref']).to eq('master')
             expect(json_response['coverage']).to eq(80.0)
             expect(json_response['description']).to eq('test')
             expect(json_response['target_url']).to eq('http://gitlab.com/status')
           end
+
+          context 'when merge request exists for given branch' do
+            let!(:merge_request) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'develop') }
+
+            it 'sets head pipeline' do
+              subject
+
+              expect(response).to have_http_status(201)
+              expect(merge_request.reload.head_pipeline).not_to be_nil
+            end
+          end
         end
 
         context 'when updatig a commit status' do
@@ -190,7 +205,7 @@ describe API::CommitStatuses do
             post api(post_url, developer), {
               state: 'running',
               context: 'coverage',
-              ref: 'develop',
+              ref: 'master',
               description: 'coverage test',
               coverage: 0.0,
               target_url: 'http://gitlab.com/status'
@@ -199,7 +214,7 @@ describe API::CommitStatuses do
             post api(post_url, developer), {
               state: 'success',
               name: 'coverage',
-              ref: 'develop',
+              ref: 'master',
               description: 'new description',
               coverage: 90.0
             }
@@ -210,7 +225,7 @@ describe API::CommitStatuses do
             expect(json_response['sha']).to eq(commit.id)
             expect(json_response['status']).to eq('success')
             expect(json_response['name']).to eq('coverage')
-            expect(json_response['ref']).to eq('develop')
+            expect(json_response['ref']).to eq('master')
             expect(json_response['coverage']).to eq(90.0)
             expect(json_response['description']).to eq('new description')
             expect(json_response['target_url']).to eq('http://gitlab.com/status')
diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb
index f169e6661d1..1d8aaeea8f2 100644
--- a/spec/requests/api/features_spec.rb
+++ b/spec/requests/api/features_spec.rb
@@ -4,6 +4,13 @@ describe API::Features do
   let(:user)  { create(:user) }
   let(:admin) { create(:admin) }
 
+  before do
+    Flipper.unregister_groups
+    Flipper.register(:perf_team) do |actor|
+      actor.respond_to?(:admin) && actor.admin?
+    end
+  end
+
   describe 'GET /features' do
     let(:expected_features) do
       [
@@ -16,6 +23,14 @@ describe API::Features do
           'name' => 'feature_2',
           'state' => 'off',
           'gates' => [{ 'key' => 'boolean', 'value' => false }]
+        },
+        {
+          'name' => 'feature_3',
+          'state' => 'conditional',
+          'gates' => [
+            { 'key' => 'boolean', 'value' => false },
+            { 'key' => 'groups', 'value' => ['perf_team'] }
+          ]
         }
       ]
     end
@@ -23,6 +38,7 @@ describe API::Features do
     before do
       Feature.get('feature_1').enable
       Feature.get('feature_2').disable
+      Feature.get('feature_3').enable Feature.group(:perf_team)
     end
 
     it 'returns a 401 for anonymous users' do
@@ -47,30 +63,70 @@ describe API::Features do
 
   describe 'POST /feature' do
     let(:feature_name) { 'my_feature' }
-    it 'returns a 401 for anonymous users' do
-      post api("/features/#{feature_name}")
 
-      expect(response).to have_http_status(401)
-    end
+    context 'when the feature does not exist' do
+      it 'returns a 401 for anonymous users' do
+        post api("/features/#{feature_name}")
 
-    it 'returns a 403 for users' do
-      post api("/features/#{feature_name}", user)
+        expect(response).to have_http_status(401)
+      end
 
-      expect(response).to have_http_status(403)
-    end
+      it 'returns a 403 for users' do
+        post api("/features/#{feature_name}", user)
 
-    it 'creates an enabled feature if passed true' do
-      post api("/features/#{feature_name}", admin), value: 'true'
+        expect(response).to have_http_status(403)
+      end
 
-      expect(response).to have_http_status(201)
-      expect(Feature.get(feature_name)).to be_enabled
-    end
+      context 'when passed value=true' do
+        it 'creates an enabled feature' do
+          post api("/features/#{feature_name}", admin), value: 'true'
 
-    it 'creates a feature with the given percentage if passed an integer' do
-      post api("/features/#{feature_name}", admin), value: '50'
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'on',
+            'gates' => [{ 'key' => 'boolean', 'value' => true }])
+        end
+
+        it 'creates an enabled feature for the given Flipper group when passed feature_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', feature_group: 'perf_team'
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'conditional',
+            'gates' => [
+              { 'key' => 'boolean', 'value' => false },
+              { 'key' => 'groups', 'value' => ['perf_team'] }
+            ])
+        end
+
+        it 'creates an enabled feature for the given user when passed user=username' do
+          post api("/features/#{feature_name}", admin), value: 'true', user: user.username
 
-      expect(response).to have_http_status(201)
-      expect(Feature.get(feature_name).percentage_of_time_value).to be(50)
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'conditional',
+            'gates' => [
+              { 'key' => 'boolean', 'value' => false },
+              { 'key' => 'actors', 'value' => ["User:#{user.id}"] }
+            ])
+        end
+      end
+
+      it 'creates a feature with the given percentage if passed an integer' do
+        post api("/features/#{feature_name}", admin), value: '50'
+
+        expect(response).to have_http_status(201)
+        expect(json_response).to eq(
+          'name' => 'my_feature',
+          'state' => 'conditional',
+          'gates' => [
+            { 'key' => 'boolean', 'value' => false },
+            { 'key' => 'percentage_of_time', 'value' => 50 }
+          ])
+      end
     end
 
     context 'when the feature exists' do
@@ -80,11 +136,83 @@ describe API::Features do
         feature.disable # This also persists the feature on the DB
       end
 
-      it 'enables the feature if passed true' do
-        post api("/features/#{feature_name}", admin), value: 'true'
+      context 'when passed value=true' do
+        it 'enables the feature' do
+          post api("/features/#{feature_name}", admin), value: 'true'
 
-        expect(response).to have_http_status(201)
-        expect(feature).to be_enabled
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'on',
+            'gates' => [{ 'key' => 'boolean', 'value' => true }])
+        end
+
+        it 'enables the feature for the given Flipper group when passed feature_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', feature_group: 'perf_team'
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'conditional',
+            'gates' => [
+              { 'key' => 'boolean', 'value' => false },
+              { 'key' => 'groups', 'value' => ['perf_team'] }
+            ])
+        end
+
+        it 'enables the feature for the given user when passed user=username' do
+          post api("/features/#{feature_name}", admin), value: 'true', user: user.username
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'conditional',
+            'gates' => [
+              { 'key' => 'boolean', 'value' => false },
+              { 'key' => 'actors', 'value' => ["User:#{user.id}"] }
+            ])
+        end
+      end
+
+      context 'when feature is enabled and value=false is passed' do
+        it 'disables the feature' do
+          feature.enable
+          expect(feature).to be_enabled
+
+          post api("/features/#{feature_name}", admin), value: 'false'
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'off',
+            'gates' => [{ 'key' => 'boolean', 'value' => false }])
+        end
+
+        it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do
+          feature.enable(Feature.group(:perf_team))
+          expect(Feature.get(feature_name).enabled?(admin)).to be_truthy
+
+          post api("/features/#{feature_name}", admin), value: 'false', feature_group: 'perf_team'
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'off',
+            'gates' => [{ 'key' => 'boolean', 'value' => false }])
+        end
+
+        it 'disables the feature for the given user when passed user=username' do
+          feature.enable(user)
+          expect(Feature.get(feature_name).enabled?(user)).to be_truthy
+
+          post api("/features/#{feature_name}", admin), value: 'false', user: user.username
+
+          expect(response).to have_http_status(201)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'off',
+            'gates' => [{ 'key' => 'boolean', 'value' => false }])
+        end
       end
 
       context 'with a pre-existing percentage value' do
@@ -96,7 +224,13 @@ describe API::Features do
           post api("/features/#{feature_name}", admin), value: '30'
 
           expect(response).to have_http_status(201)
-          expect(Feature.get(feature_name).percentage_of_time_value).to be(30)
+          expect(json_response).to eq(
+            'name' => 'my_feature',
+            'state' => 'conditional',
+            'gates' => [
+              { 'key' => 'boolean', 'value' => false },
+              { 'key' => 'percentage_of_time', 'value' => 30 }
+            ])
         end
       end
     end
diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb
index 3bf16a3ae27..26cf653ca8e 100644
--- a/spec/requests/api/namespaces_spec.rb
+++ b/spec/requests/api/namespaces_spec.rb
@@ -15,6 +15,20 @@ describe API::Namespaces do
     end
 
     context "when authenticated as admin" do
+      it "returns correct attributes" do
+        get api("/namespaces", admin)
+
+        group_kind_json_response = json_response.find { |resource| resource['kind'] == 'group' }
+        user_kind_json_response = json_response.find { |resource| resource['kind'] == 'user' }
+
+        expect(response).to have_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(group_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path',
+                                                                 'parent_id', 'members_count_with_descendants')
+
+        expect(user_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id')
+      end
+
       it "admin: returns an array of all namespaces" do
         get api("/namespaces", admin)
 
@@ -37,6 +51,27 @@ describe API::Namespaces do
     end
 
     context "when authenticated as a regular user" do
+      it "returns correct attributes when user can admin group" do
+        group1.add_owner(user)
+
+        get api("/namespaces", user)
+
+        owned_group_response = json_response.find { |resource| resource['id'] == group1.id }
+
+        expect(owned_group_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path',
+                                                             'parent_id', 'members_count_with_descendants')
+      end
+
+      it "returns correct attributes when user cannot admin group" do
+        group1.add_guest(user)
+
+        get api("/namespaces", user)
+
+        guest_group_response = json_response.find { |resource| resource['id'] == group1.id }
+
+        expect(guest_group_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id')
+      end
+
       it "user: returns an array of namespaces" do
         get api("/namespaces", user)
 
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index fd7ff0b9cff..14dec3d45b1 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -698,7 +698,8 @@ describe API::Projects do
           'name' => user.namespace.name,
           'path' => user.namespace.path,
           'kind' => user.namespace.kind,
-          'full_path' => user.namespace.full_path
+          'full_path' => user.namespace.full_path,
+          'parent_id' => nil
         })
       end
 
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index bf7ed2d3ad6..aa95ae8c7cc 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -395,6 +395,7 @@ describe API::Users do
 
     it "updates user with new bio" do
       put api("/users/#{user.id}", admin), { bio: 'new test bio' }
+
       expect(response).to have_http_status(200)
       expect(json_response['bio']).to eq('new test bio')
       expect(user.reload.bio).to eq('new test bio')
@@ -427,13 +428,22 @@ describe API::Users do
 
     it 'updates user with his own email' do
       put api("/users/#{user.id}", admin), email: user.email
+
       expect(response).to have_http_status(200)
       expect(json_response['email']).to eq(user.email)
       expect(user.reload.email).to eq(user.email)
     end
 
+    it 'updates user with a new email' do
+      put api("/users/#{user.id}", admin), email: 'new@email.com'
+
+      expect(response).to have_http_status(200)
+      expect(user.reload.notification_email).to eq('new@email.com')
+    end
+
     it 'updates user with his own username' do
       put api("/users/#{user.id}", admin), username: user.username
+
       expect(response).to have_http_status(200)
       expect(json_response['username']).to eq(user.username)
       expect(user.reload.username).to eq(user.username)
@@ -441,12 +451,14 @@ describe API::Users do
 
     it "updates user's existing identity" do
       put api("/users/#{omniauth_user.id}", admin), provider: 'ldapmain', extern_uid: '654321'
+
       expect(response).to have_http_status(200)
       expect(omniauth_user.reload.identities.first.extern_uid).to eq('654321')
     end
 
     it 'updates user with new identity' do
       put api("/users/#{user.id}", admin), provider: 'github', extern_uid: 'john'
+
       expect(response).to have_http_status(200)
       expect(user.reload.identities.first.extern_uid).to eq('john')
       expect(user.reload.identities.first.provider).to eq('github')
@@ -454,12 +466,14 @@ describe API::Users do
 
     it "updates admin status" do
       put api("/users/#{user.id}", admin), { admin: true }
+
       expect(response).to have_http_status(200)
       expect(user.reload.admin).to eq(true)
     end
 
     it "updates external status" do
       put api("/users/#{user.id}", admin), { external: true }
+
       expect(response.status).to eq 200
       expect(json_response['external']).to eq(true)
       expect(user.reload.external?).to be_truthy
@@ -467,6 +481,7 @@ describe API::Users do
 
     it "does not update admin status" do
       put api("/users/#{admin_user.id}", admin), { can_create_group: false }
+
       expect(response).to have_http_status(200)
       expect(admin_user.reload.admin).to eq(true)
       expect(admin_user.can_create_group).to eq(false)
@@ -474,6 +489,7 @@ describe API::Users do
 
     it "does not allow invalid update" do
       put api("/users/#{user.id}", admin), { email: 'invalid email' }
+
       expect(response).to have_http_status(400)
       expect(user.reload.email).not_to eq('invalid email')
     end
@@ -490,6 +506,7 @@ describe API::Users do
 
     it "returns 404 for non-existing user" do
       put api("/users/999999", admin), { bio: 'update should fail' }
+
       expect(response).to have_http_status(404)
       expect(json_response['message']).to eq('404 User Not Found')
     end
@@ -540,6 +557,7 @@ describe API::Users do
 
       it 'returns 409 conflict error if email address exists' do
         put api("/users/#{@user.id}", admin), email: 'test@example.com'
+
         expect(response).to have_http_status(409)
         expect(@user.reload.email).to eq(@user.email)
       end
@@ -547,6 +565,7 @@ describe API::Users do
       it 'returns 409 conflict error if username taken' do
         @user_id = User.all.last.id
         put api("/users/#{@user.id}", admin), username: 'test'
+
         expect(response).to have_http_status(409)
         expect(@user.reload.username).to eq(@user.username)
       end
diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb
index cb74868324c..af44ffa2331 100644
--- a/spec/requests/api/v3/projects_spec.rb
+++ b/spec/requests/api/v3/projects_spec.rb
@@ -734,7 +734,8 @@ describe API::V3::Projects do
           'name' => user.namespace.name,
           'path' => user.namespace.path,
           'kind' => user.namespace.kind,
-          'full_path' => user.namespace.full_path
+          'full_path' => user.namespace.full_path,
+          'parent_id' => nil
         })
       end
 
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 83673864fe7..e0975024b80 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -82,6 +82,17 @@ describe API::Variables do
         expect(json_response['protected']).to be_truthy
       end
 
+      it 'creates variable with optional attributes' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(1)
+
+        expect(response).to have_http_status(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+        expect(json_response['protected']).to be_falsey
+      end
+
       it 'does not allow to duplicate variable key' do
         expect do
           post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2'