Skip to content

Commit

Permalink
Fix accidental extension with Rodauth::ClassMethods
Browse files Browse the repository at this point in the history
Due to how constant lookup works, defining `ClassMethods` inside the feature
block ended up extending `Rodauth::ClassMethods`, instead of creating a new
`Rodauth::OmniauthBase::ClassMethods` module. This caused issues such as
freezing the Roda app not freezing Rodauth configurations.

We fix this by making the module declaration explicit, and we also need to
explicitly reference it in the `included` hook.

This issue wasn't caught because warnings were disabled in tests. We enable them
and ignore any warnings in gems.

Fixes #31
  • Loading branch information
janko committed Jan 1, 2025
1 parent e15b67d commit 4778101
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 2 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ end

gem "rake", "~> 13.0"
gem "matrix" # rack dependency
gem "warning"
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ require "rake/testtask"
Rake::TestTask.new do |t|
t.libs << "test"
t.test_files = FileList["test/**/*_test.rb"]
t.warning = false
t.warning = true
end

task default: :test
4 changes: 3 additions & 1 deletion lib/rodauth/features/omniauth_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,12 @@ def handle_omniauth_response(res)
end

def self.included(auth)
auth.extend ClassMethods
auth.extend OmniauthBase::ClassMethods
auth.instance_variable_set(:@omniauth_providers, [])
end
end

module OmniauthBase
module ClassMethods
def inherited(subclass)
super
Expand Down
12 changes: 12 additions & 0 deletions test/omniauth_base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -497,4 +497,16 @@
omniauth_login "/auth/developer"
assert_equal "There was an error logging in with the external provider", page.find("#error_flash").text
end

it "supports freezing" do
rodauth do
enable :omniauth_base
omniauth_provider :developer
end
roda do |r|
end

app.freeze
assert app.rodauth.frozen?
end
end
3 changes: 3 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
ENV["RACK_ENV"] = "test"

require "warning"
Gem.path.each { |path| Warning.ignore(//, path) } # ignore warnings in dependencies

require "bundler/setup"

require "minitest/autorun"
Expand Down

0 comments on commit 4778101

Please sign in to comment.