Merge branch 'upgrade-devise-two-factor' into 'master'

Upgrade devise, devise-two-factor, and attr_encrypted

Devise 4 includes support for Rails 5, working towards #14286. devise-async doesn't support Devise 4.0 and in 4.1 the bug that was blocking using Devise's built-in ActiveJob integration was fixed. So devise-async is removed. devise-two-factor 3.0.0 is required for Devise 4 support.

attr_encrypted and encryptor are optional but recommended upgrades for devise-two-factor 3.0.0. The mode and algorithm will need to be changed in order to update to attr_encrypted 4.x in the future.

See merge request !4216

10 files changed, 34 insertions, 31 deletions

diff --git a/Gemfile b/Gemfile
index 540710712fc..0ab78c2a738 100644
--- a/Gemfile
+++ b/Gemfile
@@ -18,9 +18,8 @@ gem "mysql2", '~> 0.3.16', group: :mysql
 gem "pg", '~> 0.18.2', group: :postgres
 
 # Authentication libraries
-gem 'devise',                 '~> 3.5.4'
+gem 'devise',                 '~> 4.0'
 gem 'doorkeeper',             '~> 3.1'
-gem 'devise-async',           '~> 0.9.0'
 gem 'omniauth',               '~> 1.3.1'
 gem 'omniauth-auth0',         '~> 1.4.1'
 gem 'omniauth-azure-oauth2',  '~> 0.0.6'
@@ -43,9 +42,9 @@ gem 'recaptcha', require: 'recaptcha/rails'
 gem 'akismet', '~> 2.0'
 
 # Two-factor authentication
-gem 'devise-two-factor', '~> 2.0.0'
+gem 'devise-two-factor', '~> 3.0.0'
 gem 'rqrcode-rails3', '~> 0.1.7'
-gem 'attr_encrypted', '~> 1.3.4'
+gem 'attr_encrypted', '~> 3.0.0'
 
 # Browser detection
 gem "browser", '~> 1.0.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index 146e95167b2..4e000fa5b5b 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -60,8 +60,8 @@ GEM
       oauth2 (~> 1.0)
     asciidoctor (1.5.3)
     ast (2.2.0)
-    attr_encrypted (1.3.4)
-      encryptor (>= 1.3.0)
+    attr_encrypted (3.0.1)
+      encryptor (~> 3.0.0)
     attr_required (1.0.0)
     autoprefixer-rails (6.2.3)
       execjs
@@ -73,7 +73,7 @@ GEM
       thread_safe (~> 0.3, >= 0.3.1)
     babosa (1.0.2)
     base32 (0.3.2)
-    bcrypt (3.1.10)
+    bcrypt (3.1.11)
     benchmark-ips (2.3.0)
     better_errors (1.0.1)
       coderay (>= 1.0.0)
@@ -155,21 +155,18 @@ GEM
       activerecord (>= 3.2.0, < 5.0)
     descendants_tracker (0.0.4)
       thread_safe (~> 0.3, >= 0.3.1)
-    devise (3.5.4)
+    devise (4.1.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
-      railties (>= 3.2.6, < 5)
+      railties (>= 4.1.0, < 5.1)
       responders
-      thread_safe (~> 0.1)
       warden (~> 1.2.3)
-    devise-async (0.9.0)
-      devise (~> 3.2)
-    devise-two-factor (2.0.1)
+    devise-two-factor (3.0.0)
       activesupport
-      attr_encrypted (~> 1.3.2)
-      devise (~> 3.5.0)
+      attr_encrypted (>= 1.3, < 4, != 2)
+      devise (~> 4.0)
       railties
-      rotp (~> 2)
+      rotp (~> 2.0)
     diff-lcs (1.2.5)
     diffy (3.0.7)
     docile (1.1.5)
@@ -181,7 +178,7 @@ GEM
     email_spec (1.6.0)
       launchy (~> 2.1)
       mail (~> 2.2)
-    encryptor (1.3.0)
+    encryptor (3.0.0)
     equalizer (0.0.11)
     erubis (2.7.0)
     escape_utils (1.1.1)
@@ -656,7 +653,7 @@ GEM
     responders (2.1.1)
       railties (>= 4.2.0, < 5.1)
     rinku (1.7.3)
-    rotp (2.1.1)
+    rotp (2.1.2)
     rouge (1.10.1)
     rqrcode (0.7.0)
       chunky_png
@@ -859,7 +856,7 @@ GEM
       coercible (~> 1.0)
       descendants_tracker (~> 0.0, >= 0.0.3)
       equalizer (~> 0.0, >= 0.0.9)
-    warden (1.2.4)
+    warden (1.2.6)
       rack (>= 1.0)
     web-console (2.3.0)
       activemodel (>= 4.0)
@@ -894,7 +891,7 @@ DEPENDENCIES
   allocations (~> 1.0)
   asana (~> 0.4.0)
   asciidoctor (~> 1.5.2)
-  attr_encrypted (~> 1.3.4)
+  attr_encrypted (~> 3.0.0)
   awesome_print (~> 1.2.0)
   babosa (~> 1.0.2)
   base32 (~> 0.3.0)
@@ -919,9 +916,8 @@ DEPENDENCIES
   d3_rails (~> 3.5.0)
   database_cleaner (~> 1.4.0)
   default_value_for (~> 3.0.0)
-  devise (~> 3.5.4)
-  devise-async (~> 0.9.0)
-  devise-two-factor (~> 2.0.0)
+  devise (~> 4.0)
+  devise-two-factor (~> 3.0.0)
   diffy (~> 3.0.3)
   doorkeeper (~> 3.1)
   dropzonejs-rails (~> 0.7.1)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9b2a9d298b3..c28d1ca9e3b 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -232,7 +232,7 @@ class ApplicationController < ActionController::Base
   end
 
   def configure_permitted_parameters
-    devise_parameter_sanitizer.for(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me, :otp_attempt) }
+    devise_parameter_sanitizer.permit(:sign_in, keys: [:username, :email, :password, :login, :remember_me, :otp_attempt])
   end
 
   def hexdigest(string)
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 10802f64813..f8d5d4486fd 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -11,6 +11,9 @@ module Ci
       format: { with: /\A[a-zA-Z0-9_]+\z/,
                 message: "can contain only letters, digits and '_'." }
 
