Merge pull request #45657 from jonathanhefner/encrypted_file-post-hoc…

…-key-file

Make `EncryptedFile` more memoization-friendly

activesupport/lib/active_support/encrypted_file.rb

@@ -114,8 +114,7 @@ def read_env_key
       end
 
       def read_key_file
-        return @key_file_contents if defined?(@key_file_contents)
-        @key_file_contents = (key_path.binread.strip if key_path.exist?)
+        @key_file_contents ||= (key_path.binread.strip if key_path.exist?)
       end
 
       def handle_missing_key

activesupport/test/encrypted_file_test.rb

@@ -10,8 +10,9 @@ class EncryptedFileTest < ActiveSupport::TestCase
     @tmpdir = Dir.mktmpdir("encrypted-file-test-")
     @content_path = File.join(@tmpdir, "content.txt.enc")
 
+    @key = ActiveSupport::EncryptedFile.generate_key
     @key_path = File.join(@tmpdir, "content.txt.key")
-    File.write(@key_path, ActiveSupport::EncryptedFile.generate_key)
+    File.write(@key_path, @key)
 
     @encrypted_file = encrypted_file(@content_path)
   end
@@ -26,7 +27,7 @@ class EncryptedFileTest < ActiveSupport::TestCase
     FileUtils.rm_rf @key_path
 
     begin
-      ENV["CONTENT_KEY"] = ActiveSupport::EncryptedFile.generate_key
+      ENV["CONTENT_KEY"] = @key
       @encrypted_file.write @content
 
       assert_equal @content, @encrypted_file.read
@@ -71,6 +72,20 @@ class EncryptedFileTest < ActiveSupport::TestCase
     end
   end
 
+  test "key can be added after MissingKeyError raised" do
+    FileUtils.rm_rf @key_path
+
+    assert_raise ActiveSupport::EncryptedFile::MissingKeyError do
+      @encrypted_file.key
+    end
+
+    File.write(@key_path, @key)
+
+    assert_nothing_raised do
+      assert_equal @key, @encrypted_file.key
+    end
+  end
+
   test "raise InvalidKeyLengthError when key is too short" do
     File.write(@key_path, ActiveSupport::EncryptedFile.generate_key[0..-2])
 

railties/lib/rails/commands/credentials/credentials_command.rb

@@ -74,7 +74,7 @@ def diff(content_path = nil)
 
       private
         def credentials
-          Rails.application.encrypted(content_path, key_path: key_path)
+          @credentials ||= Rails.application.encrypted(content_path, key_path: key_path)
         end
 
         def ensure_encryption_key_has_been_added

railties/test/commands/credentials_test.rb

@@ -43,15 +43,38 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
     assert_no_match DEFAULT_CREDENTIALS_PATTERN, output
   end
 
+  test "edit command adds master key" do
+    remove_file "config/credentials.yml.enc"
+    remove_file "config/master.key"
+    app_file ".gitignore", ""
+    run_edit_command
+
+    assert_file "config/master.key"
+    assert_match "config/master.key", read_file(".gitignore")
+  end
+
+  test "edit command does not overwrite master key file if it already exists" do
+    master_key = read_file("config/master.key")
+    run_edit_command
+
+    assert_equal master_key, read_file("config/master.key")
+  end
+
+  test "edit command does not add duplicate master key entries to gitignore" do
+    2.times { run_edit_command }
+
+    assert_equal 1, read_file(".gitignore").scan("config/master.key").length
+  end
+
   test "edit command does not add master key when `RAILS_MASTER_KEY` env specified" do
-    Dir.chdir(app_path) do
-      key = IO.binread("config/master.key").strip
-      FileUtils.rm("config/master.key")
+    master_key = read_file("config/master.key")
+    remove_file "config/master.key"
+    app_file ".gitignore", ""
 
-      switch_env("RAILS_MASTER_KEY", key) do
-        assert_match DEFAULT_CREDENTIALS_PATTERN, run_edit_command
-        assert_not File.exist?("config/master.key")
-      end
+    switch_env("RAILS_MASTER_KEY", master_key) do
+      assert_match DEFAULT_CREDENTIALS_PATTERN, run_edit_command
+      assert_no_file "config/master.key"
+      assert_no_match "config/master.key", read_file(".gitignore")
     end
   end
 
@@ -258,6 +281,10 @@ def write_credentials(content, **options)
       end
     end
 
+    def read_file(relative)
+      File.read(app_path(relative))
+    end
+
     def assert_file(relative)
       assert File.exist?(app_path(relative)), "Expected file #{relative.inspect} to exist, but it does not"
     end