diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e70247271..b2d277584 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,9 +9,9 @@ name: Ruby on: push: - branches: [ master ] + branches: [ master, 2_0-indev ] pull_request: - branches: [ master ] + branches: [ master, 2_0-indev ] jobs: test: @@ -54,6 +54,21 @@ jobs: env: JRUBY_OPTS: --debug run: bundle exec rake + frozen-string-compat: + runs-on: ubuntu-18.04 + steps: + - uses: actions/checkout@v2 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 2.6 + bundler-cache: true + - name: Install dependencies + run: bundle install + - name: Run tests + env: + RUBYOPT: "--enable-frozen-string-literal" + run: bundle exec rake coveralls: runs-on: ubuntu-18.04 steps: @@ -61,7 +76,7 @@ jobs: - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: 2.5 + ruby-version: 2.6 bundler-cache: true - name: Install dependencies run: bundle install diff --git a/Gemfile b/Gemfile index 7c990ebde..9f08f3a34 100644 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,8 @@ group :test do gem 'rack', '>= 2.0.6', :platforms => %i[jruby_18 jruby_19 ruby_19 ruby_20 ruby_21] gem 'rack-test' gem 'rest-client', '~> 2.0.0', :platforms => [:jruby_18] - gem 'rspec', '~> 3.5.0' + gem 'rspec', '~> 3.5' + gem 'rack-freeze' gem 'rubocop', '>= 0.58.2', '< 0.69.0', :platforms => %i[ruby_20 ruby_21 ruby_22 ruby_23 ruby_24] gem 'simplecov-lcov' gem 'tins', '~> 1.13', :platforms => %i[jruby_18 jruby_19 ruby_19] diff --git a/README.md b/README.md index b4b4c2311..4eaf661f0 100644 --- a/README.md +++ b/README.md @@ -2,9 +2,8 @@ [![Gem Version](http://img.shields.io/gem/v/omniauth.svg)][gem] [![Build Status](http://img.shields.io/travis/omniauth/omniauth.svg)][travis] -[![Code Climate](http://img.shields.io/codeclimate/github/omniauth/omniauth.svg)][codeclimate] +[![Code Climate](https://api.codeclimate.com/v1/badges/ffd33970723587806744/maintainability)][codeclimate] [![Coverage Status](http://img.shields.io/coveralls/omniauth/omniauth.svg)][coveralls] -[![Security](https://hakiri.io/github/omniauth/omniauth/master.svg)](https://hakiri.io/github/omniauth/omniauth/master) [gem]: https://rubygems.org/gems/omniauth [travis]: http://travis-ci.org/omniauth/omniauth @@ -33,8 +32,8 @@ development and easily swap in other strategies later. ## Getting Started Each OmniAuth strategy is a Rack Middleware. That means that you can use it the same way that you use any other Rack middleware. For example, to -use the built-in Developer strategy in a Sinatra application I might do -this: +use the built-in Developer strategy in a Sinatra application you might +do this: ```ruby require 'sinatra' @@ -46,7 +45,7 @@ class MyApplication < Sinatra::Base end ``` -Because OmniAuth is built for *multi-provider* authentication, I may +Because OmniAuth is built for *multi-provider* authentication, you may want to leave room to run multiple strategies. For this, the built-in `OmniAuth::Builder` class gives you an easy way to specify multiple strategies. Note that there is **no difference** between the following @@ -83,14 +82,14 @@ environment of a request to `/auth/:provider/callback`. This hash contains as much information about the user as OmniAuth was able to glean from the utilized strategy. You should set up an endpoint in your application that matches to the callback URL and then performs whatever -steps are necessary for your application. For example, in a Rails app I -would add a line in my `routes.rb` file like this: +steps are necessary for your application. For example, in a Rails app +you would add a line in your `routes.rb` file like this: ```ruby post '/auth/:provider/callback', to: 'sessions#create' ``` -And I might then have a `SessionsController` with code that looks +And you might then have a `SessionsController` with code that looks something like this: ```ruby @@ -112,7 +111,7 @@ class SessionsController < ApplicationController end ``` -The `omniauth.auth` key in the environment hash gives me my +The `omniauth.auth` key in the environment hash provides an Authentication Hash which will contain information about the just authenticated user including a unique id, the strategy they just used for authentication, and personal details such as name and email address @@ -167,7 +166,7 @@ a `session_store.rb` initializer, add `use ActionDispatch::Session::CookieStore` and have sessions functioning as normal. To be clear: sessions may work, but your session options will be ignored -(i.e the session key will default to `_session_id`). Instead of the +(i.e. the session key will default to `_session_id`). Instead of the initializer, you'll have to set the relevant options somewhere before your middleware is built (like `application.rb`) and pass them to your preferred middleware, like this: diff --git a/lib/omniauth.rb b/lib/omniauth.rb index dc315ee2c..034e7e8fe 100644 --- a/lib/omniauth.rb +++ b/lib/omniauth.rb @@ -15,6 +15,7 @@ module Strategies autoload :Form, 'omniauth/form' autoload :AuthHash, 'omniauth/auth_hash' autoload :FailureEndpoint, 'omniauth/failure_endpoint' + autoload :AuthenticityTokenProtection, 'omniauth/authenticity_token_protection' def self.strategies @strategies ||= [] @@ -29,20 +30,22 @@ def self.default_logger logger end - def self.defaults + def self.defaults # rubocop:disable MethodLength @defaults ||= { :camelizations => {}, :path_prefix => '/auth', :on_failure => OmniAuth::FailureEndpoint, :failure_raise_out_environments => ['development'], + :request_validation_phase => OmniAuth::AuthenticityTokenProtection, :before_request_phase => nil, :before_callback_phase => nil, :before_options_phase => nil, :form_css => Form::DEFAULT_CSS, :test_mode => false, :logger => default_logger, - :allowed_request_methods => %i[get post], - :mock_auth => {:default => AuthHash.new('provider' => 'default', 'uid' => '1234', 'info' => {'name' => 'Example User'})} + :allowed_request_methods => %i[post], + :mock_auth => {:default => AuthHash.new('provider' => 'default', 'uid' => '1234', 'info' => {'name' => 'Example User'})}, + :silence_get_warning => false } end @@ -74,6 +77,14 @@ def before_options_phase(&block) end end + def request_validation_phase(&block) + if block_given? + @request_validation_phase = block + else + @request_validation_phase + end + end + def before_request_phase(&block) if block_given? @before_request_phase = block @@ -111,8 +122,9 @@ def add_camelization(name, camelized) camelizations[name.to_s] = camelized.to_s end - attr_writer :on_failure, :before_callback_phase, :before_options_phase, :before_request_phase - attr_accessor :failure_raise_out_environments, :path_prefix, :allowed_request_methods, :form_css, :test_mode, :mock_auth, :full_host, :camelizations, :logger + attr_writer :on_failure, :before_callback_phase, :before_options_phase, :before_request_phase, :request_validation_phase + attr_accessor :failure_raise_out_environments, :path_prefix, :allowed_request_methods, :form_css, + :test_mode, :mock_auth, :full_host, :camelizations, :logger, :silence_get_warning end def self.config @@ -159,7 +171,7 @@ def camelize(word, first_letter_in_uppercase = true) if first_letter_in_uppercase word.to_s.gsub(%r{/(.?)}) { '::' + Regexp.last_match[1].upcase }.gsub(/(^|_)(.)/) { Regexp.last_match[2].upcase } else - word.first + camelize(word)[1..-1] + camelize(word).tap { |w| w[0] = w[0].downcase } end end end diff --git a/lib/omniauth/authenticity_token_protection.rb b/lib/omniauth/authenticity_token_protection.rb new file mode 100644 index 000000000..6dea46b93 --- /dev/null +++ b/lib/omniauth/authenticity_token_protection.rb @@ -0,0 +1,30 @@ +require 'rack-protection' + +module OmniAuth + class AuthenticityError < StandardError; end + class AuthenticityTokenProtection < Rack::Protection::AuthenticityToken + def initialize(options = {}) + @options = default_options.merge(options) + end + + def self.call(env) + new.call!(env) + end + + def call!(env) + return if accepts?(env) + + instrument env + react env + end + + private + + def deny(_env) + OmniAuth.logger.send(:warn, "Attack prevented by #{self.class}") + raise AuthenticityError.new(options[:message]) + end + + alias default_reaction deny + end +end diff --git a/lib/omniauth/builder.rb b/lib/omniauth/builder.rb index 717b4be8f..2882076cf 100644 --- a/lib/omniauth/builder.rb +++ b/lib/omniauth/builder.rb @@ -31,7 +31,7 @@ def provider(klass, *args, &block) middleware = klass else begin - middleware = OmniAuth::Strategies.const_get(OmniAuth::Utils.camelize(klass.to_s).to_s) + middleware = OmniAuth::Strategies.const_get(OmniAuth::Utils.camelize(klass.to_s).to_s, false) rescue NameError raise(LoadError.new("Could not find matching strategy for #{klass.inspect}. You may need to install an additional gem (such as omniauth-#{klass}).")) end diff --git a/lib/omniauth/failure_endpoint.rb b/lib/omniauth/failure_endpoint.rb index 7c39a347b..84de91e97 100644 --- a/lib/omniauth/failure_endpoint.rb +++ b/lib/omniauth/failure_endpoint.rb @@ -27,10 +27,19 @@ def raise_out! def redirect_to_failure message_key = env['omniauth.error.type'] - new_path = "#{env['SCRIPT_NAME']}#{OmniAuth.config.path_prefix}/failure?message=#{message_key}#{origin_query_param}#{strategy_name_query_param}" + + new_path = "#{env['SCRIPT_NAME']}#{strategy_path_prefix}/failure?message=#{Rack::Utils.escape(message_key)}#{origin_query_param}#{strategy_name_query_param}" Rack::Response.new(['302 Moved'], 302, 'Location' => new_path).finish end + def strategy_path_prefix + if env['omniauth.error.strategy'] + env['omniauth.error.strategy'].path_prefix + else + OmniAuth.config.path_prefix + end + end + def strategy_name_query_param return '' unless env['omniauth.error.strategy'] diff --git a/lib/omniauth/form.rb b/lib/omniauth/form.rb index abd0b4320..def52e3de 100644 --- a/lib/omniauth/form.rb +++ b/lib/omniauth/form.rb @@ -9,7 +9,7 @@ def initialize(options = {}) options[:header_info] ||= '' self.options = options - @html = '' + @html = +'' # unary + string allows it to be mutable if strings are frozen @with_custom_button = false @footer = nil header(options[:title], options[:header_info]) diff --git a/lib/omniauth/strategy.rb b/lib/omniauth/strategy.rb index ce8b34871..d8468bcbc 100644 --- a/lib/omniauth/strategy.rb +++ b/lib/omniauth/strategy.rb @@ -180,18 +180,44 @@ def call!(env) # rubocop:disable CyclomaticComplexity, PerceivedComplexity raise(error) end + warn_if_using_get + @env = env @env['omniauth.strategy'] = self if on_auth_path? return mock_call!(env) if OmniAuth.config.test_mode - return options_call if on_auth_path? && options_request? - return request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym) - return callback_call if on_callback_path? - return other_phase if respond_to?(:other_phase) + + begin + return options_call if on_auth_path? && options_request? + return request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym) + return callback_call if on_callback_path? + return other_phase if respond_to?(:other_phase) + rescue StandardError => e + return fail!(e.message, e) + end @app.call(env) end + def warn_if_using_get + return unless OmniAuth.config.allowed_request_methods.include?(:get) + return if OmniAuth.config.silence_get_warning + + log :warn, <<-WARN + You are using GET as an allowed request method for OmniAuth. This may leave + you open to CSRF attacks. As of v2.0.0, OmniAuth by default allows only POST + to its own routes. You should review the following resources to guide your + mitigation: + https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284 + https://github.com/omniauth/omniauth/issues/960 + https://nvd.nist.gov/vuln/detail/CVE-2015-9284 + https://github.com/omniauth/omniauth/pull/809 + + You can ignore this warning by setting: + OmniAuth.config.silence_get_warning = true + WARN + end + # Responds to an OPTIONS request. def options_call OmniAuth.config.before_options_phase.call(env) if OmniAuth.config.before_options_phase @@ -202,17 +228,19 @@ def options_call # Performs the steps necessary to run the request phase of a strategy. def request_call # rubocop:disable CyclomaticComplexity, MethodLength, PerceivedComplexity setup_phase - log :info, 'Request phase initiated.' + log :debug, 'Request phase initiated.' # store query params from the request url, extracted in the callback_phase session['omniauth.params'] = request.GET + + OmniAuth.config.request_validation_phase.call(env) if OmniAuth.config.request_validation_phase OmniAuth.config.before_request_phase.call(env) if OmniAuth.config.before_request_phase if options.form.respond_to?(:call) - log :info, 'Rendering form from supplied Rack endpoint.' + log :debug, 'Rendering form from supplied Rack endpoint.' options.form.call(env) elsif options.form - log :info, 'Rendering form from underlying application.' + log :debug, 'Rendering form from underlying application.' call_app! elsif !options.origin_param request_phase @@ -225,12 +253,14 @@ def request_call # rubocop:disable CyclomaticComplexity, MethodLength, Perceived request_phase end + rescue OmniAuth::AuthenticityError => e + fail!(:authenticity_error, e) end # Performs the steps necessary to run the callback phase of a strategy. def callback_call setup_phase - log :info, 'Callback phase initiated.' + log :debug, 'Callback phase initiated.' @env['omniauth.origin'] = session.delete('omniauth.origin') @env['omniauth.origin'] = nil if env['omniauth.origin'] == '' @env['omniauth.params'] = session.delete('omniauth.params') || {} @@ -268,8 +298,13 @@ def options_request? # in the event that OmniAuth has been configured to be # in test mode. def mock_call!(*) - return mock_request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym) - return mock_callback_call if on_callback_path? + begin + OmniAuth.config.request_validation_phase.call(env) if OmniAuth.config.request_validation_phase + return mock_request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym) + return mock_callback_call if on_callback_path? + rescue StandardError => e + return fail!(e.message, e) + end call_app! end @@ -312,10 +347,10 @@ def mock_callback_call # underlying application. This will default to `/auth/:provider/setup`. def setup_phase if options[:setup].respond_to?(:call) - log :info, 'Setup endpoint detected, running now.' + log :debug, 'Setup endpoint detected, running now.' options[:setup].call(env) elsif options[:setup] - log :info, 'Calling through to underlying application for setup.' + log :debug, 'Calling through to underlying application for setup.' setup_env = env.merge('PATH_INFO' => setup_path, 'REQUEST_METHOD' => 'GET') call_app!(setup_env) end @@ -345,11 +380,13 @@ def extra end def auth_hash - hash = AuthHash.new(:provider => name, :uid => uid) - hash.info = info unless skip_info? - hash.credentials = credentials if credentials - hash.extra = extra if extra - hash + credentials_data = credentials + extra_data = extra + AuthHash.new(:provider => name, :uid => uid).tap do |auth| + auth.info = info unless skip_info? + auth.credentials = credentials_data if credentials_data + auth.extra = extra_data if extra_data + end end # Determines whether or not user info should be retrieved. This @@ -389,7 +426,12 @@ def custom_path(kind) end def request_path - @request_path ||= options[:request_path].is_a?(String) ? options[:request_path] : "#{path_prefix}/#{name}" + @request_path ||= + if options[:request_path].is_a?(String) + options[:request_path] + else + "#{script_name}#{path_prefix}/#{name}" + end end def callback_path @@ -397,7 +439,7 @@ def callback_path path = options[:callback_path] if options[:callback_path].is_a?(String) path ||= current_path if options[:callback_path].respond_to?(:call) && options[:callback_path].call(env) path ||= custom_path(:request_path) - path ||= "#{path_prefix}/#{name}/callback" + path ||= "#{script_name}#{path_prefix}/#{name}/callback" path end end @@ -409,7 +451,7 @@ def setup_path CURRENT_PATH_REGEX = %r{/$}.freeze EMPTY_STRING = ''.freeze def current_path - @current_path ||= request.path_info.downcase.sub(CURRENT_PATH_REGEX, EMPTY_STRING) + @current_path ||= request.path.downcase.sub(CURRENT_PATH_REGEX, EMPTY_STRING) end def query_string @@ -441,7 +483,7 @@ def full_host end def callback_url - full_host + script_name + callback_path + query_string + full_host + callback_path + query_string end def script_name @@ -491,16 +533,15 @@ def fail!(message_key, exception = nil) OmniAuth.config.on_failure.call(env) end - def dup - super.tap do - @options = @options.dup - end - end - class Options < OmniAuth::KeyStore; end protected + def initialize_copy(*args) + super + @options = @options.dup + end + def merge_stack(stack) stack.inject({}) do |a, e| a.merge!(e) diff --git a/lib/omniauth/version.rb b/lib/omniauth/version.rb index ea777f2f1..957ed63f4 100644 --- a/lib/omniauth/version.rb +++ b/lib/omniauth/version.rb @@ -1,3 +1,3 @@ module OmniAuth - VERSION = '1.9.1'.freeze + VERSION = '2.0.0'.freeze end diff --git a/omniauth.gemspec b/omniauth.gemspec index cd2c25a07..dc0a90f2a 100644 --- a/omniauth.gemspec +++ b/omniauth.gemspec @@ -8,6 +8,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'hashie', ['>= 3.4.6'] spec.add_dependency 'rack', ['>= 1.6.2', '< 3'] spec.add_development_dependency 'bundler', '~> 2.0' + spec.add_dependency 'rack-protection' spec.add_development_dependency 'rake', '~> 12.0' spec.authors = ['Michael Bleigh', 'Erik Michaels-Ober', 'Tom Milewski'] spec.description = 'A generalized Rack framework for multiple-provider authentication.' diff --git a/spec/helper.rb b/spec/helper.rb index 97b72bd14..65839922d 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -20,10 +20,12 @@ require 'rspec' require 'rack/test' +require 'rack/freeze' require 'omniauth' require 'omniauth/test' OmniAuth.config.logger = Logger.new('/dev/null') +OmniAuth.config.request_validation_phase = nil RSpec.configure do |config| config.include Rack::Test::Methods @@ -49,7 +51,7 @@ def initialize(*args, &block) def request_phase options[:mutate_on_request].call(options) if options[:mutate_on_request] - @fail = fail!(options[:failure]) if options[:failure] + @fail = fail!(options[:failure], options[:failure_exception]) if options[:failure] @last_env = env return @fail if @fail @@ -58,7 +60,7 @@ def request_phase def callback_phase options[:mutate_on_callback].call(options) if options[:mutate_on_callback] - @fail = fail!(options[:failure]) if options[:failure] + @fail = fail!(options[:failure], options[:failure_exception]) if options[:failure] @last_env = env return @fail if @fail diff --git a/spec/omniauth/auth_hash_spec.rb b/spec/omniauth/auth_hash_spec.rb index 8a1a3c784..9467e86d6 100644 --- a/spec/omniauth/auth_hash_spec.rb +++ b/spec/omniauth/auth_hash_spec.rb @@ -13,6 +13,10 @@ expect(subject.weird_field.info).to eq 'string' end + it 'has a subkey_class' do + expect(OmniAuth::AuthHash.subkey_class).to eq Hashie::Mash + end + describe '#valid?' do subject { OmniAuth::AuthHash.new(:uid => '123', :provider => 'example', :info => {:name => 'Steven'}) } @@ -111,6 +115,10 @@ end end + it 'has a subkey_class' do + expect(OmniAuth::AuthHash::InfoHash.subkey_class).to eq Hashie::Mash + end + require 'hashie/version' if Gem::Version.new(Hashie::VERSION) >= Gem::Version.new('3.5.1') context 'with Hashie 3.5.1+' do diff --git a/spec/omniauth/builder_spec.rb b/spec/omniauth/builder_spec.rb index 4e04e3844..da6a2e49b 100644 --- a/spec/omniauth/builder_spec.rb +++ b/spec/omniauth/builder_spec.rb @@ -3,7 +3,7 @@ describe OmniAuth::Builder do describe '#provider' do it 'translates a symbol to a constant' do - expect(OmniAuth::Strategies).to receive(:const_get).with('MyStrategy').and_return(Class.new) + expect(OmniAuth::Strategies).to receive(:const_get).with('MyStrategy', false).and_return(Class.new) OmniAuth::Builder.new(nil) do provider :my_strategy end @@ -26,6 +26,16 @@ class ExampleClass; end end end.to raise_error(LoadError, 'Could not find matching strategy for :lorax. You may need to install an additional gem (such as omniauth-lorax).') end + + it "doesn't translate a symbol to a top-level constant" do + class MyStrategy; end + + expect do + OmniAuth::Builder.new(nil) do + provider :my_strategy + end + end.to raise_error(LoadError, 'Could not find matching strategy for :my_strategy. You may need to install an additional gem (such as omniauth-my_strategy).') + end end describe '#options' do @@ -111,14 +121,11 @@ class ExampleClass; end describe '#call' do it 'passes env to to_app.call' do - app = lambda { |_env| [200, {}, []] } + app = lambda { |env| [200, {}, env['CUSTOM_ENV_VALUE']] } builder = OmniAuth::Builder.new(app) - env = {'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/some/path'} - allow(app).to receive(:call).and_call_original - - builder.call(env) + env = {'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/some/path', 'CUSTOM_ENV_VALUE' => 'VALUE'} - expect(app).to have_received(:call).with(env) + expect(builder.call(env)).to eq([200, {}, 'VALUE']) end end diff --git a/spec/omniauth/failure_endpoint_spec.rb b/spec/omniauth/failure_endpoint_spec.rb index 714e02962..1c448847b 100644 --- a/spec/omniauth/failure_endpoint_spec.rb +++ b/spec/omniauth/failure_endpoint_spec.rb @@ -43,16 +43,27 @@ expect(head['Location']).to eq('/random/auth/failure?message=invalid_request&strategy=test') end - it 'respects the configured path prefix' do + it 'respects the globally configured path prefix' do allow(OmniAuth.config).to receive(:path_prefix).and_return('/boo') _, head, = *subject.call(env) expect(head['Location']).to eq('/boo/failure?message=invalid_request&strategy=test') end + it 'respects the custom path prefix configured on the strategy' do + env['omniauth.error.strategy'] = ExampleStrategy.new({}, path_prefix: "/some/custom/path") + _, head, = *subject.call(env) + expect(head['Location']).to eq('/some/custom/path/failure?message=invalid_request&strategy=test') + end + it 'includes the origin (escaped) if one is provided' do env['omniauth.origin'] = '/origin-example' _, head, = *subject.call(env) expect(head['Location']).to be_include('&origin=%2Forigin-example') end + + it 'escapes the message key' do + _, head = *subject.call(env.merge('omniauth.error.type' => 'Connection refused!')) + expect(head['Location']).to be_include('message=Connection+refused%21') + end end end diff --git a/spec/omniauth/form_spec.rb b/spec/omniauth/form_spec.rb index 69b91d947..1354a16c7 100644 --- a/spec/omniauth/form_spec.rb +++ b/spec/omniauth/form_spec.rb @@ -7,7 +7,8 @@ end it 'evaluates in the instance when called with a block and no argument' do - OmniAuth::Form.build { |f| expect(f.class).to eq(OmniAuth::Form) } + f = OmniAuth::Form.build { @html = '

OmniAuth

' } + expect(f.instance_variable_get(:@html)).to eq('

OmniAuth

') end end @@ -20,4 +21,34 @@ expect(OmniAuth::Form.new(:title => 'Something Cool').to_html).to be_include('

Something Cool

') end end + + describe '#password_field' do + it 'adds a labeled input field' do + form = OmniAuth::Form.new.password_field('pass', 'password') + form_html = form.to_html + expect(form_html).to include('') + expect(form_html).to include('') + end + end + + describe '#html' do + it 'appends to the html body' do + form = OmniAuth::Form.build { @html = +'

' } + form.html('

') + + expect(form.instance_variable_get(:@html)).to eq '

' + end + end + + describe 'fieldset' do + it 'creates a fieldset with options' do + form = OmniAuth::Form.new + options = {:style => 'color: red', :id => 'fieldSetId'} + expected = "
\n legendary\n\n
" + + form.fieldset('legendary', options) {} + + expect(form.to_html).to include expected + end + end end diff --git a/spec/omniauth/key_store_spec.rb b/spec/omniauth/key_store_spec.rb index 559cba69e..54c87fac5 100644 --- a/spec/omniauth/key_store_spec.rb +++ b/spec/omniauth/key_store_spec.rb @@ -25,6 +25,7 @@ it 'does not log anything to the console' do stub_const('Hashie::VERSION', version) + allow(OmniAuth::KeyStore).to receive(:respond_to?).with(:disable_warnings).and_return(false) OmniAuth::KeyStore.override_logging expect(logger).not_to receive(:info) OmniAuth::KeyStore.new(:id => 1234) diff --git a/spec/omniauth/strategies/developer_spec.rb b/spec/omniauth/strategies/developer_spec.rb index fdeb10970..309b8f7b8 100644 --- a/spec/omniauth/strategies/developer_spec.rb +++ b/spec/omniauth/strategies/developer_spec.rb @@ -10,7 +10,7 @@ end context 'request phase' do - before(:each) { get '/auth/developer' } + before(:each) { post '/auth/developer' } it 'displays a form' do expect(last_response.status).to eq(200) diff --git a/spec/omniauth/strategy_spec.rb b/spec/omniauth/strategy_spec.rb index de670056b..b4a86dace 100644 --- a/spec/omniauth/strategy_spec.rb +++ b/spec/omniauth/strategy_spec.rb @@ -2,7 +2,7 @@ def make_env(path = '/auth/test', props = {}) { - 'REQUEST_METHOD' => 'GET', + 'REQUEST_METHOD' => 'POST', 'PATH_INFO' => path, 'rack.session' => {}, 'rack.input' => StringIO.new('test=true') @@ -32,6 +32,12 @@ def make_env(path = '/auth/test', props = {}) end end + describe 'user_info' do + it 'should default to an empty hash' do + expect(fresh_strategy.new(app, :skip_info => true).user_info).to eq({}) + end + end + describe '.configure' do subject do c = Class.new @@ -63,6 +69,29 @@ def make_env(path = '/auth/test', props = {}) end end + describe '#fail!' do + it 'provides exception information when one is provided' do + env = make_env + exception = RuntimeError.new('No session!') + + expect(OmniAuth.logger).to receive(:error).with( + "(test) Authentication failure! failed: #{exception.class}, #{exception.message}" + ) + + ExampleStrategy.new(app, :failure => :failed, :failure_exception => exception).call(env) + end + + it 'provides a generic message when not provided an exception' do + env = make_env + + expect(OmniAuth.logger).to receive(:error).with( + '(test) Authentication failure! Some Issue encountered.' + ) + + ExampleStrategy.new(app, :failure => 'Some Issue').call(env) + end + end + describe '#skip_info?' do it 'is true if options.skip_info is true' do expect(ExampleStrategy.new(app, :skip_info => true)).to be_skip_info @@ -173,19 +202,25 @@ def make_env(path = '/auth/test', props = {}) end let(:instance) { subject.new(app) } - it 'calls through to uid and info' do + it 'calls through to uid, info, credentials, and extra' do expect(instance).to receive(:uid) expect(instance).to receive(:info) + expect(instance).to receive(:credentials).and_return(expires: true).once + expect(instance).to receive(:extra).and_return(something: 'else').once instance.auth_hash end it 'returns an AuthHash' do allow(instance).to receive(:uid).and_return('123') allow(instance).to receive(:info).and_return(:name => 'Hal Awesome') + allow(instance).to receive(:credentials).and_return(expires: true) + allow(instance).to receive(:extra).and_return(something: 'else') hash = instance.auth_hash expect(hash).to be_kind_of(OmniAuth::AuthHash) expect(hash.uid).to eq('123') expect(hash.info.name).to eq('Hal Awesome') + expect(hash.credentials.expires).to eq(true) + expect(hash.extra.something).to eq('else') end end @@ -312,7 +347,9 @@ def make_env(path = '/auth/test', props = {}) context 'disabled' do it 'does not set omniauth.origin' do @options = {:origin_param => false} - expect { strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'return=/foo')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'return=/foo')) expect(strategy.last_env['rack.session']['omniauth.origin']).to eq(nil) end end @@ -320,24 +357,31 @@ def make_env(path = '/auth/test', props = {}) context 'custom' do it 'sets from a custom param' do @options = {:origin_param => 'return'} - expect { strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'return=/foo')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'return=/foo')) expect(strategy.last_env['rack.session']['omniauth.origin']).to eq('/foo') end end context 'default flow' do it 'is set on the request phase' do - expect { strategy.call(make_env('/auth/test', 'HTTP_REFERER' => 'http://example.com/origin')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with("Request Phase", kind_of(StandardError)) + strategy.call(make_env('/auth/test', 'HTTP_REFERER' => 'http://example.com/origin')) + expect(strategy.last_env['rack.session']['omniauth.origin']).to eq('http://example.com/origin') end it 'is turned into an env variable on the callback phase' do - expect { strategy.call(make_env('/auth/test/callback', 'rack.session' => {'omniauth.origin' => 'http://example.com/origin'})) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with("Callback Phase", kind_of(StandardError)) + strategy.call(make_env('/auth/test/callback', 'rack.session' => {'omniauth.origin' => 'http://example.com/origin'})) + expect(strategy.last_env['omniauth.origin']).to eq('http://example.com/origin') end it 'sets from the params if provided' do - expect { strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'origin=/foo')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/auth/test', 'QUERY_STRING' => 'origin=/foo')) expect(strategy.last_env['rack.session']['omniauth.origin']).to eq('/foo') end @@ -350,7 +394,9 @@ def make_env(path = '/auth/test', props = {}) context 'with script_name' do it 'is set on the request phase, containing full path' do env = {'HTTP_REFERER' => 'http://example.com/sub_uri/origin', 'SCRIPT_NAME' => '/sub_uri'} - expect { strategy.call(make_env('/auth/test', env)) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test', env)) expect(strategy.last_env['rack.session']['omniauth.origin']).to eq('http://example.com/sub_uri/origin') end @@ -359,8 +405,9 @@ def make_env(path = '/auth/test', props = {}) 'rack.session' => {'omniauth.origin' => 'http://example.com/sub_uri/origin'}, 'SCRIPT_NAME' => '/sub_uri' } + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) - expect { strategy.call(make_env('/auth/test/callback', env)) }.to raise_error('Callback Phase') + strategy.call(make_env('/auth/test/callback', env)) expect(strategy.last_env['omniauth.origin']).to eq('http://example.com/sub_uri/origin') end end @@ -369,34 +416,41 @@ def make_env(path = '/auth/test', props = {}) context 'default paths' do it 'uses the default request path' do - expect { strategy.call(make_env) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env) end it 'is case insensitive on request path' do - expect { strategy.call(make_env('/AUTH/Test')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/AUTH/Test')) end it 'is case insensitive on callback path' do - expect { strategy.call(make_env('/AUTH/TeSt/CaLlBAck')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + strategy.call(make_env('/AUTH/TeSt/CaLlBAck')) end it 'uses the default callback path' do - expect { strategy.call(make_env('/auth/test/callback')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + strategy.call(make_env('/auth/test/callback')) end it 'strips trailing spaces on request' do - expect { strategy.call(make_env('/auth/test/')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/auth/test/')) end it 'strips trailing spaces on callback' do - expect { strategy.call(make_env('/auth/test/callback/')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + strategy.call(make_env('/auth/test/callback/')) end context 'callback_url' do it 'uses the default callback_path' do expect(strategy).to receive(:full_host).and_return('http://example.com') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) - expect { strategy.call(make_env) }.to raise_error('Request Phase') + strategy.call(make_env) expect(strategy.callback_url).to eq('http://example.com/auth/test/callback') end @@ -436,12 +490,15 @@ def make_env(path = '/auth/test', props = {}) context 'dynamic paths' do it 'runs the request phase if the custom request path evaluator is truthy' do @options = {:request_path => lambda { |_env| true }} - expect { strategy.call(make_env('/asoufibasfi')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/asoufibasfi')) end it 'runs the callback phase if the custom callback path evaluator is truthy' do @options = {:callback_path => lambda { |_env| true }} - expect { strategy.call(make_env('/asoufiasod')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + + strategy.call(make_env('/asoufiasod')) end it 'provides a custom callback path if request_path evals to a string' do @@ -451,8 +508,9 @@ def make_env(path = '/auth/test', props = {}) it 'correctly reports the callback path when the custom callback path evaluator is truthy' do strategy_instance = ExampleStrategy.new(app, :callback_path => lambda { |env| env['PATH_INFO'] == '/auth/bish/bosh/callback' }) + expect(strategy_instance).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) - expect { strategy_instance.call(make_env('/auth/bish/bosh/callback')) }.to raise_error('Callback Phase') + strategy_instance.call(make_env('/auth/bish/bosh/callback')) expect(strategy_instance.callback_path).to eq('/auth/bish/bosh/callback') end end @@ -460,20 +518,25 @@ def make_env(path = '/auth/test', props = {}) context 'custom paths' do it 'uses a custom request_path if one is provided' do @options = {:request_path => '/awesome'} - expect { strategy.call(make_env('/awesome')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/awesome')) end it 'uses a custom callback_path if one is provided' do @options = {:callback_path => '/radical'} - expect { strategy.call(make_env('/radical')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + + strategy.call(make_env('/radical')) end context 'callback_url' do it 'uses a custom callback_path if one is provided' do @options = {:callback_path => '/radical'} expect(strategy).to receive(:full_host).and_return('http://example.com') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) - expect { strategy.call(make_env('/radical')) }.to raise_error('Callback Phase') + strategy.call(make_env('/radical')) expect(strategy.callback_url).to eq('http://example.com/radical') end @@ -496,18 +559,20 @@ def make_env(path = '/auth/test', props = {}) end it 'uses a custom prefix for request' do - expect { strategy.call(make_env('/wowzers/test')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/wowzers/test')) end it 'uses a custom prefix for callback' do - expect { strategy.call(make_env('/wowzers/test/callback')) }.to raise_error('Callback Phase') + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + strategy.call(make_env('/wowzers/test/callback')) end context 'callback_url' do it 'uses a custom prefix' do expect(strategy).to receive(:full_host).and_return('http://example.com') - - expect { strategy.call(make_env('/wowzers/test')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/wowzers/test')) expect(strategy.callback_url).to eq('http://example.com/wowzers/test/callback') end @@ -523,21 +588,66 @@ def make_env(path = '/auth/test', props = {}) end end + context 'with relative url root' do + let(:props) { {'SCRIPT_NAME' => '/myapp'} } + it 'accepts the request' do + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test', props)) + expect(strategy.request_path).to eq('/myapp/auth/test') + end + + it 'accepts the callback' do + expect(strategy).to receive(:fail!).with('Callback Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test/callback', props)) + end + + context 'callback_url' do + it 'redirects to the correctly prefixed callback' do + expect(strategy).to receive(:full_host).and_return('http://example.com') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + strategy.call(make_env('/auth/test', props)) + + expect(strategy.callback_url).to eq('http://example.com/myapp/auth/test/callback') + end + end + + context 'custom request' do + before do + @options = {:request_path => '/myapp/override', :callback_path => '/myapp/override/callback'} + end + it 'does not prefix a custom request path' do + expect(strategy).to receive(:full_host).and_return('http://example.com') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + expect(strategy.request_path).to eq('/myapp/override') + + strategy.call(make_env('/override', props)) + + expect(strategy.callback_url).to eq('http://example.com/myapp/override/callback') + end + end + end + + context 'request method restriction' do - before do - OmniAuth.config.allowed_request_methods = [:post] + before(:context) do + OmniAuth.config.allowed_request_methods = %i[put post] end it 'does not allow a request method of the wrong type' do - expect { strategy.call(make_env) }.not_to raise_error + expect { strategy.call(make_env('/auth/test', 'REQUEST_METHOD' => 'GET')) }.not_to raise_error end it 'allows a request method of the correct type' do - expect { strategy.call(make_env('/auth/test', 'REQUEST_METHOD' => 'POST')) }.to raise_error('Request Phase') + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + strategy.call(make_env('/auth/test')) end - after do - OmniAuth.config.allowed_request_methods = %i[get post] + after(:context) do + OmniAuth.config.allowed_request_methods = %i[post] end end @@ -548,7 +658,7 @@ def make_env(path = '/auth/test', props = {}) end it 'sets the Allow header properly' do - expect(response[1]['Allow']).to eq('GET, POST') + expect(response[1]['Allow']).to eq('POST') end end @@ -579,14 +689,16 @@ def make_env(path = '/auth/test', props = {}) it 'does not affect original options' do @options[:test_option] = true @options[:mutate_on_request] = proc { |options| options.delete(:test_option) } - expect { strategy.call(make_env) }.to raise_error('Request Phase') + + strategy.call(make_env) expect(strategy.options).to have_key(:test_option) end it 'does not affect deep options' do @options[:deep_option] = {:test_option => true} @options[:mutate_on_request] = proc { |options| options[:deep_option].delete(:test_option) } - expect { strategy.call(make_env) }.to raise_error('Request Phase') + + strategy.call(make_env) expect(strategy.options[:deep_option]).to have_key(:test_option) end end @@ -595,14 +707,16 @@ def make_env(path = '/auth/test', props = {}) it 'does not affect original options' do @options[:test_option] = true @options[:mutate_on_callback] = proc { |options| options.delete(:test_option) } - expect { strategy.call(make_env('/auth/test/callback', 'REQUEST_METHOD' => 'POST')) }.to raise_error('Callback Phase') + + strategy.call(make_env('/auth/test/callback', 'REQUEST_METHOD' => 'POST')) expect(strategy.options).to have_key(:test_option) end it 'does not affect deep options' do @options[:deep_option] = {:test_option => true} @options[:mutate_on_callback] = proc { |options| options[:deep_option].delete(:test_option) } - expect { strategy.call(make_env('/auth/test/callback', 'REQUEST_METHOD' => 'POST')) }.to raise_error('Callback Phase') + + strategy.call(make_env('/auth/test/callback', 'REQUEST_METHOD' => 'POST')) expect(strategy.options[:deep_option]).to have_key(:test_option) end end @@ -771,6 +885,12 @@ def make_env(path = '/auth/test', props = {}) expect(strategy.env['omniauth.params']).to eq('foo' => 'bar') end + it 'rescues errors in request_call' do + allow(strategy).to receive(:mock_request_call).and_raise(StandardError.new('Oh no')) + expect(strategy).to receive(:fail!).with('Oh no', kind_of(StandardError)) + strategy.call(make_env) + end + after do OmniAuth.config.test_mode = false end @@ -809,6 +929,65 @@ def make_env(path = '/auth/test', props = {}) OmniAuth.config.test_mode = false end end + + context 'authenticity validation' do + let(:app) { lambda { |_env| [200, {}, ['reached our target']] } } + let(:strategy) { ExampleStrategy.new(app, :request_path => '/auth/test') } + before do + OmniAuth.config.request_validation_phase = OmniAuth::AuthenticityTokenProtection + end + + context 'with default POST only request methods' do + let!(:csrf_token) { SecureRandom.base64(32) } + let(:escaped_token) { URI.encode_www_form_component(csrf_token, Encoding::UTF_8) } + + it 'allows a request with matching authenticity_token' do + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + post_env = make_env('/auth/test', 'rack.session' => {:csrf => csrf_token}, 'rack.input' => StringIO.new("authenticity_token=#{escaped_token}")) + strategy.call(post_env) + end + + it 'does not allow a request without a matching authenticity token' do + post_env = make_env('/auth/test', 'rack.input' => StringIO.new("authenticity_token=#{escaped_token}")) + expect(strategy.call(post_env)[0]).to eq(302) + expect(strategy.call(post_env)[2]).to eq(['302 Moved']) + end + end + + context 'with allowed GET' do + before(:context) do + @old_allowed_request_methods = OmniAuth.config.allowed_request_methods + OmniAuth.config.allowed_request_methods = %i[post get] + end + + it 'allows a request without authenticity token' do + expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError)) + + get_env = make_env('/auth/test', 'REQUEST_METHOD' => 'GET') + strategy.call(get_env) + end + + after(:context) do + OmniAuth.config.allowed_request_methods = @old_allowed_request_methods + end + end + + after do + OmniAuth.config.request_validation_phase = nil + end + end + + it 'calls fail! when encountering an unhandled exception' do + allow(strategy).to receive(:request_phase).and_raise(Errno::ECONNREFUSED) + expect(strategy).to receive(:fail!).with('Connection refused', kind_of(Errno::ECONNREFUSED)) + strategy.call(make_env) + end + + it 'redirects to the fail! result when encountering an unhandled exception' do + OmniAuth.config.test_mode = false + expect(strategy.call(make_env).first).to eq 302 + end end context 'setup phase' do diff --git a/spec/omniauth_spec.rb b/spec/omniauth_spec.rb index 12f316125..e8499d8b5 100644 --- a/spec/omniauth_spec.rb +++ b/spec/omniauth_spec.rb @@ -26,20 +26,22 @@ class ExampleStrategy end before do - @old_path_prefix = OmniAuth.config.path_prefix - @old_on_failure = OmniAuth.config.on_failure - @old_before_callback_phase = OmniAuth.config.before_callback_phase - @old_before_options_phase = OmniAuth.config.before_options_phase - @old_before_request_phase = OmniAuth.config.before_request_phase + @old_path_prefix = OmniAuth.config.path_prefix + @old_on_failure = OmniAuth.config.on_failure + @old_before_callback_phase = OmniAuth.config.before_callback_phase + @old_before_options_phase = OmniAuth.config.before_options_phase + @old_before_request_phase = OmniAuth.config.before_request_phase + @old_request_validation_phase = OmniAuth.config.request_validation_phase end after do OmniAuth.configure do |config| - config.path_prefix = @old_path_prefix - config.on_failure = @old_on_failure - config.before_callback_phase = @old_before_callback_phase - config.before_options_phase = @old_before_options_phase - config.before_request_phase = @old_before_request_phase + config.path_prefix = @old_path_prefix + config.on_failure = @old_on_failure + config.before_callback_phase = @old_before_callback_phase + config.before_options_phase = @old_before_options_phase + config.before_request_phase = @old_before_request_phase + config.request_validation_phase = @old_request_validation_phase end end @@ -88,6 +90,15 @@ class ExampleStrategy expect(OmniAuth.config.before_callback_phase.call).to eq('heyhey') end + it 'is able to set request_validation_phase' do + OmniAuth.configure do |config| + config.request_validation_phase do + 'validated' + end + end + expect(OmniAuth.config.request_validation_phase.call).to eq('validated') + end + describe 'mock auth' do before do @auth_hash = {:uid => '12345', :info => {:name => 'Joe', :email => 'joe@example.com'}} @@ -128,6 +139,13 @@ class ExampleStrategy end describe '::Utils' do + describe 'form_css' do + it 'returns a style tag with the configured form_css' do + allow(OmniAuth).to receive(:config).and_return(double(:form_css => 'css.css')) + expect(OmniAuth::Utils.form_css).to eq "" + end + end + describe '.deep_merge' do it 'combines hashes' do expect(OmniAuth::Utils.deep_merge({'abc' => {'def' => 123}}, 'abc' => {'foo' => 'bar'})).to eq('abc' => {'def' => 123, 'foo' => 'bar'}) @@ -148,6 +166,15 @@ class ExampleStrategy OmniAuth.config.add_camelization('oauth', 'OAuth') expect(OmniAuth::Utils.camelize(:oauth)).to eq('OAuth') end + + it 'doesn\'t uppercase the first letter when passed false' do + expect(OmniAuth::Utils.camelize('apple_jack', false)).to eq('appleJack') + end + + it 'replaces / with ::' do + expect(OmniAuth::Utils.camelize('apple_jack/cereal')).to eq('AppleJack::Cereal') + expect(OmniAuth::Utils.camelize('apple_jack/cereal', false)).to eq('appleJack::Cereal') + end end end end