diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..6fd6061 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +.rubocop_todo.yml linguist-generated diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml new file mode 100644 index 0000000..a99411a --- /dev/null +++ b/.github/workflows/rubocop.yml @@ -0,0 +1,28 @@ +name: RuboCop + +on: + push: + branches: [ master, develop ] + pull_request: + branches: [ master, develop ] + +permissions: + contents: read + +jobs: + rubocop: + name: RuboCop + runs-on: ubuntu-latest + strategy: + matrix: + ruby-version: ['3.2'] + + steps: + - uses: actions/checkout@v4 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true + - name: RuboCop + run: bundle exec rubocop diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..7122c81 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,19 @@ +inherit_from: .rubocop_todo.yml + +require: + - rubocop-performance + - rubocop-rake + - rubocop-rspec + +AllCops: + TargetRubyVersion: 2.1 + NewCops: enable + Exclude: + - 'bin/*' + - 'coverage/**/*' + - 'doc/**/*' + - 'gemfiles/*' + - 'vendor/**/*' + +Gemspec: + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..4e41bc6 --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,149 @@ +# This configuration was generated by +# `rubocop --auto-gen-config --no-offense-counts --no-auto-gen-timestamp` +# using RuboCop version 1.57.2. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# This cop supports safe autocorrection (--autocorrect). +Lint/EnsureReturn: + Exclude: + - 'lib/omniauth/strategies/cas/logout_request.rb' + +Lint/UselessRescue: + Exclude: + - 'lib/omniauth/strategies/cas/logout_request.rb' + +# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. +Metrics/AbcSize: + Max: 26 + +# Configuration parameters: CountBlocks. +Metrics/BlockNesting: + Max: 4 + +# Configuration parameters: CountComments, CountAsOne. +Metrics/ClassLength: + Max: 175 + +# Configuration parameters: AllowedMethods, AllowedPatterns. +Metrics/CyclomaticComplexity: + Max: 9 + +# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. +Metrics/MethodLength: + Max: 18 + +# Configuration parameters: AllowedMethods, AllowedPatterns. +Metrics/PerceivedComplexity: + Max: 11 + +Naming/AccessorMethodName: + Exclude: + - 'lib/omniauth/strategies/cas/service_ticket_validator.rb' + +Naming/ConstantName: + Exclude: + - 'lib/omniauth/strategies/cas.rb' + +RSpec/AnyInstance: + Exclude: + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: Prefixes, AllowedPatterns. +# Prefixes: when, with, without +RSpec/ContextWording: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: CountAsOne. +RSpec/ExampleLength: + Max: 8 + +RSpec/ExpectInHook: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + +# Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly. +# Include: **/*_spec*rb*, **/spec/**/* +RSpec/FilePath: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas/service_ticket_validator_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: AssignmentOnly. +RSpec/InstanceVariable: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +RSpec/MultipleExpectations: + Max: 8 + +# Configuration parameters: AllowSubject. +RSpec/MultipleMemoizedHelpers: + Max: 10 + +# Configuration parameters: EnforcedStyle, IgnoreSharedExamples. +# SupportedStyles: always, named_only +RSpec/NamedSubject: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas/service_ticket_validator_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: AllowedGroups. +RSpec/NestedGroups: + Max: 5 + +# Configuration parameters: AllowedPatterns. +# AllowedPatterns: ^expect_, ^assert_ +RSpec/NoExpectationExample: + Exclude: + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: Include, CustomTransform, IgnoreMethods, IgnoreMetadata. +# Include: **/*_spec.rb +RSpec/SpecFilePathFormat: + Exclude: + - '**/spec/routing/**/*' + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas/service_ticket_validator_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: EnforcedStyle, AllowedPatterns. +# SupportedStyles: snake_case, camelCase +RSpec/VariableName: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. +RSpec/VerifiedDoubles: + Exclude: + - 'spec/omniauth/strategies/cas/logout_request_spec.rb' + - 'spec/omniauth/strategies/cas/service_ticket_validator_spec.rb' + - 'spec/omniauth/strategies/cas_spec.rb' + +# This cop supports safe autocorrection (--autocorrect). +Rake/Desc: + Exclude: + - 'Rakefile' + +# Configuration parameters: AllowedConstants. +Style/Documentation: + Exclude: + - 'spec/**/*' + - 'test/**/*' + - 'lib/omniauth/strategies/cas.rb' + - 'lib/omniauth/strategies/cas/logout_request.rb' + - 'lib/omniauth/strategies/cas/service_ticket_validator.rb' + +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns. +# URISchemes: http, https +Layout/LineLength: + Max: 210 diff --git a/Gemfile b/Gemfile index d4295d7..078cc2f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,10 +1,17 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gem 'byebug' # Specify your gem's dependencies in omniauth-cas.gemspec gemspec -if RUBY_VERSION < "2.2.2" +if RUBY_VERSION < '2.2.2' # Rack 2 needs Ruby 2.2.2+ gem 'rack', '< 2' end + +gem 'rubocop' +gem 'rubocop-performance' +gem 'rubocop-rake' +gem 'rubocop-rspec' diff --git a/Rakefile b/Rakefile old mode 100644 new mode 100755 index af92638..43c96c7 --- a/Rakefile +++ b/Rakefile @@ -1,4 +1,6 @@ #!/usr/bin/env rake +# frozen_string_literal: true + require 'bundler/gem_tasks' require 'rspec/core/rake_task' @@ -11,5 +13,5 @@ RSpec::Core::RakeTask.new(:spec) do |t| end task :test do - fail %q{This application uses RSpec. Try running "rake spec"} + raise 'This application uses RSpec. Try running "rake spec"' end diff --git a/lib/omniauth-cas.rb b/lib/omniauth-cas.rb deleted file mode 100644 index 3de039d..0000000 --- a/lib/omniauth-cas.rb +++ /dev/null @@ -1 +0,0 @@ -require 'omniauth/cas' diff --git a/lib/omniauth/cas.rb b/lib/omniauth/cas.rb index dab016a..5990130 100644 --- a/lib/omniauth/cas.rb +++ b/lib/omniauth/cas.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + require 'omniauth/cas/version' -require 'omniauth/strategies/cas' \ No newline at end of file +require 'omniauth/strategies/cas' diff --git a/lib/omniauth/cas/version.rb b/lib/omniauth/cas/version.rb index 1910c93..833d16f 100644 --- a/lib/omniauth/cas/version.rb +++ b/lib/omniauth/cas/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Omniauth module Cas VERSION = '2.0.0' diff --git a/lib/omniauth/strategies/cas.rb b/lib/omniauth/strategies/cas.rb index 19a866d..ae2df1d 100644 --- a/lib/omniauth/strategies/cas.rb +++ b/lib/omniauth/strategies/cas.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'omniauth' require 'addressable/uri' @@ -16,7 +18,7 @@ class InvalidReturnURL < StandardError; end autoload :LogoutRequest, 'omniauth/strategies/cas/logout_request' attr_accessor :raw_info - alias_method :user_info, :raw_info + alias user_info raw_info option :name, :cas # Required property by OmniAuth::Strategy @@ -28,7 +30,7 @@ class InvalidReturnURL < StandardError; end option :service_validate_url, '/serviceValidate' option :login_url, '/login' option :logout_url, '/logout' - option :on_single_sign_out, Proc.new {} + option :on_single_sign_out, proc {} # A Proc or lambda that returns a Hash of additional user info to be # merged with the info returned by the CAS server. # @@ -37,7 +39,7 @@ class InvalidReturnURL < StandardError; end # @param [Hash] The user info for the Service Ticket returned by the CAS server # # @return [Hash] Extra user info - option :fetch_raw_info, Proc.new { Hash.new } + option :fetch_raw_info, proc { {} } # Make all the keys configurable with some defaults set here option :uid_field, 'user' option :name_key, 'name' @@ -50,23 +52,23 @@ class InvalidReturnURL < StandardError; end option :phone_key, 'phone' # As required by https://github.com/intridea/omniauth/wiki/Auth-Hash-Schema - AuthHashSchemaKeys = %w{name email nickname first_name last_name location image phone} + AuthHashSchemaKeys = %w[name email nickname first_name last_name location image phone].freeze info do prune!({ - name: raw_info[options[:name_key].to_s], - email: raw_info[options[:email_key].to_s], - nickname: raw_info[options[:nickname_key].to_s], - first_name: raw_info[options[:first_name_key].to_s], - last_name: raw_info[options[:last_name_key].to_s], - location: raw_info[options[:location_key].to_s], - image: raw_info[options[:image_key].to_s], - phone: raw_info[options[:phone_key].to_s] - }) + name: raw_info[options[:name_key].to_s], + email: raw_info[options[:email_key].to_s], + nickname: raw_info[options[:nickname_key].to_s], + first_name: raw_info[options[:first_name_key].to_s], + last_name: raw_info[options[:last_name_key].to_s], + location: raw_info[options[:location_key].to_s], + image: raw_info[options[:image_key].to_s], + phone: raw_info[options[:phone_key].to_s] + }) end extra do prune!( - raw_info.delete_if{ |k,v| AuthHashSchemaKeys.include?(k) } + raw_info.delete_if { |k, _v| AuthHashSchemaKeys.include?(k) } ) end @@ -84,8 +86,10 @@ def callback_phase else @ticket = request.params['ticket'] return fail!(:no_ticket, MissingCASTicket.new('No CAS Ticket')) unless @ticket + fetch_raw_info(@ticket) return fail!(:invalid_ticket, InvalidCASTicket.new('Invalid CAS Ticket')) if raw_info.empty? + super end end @@ -100,15 +104,15 @@ def request_phase 'Location' => login_url(service_url), 'Content-Type' => 'text/plain' }, - ["You are being redirected to CAS for sign-in."] + ['You are being redirected to CAS for sign-in.'] ] else - [ 400, {}, [ "Bad request" ] ] + [400, {}, ['Bad request']] end end def on_sso_path? - request.post? && request.params.has_key?('logoutRequest') + request.post? && request.params.key?('logoutRequest') end def single_sign_out_phase @@ -127,15 +131,15 @@ def cas_url def by_host_cas_url return unless options.url_by_request_host && \ - options.url_by_request_host.respond_to?(:fetch) + options.url_by_request_host.respond_to?(:fetch) uri = options.url_by_request_host.fetch(request.host) Addressable::URI.parse(uri).to_s - rescue + rescue StandardError nil # When request.host is not defined or it raises, - # or when Addressable raises, we can only resort - # to the default. + # or when Addressable raises, we can only resort + # to the default. end def static_cas_url @@ -158,9 +162,9 @@ def extract_url end def validate_cas_setup - if options.host.nil? || options.login_url.nil? - raise ArgumentError.new(":host and :login_url MUST be provided") - end + return unless options.host.nil? || options.login_url.nil? + + raise ArgumentError, ':host and :login_url MUST be provided' end # Checks that the callback URL is within the scope of the target @@ -187,7 +191,7 @@ def validate_service_url!(service_url) return false end - return true + true end # Build a service-validation URL from +service+ and +ticket+. @@ -203,9 +207,9 @@ def service_validate_url(service_url, ticket) service_url = Addressable::URI.parse(service_url) service_url.query_values = service_url.query_values.tap { |qs| qs.delete('ticket') } cas_url + append_params(options.service_validate_url, { - service: service_url.to_s, - ticket: ticket - }) + service: service_url.to_s, + ticket: ticket + }) end # Build a CAS login URL from +service+. @@ -224,7 +228,7 @@ def login_url(service) # # @return [String] the new joined URL. def append_params(base, params) - params = params.each { |k,v| v = Rack::Utils.escape(v) } + params = params.each { |_k, v| Rack::Utils.escape(v) } Addressable::URI.parse(base).tap do |base_uri| base_uri.query_values = (base_uri.query_values || {}).merge(params) end.to_s @@ -236,14 +240,14 @@ def validate_service_ticket(ticket) ServiceTicketValidator.new(self, options, callback_url, ticket).call end - private + private def fetch_raw_info(ticket) validator = validate_service_ticket(ticket) ticket_user_info = validator.user_info ticket_success_body = validator.success_body custom_user_info = options.fetch_raw_info.call(self, - options, ticket, ticket_user_info, ticket_success_body) + options, ticket, ticket_user_info, ticket_success_body) self.raw_info = ticket_user_info.merge(custom_user_info) end diff --git a/lib/omniauth/strategies/cas/logout_request.rb b/lib/omniauth/strategies/cas/logout_request.rb index eba676c..0a33cdc 100644 --- a/lib/omniauth/strategies/cas/logout_request.rb +++ b/lib/omniauth/strategies/cas/logout_request.rb @@ -1,9 +1,12 @@ +# frozen_string_literal: true + module OmniAuth module Strategies class CAS class LogoutRequest def initialize(strategy, request) - @strategy, @request = strategy, request + @strategy = strategy + @request = request end def call(options = {}) @@ -11,10 +14,10 @@ def call(options = {}) begin result = single_sign_out_callback.call(*logout_request) - rescue StandardError => err - return @strategy.fail! :logout_request, err + rescue StandardError => e + @strategy.fail! :logout_request, e else - result = [200,{},'OK'] if result == true || result.nil? + result = [200, {}, 'OK'] if result == true || result.nil? ensure return unless result @@ -22,18 +25,18 @@ def call(options = {}) # when Rack::Response#new wants [body,status,headers]? Additionally, # why does Rack::Response differ in argument order from the usual # Rack-like [status,headers,body] array? - return Rack::Response.new(result[2],result[0],result[1]).finish + return Rack::Response.new(result[2], result[0], result[1]).finish end end - private + private def logout_request @logout_request ||= begin saml = Nokogiri.parse(@request.params['logoutRequest']) name_id = saml.xpath('//saml:NameID').text sess_idx = saml.xpath('//samlp:SessionIndex').text - inject_params(name_id:name_id, session_index:sess_idx) + inject_params(name_id: name_id, session_index: sess_idx) @request end end @@ -42,7 +45,7 @@ def inject_params(new_params) rack_input = @request.env['rack.input'].read params = Rack::Utils.parse_query(rack_input, '&').merge new_params @request.env['rack.input'] = StringIO.new(Rack::Utils.build_query(params)) - rescue + rescue StandardError # A no-op intended to ensure that the ensure block is run raise ensure diff --git a/lib/omniauth/strategies/cas/service_ticket_validator.rb b/lib/omniauth/strategies/cas/service_ticket_validator.rb index c5d8d74..64a9a09 100644 --- a/lib/omniauth/strategies/cas/service_ticket_validator.rb +++ b/lib/omniauth/strategies/cas/service_ticket_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'net/http' require 'net/https' require 'nokogiri' @@ -6,7 +8,7 @@ module OmniAuth module Strategies class CAS class ServiceTicketValidator - VALIDATION_REQUEST_HEADERS = { 'Accept' => '*/*' } + VALIDATION_REQUEST_HEADERS = { 'Accept' => '*/*' }.freeze attr_reader :success_body @@ -40,7 +42,7 @@ def user_info parse_user_info(@success_body) end - private + private # Merges attributes with multiple values into an array if support is # enabled (disabled by default) @@ -56,12 +58,13 @@ def attribute_value(user_info, attribute, value) # returns nil if given nil def parse_user_info(node) return nil if node.nil? + {}.tap do |hash| node.children.each do |e| node_name = e.name.sub(/^cas:/, '') - unless e.kind_of?(Nokogiri::XML::Text) || node_name == 'proxies' + unless e.is_a?(Nokogiri::XML::Text) || node_name == 'proxies' # There are no child elements - if e.element_children.count == 0 + if e.element_children.count.zero? hash[node_name] = attribute_value(hash, node_name, e.content) elsif e.element_children.count # JASIG style extra attributes @@ -82,6 +85,7 @@ def parse_user_info(node) # if the passed body is nil or if there is no such node. def find_authentication_success(body) return nil if body.nil? || body == '' + begin doc = Nokogiri::XML(body) begin diff --git a/lib/omniauth_cas.rb b/lib/omniauth_cas.rb new file mode 100644 index 0000000..a98c23d --- /dev/null +++ b/lib/omniauth_cas.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +require 'omniauth/cas' diff --git a/omniauth-cas.gemspec b/omniauth-cas.gemspec index 5eca2ed..2251b50 100644 --- a/omniauth-cas.gemspec +++ b/omniauth-cas.gemspec @@ -1,18 +1,19 @@ -# -*- encoding: utf-8 -*- -require File.expand_path('../lib/omniauth/cas/version', __FILE__) +# frozen_string_literal: true + +require File.expand_path('lib/omniauth/cas/version', __dir__) Gem::Specification.new do |gem| - gem.authors = ["Derek Lindahl"] - gem.email = ["dlindahl@customink.com"] - gem.summary = %q{CAS Strategy for OmniAuth} + gem.authors = ['Derek Lindahl'] + gem.email = ['dlindahl@customink.com'] + gem.summary = 'CAS Strategy for OmniAuth' gem.description = gem.summary - gem.homepage = "https://github.com/dlindahl/omniauth-cas" + gem.homepage = 'https://github.com/dlindahl/omniauth-cas' - gem.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) } + gem.executables = `git ls-files -- bin/*`.split("\n").map { |f| File.basename(f) } gem.files = `git ls-files`.split("\n") gem.test_files = `git ls-files -- {test,spec,features}/*`.split("\n") - gem.name = "omniauth-cas" - gem.require_paths = ["lib"] + gem.name = 'omniauth-cas' + gem.require_paths = ['lib'] gem.version = Omniauth::Cas::VERSION gem.add_dependency 'omniauth', '~> 1.2' diff --git a/spec/omniauth/strategies/cas/logout_request_spec.rb b/spec/omniauth/strategies/cas/logout_request_spec.rb index 830c00c..cdf2952 100644 --- a/spec/omniauth/strategies/cas/logout_request_spec.rb +++ b/spec/omniauth/strategies/cas/logout_request_spec.rb @@ -1,26 +1,28 @@ +# frozen_string_literal: true + require 'spec_helper' describe OmniAuth::Strategies::CAS::LogoutRequest do + subject { described_class.new(strategy, request).call(options) } + let(:strategy) { double('strategy') } let(:env) do - { 'rack.input' => StringIO.new('','r') } + { 'rack.input' => StringIO.new('', 'r') } end - let(:request) { double('request', params:params, env:env) } + let(:request) { double('request', params: params, env: env) } let(:params) { { 'url' => url, 'logoutRequest' => logoutRequest } } let(:url) { 'http://notes.dev/signed_in' } let(:logoutRequest) do - %Q[ - + %( + @NOT_USED@ ST-123456-123abc456def - ] + ) end - subject { described_class.new(strategy, request).call(options) } - describe 'SAML attributes' do - let(:callback) { Proc.new{} } + let(:callback) { proc {} } let(:options) do { on_single_sign_out: callback } end @@ -61,7 +63,7 @@ let(:response_body) { subject[2].respond_to?(:body) ? subject[2].body : subject[2] } context 'that returns TRUE' do - let(:callback) { Proc.new{true} } + let(:callback) { proc { true } } it 'responds with OK' do expect(subject[0]).to eq 200 @@ -70,7 +72,7 @@ end context 'that returns Nil' do - let(:callback) { Proc.new{} } + let(:callback) { proc {} } it 'responds with OK' do expect(subject[0]).to eq 200 @@ -79,7 +81,7 @@ end context 'that returns a tuple' do - let(:callback) { Proc.new{ [400,{},'Bad Request'] } } + let(:callback) { proc { [400, {}, 'Bad Request'] } } it 'responds with OK' do expect(subject[0]).to eq 400 @@ -88,8 +90,8 @@ end context 'that raises an error' do - let(:exception) { RuntimeError.new('error' )} - let(:callback) { Proc.new{raise exception} } + let(:exception) { RuntimeError.new('error') } + let(:callback) { proc { raise exception } } before do allow(strategy).to receive(:fail!) diff --git a/spec/omniauth/strategies/cas/service_ticket_validator_spec.rb b/spec/omniauth/strategies/cas/service_ticket_validator_spec.rb index 92f40d1..fb68cb1 100644 --- a/spec/omniauth/strategies/cas/service_ticket_validator_spec.rb +++ b/spec/omniauth/strategies/cas/service_ticket_validator_spec.rb @@ -1,30 +1,30 @@ +# frozen_string_literal: true + require 'spec_helper' describe OmniAuth::Strategies::CAS::ServiceTicketValidator do let(:strategy) do double('strategy', - service_validate_url: 'https://example.org/serviceValidate' - ) + service_validate_url: 'https://example.org/serviceValidate') end let(:provider_options) do double('provider_options', - disable_ssl_verification?: false, - merge_multivalued_attributes: false, - ca_path: '/etc/ssl/certsZOMG' - ) + disable_ssl_verification?: false, + merge_multivalued_attributes: false, + ca_path: '/etc/ssl/certsZOMG') end let(:validator) do - OmniAuth::Strategies::CAS::ServiceTicketValidator.new( strategy, provider_options, '/foo', nil ) + described_class.new(strategy, provider_options, '/foo', nil) end describe '#call' do + subject { validator.call } + before do stub_request(:get, 'https://example.org/serviceValidate?') .to_return(status: 200, body: '') end - subject { validator.call } - it 'returns itself' do expect(subject).to eq validator end @@ -36,6 +36,8 @@ end describe '#user_info' do + subject { validator.user_info } + let(:ok_fixture) do File.expand_path(File.join(File.dirname(__FILE__), '../../../fixtures/cas_success.xml')) end @@ -43,12 +45,10 @@ before do stub_request(:get, 'https://example.org/serviceValidate?') - .to_return(status: 200, body:service_response) + .to_return(status: 200, body: service_response) validator.call end - subject { validator.user_info } - context 'with default settings' do it 'parses user info from the response' do expect(subject).to include 'user' => 'psegel' @@ -59,10 +59,9 @@ context 'when merging multivalued attributes' do let(:provider_options) do double('provider_options', - disable_ssl_verification?: false, - merge_multivalued_attributes: true, - ca_path: '/etc/ssl/certsZOMG' - ) + disable_ssl_verification?: false, + merge_multivalued_attributes: true, + ca_path: '/etc/ssl/certsZOMG') end it 'parses multivalued user info from the response' do diff --git a/spec/omniauth/strategies/cas_spec.rb b/spec/omniauth/strategies/cas_spec.rb index 3efe7e4..bc17b48 100644 --- a/spec/omniauth/strategies/cas_spec.rb +++ b/spec/omniauth/strategies/cas_spec.rb @@ -1,39 +1,46 @@ +# frozen_string_literal: true + require 'spec_helper' describe OmniAuth::Strategies::CAS, type: :strategy do include Rack::Test::Methods - let(:my_cas_provider) { Class.new(OmniAuth::Strategies::CAS) } - before do - stub_const 'MyCasProvider', my_cas_provider - end + let(:my_cas_provider) { Class.new(described_class) } let(:app) do - Rack::Builder.new { + Rack::Builder.new do use OmniAuth::Test::PhonySession use MyCasProvider, - name: :cas, - host: 'cas.example.org', - ssl: false, - port: 8080, - uid_field: :employeeid, - fetch_raw_info: Proc.new { |v, opts, ticket, info, node| - info.empty? ? {} : { - "roles" => node.xpath('//cas:roles').map(&:text), + name: :cas, + host: 'cas.example.org', + ssl: false, + port: 8080, + uid_field: :employeeid, + fetch_raw_info: proc { |_v, _opts, _ticket, info, node| + if info.empty? + {} + else + { + 'roles' => node.xpath('//cas:roles').map(&:text) + } + end } - } - run lambda { |env| [404, {'Content-Type' => 'text/plain'}, [env.key?('omniauth.auth').to_s]] } - }.to_app + run ->(env) { [404, { 'Content-Type' => 'text/plain' }, [env.key?('omniauth.auth').to_s]] } + end.to_app + end + + before do + stub_const 'MyCasProvider', my_cas_provider end # TODO: Verify that these are even useful tests shared_examples_for 'a CAS redirect response' do - let(:redirect_params) { 'service=' + Rack::Utils.escape("http://example.org/auth/cas/callback?url=#{Rack::Utils.escape(return_url)}") } + subject { last_response } - before { get url, nil, request_env } + let(:redirect_params) { "service=#{Rack::Utils.escape("http://example.org/auth/cas/callback?url=#{Rack::Utils.escape(return_url)}")}" } - subject { last_response } + before { get url, nil, request_env } - it { should be_redirect } + it { is_expected.to be_redirect } it 'redirects to the CAS server' do expect(subject.headers).to include 'Location' => "http://cas.example.org:8080/login?#{redirect_params}" @@ -41,54 +48,56 @@ end describe '#cas_url' do - let(:params) { Hash.new } - let(:provider) { MyCasProvider.new(nil, params) } - subject { provider.cas_url } + let(:params) { {} } + let(:provider) { MyCasProvider.new(nil, params) } + it 'raises an ArgumentError' do - expect{subject}.to raise_error ArgumentError, %r{:host and :login_url MUST be provided} + expect { subject }.to raise_error ArgumentError, /:host and :login_url MUST be provided/ end context 'with an explicit :url option' do let(:url) { 'https://example.org:8080/my_cas' } - let(:params) { super().merge url:url } + let(:params) { super().merge url: url } before { subject } - it { should eq url } + it { is_expected.to eq url } it 'parses the URL into it the appropriate strategy options' do - expect(provider.options).to include ssl:true - expect(provider.options).to include host:'example.org' - expect(provider.options).to include port:8080 - expect(provider.options).to include path:'/my_cas' + expect(provider.options).to include ssl: true + expect(provider.options).to include host: 'example.org' + expect(provider.options).to include port: 8080 + expect(provider.options).to include path: '/my_cas' end end context 'with explicit URL component' do - let(:params) { super().merge host:'example.org', port:1234, ssl:true, path:'/a/path' } + let(:params) { super().merge host: 'example.org', port: 1234, ssl: true, path: '/a/path' } before { subject } - it { should eq 'https://example.org:1234/a/path' } + it { is_expected.to eq 'https://example.org:1234/a/path' } it 'parses the URL into it the appropriate strategy options' do - expect(provider.options).to include ssl:true - expect(provider.options).to include host:'example.org' - expect(provider.options).to include port:1234 - expect(provider.options).to include path:'/a/path' + expect(provider.options).to include ssl: true + expect(provider.options).to include host: 'example.org' + expect(provider.options).to include port: 1234 + expect(provider.options).to include path: '/a/path' end end context 'with a URL by host mapping' do - let(:params) { super().merge( - url: 'https://default.cas.host', - url_by_request_host: { - 'host1.example.org' => 'https://host1.cas.host', - 'host2.example.org' => 'https://host2.cas.host', - }) - } + let(:params) do + super().merge( + url: 'https://default.cas.host', + url_by_request_host: { + 'host1.example.org' => 'https://host1.cas.host', + 'host2.example.org' => 'https://host2.cas.host' + } + ) + end let(:request_host) { nil } @@ -121,18 +130,18 @@ describe 'defaults' do subject { MyCasProvider.default_options.to_hash } - it { should include('ssl' => true) } + it { is_expected.to include('ssl' => true) } end describe 'GET /auth/cas' do let(:return_url) { 'http://example.org/admin/foo' } context 'with a return url on a different host than the service url' do - before { get '/auth/cas?url=http://attack.example.org/' } - subject { last_response } - it { should be_bad_request } + before { get '/auth/cas?url=http://attack.example.org/' } + + it { is_expected.to be_bad_request } end context 'with a referer' do @@ -154,11 +163,11 @@ describe 'GET /auth/cas/callback' do context 'without a ticket' do - before { get '/auth/cas/callback' } - subject { last_response } - it { should be_redirect } + before { get '/auth/cas/callback' } + + it { is_expected.to be_redirect } it 'redirects with a failure message' do expect(subject.headers).to include 'Location' => '/auth/failure?message=no_ticket&strategy=cas' @@ -166,15 +175,15 @@ end context 'with an invalid ticket' do + subject { last_response } + before do - stub_request(:get, /^http:\/\/cas.example.org:8080?\/serviceValidate\?([^&]+&)?ticket=9391d/). - to_return( body: File.read('spec/fixtures/cas_failure.xml') ) + stub_request(:get, %r{^http://cas.example.org:8080?/serviceValidate\?([^&]+&)?ticket=9391d}) + .to_return(body: File.read('spec/fixtures/cas_failure.xml')) get '/auth/cas/callback?ticket=9391d' end - subject { last_response } - - it { should be_redirect } + it { is_expected.to be_redirect } it 'redirects with a failure message' do expect(subject.headers).to include 'Location' => '/auth/failure?message=invalid_ticket&strategy=cas' @@ -182,11 +191,11 @@ end describe 'with a valid ticket' do - shared_examples :successful_validation do + shared_examples 'successful validation' do before do - stub_request(:get, /^http:\/\/cas.example.org:8080?\/serviceValidate\?([^&]+&)?ticket=593af/) + stub_request(:get, %r{^http://cas.example.org:8080?/serviceValidate\?([^&]+&)?ticket=593af}) .with { |request| @request_uri = request.uri.to_s } - .to_return( body: File.read("spec/fixtures/#{xml_file_name}") ) + .to_return(body: File.read("spec/fixtures/#{xml_file_name}")) get "/auth/cas/callback?ticket=593af&url=#{return_url}" end @@ -198,15 +207,15 @@ it 'properly encodes the service URL' do expect(WebMock).to have_requested(:get, 'http://cas.example.org:8080/serviceValidate') .with(query: { - ticket: '593af', - service: 'http://example.org/auth/cas/callback?url=' + Rack::Utils.escape('http://127.0.0.10/?some=parameter') - }) + ticket: '593af', + service: "http://example.org/auth/cas/callback?url=#{Rack::Utils.escape('http://127.0.0.10/?some=parameter')}" + }) end context "request.env['omniauth.auth']" do subject { last_request.env['omniauth.auth'] } - it { should be_kind_of Hash } + it { is_expected.to be_a Hash } it 'identifes the provider' do expect(subject.provider).to eq :cas @@ -238,7 +247,7 @@ expect(subject.user).to eq 'psegel' expect(subject.employeeid).to eq '54' expect(subject.hire_date).to eq '2004-07-13' - expect(subject.roles).to eq %w(senator lobbyist financier) + expect(subject.roles).to eq %w[senator lobbyist financier] end end @@ -261,38 +270,38 @@ context 'with JASIG flavored XML' do let(:xml_file_name) { 'cas_success_jasig.xml' } - it_behaves_like :successful_validation + it_behaves_like 'successful validation' end context 'with classic XML' do let(:xml_file_name) { 'cas_success.xml' } - it_behaves_like :successful_validation + it_behaves_like 'successful validation' end end end describe 'POST /auth/cas/callback' do describe 'with a Single Sign-Out logoutRequest' do + subject do + post 'auth/cas/callback', logoutRequest: logoutRequest + end + let(:logoutRequest) do - %Q[ - + %( + @NOT_USED@ ST-123456-123abc456def - ] + ) end - let(:logout_request) { double('logout_request', call:[200,{},'OK']) } - - subject do - post 'auth/cas/callback', logoutRequest:logoutRequest - end + let(:logout_request) { double('logout_request', call: [200, {}, 'OK']) } before do allow_any_instance_of(MyCasProvider) .to receive(:logout_request_service) - .and_return double('LogoutRequest', new:logout_request) + .and_return double('LogoutRequest', new: logout_request) subject end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5f3ac90..8181cc0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'bundler/setup' require 'awesome_print' @@ -8,6 +10,6 @@ require 'rack/test' require 'webmock/rspec' -require 'omniauth-cas' +require 'omniauth_cas' -OmniAuth.config.logger = Logger.new( '/dev/null' ) +OmniAuth.config.logger = Logger.new('/dev/null')