rspec/rspec-rails

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

mbaird opened this issue · 11 comments

mbaird commented

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.

pirj commented

Thanks for reporting. The first three links are broken.

Shouldn’t the build fail because it expects an encrypted value for the email?

mbaird commented

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.

pirj commented

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?

mbaird commented

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.

pirj commented

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?

mbaird commented

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.

mbaird commented

Thanks for that @maximerety, I can confirm this fixes the issue!

pirj commented

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.

pirj commented

Fixed in Rails.
Thank you, @maximerety and @mbaird !