option to make variables protected by default

10 files changed, 82 insertions, 5 deletions

diff --git a/app/assets/javascripts/ci_variable_list/ci_variable_list.js b/app/assets/javascripts/ci_variable_list/ci_variable_list.js
index ee0f7cda189..5b20fa141cd 100644
--- a/app/assets/javascripts/ci_variable_list/ci_variable_list.js
+++ b/app/assets/javascripts/ci_variable_list/ci_variable_list.js
@@ -36,7 +36,9 @@ export default class VariableList {
       },
       protected: {
         selector: '.js-ci-variable-input-protected',
-        default: 'false',
+        // use `attr` instead of `data` as we don't want the value to be
+        // converted. we need the value as a string.
+        default: $('.js-ci-variable-input-protected').attr('data-default'),
       },
       environment_scope: {
         // We can't use a `.js-` class here because
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 086bb38ce9a..72731d969a2 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -218,7 +218,8 @@ module ApplicationSettingsHelper
       :version_check_enabled,
       :web_ide_clientside_preview_enabled,
       :diff_max_patch_bytes,
-      :commit_email_hostname
+      :commit_email_hostname,
+      :protected_ci_variables
     ]
   end
 
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 207ffae873a..da2e095e336 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -302,7 +302,8 @@ class ApplicationSetting < ActiveRecord::Base
       user_show_add_ssh_key_message: true,
       usage_stats_set_by_user_id: nil,
       diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
-      commit_email_hostname: default_commit_email_hostname
+      commit_email_hostname: default_commit_email_hostname,
+      protected_ci_variables: false
     }
   end
 
diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml
index 0d42094fc89..fdaad1cf181 100644
--- a/app/views/admin/application_settings/_ci_cd.html.haml
+++ b/app/views/admin/application_settings/_ci_cd.html.haml
@@ -49,5 +49,12 @@
         Once that time passes, the jobs will be archived and no longer able to be
         retried. Make it empty to never expire jobs. It has to be no less than 1 day,
         for example: <code>15 days</code>, <code>1 month</code>, <code>2 years</code>.
+    .form-group
+      .form-check
+        = f.check_box :protected_ci_variables, class: 'form-check-input'
+        = f.label :protected_ci_variables, class: 'form-check-label' do
+          = s_('AdminSettings|Environment variables are protected by default')
+      .form-text.text-muted
+        = s_('AdminSettings|When creating a new environment variable it will be protected by default.')
 
   = f.submit 'Save changes', class: "btn btn-success"
diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml
index 6ee55836dd2..151a329228e 100644
--- a/app/views/ci/variables/_variable_row.html.haml
+++ b/app/views/ci/variables/_variable_row.html.haml
@@ -5,7 +5,8 @@
 - id = variable&.id
 - key = variable&.key
 - value = variable&.value
-- is_protected = variable && !only_key_value ? variable.protected : false
+- is_protected_default = Gitlab::CurrentSettings.current_application_settings.protected_ci_variables
+- is_protected = variable && !only_key_value ? variable.protected : is_protected_default
 
 - id_input_name = "#{form_field}[variables_attributes][][id]"
 - destroy_input_name = "#{form_field}[variables_attributes][][_destroy]"
@@ -39,7 +40,8 @@
           %input{ type: "hidden",
             class: 'js-ci-variable-input-protected js-project-feature-toggle-input',
             name: protected_input_name,
-            value: is_protected }
+            value: is_protected,
+            data: { default: is_protected_default.to_s } }
           %span.toggle-icon
             = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked')
             = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked')
diff --git a/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb b/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb
new file mode 100644
index 00000000000..85ee34afe1e
--- /dev/null
+++ b/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddProtectedCiVariablesToApplicationSettings < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:application_settings, :protected_ci_variables, :boolean, default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:application_settings, :protected_ci_variables)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9c9c19aa897..a6ce749f45f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -166,6 +166,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do
     t.integer "diff_max_patch_bytes", default: 102400, null: false
     t.integer "archive_builds_in_seconds"
     t.string "commit_email_hostname"
+    t.boolean "protected_ci_variables", default: false, null: false
     t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
   end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 172a0dc5e91..f06c1a24759 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -414,9 +414,15 @@ msgstr ""
 msgid "AdminProjects|Delete project"
 msgstr ""
 
+msgid "AdminSettings|Environment variables are protected by default"
+msgstr ""
+
 msgid "AdminSettings|Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages."
 msgstr ""
 
+msgid "AdminSettings|When creating a new environment variable it will be protected by default."
+msgstr ""
+
 msgid "AdminUsers|Block user"
 msgstr ""
 
diff --git a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js
index 30b15011def..bef59b86d0c 100644
--- a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js
+++ b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js
@@ -118,6 +118,8 @@ describe('VariableList', () => {
       loadFixtures('projects/ci_cd_settings.html.raw');
       $wrapper = $('.js-ci-variable-list-section');
 
+      $wrapper.find('.js-ci-variable-input-protected').attr('data-default', 'false');
+
       variableList = new VariableList({
         container: $wrapper,
         formField: 'variables',
diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb
index bce1fb01355..95f26a01f79 100644
--- a/spec/support/features/variable_list_shared_examples.rb
+++ b/spec/support/features/variable_list_shared_examples.rb
@@ -63,6 +63,44 @@ shared_examples 'variable list' do
     end
   end
 
+  context 'defaults to the application setting' do
+    context 'application setting is true' do
+      before do
+        stub_application_setting(protected_ci_variables: true)
+      end
+
+      it 'defaults to protected' do
+        visit page_path
+
+        page.within('.js-ci-variable-list-section .js-row:last-child') do
+          find('.js-ci-variable-input-key').set('key')
+        end
+
+        values = all('.js-ci-variable-input-protected', visible: false).map(&:value)
+
+        expect(values).to eq %w(false true true)
+      end
+    end
+
+    context 'application setting is false' do
+      before do
+        stub_application_setting(protected_ci_variables: false)
+      end
+
+      it 'defaults to unprotected' do
+        visit page_path
+
+        page.within('.js-ci-variable-list-section .js-row:last-child') do
+          find('.js-ci-variable-input-key').set('key')
+        end
+
+        values = all('.js-ci-variable-input-protected', visible: false).map(&:value)
+
+        expect(values).to eq %w(false false false)
+      end
+    end
+  end
+
   it 'reveals and hides variables' do
     page.within('.js-ci-variable-list-section') do
       expect(first('.js-ci-variable-input-key').value).to eq(variable.key)