Doorkeeper: The authorization should not be triggered when a previous expired token is provided and `reuse_access_token` is enabled.

[Erowlin]

Steps to reproduce

Flag reuse_access_token is turned on. Whenever the access_token expires, instead of silently renewing the access_token, the user is redirected to the authorization/new page where she has to click on "Accept" or "Decline". (The user approved the application previously, and gave the expired access token in the request).

The incriminating commit: https://github.com/doorkeeper-gem/doorkeeper/commit/c87b13c363073d4a06bada2ee2efd979b6329090#diff-1282d211312f992c2a614d1f4a6302fe - especially the token.accessible? check.

• it's not mentioned in the Changelog for 5.0.0 while it should've been, it's a change of behavior
• It now triggers an authorization window whenever my access_token is expired, while I specified to reuse_access_token (so the renewal should be invisible, the user should not have to approve again the application).

doorkeeper.rb:

Doorkeeper.configure do
  # Change the ORM that doorkeeper will use (needs plugins)
  orm :active_record

  # This block will be called to check whether the resource owner is authenticated or not.
  resource_owner_authenticator do
    sign_out(current_user) unless current_user&.is_a?(Admin)
    session[:user_return_to] = request.fullpath
    current_user&.is_a?(Admin) ? current_user : redirect_to(new_user_session_url)
  end

  # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
  admin_authenticator do
    current_user.is_a?(Admin) || redirect_to(new_user_session_url)
  end

  # resource_owner_from_credentials do |routes|
  #   admin = Admin.find_for_database_authentication(email: params[:username])
  #   if admin && admin.valid_for_authentication? { admin.valid_password?(params[:password]) }
  #     admin
  #   end
  # end

  # Authorization Code expiration time (default 10 minutes).
  # authorization_code_expires_in 10.minutes

  # Access token expiration time (default 2 hours).
  # If you want to disable expiration, set this to nil.
  # access_token_expires_in 2.hours

  # Assign a custom TTL for implicit grants.
  # custom_access_token_expires_in do |oauth_client|
  #   oauth_client.application.additional_settings.implicit_oauth_expiration
  # end

  # Use a custom class for generating the access token.
  # https://github.com/doorkeeper-gem/doorkeeper#custom-access-token-generator
  # access_token_generator '::Doorkeeper::JWT'

  # The controller Doorkeeper::ApplicationController inherits from.
  # Defaults to ActionController::Base.
  # https://github.com/doorkeeper-gem/doorkeeper#custom-base-controller
  base_controller 'ActionController::Base'

  # Reuse access token for the same resource owner within an application (disabled by default)
  # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383
  reuse_access_token

  # Issue access tokens with refresh token (disabled by default)
  # use_refresh_token

  # Provide support for an owner to be assigned to each registered application (disabled by default)
  # Optional parameter confirmation: true (default false) if you want to enforce ownership of
  # a registered application
  # Note: you must also run the rails g doorkeeper:application_owner generator to provide the necessary support
  # enable_application_owner confirmation: false

  # Define access token scopes for your provider
  # For more information go to
  # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
  # default_scopes  :public
  # optional_scopes :write, :update

  # Change the way client credentials are retrieved from the request object.
  # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
  # falls back to the `:client_id` and `:client_secret` params from the `params` object.
  # Check out the wiki for more information on customization
  # client_credentials :from_basic, :from_params

  # Change the way access token is authenticated from the request object.
  # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
  # falls back to the `:access_token` or `:bearer_token` params from the `params` object.
  # Check out the wiki for more information on customization
  # access_token_methods :from_bearer_authorization, :from_access_token_param, :from_bearer_param

  # Change the native redirect uri for client apps
  # When clients register with the following redirect uri, they won't be redirected to any server and the authorization code will be displayed within the provider
  # The value can be any string. Use nil to disable this feature. When disabled, clients must provide a valid URL
  # (Similar behaviour: https://developers.google.com/accounts/docs/OAuth2InstalledApp#choosingredirecturi)
  #
  native_redirect_uri 'urn:ietf:wg:oauth:2.0:oob'

  # Forces the usage of the HTTPS protocol in non-native redirect uris (enabled
  # by default in non-development environments). OAuth2 delegates security in
  # communication to the HTTPS protocol so it is wise to keep this enabled.
  #
  # force_ssl_in_redirect_uri !Rails.env.development?

  # Specify what grant flows are enabled in array of Strings. The valid
  # strings and the flows they enable are:
  #
  # "authorization_code" => Authorization Code Grant Flow
  # "implicit"           => Implicit Grant Flow
  # "password"           => Resource Owner Password Credentials Grant Flow
  # "client_credentials" => Client Credentials Grant Flow
  #
  # If not specified, Doorkeeper enables authorization_code and
  # client_credentials.
  #
  # implicit and password grant flows have risks that you should understand
  # before enabling:
  #   http://tools.ietf.org/html/rfc6819#section-4.4.2
  #   http://tools.ietf.org/ht§l/rfc6819#section-4.4.3
  #
  grant_flows %w(implicit)

  # Under some circumstances you might want to have applications auto-approved,
  # so that the user skips the authorization step.
  # For example if dealing with a trusted application.
  # skip_authorization do |resource_owner, client|
  #   client.superapp? or resource_owner.admin?
  # end

  # WWW-Authenticate Realm (default "Doorkeeper").
  # realm "Doorkeeper"
