Doorkeeper: Cleanup AccessGrant and AccessToken?

[waynerobinson]

I've had a look through the source code and I can't seem to find any existing method to cleanup the DB tables for AccessGrant and AccessToken for expired and revoked entries.

Have I missed an existing cleanup method?

Is there a reason why I wouldn't want to delete expired and revoked entries?

Would you accept a PR for methods to run regularly that delete tokens expired/revoked after a period of time (e.g. daily script that deletes expired/revoked tokens older than 3 months)?

[renatovico]

+1

[tute]

I'm happy to see a wiki page explaining how to do it in code. This would just be ORM (say, ActiveRecord) application syntax. If a lot of people need it and like it I will consider a PR. Thanks!

[waynerobinson]

NP, let me do a little more research.

[waynerobinson]

Just to confirm I haven't missed anything, would the following service be sufficient for cleaning up old, expired & revoked tokens? I understand this is MySQL-specific code.

class CleanupTokens

  attr_reader :keep_for_days

  def initialize(keep_for_days: 30)
    @keep_for_days = keep_for_days
  end

  def cleanup!
    Doorkeeper::AccessGrant.transaction {
      expired_grants.delete_all
      expired_tokens.delete_all
    }
  end

  def expired_grants
    Doorkeeper::AccessGrant.where(expiry_params)
  end

  def expired_tokens
    Doorkeeper::AccessToken.where(expiry_params)
  end

  def delete_before
    Time.now - keep_for_days.days
  end

  protected

    def expiry_params
      ["revoked_at < :delete_before OR (created_at + INTERVAL expires_in SECOND) < :delete_before", delete_before: delete_before]
    end

end

[bastien-tilkee]

+1, is there a reason expired tokens are kept in the DB ?

[tute]

There's no specific reason that I know of. We could use them to return more meaningful error messages, but AFAIK we don't.

[francesle]

@bastien-tilkee @tute

Is it safe to remove tokens that haven't been revoked? The second part of the query in expiry_params could match on an oauth_access_token that hasn't been revoked yet, removing a refresh token even though they're not supposed have a TTL expiration. What happens when the client makes a request with the associated refresh token? Won't the user have to re-authorize the app?

Seems like we should only be cleaning out any tokens that have been revoked in the past, right?

[warmwind]

+1. What is the best way to cleanup all the revoked access_tokens? There are too many of them kept in db and not used any longer.

[TheDeadCode]

+1

[tute]

+1 to what idea, specifically?

Does this comment address your need?

[TheDeadCode]

Yes, as it can be seamlessly integrated into sidekiq for crons. Any feature that let's me clear revoked or expired tokens would be perfectly sufficient.

[tute]

Great! For now I'd rather let doorkeeper keep the tokens by default. Can you incorporate that code into your application, and solve your issue? If so, would you be so kind to document this in the wiki? Thank you very much!

[TheDeadCode]

I will do a PR into gitlab in the upcoming weeks containing this feature. I'll document it right after, if I ever find the time. Cheers!

[GUI]

If it helps anyone else, here's a slight variation on @waynerobinson's approach, with the following differences:

• Uses PostgreSQL compatible SQL query for the created_at and expires_in math.
• Getting to @francesle's earlier point, it does not remove tokens unless they're explicitly revoked or expired (tokens that aren't expired or revoked will remain around indefinitely).
• Rake task instead of a class.

desc "Trim old OAuth tokens from the tables (default: > 30 days)."
task :trim_doorkeeper_tokens => :environment do
  delete_before = (ENV["DOORKEEPER_DAYS_TRIM_THRESHOLD"] || 30).to_i.days.ago
  expire = [
    "(revoked_at IS NOT NULL AND revoked_at < :delete_before) OR " +
    "(expires_in IS NOT NULL AND (created_at + expires_in * INTERVAL '1 second') < :delete_before)",
    { :delete_before => delete_before },
  ]
  Doorkeeper::AccessGrant.where(expire).delete_all
  Doorkeeper::AccessToken.where(expire).delete_all
end

[hubertjakubiak]

If you are going to delete old access tokens for the first time after long period of time not doing it at all, it could be safer to remove them in batches like:

Doorkeeper::AccessGrant.where(expire).in_batches(&:delete_all)
Doorkeeper::AccessToken.where(expire).in_batches(&:delete_all)