Skip to content

Conversation

maximerety
Copy link
Contributor

@maximerety maximerety commented Aug 21, 2024

Motivation / Background

This Pull Request has been created in order to fix rspec/rspec-rails#2779.

Detail

Previously, ActiveRecord::Encryption configuration was deferred until ActiveRecord::Base was loaded. Therefore, accessing ActiveRecord::Encryption.config properties before ActiveRecord::Base was loaded would give incorrect results.

ActiveRecord::Encryption now has its own loading hook so that its configuration is set as soon as needed.

When ActiveRecord::Base is loaded, even lazily, it in turn triggers the loading of ActiveRecord::Encryption, thus preserving the original behavior of having its config ready before any use of ActiveRecord::Base.

This fix complements one of my previous fixes in #50606.

Since both Rails 7.1.4 and 7.2.* versions are affected, I'd recommend backporting this fix to 7-1-stable and 7-2-stable.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@mbaird
Copy link

mbaird commented Aug 22, 2024

Thanks for this @maximerety, I can confirm this fixes the issue in rspec/rspec-rails#2779. 🙌🏻

@maximerety maximerety force-pushed the fix-rspec-rails-issue-2779 branch 2 times, most recently from 1499846 to ee50331 Compare August 23, 2024 13:13
@rails-bot rails-bot bot added the railties label Aug 23, 2024
@maximerety maximerety marked this pull request as ready for review August 23, 2024 13:45
@maximerety maximerety force-pushed the fix-rspec-rails-issue-2779 branch 2 times, most recently from e118402 to 6c2d1f9 Compare August 24, 2024 13:20
@rails-bot rails-bot bot added the docs label Aug 24, 2024
@maximerety
Copy link
Contributor Author

@rafaelfranca Thanks for reviewing, all comments addressed 🙏

@maximerety maximerety force-pushed the fix-rspec-rails-issue-2779 branch from 6c2d1f9 to 4936e2f Compare August 26, 2024 20:57
Previously, ActiveRecord::Encryption configuration was deferred until
ActiveRecord::Base was loaded. Therefore, accessing
ActiveRecord::Encryption.config properties before ActiveRecord::Base was
loaded would give incorrect results.

ActiveRecord::Encryption now has its own loading hook so that its
configuration is set as soon as needed.

When ActiveRecord::Base is loaded, even lazily, it in turn triggers the
loading of ActiveRecord::Encryption, thus preserving the original
behavior of having its config ready before any use of
ActiveRecord::Base.
@rafaelfranca rafaelfranca force-pushed the fix-rspec-rails-issue-2779 branch from 4936e2f to 6f7f624 Compare August 27, 2024 16:48
@rafaelfranca rafaelfranca merged commit f24c42d into rails:main Aug 27, 2024
3 checks passed
rafaelfranca added a commit that referenced this pull request Aug 27, 2024
Ensure ActiveRecord::Encryption.config is always ready before access
rafaelfranca added a commit that referenced this pull request Aug 27, 2024
Ensure ActiveRecord::Encryption.config is always ready before access
@maximerety maximerety deleted the fix-rspec-rails-issue-2779 branch August 29, 2024 08:31
@pedantic-git
Copy link
Contributor

Hi @maximerety @rafaelfranca - sorry to dig out this merged PR. I think it caused a regression in my app and I thought you might like to know in case there's a potential fix.

Following the instructions in https://guides.rubyonrails.org/active_record_encryption.html#basic-usage I had set the credentials from the environment in an initializer:

# config/initializers/active_record_encryption.rb
Rails.application.config.active_record.encryption.primary_key = ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY']
Rails.application.config.active_record.encryption.deterministic_key = ENV['ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY']
Rails.application.config.active_record.encryption.key_derivation_salt = ENV['ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT']

My tests started failing on upgrade to 7.1.4.x because my credentials in ActiveRecord::Encryption.config were all set to nil even though the values were correct in Rails.application.config.active_record.encryption.

Moving the config out of the initializer and into application.rb fixed the issue for me, which suggests it's something to do with this code now running between application.rb and the initializers when it previously didn't run till after the initializers.

Would it be worth adding another check later to copy them across again if an initializer changes config.active_record.encryption?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for encrypted attributes in fixtures cannot be enabled when using Rails 7.2.0
4 participants