From 2768051d164c8cca746728a61fd24f3b91796e27 Mon Sep 17 00:00:00 2001 From: Russell Garner Date: Thu, 16 Jan 2025 18:06:50 +0000 Subject: [PATCH] 5.x devise-two-factor upgrade phase 1 All non-Rails 7 steps already completed from https://github.com/devise-two-factor/devise-two-factor/blob/main/UPGRADING.md#phase-1-upgrading-devise-two-factor-as-part-of-rails-7-upgrade Although given that this is the first commit to `schema.rb` since the upgrade, some bits like `precision` go away with the Rails 7.0 upgrade. --- .env.example | 3 + .env.test | 3 + Gemfile | 2 +- Gemfile.lock | 12 +-- app/models/user.rb | 68 ++++++++++++++++ config/application.rb | 6 +- .../20250116172647_add_otp_secret_to_user.rb | 5 ++ db/schema.rb | 77 ++++++++++--------- 8 files changed, 127 insertions(+), 49 deletions(-) create mode 100644 db/migrate/20250116172647_add_otp_secret_to_user.rb diff --git a/.env.example b/.env.example index ac240d62a..74e8f9ed8 100644 --- a/.env.example +++ b/.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 diff --git a/.env.test b/.env.test index e8d21b65f..b0c229130 100644 --- a/.env.test +++ b/.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 diff --git a/Gemfile b/Gemfile index b1d4d2266..1a6335845 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index ce16b820c..34d702cf5 100644 --- a/Gemfile.lock +++ b/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) audited (5.4.3) activerecord (>= 5.0, < 7.2) request_store (~> 1.2) @@ -153,11 +151,10 @@ 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) @@ -165,7 +162,6 @@ GEM 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) @@ -563,7 +559,7 @@ DEPENDENCIES database_cleaner devise devise-security - devise-two-factor + devise-two-factor (~> 5.1) dotenv-rails factory_bot_rails faker diff --git a/app/models/user.rb b/app/models/user.rb index c13cbc006..d87cbf4be 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,4 +74,72 @@ def email_cannot_be_changed_after_create errors.add(:email, :cannot_be_changed) end end + + ## + # Decrypt and return the `encrypted_otp_secret` attribute which was used in + # prior versions of devise-two-factor + # @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 end diff --git a/config/application.rb b/config/application.rb index e8e3cb69d..2f9e14ec0 100644 --- a/config/application.rb +++ b/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" diff --git a/db/migrate/20250116172647_add_otp_secret_to_user.rb b/db/migrate/20250116172647_add_otp_secret_to_user.rb new file mode 100644 index 000000000..463cbb1f5 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 6eea71ead..b08862fab 100644 --- a/db/schema.rb +++ b/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