diff --git a/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb new file mode 100644 index 0000000000000000000000000000000000000000..8d37f6c1dd4a105229518b2a10835d76408a5bc6 --- /dev/null +++ b/db/migrate/20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard < ActiveRecord::Migration[6.0] + DOWNTIME = false + + class FeatureGate < ApplicationRecord + self.table_name = 'feature_gates' + end + + def up + security_dashboard_feature = FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean) + + if security_dashboard_feature.present? + FeatureGate.safe_find_or_create_by!( + feature_key: :instance_security_dashboard, + key: :boolean, + value: security_dashboard_feature.value + ) + end + end + + def down + FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean)&.delete + end +end diff --git a/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb new file mode 100644 index 0000000000000000000000000000000000000000..7972361953375a651d645215ed1dc89ef98f3f8f --- /dev/null +++ b/db/post_migrate/20200214034836_remove_security_dashboard_feature_flag.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class RemoveSecurityDashboardFeatureFlag < ActiveRecord::Migration[6.0] + DOWNTIME = false + + class FeatureGate < ApplicationRecord + self.table_name = 'feature_gates' + end + + def up + FeatureGate.find_by(feature_key: :security_dashboard, key: :boolean)&.delete + end + + def down + instance_security_dashboard_feature = FeatureGate.find_by(feature_key: :instance_security_dashboard, key: :boolean) + + if instance_security_dashboard_feature.present? + FeatureGate.safe_find_or_create_by!( + feature_key: :security_dashboard, + key: instance_security_dashboard_feature.key, + value: instance_security_dashboard_feature.value + ) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index fec0ba6e7530ff8023e3717a7c8a821ceefc66fa..962f9eb6012671597e3d13c4ad47247ff3312193 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_02_13_220211) do +ActiveRecord::Schema.define(version: 2020_02_14_034836) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" diff --git a/ee/app/assets/javascripts/pages/security/dashboard/show/index.js b/ee/app/assets/javascripts/pages/security/dashboard/show/index.js index 4d1aba7ae1b39fd456506fa670251602aaa75231..c08aba4904079211c41f47507df55c31485b30ae 100644 --- a/ee/app/assets/javascripts/pages/security/dashboard/show/index.js +++ b/ee/app/assets/javascripts/pages/security/dashboard/show/index.js @@ -5,7 +5,7 @@ import syncWithRouter from 'ee/security_dashboard/store/plugins/sync_with_router import createStore from 'ee/security_dashboard/store'; import InstanceSecurityDashboard from 'ee/security_dashboard/components/instance_security_dashboard.vue'; -if (gon.features && gon.features.securityDashboard) { +if (gon.features && gon.features.instanceSecurityDashboard) { document.addEventListener('DOMContentLoaded', () => { const el = document.querySelector('#js-security'); const { diff --git a/ee/app/controllers/security/application_controller.rb b/ee/app/controllers/security/application_controller.rb index 4a037051849460a5d575d234aa1658534e264290..634d1adbfe374dceb493de43f3de744abddbcb55 100644 --- a/ee/app/controllers/security/application_controller.rb +++ b/ee/app/controllers/security/application_controller.rb @@ -6,13 +6,13 @@ class ApplicationController < ::ApplicationController before_action :check_feature_enabled! before_action do - push_frontend_feature_flag(:security_dashboard, default_enabled: true) + push_frontend_feature_flag(:instance_security_dashboard, default_enabled: true) end protected def check_feature_enabled! - render_404 unless Feature.enabled?(:security_dashboard, default_enabled: true) + render_404 unless Feature.enabled?(:instance_security_dashboard, default_enabled: true) end def vulnerable diff --git a/ee/app/helpers/ee/dashboard_helper.rb b/ee/app/helpers/ee/dashboard_helper.rb index 5fd3a0fa71adfbad9fef4d78c0b86099f210704d..d865190014172a4b7b9234c64280fc75ec2ebf02 100644 --- a/ee/app/helpers/ee/dashboard_helper.rb +++ b/ee/app/helpers/ee/dashboard_helper.rb @@ -64,7 +64,7 @@ def get_dashboard_nav_links def security_dashboard_available? security_dashboard = InstanceSecurityDashboard.new(current_user) - ::Feature.enabled?(:security_dashboard, default_enabled: true) && + ::Feature.enabled?(:instance_security_dashboard, default_enabled: true) && security_dashboard.feature_available?(:security_dashboard) && can?(current_user, :read_instance_security_dashboard, security_dashboard) end diff --git a/ee/spec/controllers/security/projects_controller_spec.rb b/ee/spec/controllers/security/projects_controller_spec.rb index 11fb4718f42c0ad65894422dc653f99b9d85e63f..af84e4dfd98f6c28f1e6898dacee27f745b64990 100644 --- a/ee/spec/controllers/security/projects_controller_spec.rb +++ b/ee/spec/controllers/security/projects_controller_spec.rb @@ -155,7 +155,7 @@ context 'and the security dashboard feature is disabled' do it '404s' do - stub_feature_flags(security_dashboard: false) + stub_feature_flags(instance_security_dashboard: false) subject diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb index bcff607bc2dc6394d10b055694fbe78edc05a17a..ed5828ac4a26466e27fe0e4e86bb6b298b1a779a 100644 --- a/ee/spec/helpers/ee/dashboard_helper_spec.rb +++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb @@ -13,13 +13,11 @@ end describe 'analytics' do - before do - allow(helper).to receive(:can?) { true } - end - context 'when at least one analytics feature is enabled' do before do enable_only_one_analytics_feature_flag + + stub_user_permissions_for(:analytics, false) end it 'includes analytics' do @@ -34,7 +32,7 @@ context 'and the user has no access to instance statistics features' do before do - allow(helper).to receive(:can?) { false } + stub_user_permissions_for(:analytics, false) end it 'does not include analytics' do @@ -43,6 +41,10 @@ end context 'and the user has access to instance statistics features' do + before do + stub_user_permissions_for(:analytics, true) + end + it 'does include analytics' do expect(helper.dashboard_nav_links).to include(:analytics) end @@ -50,96 +52,162 @@ end end - describe 'operations, environments and security' do - using RSpec::Parameterized::TableSyntax + describe 'operations dashboard link' do + context 'when the feature is available on the license' do + context 'and the user is authenticated' do + before do + stub_user_permissions_for(:operations, true) + end - before do - allow(helper).to receive(:can?).and_return(false) + it 'is included in the nav' do + expect(helper.dashboard_nav_links).to include(:operations) + end + end + + context 'and the user is not authenticated' do + before do + stub_user_permissions_for(:operations, false) + end + + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:operations) + end + end end - where(:ability, :feature_flag, :nav_link) do - :read_operations_dashboard | nil | :operations - :read_operations_dashboard | :environments_dashboard | :environments - :read_instance_security_dashboard | :security_dashboard | :security + context 'when the feature is not available on the license' do + before do + stub_user_permissions_for(:operations, false) + end + + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:operations) + end end + end - with_them do - describe 'when the feature is enabled' do - before do - stub_feature_flags(feature_flag => true) unless feature_flag.nil? - end - - context 'and the feature is available on the license' do - context 'and the user is authenticated' do - before do - stub_resource_visibility( - feature_flag, - read_other_resources: true, - read_security_dashboard: true, - security_dashboard_available: true - ) - end - - it 'includes the nav link' do - expect(helper.dashboard_nav_links).to include(nav_link) - end + describe 'environments dashboard link' do + context 'when the feature is enabled' do + before do + stub_feature_flags(environments_dashboard: true) + end + + context 'and the feature is available on the license' do + context 'and the user is authenticated' do + before do + stub_user_permissions_for(:operations, true) + end + + it 'is included in the nav' do + expect(helper.dashboard_nav_links).to include(:environments) + end + end + + context 'and the user is not authenticated' do + before do + stub_user_permissions_for(:operations, false) end - context 'and the user is not authenticated' do - let(:user) { nil } - - before do - stub_resource_visibility( - feature_flag, - read_other_resources: false, - read_security_dashboard: false, - security_dashboard_available: true - ) - end - - it 'does not include the nav link' do - expect(helper.dashboard_nav_links).not_to include(nav_link) - end + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:environments) end end + end + + context 'and the feature is not available on the license' do + before do + stub_user_permissions_for(:operations, false) + end + + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:environments) + end + end + end + + context 'when the feature is not enabled' do + before do + stub_feature_flags(environments_dashboard: false) + stub_user_permissions_for(:operations, false) + end + + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:environments) + end + end + end + + describe 'security dashboard link' do + context 'when the feature is enabled' do + before do + stub_feature_flags(instance_security_dashboard: true) + end - context 'and the feature is not available on the license' do + context 'and the feature is available on the license' do + before do + stub_licensed_features(security_dashboard: true) + end + + context 'and the user is authenticated' do before do - stub_resource_visibility( - feature_flag, - read_other_resources: false, - read_security_dashboard: true, - security_dashboard_available: false - ) + stub_user_permissions_for(:security, true) end - it 'does not include the nav link' do - expect(helper.dashboard_nav_links).not_to include(nav_link) + it 'is included in the nav' do + expect(helper.dashboard_nav_links).to include(:security) end end - def stub_resource_visibility(feature_flag, read_other_resources:, read_security_dashboard:, security_dashboard_available:) - if feature_flag == :security_dashboard - stub_licensed_features(feature_flag => security_dashboard_available) + context 'and the user is not authenticated' do + before do + stub_user_permissions_for(:security, false) + end - allow(helper).to receive(:can?).and_return(read_security_dashboard) - else - allow(helper).to receive(:can?).with(user, ability).and_return(read_other_resources) + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:security) end end end - describe 'when the feature is disabled' do + context 'when the feature is not available on the license' do before do - stub_feature_flags(feature_flag => false) unless feature_flag.nil? - allow(helper).to receive(:can?).and_return(false) + stub_licensed_features(security_dashboard: false) + stub_user_permissions_for(:security, true) end - it 'does not include the nav link' do - expect(helper.dashboard_nav_links).not_to include(nav_link) + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:security) end end end + + context 'when the feature is not enabled' do + before do + stub_feature_flags(instance_security_dashboard: false) + stub_licensed_features(security_dashboard: true) + stub_user_permissions_for(:security, true) + end + + it 'is not included in the nav' do + expect(helper.dashboard_nav_links).not_to include(:security) + end + end + end + + def stub_user_permissions_for(feature, enabled) + allow(helper).to receive(:can?).with(user, :read_cross_project).and_return(false) + + can_read_instance_statistics = enabled && feature == :analytics + can_read_operations_dashboard = enabled && feature == :operations + can_read_instance_security_dashboard = enabled && feature == :security + + allow(helper).to receive(:can?).with(user, :read_instance_statistics).and_return(can_read_instance_statistics) + allow(helper).to receive(:can?).with(user, :read_operations_dashboard).and_return(can_read_operations_dashboard) + allow_next_instance_of(InstanceSecurityDashboard) do |dashboard| + allow(helper).to( + receive(:can?).with(user, :read_instance_security_dashboard, dashboard).and_return(can_read_instance_security_dashboard) + ) + end end end diff --git a/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb b/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb index 990c81cf25d7398544d9376e70754b5c9311f26d..6c2dfe8b9b3f31c553d93ac77c54b72931e06287 100644 --- a/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb +++ b/ee/spec/support/shared_examples/controllers/security/application_controller_shared_examples.rb @@ -27,7 +27,7 @@ context 'and the security dashboard feature is disabled' do it '404s' do - stub_feature_flags(security_dashboard: false) + stub_feature_flags(instance_security_dashboard: false) security_application_controller_child_action diff --git a/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb b/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb index 5471df8cf769082f616aecfbdfee08777cfee002..8c41492810a14ce0f49d41a42078224c82eaf2d7 100644 --- a/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb +++ b/ee/spec/support/shared_examples/requests/security/security_dashboard_json_endpoint_shared_examples.rb @@ -28,7 +28,7 @@ context 'and the security dashboard feature is disabled' do it '404s' do - stub_feature_flags(security_dashboard: false) + stub_feature_flags(instance_security_dashboard: false) security_dashboard_request diff --git a/spec/migrations/remove_security_dashboard_feature_flag_spec.rb b/spec/migrations/remove_security_dashboard_feature_flag_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7ef43134d24d85fa1631c9f730de2786b681f27a --- /dev/null +++ b/spec/migrations/remove_security_dashboard_feature_flag_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require Rails.root.join('db', 'post_migrate', '20200214034836_remove_security_dashboard_feature_flag.rb') + +describe RemoveSecurityDashboardFeatureFlag, :migration do + let(:feature_gates) { table(:feature_gates) } + + subject(:migration) { described_class.new } + + describe '#up' do + it 'deletes the security_dashboard feature gate' do + security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: :boolean, value: 'false') + actors_security_dashboard_feature = feature_gates.create!(feature_key: :security_dashboard, key: :actors, value: 'Project:1') + + migration.up + + expect { security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(actors_security_dashboard_feature.reload).to be_present + end + end + + describe '#down' do + it 'copies the instance_security_dashboard feature gate to a security_dashboard gate' do + feature_gates.create!(feature_key: :instance_security_dashboard, key: :actors, value: 'Project:1') + feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false') + + migration.down + + security_dashboard_feature = feature_gates.find_by(feature_key: :security_dashboard, key: :boolean) + expect(security_dashboard_feature.value).to eq('false') + end + + context 'when there is no instance_security_dashboard gate' do + it 'does nothing' do + migration.down + + security_dashboard_feature = feature_gates.find_by(feature_key: :security_dashboard, key: :boolean) + expect(security_dashboard_feature).to be_nil + end + end + + context 'when there already is a security_dashboard gate' do + it 'does nothing' do + feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false') + feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false') + + expect { migration.down }.not_to raise_error + end + end + end +end diff --git a/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..bc982e8952e2f063cb7db5337f2fcae02b9147f6 --- /dev/null +++ b/spec/migrations/rename_security_dashboard_feature_flag_to_instance_security_dashboard_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require Rails.root.join('db', 'migrate', '20200212014653_rename_security_dashboard_feature_flag_to_instance_security_dashboard.rb') + +describe RenameSecurityDashboardFeatureFlagToInstanceSecurityDashboard, :migration do + let(:feature_gates) { table(:feature_gates) } + + subject(:migration) { described_class.new } + + describe '#up' do + it 'copies the security_dashboard feature gate to a new instance_security_dashboard gate' do + feature_gates.create!(feature_key: :security_dashboard, key: :actors, value: 'Project:1') + feature_gates.create!(feature_key: :security_dashboard, key: :boolean, value: 'false') + + migration.up + + instance_security_dashboard_feature = feature_gates.find_by(feature_key: :instance_security_dashboard, key: :boolean) + expect(instance_security_dashboard_feature.value).to eq('false') + end + + context 'when there is no security_dashboard gate' do + it 'does nothing' do + migration.up + + instance_security_dashboard_feature = feature_gates.find_by(feature_key: :instance_security_dashboard, key: :boolean) + expect(instance_security_dashboard_feature).to be_nil + end + end + + context 'when there is already an instance_security_dashboard gate' do + it 'does nothing' do + feature_gates.create!(feature_key: :security_dashboard, key: 'boolean', value: 'false') + feature_gates.create!(feature_key: :instance_security_dashboard, key: 'boolean', value: 'false') + + expect { migration.up }.not_to raise_error + end + end + end + + describe '#down' do + it 'removes the instance_security_dashboard gate' do + actors_instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: :actors, value: 'Project:1') + instance_security_dashboard_feature = feature_gates.create!(feature_key: :instance_security_dashboard, key: :boolean, value: 'false') + + migration.down + + expect { instance_security_dashboard_feature.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(actors_instance_security_dashboard_feature.reload).to be_present + end + end +end