Doorkeeper: Unable to delete authorized RP application

[syakovyn]

Steps to reproduce

1. Create an application
2. User authorizes application
3. Go to doorkeeper admin page and delete application

Expected behavior
Able to delete application

Actual behavior
Unable to delete application:

Mysql2::Error - Cannot delete or update a parent row: a foreign key constraint fails (sample_app.oauth_openid_requests, CONSTRAINT fk_rails_77114b3b09 FOREIGN KEY (access_grant_id) REFERENCES oauth_access_grants (id)):

Reason
Doorkeeper::Application
https://github.com/doorkeeper-gem/doorkeeper/blob/45b6ce568523a5e2f2cdc9c3ed60254d7891f8dc/lib/doorkeeper/orm/active_record/application.rb#L3-L11

Doorkeeper::OpenidConnect::AccessGrant
https://github.com/doorkeeper-gem/doorkeeper-openid_connect/blob/302575597532ecaf5a4ae8000b2c387f7d08371a/lib/doorkeeper/openid_connect/orm/active_record/access_grant.rb#L1-L10

module Doorkeeper
  module OpenidConnect
    module AccessGrant
      def self.prepended(base)
        base.class_eval do
          has_one :openid_request,
            class_name: 'Doorkeeper::OpenidConnect::Request',
            inverse_of: :access_grant,
            dependent: :delete
        end

To recap:

Application has_many :access_grants, dependent: :delete_all
AccessGrant has_one :openid_request, dependent: :delete

Changing Application has_many :access_grants, dependent: :delete_all to Application has_many :access_grants, dependent: :destroy should fix the issue.

[truongnmt]

@syakovyn Can you update the description so that this issue will be more doorkeeper related? Thanks for reporting.
Btw I'm happy to work on such PR, Hacktoberfest is coming :D

[syakovyn]

@truongnmt, updated. I just extracted the key points from your description in https://github.com/doorkeeper-gem/doorkeeper-openid_connect/issues/82

[nbulaj]

Hi @syakovyn , @truongnmt .

I remember we changed/added the dependent option to delete_all because of performance. In case application has too much tokens/grants, destroy_all (yep, it's not destroy because it's has_many association) will load all the records to the memory and call #destroy for each. It costs.

[syakovyn]

In our app we fixed that by overriding Doorkeeper::Application#destroy that way:

def destroy
  Doorkeeper::OpenidConnect::Request.where(access_grant_id: access_grants.select(:id)).delete_all
  super
end

[nbulaj]

I think the same could be done inside Doorkeeper::OpenidConnect, WDYT ? It's already patches some Doorkeeper internals so ... why not? :see_no_evil: @syakovyn

[nbulaj]

So do we have any reason to not to close this issue? :)

[toupeira]

@nbulaj thanks for your response, looks like this is a known problem in Rails: https://github.com/rails/rails/issues/22510

I'm happy to accept a monkey-patch for this in Doorkeeper::OpenidConnect, so yes I think this issue can be closed.