5.x devise-two-factor upgrade phase 1

.env.example

@@ -21,3 +21,6 @@ NOTIFY_KEY=
 NOTIFY_WELCOME_EMAIL_TEMPLATE=b1282182-887e-4c8a-8a39-649c9215cd41
 NOTIFY_VIEW_TEMPLATE=b541df04-add8-458e-a7d3-2e156386e150
 NOTIFY_OTP_VERIFICATION_TEMPLATE=1a832f2e-b13b-47f0-b32a-9cd6672364d2
+ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY=yO1HDsDP6Eu5y7zkgqD97T6w5U4xmhH2
+ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY=AS3X6Ikv2qCawCIYKJpIe5NHJXFaYLg3
+ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT=gCtDin0pxDs3jE6OyzC1oTooNa0YICcF

.env.test

@@ -13,3 +13,6 @@ REDIS_URL=redis://localhost:6379
 SECRET_KEY_BASE=abcdefghijklmnopqrstuvwxyz12345678
 NOTIFY_VIEW_TEMPLATE=b541df04-add8-458e-a7d3-2e156386e150
 NOTIFY_OTP_VERIFICATION_TEMPLATE=1a832f2e-b13b-47f0-b32a-9cd6672364d2
+ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY=yO1HDsDP6Eu5y7zkgqD97T6w5U4xmhH2
+ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY=AS3X6Ikv2qCawCIYKJpIe5NHJXFaYLg3
+ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT=gCtDin0pxDs3jE6OyzC1oTooNa0YICcF

Gemfile

@@ -45,7 +45,7 @@ gem "sprockets-rails"
 
 # Authentication
 gem "devise"
-gem "devise-two-factor"
+gem "devise-two-factor", "~> 5.1"
 gem "devise-security"
 
 group :development, :test do

Gemfile.lock

@@ -71,8 +71,6 @@ GEM
     addressable (2.8.6)
       public_suffix (>= 2.0.2, < 6.0)
     ast (2.4.2)
-    attr_encrypted (4.0.0)
-      encryptor (~> 3.0.0)
     attr_extras (7.1.0)
     audited (5.4.3)
       activerecord (>= 5.0, < 7.2)
@@ -154,19 +152,17 @@ GEM
       warden (~> 1.2.3)
     devise-security (0.18.0)
       devise (>= 4.3.0)
-    devise-two-factor (4.1.0)
-      activesupport (< 7.1)
-      attr_encrypted (>= 1.3, < 5, != 2)
+    devise-two-factor (5.1.0)
+      activesupport (~> 7.0)
       devise (~> 4.0)
-      railties (< 7.1)
+      railties (~> 7.0)
       rotp (~> 6.0)
     diff-lcs (1.5.1)
     docile (1.4.0)
     dotenv (3.1.7)
     dotenv-rails (3.1.7)
       dotenv (= 3.1.7)
       railties (>= 6.1)
-    encryptor (3.0.0)
     erubi (1.13.1)
     erubis (2.7.0)
     factory_bot (6.5.0)
@@ -572,7 +568,7 @@ DEPENDENCIES
   database_cleaner
   devise
   devise-security
-  devise-two-factor
+  devise-two-factor (~> 5.1)
   dotenv-rails
   factory_bot_rails
   faker

app/models/user.rb

@@ -76,4 +76,78 @@ def email_cannot_be_changed_after_create
       errors.add(:email, :cannot_be_changed)
     end
   end
