search

doorkeeper-gem / doorkeeper

Doorkeeper is an OAuth 2 provider for Ruby on Rails / Grape.
https://doorkeeper.gitbook.io/guides/
MIT License
5.34k stars 1.07k forks source link

Regression with Errors in 5.6.8 #1685

Closed henrikbjorn closed 9 months ago

henrikbjorn commented 10 months ago

Steps to reproduce

Do a clean install with Rails, latest Doorkeeper release and latest https://github.com/exop-group/doorkeeper-device_authorization_grant release.

commit that broke it: https://github.com/doorkeeper-gem/doorkeeper/commit/bdf3d50cff437b04d7c42c08481956a27ecf431a

Expected behavior

Should work without problems.

Actual behavior

Exception is raised since errors are now an object and not a simple Symbol. I would suspect this might have broken other gems aswell.

System configuration

You can help us to understand your problem if you will share some very useful information about your project environment (don't forget to remove any confidential data if it exists).

Doorkeeper initializer:

# config/initializers/doorkeeper.rb
Doorkeeper.configure do
end

Ruby version: 3.2

Gemfile.lock:

Gemfile.lock content ``` GIT remote: https://github.com/rails/rails.git revision: 4fb230d2140054f8d0114c2c8dcc659af10a7972 specs: actioncable (7.2.0.alpha) actionpack (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) actionmailbox (7.2.0.alpha) actionpack (= 7.2.0.alpha) activejob (= 7.2.0.alpha) activerecord (= 7.2.0.alpha) activestorage (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) mail (>= 2.8.0) actionmailer (7.2.0.alpha) actionpack (= 7.2.0.alpha) actionview (= 7.2.0.alpha) activejob (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) mail (>= 2.8.0) rails-dom-testing (~> 2.2) actionpack (7.2.0.alpha) actionview (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) nokogiri (>= 1.8.5) racc rack (>= 2.2.4) rack-session (>= 1.0.1) rack-test (>= 0.6.3) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) actiontext (7.2.0.alpha) actionpack (= 7.2.0.alpha) activerecord (= 7.2.0.alpha) activestorage (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) globalid (>= 0.6.0) nokogiri (>= 1.8.5) actionview (7.2.0.alpha) activesupport (= 7.2.0.alpha) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) activejob (7.2.0.alpha) activesupport (= 7.2.0.alpha) globalid (>= 0.3.6) activemodel (7.2.0.alpha) activesupport (= 7.2.0.alpha) activerecord (7.2.0.alpha) activemodel (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) timeout (>= 0.4.0) activestorage (7.2.0.alpha) actionpack (= 7.2.0.alpha) activejob (= 7.2.0.alpha) activerecord (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) marcel (~> 1.0) activesupport (7.2.0.alpha) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0, >= 2.0.5) rails (7.2.0.alpha) actioncable (= 7.2.0.alpha) actionmailbox (= 7.2.0.alpha) actionmailer (= 7.2.0.alpha) actionpack (= 7.2.0.alpha) actiontext (= 7.2.0.alpha) actionview (= 7.2.0.alpha) activejob (= 7.2.0.alpha) activemodel (= 7.2.0.alpha) activerecord (= 7.2.0.alpha) activestorage (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) bundler (>= 1.15.0) railties (= 7.2.0.alpha) railties (7.2.0.alpha) actionpack (= 7.2.0.alpha) activesupport (= 7.2.0.alpha) irb rackup (>= 1.0.0) rake (>= 12.2) thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) GEM remote: https://rubygems.org/ specs: activeadmin (3.2.0) arbre (~> 1.2, >= 1.2.1) formtastic (>= 3.1) formtastic_i18n (>= 0.4) inherited_resources (~> 1.7) jquery-rails (>= 4.2) kaminari (>= 1.2.1) railties (>= 6.1) ransack (>= 4.0) acts_as_list (1.1.0) activerecord (>= 4.2) addressable (2.8.6) public_suffix (>= 2.0.2, < 6.0) arbre (1.7.0) activesupport (>= 3.0.0) ruby2_keywords (>= 0.0.2) ast (2.4.2) base64 (0.2.0) bcrypt (3.1.20) bigdecimal (3.1.5) bootsnap (1.17.1) msgpack (~> 1.2) brakeman (6.1.1) racc builder (3.2.4) cancancan (3.5.0) concurrent-ruby (1.2.2) connection_pool (2.4.1) crass (1.0.6) css_parser (1.16.0) addressable dartsass-rails (0.5.0) railties (>= 6.0.0) sass-embedded (~> 1.63) date (3.3.4) debug (1.9.1) irb (~> 1.10) reline (>= 0.3.8) doorkeeper (5.6.8) railties (>= 5) doorkeeper-device_authorization_grant (1.0.3) doorkeeper (~> 5.5) drb (2.2.0) ruby2_keywords erubi (1.12.0) formtastic (5.0.0) actionpack (>= 6.0.0) formtastic_i18n (0.7.0) globalid (1.2.1) activesupport (>= 6.1) google-protobuf (3.25.2) google-protobuf (3.25.2-aarch64-linux) google-protobuf (3.25.2-arm64-darwin) google-protobuf (3.25.2-x86-linux) google-protobuf (3.25.2-x86_64-darwin) google-protobuf (3.25.2-x86_64-linux) has_scope (0.8.2) actionpack (>= 5.2) activesupport (>= 5.2) htmlbeautifier (1.4.2) htmlentities (4.3.4) i18n (1.14.1) concurrent-ruby (~> 1.0) importmap-rails (2.0.1) actionpack (>= 6.0.0) activesupport (>= 6.0.0) railties (>= 6.0.0) inherited_resources (1.14.0) actionpack (>= 6.0) has_scope (>= 0.6) railties (>= 6.0) responders (>= 2) io-console (0.7.1) irb (1.11.1) rdoc reline (>= 0.4.2) jquery-rails (4.6.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (2.7.1) kaminari (1.2.2) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.2) kaminari-activerecord (= 1.2.2) kaminari-core (= 1.2.2) kaminari-actionview (1.2.2) actionview kaminari-core (= 1.2.2) kaminari-activerecord (1.2.2) activerecord kaminari-core (= 1.2.2) kaminari-core (1.2.2) language_server-protocol (3.17.0.3) lograge (0.14.0) actionpack (>= 4) activesupport (>= 4) railties (>= 4) request_store (~> 1.0) loofah (2.22.0) crass (~> 1.0.2) nokogiri (>= 1.12.0) lookbook (2.2.0) activemodel css_parser htmlbeautifier (~> 1.3) htmlentities (~> 4.3.4) marcel (~> 1.0) railties (>= 5.0) redcarpet (~> 3.5) rouge (>= 3.26, < 5.0) view_component (>= 2.0) yard (~> 0.9.25) zeitwerk (~> 2.5) mail (2.8.1) mini_mime (>= 0.1.1) net-imap net-pop net-smtp marcel (1.0.2) method_source (1.0.0) mini_mime (1.1.5) minitest (5.21.1) msgpack (1.7.2) net-imap (0.4.9.1) date net-protocol net-pop (0.1.2) net-protocol net-protocol (0.2.2) timeout net-smtp (0.4.0.1) net-protocol nio4r (2.7.0) nokogiri (1.16.0-aarch64-linux) racc (~> 1.4) nokogiri (1.16.0-arm-linux) racc (~> 1.4) nokogiri (1.16.0-arm64-darwin) racc (~> 1.4) nokogiri (1.16.0-x86-linux) racc (~> 1.4) nokogiri (1.16.0-x86_64-darwin) racc (~> 1.4) nokogiri (1.16.0-x86_64-linux) racc (~> 1.4) pagy (6.3.0) parallel (1.24.0) parser (3.3.0.3) ast (~> 2.4.1) racc passwordless (1.2.0) bcrypt (>= 3.1.11) rails (>= 5.1.4) pg (1.5.4) postmark (1.25.0) json postmark-rails (0.22.1) actionmailer (>= 3.0.0) postmark (>= 1.21.3, < 2.0) propshaft (0.8.0) actionpack (>= 7.0.0) activesupport (>= 7.0.0) rack railties (>= 7.0.0) psych (5.1.2) stringio public_suffix (5.0.4) puma (6.4.2) nio4r (~> 2.0) racc (1.7.3) rack (3.0.8) rack-session (2.0.0) rack (>= 3.0.0) rack-test (2.1.0) rack (>= 1.3) rackup (2.1.0) rack (>= 3) webrick (~> 1.8) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) rails_heroicon (2.2.0) actionview railties rainbow (3.1.1) rake (13.1.0) ransack (4.1.1) activerecord (>= 6.1.5) activesupport (>= 6.1.5) i18n rdoc (6.6.2) psych (>= 4.0.0) redcarpet (3.6.0) regexp_parser (2.9.0) reline (0.4.2) io-console (~> 0.5) request_store (1.5.1) rack (>= 1.4) responders (3.1.1) actionpack (>= 5.2) railties (>= 5.2) rexml (3.2.6) rouge (4.2.0) rubocop (1.59.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.2.2.4) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) rubocop-ast (>= 1.30.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) rubocop-ast (1.30.0) parser (>= 3.2.1.0) rubocop-minitest (0.34.4) rubocop (>= 1.39, < 2.0) rubocop-ast (>= 1.30.0, < 2.0) rubocop-performance (1.20.2) rubocop (>= 1.48.1, < 2.0) rubocop-ast (>= 1.30.0, < 2.0) rubocop-rails (2.23.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.33.0, < 2.0) rubocop-ast (>= 1.30.0, < 2.0) rubocop-rails-omakase (1.0.0) rubocop rubocop-minitest rubocop-performance rubocop-rails ruby-progressbar (1.13.0) ruby2_keywords (0.0.5) sass-embedded (1.69.7-aarch64-linux-gnu) google-protobuf (~> 3.25) sass-embedded (1.69.7-arm-linux-gnueabihf) google-protobuf (~> 3.25) sass-embedded (1.69.7-arm64-darwin) google-protobuf (~> 3.25) sass-embedded (1.69.7-x86-linux-gnu) google-protobuf (~> 3.25) sass-embedded (1.69.7-x86_64-darwin) google-protobuf (~> 3.25) sass-embedded (1.69.7-x86_64-linux-gnu) google-protobuf (~> 3.25) solid_queue (0.1.2) rails (>= 7.0.3.1) sqids (0.2.0) stimulus-rails (1.3.3) railties (>= 6.0.0) stringio (3.1.0) tailwindcss-rails (2.3.0) railties (>= 6.0.0) tailwindcss-rails (2.3.0-aarch64-linux) railties (>= 6.0.0) tailwindcss-rails (2.3.0-arm-linux) railties (>= 6.0.0) tailwindcss-rails (2.3.0-arm64-darwin) railties (>= 6.0.0) tailwindcss-rails (2.3.0-x86_64-darwin) railties (>= 6.0.0) tailwindcss-rails (2.3.0-x86_64-linux) railties (>= 6.0.0) thor (1.3.0) timeout (0.4.1) turbo-rails (1.5.0) actionpack (>= 6.0.0) activejob (>= 6.0.0) railties (>= 6.0.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (2.5.0) useragent (0.16.10) validate_url (1.0.15) activemodel (>= 3.0.0) public_suffix view_component (3.10.0) activesupport (>= 5.2.0, < 8.0) concurrent-ruby (~> 1.0) method_source (~> 1.0) view_component-form (0.2.6) actionview (>= 6.0.0, < 7.2) activesupport (>= 6.0.0, < 7.2) view_component (>= 2.34.0, < 4.0) zeitwerk (~> 2.5) webrick (1.8.1) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) yard (0.9.34) zeitwerk (2.6.12) PLATFORMS aarch64-linux arm-linux arm64-darwin x86-linux x86_64-darwin x86_64-linux DEPENDENCIES activeadmin acts_as_list bootsnap brakeman cancancan dartsass-rails debug doorkeeper doorkeeper-device_authorization_grant importmap-rails lograge lookbook pagy passwordless pg (~> 1.1) postmark-rails propshaft puma (>= 5.0) rails! rails_heroicon responders rubocop-rails-omakase solid_queue sqids stimulus-rails tailwindcss-rails turbo-rails validate_url view_component view_component-form RUBY VERSION ruby 3.3.0p0 BUNDLED WITH 2.4.10 ```
nbulaj commented 10 months ago

Hey @henrikbjorn . Unfortunately this is an expected behavior as we refactored gem internals. So I can suggest you to follow the new way how errors work.

Can you please give more details about an error you're facing with the latest update and your gem? Maybe we can do some backward compatible changes to support something you (and maybe anyone else) need in your gem. Also will ping @camero2734 just in case

henrikbjorn commented 10 months ago

Hey @henrikbjorn . Unfortunately this is an expected behavior as we refactored gem internals. So I can suggest you to follow the new way how errors work.

Can you please give more details about an error you're facing with the latest update and your gem? Maybe we can do some backward compatible changes to support something you (and maybe anyone else) need in your gem. Also will ping @camero2734 just in case

It make sense that such a change would requiring changes to other gems, however I think it is a regression it was done in a minor release.

I am getting a MethodNotDefined error as the symbol does not have to new to_response_name method.

henrikbjorn commented 10 months ago

A mitigation could be to convert the symbol into the matching error class dynamically. eg. "Errors::#{error.to_s.classify}".constantize

bockets commented 9 months ago

Seeing a similar issue while using doorkeeper-openid__connect v1.8.7:

undefined method `name_for_response' for :access_denied:Symbol (NoMethodError)

            name: request.error&.name_for_response,
                               ^^^^^^^^^^^^^^^^^^^
doorkeeper (5.6.8) lib/doorkeeper/oauth/error_response.rb in from_request at line 13
nbulaj commented 9 months ago

A mitigation could be to convert the symbol into the matching error class dynamically. eg. "Errors::#{error.to_s.classify}".constantize

Hey @henrikbjorn . Yeah, looks like this. We have to check if responds_to?(:name_for_response) and try to constantize if it doesn't (or constantize if Symbol otherwise call a method). Would you like to produce a PR?