Support for Rails 7

[snicky]

Currently, the last supported Rails version is < 6.2:
https://github.com/tinfoil/devise-two-factor/blob/ecd182e97e8e92436bd908af30165c18f17fe67e/devise-two-factor.gemspec#L24-L25

Upgrading to Rails 7 requires downgrading the gem to 3.0.0.

[elsurudo]

Btw bundle install works when you use the latest master: gem 'devise-two-factor', git: 'https://github.com/tinfoil/devise-two-factor.git'

However, yeah, it's be nice if there was a new release. I'm having other issues after bundle, anyways. If you do get it working, let me know.

[eoinkelly]

I have made a start on Rails 7 support in main...eoinkelly:rails-7-support . The code isn't polished enough for a PR yet but I'd love some feedback from maintainers here on whether my approach is something they would be happy to consider.

My approach is:

Overview

Remove the dependency on attr_encrypted gem and use Rails 7 encrypted attributes instead. attr_encrypted is not currently maintained and does not support Rails7. I think it's functionality has been largely superseded by the Rails 7 encrypted attributes.

Handling upgrades

Inline a method to fetch and decrypt the "legacy" otp_secret from the attr_encrypted fields. This allows us to read from the Rails7 encrypted attribute if it is available, falling back to the legacy encrypted attribute. Writes to otp_secret= always use the Rails7 encrypted attribute. This means that the OTP secret will be copied to the Rails7 attribute whenever it is changed.

The following migration will be required in the app in tandem with this upgrade:

class AddOtpSecretToUser < ActiveRecord::Migration[7.0]
  def change
    # this attribute encrypted by Rails7
    add_column :users, :otp_secret, :string
  end
end

I will create a generator for this.

I plan to include a rake task to copy the secret from old to new in all rows of the users table. Using this will allow dev teams to remove the old encrypted_otp_secret, encrypted_otp_secret_iv, encrypted_otp_secret_salt columns. Using the rake task is recommended but not required.

These changes feel big enough to require a major version bump so I've done that in my PR but obviously happy to go along with whatever the maintainers want.

[elsurudo]

Btw check out this related thread: #192

There is also a (working) rails 7 branch linked in there. Perhaps you guys can join forces.

[geoffharcourt]

A note here: we've been using @eoinkelly 's branch successfully for several weeks.

[andyjeffries]

I've been using cybersecuricy's branch successfully for about the same period, as described at #204 up until the recent CVE came out for Devise Two Factor. We really need the upstream fixed with all the fixes (CVE and Rails 7 support).

[elsurudo]

Yes, I can also confirm to be using an offshoot of the cybersecuricy branch in development/testing for now, but it would be nice if rails 7 (+ CVE) were merged upstream 🤔

Not sure I want to release MFA in my app without this, as it's kind of building on a shaky foundation.

[andyjeffries]

I submitted #211 which has the CVE fix and Rails 7 proper support both included.

[rmaspero]

Any update on this? Keen to upgrade to rails 7 but want to work off main rather than a branch

[kzkn]

I think @eoinkelly 's approach is better. It can transparently handle the current version of encrypted_otp_secret. It makes upgrading easier.

[andyjeffries]

To be clear, I don't mind the mechanism, but Rails 7 has been out a while so the gem should support it out of the box by now... (not really complaining, just saying the maintainer should make a decision and get it working/merged - if not, reach out to the community for additional help maintaining it, if they're too busy)

[eoinkelly]

I created #214 which has my take on Rails 7 support. I'm not at all bothered which Rails 7 PR lands but it would be awesome to get one of them landed. I'd like to +1 what @andyjeffries said above that if the maintainers need some help on this there are some folks around (myself included) who can help. ❤️ your work on creating and maintaining this gem!

[eoinkelly]

Heads up @geoffharcourt (and any other folks who may be using my branch):

At the suggestion of the maintainer, I'm about to remove the implementation of legacy_otp_secret from this gem and provide it in the upgrade guide as something you copy & paste into your User model (or whatever name your user model has).

Basically, just copy https://github.com/eoinkelly/devise-two-factor/blob/c26ffee689f3d4d1f05d22422aa8bd55e3ff4a9f/lib/devise_two_factor/models/two_factor_authenticatable.rb#L22-L88 into your user model and everything will continue working as before.

[helmiattastify]

any updates?

[eoinkelly]

@helmiattastify #214

[bsedat]

#214 has landed! Rails < 7 support will continue in the v4.x branch and v5+ will use native Rails attribute encryption instead of attr_encrypted.