Merge pull request rails#46284 from jonathanhefner/active_record-ciphertext_for-not-yet-encrypted

Fix `ciphertext_for` for yet-to-be-encrypted values

Author: jonathanhefner

activerecord/CHANGELOG.md

@@ -1,3 +1,37 @@
+*   Fix `ciphertext_for` for yet-to-be-encrypted values.
+
+    Previously, `ciphertext_for` returned the cleartext of values that had not
+    yet been encrypted, such as with an unpersisted record:
+
+      ```ruby
+      Post.encrypts :body
+
+      post = Post.create!(body: "Hello")
+      post.ciphertext_for(:body)
+      # => "{\"p\":\"abc..."
+
+      post.body = "World"
+      post.ciphertext_for(:body)
+      # => "World"
+      ```
+
+    Now, `ciphertext_for` will always return the ciphertext of encrypted
+    attributes:
+
+      ```ruby
+      Post.encrypts :body
+
+      post = Post.create!(body: "Hello")
+      post.ciphertext_for(:body)
+      # => "{\"p\":\"abc..."
+
+      post.body = "World"
+      post.ciphertext_for(:body)
+      # => "{\"p\":\"xyz..."
+      ```
+
+    *Jonathan Hefner*
+
 *   Fix a bug where using groups and counts with long table names would return incorrect results.
 
     *Shota Toguchi*, *Yusaku Ono*

activerecord/lib/active_record/encryption/encryptable_record.rb

@@ -140,12 +140,16 @@ def validate_column_size(attribute_name)
 
       # Returns whether a given attribute is encrypted or not.
       def encrypted_attribute?(attribute_name)
-        ActiveRecord::Encryption.encryptor.encrypted? ciphertext_for(attribute_name)
+        ActiveRecord::Encryption.encryptor.encrypted? read_attribute_before_type_cast(attribute_name)
       end
 
       # Returns the ciphertext for +attribute_name+.
       def ciphertext_for(attribute_name)
-        read_attribute_before_type_cast(attribute_name)
+        if encrypted_attribute?(attribute_name)
+          read_attribute_before_type_cast(attribute_name)
+        else
+          read_attribute_for_database(attribute_name)
+        end
       end
 
       # Encrypts all the encryptable attributes and saves the changes.

activerecord/test/cases/encryption/contexts_test.rb

@@ -11,24 +11,26 @@ class ActiveRecord::Encryption::ContextsTest < ActiveRecord::EncryptionTestCase
     ActiveRecord::Encryption.config.support_unencrypted_data = true
 
     @post = EncryptedPost.create!(title: "Some encrypted post title", body: "Some body")
-    @clean_title = @post.title
+    @title_cleartext = @post.title
+    @title_ciphertext = @post.ciphertext_for(:title)
   end
 
   test ".with_encryption_context lets you override properties" do
     ActiveRecord::Encryption.with_encryption_context(encryptor: ActiveRecord::Encryption::NullEncryptor.new) do
-      assert_protected_encrypted_attribute(@post, :title, @clean_title)
+      assert_equal @title_ciphertext, @post.reload.title
+
       @post.update!(title: "Some new title")
     end
 
-    assert_equal "Some new title", @post.title
+    assert_equal "Some new title", @post.title_before_type_cast
   end
 
   test ".with_encryption_context will restore previous context properties when there is an error" do
     ActiveRecord::Encryption.with_encryption_context(encryptor: ActiveRecord::Encryption::NullEncryptor.new) do
       raise "Some error"
     end
   rescue
-    assert_encrypted_attribute @post.reload, :title, @clean_title
+    assert_encrypted_attribute @post.reload, :title, @title_cleartext
   end
 
   test ".with_encryption_context can be nested multiple times" do
@@ -51,17 +53,17 @@ class ActiveRecord::Encryption::ContextsTest < ActiveRecord::EncryptionTestCase
 
   test ".without_encryption won't decrypt or encrypt data automatically" do
     ActiveRecord::Encryption.without_encryption do
-      assert_protected_encrypted_attribute(@post, :title, @clean_title)
+      assert_equal @title_ciphertext, @post.reload.title
 
       @post.update!(title: "Some new title")
     end
 
-    assert_equal "Some new title", @post.title
+    assert_not_encrypted_attribute @post, :title, "Some new title"
   end
 
   test ".protecting_encrypted_data don't decrypt attributes automatically" do
     ActiveRecord::Encryption.protecting_encrypted_data do
