Restore some config.secret_key_base functionality

The [deprecated secrets removal][1] ended up removing a bit of
non-deprecated functionality related to config.secret_key_base:

- the original implementation prioritized the value of
  config.secret_key_base over other sources in all environments
- if unset, the value of config.secret_key_base would be updated to
  whichever fallback value was found

The new implementation only sets config.secret_key_base to a fallback
value when Rails.env.local?, and never considers it at all in
production.

This commit aims to restore this missing functionality as well as
simplify the implementation:

- Rails.application.secret_key_base now always delegates to
  config.secret_key_base (like the pre-secret-removal implementation)
- secret_key_base validation was moved from the reader to the writer
- config.secret_key_base now handles setting itself to a fallback value
  when unset
- In addition, generate_local_secret was simplified because it
  previously did 3 things: file manipulation, setting
  config.secret_key_base, and returning a value. Now it only creates the
  file if necessary and returns the value stored in it

The new implementation has an additional benefit, which is that manually
set config.secret_key_base values are now validated, whereas previously
only fallback values were validated.

[1]: 0c76f17

Co-authored-by: Petrik <[email protected]>

Author: skipkayhil

railties/lib/rails/application.rb

@@ -462,13 +462,7 @@ def config # :nodoc:
     # then +credentials.secret_key_base+. For most applications, the correct place to store it is in the
     # encrypted credentials file.
     def secret_key_base
-      if Rails.env.local? || ENV["SECRET_KEY_BASE_DUMMY"]
-        config.secret_key_base ||= generate_local_secret
-      else
-        validate_secret_key_base(
-          ENV["SECRET_KEY_BASE"] || credentials.secret_key_base
-        )
-      end
+      config.secret_key_base
     end
 
     # Returns an ActiveSupport::EncryptedConfiguration instance for the
@@ -621,39 +615,12 @@ def default_middleware_stack # :nodoc:
       default_stack.build_stack
     end
 
-    def validate_secret_key_base(secret_key_base)
-      if secret_key_base.is_a?(String) && secret_key_base.present?
-        secret_key_base
-      elsif secret_key_base
-        raise ArgumentError, "`secret_key_base` for #{Rails.env} environment must be a type of String`"
-      else
-        raise ArgumentError, "Missing `secret_key_base` for '#{Rails.env}' environment, set this string with `bin/rails credentials:edit`"
-      end
-    end
-
     def ensure_generator_templates_added
       configured_paths = config.generators.templates
       configured_paths.unshift(*(paths["lib/templates"].existent - configured_paths))
     end
 
     private
-      def generate_local_secret
-        if config.secret_key_base.nil?
-          key_file = Rails.root.join("tmp/local_secret.txt")
-
-          if File.exist?(key_file)
-            config.secret_key_base = File.binread(key_file)
-          else
-            random_key = SecureRandom.hex(64)
-            FileUtils.mkdir_p(key_file.dirname)
-            File.binwrite(key_file, random_key)
-            config.secret_key_base = File.binread(key_file)
-          end
-        end
-
-        config.secret_key_base
-      end
-
       def build_request(env)
         req = super
         env["ORIGINAL_FULLPATH"] = req.fullpath

railties/lib/rails/application/configuration.rb

@@ -15,7 +15,7 @@ class Configuration < ::Rails::Engine::Configuration
                     :cache_classes, :cache_store, :consider_all_requests_local, :console,
                     :eager_load, :exceptions_app, :file_watcher, :filter_parameters, :precompile_filter_parameters,
                     :force_ssl, :helpers_paths, :hosts, :host_authorization, :logger, :log_formatter,
-                    :log_tags, :railties_order, :relative_url_root, :secret_key_base,
+                    :log_tags, :railties_order, :relative_url_root,
                     :ssl_options, :public_file_server,
                     :session_options, :time_zone, :reload_classes_only_on_change,
                     :beginning_of_week, :filter_redirect, :x,
@@ -500,6 +500,26 @@ def colorize_logging=(val)
         generators.colorize_logging = val
       end
 
+      def secret_key_base
+        @secret_key_base || begin
+          self.secret_key_base = if Rails.env.local? || ENV["SECRET_KEY_BASE_DUMMY"]
+            generate_local_secret
+          else
+            ENV["SECRET_KEY_BASE"] || Rails.application.credentials.secret_key_base
+          end
+        end
+      end
+
+      def secret_key_base=(new_secret_key_base)
+        if new_secret_key_base.is_a?(String) && new_secret_key_base.present?
+          @secret_key_base = new_secret_key_base
+        elsif new_secret_key_base
+          raise ArgumentError, "`secret_key_base` for #{Rails.env} environment must be a type of String`"
+        else
+          raise ArgumentError, "Missing `secret_key_base` for '#{Rails.env}' environment, set this string with `bin/rails credentials:edit`"
+        end
+      end
+
       # Specifies what class to use to store the session. Possible values
       # are +:cache_store+, +:cookie_store+, +:mem_cache_store+, a custom
       # store, or +:disabled+. +:disabled+ tells \Rails not to deal with
@@ -605,6 +625,18 @@ def credentials_defaults
 
           { content_path: content_path, key_path: key_path }
         end
+
+        def generate_local_secret
+          key_file = root.join("tmp/local_secret.txt")
+
+          unless File.exist?(key_file)
+            random_key = SecureRandom.hex(64)
+            FileUtils.mkdir_p(key_file.dirname)
+            File.binwrite(key_file, random_key)
+          end
+
+          File.binread(key_file)
+        end
     end
   end
 end

railties/test/application/configuration_test.rb

@@ -919,7 +919,7 @@ def index
     end
 
 
-    test "secret_key_base is copied from config.secret_key_base when set" do
+    test "app.secret_key_base uses config.secret_key_base in development" do
       app_file "config/initializers/secret_token.rb", <<-RUBY
         Rails.application.config.secret_key_base = "3b7cd727ee24e8444053437c36cc66c3"
       RUBY
@@ -928,12 +928,13 @@ def index
       assert_equal "3b7cd727ee24e8444053437c36cc66c3", app.secret_key_base
     end
 
-    test "config.secret_key_base over-writes a blank app.secret_key_base" do
+    test "app.secret_key_base uses config.secret_key_base in production" do
+      remove_file "config/credentials.yml.enc"
       app_file "config/initializers/secret_token.rb", <<-RUBY
         Rails.application.config.secret_key_base = "iaminallyoursecretkeybase"
       RUBY
 
-      app "development"
+      app "production"
 
       assert_equal "iaminallyoursecretkeybase", app.secret_key_base
     end