Merge pull request #45663 from jonathanhefner/encrypted_command-refac…

…tor-like-credentials_command

Refactor `EncryptedCommand` a la `CredentialsCommand`

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

@@ -20,42 +20,52 @@ def help
         end
       end
 
-      def edit(file_path)
+      def edit(*)
         require_application!
-        encrypted = Rails.application.encrypted(file_path, key_path: options[:key])
 
         ensure_editor_available(command: "bin/rails encrypted:edit") || (return)
-        ensure_encryption_key_has_been_added(options[:key]) if encrypted.key.nil?
-        ensure_encrypted_file_has_been_added(file_path, options[:key])
+        ensure_encryption_key_has_been_added if encrypted_configuration.key.nil?
+        ensure_encrypted_configuration_has_been_added
 
         catch_editing_exceptions do
-          change_encrypted_file_in_system_editor(file_path, options[:key])
+          change_encrypted_configuration_in_system_editor
         end
 
         say "File encrypted and saved."
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
-        say "Couldn't decrypt #{file_path}. Perhaps you passed the wrong key?"
+        say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?"
       end
 
-      def show(file_path)
+      def show(*)
         require_application!
-        encrypted = Rails.application.encrypted(file_path, key_path: options[:key])
 
-        say encrypted.read.presence || missing_encrypted_message(key: encrypted.key, key_path: options[:key], file_path: file_path)
+        say encrypted_configuration.read.presence || missing_encrypted_configuration_message
       end
 
       private
-        def ensure_encryption_key_has_been_added(key_path)
+        def content_path
+          @content_path ||= args[0]
+        end
+
+        def key_path
+          options[:key]
+        end
+
+        def encrypted_configuration
+          @encrypted_configuration ||= Rails.application.encrypted(content_path, key_path: key_path)
+        end
+
+        def ensure_encryption_key_has_been_added
           encryption_key_file_generator.add_key_file(key_path)
           encryption_key_file_generator.ignore_key_file(key_path)
         end
 
-        def ensure_encrypted_file_has_been_added(file_path, key_path)
-          encrypted_file_generator.add_encrypted_file_silently(file_path, key_path)
+        def ensure_encrypted_configuration_has_been_added
+          encrypted_file_generator.add_encrypted_file_silently(content_path, key_path)
         end
 
-        def change_encrypted_file_in_system_editor(file_path, key_path)
-          Rails.application.encrypted(file_path, key_path: key_path).change do |tmp_path|
+        def change_encrypted_configuration_in_system_editor
+          encrypted_configuration.change do |tmp_path|
             system("#{ENV["EDITOR"]} #{tmp_path}")
           end
         end
@@ -75,11 +85,11 @@ def encrypted_file_generator
           Rails::Generators::EncryptedFileGenerator.new
         end
 
-        def missing_encrypted_message(key:, key_path:, file_path:)
-          if key.nil?
+        def missing_encrypted_configuration_message
+          if encrypted_configuration.key.nil?
             "Missing '#{key_path}' to decrypt data. See `bin/rails encrypted:help`"
           else
-            "File '#{file_path}' does not exist. Use `bin/rails encrypted:edit #{file_path}` to change that."
+            "File '#{content_path}' does not exist. Use `bin/rails encrypted:edit #{content_path}` to change that."
           end
         end
     end

railties/test/commands/encrypted_test.rb

@@ -11,8 +11,12 @@ class Rails::Command::EncryptedCommandTest < ActiveSupport::TestCase
   setup :build_app
   teardown :teardown_app
 
+  setup do
+    @encrypted_file = "config/tokens.yml.enc"
+  end
+
   test "edit without editor gives hint" do
-    run_edit_command("config/tokens.yml.enc", editor: "").tap do |output|
+    run_edit_command(editor: "").tap do |output|
       assert_match "No $EDITOR to open file in", output
       assert_match "rails encrypted:edit", output
     end
@@ -21,90 +25,102 @@ class Rails::Command::EncryptedCommandTest < ActiveSupport::TestCase
   test "edit encrypted file" do
     # Run twice to ensure file can be reread after first edit pass.
     2.times do
-      assert_match(/access_key_id: 123/, run_edit_command("config/tokens.yml.enc"))
+      assert_match(/access_key_id: 123/, run_edit_command)
     end
   end
 