+
+  # :nocov:
+  ##
+  # Decrypt and return the `encrypted_otp_secret` attribute which was used in
+  # versions of devise-two-factor < 5.x. In practice this will be in use for the
+  # gap between deployment of 5.x and the running of
+  # db/data/20250117151047_regenerate_otp_secrets.rb, and will be removed in
+  # the very next release. Lifted from
+  # https://github.com/devise-two-factor/devise-two-factor/blob/main/UPGRADING.md
+  # @return [String] The decrypted OTP secret
+  def legacy_otp_secret
+    return nil unless self[:encrypted_otp_secret]
+    return nil unless self.class.otp_secret_encryption_key
+
+    hmac_iterations = 2000 # a default set by the Encryptor gem
+    key = self.class.otp_secret_encryption_key
+    salt = Base64.decode64(encrypted_otp_secret_salt)
+    iv = Base64.decode64(encrypted_otp_secret_iv)
+
+    raw_cipher_text = Base64.decode64(encrypted_otp_secret)
+    # The last 16 bytes of the ciphertext are the authentication tag - we use
+    # Galois Counter Mode which is an authenticated encryption mode
+    cipher_text = raw_cipher_text[0..-17]
+    auth_tag = raw_cipher_text[-16..-1] # standard:disable Style/SlicingWithRange
+
+    # this algorithm lifted from
+    # https://github.com/attr-encrypted/encryptor/blob/master/lib/encryptor.rb#L54
+
+    # create an OpenSSL object which will decrypt the AES cipher with 256 bit
+    # keys in Galois Counter Mode (GCM). See
+    # https://ruby.github.io/openssl/OpenSSL/Cipher.html
+    cipher = OpenSSL::Cipher.new("aes-256-gcm")
+
+    # tell the cipher we want to decrypt. Symmetric algorithms use a very
+    # similar process for encryption and decryption, hence the same object can
+    # do both.
+    cipher.decrypt
+
+    # Use a Password-Based Key Derivation Function to generate the key actually
+    # used for encryption from the key we got as input.
+    cipher.key = OpenSSL::PKCS5.pbkdf2_hmac_sha1(key, salt, hmac_iterations, cipher.key_len)
+
+    # set the Initialization Vector (IV)
+    cipher.iv = iv
+
+    # The tag must be set after calling Cipher#decrypt, Cipher#key= and
+    # Cipher#iv=, but before calling Cipher#final. After all decryption is
+    # performed, the tag is verified automatically in the call to Cipher#final.
+    #
+    # If the auth_tag does not verify, then #final will raise OpenSSL::Cipher::CipherError
+    cipher.auth_tag = auth_tag
+
+    # auth_data must be set after auth_tag has been set when decrypting See
+    # http://ruby-doc.org/stdlib-2.0.0/libdoc/openssl/rdoc/OpenSSL/Cipher.html#method-i-auth_data-3D
+    # we are not adding any authenticated data but OpenSSL docs say this should
+    # still be called.
+    cipher.auth_data = ""
+
+    # #update is (somewhat confusingly named) the method which actually
+    # performs the decryption on the given chunk of data. Our OTP secret is
+    # short so we only need to call it once.
+    #
+    # It is very important that we call #final because:
+    #
+    # 1. The authentication tag is checked during the call to #final
+    # 2. Block based cipher modes (e.g. CBC) work on fixed size chunks. We need
+    #    to call #final to get it to process the last chunk properly. The output
+    #    of #final should be appended to the decrypted value. This isn't
+    #    required for streaming cipher modes but including it is a best practice
+    #    so that your code will continue to function correctly even if you later
+    #    change to a block cipher mode.
+    cipher.update(cipher_text) + cipher.final
+  end
+  # :nocov:
 end

config/application.rb

@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 require_relative "boot"
 
 require "rails"
@@ -72,6 +70,10 @@ class Application < Rails::Application
 
     config.time_zone = "London"
 
+    config.active_record.encryption.primary_key = ENV["ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY"]
+    config.active_record.encryption.deterministic_key = ENV["ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY"]
+    config.active_record.encryption.key_derivation_salt = ENV["ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT"]
+
     # Default headers
     config.action_dispatch.default_headers["X-XSS-Protection"] = "0"
 

db/data/20250117151047_regenerate_otp_secrets.rb

