Commit 4736db85 authored by Mark Florian's avatar Mark Florian Committed by Adam Hegyi

Use unique feature flag name for ISD

This should allow _only_ the Instance Security Dashboard to be disabled,
without affecting the other Security Dashboards.
parent 81a5fdac
# 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
# 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
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm" enable_extension "pg_trgm"
......
...@@ -5,7 +5,7 @@ import syncWithRouter from 'ee/security_dashboard/store/plugins/sync_with_router ...@@ -5,7 +5,7 @@ import syncWithRouter from 'ee/security_dashboard/store/plugins/sync_with_router
import createStore from 'ee/security_dashboard/store'; import createStore from 'ee/security_dashboard/store';
import InstanceSecurityDashboard from 'ee/security_dashboard/components/instance_security_dashboard.vue'; 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', () => { document.addEventListener('DOMContentLoaded', () => {
const el = document.querySelector('#js-security'); const el = document.querySelector('#js-security');
const { const {
......
...@@ -6,13 +6,13 @@ module Security ...@@ -6,13 +6,13 @@ module Security
before_action :check_feature_enabled! before_action :check_feature_enabled!
before_action do before_action do
push_frontend_feature_flag(:security_dashboard, default_enabled: true) push_frontend_feature_flag(:instance_security_dashboard, default_enabled: true)
end end
protected protected
def check_feature_enabled! 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 end
def vulnerable def vulnerable
......
...@@ -64,7 +64,7 @@ module EE ...@@ -64,7 +64,7 @@ module EE
def security_dashboard_available? def security_dashboard_available?
security_dashboard = InstanceSecurityDashboard.new(current_user) 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) && security_dashboard.feature_available?(:security_dashboard) &&
can?(current_user, :read_instance_security_dashboard, security_dashboard) can?(current_user, :read_instance_security_dashboard, security_dashboard)
end end
......
...@@ -155,7 +155,7 @@ describe Security::ProjectsController do ...@@ -155,7 +155,7 @@ describe Security::ProjectsController do
context 'and the security dashboard feature is disabled' do context 'and the security dashboard feature is disabled' do
it '404s' do it '404s' do
stub_feature_flags(security_dashboard: false) stub_feature_flags(instance_security_dashboard: false)
subject subject
......
...@@ -13,13 +13,11 @@ describe DashboardHelper, type: :helper do ...@@ -13,13 +13,11 @@ describe DashboardHelper, type: :helper do
end end
describe 'analytics' do describe 'analytics' do
before do
allow(helper).to receive(:can?) { true }
end
context 'when at least one analytics feature is enabled' do context 'when at least one analytics feature is enabled' do
before do before do
enable_only_one_analytics_feature_flag enable_only_one_analytics_feature_flag
stub_user_permissions_for(:analytics, false)
end end
it 'includes analytics' do it 'includes analytics' do
...@@ -34,7 +32,7 @@ describe DashboardHelper, type: :helper do ...@@ -34,7 +32,7 @@ describe DashboardHelper, type: :helper do
context 'and the user has no access to instance statistics features' do context 'and the user has no access to instance statistics features' do
before do before do
allow(helper).to receive(:can?) { false } stub_user_permissions_for(:analytics, false)
end end
it 'does not include analytics' do it 'does not include analytics' do
...@@ -43,6 +41,10 @@ describe DashboardHelper, type: :helper do ...@@ -43,6 +41,10 @@ describe DashboardHelper, type: :helper do
end end
context 'and the user has access to instance statistics features' do 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 it 'does include analytics' do
expect(helper.dashboard_nav_links).to include(:analytics) expect(helper.dashboard_nav_links).to include(:analytics)
end end
...@@ -50,96 +52,162 @@ describe DashboardHelper, type: :helper do ...@@ -50,96 +52,162 @@ describe DashboardHelper, type: :helper do
end end
end end
describe 'operations, environments and security' do describe 'operations dashboard link' do
using RSpec::Parameterized::TableSyntax 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 it 'is included in the nav' do
allow(helper).to receive(:can?).and_return(false) 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 end
where(:ability, :feature_flag, :nav_link) do context 'when the feature is not available on the license' do
:read_operations_dashboard | nil | :operations before do
:read_operations_dashboard | :environments_dashboard | :environments stub_user_permissions_for(:operations, false)
:read_instance_security_dashboard | :security_dashboard | :security end
it 'is not included in the nav' do
expect(helper.dashboard_nav_links).not_to include(:operations)
end
end end
end
with_them do describe 'environments dashboard link' do
describe 'when the feature is enabled' do context 'when the feature is enabled' do
before do before do
stub_feature_flags(feature_flag => true) unless feature_flag.nil? stub_feature_flags(environments_dashboard: true)
end end
context 'and the feature is available on the license' do context 'and the feature is available on the license' do
context 'and the user is authenticated' do context 'and the user is authenticated' do
before do before do
stub_resource_visibility( stub_user_permissions_for(:operations, true)
feature_flag, end
read_other_resources: true,
read_security_dashboard: true, it 'is included in the nav' do
security_dashboard_available: true expect(helper.dashboard_nav_links).to include(:environments)
) end
end end
it 'includes the nav link' do context 'and the user is not authenticated' do
expect(helper.dashboard_nav_links).to include(nav_link) before do
end stub_user_permissions_for(:operations, false)
end end
context 'and the user is not authenticated' do it 'is not included in the nav' do
let(:user) { nil } expect(helper.dashboard_nav_links).not_to include(:environments)
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
end end
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 before do
stub_resource_visibility( stub_user_permissions_for(:security, true)
feature_flag,
read_other_resources: false,
read_security_dashboard: true,
security_dashboard_available: false
)
end end
it 'does not include the nav link' do it 'is included in the nav' do
expect(helper.dashboard_nav_links).not_to include(nav_link) expect(helper.dashboard_nav_links).to include(:security)
end end
end end
def stub_resource_visibility(feature_flag, read_other_resources:, read_security_dashboard:, security_dashboard_available:) context 'and the user is not authenticated' do
if feature_flag == :security_dashboard before do
stub_licensed_features(feature_flag => security_dashboard_available) stub_user_permissions_for(:security, false)
end
allow(helper).to receive(:can?).and_return(read_security_dashboard) it 'is not included in the nav' do
else expect(helper.dashboard_nav_links).not_to include(:security)
allow(helper).to receive(:can?).with(user, ability).and_return(read_other_resources)
end end
end end
end end
describe 'when the feature is disabled' do context 'when the feature is not available on the license' do
before do before do
stub_feature_flags(feature_flag => false) unless feature_flag.nil? stub_licensed_features(security_dashboard: false)
allow(helper).to receive(:can?).and_return(false) stub_user_permissions_for(:security, true)
end end
it 'does not include the nav link' do it 'is not included in the nav' do
expect(helper.dashboard_nav_links).not_to include(nav_link) expect(helper.dashboard_nav_links).not_to include(:security)
end end
end 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
end end
......
...@@ -27,7 +27,7 @@ RSpec.shared_examples Security::ApplicationController do ...@@ -27,7 +27,7 @@ RSpec.shared_examples Security::ApplicationController do
context 'and the security dashboard feature is disabled' do context 'and the security dashboard feature is disabled' do
it '404s' do it '404s' do
stub_feature_flags(security_dashboard: false) stub_feature_flags(instance_security_dashboard: false)
security_application_controller_child_action security_application_controller_child_action
......
...@@ -28,7 +28,7 @@ shared_examples 'security dashboard JSON endpoint' do ...@@ -28,7 +28,7 @@ shared_examples 'security dashboard JSON endpoint' do
context 'and the security dashboard feature is disabled' do context 'and the security dashboard feature is disabled' do
it '404s' do it '404s' do
stub_feature_flags(security_dashboard: false) stub_feature_flags(instance_security_dashboard: false)
security_dashboard_request security_dashboard_request
......
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment