Merge pull request #12 from ninech/support-jasig-style-extra-attributes

Support JASIG style extra attributes
dlindahl · Jul 10, 2013 · 0082b58 · 0082b58
2 parents 9af0eb5 + 8de378c
commit 0082b58
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 51 deletions.
diff --git a/lib/omniauth/strategies/cas/service_ticket_validator.rb b/lib/omniauth/strategies/cas/service_ticket_validator.rb
@@ -41,15 +41,20 @@ def parse_user_info(node)
 
           {}.tap do |hash|
             node.children.each do |e|
+              node_name = e.name.sub(/^cas:/, '')
               unless e.kind_of?(Nokogiri::XML::Text) ||
-                     e.name == 'cas:proxies' ||
-                     e.name == 'proxies'
+                     node_name == 'proxies'
                 # There are no child elements
                 if e.element_children.count == 0
-                  hash[e.name.sub(/^cas:/, '')] = e.content
+                  hash[node_name] = e.content
                 elsif e.element_children.count
-                  hash[e.name.sub(/^cas:/, '')] = [] if hash[e.name.sub(/^cas:/, '')].nil?
-                  hash[e.name.sub(/^cas:/, '')].push parse_user_info e
+                  # JASIG style extra attributes
+                  if node_name == 'attributes'
+                    hash.merge! parse_user_info e
+                  else
+                    hash[node_name] = [] if hash[node_name].nil?
+                    hash[node_name].push parse_user_info e
+                  end
                 end
               end
             end

diff --git a/spec/fixtures/cas_success_jasig.xml b/spec/fixtures/cas_success_jasig.xml
@@ -0,0 +1,16 @@
+<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>
+  <cas:authenticationSuccess>
+    <cas:user>psegel</cas:user>
+    <cas:attributes>
+      <cas:employeeid>54</cas:employeeid>
+      <cas:first_name>P. Segel</cas:first_name>
+      <cas:first_name>Peter</cas:first_name>
+      <cas:last_name>Segel</cas:last_name>
+      <cas:email>psegel@intridea.com</cas:email>
+      <cas:location>Washington, D.C.</cas:location>
+      <cas:image>/images/user.jpg</cas:image>
+      <cas:phone>555-555-5555</cas:phone>
+      <cas:hire_date>2004-07-13</cas:hire_date>
+    </cas:attributes>
+  </cas:authenticationSuccess>
+</cas:serviceResponse>
diff --git a/spec/omniauth/strategies/cas_spec.rb b/spec/omniauth/strategies/cas_spec.rb
@@ -81,72 +81,84 @@ def app
   end
 
   describe 'GET /auth/cas/callback with a valid ticket' do
-    let(:return_url) { 'http://127.0.0.10/?some=parameter' }
+    shared_examples :successful_validation do
+      before do
+        stub_request(:get, /^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}") )
 
-    before do
-      stub_request(:get, /^http:\/\/cas.example.org:8080?\/serviceValidate\?([^&]+&)?ticket=593af/)
-        .with { |request| @request_uri = request.uri.to_s }
-        .to_return( body: File.read('spec/fixtures/cas_success.xml') )
+        get "/auth/cas/callback?ticket=593af&url=#{return_url}"
+      end
 
-      get "/auth/cas/callback?ticket=593af&url=#{return_url}"
-    end
+      it 'should strip the ticket parameter from the callback URL' do
+        @request_uri.scan('ticket=').length.should == 1
+      end
 
-    it 'should strip the ticket parameter from the callback URL' do
-      @request_uri.scan('ticket=').length.should == 1
-    end
+      it 'should properly encode the service URL' do
+        WebMock.should 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')
+          })
+      end
 
-    it 'should properly encode the service URL' do
-      WebMock.should 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')
-        })
-    end
+      context "request.env['omniauth.auth']" do
+        subject { last_request.env['omniauth.auth'] }
 
-    context "request.env['omniauth.auth']" do
-      subject { last_request.env['omniauth.auth'] }
+        it { should be_kind_of Hash }
 
-      it { should be_kind_of Hash }
+        its(:provider) { should == :cas }
 
-      its(:provider) { should == :cas }
+        its(:uid) { should == '54'}
 
-      its(:uid) { should == '54'}
+        context 'the info hash' do
+          subject { last_request.env['omniauth.auth']['info'] }
 
-      context 'the info hash' do
-        subject { last_request.env['omniauth.auth']['info'] }
+          it { should have(6).items }
 
-        it { should have(6).items }
+          its(:name)       { should == 'Peter Segel' }
+          its(:first_name) { should == 'Peter' }
+          its(:last_name)  { should == 'Segel' }
+          its(:email)      { should == 'psegel@intridea.com' }
+          its(:location)   { should == 'Washington, D.C.' }
+          its(:image)      { should == '/images/user.jpg' }
+          its(:phone)      { should == '555-555-5555' }
+        end
 
-        its(:name)       { should == 'Peter Segel' }
-        its(:first_name) { should == 'Peter' }
-        its(:last_name)  { should == 'Segel' }
-        its(:email)      { should == 'psegel@intridea.com' }
-        its(:location)   { should == 'Washington, D.C.' }
-        its(:image)      { should == '/images/user.jpg' }
-        its(:phone)      { should == '555-555-5555' }
-      end
+        context 'the extra hash' do
+          subject { last_request.env['omniauth.auth']['extra'] }
 
-      context 'the extra hash' do
-        subject { last_request.env['omniauth.auth']['extra'] }
+          it { should have(3).items }
 
-        it { should have(3).items }
+          its(:user)       { should == 'psegel' }
+          its(:employeeid) { should == '54' }
+          its(:hire_date)  { should == '2004-07-13' }
+        end
 
-        its(:user)       { should == 'psegel' }
-        its(:employeeid) { should == '54' }
-        its(:hire_date)  { should == '2004-07-13' }
-      end
+        context 'the credentials hash' do
+          subject { last_request.env['omniauth.auth']['credentials'] }
 
-      context 'the credentials hash' do
-        subject { last_request.env['omniauth.auth']['credentials'] }
+          it { should have(1).items }
 
-        it { should have(1).items }
+          its(:ticket) { should == '593af' }
+        end
+      end
 
-        its(:ticket) { should == '593af' }
+      it 'should call through to the master app' do
+        last_response.body.should == 'true'
       end
     end
 
-    it 'should call through to the master app' do
-      last_response.body.should == 'true'
+    let(:return_url) { 'http://127.0.0.10/?some=parameter' }
+
+    context 'with JASIG flavored XML' do
+      let(:xml_file_name) { 'cas_success_jasig.xml' }
+      it_behaves_like :successful_validation
+    end
+
+    context 'with classic XML' do
+      let(:xml_file_name) { 'cas_success.xml' }
+      it_behaves_like :successful_validation
     end
   end