Skip to content

Commit

Permalink
Create Devise::SecretKeyFinder
Browse files Browse the repository at this point in the history
When supporting Rails 5.2 credentials on
#4712, we ended up breaking
apps that were upgraded to Rails 5.2 and weren't using `credentials`
to store their `secret_key_base`. See
#4807 for more context.
To fix it, we're now checking whether the key is present before using it.
Since there weren't any automated test for this - the conditionals were
in a Rails engine initializer - I've extracted it to a new class so that
we are able to test it easily.

Fixes #4807
  • Loading branch information
tegon committed Mar 17, 2018
1 parent 64aad8b commit 962cea2
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 7 deletions.
1 change: 1 addition & 0 deletions lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module Devise
autoload :TestHelpers, 'devise/test_helpers'
autoload :TimeInflector, 'devise/time_inflector'
autoload :TokenGenerator, 'devise/token_generator'
autoload :SecretKeyFinder, 'devise/secret_key_finder'

module Controllers
autoload :Helpers, 'devise/controllers/helpers'
Expand Down
8 changes: 1 addition & 7 deletions lib/devise/rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ class Engine < ::Rails::Engine
end

initializer "devise.secret_key" do |app|
if app.respond_to?(:credentials)
Devise.secret_key ||= app.credentials.secret_key_base
elsif app.respond_to?(:secrets)
Devise.secret_key ||= app.secrets.secret_key_base
elsif app.config.respond_to?(:secret_key_base)
Devise.secret_key ||= app.config.secret_key_base
end
Devise.secret_key ||= Devise::SecretKeyFinder.new(app).find

Devise.token_generator ||=
if secret_key = Devise.secret_key
Expand Down
25 changes: 25 additions & 0 deletions lib/devise/secret_key_finder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

module Devise
class SecretKeyFinder
def initialize(application)
@application = application
end

def find
if @application.respond_to?(:credentials) && key_exists?(@application.credentials)
@application.credentials.secret_key_base
elsif @application.respond_to?(:secrets) && key_exists?(@application.secrets)
@application.secrets.secret_key_base
elsif @application.config.respond_to?(:secret_key_base) && key_exists?(@application.config)
@application.config.secret_key_base
end
end

private

def key_exists?(object)
object.secret_key_base.present?
end
end
end
97 changes: 97 additions & 0 deletions test/secret_key_finder_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# frozen_string_literal: true

require 'test_helper'

class Rails52Credentials
def credentials
OpenStruct.new(secret_key_base: 'credentials')
end
end

class Rails52Secrets
def credentials
OpenStruct.new(secret_key_base: nil)
end

def secrets
OpenStruct.new(secret_key_base: 'secrets')
end
end

class Rails52Config
def credentials
OpenStruct.new(secret_key_base: nil)
end

def secrets
OpenStruct.new(secret_key_base: nil)
end

def config
OpenStruct.new(secret_key_base: 'config')
end
end

class Rails41Secrets
def secrets
OpenStruct.new(secret_key_base: 'secrets')
end

def config
OpenStruct.new(secret_key_base: nil)
end
end

class Rails41Config
def secrets
OpenStruct.new(secret_key_base: nil)
end

def config
OpenStruct.new(secret_key_base: 'config')
end
end

class Rails40Config
def config
OpenStruct.new(secret_key_base: 'config')
end
end

class SecretKeyFinderTest < ActiveSupport::TestCase
test "rails 5.2 uses credentials when they're available" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails52Credentials.new)

assert_equal 'credentials', secret_key_finder.find
end

test "rails 5.2 uses secrets when credentials are empty" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails52Secrets.new)

assert_equal 'secrets', secret_key_finder.find
end

test "rails 5.2 uses config when secrets are empty" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails52Config.new)

assert_equal 'config', secret_key_finder.find
end

test "rails 4.1 uses secrets" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails41Secrets.new)

assert_equal 'secrets', secret_key_finder.find
end

test "rails 4.1 uses config when secrets are empty" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails41Config.new)

assert_equal 'config', secret_key_finder.find
end

test "rails 4.0 uses config" do
secret_key_finder = Devise::SecretKeyFinder.new(Rails40Config.new)

assert_equal 'config', secret_key_finder.find
end
end

0 comments on commit 962cea2

Please sign in to comment.