Merge pull request rails#45672 from jonathanhefner/generate-master-key-despite-require_master_key

Generate `master.key` even when `require_master_key`

Author: jonathanhefner

activesupport/lib/active_support/encrypted_file.rb

@@ -53,6 +53,12 @@ def key
       read_env_key || read_key_file || handle_missing_key
     end
 
+    # Returns truthy if #key is truthy. Returns falsy otherwise. Unlike #key,
+    # does not raise MissingKeyError when +raise_if_missing_key+ is true.
+    def key?
+      read_env_key || read_key_file
+    end
+
     # Reads the file and returns the decrypted content.
     #
     # Raises:

activesupport/test/encrypted_file_test.rb

@@ -5,6 +5,9 @@
 
 class EncryptedFileTest < ActiveSupport::TestCase
   setup do
+    @original_env_content_key = ENV["CONTENT_KEY"]
+    ENV["CONTENT_KEY"] = nil
+
     @content = "One little fox jumped over the hedge"
 
     @tmpdir = Dir.mktmpdir("encrypted-file-test-")
@@ -21,19 +24,17 @@ class EncryptedFileTest < ActiveSupport::TestCase
     FileUtils.rm_rf @content_path
     FileUtils.rm_rf @key_path
     FileUtils.rm_rf @tmpdir
+
+    ENV["CONTENT_KEY"] = @original_env_content_key
   end
 
   test "reading content by env key" do
     FileUtils.rm_rf @key_path
 
-    begin
-      ENV["CONTENT_KEY"] = @key
-      @encrypted_file.write @content
+    ENV["CONTENT_KEY"] = @key
+    @encrypted_file.write @content
 
-      assert_equal @content, @encrypted_file.read
-    ensure
-      ENV["CONTENT_KEY"] = nil
-    end
+    assert_equal @content, @encrypted_file.read
   end
 
   test "reading content by key file" do
@@ -59,17 +60,13 @@ class EncryptedFileTest < ActiveSupport::TestCase
   test "raise MissingKeyError when env key is blank" do
     FileUtils.rm_rf @key_path
 
-    begin
-      ENV["CONTENT_KEY"] = ""
-      raised = assert_raise ActiveSupport::EncryptedFile::MissingKeyError do
-        @encrypted_file.write @content
-        @encrypted_file.read
-      end
-
-      assert_match(/Missing encryption key to decrypt file/, raised.message)
-    ensure
-      ENV["CONTENT_KEY"] = nil
+    ENV["CONTENT_KEY"] = ""
+    raised = assert_raise ActiveSupport::EncryptedFile::MissingKeyError do
+      @encrypted_file.write @content
+      @encrypted_file.read
     end
+
+    assert_match(/Missing encryption key to decrypt file/, raised.message)
   end
 
   test "key can be added after MissingKeyError raised" do
@@ -86,6 +83,25 @@ class EncryptedFileTest < ActiveSupport::TestCase
     end
   end
 
+  test "key? is true when key file exists" do
+    assert @encrypted_file.key?
+  end
+
+  test "key? is true when env key is present" do
+    FileUtils.rm_rf @key_path
+    ENV["CONTENT_KEY"] = @key
+
+    assert @encrypted_file.key?
+  end
+
+  test "key? is false and does not raise when the key is missing" do
+    FileUtils.rm_rf @key_path
+
+    assert_nothing_raised do
+      assert_not @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

@@ -32,7 +32,7 @@ def edit
 
         ensure_editor_available(command: "bin/rails credentials:edit") || (return)
 
-        ensure_encryption_key_has_been_added if credentials.key.nil?
+        ensure_encryption_key_has_been_added
         ensure_credentials_have_been_added
         ensure_diffing_driver_is_configured
 
@@ -78,6 +78,8 @@ def credentials
         end
 
         def ensure_encryption_key_has_been_added
+          return if credentials.key?
+
           require "rails/generators/rails/encryption_key_file/encryption_key_file_generator"
 
           encryption_key_file_generator = Rails::Generators::EncryptionKeyFileGenerator.new
@@ -102,7 +104,7 @@ def change_credentials_in_system_editor
         end
 
         def missing_credentials_message
-          if credentials.key.nil?
+          if !credentials.key?
             "Missing '#{key_path}' to decrypt credentials. See `bin/rails credentials:help`"
           else
             "File '#{content_path}' does not exist. Use `bin/rails credentials:edit` to change that."

railties/lib/rails/commands/encrypted/encrypted_command.rb

@@ -24,7 +24,7 @@ def edit(*)
         require_application!
 
         ensure_editor_available(command: "bin/rails encrypted:edit") || (return)
-        ensure_encryption_key_has_been_added if encrypted_configuration.key.nil?
+        ensure_encryption_key_has_been_added
         ensure_encrypted_configuration_has_been_added
 
         catch_editing_exceptions do
@@ -56,6 +56,7 @@ def encrypted_configuration
         end
 
         def ensure_encryption_key_has_been_added
+          return if encrypted_configuration.key?
           encryption_key_file_generator.add_key_file(key_path)
           encryption_key_file_generator.ignore_key_file(key_path)
         end
@@ -86,7 +87,7 @@ def encrypted_file_generator
         end
 
         def missing_encrypted_configuration_message
-          if encrypted_configuration.key.nil?
+          if !encrypted_configuration.key?
             "Missing '#{key_path}' to decrypt data. See `bin/rails encrypted:help`"
           else
             "File '#{content_path}' does not exist. Use `bin/rails encrypted:edit #{content_path}` to change that."

railties/test/commands/credentials_test.rb

@@ -66,6 +66,15 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
     assert_equal 1, read_file(".gitignore").scan("config/master.key").length
   end
 
+  test "edit command can add master key when require_master_key is true" do
+    remove_file "config/credentials.yml.enc"
+    remove_file "config/master.key"
+    add_to_config "config.require_master_key = true"
+
+    assert_nothing_raised { run_edit_command }
+    assert_file "config/master.key"
+  end
+
   test "edit command does not add master key when `RAILS_MASTER_KEY` env specified" do
     master_key = read_file("config/master.key")
     remove_file "config/master.key"

railties/test/commands/encrypted_test.rb

@@ -51,6 +51,14 @@ class Rails::Command::EncryptedCommandTest < ActiveSupport::TestCase
     assert_equal 1, read_file(".gitignore").scan("config/master.key").length
   end
 
+  test "edit command can add master key when require_master_key is true" do
+    remove_file "config/master.key"
+    add_to_config "config.require_master_key = true"
+
+    assert_nothing_raised { run_edit_command }
+    assert_file "config/master.key"
+  end
+
   test "edit command does not add master key when `RAILS_MASTER_KEY` env specified" do
     master_key = read_file("config/master.key")
     remove_file "config/master.key"