@@ -0,0 +1,14 @@
+# Run me with `rails runner db/data/20250117151047_regenerate_otp_secrets.rb`
+
+# On migrating from devise-two-factor 4.x to 5.x, we need to regenerate all our OTP secrets.
+# 4.x used three columns, encrypted_otp_secret, encrypted_otp_secret_iv, and encrypted_otp_secret_salt.
+# 5.x uses a single JSON-containing VARCHAR containing multiple values, for example:
+# {"p":"yrnrIYTCt+/YGEg2F8QCfieMHJ02zjEi","h":{"iv":"fmmU6Jx5f+XEg9u0","at":"LozZUSLUGwo6US0sW39Vmw==","e":"QVNDSUktOEJJVA=="}}
+#
+# Once `otp_secret` has been populated, we can do a Phase 2 release to remove the old encrypted_otp_secret,
+# encrypted_otp_secret_iv, and encrypted_otp_secret_salt columns.
+User.transaction do
+  User.find_each do |user|
+    user.update!(otp_secret: User.generate_otp_secret)
+  end
+end

db/migrate/20250116172647_add_otp_secret_to_user.rb

@@ -0,0 +1,5 @@
+class AddOtpSecretToUser < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :otp_secret, :string
+  end
+end

db/schema.rb

@@ -10,15 +10,15 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[6.1].define(version: 2024_12_12_115356) do
+ActiveRecord::Schema[7.0].define(version: 2025_01_16_172647) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
 
   create_table "activities", id: :uuid, default: -> { "public.gen_random_uuid()" }, force: :cascade do |t|
     t.uuid "organisation_id"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.string "partner_organisation_identifier"
     t.string "sector"
     t.string "title"
@@ -97,8 +97,8 @@
     t.uuid "adjustment_id"
     t.uuid "user_id"
     t.string "adjustment_type"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.index ["adjustment_id"], name: "index_adjustment_details_on_adjustment_id"
     t.index ["adjustment_type"], name: "index_adjustment_details_on_adjustment_type"
     t.index ["user_id"], name: "index_adjustment_details_on_user_id"
@@ -118,7 +118,7 @@
     t.text "comment"
     t.string "remote_address"
     t.string "request_uuid"
-    t.datetime "created_at"
+    t.datetime "created_at", precision: nil
     t.index ["associated_type", "associated_id"], name: "associated_index"
     t.index ["auditable_type", "auditable_id", "version"], name: "auditable_index"
     t.index ["created_at"], name: "index_audits_on_created_at"
@@ -141,8 +141,8 @@
     t.string "providing_organisation_type"
     t.string "providing_organisation_reference"
     t.uuid "providing_organisation_id"
-    t.datetime "created_at", precision: 6
-    t.datetime "updated_at", precision: 6
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.index ["parent_activity_id"], name: "index_budgets_on_parent_activity_id"
     t.index ["providing_organisation_id"], name: "index_budgets_on_providing_organisation_id"
     t.index ["report_id"], name: "index_budgets_on_report_id"
@@ -152,8 +152,8 @@
     t.uuid "commentable_id"
     t.string "commentable_type"
     t.text "body"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.uuid "owner_id"
     t.uuid "report_id"
     t.index ["commentable_id"], name: "index_comments_on_commentable_id"
@@ -165,8 +165,8 @@
   create_table "commitments", id: :uuid, default: -> { "public.gen_random_uuid()" }, force: :cascade do |t|
     t.decimal "value", precision: 13, scale: 2
     t.uuid "activity_id"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.date "transaction_date"
     t.index ["activity_id"], name: "index_commitments_on_activity_id", unique: true
   end
@@ -178,8 +178,8 @@
     t.integer "financial_quarter"
     t.integer "financial_year"
     t.boolean "oda_funding"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.index ["activity_id"], name: "index_external_incomes_on_activity_id"
     t.index ["organisation_id"], name: "index_external_incomes_on_organisation_id"
   end
