Merge 8a54320 into f1efa3b

Gemfile

@@ -7,6 +7,6 @@ gem "appraisal"
 gem "bcrypt", "~> 3.1", require: false
 
 gem "activerecord-jdbcsqlite3-adapter", platform: :jruby
-gem "sqlite3", platform: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw]
 gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw]
 gemspec

NEWS.md

@@ -13,6 +13,16 @@ User-visible changes worth mentioning.
   their block implementation: if you are using `oauth_client.application` to get `Doorkeeper::Application`
   instance, then you need to replace it with just `oauth_client`.
 
+- [#1200] Increase default Doorkeeper access token value complexity (`urlsafe_base64` instead of just `hex`)
+  matching RFC6749/RFC6750.
+
+  **[IMPORTANT]**: this change have possible side-effects in case you have custom database constraints for
+  access token value, application secrets, refresh tokens or you patched Doorkeeper models and introduced
+  token value validations, or you are using database with case-insensitive WHERE clause like MySQL
+  (you can face some collisions). Before this change access token value matched `[a-f0-9]` regex, and now
+  it matches `[a-zA-Z0-9\-_]`. In case you have such restrictions and your don't use custom token generator
+  please change configuration option `default_generator_method ` to `:hex`.
+
 - [#1195] Allow to customize Token Introspection response (fixes #1194).
 - [#1189] Option to set `token_reuse_limit`.
 - [#1191] Try to load bcrypt for hashing of application secrets, but add fallback.

gemfiles/rails_4_2.gemfile

@@ -6,7 +6,7 @@ gem "rails", "~> 4.2.0"
 gem "bcrypt", "~> 3.1", require: false
 gem "appraisal"
 gem "activerecord-jdbcsqlite3-adapter", platform: :jruby
-gem "sqlite3", platform: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw]
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw]
 # Older Grape requires Ruby >= 2.2.2
 gem "grape", '~> 0.16', '< 0.19.2'

gemfiles/rails_5_0.gemfile

@@ -6,7 +6,7 @@ gem "rails", "~> 5.0.0"
 gem "bcrypt", "~> 3.1", require: false
 gem "appraisal"
 gem "activerecord-jdbcsqlite3-adapter", platforms: :jruby
-gem "sqlite3", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw]
 gem "rspec-rails", "~> 3.5"
 

gemfiles/rails_5_1.gemfile

@@ -6,7 +6,7 @@ gem "rails", "~> 5.1.0"
 gem "bcrypt", "~> 3.1", require: false
 gem "appraisal"
 gem "activerecord-jdbcsqlite3-adapter", platforms: :jruby
-gem "sqlite3", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw]
 gem "rspec-rails", "~> 3.7"
 

gemfiles/rails_5_2.gemfile

@@ -6,7 +6,7 @@ gem "rails", "~> 5.2.0"
 gem "bcrypt", "~> 3.1", require: false
 gem "appraisal"
 gem "activerecord-jdbcsqlite3-adapter", platforms: :jruby
-gem "sqlite3", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platforms: [:ruby, :mswin, :mingw, :x64_mingw]
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw]
 gem "rspec-rails", "~> 3.7"
 

gemfiles/rails_master.gemfile

@@ -8,7 +8,7 @@ gem "arel", git: 'https://github.com/rails/arel'
 gem "appraisal"
 gem "bcrypt", "~> 3.1", require: false
 gem "activerecord-jdbcsqlite3-adapter", platform: :jruby
-gem "sqlite3", platform: [:ruby, :mswin, :mingw, :x64_mingw]
+gem "sqlite3", "~> 1.3", "< 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw]
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw]
 
 %w[rspec-core rspec-expectations rspec-mocks rspec-rails rspec-support].each do |lib|

lib/doorkeeper/config.rb

@@ -318,6 +318,15 @@ def option(name, options = {})
     option :access_token_generator,
            default: 'Doorkeeper::OAuth::Helpers::UniqueToken'
 
+
+    # Default access token generator is a SecureRandom class from Ruby stdlib.
+    # This option defines which method will be used to generate a unique token value.
+    #
+    # @param access_token_generator [String]
+    #   the name of the access token generator class
+    #
+    option :default_generator_method,  default: :urlsafe_base64
+
     # The controller Doorkeeper::ApplicationController inherits from.
     # Defaults to ActionController::Base.
     # https://github.com/doorkeeper-gem/doorkeeper#custom-base-controller

lib/doorkeeper/oauth/helpers/unique_token.rb

@@ -5,10 +5,21 @@ module OAuth
     module Helpers
       module UniqueToken
         def self.generate(options = {})
-          generator_method = options.delete(:generator) || SecureRandom.method(:hex)
+          # Access Token value must be 1*VSCHAR or 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" ) *"="
+          #
+          # @see https://tools.ietf.org/html/rfc6749#appendix-A.12
+          # @see https://tools.ietf.org/html/rfc6750#section-2.1
+          #
+          generator_method = options.delete(:generator) || SecureRandom.method(self.generator_method)
           token_size       = options.delete(:size)      || 32
           generator_method.call(token_size)
         end
+
+        # Generator method for default generator class (SecureRandom)
+        #
+        def self.generator_method
+          Doorkeeper.configuration.default_generator_method
+        end
       end
     end
   end

spec/controllers/authorizations_controller_spec.rb

@@ -91,7 +91,7 @@ def translated_error_message(key)
     end
 
     it "includes access token in fragment" do
-      expect(redirect_uri.match(/access_token=([a-f0-9]+)&?/)[1]).to eq(Doorkeeper::AccessToken.first.token)
+      expect(redirect_uri.match(/access_token=([a-zA-Z0-9\-_]+)&?/)[1]).to eq(Doorkeeper::AccessToken.first.token)
     end
 
     it "includes token type in fragment" do
@@ -408,7 +408,7 @@ def translated_error_message(key)
       expect(redirect_uri.match(/token_type=(\w+)&?/)[1]).to eq "Bearer"
       expect(redirect_uri.match(/expires_in=(\d+)&?/)[1].to_i).to eq 1234
       expect(
-        redirect_uri.match(/access_token=([a-f0-9]+)&?/)[1]
+        redirect_uri.match(/access_token=([a-zA-Z0-9\-_]+)&?/)[1]
       ).to eq Doorkeeper::AccessToken.first.token
     end
 

spec/lib/config_spec.rb

@@ -482,6 +482,22 @@
     end
   end
 
+  describe 'default_generator_method' do
+    it "is :urlsafe_base64 by default" do
+      expect(Doorkeeper.configuration.default_generator_method)
+        .to eq(:urlsafe_base64)
+    end
+
+    it 'can change the value' do
+      Doorkeeper.configure do
+        orm DOORKEEPER_ORM
+        default_generator_method :hex
+      end
+
+      expect(subject.default_generator_method).to eq(:hex)
+    end
+  end
+
   describe 'base_controller' do
     context 'default' do
       it { expect(Doorkeeper.configuration.base_controller).to eq('ActionController::Base') }