Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doorkeeper::AccessToken.find_or_create_for unnecessarily refetches application object #1574

Closed
stereoscott opened this issue Jun 6, 2022 · 1 comment · Fixed by #1599
Closed
Labels

Comments

@stereoscott
Copy link

stereoscott commented Jun 6, 2022

When creating a new token for an application, despite passing an application object into the method, it's refetched from the database when the token is generated.

Steps to reproduce

Create a token for an existing application:

app = Doorkeeper::Application.find_or_create_by(name: "API")
Doorkeeper::AccessToken.find_or_create_for(
      application: app,
      resource_owner: id,
      scopes: Doorkeeper::OAuth::Scopes.from_array(%w(public))
)

Expected behavior

When passing an object into the find_or_create_for method, it should use that object reference rather than refetching the application object from the DB.

Actual behavior

Here is the create_for(...) method

      def create_for(application:, resource_owner:, scopes:, **token_attributes)
        token_attributes[:application_id] = application&.id
        token_attributes[:scopes] = scopes.to_s

        if Doorkeeper.config.polymorphic_resource_owner?
          token_attributes[:resource_owner] = resource_owner
        else
          token_attributes[:resource_owner_id] = resource_owner_id_for(resource_owner)
        end
        create!(token_attributes)
      end

Because :application is used as a keyword argument, it's removed from the token_attributes hash which is passed further down into the create! method. Anytime a new AccessToken is created, generate_token is called which in turn calls attributes_for_token_generator, which passes a full application reference to the token generator.

    # Set of attributes that would be passed to token generator to
    # generate unique token based on them.
    #
    #  @return [Hash] set of attributes
    #
    def attributes_for_token_generator
      {
        resource_owner_id: resource_owner_id,
        scopes: scopes,
        application: application, # see here, this causes a refetch
        expires_in: expires_in,
        created_at: created_at,
      }.tap do |attributes|
        if Doorkeeper.config.polymorphic_resource_owner?
          attributes[:resource_owner] = resource_owner
        end
      end
    end

Because the create! method is only getting passed the application_id and not the full application, when application is called from inside attributes_for_token_generator it causes the application to be fetched from the database again.

This could be avoided by passing the application into token_attributes rather than passing in the id only.

System configuration

Doorkeeper 5.4.0

stereoscott added a commit to quikly/doorkeeper that referenced this issue Jun 6, 2022
This avoids refetching the application object from the database when it's referenced inside of `attributes_for_token_generator`. Closes doorkeeper-gem#1574
stereoscott added a commit to quikly/doorkeeper that referenced this issue Jun 6, 2022
This avoids refetching the application object from the database when it's referenced inside of `attributes_for_token_generator`. Closes doorkeeper-gem#1574
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant