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

Authorizations Controller: service.pre_auth.scopes different than service.request.pre_auth.scopes #1576

Open
notfelineit opened this issue Jun 14, 2022 · 2 comments
Labels
pinned For issues that can be stale

Comments

@notfelineit
Copy link

notfelineit commented Jun 14, 2022

Steps to reproduce

I have a custom authorizations_controller#create method. I'm passing in scope in the params such that params[:scope] is a non default scope write_organization.

The token that is generated is generated with the default scopes, and not the scope that was in params.

My code is as follows for the create method:

def create
        # some unrelated preprocessing above
        response = authorize_response
        binding.pry
        redirect_or_render(authorize_response)
end

at the binding.pry breakpoint, my params are as follows:

[69] pry(#<Api::Internal::OauthAuthorizationsController>)> params
=> #<ActionController::Parameters {"client_id"=>"<SOME_ID>", "redirect_uri"=>"<SOME_REDIRECT_URI>", "scope"=>"write_organization", "response_type"=>"code", "format"=>:json, "controller"=>"api/internal/oauth_authorizations", "action"=>"create"} permitted: false>

Using binding.pry, I can inspect the differences in OauthAuthorizationsController, which inherits from Doorkeeper::AuthorizationsController:

[67] pry(#<Api::Internal::OauthAuthorizationsController>)> strategy.request.pre_auth.scopes
=> #<Doorkeeper::OAuth::Scopes:0x00000001410eef10 @scopes=["read_databases", "read_user", "read_organization"]>
[68] pry(#<Api::Internal::OauthAuthorizationsController>)> strategy.pre_auth.scopes
=> #<Doorkeeper::OAuth::Scopes:0x000000014111e990 @scopes=["write_organization"]>

Expected behavior

I would expect strategy.request.pre_auth.scopes to match the scopes in strategy.pre_auth.scopes.

Actual behavior

strategy.request.pre_auth.scopes always returns the default scopes, instead of the scope explicitly passed in via params.

System configuration

You can help us to understand your problem if you will share some very
useful information about your project environment (don't forget to
remove any confidential data if it exists).

Doorkeeper initializer:

# config/initializers/doorkeeper.rb
# frozen_string_literal: true

Doorkeeper.configure do
  Devise::Doorkeeper.configure_doorkeeper(self)

  default_scopes :read_databases, :read_user, :read_organization
  optional_scopes :write_databases, :write_user, :write_organization,
    # Oauth v2 scopes that match service token scopes
    # Database scopes
    :create_database, :read_database, :delete_database,
    # Branch scopes
    :create_branch, :read_branch, :delete_branch, :delete_production_branch, :connect_branch, :connect_production_branch,
    # DR scopes
    :create_deploy_request, :read_deploy_request, :approve_deploy_request, :create_comment, :read_comment

  enforce_configured_scopes

  # Change the ORM that doorkeeper will use (requires ORM extensions installed).
  # Check the list of supported ORMs here: https://github.com/doorkeeper-gem/doorkeeper#orms
  orm :active_record

  # This block will be called to check whether the resource owner is authenticated or not.
  resource_owner_authenticator do
    current_user || warden.authenticate!(scope: :user)
  end

  # If you didn't skip applications controller from Doorkeeper routes in your application routes.rb
  # file then you need to declare this block in order to restrict access to the web interface for
  # adding oauth authorized applications. In other case it will return 403 Forbidden response
  # every time somebody will try to access the admin web interface.
  #
  admin_authenticator do
    unless current_user
      redirect_to sign_in_url
    end
  end

  grant_flows [
    # This is a non-standard grant flow, used instead of the
    # official `urn:ietf:params:oauth:grant-type:device_code` due to
    # current Doorkeeper limitations.
    "device_code",

    # together with all the other grant flows you already enabled, for example:
    "authorization_code",
    "client_credentials",
  ]

  # You can use your own model classes if you need to extend (or even override) default
  # Doorkeeper models such as `Application`, `AccessToken` and `AccessGrant.
  #
  # Be default Doorkeeper ActiveRecord ORM uses it's own classes:
  #
  access_token_class "AccessToken"
  access_grant_class "AccessGrant"
  application_class "OauthApplication"
  #
  # Don't forget to include Doorkeeper ORM mixins into your custom models:
  #
  #   *  ::Doorkeeper::Orm::ActiveRecord::Mixins::AccessToken - for access token
  #   *  ::Doorkeeper::Orm::ActiveRecord::Mixins::AccessGrant - for access grant
  #   *  ::Doorkeeper::Orm::ActiveRecord::Mixins::Application - for application (OAuth2 clients)
  #
  # For example:
  #
  # access_token_class "MyAccessToken"
  #
  # class MyAccessToken < ApplicationRecord
  #   include ::Doorkeeper::Orm::ActiveRecord::Mixins::AccessToken
  #
  #   self.table_name = "hey_i_wanna_my_name"
  #
  #   def destroy_me!
  #     destroy
  #   end
  # end

  # Enables polymorphic Resource Owner association for Access Tokens and Access Grants.
  # By default this option is disabled.
  #
  # Make sure you properly setup you database and have all the required columns (run
  # `bundle exec rails generate doorkeeper:enable_polymorphic_resource_owner` and execute Rails
  # migrations).
  #
  # If this option enabled, Doorkeeper will store not only Resource Owner primary key
  # value, but also it's type (class name). See "Polymorphic Associations" section of
  # Rails guides: https://guides.rubyonrails.org/association_basics.html#polymorphic-associations
  #
  # [NOTE] If you apply this option on already existing project don't forget to manually
  # update `resource_owner_type` column in the database and fix migration template as it will
  # set NOT NULL constraint for Access Grants table.
  #
  # use_polymorphic_resource_owner

  # If you are planning to use Doorkeeper in Rails 5 API-only application, then you might
  # want to use API mode that will skip all the views management and change the way how
  # Doorkeeper responds to a requests.
  #
  # api_only

  # Enforce token request content type to application/x-www-form-urlencoded.
  # It is not enabled by default to not break prior versions of the gem.
  #
  # enforce_content_type

  # 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 1.month

  # Assign custom TTL for access tokens. Will be used instead of access_token_expires_in
  # option if defined. In case the block returns `nil` value Doorkeeper fallbacks to
  # +access_token_expires_in+ configuration option value. If you really need to issue a
  # non-expiring access token (which is not recommended) then you need to return
  # Float::INFINITY from this block.
  #
  # `context` has the following properties available:
  #
  # `client` - the OAuth client application (see Doorkeeper::OAuth::Client)
  # `grant_type` - the grant type of the request (see Doorkeeper::OAuth)
  # `scopes` - the requested scopes (see Doorkeeper::OAuth::Scopes)
  #
  custom_access_token_expires_in do |context|
    if context.grant_type == "refresh_token"
      200.days
    end
  end

  # Use a custom class for generating the access token.
  # See https://doorkeeper.gitbook.io/guides/configuration/other-configurations#custom-access-token-generator
  #
  access_token_generator "AccessTokenGenerator"

  # The controller +Doorkeeper::ApplicationController+ inherits from.
  # Defaults to +ActionController::Base+ unless +api_only+ is set, which changes the default to
  # +ActionController::API+. The return value of this option must be a stringified class name.
  # See https://doorkeeper.gitbook.io/guides/configuration/other-configurations#custom-base-controller
  #
  # base_controller 'ApplicationController'

  # Reuse access token for the same resource owner within an application (disabled by default).
  #
  # This option protects your application from creating new tokens before old valid one becomes
  # expired so your database doesn't bloat. Keep in mind that when this option is `on` Doorkeeper
  # doesn't updates existing token expiration time, it will create a new token instead.
  # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383
  #
  # You can not enable this option together with +hash_token_secrets+.
  #
  # reuse_access_token

  # In case you enabled `reuse_access_token` option Doorkeeper will try to find matching
  # token using `matching_token_for` Access Token API that searches for valid records
  # in batches in order not to pollute the memory with all the database records. By default
  # Doorkeeper uses batch size of 10 000 records. You can increase or decrease this value
  # depending on your needs and server capabilities.
  #
  # token_lookup_batch_size 10_000

  # Set a limit for token_reuse if using reuse_access_token option
  #
  # This option limits token_reusability to some extent.
  # If not set then access_token will be reused unless it expires.
  # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/1189
  #
  # This option should be a percentage(i.e. (0,100])
  #
  # token_reuse_limit 100

  # Only allow one valid access token obtained via client credentials
  # per client. If a new access token is obtained before the old one
  # expired, the old one gets revoked (disabled by default)
  #
  # When enabling this option, make sure that you do not expect multiple processes
  # using the same credentials at the same time (e.g. web servers spanning
  # multiple machines and/or processes).
  #
  # revoke_previous_client_credentials_token

  # Hash access and refresh tokens before persisting them.
  # This will disable the possibility to use +reuse_access_token+
  # since plain values can no longer be retrieved.
  #
  # Note: If you are already a user of doorkeeper and have existing tokens
  # in your installation, they will be invalid without enabling the additional
  # setting `fallback_to_plain_secrets` below.
  #
  hash_token_secrets
  # By default, token secrets will be hashed using the
  # +Doorkeeper::Hashing::SHA256+ strategy.
  #
  # If you wish to use another hashing implementation, you can override
  # this strategy as follows:
  #
  # hash_token_secrets using: '::Doorkeeper::Hashing::MyCustomHashImpl'
  #
  # Keep in mind that changing the hashing function will invalidate all existing
  # secrets, if there are any.

  # Hash application secrets before persisting them.
  #
  hash_application_secrets
  #
  # By default, applications will be hashed
  # with the +Doorkeeper::SecretStoring::SHA256+ strategy.
  #
  # If you wish to use bcrypt for application secret hashing, uncomment
  # this line instead:
  #
  # hash_application_secrets using: '::Doorkeeper::SecretStoring::BCrypt'

  # Issue access tokens with refresh token (disabled by default), you may also
  # pass a block which accepts `context` to customize when to give a refresh
  # token or not. Similar to +custom_access_token_expires_in+, `context` has
  # the following properties:
  #
  # `client` - the OAuth client application (see Doorkeeper::OAuth::Client)
  # `grant_type` - the grant type of the request (see Doorkeeper::OAuth)
  # `scopes` - the requested scopes (see Doorkeeper::OAuth::Scopes)
  #
  use_refresh_token

  # Define access token scopes for your provider
  # For more information go to
  # https://doorkeeper.gitbook.io/guides/ruby-on-rails/scopes
  #
  # default_scopes  :public
  # optional_scopes :write, :update

  force_ssl_in_redirect_uri { |uri| uri.host != "localhost" }
end

Ruby version: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-darwin20]

@notfelineit
Copy link
Author

I think the default scopes get created at/after this line:

the strategy Doorkeeper::Request::Code instance still has a pre_auth whose scopes are correct write_organization.:

[93] pry(#<Api::Internal::OauthAuthorizationsController>)> strategy.pre_auth.scopes
=> #<Doorkeeper::OAuth::Scopes:0x0000000140f0c9e0 @scopes=["write_organization"]>

After authorize, the pre_auth with the incorrect scopes is generated:

[95] pry(#<Api::Internal::OauthAuthorizationsController>)> strategy.authorize.pre_auth.scopes
=> #<Doorkeeper::OAuth::Scopes:0x00000001414ae128 @scopes=["read_databases", "read_user", "read_organization"]>

@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.

@stale stale bot added the wontfix label Sep 21, 2022
@nbulaj nbulaj added the pinned For issues that can be stale label Sep 22, 2022
@stale stale bot removed the wontfix label Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned For issues that can be stale
Projects
None yet
Development

No branches or pull requests

2 participants