-      assert_protected_encrypted_attribute(@post, :title, @clean_title)
+      assert_equal @title_ciphertext, @post.reload.title
     end
   end
 
@@ -92,10 +94,4 @@ class ActiveRecord::Encryption::ContextsTest < ActiveRecord::EncryptionTestCase
       end
     end
   end
-
-  private
-    def assert_protected_encrypted_attribute(model, attribute_name, clean_value)
-      assert_equal model.reload.ciphertext_for(attribute_name), model.public_send(attribute_name)
-      assert_not_equal clean_value, model.ciphertext_for(:title)
-    end
 end

activerecord/test/cases/encryption/encryptable_record_api_test.rb

@@ -73,14 +73,40 @@ class ActiveRecord::Encryption::EncryptableRecordApiTest < ActiveRecord::Encrypt
 
   test "encrypted_attribute? returns false for encrypted attributes which content is not encrypted" do
     book = ActiveRecord::Encryption.without_encryption { EncryptedBook.create!(name: "Dune") }
-    assert_not book.encrypted_attribute?(:title)
+    assert_not book.encrypted_attribute?(:name)
   end
 
-  test "ciphertext_for returns the chiphertext for a given attributes" do
+  test "ciphertext_for returns the ciphertext for a given attribute" do
     book = EncryptedBook.create!(name: "Dune")
 
-    assert_equal book.ciphertext_for(:name), book.ciphertext_for(:name)
-    assert_not_equal book.name, book.ciphertext_for(:name)
+    assert_ciphertext_decrypts_to book, :name, book.ciphertext_for(:name)
+  end
+
+  test "ciphertext_for returns the persisted ciphertext for a non-deterministically encrypted attribute" do
+    post = EncryptedPost.create!(title: "Fear is the mind-killer", body: "Fear is the little-death...")
+
+    assert_equal post.title_before_type_cast, post.ciphertext_for(:title)
+    assert_ciphertext_decrypts_to post, :title, post.ciphertext_for(:title)
+  end
+
+  test "ciphertext_for returns the ciphertext of a new value" do
+    book = EncryptedBook.create!(name: "Dune")
+    book.name = "Arrakis"
+
+    assert_ciphertext_decrypts_to book, :name, book.ciphertext_for(:name)
+  end
+
+  test "ciphertext_for returns the ciphertext of a decrypted value" do
+    book = EncryptedBook.create!(name: "Dune")
+    book.decrypt
+
+    assert_ciphertext_decrypts_to book, :name, book.ciphertext_for(:name)
+  end
+
+  test "ciphertext_for returns the ciphertext of a value when the record is new" do
+    book = EncryptedBook.new(name: "Dune")
+
+    assert_ciphertext_decrypts_to book, :name, book.ciphertext_for(:name)
   end
 
   test "encrypt won't change the encoding of strings even when compression is used" do

activerecord/test/cases/encryption/helper.rb

@@ -9,12 +9,19 @@ class ActiveRecord::Fixture
 
 module ActiveRecord::Encryption
   module EncryptionHelpers
+    def assert_ciphertext_decrypts_to(model, attribute_name, ciphertext)
+      assert_not_equal model.public_send(attribute_name), ciphertext
+      assert_not_equal model.read_attribute(attribute_name), ciphertext
+      cleartext = model.type_for_attribute(attribute_name).deserialize(ciphertext)
+      assert_equal model.read_attribute(attribute_name), cleartext
+    end
+
     def assert_encrypted_attribute(model, attribute_name, expected_value)
-      assert_not_equal expected_value, model.ciphertext_for(attribute_name)
+      assert_ciphertext_decrypts_to model, attribute_name, model.read_attribute_before_type_cast(attribute_name)
       assert_equal expected_value, model.public_send(attribute_name)
       unless model.new_record?
         model.reload
-        assert_not_equal expected_value, model.ciphertext_for(attribute_name)
+        assert_ciphertext_decrypts_to model, attribute_name, model.read_attribute_before_type_cast(attribute_name)
         assert_equal expected_value, model.public_send(attribute_name)
       end
     end
@@ -29,7 +36,7 @@ def assert_invalid_key_cant_read_attribute(model, attribute_name)
 
     def assert_not_encrypted_attribute(model, attribute_name, expected_value)
       assert_equal expected_value, model.send(attribute_name)
-      assert_equal expected_value, model.ciphertext_for(attribute_name)
+      assert_equal expected_value, model.read_attribute_before_type_cast(attribute_name)
     end
 
     def assert_encrypted_record(model)