end

System configuration

Doorkeeper 5.0.0

[nbulaj]

Hi @Erowlin . I'm terribly busy right now, would you like to submit a revert fix for this?

[Erowlin]

Sure, do you confirm that it is RFC compliant?

[nbulaj]

As far as I remember RFC 6749 says nothing about token reuse, so this functionality looks like an extension to OAuth protocol. Fix me if I'm wrong. This changes were done during the investigation of expired tokens issues, and now looks more like a bug.

[nbulaj]

Hi @Erowlin . Would you like to propose a fix? I have some troubles with a free time now, so don't sure when I will be able to fix it myself

[nbulaj]

Hi @Erowlin . I have 10 minutes of free time :D
Could you please check this branch with the changes related to this issue? https://github.com/doorkeeper-gem/doorkeeper/compare/revert-expiration-check-for-matching-token

Thanks

[Erowlin]

Thanks for your time, sorry for my silence radio, busy times here as well.

I'm checking now.

[Erowlin]

Ok, it seems to be fixed, however, 1 behavior still not fixed:

• When I update manually my expires_in in 1 second, a new token is issued instead of renewing the actual token (And extending the expires_in). Is it the intended behavior?

(It's my localhost, don't worry).

[nbulaj]

I think this is related to this line

[Erowlin]

All right. Don’t you think we should add a condition on the fact we enabled
“reuse_access_token” in the configuration file? Do you have any though
about this?

On Thu 20 Sep 2018 at 23:37, Nikita Bulai [e-mail] wrote:

I think this is related to this line
https://github.com/doorkeeper-gem/doorkeeper/blob/master/lib/doorkeeper/models/access_token_mixin.rb#L126

[nbulaj]

I think that reuse access token currently works fine - it protects from creating new tokens before old one becomes expired. Please, read the orinal PR and issue: #383 and #387 . It doesn't work ad "update old token expiration date instead of creating new token" . Also there are refresh token strategy also..

[Erowlin]

All right, let’s keep it as it is then, thanks for your investigation :)

On Thu 20 Sep 2018 at 23:42, Nikita Bulai [e-mail] wrote:

I think that reuse access token currently works fine - it protects from
creating new tokens before old one becomes expired. Please, read the orinal
PR and issue: #383
https://github.com/doorkeeper-gem/doorkeeper/issues/383 and #387
https://github.com/doorkeeper-gem/doorkeeper/pull/387 . It doesn't work
ad "update old token expiration date instead of creating new token" . Also
there are refresh token strategy also..