before/after successful_strategy_response not called on Token requests

[vovimayhem]

Steps to reproduce

• Configure the before_successful_strategy_response and/or after_successful_strategy_response callbacks at the Doorkeeper initializer.

• Run an "implicit" grant flow.

Expected behavior

The configured callbacks should run.

Actual behavior

The configured callbacks are ignored.

System configuration

Running inside a Docker container, from an image based on ruby:2.6.3-alpine

Doorkeeper initializer:

Doorkeeper.configure do
  orm :active_record

  resource_owner_authenticator do
    raise Oauth::AuthorizationsController::WrongAppError \
      unless Doorkeeper::Application.where(uid: params[:client_id]).any?

    current_user || begin
      store_location_for :user, request.fullpath
      redirect_to(new_user_session_url)
    end
  end

  admin_authenticator do
    redirect_to sign_in_url unless current_user.present?
  end

  custom_access_token_expires_in do |_context|
    # context.client.application.additional_settings.implicit_oauth_expiration
    Float::INFINITY
  end

  force_ssl_in_redirect_uri (
    ENV.fetch('OAUTH_REQUIRE_SSL_ON_REDIRECT_URI', 'false') == 'true'
  )

  grant_flows %w[authorization_code implicit client_credentials]

  before_successful_strategy_response do |request|
    puts "BEFORE HOOK FIRED! #{request}"
  end

  after_successful_strategy_response do |request, response|
    puts "AFTER HOOK FIRED! #{request}, #{response}"
  end

  skip_authorization do |_resource_owner, client|
    # Skip authorization for trusted apps:
    client.application.trusted?
  end

  ActiveSupport::Reloader.to_prepare do
    ActiveSupport.on_load(:active_record) do
      Doorkeeper::Application.send :include, TrustableOauthApp
      Doorkeeper::AccessToken.send :include, UserAccessToken
      Doorkeeper::ApplicationsController.layout 'application'
      Doorkeeper::AuthorizationsController.layout 'application'
      Doorkeeper::AuthorizedApplicationsController.layout 'application'
    end
  end
end

Ruby version:

ruby 2.6.3p62

Gemfile.lock:

Gemfile.lock content

GIT
  remote: https://github.com/DatabaseCleaner/database_cleaner.git
  revision: 4c2408ffdbbd990e78d6590f4ef6ba5e58aca673
  ref: 4c2408ffdbbd990e78d6590f4ef6ba5e58aca673
  specs:
    database_cleaner (1.7.0)

GIT
  remote: https://github.com/plataformatec/devise.git
  revision: 12fc5b76d89cf6e9c47289416fb24bf1a85f03da
  ref: 12fc5b76d89cf6e9c47289416fb24bf1a85f03da
  specs:
    devise (4.6.2)
      bcrypt (~> 3.0)
      orm_adapter (~> 0.1)
      railties (>= 4.1.0)
      responders
      warden (~> 1.2.3)

PATH
  remote: vendor/doorkeeper/doorkeeper
  specs:
    doorkeeper (5.1.0)
      railties (>= 5)