-    attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base
+    attr_encrypted :value, 
+       mode: :per_attribute_iv_and_salt,
+       key: Gitlab::Application.secrets.db_key_base,
+       algorithm: 'aes-256-cbc'
   end
 end
diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb
index e2f9ffb69ac..ca8a9b4217b 100644
--- a/app/models/project_import_data.rb
+++ b/app/models/project_import_data.rb
@@ -6,7 +6,8 @@ class ProjectImportData < ActiveRecord::Base
                  key: Gitlab::Application.secrets.db_key_base,
                  marshal: true,
                  encode: true,
-                 mode: :per_attribute_iv_and_salt
+                 mode: :per_attribute_iv_and_salt,
+                 algorithm: 'aes-256-cbc'
 
   serialize :data, JSON
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 6a09b78455b..15b6cbc2255 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -20,6 +20,11 @@ class User < ActiveRecord::Base
   default_value_for :hide_no_password, false
   default_value_for :theme_id, gitlab_config.default_theme
 
+  attr_encrypted :otp_secret,
+    key:       Gitlab::Application.config.secret_key_base,
+    mode:      :per_attribute_iv_and_salt,
+    algorithm: 'aes-256-cbc'
+
   devise :two_factor_authenticatable,
          otp_secret_encryption_key: Gitlab::Application.config.secret_key_base
   alias_attribute :two_factor_enabled, :otp_required_for_login
@@ -27,7 +32,7 @@ class User < ActiveRecord::Base
   devise :two_factor_backupable, otp_number_of_backup_codes: 10
   serialize :otp_backup_codes, JSON
 
-  devise :lockable, :async, :recoverable, :rememberable, :trackable,
+  devise :lockable, :recoverable, :rememberable, :trackable,
     :validatable, :omniauthable, :confirmable, :registerable
 
   attr_accessor :force_random_password
diff --git a/config/initializers/devise_async.rb b/config/initializers/devise_async.rb
deleted file mode 100644
index 05a1852cdbd..00000000000
--- a/config/initializers/devise_async.rb
+++ /dev/null
@@ -1 +0,0 @@
-Devise::Async.backend = :sidekiq
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index b39d8c8cd5b..5dc8724fb50 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -12,7 +12,7 @@ describe SessionsController do
           post(:create, user: { login: 'invalid', password: 'invalid' })
 
           expect(response)
-            .to set_flash.now[:alert].to /Invalid login or password/
+            .to set_flash.now[:alert].to /Invalid Login or password/
         end
       end
 
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index a7dc3b2701b..c1b178c3b6c 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -127,7 +127,7 @@ feature 'Login', feature: true do
       user = create(:user, password: 'not-the-default')
 
       login_with(user)
-      expect(page).to have_content('Invalid login or password.')
+      expect(page).to have_content('Invalid Login or password.')
     end
   end
 
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index c712d211b0f..98f60087cf5 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -23,7 +23,7 @@ describe Ci::Variable, models: true do
     end
 
     it 'fails to decrypt if iv is incorrect' do
-      subject.encrypted_value_iv = nil
+      subject.encrypted_value_iv = SecureRandom.hex
       subject.instance_variable_set(:@value, nil)
       expect { subject.value }.
         to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt')