Add latest changes from gitlab-org/gitlab@master

.gitlab/ci/reports.gitlab-ci.yml

@@ -15,7 +15,7 @@ code_quality:
   stage: test
   needs: []
   variables:
-    CODE_QUALITY_IMAGE: "registry.gitlab.com/gitlab-org/ci-cd/codequality:0.85.9"
+    CODE_QUALITY_IMAGE: "registry.gitlab.com/gitlab-org/ci-cd/codequality:0.85.10"
   script:
     - |
       if ! docker info &>/dev/null; then

app/assets/javascripts/users_select/index.js

@@ -263,7 +263,7 @@ function UsersSelect(currentUser, els, options = {}) {
               const userId = parseInt(input.value, 10);
               const { avatarUrl, avatar_url, name, username, canMerge } = input.dataset;
               return {
-                avatar_url: avatarUrl || avatar_url,
+                avatar_url: avatarUrl || avatar_url || gon.default_avatar_url,
                 id: userId,
                 name,
                 username,

app/controllers/repositories/git_http_client_controller.rb

@@ -18,8 +18,7 @@ module Repositories
     skip_around_action :set_session_storage
     skip_before_action :verify_authenticity_token
 
-    before_action :parse_repo_path
-    before_action :authenticate_user
+    prepend_before_action :authenticate_user, :parse_repo_path
 
     private
 

changelogs/unreleased/221184-rolling-28-day-time-period-for-all-usage-ping-counters.yml

+---
+title: Rolling 28 day time period counters for snippets
+merge_request: 34363
+author:
+type: added

changelogs/unreleased/bvl-fix-git-http-user-logging.yml

+---
+title: Load user before logging git http-requests
+merge_request: 34923
+author:
+type: fixed

changelogs/unreleased/remove_old_csrf_generation_monkey_patch.yml

+---
+title: Removes monkey patch to generate 6.0.3 style token
+merge_request: 35104
+author:
+type: other

config/initializers/actionpack_generate_old_csrf_token.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module RequestForgeryProtectionPatch
-    private
-
-    # Patch to generate 6.0.3 tokens so that we do not have CSRF errors while
-    # rolling out 6.0.3.1. This enables GitLab to have a mix of 6.0.3 and
-    # 6.0.3.1 Rails servers
-    #
-    # 1. Deploy this patch with :global_csrf_token FF disabled.
-    # 2. Once all Rails servers are on 6.0.3.1, enable :global_csrf_token FF.
-    # 3. On GitLab 13.2, remove this patch
-    def masked_authenticity_token(session, form_options: {})
-      action, method = form_options.values_at(:action, :method)
-
-      raw_token = if per_form_csrf_tokens && action && method
-                    action_path = normalize_action_path(action)
-                    per_form_csrf_token(session, action_path, method)
-                  else
-                    if Feature.enabled?(:global_csrf_token)
-                      global_csrf_token(session)
-                    else
-                      real_csrf_token(session)
-                    end
-                  end
-
-      mask_token(raw_token)
-    end
-  end
-end
-
-ActionController::Base.include Gitlab::RequestForgeryProtectionPatch

doc/api/environments.md

@@ -13,6 +13,7 @@ GET /projects/:id/environments
 | `id`      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
 | `name`    | string  | no       | Return the environment with this name. Mutually exclusive with `search` |
 | `search`  | string  | no       | Return list of environments matching the search criteria. Mutually exclusive with `name` |
+| `states`  | string  | no       | List all environments that match a specific state. Accepted values: `available` or `stopped`. If no state value given, returns all environments. |
 
 ```shell
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/environments?name=review%2Ffix-foo"

doc/api/projects.md

@@ -891,6 +891,7 @@ GET /projects/:id
   "suggestion_commit_message": null,
   "marked_for_deletion_at": "2020-04-03", // Deprecated and will be removed in API v5 in favor of marked_for_deletion_on
   "marked_for_deletion_on": "2020-04-03",
+  "compliance_frameworks": [ "sox" ],
   "statistics": {
     "commit_count": 37,
     "storage_size": 1038090,

lib/gitlab/usage_data.rb

@@ -32,6 +32,7 @@ module Gitlab
         with_finished_at(:recording_ce_finished_at) do
           license_usage_data
             .merge(system_usage_data)
+            .merge(system_usage_data_monthly)
             .merge(features_usage_data)
             .merge(components_usage_data)
             .merge(cycle_analytics_usage_data)
@@ -164,9 +165,17 @@ module Gitlab
           )
         }
       end
-      # rubocop: enable CodeReuse/ActiveRecord
       # rubocop: enable Metrics/AbcSize
 
+      def system_usage_data_monthly
+        {
+          counts_monthly: {
+            snippets: count(Snippet.where(default_time_period))
+          }
+        }
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
       def cycle_analytics_usage_data
         Gitlab::CycleAnalytics::UsageData.new.to_json
       rescue ActiveRecord::StatementInvalid

spec/controllers/repositories/git_http_controller_spec.rb

@@ -60,10 +60,21 @@ RSpec.describe Repositories::GitHttpController do
 
           get :info_refs, params: params
         end
+
+        include_context 'parsed logs' do
+          it 'adds user info to the logs' do
+            get :info_refs, params: params
+
+            expect(log_data).to include('username' => user.username,
+                                        'user_id' => user.id,
+                                        'meta.user' => user.username)
+          end
+        end
       end
 
       context 'with exceptions' do
         before do
+          allow(controller).to receive(:authenticate_user).and_return(true)
           allow(controller).to receive(:verify_workhorse_api!).and_return(true)
         end
 

spec/factories/usage_data.rb

@@ -89,6 +89,11 @@ FactoryBot.define do
       create(:grafana_integration, project: projects[2], enabled: false)
 
       ProjectFeature.first.update_attribute('repository_access_level', 0)
+
+      # Create fresh & a month (28-days SMAU) old  data
+      [2, 29].each do |n|
+        create(:snippet, created_at: n.days.ago)
+      end
     end
   end
 end

spec/initializers/actionpack_generate_old_csrf_token_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe ActionController::Base, 'CSRF token generation patch', type: :controller do # rubocop:disable RSpec/FilePath
-  let(:fixed_seed) { SecureRandom.random_bytes(described_class::AUTHENTICITY_TOKEN_LENGTH) }
-
-  context 'global_csrf_token feature flag is enabled' do
-    it 'generates 6.0.3.1 style CSRF token', :aggregate_failures do
-      generated_token = controller.send(:form_authenticity_token)
-
-      expect(valid_authenticity_token?(generated_token)).to be_truthy
-      expect(compare_with_real_token(generated_token)).to be_falsey
-      expect(compare_with_global_token(generated_token)).to be_truthy
-    end
-  end
-
-  context 'global_csrf_token feature flag is disabled' do
-    before do
-      stub_feature_flags(global_csrf_token: false)
-    end
-
-    it 'generates 6.0.3 style CSRF token', :aggregate_failures do
-      generated_token = controller.send(:form_authenticity_token)
-
-      expect(valid_authenticity_token?(generated_token)).to be_truthy
-      expect(compare_with_real_token(generated_token)).to be_truthy
-      expect(compare_with_global_token(generated_token)).to be_falsey
-    end
-  end
-
-  def compare_with_global_token(token)
-    unmasked_token = controller.send :unmask_token, Base64.strict_decode64(token)
-
-    controller.send(:compare_with_global_token, unmasked_token, session)
-  end
-
-  def compare_with_real_token(token)
-    unmasked_token = controller.send :unmask_token, Base64.strict_decode64(token)
-
-    controller.send(:compare_with_real_token, unmasked_token, session)
-  end
-
-  def valid_authenticity_token?(token)
-    controller.send(:valid_authenticity_token?, session, token)
-  end
-end

spec/initializers/lograge_spec.rb

@@ -99,6 +99,8 @@ describe 'lograge', type: :request do
   end
 
   context 'with a log subscriber' do
+    include_context 'parsed logs'
+
     let(:subscriber) { Lograge::LogSubscribers::ActionController.new }
 
     let(:event) do
@@ -119,16 +121,6 @@ describe 'lograge', type: :request do
       )
     end
 
-    let(:log_output) { StringIO.new }
-    let(:logger) do
-      Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
-    end
-    let(:log_data) { Gitlab::Json.parse(log_output.string) }
-
-    before do
-      Lograge.logger = logger
-    end
-
     describe 'with an exception' do
       let(:exception) { RuntimeError.new('bad request') }
       let(:backtrace) { caller }

spec/lib/gitlab/analytics/unique_visits_spec.rb

@@ -10,6 +10,15 @@ describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state do
   let(:visitor1_id) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' }
   let(:visitor2_id) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' }
 
+  around do |example|
+    # We need to freeze to a reference time
+    # because visits are grouped by the week number in the year
+    # Without freezing the time, the test may behave inconsistently
+    # depending on which day of the week test is run.
+    reference_time = Time.utc(2020, 6, 1)
+    Timecop.freeze(reference_time) { example.run }
+  end
+
   describe '#track_visit' do
     it 'tracks the unique weekly visits for targets' do
       unique_visits.track_visit(visitor1_id, target1_id, 7.days.ago)

spec/lib/gitlab/usage_data_spec.rb

@@ -110,6 +110,10 @@ describe Gitlab::UsageData, :aggregate_failures do
       expect(UsageDataHelpers::COUNTS_KEYS - count_data.keys).to be_empty
     end
 
+    it 'gathers usage counts monthly hash' do
+      expect(subject[:counts_monthly]).to be_an(Hash)
+    end
+
     it 'gathers projects data correctly' do
       count_data = subject[:counts]
 
@@ -169,6 +173,8 @@ describe Gitlab::UsageData, :aggregate_failures do
       expect(count_data[:grafana_integrated_projects]).to eq(2)
       expect(count_data[:clusters_applications_jupyter]).to eq(1)
       expect(count_data[:clusters_management_project]).to eq(1)
+
+      expect(count_data[:snippets]).to eq(2)
     end
 
     it 'gathers object store usage correctly' do
@@ -247,6 +253,18 @@ describe Gitlab::UsageData, :aggregate_failures do
     end
   end
 
+  describe '.system_usage_data_monthly' do
+    let!(:ud) { build(:usage_data) }
+
+    subject { described_class.system_usage_data_monthly }
+
+    it 'gathers projects data correctly' do
+      counts_monthly = subject[:counts_monthly]
+
+      expect(counts_monthly[:snippets]).to eq(1)
+    end
+  end
+
   describe '#usage_data_counters' do
     subject { described_class.usage_data_counters }
 

spec/requests/jwt_controller_spec.rb

@@ -3,19 +3,14 @@
 require 'spec_helper'
 
 describe JwtController do
+  include_context 'parsed logs'
+
   let(:service) { double(execute: {}) }
   let(:service_class) { double(new: service) }
   let(:service_name) { 'test' }
   let(:parameters) { { service: service_name } }
-  let(:log_output) { StringIO.new }
-  let(:logger) do
-    Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
-  end
-  let(:log_data) { Gitlab::Json.parse(log_output.string) }
 
   before do
-    Lograge.logger = logger
-
     stub_const('JwtController::SERVICES', service_name => service_class)
   end
 

spec/services/snippets/update_service_spec.rb

@@ -328,7 +328,7 @@ describe Snippets::UpdateService do
         expect(snippet.content).to eq(content)
       end
 
-      it 'commit the files to the repository' do
+      it 'commits the files to the repository' do
         subject
 
         blob = snippet.repository.blob_at('master', file_path)
@@ -388,6 +388,218 @@ describe Snippets::UpdateService do
           expect(snippet.content).to eq(content)
         end
       end
+
+      context 'commit actions' do
+        let(:new_path) { 'created_new_file' }
+        let(:base_opts) { { snippet_files: snippet_files } }
+
+        shared_examples 'returns an error' do |error_msg|
+          specify do
+            response = subject
+
+            expect(response).to be_error
+            expect(response.message).to eq error_msg
+          end
+        end
+
+        context 'update action' do
+          let(:snippet_files) { [{ action: :update, file_path: file_path, content: content }] }
+
+          it 'updates the file content' do
+            expect(subject).to be_success
+
+            blob = blob(file_path)
+
+            expect(blob.data).to eq content
+          end
+
+          context 'when previous_path is present' do
+            let(:snippet_files) { [{ action: :update, previous_path: file_path, file_path: file_path, content: content }] }
+
+            it 'updates the file content' do
+              expect(subject).to be_success
+
+              blob = blob(file_path)
+
+              expect(blob.data).to eq content
+            end
+          end
+
+          context 'when content is not present' do
+            let(:snippet_files) { [{ action: :update, file_path: file_path }] }
+
+            it_behaves_like 'returns an error', 'Snippet files have invalid data'
+          end
+
+          context 'when file_path does not exist' do
+            let(:snippet_files) { [{ action: :update, file_path: 'makeup_name', content: content }] }
+
+            it_behaves_like 'returns an error', 'Repository Error updating the snippet'
+          end
+        end
+
+        context 'move action' do
+          context 'when file_path and previous_path are the same' do
+            let(:snippet_files) { [{ action: :move, previous_path: file_path, file_path: file_path }] }
+
+            it_behaves_like 'returns an error', 'Snippet files have invalid data'
+          end
+
+          context 'when file_path and previous_path are different' do
+            let(:snippet_files) { [{ action: :move, previous_path: file_path, file_path: new_path }] }
+
+            it 'renames the file' do
+              old_blob = blob(file_path)
+
+              expect(subject).to be_success
+
+              blob = blob(new_path)
+
+              expect(blob).to be_present
+              expect(blob.data).to eq old_blob.data
+            end
+          end
+
+          context 'when previous_path does not exist' do
+            let(:snippet_files) { [{ action: :move, previous_path: 'makeup_name', file_path: new_path }] }
+
+            it_behaves_like 'returns an error', 'Repository Error updating the snippet'
+          end
+
+          context 'when user wants to rename the file and update content' do
+            let(:snippet_files) { [{ action: :move, previous_path: file_path, file_path: new_path, content: content }] }
+
+            it 'performs both operations' do
+              expect(subject).to be_success
+
+              blob = blob(new_path)
+
+              expect(blob).to be_present
+              expect(blob.data).to eq content
+            end
+          end
+        end
+
+        context 'delete action' do
+          let(:snippet_files) { [{ action: :delete, file_path: file_path }] }
+
+          shared_examples 'deletes the file' do
+            specify do
+              old_blob = blob(file_path)
+              expect(old_blob).to be_present
+
+              expect(subject).to be_success
+              expect(blob(file_path)).to be_nil
+            end
+          end
+
+          it_behaves_like 'deletes the file'
+
+          context 'when previous_path is present and same as file_path' do
+            let(:snippet_files) { [{ action: :delete, previous_path: file_path, file_path: file_path }] }
+
+            it_behaves_like 'deletes the file'
+          end
+
+          context 'when previous_path is present and is different from file_path' do
+            let(:snippet_files) { [{ action: :delete, previous_path: 'foo', file_path: file_path }] }
+
+            it_behaves_like 'deletes the file'
+          end
+
+          context 'when content is present' do
+            let(:snippet_files) { [{ action: :delete, file_path: file_path, content: 'foo' }] }
+
+            it_behaves_like 'deletes the file'
+          end
+
+          context 'when file_path does not exist' do
+            let(:snippet_files) { [{ action: :delete, file_path: 'makeup_name' }] }
+
+            it_behaves_like 'returns an error', 'Repository Error updating the snippet'
+          end
+        end
+
+        context 'create action' do
+          let(:snippet_files) { [{ action: :create, file_path: new_path, content: content }] }
+
+          it 'creates the file' do
+            expect(subject).to be_success
+
+            blob = blob(new_path)
+            expect(blob).to be_present
+            expect(blob.data).to eq content
+          end
+
+          context 'when content is not present' do
+            let(:snippet_files) { [{ action: :create, file_path: new_path }] }
+
+            it_behaves_like 'returns an error', 'Snippet files have invalid data'
+          end
+
+          context 'when file_path is not present' do
+            let(:snippet_files) { [{ action: :create, content: content }] }
+
+            it_behaves_like 'returns an error', 'Snippet files have invalid data'
+          end
+
+          context 'when file_path already exists in the repository' do
+            let(:snippet_files) { [{ action: :create, file_path: file_path, content: content }] }
+
+            it_behaves_like 'returns an error', 'Repository Error updating the snippet'
+          end
+
+          context 'when previous_path is present' do
+            let(:snippet_files) { [{ action: :create, previous_path: 'foo', file_path: new_path, content: content }] }
+
+            it 'creates the file' do
+              expect(subject).to be_success
+
+              blob = blob(new_path)
+              expect(blob).to be_present
+              expect(blob.data).to eq content
+            end
+          end
+        end
+
+        context 'combination of actions' do
+          let(:delete_file_path) { 'CHANGELOG' }
+          let(:create_file_path) { 'created_new_file' }
+          let(:update_file_path) { 'LICENSE' }
+          let(:move_previous_path) { 'VERSION' }
+          let(:move_file_path) { 'VERSION_new' }
+
+          let(:snippet_files) do
+            [
+              { action: :create, file_path: create_file_path, content: content },
+              { action: :update, file_path: update_file_path, content: content },
+              { action: :delete, file_path: delete_file_path },
+              { action: :move, previous_path: move_previous_path, file_path: move_file_path, content: content }
+            ]
+          end
+
+          it 'performs all operations' do
+            expect(subject).to be_success
+
+            expect(blob(delete_file_path)).to be_nil
+
+            created_blob = blob(create_file_path)
+            expect(created_blob.data).to eq content
+
+            updated_blob = blob(update_file_path)
+            expect(updated_blob.data).to eq content
+
+            expect(blob(move_previous_path)).to be_nil
+
+            moved_blob = blob(move_file_path)
+            expect(moved_blob.data).to eq content
+          end
+        end
+
+        def blob(path)
+          snippet.repository.blob_at('master', path)
+        end
+      end
     end
 
     shared_examples 'only file_name is present' do

spec/support/helpers/usage_data_helpers.rb

@@ -233,4 +233,12 @@ module UsageDataHelpers
       allow(client).to receive(:aggregate).and_return({})
     end
   end
+
+  def for_defined_days_back(days: [29, 2])
+    days.each do |n|
+      Timecop.travel(n.days.ago) do
+        yield
+      end
+    end
+  end
 end

spec/support/shared_contexts/controllers/logging_shared_context.rb

+# frozen_string_literal: true
+
+# This context replaces the logger and exposes the `log_data` variable for
+# inspection
+RSpec.shared_context 'parsed logs' do
+  let(:logger) do
+    Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
+  end
+
+  let(:log_output) { StringIO.new }
+  let(:log_data) { Gitlab::Json.parse(log_output.string) }
+
+  around do |example|
+    initial_logger = Lograge.logger
+    Lograge.logger = logger
+
+    example.run
+
+    Lograge.logger = initial_logger
+  end
+end