GEM
  remote: https://rubygems.org/
  specs:
    actioncable (6.0.0)
      actionpack (= 6.0.0)
      nio4r (~> 2.0)
      websocket-driver (>= 0.6.1)
    actionmailbox (6.0.0)
      actionpack (= 6.0.0)
      activejob (= 6.0.0)
      activerecord (= 6.0.0)
      activestorage (= 6.0.0)
      activesupport (= 6.0.0)
      mail (>= 2.7.1)
    actionmailer (6.0.0)
      actionpack (= 6.0.0)
      actionview (= 6.0.0)
      activejob (= 6.0.0)
      mail (~> 2.5, >= 2.5.4)
      rails-dom-testing (~> 2.0)
    actionpack (6.0.0)
      actionview (= 6.0.0)
      activesupport (= 6.0.0)
      rack (~> 2.0)
      rack-test (>= 0.6.3)
      rails-dom-testing (~> 2.0)
      rails-html-sanitizer (~> 1.0, >= 1.2.0)
    actiontext (6.0.0)
      actionpack (= 6.0.0)
      activerecord (= 6.0.0)
      activestorage (= 6.0.0)
      activesupport (= 6.0.0)
      nokogiri (>= 1.8.5)
    actionview (6.0.0)
      activesupport (= 6.0.0)
      builder (~> 3.1)
      erubi (~> 1.4)
      rails-dom-testing (~> 2.0)
      rails-html-sanitizer (~> 1.1, >= 1.2.0)
    activejob (6.0.0)
      activesupport (= 6.0.0)
      globalid (>= 0.3.6)
    activemodel (6.0.0)
      activesupport (= 6.0.0)
    activerecord (6.0.0)
      activemodel (= 6.0.0)
      activesupport (= 6.0.0)
    activestorage (6.0.0)
      actionpack (= 6.0.0)
      activejob (= 6.0.0)
      activerecord (= 6.0.0)
      marcel (~> 0.3.1)
    activesupport (6.0.0)
      concurrent-ruby (~> 1.0, >= 1.0.2)
      i18n (>= 0.7, < 2)
      minitest (~> 5.1)
      tzinfo (~> 1.1)
      zeitwerk (~> 2.1, >= 2.1.8)
    addressable (2.6.0)
      public_suffix (>= 2.0.2, < 4.0)
    aws-eventstream (1.0.3)
    aws-partitions (1.205.0)
    aws-sdk-core (3.64.0)
      aws-eventstream (~> 1.0, >= 1.0.2)
      aws-partitions (~> 1.0)
      aws-sigv4 (~> 1.1)
      jmespath (~> 1.0)
    aws-sdk-sns (1.19.0)
      aws-sdk-core (~> 3, >= 3.61.1)
      aws-sigv4 (~> 1.1)
    aws-sdk-sqs (1.21.0)
      aws-sdk-core (~> 3, >= 3.61.1)
      aws-sigv4 (~> 1.1)
    aws-sigv4 (1.1.0)
      aws-eventstream (~> 1.0, >= 1.0.2)
    bcrypt (3.1.13)
    bindex (0.8.1)
    bootsnap (1.4.4)
      msgpack (~> 1.0)
    builder (3.2.3)
    byebug (11.0.1)
    capybara (3.28.0)
      addressable
      mini_mime (>= 0.1.3)
      nokogiri (~> 1.8)
      rack (>= 1.6.0)
      rack-test (>= 0.6.3)
      regexp_parser (~> 1.5)
      xpath (~> 3.2)
    childprocess (1.0.1)
      rake (< 13.0)
    coderay (1.1.2)
    concurrent-ruby (1.1.5)
    crass (1.0.4)
    devise_invitable (2.0.1)
      actionmailer (>= 5.0)
      devise (>= 4.6)
    diff-lcs (1.3)
    docile (1.3.2)
    dox (1.1.0)
      rails (>= 4.0)
      rspec-core
    erubi (1.8.0)
    factory_bot (5.0.2)
      activesupport (>= 4.2.0)
    factory_bot_rails (5.0.2)
      factory_bot (~> 5.0.2)
      railties (>= 4.2.0)
    faraday (0.15.4)
      multipart-post (>= 1.2, < 3)
    ffi (1.11.1)
    formatador (0.2.5)
    globalid (0.4.2)
      activesupport (>= 4.2.0)
    guard (2.15.0)
      formatador (>= 0.2.4)
      listen (>= 2.7, < 4.0)
      lumberjack (>= 1.0.12, < 2.0)
      nenv (~> 0.1)
      notiffany (~> 0.0)
      pry (>= 0.9.12)
      shellany (~> 0.0)
      thor (>= 0.18.1)
    guard-compat (1.2.1)
    guard-rspec (4.7.3)
      guard (~> 2.1)
      guard-compat (~> 1.1)
      rspec (>= 2.99.0, < 4.0)
    hashie (3.6.0)
    i18n (1.6.0)
      concurrent-ruby (~> 1.0)
    icalia-sdk-event-core (0.1.10)
      jsonapi-deserializable (~> 0.2.0)
    icalia-sdk-event-notification (0.1.10)
      activesupport (>= 5.2.0)
      aws-sdk-sns (~> 1.19)
      aws-sdk-sqs (~> 1.20)
      icalia-sdk-event-core (= 0.1.10)
    jmespath (1.4.0)
    json (2.2.0)
    jsonapi-deserializable (0.2.0)
    jsonapi-parser (0.1.1)
    jsonapi-rails (0.4.0)
      jsonapi-parser (~> 0.1.0)
      jsonapi-rb (~> 0.5.0)
    jsonapi-rb (0.5.0)
      jsonapi-deserializable (~> 0.2.0)
      jsonapi-serializable (~> 0.3.0)
    jsonapi-renderer (0.2.2)
    jsonapi-serializable (0.3.1)
      jsonapi-renderer (~> 0.2.0)
    jsonapi_spec_helpers (0.4.10)
      rspec (~> 3.0)
    jwt (2.2.1)
    listen (3.1.5)
      rb-fsevent (~> 0.9, >= 0.9.4)
      rb-inotify (~> 0.9, >= 0.9.7)
      ruby_dep (~> 1.2)
    lograge (0.11.2)
      actionpack (>= 4)
      activesupport (>= 4)
      railties (>= 4)
      request_store (~> 1.0)
    loofah (2.2.3)
      crass (~> 1.0.2)
      nokogiri (>= 1.5.9)
    lumberjack (1.0.13)
    mail (2.7.1)
      mini_mime (>= 0.1.1)
    marcel (0.3.3)
      mimemagic (~> 0.3.2)
    method_source (0.9.2)
    mimemagic (0.3.3)
    mini_mime (1.0.2)
    mini_portile2 (2.4.0)
    minitest (5.11.3)
    msgpack (1.3.1)
    multi_json (1.13.1)
    multi_xml (0.6.0)
    multipart-post (2.1.1)
    nenv (0.3.0)
    nio4r (2.4.0)
    nokogiri (1.10.4)
      mini_portile2 (~> 2.4.0)
    notiffany (0.1.3)
      nenv (~> 0.1)
      shellany (~> 0.0)
    oauth2 (1.4.1)
      faraday (>= 0.8, < 0.16.0)
      jwt (>= 1.0, < 3.0)
      multi_json (~> 1.3)
      multi_xml (~> 0.5)
      rack (>= 1.2, < 3)
    octokit (4.14.0)
      sawyer (~> 0.8.0, >= 0.5.3)
    oj (3.8.1)
    omniauth (1.9.0)
      hashie (>= 3.4.6, < 3.7.0)
      rack (>= 1.6.2, < 3)
    omniauth-github (1.3.0)
      omniauth (~> 1.5)
      omniauth-oauth2 (>= 1.4.0, < 2.0)
    omniauth-oauth2 (1.6.0)
      oauth2 (~> 1.1)
      omniauth (~> 1.9)
    orm_adapter (0.5.0)
    pg (1.1.4)
    pry (0.12.2)
      coderay (~> 1.1.0)
      method_source (~> 0.9.0)
    pry-rails (0.3.9)
      pry (>= 0.10.4)
    public_suffix (3.1.1)
    puma (3.12.1)
    rack (2.0.7)
    rack-cors (1.0.3)
    rack-proxy (0.6.5)
      rack
    rack-test (1.1.0)
      rack (>= 1.0, < 3)
    rails (6.0.0)
      actioncable (= 6.0.0)
      actionmailbox (= 6.0.0)
      actionmailer (= 6.0.0)
      actionpack (= 6.0.0)
      actiontext (= 6.0.0)
      actionview (= 6.0.0)
      activejob (= 6.0.0)
      activemodel (= 6.0.0)
      activerecord (= 6.0.0)
      activestorage (= 6.0.0)
      activesupport (= 6.0.0)
      bundler (>= 1.3.0)
      railties (= 6.0.0)
      sprockets-rails (>= 2.0.0)
    rails-dom-testing (2.0.3)
      activesupport (>= 4.2.0)
      nokogiri (>= 1.6)
    rails-html-sanitizer (1.2.0)
      loofah (~> 2.2, >= 2.2.2)
    railties (6.0.0)
      actionpack (= 6.0.0)
      activesupport (= 6.0.0)
      method_source
      rake (>= 0.8.7)
      thor (>= 0.20.3, < 2.0)
    rake (12.3.3)
    rb-fsevent (0.10.3)
    rb-inotify (0.10.0)
      ffi (~> 1.0)
    regexp_parser (1.6.0)
    request_store (1.4.1)
      rack (>= 1.4)
    responders (3.0.0)
      actionpack (>= 5.0)
      railties (>= 5.0)
    rspec (3.8.0)
      rspec-core (~> 3.8.0)
      rspec-expectations (~> 3.8.0)
      rspec-mocks (~> 3.8.0)
    rspec-core (3.8.2)
      rspec-support (~> 3.8.0)
    rspec-expectations (3.8.4)
      diff-lcs (>= 1.2.0, < 2.0)
      rspec-support (~> 3.8.0)
    rspec-mocks (3.8.1)
      diff-lcs (>= 1.2.0, < 2.0)
      rspec-support (~> 3.8.0)
    rspec-rails (3.8.2)
      actionpack (>= 3.0)
      activesupport (>= 3.0)
      railties (>= 3.0)
      rspec-core (~> 3.8.0)
      rspec-expectations (~> 3.8.0)
      rspec-mocks (~> 3.8.0)
      rspec-support (~> 3.8.0)
    rspec-support (3.8.2)
    ruby_dep (1.5.0)
    rubyzip (1.2.3)
    sass (3.7.4)
      sass-listen (~> 4.0.0)
    sass-listen (4.0.0)
      rb-fsevent (~> 0.9, >= 0.9.4)
      rb-inotify (~> 0.9, >= 0.9.7)
    sass-rails (5.1.0)
      railties (>= 5.2.0)
      sass (~> 3.1)
      sprockets (>= 2.8, < 4.0)
      sprockets-rails (>= 2.0, < 4.0)
      tilt (>= 1.1, < 3)
    sawyer (0.8.2)
      addressable (>= 2.3.5)
      faraday (> 0.8, < 2.0)
    selenium-webdriver (3.142.3)
      childprocess (>= 0.5, < 2.0)
      rubyzip (~> 1.2, >= 1.2.2)
    sentry-raven (2.11.0)
      faraday (>= 0.7.6, < 1.0)
    shellany (0.0.1)
    shoryuken (5.0.1)
      aws-sdk-core (>= 2)
      concurrent-ruby
      thor
    shoulda-matchers (4.1.2)
      activesupport (>= 4.2.0)
    simplecov (0.17.0)
      docile (~> 1.1)
      json (>= 1.8, < 3)
      simplecov-html (~> 0.10.0)
    simplecov-html (0.10.2)
    spring (2.1.0)
    spring-commands-rspec (1.0.4)
      spring (>= 0.9.1)
    spring-commands-shoryuken (1.0.0)
      spring (>= 0.9.1)
    spring-watcher-listen (2.0.1)
      listen (>= 2.7, < 4.0)
      spring (>= 1.2, < 3.0)
    sprockets (3.7.2)
      concurrent-ruby (~> 1.0)
      rack (> 1, < 3)
    sprockets-rails (3.2.1)
      actionpack (>= 4.0)
      activesupport (>= 4.0)
      sprockets (>= 3.0.0)
    thor (0.20.3)
    thread_safe (0.3.6)
    tilt (2.0.9)
    turbolinks (5.2.0)
      turbolinks-source (~> 5.2)
    turbolinks-source (5.2.0)
    tzinfo (1.2.5)
      thread_safe (~> 0.1)
    warden (1.2.8)
      rack (>= 2.0.6)
    web-console (4.0.1)
      actionview (>= 6.0.0)
      activemodel (>= 6.0.0)
      bindex (>= 0.4.0)
      railties (>= 6.0.0)
    webpacker (4.0.7)
      activesupport (>= 4.2)
      rack-proxy (>= 0.6.1)
      railties (>= 4.2)
    websocket-driver (0.7.1)
      websocket-extensions (>= 0.1.0)
    websocket-extensions (0.1.4)
    xpath (3.2.0)
      nokogiri (~> 1.8)
    zeitwerk (2.1.9)

