It is not possible to revoke refresh token bound to the expired access token

[syakovyn]

A copy of #1579 but with a focus on refresh tokens.

Steps to reproduce

Request to revoke a refresh token bound to the expired access token.

Expected behavior

The refresh token is revoked and can not be used to get a new access token.

Actual behavior

The refresh token is not revoked and can be used to get a new access token.

System configuration

Doorkeeper initializer:

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

Ruby version: 3.1.4

Gemfile.lock:

Gemfile.lock content

...
doorkeeper (5.6.6)
...

Reproduction tests

RSpec test to show the issue (simplified):

# Simplified User that has no validations and has only an ID column. That is enough to reproduce the issue
class User < ActiveRecord::Base
end

# The spec
require 'rails_helper'

describe Doorkeeper::TokensController, type: :request do
  it 'revokes refresh token bound to expired access token' do
    mobile_app = Doorkeeper::Application.create!(name: 'Mobile App', redirect_uri: 'mobile://callback', confidential: false)
    user = User.create!
    access_token =  Dookeeper::AccessToken.create!(resource_owner: user, application: mobile_app, use_refresh_token: true, expires_in: 0) # Using expires_in = 0 to create an expired access token

    post '/oauth/revoke', params: { client_id: mobile_app.uid, token: access_token.refresh_token, token_type_hint: 'refresh_token' }

    expect(response).to have_http_status :ok
    expect(access_token.reload).to be_revoked # That means the refresh token remains valid despite the fact that the revoke endpoint returned OK

    # Comment`expect(access_token.reload).to be_revoked` to get here
    basic_auth = "Basic #{Base64.strict_encode64("#{mobile_app.uid}:#{mobile_app.secret}")}"
    post '/oauth/token', params: { grant_type: :refresh_token, refresh_token: access_token.refresh_token }, headers: { Authorization: basic_auth }

    expect(response).not_to have_http_status :ok # It fails meaning that the "revoked" refresh token can be successfully used to get a new access token.
  end
end

[syakovyn]

Moreover, introspection of such a refresh token (bound to an expired access token) will respond with a misleading "active": false.

[ThisIsMissEm]

I think this is because the refresh tokens aren't actually Doorkeeper::AccessTokens themselves, but just a string value on an AccessToken, so you can't "revoke" the refresh token, but you can revoke the access token it belongs to.

I think what you're seeing with the revoke endpoint is that in order to not disclose that the token did exist, it's always return 200, regardless of whether or not a token was revoked.

[mquan]

imo this is a bug. The token parameter used in the oauth/revoke endpoint is the refresh_token, the expires_at field is only applicable to the access_token.

Taking a step back, the point of revocation is to remove further usages. It's a serious flaw if tokens cannot be revoked b/c of expiration but can be refreshed. It's kinda silly to workaround by refreshing for a new token in order to revoke.