@@ -198,8 +198,8 @@
     t.string "receiving_organisation_reference"
     t.boolean "ingested", default: false
     t.uuid "parent_activity_id", null: false
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.uuid "report_id"
     t.integer "financial_quarter", null: false
     t.integer "financial_year", null: false
@@ -216,8 +216,8 @@
     t.text "new_value"
     t.text "previous_value"
     t.text "reference"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.uuid "report_id"
     t.string "trackable_id"
     t.string "trackable_type"
@@ -234,8 +234,8 @@
     t.decimal "value", precision: 13, scale: 2, null: false
     t.integer "financial_year"
     t.integer "financial_quarter"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.string "beis_identifier"
     t.index ["destination_id"], name: "index_incoming_transfers_on_destination_id"
     t.index ["report_id"], name: "index_incoming_transfers_on_report_id"
@@ -252,8 +252,8 @@
     t.decimal "exchange_rate", precision: 14, scale: 12
     t.date "date_of_exchange_rate"
     t.text "notes"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.index ["activity_id"], name: "index_matched_efforts_on_activity_id"
     t.index ["organisation_id"], name: "index_matched_efforts_on_organisation_id"
   end
@@ -262,8 +262,8 @@
     t.uuid "organisation_id"
     t.uuid "activity_id"
     t.integer "role", null: false
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.index ["activity_id"], name: "index_org_participations_on_activity_id"
     t.index ["organisation_id", "activity_id", "role"], name: "idx_org_participations_on_org_and_act_and_role", unique: true
     t.index ["organisation_id"], name: "index_org_participations_on_organisation_id"
@@ -275,8 +275,8 @@
     t.string "organisation_type"
     t.string "language_code"
     t.string "default_currency"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.string "iati_reference"
     t.string "beis_organisation_reference"
     t.integer "role"
@@ -310,13 +310,13 @@
     t.string "description"
     t.uuid "fund_id"
     t.uuid "organisation_id"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.date "deadline"
     t.integer "financial_quarter"
     t.integer "financial_year"
     t.string "export_filename"
-    t.datetime "approved_at"
+    t.datetime "approved_at", precision: nil
     t.boolean "is_oda"
     t.index ["fund_id", "organisation_id", "is_oda"], name: "enforce_one_editable_report_per_series", unique: true, where: "((state)::text <> 'approved'::text)"
     t.index ["fund_id", "organisation_id"], name: "enforce_one_historic_report_per_series", unique: true, where: "((financial_quarter IS NULL) OR (financial_year IS NULL))"
@@ -332,8 +332,8 @@
     t.decimal "value", precision: 13, scale: 2
     t.string "disbursement_channel"
     t.string "currency"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.string "providing_organisation_name"
     t.string "providing_organisation_type"
     t.string "receiving_organisation_name"
@@ -354,22 +354,23 @@
   create_table "users", id: :uuid, default: -> { "public.gen_random_uuid()" }, force: :cascade do |t|
     t.string "name"
     t.string "email"
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
     t.uuid "organisation_id"
     t.string "encrypted_password", default: "", null: false
     t.string "reset_password_token"
-    t.datetime "reset_password_sent_at"
-    t.datetime "remember_created_at"
+    t.datetime "reset_password_sent_at", precision: nil
+    t.datetime "remember_created_at", precision: nil
     t.string "encrypted_otp_secret"
     t.string "encrypted_otp_secret_iv"
     t.string "encrypted_otp_secret_salt"
     t.integer "consumed_timestep"
     t.boolean "otp_required_for_login", default: true
     t.string "mobile_number"
-    t.datetime "mobile_number_confirmed_at"
-    t.datetime "deactivated_at"
-    t.datetime "anonymised_at"
+    t.datetime "mobile_number_confirmed_at", precision: nil
+    t.datetime "deactivated_at", precision: nil
+    t.datetime "anonymised_at", precision: nil
+    t.string "otp_secret"
     t.index ["organisation_id"], name: "index_users_on_organisation_id"
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
   end