Refreshing a token sending scopes separated by + does not work

[danilokleber]

Steps to reproduce

Try to refresh a token sending the scope field with strings separated by +. The users of the API just tried to use the same format they used on issuing a token (authorization_code grant).

Expected behavior

It refreshes the access token like when sending scopes separated by space.

Actual behavior

It returns a 401 with:

{
  "error": "invalid_scope",
  "error_description": "O escopo requisitado é inválido, desconhecido ou malformado."
}

System configuration

Doorkeeper initializer:

# config/initializers/doorkeeper.rb
Doorkeeper.configure do
  optional_scopes SCOPES

  native_redirect_uri URIS 

  use_refresh_token

  grant_flows %w(authorization_code)

  skip_authorization do |_, client|
    client.scopes.include?("mobile")
  end
end

# just injects some stuff in the response
Doorkeeper::OAuth::TokenResponse.send :prepend, Doorkeeper::CustomTokenResponse

Ruby version: 2.6.10

Gemfile.lock:

Gemfile.lock content

GEM
  remote: http://rubygems.org/
  specs:
    actioncable (5.2.8.1)
      actionpack (= 5.2.8.1)
      nio4r (~> 2.0)
      websocket-driver (>= 0.6.1)
    actionmailer (5.2.8.1)
      actionpack (= 5.2.8.1)
      actionview (= 5.2.8.1)
      activejob (= 5.2.8.1)
      mail (~> 2.5, >= 2.5.4)
      rails-dom-testing (~> 2.0)
    actionpack (5.2.8.1)
      actionview (= 5.2.8.1)
      activesupport (= 5.2.8.1)
      rack (~> 2.0, >= 2.0.8)
      rack-test (>= 0.6.3)
      rails-dom-testing (~> 2.0)
      rails-html-sanitizer (~> 1.0, >= 1.0.2)
    actionview (5.2.8.1)
      activesupport (= 5.2.8.1)
      builder (~> 3.1)
      erubi (~> 1.4)
      rails-dom-testing (~> 2.0)
      rails-html-sanitizer (~> 1.0, >= 1.0.3)
    active_model_serializers (0.9.8)
      activemodel (>= 3.2)
      concurrent-ruby (~> 1.0)
    activejob (5.2.8.1)
      activesupport (= 5.2.8.1)
      globalid (>= 0.3.6)
    activemodel (5.2.8.1)
      activesupport (= 5.2.8.1)
    activerecord (5.2.8.1)
      activemodel (= 5.2.8.1)
      activesupport (= 5.2.8.1)
      arel (>= 9.0)
    activerecord-session_store (1.1.3)
      actionpack (>= 4.0)
      activerecord (>= 4.0)
      multi_json (~> 1.11, >= 1.11.2)
      rack (>= 1.5.2, < 3)
      railties (>= 4.0)
    activestorage (5.2.8.1)
      actionpack (= 5.2.8.1)
      activerecord (= 5.2.8.1)
      marcel (~> 1.0.0)
    activesupport (5.2.8.1)
      concurrent-ruby (~> 1.0, >= 1.0.2)
      i18n (>= 0.7, < 2)
      minitest (~> 5.1)
      tzinfo (~> 1.1)
    addressable (2.8.1)
      public_suffix (>= 2.0.2, < 6.0)
    appsignal (3.2.1)
      rack
    arel (9.0.0)
    audited (5.0.2)
      activerecord (>= 5.0, < 7.1)
    autoprefixer-rails (8.6.5)
      execjs
    aws-eventstream (1.1.1)
    aws-partitions (1.681.0)
    aws-sdk-core (3.185.1)
      aws-eventstream (~> 1, >= 1.0.2)
      aws-partitions (~> 1, >= 1.651.0)
      aws-sigv4 (~> 1.5)
      jmespath (~> 1, >= 1.6.1)
    aws-sdk-kms (1.58.0)
      aws-sdk-core (~> 3, >= 3.127.0)
      aws-sigv4 (~> 1.1)
    aws-sdk-lambda (1.106.0)
      aws-sdk-core (~> 3, >= 3.184.0)
      aws-sigv4 (~> 1.1)
    aws-sdk-quicksight (1.94.0)
      aws-sdk-core (~> 3, >= 3.184.0)
      aws-sigv4 (~> 1.1)
    aws-sdk-s3 (1.136.0)
      aws-sdk-core (~> 3, >= 3.181.0)
      aws-sdk-kms (~> 1)
      aws-sigv4 (~> 1.6)
    aws-sdk-sqs (1.64.0)
      aws-sdk-core (~> 3, >= 3.184.0)
      aws-sigv4 (~> 1.1)
    aws-sigv4 (1.6.1)
      aws-eventstream (~> 1, >= 1.0.2)
    bcrypt (3.1.18)
    better_errors (2.9.1)
      coderay (>= 1.0.0)
      erubi (>= 1.0.0)
      rack (>= 0.9.0)
    bourbon (3.2.4)
      sass (~> 3.2)
      thor
    breadcrumbs (0.2.0)
      actionview
      activesupport
      i18n
    browser (5.3.1)
    builder (3.2.4)
    byebug (11.1.3)
    cancancan (2.3.0)
    carrierwave (2.2.3)
      activemodel (>= 5.0.0)
      activesupport (>= 5.0.0)
      addressable (~> 2.6)
      image_processing (~> 1.1)
      marcel (~> 1.0.0)
      mini_mime (>= 0.1.3)
      ssrf_filter (~> 1.0)
    cells (4.1.7)
      declarative-builder (< 0.2.0)
      declarative-option (< 0.2.0)
      tilt (>= 1.4, < 3)
      uber (< 0.2.0)
    cells-erb (0.1.0)
      cells (~> 4.0)
      erbse (>= 0.1.1)
    cells-rails (0.1.5)
      actionpack (>= 5.0)
      cells (>= 4.1.6, < 5.0.0)
    clockwork (3.0.1)
      activesupport
      tzinfo
    clockwork-test (0.4.0)
      clockwork
      timecop
    coderay (1.1.3)
    coffee-rails (5.0.0)
      coffee-script (>= 2.2.0)
      railties (>= 5.2.0)
    coffee-script (2.4.1)
      coffee-script-source
      execjs
    coffee-script-source (1.12.2)
    concurrent-ruby (1.1.10)
    cpf_cnpj (0.5.0)
    crack (0.4.5)
      rexml
    crass (1.0.6)
    css_parser (1.12.0)
      addressable
    date (3.3.3)
    declarative-builder (0.1.0)
      declarative-option (< 0.2.0)
    declarative-option (0.1.0)
    devise (4.6.2)
      bcrypt (~> 3.0)
      orm_adapter (~> 0.1)
      railties (>= 4.1.0, < 6.0)
      responders
      warden (~> 1.2.3)
    devise_invitable (1.7.5)
      actionmailer (>= 4.1.0)
      devise (>= 4.0.0)
    diff-lcs (1.5.0)
    docile (1.4.0)
    domain_name (0.5.20190701)
      unf (>= 0.0.5, < 1.0.0)
    doorkeeper (4.2.6)
      railties (>= 4.2)
    dotenv (2.8.1)
    dotenv-rails (2.8.1)
      dotenv (= 2.8.1)
      railties (>= 3.2)
    erbse (0.1.4)
      temple
    erubi (1.11.0)
    ethon (0.15.0)
      ffi (>= 1.15.0)
    excon (0.95.0)
    execjs (2.8.1)
    factory_bot (4.11.1)
      activesupport (>= 3.0.0)
    faker (2.22.0)
      i18n (>= 1.8.11, < 2)
    ffi (1.15.5)
    flipper (0.26.0)
      concurrent-ruby (< 2)
    flipper-active_record (0.26.0)
      activerecord (>= 4.2, < 8)
      flipper (~> 0.26.0)
    flipper-api (0.26.0)
      flipper (~> 0.26.0)
      rack (>= 1.4, < 3)
    flipper-ui (0.26.0)
      erubi (>= 1.0.0, < 2.0.0)
      flipper (~> 0.26.0)
      rack (>= 1.4, < 3)
      rack-protection (>= 1.5.3, <= 4.0.0)
      sanitize (< 7)
    fog-aws (3.15.0)
      fog-core (~> 2.1)
      fog-json (~> 1.1)
      fog-xml (~> 0.1)
    fog-core (2.3.0)
      builder
      excon (~> 0.71)
      formatador (>= 0.2, < 2.0)
      mime-types
    fog-json (1.2.0)
      fog-core
      multi_json (~> 1.10)
    fog-xml (0.1.4)
      fog-core
      nokogiri (>= 1.5.11, < 2.0.0)
    formatador (1.1.0)
    globalid (1.0.0)
      activesupport (>= 5.0)
    handlebars_assets (0.23.2)
      execjs (~> 2.0)
      sprockets (>= 2.0.0)
      tilt (>= 1.2)
    hashdiff (1.0.1)
    htmlentities (4.3.4)
    http-cookie (1.0.4)
      domain_name (~> 0.5)
    httpclient (2.8.3)
    i18n (1.12.0)
      concurrent-ruby (~> 1.0)
    image_processing (1.12.2)
      mini_magick (>= 4.9.5, < 5)
      ruby-vips (>= 2.0.17, < 3)
    jbuilder (2.11.5)
      actionview (>= 5.0.0)
      activesupport (>= 5.0.0)
    jmespath (1.6.2)
    jquery-rails (4.5.1)
      rails-dom-testing (>= 1, < 3)
      railties (>= 4.2.0)
      thor (>= 0.14, < 2.0)
    jwt (2.6.0)
    keen (1.1.1)
      addressable (~> 2.5)
      multi_json (~> 1.12)
    loofah (2.19.1)
      crass (~> 1.0.2)
      nokogiri (>= 1.5.9)
    mail (2.8.0)
      mini_mime (>= 0.1.1)
      net-imap
      net-pop
      net-smtp
    marcel (1.0.2)
    method_source (1.0.0)
    mime-types (3.3.1)
      mime-types-data (~> 3.2015)
    mime-types-data (3.2021.0901)
    mini_magick (4.12.0)
    mini_mime (1.1.2)
    mini_portile2 (2.8.1)
    minitest (5.16.3)
    monetize (1.12.0)
      money (~> 6.12)
    money (6.16.0)
      i18n (>= 0.6.4, <= 2)
    multi_json (1.15.0)
    net-imap (0.3.3)
      date
      net-protocol
    net-pop (0.1.2)
      net-protocol
    net-protocol (0.2.1)
      timeout
    net-smtp (0.3.3)
      net-protocol
    netrc (0.11.0)
    nio4r (2.5.8)
    nokogiri (1.13.10)
      mini_portile2 (~> 2.8.0)
      racc (~> 1.4)
    nori (2.6.0)
    orm_adapter (0.5.0)
    pagy (6.0.4)
    paper_trail (13.0.0)
      activerecord (>= 5.2)
      request_store (~> 1.1)
    paper_trail-association_tracking (2.2.1)
      paper_trail (>= 12.0)
    pg (1.4.5)
    pg_search (2.1.7)
      activerecord (>= 4.2)
      activesupport (>= 4.2)
    premailer (1.15.0)
      addressable
      css_parser (>= 1.6.0)
      htmlentities (>= 4.0.0)
    premailer-rails (1.12.0)
      actionmailer (>= 3)
      net-smtp
      premailer (~> 1.7, >= 1.7.9)
    pry (0.13.1)
      coderay (~> 1.1)
      method_source (~> 1.0)
    pry-byebug (3.9.0)
      byebug (~> 11.0)
      pry (~> 0.13.0)
    pry-rails (0.3.9)
      pry (>= 0.10.4)
    public_suffix (5.0.1)
    puma (4.3.12)
      nio4r (~> 2.0)
    pusher (2.0.3)
      httpclient (~> 2.8)
      multi_json (~> 1.15)
      pusher-signature (~> 0.1.8)
    pusher-signature (0.1.8)
    racc (1.6.2)
    rack (2.2.4)
    rack-cors (1.1.1)
      rack (>= 2.0.0)
    rack-protection (2.2.2)
      rack
    rack-test (2.0.2)
      rack (>= 1.3)
    rails (5.2.8.1)
      actioncable (= 5.2.8.1)
      actionmailer (= 5.2.8.1)
      actionpack (= 5.2.8.1)
      actionview (= 5.2.8.1)
      activejob (= 5.2.8.1)
      activemodel (= 5.2.8.1)
      activerecord (= 5.2.8.1)
      activestorage (= 5.2.8.1)
      activesupport (= 5.2.8.1)
      bundler (>= 1.3.0)
      railties (= 5.2.8.1)
      sprockets-rails (>= 2.0.0)
    rails-controller-testing (1.0.5)
      actionpack (>= 5.0.1.rc1)
      actionview (>= 5.0.1.rc1)
      activesupport (>= 5.0.1.rc1)
    rails-dom-testing (2.0.3)
      activesupport (>= 4.2.0)
      nokogiri (>= 1.6)
    rails-html-sanitizer (1.4.4)
      loofah (~> 2.19, >= 2.19.1)
    railties (5.2.8.1)
      actionpack (= 5.2.8.1)
      activesupport (= 5.2.8.1)
      method_source
      rake (>= 0.8.7)
      thor (>= 0.19.0, < 2.0)
    rake (13.0.6)
    ransack (2.1.1)
      actionpack (>= 5.0)
      activerecord (>= 5.0)
      activesupport (>= 5.0)
      i18n
    request_store (1.5.1)
      rack (>= 1.4)
    responders (3.0.1)
      actionpack (>= 5.0)
      railties (>= 5.0)
    rest-client (2.0.2)
      http-cookie (>= 1.0.2, < 2.0)
      mime-types (>= 1.16, < 4.0)
      netrc (~> 0.8)
    rexml (3.2.5)
    rmagick (3.2.0)
    rspec-core (3.12.0)
      rspec-support (~> 3.12.0)
    rspec-expectations (3.12.1)
      diff-lcs (>= 1.2.0, < 2.0)
      rspec-support (~> 3.12.0)
    rspec-mocks (3.12.1)
      diff-lcs (>= 1.2.0, < 2.0)
      rspec-support (~> 3.12.0)
    rspec-rails (5.1.2)
      actionpack (>= 5.2)
      activesupport (>= 5.2)
      railties (>= 5.2)
      rspec-core (~> 3.10)
      rspec-expectations (~> 3.10)
      rspec-mocks (~> 3.10)
      rspec-support (~> 3.10)
    rspec-support (3.12.0)
    ruby-vips (2.1.4)
      ffi (~> 1.12)
    sanitize (6.0.0)
      crass (~> 1.0.2)
      nokogiri (>= 1.12.0)
    sass (3.2.19)
    sass-rails (6.0.0)
      sassc-rails (~> 2.1, >= 2.1.1)
    sassc (2.4.0)
      ffi (~> 1.9)
    sassc-rails (2.1.2)
      railties (>= 4.0.0)
      sassc (>= 2.0)
      sprockets (> 3.0)
      sprockets-rails
      tilt
    shoryuken (5.3.2)
      aws-sdk-core (>= 2)
      concurrent-ruby
      thor
    shoulda-matchers (4.5.1)
      activesupport (>= 4.2.0)
    simple_form (5.1.0)
      actionpack (>= 5.2)
      activemodel (>= 5.2)
    simplecov (0.21.2)
      docile (~> 1.1)
      simplecov-html (~> 0.11)
      simplecov_json_formatter (~> 0.1)
    simplecov-html (0.12.3)
    simplecov_json_formatter (0.1.4)
    smart_assets (0.5.0)
      rails (>= 4.0, < 7.1)
      sprockets-rails (>= 2, < 4)
    sprockets (3.7.2)
      concurrent-ruby (~> 1.0)
      rack (> 1, < 3)
    sprockets-rails (3.4.2)
      actionpack (>= 5.2)
      activesupport (>= 5.2)
      sprockets (>= 3.0.0)
    ssrf_filter (1.1.1)
    state_machines (0.5.0)
    state_machines-activemodel (0.8.0)
      activemodel (>= 5.1)
      state_machines (>= 0.5.0)
    state_machines-activerecord (0.8.0)
      activerecord (>= 5.1)
      state_machines-activemodel (>= 0.8.0)
    temple (0.9.1)
    thor (1.2.1)
    thread_safe (0.3.6)
    tilt (2.0.11)
    timecop (0.9.6)
    timeliness (0.4.5)
    timeout (0.3.1)
    turbolinks (2.5.4)
      coffee-rails
    typhoeus (1.4.0)
      ethon (>= 0.9.0)
    tzinfo (1.2.10)
      thread_safe (~> 0.1)
    uber (0.1.0)
    uglifier (4.2.0)
      execjs (>= 0.3.0, < 3)
    unf (0.1.4)
      unf_ext
    unf_ext (0.0.7.7)
    validates_timeliness (5.0.1)
      timeliness (>= 0.3.10, < 1)
    warden (1.2.9)
      rack (>= 2.0.9)
    webmock (3.18.1)
      addressable (>= 2.8.0)
      crack (>= 0.3.2)
      hashdiff (>= 0.4.0, < 2.0.0)
    websocket-driver (0.7.5)
      websocket-extensions (>= 0.1.0)
    websocket-extensions (0.1.5)
    wicked_pdf (2.6.3)
      activesupport
    wkhtmltopdf-binary (0.12.3.1)
    xml-simple (1.1.9)
      rexml

