Support for encrypted attributes in fixtures cannot be enabled when using Rails 7.2.0
mbaird opened this issue · 11 comments
What Ruby, Rails and RSpec versions are you using?
Ruby version: 3.3.4
Rails version: 7.2.0
RSpec version: 3.13
- rspec-core 3.13.0
- rspec-expectations 3.13.1
- rspec-mocks 3.13.1
- rspec-rails 6.1.3
- rspec-support 3.13.1
Observed behaviour
Support for encrypted attributes in fixtures cannot be enabled when using Rails 7.2.0.
When the active_record_fixture_set hook in the Active Record railtie runs, the value of ActiveRecord::Encryption.config.encrypt_fixtures is false, and the module is not prepended.
This is because the active_record hook has not yet run, and encryption has not been configured.
(Note, this is only an issue when eager_loading is not enabled, as eager loading the application causes Active Record to be loaded before RSpec loads the Active Record Test Fixtures.)
Expected behaviour
Setting config.active_record.encryption.encrypt_fixtures = true should allow fixtures to set encrypted attributes. Instead, a ActiveRecord::Encryption::Errors::Decryption exception is raised whenever an encrypted attribute is attempted to be read.
Can you provide an example reproduction?
I've made a simple reproduction repo here: https://github.com/mbaird/fixture-encryption-bug
The main branch includes the failing example, the output can be seen in the GitHub Action run.
The rails-7-1 branch switches to Rails 7.1, the tests pass on this branch.
I think these Rails issues may be related rails/rails#50606, rails/rails#48577.
Thanks for reporting. The first three links are broken.
Shouldn’t the build fail because it expects an encrypted value for the email?
Thanks for reporting. The first three links are broken.
Fixed now, thanks for flagging.
Shouldn’t the build fail because it expects an encrypted value for the email?
The test failure is because ActiveRecord is attempting to decrypt the un-encrypted value set by the fixture.
If config.active_record.encryption.encrypt_fixtures is set to true, the EncryptedFixtures module should be prepended, and the encrypt_fixture_data method encrypts each of the attributes before the record is created, allowing the use of plain, unencrypted values in fixture files.
the active_record hook has not yet run
Active Record to be loaded before RSpec loads the Active Record Test Fixtures
Sounds like we need to change the order. We can work without activerecord. And we seem to trigger loading fixtures here. Should we also trigger loading AR somehow, too?
Sounds like we need to change the order. We can work without activerecord. And we seem to trigger loading fixtures here. Should we also trigger loading AR somehow, too?
The hook runs a little earlier when ActiveSupport::TestFixtures is included here, it runs hooks, so that's when FixtureSupport is required in rails.rb.
The AR hooks are run here, on require of active_record/base.
How comes if defined?(ActiveRecord::TestFixtures) is true if the hook activerecord/base wasn’t run? 🤔
Shouldn’t test fixtures depend on activerecord/base to be loaded? Or can test fixtures, in theory, be used without activerecord? Like with bare activemodel?
How comes
if defined?(ActiveRecord::TestFixtures)is true if the hook activerecord/base wasn’t run? 🤔
I think this is because of autoloading. The constant is defined, but hasn't yet been loaded so this guard will pass. It'll then be loaded by the include a few lines later.
Shouldn’t test fixtures depend on activerecord/base to be loaded? Or can test fixtures, in theory, be used without activerecord? Like with bare activemodel?
I think they need ActiveRecord, and in fact there is a reference to ActiveRecord::Base inside TestFixtures, so the hook will be run then but this is too late.
Hello,
I'm the implementer of the fix from rails/rails#50606 that you mentioned above in the description of this issue.
I've tried a fix in rails/rails#52669, to support access to properly configured ActiveRecord::Encryption.config properties before the lazy loading of ActiveRecord::Base.
Could you test if this solves your problem?
Thank you.
Thanks for that @maximerety, I can confirm this fixes the issue!
Fantastic!
Thanks a lot for the fix and keeping an eye open @maximerety ! 🙌
I’ml close this once the Rails PR is merged.
@mbaird / @pirj Thanks for your feedbacks.
I've added missing tests to rails/rails#52669, which is now in good shape. We'll have to wait for a review.
Fixed in Rails.
Thank you, @maximerety and @mbaird !