diff --git a/Gemfile.plugins b/Gemfile.plugins new file mode 100644 index 0000000000..d3edbb6aae --- /dev/null +++ b/Gemfile.plugins @@ -0,0 +1,7 @@ +# Used by travis to bundle this plugin with the OpenProject core. +# The tested plugin will be moved to the path `./plugins/this` +# whereas OpenProject will be checked out to `.`. + +gem 'openproject-two_factor_authentication', path: 'plugins/this' + +# If the plugin has any dependencies declare them here: diff --git a/lib/open_project/two_factor_authentication/engine.rb b/lib/open_project/two_factor_authentication/engine.rb index 04de53b32c..b72c0a0e80 100644 --- a/lib/open_project/two_factor_authentication/engine.rb +++ b/lib/open_project/two_factor_authentication/engine.rb @@ -8,6 +8,13 @@ module OpenProject::TwoFactorAuthentication register 'openproject-two_factor_authentication', author_url: 'http://openproject.com', + settings: { + default: { + enforced: false, + active_strategies: [] + }, + partial: 'settings/openproject_two_factor_authentication' + }, requires_openproject: '>= 4.0.0' do menu :my_menu, :two_factor_authentication, diff --git a/lib/open_project/two_factor_authentication/token_strategy_manager.rb b/lib/open_project/two_factor_authentication/token_strategy_manager.rb index 54a8622e93..4ede4b42b1 100644 --- a/lib/open_project/two_factor_authentication/token_strategy_manager.rb +++ b/lib/open_project/two_factor_authentication/token_strategy_manager.rb @@ -34,7 +34,7 @@ module OpenProject::TwoFactorAuthentication ## # Whether any active strategy exists def enabled? - !active_strategies.empty? + !active_strategies.empty? && EnterpriseToken.allows_to?(:two_factor_authentication) end ## @@ -55,7 +55,6 @@ module OpenProject::TwoFactorAuthentication enforced? && !enabled? end - ## # Fetch all active strategies def active_strategies @@ -84,7 +83,24 @@ module OpenProject::TwoFactorAuthentication ## # 2FA Plugin configuration def configuration - OpenProject::Configuration['2fa'] || {} + config = OpenProject::Configuration['2fa'] || {} + settings = Setting.plugin_openproject_two_factor_authentication || {} + + merge_with_settings! config, settings + + config + end + + def merge_with_settings!(config, settings) + # Allow enforcing from settings if not true in configuration + unless config[:enforced] + config[:enforced] = settings[:enforced] + end + + predefined_strategies = config.fetch(:active_strategies, []) + additional_strategies = settings.fetch(:active_strategies, []) + + config[:active_strategies] = predefined_strategies | additional_strategies end def lookup_active_strategy(klazz) diff --git a/spec/controllers/two_factor_authentication/authentication_controller_spec.rb b/spec/controllers/two_factor_authentication/authentication_controller_spec.rb index 2c5382113e..950725d488 100644 --- a/spec/controllers/two_factor_authentication/authentication_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/authentication_controller_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative './authentication_controller_shared_examples' -describe ::TwoFactorAuthentication::AuthenticationController, with_settings: { login_required?: true } do +describe ::TwoFactorAuthentication::AuthenticationController, with_2fa_ee: true, with_settings: { login_required?: true } do let(:valid_credentials) do { username: 'foobar', password: 'AAA1111!!!!' } end diff --git a/spec/controllers/two_factor_authentication/forced_registration/two_factor_devices_controller_spec.rb b/spec/controllers/two_factor_authentication/forced_registration/two_factor_devices_controller_spec.rb index fca12f3f7e..f3b142ea50 100644 --- a/spec/controllers/two_factor_authentication/forced_registration/two_factor_devices_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/forced_registration/two_factor_devices_controller_spec.rb @@ -1,7 +1,7 @@ require_relative '../../../spec_helper' require_relative './../authentication_controller_shared_examples' -describe ::TwoFactorAuthentication::ForcedRegistration::TwoFactorDevicesController do +describe ::TwoFactorAuthentication::ForcedRegistration::TwoFactorDevicesController, with_2fa_ee: true do let(:user) { FactoryGirl.create(:user, login: 'foobar') } let(:logged_in_user) { User.anonymous } let(:active_strategies) { [] } diff --git a/spec/controllers/two_factor_authentication/my/two_factor_devices_controller_spec.rb b/spec/controllers/two_factor_authentication/my/two_factor_devices_controller_spec.rb index e62ca93149..2a0a7d9ad0 100644 --- a/spec/controllers/two_factor_authentication/my/two_factor_devices_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/my/two_factor_devices_controller_spec.rb @@ -1,7 +1,7 @@ require_relative '../../../spec_helper' require_relative './../authentication_controller_shared_examples' -describe ::TwoFactorAuthentication::My::TwoFactorDevicesController do +describe ::TwoFactorAuthentication::My::TwoFactorDevicesController, with_2fa_ee: true do let(:user) { FactoryGirl.create(:user, login: 'foobar') } let(:other_user) { FactoryGirl.create(:user) } let(:logged_in_user) { user } diff --git a/spec/controllers/two_factor_authentication/users/two_factor_devices_controller_spec.rb b/spec/controllers/two_factor_authentication/users/two_factor_devices_controller_spec.rb index a274076ebd..1c8498eceb 100644 --- a/spec/controllers/two_factor_authentication/users/two_factor_devices_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/users/two_factor_devices_controller_spec.rb @@ -1,7 +1,7 @@ require_relative '../../../spec_helper' require_relative './../authentication_controller_shared_examples' -describe ::TwoFactorAuthentication::Users::TwoFactorDevicesController do +describe ::TwoFactorAuthentication::Users::TwoFactorDevicesController, with_2fa_ee: true do let(:admin) { FactoryGirl.create :admin } let(:user) { FactoryGirl.create(:user, login: 'foobar') } let(:other_user) { FactoryGirl.create(:user) } diff --git a/spec/features/account_activation_spec.rb b/spec/features/account_activation_spec.rb index 0754f99fc2..cc170cdb5b 100644 --- a/spec/features/account_activation_spec.rb +++ b/spec/features/account_activation_spec.rb @@ -2,6 +2,7 @@ require_relative '../spec_helper' require_relative './shared_2fa_examples' describe 'activating an invited account', + with_2fa_ee: true, type: :feature, js: true, with_config: {:'2fa' => {active_strategies: [:developer]}} do diff --git a/spec/features/admin_edit_two_factor_devices_spec.rb b/spec/features/admin_edit_two_factor_devices_spec.rb index c6e935e322..ac0d1ef145 100644 --- a/spec/features/admin_edit_two_factor_devices_spec.rb +++ b/spec/features/admin_edit_two_factor_devices_spec.rb @@ -1,6 +1,6 @@ require_relative '../spec_helper' -describe 'Admin 2FA management', type: :feature, +describe 'Admin 2FA management', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer, :totp]}}, js: true do let(:dialog) { ::Components::PasswordConfirmationDialog.new } diff --git a/spec/features/backup_codes/generate_backup_codes.rb b/spec/features/backup_codes/generate_backup_codes.rb index 218fb88262..5d90b6d99b 100644 --- a/spec/features/backup_codes/generate_backup_codes.rb +++ b/spec/features/backup_codes/generate_backup_codes.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Generate 2FA backup codes', type: :feature, +describe 'Generate 2FA backup codes', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer]}}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/backup_codes/login_with_backup_code_spec.rb b/spec/features/backup_codes/login_with_backup_code_spec.rb index 7fd21b8403..b62c9a325f 100644 --- a/spec/features/backup_codes/login_with_backup_code_spec.rb +++ b/spec/features/backup_codes/login_with_backup_code_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Login with 2FA backup code', type: :feature, +describe 'Login with 2FA backup code', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer]}}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/login/login_enforced_2fa_spec.rb b/spec/features/login/login_enforced_2fa_spec.rb index 08f8d0e11e..dbc3aec1a8 100644 --- a/spec/features/login/login_enforced_2fa_spec.rb +++ b/spec/features/login/login_enforced_2fa_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Login with enforced 2FA', type: :feature, +describe 'Login with enforced 2FA', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer], enforced: true }}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/login/login_with_2fa_spec.rb b/spec/features/login/login_with_2fa_spec.rb index 93789af1ec..04b9f293e1 100644 --- a/spec/features/login/login_with_2fa_spec.rb +++ b/spec/features/login/login_with_2fa_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Login with 2FA device', type: :feature, +describe 'Login with 2FA device', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer]}}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/login/login_without_2fa_spec.rb b/spec/features/login/login_without_2fa_spec.rb index f486930536..1caf2c9e14 100644 --- a/spec/features/login/login_without_2fa_spec.rb +++ b/spec/features/login/login_without_2fa_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Login with no required OTP', type: :feature, +describe 'Login with no required OTP', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer]}}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/login/switch_available_devices_spec.rb b/spec/features/login/switch_available_devices_spec.rb index 2dfabb7e42..de3de60447 100644 --- a/spec/features/login/switch_available_devices_spec.rb +++ b/spec/features/login/switch_available_devices_spec.rb @@ -1,7 +1,7 @@ require_relative '../../spec_helper' require_relative '../shared_2fa_examples' -describe 'Login by switching 2FA device', type: :feature, +describe 'Login by switching 2FA device', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer, :totp]}}, js: true do let(:user_password) {'bob!' * 4} diff --git a/spec/features/my_two_factor_devices_spec.rb b/spec/features/my_two_factor_devices_spec.rb index 2c5cb6db54..4116ae5473 100644 --- a/spec/features/my_two_factor_devices_spec.rb +++ b/spec/features/my_two_factor_devices_spec.rb @@ -1,6 +1,6 @@ require_relative '../spec_helper' -describe 'My Account 2FA configuration', type: :feature, +describe 'My Account 2FA configuration', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer, :totp]}}, js: true do let(:dialog) { ::Components::PasswordConfirmationDialog.new } diff --git a/spec/features/password_change_spec.rb b/spec/features/password_change_spec.rb index e5a1b8814a..a4e3ba505f 100644 --- a/spec/features/password_change_spec.rb +++ b/spec/features/password_change_spec.rb @@ -1,6 +1,6 @@ require_relative '../spec_helper' -describe 'Password change with OTP', type: :feature, +describe 'Password change with OTP', with_2fa_ee: true, type: :feature, with_config: {:'2fa' => {active_strategies: [:developer]}}, js: true do let(:user_password) {'bob' * 4} diff --git a/spec/lib/token_strategy_manager_spec.rb b/spec/lib/token_strategy_manager_spec.rb index e40ed9515a..a13c33b0ff 100644 --- a/spec/lib/token_strategy_manager_spec.rb +++ b/spec/lib/token_strategy_manager_spec.rb @@ -1,7 +1,8 @@ -require 'spec_helper' +require_relative '../spec_helper' describe ::OpenProject::TwoFactorAuthentication::TokenStrategyManager do let(:dev_strategy) { ::OpenProject::TwoFactorAuthentication::TokenStrategy::Developer } + let(:totp_strategy) { ::OpenProject::TwoFactorAuthentication::TokenStrategy::Totp } let(:configuration) do { active_strategies: active_strategies, @@ -10,99 +11,160 @@ describe ::OpenProject::TwoFactorAuthentication::TokenStrategyManager do end let(:enforced) { false } - before do - allow(OpenProject::Configuration) - .to receive(:[]).with('2fa') - .and_return(configuration) + context 'without EE' do + before do + allow(OpenProject::Configuration) + .to receive(:[]).with('2fa') + .and_return(active_strategies: [:developer]) + end + + it 'is not enabled' do + expect(described_class).not_to be_enabled + end end - describe '#find_matching_strategy' do - subject { described_class.find_matching_strategy(:sms) } + context 'with EE', with_2fa_ee: true do + before do + allow(OpenProject::Configuration) + .to receive(:[]).with('2fa') + .and_return(configuration) + end + + describe '#find_matching_strategy' do + subject { described_class.find_matching_strategy(:sms) } - context 'when no strategy is set' do - let(:active_strategies) { [] } - it 'returns nil' do - expect(subject).to be_nil + context 'when no strategy is set' do + let(:active_strategies) { [] } + it 'returns nil' do + expect(subject).to be_nil + end end - end - context 'when matching strategy exists' do - let(:active_strategies) { [:developer] } + context 'when matching strategy exists' do + let(:active_strategies) { [:developer] } - it 'returns the strategy' do - expect(subject).to eq(dev_strategy) + it 'returns the strategy' do + expect(subject).to eq(dev_strategy) + end end - end - context 'when non-matching strategy exists' do - let(:active_strategies) { [:totp] } + context 'when non-matching strategy exists' do + let(:active_strategies) { [:totp] } - it 'returns the strategy' do - expect(subject).to eq(nil) + it 'returns the strategy' do + expect(subject).to eq(nil) + end end end - end - describe '#active_strategies' do - context 'with bogus strategy' do - let(:active_strategies) { [:doesnotexist] } + describe '#active_strategies' do + context 'with bogus strategy' do + let(:active_strategies) { [:doesnotexist] } - it 'raises when accessing' do - expect { described_class.active_strategies }.to raise_error(ArgumentError) - end + it 'raises when accessing' do + expect { described_class.active_strategies }.to raise_error(ArgumentError) + end - it 'raises when validating' do - expect { described_class.validate_active_strategies! }.to raise_error(ArgumentError) + it 'raises when validating' do + expect { described_class.validate_active_strategies! }.to raise_error(ArgumentError) + end end end - end - describe '#validate_active_strategies!' do - subject { described_class.validate_active_strategies! } - context 'when no strategy is set' do - let(:active_strategies) { [] } + describe 'with additional settings given' do + let(:active_strategies) { [:developer] } + let(:enforced) { false } + + before do + allow(Setting).to receive(:plugin_openproject_two_factor_authentication).and_return(settings) + end + + context 'when nothing given' do + let(:settings) { nil } + + it 'uses the configuration' do + expect(described_class.active_strategies).to eq([dev_strategy]) + expect(described_class).not_to be_enforced + end + end - context 'and enforced is false' do - let(:enforced) { false } + context 'when additional strategy given' do + let(:settings) { { active_strategies: [:totp] } } - it 'accepts that' do - expect { subject }.not_to raise_error - expect(described_class).not_to be_enabled + it 'merges configuration and settings' do + expect(described_class.active_strategies).to eq([dev_strategy, totp_strategy]) expect(described_class).not_to be_enforced end end - context 'and enforced is true' do - let(:enforced) { true } + context 'when enforced set' do + context 'when true and config is false' do + let(:enforced) { false } + let(:settings) { { enforced: true } } - it 'raises and error that a strategy is needed' do - expect { subject }.to raise_error(ArgumentError) - expect(described_class).not_to be_enabled - expect(described_class).to be_enforced + it 'does override the configuration' do + expect(described_class).to be_enforced + end + end + + context 'when false and config is true' do + let(:enforced) { true } + let(:settings) { { enforced: false } } + + it 'does not override the configuration' do + expect(described_class).to be_enforced + end end end end - context 'when a strategy is set' do - let(:active_strategies) { [:developer] } + describe '#validate_active_strategies!' do + subject { described_class.validate_active_strategies! } + context 'when no strategy is set' do + let(:active_strategies) { [] } - context 'and it is valid' do - it 'returns that' do - expect { subject }.not_to raise_error - expect(described_class.active_strategies).to eq([dev_strategy]) - expect(described_class).to be_enabled - expect(described_class).not_to be_enforced + context 'and enforced is false' do + let(:enforced) { false } + + it 'accepts that' do + expect { subject }.not_to raise_error + expect(described_class).not_to be_enabled + expect(described_class).not_to be_enforced + end + end + + context 'and enforced is true' do + let(:enforced) { true } + + it 'raises and error that a strategy is needed' do + expect { subject }.to raise_error(ArgumentError) + expect(described_class).not_to be_enabled + expect(described_class).to be_enforced + end end end - context 'and it is invalid' do - before do - expect(dev_strategy).to receive(:validate!).and_raise 'Error!' + context 'when a strategy is set' do + let(:active_strategies) { [:developer] } + + context 'and it is valid' do + it 'returns that' do + expect { subject }.not_to raise_error + expect(described_class.active_strategies).to eq([dev_strategy]) + expect(described_class).to be_enabled + expect(described_class).not_to be_enforced + end end - it 'raises' do - expect { subject }.to raise_error 'Error!' - expect(described_class).to be_enabled - expect(described_class).not_to be_enforced + + context 'and it is invalid' do + before do + expect(dev_strategy).to receive(:validate!).and_raise 'Error!' + end + it 'raises' do + expect { subject }.to raise_error 'Error!' + expect(described_class).to be_enabled + expect(described_class).not_to be_enforced + end end end end diff --git a/spec/models/devices/default_device_spec.rb b/spec/models/devices/default_device_spec.rb index 334d1bfb71..65948e68de 100644 --- a/spec/models/devices/default_device_spec.rb +++ b/spec/models/devices/default_device_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Default device', type: :model do +describe 'Default device', with_2fa_ee: true, type: :model do let(:user) { FactoryGirl.create :user } let(:subject) { FactoryGirl.build :two_factor_authentication_device_totp, user: user, default: true } let(:other_otp) { FactoryGirl.build :two_factor_authentication_device_totp, user: user, default: true } diff --git a/spec/models/devices/totp_spec.rb b/spec/models/devices/totp_spec.rb index 2f13f59f62..85152c15ad 100644 --- a/spec/models/devices/totp_spec.rb +++ b/spec/models/devices/totp_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'timecop' -describe ::TwoFactorAuthentication::Device::Totp, type: :model do +describe ::TwoFactorAuthentication::Device::Totp, with_2fa_ee: true, type: :model do let(:user) { FactoryGirl.create :user } let(:channel) { :totp } subject { described_class.new identifier: 'foo', channel: channel, user: user, active: true } diff --git a/spec/models/login_token_spec.rb b/spec/models/login_token_spec.rb index 21254ef47f..99585e520d 100644 --- a/spec/models/login_token_spec.rb +++ b/spec/models/login_token_spec.rb @@ -1,6 +1,6 @@ require_relative '../spec_helper' -describe TwoFactorAuthentication::LoginToken do +describe TwoFactorAuthentication::LoginToken, with_2fa_ee: true do before(:each) do @user = mock_model(User, :login => "john", :password => "doe") diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cf33c67a2d..32e31e7ef2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,7 +2,7 @@ require_relative '../spec_helper' module OpenProject::TwoFactorAuthentication::Patches module UserSpec - describe User do + describe User, with_2fa_ee: true do def create_user(auth_source_id = nil) @user = FactoryGirl.build(:user) @username = @user.login diff --git a/spec/services/token_delivery/restdt_spec.rb b/spec/services/token_delivery/restdt_spec.rb index 0f0129b5ce..0e8ea19513 100644 --- a/spec/services/token_delivery/restdt_spec.rb +++ b/spec/services/token_delivery/restdt_spec.rb @@ -1,6 +1,6 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') -describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Restdt do +describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Restdt, with_2fa_ee: true do describe 'sending messages' do let!(:user) { FactoryGirl.create :user } let!(:device) { FactoryGirl.create :two_factor_authentication_device_sms, user: user, channel: channel } diff --git a/spec/services/token_delivery/sns_spec.rb b/spec/services/token_delivery/sns_spec.rb index f8516faa83..24032b944a 100644 --- a/spec/services/token_delivery/sns_spec.rb +++ b/spec/services/token_delivery/sns_spec.rb @@ -1,6 +1,6 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') -describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Sns do +describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Sns, with_2fa_ee: true do describe 'sending messages' do let(:phone) { '+49 123456789' } let!(:user) { FactoryGirl.create :user } diff --git a/spec/services/token_delivery/totp_spec.rb b/spec/services/token_delivery/totp_spec.rb index c2e8a4ba40..deef496cb0 100644 --- a/spec/services/token_delivery/totp_spec.rb +++ b/spec/services/token_delivery/totp_spec.rb @@ -1,6 +1,6 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') -describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Totp do +describe ::OpenProject::TwoFactorAuthentication::TokenStrategy::Totp, with_2fa_ee: true do describe 'sending messages' do let!(:user) { FactoryGirl.create :user } let!(:device) { FactoryGirl.create :two_factor_authentication_device_totp, user: user, default: true} diff --git a/spec/services/token_service_spec.rb b/spec/services/token_service_spec.rb index 9aeacaf7d8..858c246f9f 100644 --- a/spec/services/token_service_spec.rb +++ b/spec/services/token_service_spec.rb @@ -1,6 +1,6 @@ require_relative '../spec_helper' -describe ::TwoFactorAuthentication::TokenService do +describe ::TwoFactorAuthentication::TokenService, with_2fa_ee: true do describe 'sending messages' do let(:user) { FactoryGirl.create(:user) } let(:dev_strategy) { ::OpenProject::TwoFactorAuthentication::TokenStrategy::Developer } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 26cb3e44be..d170945a80 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,17 @@ # -- load spec_helper from OpenProject core require "spec_helper" +RSpec.configure do |config| + config.before(:each) do |example| + next unless example.metadata[:with_2fa_ee] + + allow(EnterpriseToken) + .to receive(:allows_to?) + .and_call_original + + allow(EnterpriseToken) + .to receive(:allows_to?) + .with(:two_factor_authentication) + .and_return true + end +end \ No newline at end of file