PLATFORMS
  ruby

DEPENDENCIES
  aws-sdk-sqs (~> 1.20)
  bootsnap (>= 1.4.2)
  byebug
  capybara (~> 3.28)
  database_cleaner (~> 1.7)!
  devise (~> 4.6, >= 4.6.2)!
  devise_invitable (~> 2.0, >= 2.0.1)
  doorkeeper (~> 5.1)!
  dox (~> 1.1)
  factory_bot_rails (~> 5.0, >= 5.0.2)
  guard-rspec (~> 4.7, >= 4.7.3)
  icalia-sdk-event-notification (~> 0.1.10)
  jsonapi-rails (~> 0.4.0)
  jsonapi_spec_helpers (~> 0.4.10)
  listen (>= 3.0.5, < 3.2)
  lograge (~> 0.11.2)
  octokit (~> 4.14)
  oj (~> 3.8, >= 3.8.1)
  omniauth-github (~> 1.3)
  pg (>= 0.18, < 2.0)
  pry-rails (~> 0.3.9)
  puma (~> 3.11)
  rack-cors (~> 1.0, >= 1.0.3)
  rails (~> 6.0.0)
  rspec-rails (~> 3.8, >= 3.8.2)
  sass-rails (~> 5)
  selenium-webdriver (~> 3.142, >= 3.142.3)
  sentry-raven (~> 2.11)
  shoryuken (~> 5.0, >= 5.0.1)
  shoulda-matchers (~> 4.1, >= 4.1.2)
  simplecov (~> 0.17.0)
  spring
  spring-commands-rspec (~> 1.0, >= 1.0.4)
  spring-commands-shoryuken (~> 1.0)
  spring-watcher-listen (~> 2.0.0)
  turbolinks (~> 5)
  tzinfo-data
  web-console (>= 3.3.0)
  webpacker (~> 4.0)

