Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5.x devise-two-factor upgrade phase 1 #2501

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

rgarner
Copy link
Contributor

@rgarner rgarner commented Jan 16, 2025

Post-release

Run:

rails runner db/data/20250117151047_regenerate_otp_secrets.rb on each environment in turn.

User#legacy_otp_secret will cover the period that "new" otp_secrets are NULL.

Phase 2 Cleanup release

We can remove the old DB columns and legacy_otp_secret in a separate release:

https://github.com/devise-two-factor/devise-two-factor/blob/main/UPGRADING.md#phase-2-clean-up

In our case we won't need to copy otp_secret; because we're SMS-only we're able to do that in the Phase 1 release data
migration.

NOTES:

  • 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

Changes in this PR

Screenshots of UI changes

Before

After

Next steps

See the Plan checklist on https://trello.com/c/sXsThu5A/3099-upgrade-devise-two-factor-gem-from-4x-to-5x

  • Is an ADR required? An ADR should be added if this PR introduces a change to the architecture.
  • Is a changelog entry required? An entry should always be made in CHANGELOG.md, unless this PR is a small tweak which has no impact outside the development team.
  • Do any environment variables need amending or adding?
  • Have any changes to the XML been checked with the IATI validator? See XML Validation

@rgarner rgarner force-pushed the 3099-upgrade-devise-two-factor-to-5x branch 4 times, most recently from cc3d575 to 5ad9dce Compare January 20, 2025 15:40
@rgarner rgarner marked this pull request as ready for review January 20, 2025 15:43
@rgarner rgarner changed the title 5.x devise-two-factor upgrade phase 1 DO NOT MERGE: 5.x devise-two-factor upgrade phase 1 Jan 20, 2025
@rgarner rgarner force-pushed the 3099-upgrade-devise-two-factor-to-5x branch 2 times, most recently from 1c31b24 to c2d6f80 Compare January 24, 2025 08:53
@rgarner rgarner requested a review from benshimmin January 24, 2025 10:29
Copy link
Contributor

@benshimmin benshimmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @rgarner, this looks really tidy. Jazda! 👍

@rgarner rgarner changed the title DO NOT MERGE: 5.x devise-two-factor upgrade phase 1 5.x devise-two-factor upgrade phase 1 Feb 3, 2025
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

Prior to release of this, we need to generate
`ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY`,
`ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY`, and
`ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT` for each environment,
and make sure they are preloaded in AWS. These are required by Rails 7
for https://guides.rubyonrails.org/active_record_encryption.html which
`devise_two_factor` 5.x uses to encrypt `otp_secret`s (4.x used the
`attr_encrypted` gem).

`bin/rails db:encryption:init` will generate all three; they are
assigned on startup in `application.rb`.

Run:

`rails runner db/data/20250117151047_regenerate_otp_secrets.rb` on
each environment in turn.

`legacy_otp_secret` will cover the period that "new" otp_secrets are
NULL.

We can remove the old DB columns and `legacy_otp_secret` in a separate
release:

https://github.com/devise-two-factor/devise-two-factor/blob/main/UPGRADING.md#phase-2-clean-up

In our case we won't need to copy otp_secret; because we're SMS-only
we're able to do that in the Phase 1 release data migration.

NOTES:

- 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
@rgarner rgarner force-pushed the 3099-upgrade-devise-two-factor-to-5x branch from c2d6f80 to c35a150 Compare February 3, 2025 10:40
@rgarner rgarner merged commit edb662b into develop Feb 3, 2025
3 checks passed
@rgarner rgarner deleted the 3099-upgrade-devise-two-factor-to-5x branch February 3, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants