Merge pull request #85 from dlindahl/chore/fix-some-offenses

Chore/fix some offenses

README.md

@@ -1,10 +1,4 @@
-# OmniAuth CAS Strategy [![Gem Version][version_badge]][version] [![Build Status][github_actions_status]][github_actions]
-
-[version_badge]: https://badge.fury.io/rb/omniauth-cas.svg
-[version]: https://badge.fury.io/rb/omniauth-cas
-[github_actions]: https://github.com/dlindahl/omniauth-cas/actions
-[github_actions_status]: https://github.com/dlindahl/omniauth-cas/actions/workflows/ci.yml/badge.svg
-[releases]: https://github.com/dlindahl/omniauth-cas/releases
+# OmniAuth CAS Strategy [![Gem Version][version_badge]][version] [![Build Status][github_actions_ci_status]][github_actions_ci] [![RuboCop][github_actions_rubocop_status]][github_actions_rubocop]
 
 This is a [OmniAuth][omniauth] 2.1+ compatible port of the previously available
 [OmniAuth CAS strategy][old_omniauth_cas] that was bundled with OmniAuth 0.3.
@@ -122,8 +116,15 @@ Special thanks go out to the following people
   * Elber Ribeiro (@dynaum) for Ubuntu SSL configuration support
   * @rbq for README updates and OmniAuth 0.3 migration guide
 
-[omniauth]: https://github.com/omniauth/omniauth
-[old_omniauth_cas]: https://github.com/intridea/omniauth/blob/0-3-stable/oa-enterprise/lib/omniauth/strategies/cas.rb
 [document_up]: https://dlindahl.github.io/omniauth-cas/
+[github_actions_ci]: https://github.com/dlindahl/omniauth-cas/actions/workflows/ci.yml
+[github_actions_ci_status]: https://github.com/dlindahl/omniauth-cas/actions/workflows/ci.yml/badge.svg
+[github_actions_rubocop]: https://github.com/dlindahl/omniauth-cas/actions/workflows/rubocop.yml
+[github_actions_rubocop_status]: https://github.com/dlindahl/omniauth-cas/actions/workflows/rubocop.yml/badge.svg
 [net_http]: https://ruby-doc.org/stdlib-1.9.3/libdoc/net/http/rdoc/Net/HTTP.html
+[old_omniauth_cas]: https://github.com/intridea/omniauth/blob/0-3-stable/oa-enterprise/lib/omniauth/strategies/cas.rb
+[omniauth]: https://github.com/omniauth/omniauth
+[releases]: https://github.com/dlindahl/omniauth-cas/releases
 [sso]: https://wiki.jasig.org/display/CASUM/Single+Sign+Out
+[version]: https://badge.fury.io/rb/omniauth-cas
+[version_badge]: https://badge.fury.io/rb/omniauth-cas.svg

spec/omniauth/strategies/cas/logout_request_spec.rb

@@ -3,25 +3,25 @@
 require 'spec_helper'
 
 RSpec.describe OmniAuth::Strategies::CAS::LogoutRequest do
-  subject { described_class.new(strategy, request).call(options) }
+  subject(:call) { described_class.new(strategy, request).call(options) }
 
   let(:strategy) { double('strategy') }
   let(:env) do
     { 'rack.input' => StringIO.new('', 'r') }
   end
   let(:request) { double('request', params: params, env: env) }
-  let(:params) { { 'url' => url, 'logoutRequest' => logoutRequest } }
+  let(:params) { { 'url' => url, 'logoutRequest' => logout_request_xml } }
   let(:url) { 'http://example.org/signed_in' }
-  let(:logoutRequest) do
-    %(
+  let(:logout_request_xml) do
+    <<~XML
       <samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="123abc-1234-ab12-cd34-1234abcd" Version="2.0" IssueInstant="#{Time.now}">
         <saml:NameID>@NOT_USED@</saml:NameID>
         <samlp:SessionIndex>ST-123456-123abc456def</samlp:SessionIndex>
       </samlp:LogoutRequest>
-    )
+    XML
   end
 
-  describe 'SAML attributes' do
+  context 'when parsing SAML attributes' do
     let(:callback) { proc {} }
     let(:options) do
       { on_single_sign_out: callback }
@@ -33,19 +33,19 @@
         @rack_input = req.env['rack.input'].read
         true
       end
-      subject
+      call
     end
 
-    it 'are parsed and injected into the Rack Request parameters' do
+    it 'injects them into the Rack Request parameters' do
       expect(@rack_input).to eq 'name_id=%40NOT_USED%40&session_index=ST-123456-123abc456def'
     end
 
-    context 'that raise when parsed' do
+    context 'when an error is raised' do
       let(:env) { { 'rack.input' => nil } }
 
       before do
         allow(strategy).to receive(:fail!)
-        subject
+        call
       end
 
       it 'responds with an error' do
@@ -54,47 +54,47 @@
     end
   end
 
-  describe 'with a configured callback' do
+  context 'with a configured callback' do
     let(:options) do
       { on_single_sign_out: callback }
     end
 
-    let(:response_body) { subject[2].respond_to?(:body) ? subject[2].body : subject[2] }
+    let(:response_body) { call[2].respond_to?(:body) ? call[2].body : call[2] }
 
-    context 'that returns TRUE' do
+    context 'when callback returns `true`' do
       let(:callback) { proc { true } }
 
       it 'responds with OK' do
-        expect(subject[0]).to eq 200
+        expect(call[0]).to eq 200
         expect(response_body).to eq ['OK']
       end
     end
 
-    context 'that returns Nil' do
+    context 'when callback returns `nil`' do
       let(:callback) { proc {} }
 
       it 'responds with OK' do
-        expect(subject[0]).to eq 200
+        expect(call[0]).to eq 200
         expect(response_body).to eq ['OK']
       end
     end
 
-    context 'that returns a tuple' do
+    context 'when callback returns a tuple' do
       let(:callback) { proc { [400, {}, 'Bad Request'] } }
 
       it 'responds with OK' do
-        expect(subject[0]).to eq 400
+        expect(call[0]).to eq 400
         expect(response_body).to eq ['Bad Request']
       end
     end
 
-    context 'that raises an error' do
+    context 'when callback raises an error' do
       let(:exception) { RuntimeError.new('error') }
       let(:callback) { proc { raise exception } }
 
       before do
         allow(strategy).to receive(:fail!)
-        subject
+        call
       end
 
       it 'responds with an error' do

spec/omniauth/strategies/cas/service_ticket_validator_spec.rb

@@ -18,25 +18,25 @@
   end
 
   describe '#call' do
-    subject { validator.call }
+    subject(:call) { validator.call }
 
     before do
       stub_request(:get, 'https://example.org/serviceValidate?')
         .to_return(status: 200, body: '')
     end
 
     it 'returns itself' do
-      expect(subject).to eq validator
+      expect(call).to eq validator
     end
 
     it 'uses the configured CA path' do
-      subject
+      call
       expect(provider_options).to have_received :ca_path
     end
   end
 
   describe '#user_info' do
-    subject { validator.user_info }
+    subject(:user_info) { validator.user_info }
 
     let(:ok_fixture) do
       File.expand_path(File.join(File.dirname(__FILE__), '../../../fixtures/cas_success.xml'))
@@ -51,8 +51,8 @@
 
     context 'with default settings' do
       it 'parses user info from the response' do
-        expect(subject).to include 'user' => 'psegel'
-        expect(subject).to include 'roles' => 'financier'
+        expect(user_info).to include 'user' => 'psegel'
+        expect(user_info).to include 'roles' => 'financier'
       end
     end
 
@@ -65,8 +65,8 @@
       end
 
       it 'parses multivalued user info from the response' do
-        expect(subject).to include 'user' => 'psegel'
-        expect(subject).to include 'roles' => %w[senator lobbyist financier]
+        expect(user_info).to include 'user' => 'psegel'
+        expect(user_info).to include 'roles' => %w[senator lobbyist financier]
       end
     end
   end

spec/omniauth/strategies/cas_spec.rb

@@ -48,20 +48,20 @@
   end
 
   describe '#cas_url' do
-    subject { provider.cas_url }
+    subject(:cas_url) { provider.cas_url }
 
     let(:params) { {} }
     let(:provider) { MyCasProvider.new(nil, params) }
 
     it 'raises an ArgumentError' do
-      expect { subject }.to raise_error ArgumentError, /:host and :login_url MUST be provided/
+      expect { cas_url }.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 }
 
-      before { subject }
+      before { cas_url }
 
       it { is_expected.to eq url }
 
@@ -76,7 +76,7 @@
     context 'with explicit URL component' do
       let(:params) { super().merge host: 'example.org', port: 1234, ssl: true, path: '/a/path' }
 
-      before { subject }
+      before { cas_url }
 
       it { is_expected.to eq 'https://example.org:1234/a/path' }
 
@@ -124,7 +124,7 @@
       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'
+        expect(last_response.headers).to include 'Location' => '/auth/failure?message=no_ticket&strategy=cas'
       end
     end
 
@@ -140,11 +140,11 @@
       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'
+        expect(last_response.headers).to include 'Location' => '/auth/failure?message=invalid_ticket&strategy=cas'
       end
     end
 
-    describe 'with a valid ticket' do
+    context 'with a valid ticket' do
       shared_examples 'successful validation' do
         before do
           stub_request(:get, %r{^http://cas.example.org:8080?/serviceValidate\?([^&]+&)?ticket=593af})
@@ -166,7 +166,7 @@
                   })
         end
 
-        context "request.env['omniauth.auth']" do
+        describe "request.env['omniauth.auth']" do
           subject { last_request.env['omniauth.auth'] }
 
           it { is_expected.to be_a Hash }
@@ -179,7 +179,7 @@
             expect(subject.uid).to eq '54'
           end
 
-          context 'the info hash' do
+          describe "['info']" do
             subject { last_request.env['omniauth.auth']['info'] }
 
             it 'includes user info attributes' do
@@ -194,7 +194,7 @@
             end
           end
 
-          context 'the extra hash' do
+          describe "['extra']" do
             subject { last_request.env['omniauth.auth']['extra'] }
 
             it 'includes additional user attributes' do
@@ -225,7 +225,7 @@
             end
           end
 
-          context 'the credentials hash' do
+          describe "['credentials']" do
             subject { last_request.env['omniauth.auth']['credentials'] }
 
             it 'has a ticket value' do
@@ -255,17 +255,17 @@
     end
 
     describe 'with a Single Sign-Out logoutRequest' do
-      subject do
-        post 'auth/cas/callback', logoutRequest: logoutRequest
+      subject(:sso_logout_request) do
+        post 'auth/cas/callback', logoutRequest: logout_request_xml
       end
 
-      let(:logoutRequest) do
-        %(
+      let(:logout_request_xml) do
+        <<~XML
           <samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="123abc-1234-ab12-cd34-1234abcd" Version="2.0" IssueInstant="#{Time.now}">
             <saml:NameID>@NOT_USED@</saml:NameID>
             <samlp:SessionIndex>ST-123456-123abc456def</samlp:SessionIndex>
           </samlp:LogoutRequest>
-        )
+        XML
       end
 
       let(:logout_request) { double('logout_request', call: [200, {}, 'OK']) }
@@ -275,7 +275,7 @@
           .to receive(:logout_request_service)
           .and_return double('LogoutRequest', new: logout_request)
 
-        subject
+        sso_logout_request
       end
 
       it 'initializes a LogoutRequest' do