NoMethodError raised when fallback access token lookup fails

[JGrishey]

Steps to reproduce

Create an application that uses access token hashing with an active hashing algorithm and a fallback hashing algorithm.

Expected behavior

In theory, the application should return a 401 Unauthorized when making a request to a protected resource with an invalid access token.

Actual behavior

Instead, the application returns a 500 Internal Server Error when the fallback lookup fails. Here is the backtrace, which fails when trying to use #public_send for :token=:

11:20:20 web.1    | Completed 500 Internal Server Error in 187ms
11:20:20 web.1    |
11:20:20 web.1    |
11:20:20 web.1    |
11:20:20 web.1    | NoMethodError (undefined method `token=' for nil:NilClass):
11:20:20 web.1    |
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/secret_storing/base.rb:23:in `public_send'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/secret_storing/base.rb:23:in `store_secret'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/concerns/secret_storable.rb:86:in `upgrade_fallback_value'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/concerns/secret_storable.rb:69:in `block in find_by_fallback_token'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/concerns/secret_storable.rb:68:in `tap'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/concerns/secret_storable.rb:68:in `find_by_fallback_token'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/concerns/secret_storable.rb:48:in `find_by_plaintext_token'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/models/access_token_mixin.rb:27:in `by_token'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/oauth/token.rb:17:in `authenticate'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) lib/doorkeeper/helpers/controller.rb:35:in `doorkeeper_token'
11:20:20 web.1    | doorkeeper (5.1.0.rc2) app/controllers/doorkeeper/token_info_controller.rb:6:in `show'

System configuration

Doorkeeper initializer:
Standard except for:

hash_token_secrets using: '::Doorkeeper::SecretStoring::Sha512Hash', fallback: '::Doorkeeper::SecretStoring::Sha256Hash'

Ruby version:
Ruby 2.4.5

This is in Doorkeeper version 5.1.0.rc2

[nbulaj]

Hi @JGrishey . Thanks for reporting this issue! I'll look at it a little bit later because I'm busy right now.

/cc @oliverguenther

[JGrishey]

I should include my implementation of ::Doorkeeper::SecretStoring::Sha512Hash, which mirrors the Sha256 implementation:

module Doorkeeper
  module SecretStoring
    class Sha512Hash < Base
      def self.transform_secret(plain_secret)
        ::Digest::SHA512.hexdigest plain_secret
      end

      def self.allows_restoring_secrets?
        false
      end
    end
  end
end

[JGrishey]

I think the issue lies here:

doorkeeper/lib/doorkeeper/models/concerns/secret_storable.rb

Lines 68 to 70 in 1599577

	find_by(attr => stored_token).tap do |resource|
	upgrade_fallback_value resource, attr, plain_secret
	end

#find_by will return nil, which sends nil into resource which gets sent into #upgrade_fallback_value, causing the NoMethodError on #public_send with :token=. There should probably be a guard clause here for the nil value so that nil gets returned when there is no token found by the fallback, which is indicated by the method's head comment:

upgrade_fallback_value resource, attr, plain_secret if resource

I can submit a PR with this code, if desired :)

[oliverguenther]

Thank you for finding Jacob, I have missed this. I'm on vacation without my laptop at the moment so cannot provide a PR until second week of April, but your observation is absolutely correct. Am 26. März 2019 08:59:03 schrieb Jacob Grishey <[email protected]>:

…

I think the issue lies here: https://github.com/doorkeeper-gem/doorkeeper/blob/1599577d36e55494afb6046f3d4d7515077e1b3a/lib/doorkeeper/models/concerns/secret_storable.rb#L68-L70 #find_by will return nil, which sends nil into resource which gets sent into #upgrade_fallback_value, causing the NoMethodError on #public_send with :token=. There should probably be a guard clause here for the nil value so that nil gets returned when there is no token found by the fallback, which is indicated by the method's head comment: upgrade_fallback_value resource, attr, plain_secret if resource I can submit a PR with this code, if desired :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[nbulaj]

Hi @JGrishey . Feel free to send a PR :)

[JGrishey]

Sure! I'll work on that today (hopefully!).

[nbulaj]

Fixed by #1236, thanks @JGrishey !