Merge pull request #1033 from doorkeeper-gem/force_ssl_redirect_uri_callable

Allow #force_ssl_in_redirect_uri to be callable

Author: nbulaj

NEWS.md

@@ -4,6 +4,7 @@ User-visible changes worth mentioning.
 
 ## master
 
+- [#1033] Allow Doorkeeper configuration option #force_ssl_in_redirect_uri to be a callable object.
 - Fix Grape integration & add specs for it
 - [#913] Deferred ORM (ActiveRecord) models loading
 - [#943] Fix Access Token token generation when certain errors occur in custom token generators

app/validators/redirect_uri_validator.rb

@@ -11,7 +11,7 @@ def validate_each(record, attribute, value)
     else
       value.split.each do |val|
         uri = ::URI.parse(val)
-        return if native_redirect_uri?(uri)
+        return false if native_redirect_uri?(uri)
         record.errors.add(attribute, :fragment_present) unless uri.fragment.nil?
         record.errors.add(attribute, :relative_uri) if uri.scheme.nil? || uri.host.nil?
         record.errors.add(attribute, :secured_uri) if invalid_ssl_uri?(uri)
@@ -29,6 +29,11 @@ def native_redirect_uri?(uri)
 
   def invalid_ssl_uri?(uri)
     forces_ssl = Doorkeeper.configuration.force_ssl_in_redirect_uri
-    forces_ssl && uri.try(:scheme) == 'http'
+
+    if forces_ssl.respond_to?(:call)
+      forces_ssl.call(uri)
+    else
+      forces_ssl && uri.try(:scheme) == 'http'
+    end
   end
 end

lib/doorkeeper/rails/helpers.rb

@@ -28,7 +28,8 @@ def doorkeeper_render_error
       def doorkeeper_render_error_with(error)
         options = doorkeeper_render_options(error) || {}
         status = doorkeeper_status_for_error(
-          error, options.delete(:respond_not_found_when_forbidden))
+          error, options.delete(:respond_not_found_when_forbidden)
+        )
         if options.blank?
           head status
         else

lib/generators/doorkeeper/templates/initializer.rb

@@ -82,7 +82,13 @@
   # by default in non-development environments). OAuth2 delegates security in
   # communication to the HTTPS protocol so it is wise to keep this enabled.
   #
+  # Callable objects such as proc, lambda, block or any object that responds to
+  # #call can be used in order to allow conditional checks (to allow non-SSL
+  # redirects to localhost for example).
+  #
   # force_ssl_in_redirect_uri !Rails.env.development?
+  #
+  # force_ssl_in_redirect_uri { |uri| uri.host != 'localhost' }
 
   # Specify what grant flows are enabled in array of Strings. The valid
   # strings and the flows they enable are:

spec/lib/config_spec.rb

@@ -10,6 +10,7 @@
         orm DOORKEEPER_ORM
         resource_owner_authenticator(&block)
       end
+
       expect(subject.authenticate_resource_owner).to eq(block)
     end
   end
@@ -37,6 +38,7 @@
         orm DOORKEEPER_ORM
         admin_authenticator(&block)
       end
+
       expect(subject.authenticate_admin).to eq(block)
     end
   end
@@ -59,6 +61,7 @@
         orm DOORKEEPER_ORM
         access_token_expires_in nil
       end
+
       expect(subject.access_token_expires_in).to be_nil
     end
   end
@@ -69,6 +72,7 @@
         orm DOORKEEPER_ORM
         default_scopes :public
       end
+
       expect(subject.default_scopes).to include('public')
     end
 
@@ -77,6 +81,7 @@
         orm DOORKEEPER_ORM
         optional_scopes :write, :update
       end
+
       expect(subject.optional_scopes).to include('write', 'update')
     end
 
@@ -86,6 +91,7 @@
         default_scopes  :normal
         optional_scopes :admin
       end
+
       expect(subject.scopes).to include('normal', 'admin')
     end
   end
@@ -100,6 +106,7 @@
         orm DOORKEEPER_ORM
         use_refresh_token
       end
+
       expect(subject.refresh_token_enabled?).to be_truthy
     end
 
@@ -131,6 +138,7 @@
         orm DOORKEEPER_ORM
         client_credentials :from_digest, :from_params
       end
+
       expect(subject.client_credentials_methods).to eq([:from_digest, :from_params])
     end
   end
@@ -145,8 +153,20 @@
         orm DOORKEEPER_ORM
         force_ssl_in_redirect_uri(false)
       end
+
       expect(subject.force_ssl_in_redirect_uri).to be_falsey
     end
+
+    it 'can be a callable object' do
+      block = proc { false }
+      Doorkeeper.configure do
+        orm DOORKEEPER_ORM
+        force_ssl_in_redirect_uri(&block)
+      end
+
+      expect(subject.force_ssl_in_redirect_uri).to eq(block)
+      expect(subject.force_ssl_in_redirect_uri.call).to be_falsey
+    end
   end
 
   describe 'access_token_methods' do
@@ -159,6 +179,7 @@
         orm DOORKEEPER_ORM
         access_token_methods :from_access_token_param, :from_bearer_param
       end
+
       expect(subject.access_token_methods).to eq([:from_access_token_param, :from_bearer_param])
     end
   end
@@ -175,9 +196,11 @@
           enable_application_owner
         end
       end
+
       it 'adds support for application owner' do
         expect(Doorkeeper::Application.new).to respond_to :owner
       end
+
       it 'Doorkeeper.configuration.confirm_application_owner? returns false' do
         expect(Doorkeeper.configuration.confirm_application_owner?).not_to be_truthy
       end
@@ -190,9 +213,11 @@
           enable_application_owner confirmation: true
         end
       end
+
       it 'adds support for application owner' do
         expect(Doorkeeper::Application.new).to respond_to :owner
       end
+
       it 'Doorkeeper.configuration.confirm_application_owner? returns true' do
         expect(Doorkeeper.configuration.confirm_application_owner?).to be_truthy
       end
@@ -209,6 +234,7 @@
         orm DOORKEEPER_ORM
         realm 'Example'
       end
+
       expect(subject.realm).to eq('Example')
     end
   end
@@ -220,19 +246,20 @@
     end
 
     it "can change the value" do
-      Doorkeeper.configure {
+      Doorkeeper.configure do
         orm DOORKEEPER_ORM
         grant_flows ['authorization_code', 'implicit']
-      }
+      end
+
       expect(subject.grant_flows).to eq ['authorization_code', 'implicit']
     end
 
     context "when including 'authorization_code'" do
       before do
-        Doorkeeper.configure {
+        Doorkeeper.configure do
           orm DOORKEEPER_ORM
           grant_flows ['authorization_code']
-        }
+        end
       end
 
       it "includes 'code' in authorization_response_types" do
@@ -246,10 +273,10 @@
 
     context "when including 'implicit'" do
       before do
-        Doorkeeper.configure {
+        Doorkeeper.configure do
           orm DOORKEEPER_ORM
           grant_flows ['implicit']
-        }
+        end
       end
 
       it "includes 'token' in authorization_response_types" do
@@ -259,10 +286,10 @@
 
     context "when including 'password'" do
       before do
-        Doorkeeper.configure {
+        Doorkeeper.configure do
           orm DOORKEEPER_ORM
           grant_flows ['password']
-        }
+        end
       end
 
       it "includes 'password' in token_grant_types" do
@@ -272,10 +299,10 @@
 
     context "when including 'client_credentials'" do
       before do
-        Doorkeeper.configure {
+        Doorkeeper.configure do
           orm DOORKEEPER_ORM
           grant_flows ['client_credentials']
-        }
+        end
       end
 
       it "includes 'client_credentials' in token_grant_types" do

spec/validators/redirect_uri_validator_spec.rb

@@ -68,6 +68,19 @@
       expect(subject).to be_valid
     end
 
+    it 'accepts a non secured protocol when conditional option defined' do
+      Doorkeeper.configure do
+        orm DOORKEEPER_ORM
+        force_ssl_in_redirect_uri { |uri| uri.host != 'localhost' }
+      end
+
+      application = FactoryBot.build(:application, redirect_uri: 'http://localhost/callback')
+      expect(application).to be_valid
+
+      application = FactoryBot.build(:application, redirect_uri: 'http://localhost2/callback')
+      expect(application).not_to be_valid
+    end
+
     it 'invalidates the uri when the uri does not use a secure protocol' do
       subject.redirect_uri = 'http://example.com/callback'
       expect(subject).not_to be_valid