Merge pull request #4 from opf/feature/ee

Fix tests for EE

[ci skip]

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:

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,

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)

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

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) { [] }

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 }

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) }

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

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 }

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}

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}

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}

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}

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}

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}

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 }

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}

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

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 }

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 }

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")

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

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 }

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 }

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}

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 }

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