PLATFORMS
  ruby

DEPENDENCIES
  active_model_serializers (~> 0.9.0)
  activerecord-session_store (~> 1.1.3)
  appsignal
  audited (~> 5)
  autoprefixer-rails (~> 8.6)
  aws-sdk-lambda (~> 1)
  aws-sdk-quicksight (= 1.94.0)
  aws-sdk-s3 (~> 1)
  aws-sdk-sqs (~> 1)
  better_errors
  bourbon
  breadcrumbs
  browser
  cancancan (~> 2)
  carrierwave (~> 2.2.2)
  cells-erb
  cells-rails
  clockwork
  clockwork-test
  cpf_cnpj
  devise (= 4.6.2)
  devise_invitable (= 1.7.5)
  doorkeeper (~> 4.2.0)
  dotenv-rails
  factory_bot (~> 4)
  faker
  flipper
  flipper-active_record
  flipper-api
  flipper-ui
  fog-aws (~> 3.15.0)
  handlebars_assets
  jbuilder
  jquery-rails (= 4.5.1)
  jwt (~> 2.6)
  keen
  monetize
  money
  nori
  pagy
  paper_trail (~> 13)
  paper_trail-association_tracking
  pg (~> 1)
  pg_search (~> 2.1.2)
  premailer-rails
  pry-byebug
  pry-rails
  puma (~> 4)
  pusher
  rack-cors
  rails (= 5.2.8.1)
  rails-controller-testing
  ransack (~> 2.1.1)
  rest-client (= 2.0.2)
  rmagick (~> 3.2.0)
  rspec-rails (~> 5)
  sass-rails (= 6.0.0)
  shoryuken (~> 5)
  shoulda-matchers (~> 4)
  simple_form (~> 5.1)
  simplecov
  smart_assets
  state_machines-activerecord (~> 0.8.0)
  timecop
  turbolinks (~> 2.5, >= 2.5.4)
  typhoeus (= 1.4.0)
  uglifier
  validates_timeliness (~> 5.0.1)
  webmock
  wicked_pdf
  wkhtmltopdf-binary
  xml-simple

RUBY VERSION
   ruby 2.6.10p210

BUNDLED WITH
   2.3.26

[ransombriggs]

I am not a maintainer, but I do not think this would be possible to support since + is an allowed character in a scopes according to the rfc. So if a server had scopes foo, bar and foo+bar and someone requested foo+bar in the way you are proposing, should that be interpreted as requesting [foo, bar] or [foo+bar] ?

[danilokleber]

Fair enough. I think the issue here is inconsistency? I don’t have a + interpolated scope available.

[luke-zhou]

Fair enough. I think the issue here is inconsistency? I don’t have a + interpolated scope available.

Does that only happen in the authorization_code grant? I have done the following test in password grant:

• server had scopes [foo, bar]
• client requests token with scope "foo" is successful
• client requests token with scope "bar" is successful
• client requests token with scope "foo+bar" is unsuccessful

[danilokleber]

Does that only happen in the authorization_code grant?

Interesting question. I can’t test other grants right now but yours is helpful enough.

[ransombriggs]

I do not think this is a matter of consistency, when the client does the redirect they are url-encoding spaces to +, if there were a plus in the scopes, they would url-encode them to %2B. When the web server reads the query parameters it converts + to spaces, which is the correct behavior.