-  test "edit command does not add master key to gitignore when already exist" do
-    run_edit_command("config/tokens.yml.enc")
+  test "edit command adds master key" do
+    remove_file "config/master.key"
+    app_file ".gitignore", ""
+    run_edit_command
 
-    Dir.chdir(app_path) do
-      assert_match "/config/master.key", File.read(".gitignore")
-    end
+    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
-        run_edit_command("config/tokens.yml.enc")
-        assert_not File.exist?("config/master.key")
-      end
+    switch_env("RAILS_MASTER_KEY", master_key) do
+      run_edit_command
+      assert_no_file "config/master.key"
+      assert_no_match "config/master.key", read_file(".gitignore")
     end
   end
 
   test "edit encrypts file with custom key" do
-    run_edit_command("config/tokens.yml.enc", key: "config/tokens.key")
+    run_edit_command(key: "config/tokens.key")
 
     Dir.chdir(app_path) do
-      assert File.exist?("config/tokens.yml.enc")
       assert File.exist?("config/tokens.key")
-
       assert_match "/config/tokens.key", File.read(".gitignore")
     end
 
-    assert_match(/access_key_id: 123/, run_edit_command("config/tokens.yml.enc", key: "config/tokens.key"))
+    assert_match(/access_key_id: 123/, run_edit_command(key: "config/tokens.key"))
   end
 
   test "show encrypted file with custom key" do
-    run_edit_command("config/tokens.yml.enc", key: "config/tokens.key")
+    run_edit_command(key: "config/tokens.key")
 
-    assert_match(/access_key_id: 123/, run_show_command("config/tokens.yml.enc", key: "config/tokens.key"))
+    assert_match(/access_key_id: 123/, run_show_command(key: "config/tokens.key"))
   end
 
   test "show command raise error when require_master_key is specified and key does not exist" do
     add_to_config "config.require_master_key = true"
 
     assert_match(/Missing encryption key to decrypt file with/,
-      run_show_command("config/tokens.yml.enc", key: "unexist.key", allow_failure: true))
+      run_show_command(key: "unexist.key", allow_failure: true))
   end
 
   test "show command does not raise error when require_master_key is false and master key does not exist" do
     remove_file "config/master.key"
     add_to_config "config.require_master_key = false"
 
-    assert_match(/Missing 'config\/master\.key' to decrypt data/, run_show_command("config/tokens.yml.enc"))
+    assert_match(/Missing 'config\/master\.key' to decrypt data/, run_show_command)
   end
 
   test "won't corrupt encrypted file when passed wrong key" do
-    run_edit_command("config/tokens.yml.enc", key: "config/tokens.key")
+    run_edit_command(key: "config/tokens.key")
 
     assert_match "passed the wrong key",
-      run_edit_command("config/tokens.yml.enc", allow_failure: true)
+      run_edit_command(allow_failure: true)
 
-    assert_match(/access_key_id: 123/, run_show_command("config/tokens.yml.enc", key: "config/tokens.key"))
+    assert_match(/access_key_id: 123/, run_show_command(key: "config/tokens.key"))
   end
 
   test "show command does not raise when an initializer tries to access non-existent credentials" do
     app_file "config/initializers/raise_when_loaded.rb", <<-RUBY
       Rails.application.credentials.missing_key!
     RUBY
 
-    run_edit_command("config/tokens.yml.enc", key: "config/tokens.key")
+    run_edit_command(key: "config/tokens.key")
 
-    assert_match(/access_key_id: 123/, run_show_command("config/tokens.yml.enc", key: "config/tokens.key"))
+    assert_match(/access_key_id: 123/, run_show_command(key: "config/tokens.key"))
   end
 
   private
-    def run_edit_command(file, key: nil, editor: "cat", **options)
+    def run_edit_command(file = @encrypted_file, key: nil, editor: "cat", **options)
       switch_env("EDITOR", editor) do
         rails "encrypted:edit", prepare_args(file, key), **options
       end
     end
 
-    def run_show_command(file, key: nil, **options)
+    def run_show_command(file = @encrypted_file, key: nil, **options)
       rails "encrypted:show", prepare_args(file, key), **options
     end
 
@@ -113,4 +129,16 @@ def prepare_args(file, key)
       args.push("--key", key) if key
       args
     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
+
+    def assert_no_file(relative)
+      assert_not File.exist?(app_path(relative)), "Expected file #{relative.inspect} to not exist, but it does"
+    end
 end