RUBY VERSION
   ruby 2.6.3p62

BUNDLED WITH
   1.17.2

[vovimayhem]

Is the Doorkeeper::OAuth::TokenRequest#authorize not calling any before_successful_response or after_successful_response implementation on purpose?

I noticed Doorkeeper::OAuth::TokenRequest is not inheriting from Doorkeeper::OAuth::BaseRequest. I see how this was done on purpose, but not the skipping of those calls.

(Regardless of the "implicit" grant flow being frown upon...)

[linhdangduy]

Could you consider this?
Because the result of implicit grant flow is return at method authorizations_controller.rb#authorize_response as below:

doorkeeper/app/controllers/doorkeeper/authorizations_controller.rb

Lines 88 to 97 in d84885a

	def authorize_response
	@authorize_response ||= begin
	return pre_auth.error_response unless pre_auth.authorizable?
	before_successful_authorization
	auth = strategy.authorize
	after_successful_authorization
	auth
	end
	end

How about using before/after_successful_authorization?

before_successful_authorization do |context|
  if context.pre_auth.response_type == "token"
    puts "BEFORE HOOK FIRED! #{context}"
  end
end

after_successful_authorization do |context|
  if context.pre_auth.response_type == "token"
    puts "AFTER HOOK FIRED! #{context}"
  end
end

1. These hooks are called at authorizations controller, so if you want to call it only on the implicit flow request, check it by context.pre_auth.response_type == "token".

2. We can see that after_successful_authorization do |context| hook is passed only param context, not have the response param like after_successful_strategy_response do |request, response. In this case, may be we should propose a PR that include auth (which is the response) in to params of after_successful_authorization, as below:

after_successful_authorization(self, auth)

def after_successful_authorization
  Doorkeeper.configuration.after_successful_authorization.call(self, auth)
end

[nbulaj]

@linhdangduy is right. Also try Doorkeeper from master branch, it has some impovements (like #1264).

[nbulaj]

Any update here?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nbulaj]

Manually closing staled issues without any activity and responses