CORE-359 refactoring session variables

ryarger · ryarger · commit bc93eec0f4f6 · 2011-06-21T16:12:02.000-05:00
diff --git a/lib/casclient/client.rb b/lib/casclient/client.rb
@@ -102,7 +102,13 @@ def validate_service_ticket(st)
       h['pgtUrl'] = proxy_callback_url if proxy_callback_url
       uri.query = hash_to_query(h)
       
-      st.response = request_cas_response(uri, ValidationResponse)
+      response = request_cas_response(uri, ValidationResponse)
+      st.user = response.user
+      st.extra_attributes = response.extra_attributes
+      st.pgt_iou = response.pgt_iou
+      st.success = response.is_success?
+      st.failure_code = response.failure_code
+      st.failure_message = response.failure_message
       
       return st
     end
@@ -183,10 +189,15 @@ def request_proxy_ticket(pgt, target_service)
       h['targetService'] = target_service
       uri.query = hash_to_query(h)
       
-      pr = request_cas_response(uri, ProxyResponse)
+      response = request_cas_response(uri, ProxyResponse)
       
-      pt = ProxyTicket.new(pr.proxy_ticket, target_service)
-      pt.response = pr
+      pt = ProxyTicket.new(response.proxy_ticket, target_service)
+      pt.user = response.user
+      pt.extra_attributes = response.extra_attributes
+      pt.pgt_iou = response.pgt_iou
+      pt.success = response.is_success?
+      pt.failure_code = response.failure_code
+      pt.failure_message = response.failure_message
       
       return pt
     end
diff --git a/lib/casclient/frameworks/rails/filter.rb b/lib/casclient/frameworks/rails/filter.rb
@@ -23,6 +23,7 @@ def filter(controller)
             end
             
             last_st = controller.session[:cas_last_valid_ticket]
+            last_st_service = controller.session[:cas_last_valid_ticket_service]
             
             if single_sign_out(controller)
               controller.send(:render, :text => "CAS Single-Sign-Out request intercepted.")
@@ -31,17 +32,17 @@ def filter(controller)
 
             st = read_ticket(controller)
             
-            is_new_session = true
+            #is_new_session = true
             
             if st && last_st && 
-                last_st.ticket == st.ticket && 
-                last_st.service == st.service
+                last_st == st.ticket && 
+                last_st_service == st.service
               # warn() rather than info() because we really shouldn't be re-validating the same ticket. 
               # The only situation where this is acceptable is if the user manually does a refresh and 
               # the same ticket happens to be in the URL.
               log.warn("Re-using previously validated ticket since the ticket id and service are the same.")
-              st = last_st
-              is_new_session = false
+              #st = last_st
+              return true
             elsif last_st &&
                 !config[:authenticate_on_every_request] && 
                 controller.session[client.username_session_key]
@@ -54,61 +55,70 @@ def filter(controller)
               # it will almost certainly break POST request, AJAX calls, etc.
               log.debug "Existing local CAS session detected for #{controller.session[client.username_session_key].inspect}. "+
                 "Previous ticket #{last_st.ticket.inspect} will be re-used."
-              st = last_st
-              is_new_session = false
+              #st = last_st
+              return true
             end
             
             if st
               client.validate_service_ticket(st) unless st.has_been_validated?
-              vr = st.response
               
               if st.is_valid?
-                if is_new_session
-                  log.info("Ticket #{st.ticket.inspect} for service #{st.service.inspect} belonging to user #{vr.user.inspect} is VALID.")
-                  controller.session[client.username_session_key] = vr.user.dup
-                  controller.session[client.extra_attributes_session_key] = HashWithIndifferentAccess.new(vr.extra_attributes) if vr.extra_attributes
+                #if is_new_session
+                  log.info("Ticket #{st.ticket.inspect} for service #{st.service.inspect} belonging to user #{st.user.inspect} is VALID.")
+                  controller.session[client.username_session_key] = st.user.dup
+                  controller.session[client.extra_attributes_session_key] = HashWithIndifferentAccess.new(st.extra_attributes) if st.extra_attributes
                   
-                  if vr.extra_attributes
-                    log.debug("Extra user attributes provided along with ticket #{st.ticket.inspect}: #{vr.extra_attributes.inspect}.")
+                  if st.extra_attributes
+                    log.debug("Extra user attributes provided along with ticket #{st.ticket.inspect}: #{st.extra_attributes.inspect}.")
                   end
                   
                   # RubyCAS-Client 1.x used :casfilteruser as it's username session key,
                   # so we need to set this here to ensure compatibility with configurations
                   # built around the old client.
-                  controller.session[:casfilteruser] = vr.user
+                  controller.session[:casfilteruser] = st.user
                   
                   if config[:enable_single_sign_out]
-                    @@client.ticket_store.store_service_session_lookup(st, controller)
+                    client.ticket_store.store_service_session_lookup(st, controller)
                   end
-                end
+                #end
               
                 # Store the ticket in the session to avoid re-validating the same service
                 # ticket with the CAS server.
-                controller.session[:cas_last_valid_ticket] = st
+                controller.session[:cas_last_valid_ticket] = st.ticket
+                controller.session[:cas_last_valid_ticket_service] = st.service
                 
-                if vr.pgt_iou
-                  unless controller.session[:cas_pgt] && controller.session[:cas_pgt].ticket && controller.session[:cas_pgt].iou == vr.pgt_iou
+                if st.pgt_iou
+                  unless controller.session[:cas_pgt] && controller.session[:cas_pgt].ticket && controller.session[:cas_pgt].iou == st.pgt_iou
                     log.info("Receipt has a proxy-granting ticket IOU. Attempting to retrieve the proxy-granting ticket...")
-                    pgt = client.retrieve_proxy_granting_ticket(vr.pgt_iou)
+                    pgt = client.retrieve_proxy_granting_ticket(st.pgt_iou)
 
                     if pgt
                       log.debug("Got PGT #{pgt.ticket.inspect} for PGT IOU #{pgt.iou.inspect}. This will be stored in the session.")
                       controller.session[:cas_pgt] = pgt
                       # For backwards compatibility with RubyCAS-Client 1.x configurations...
                       controller.session[:casfilterpgt] = pgt
                     else
-                      log.error("Failed to retrieve a PGT for PGT IOU #{vr.pgt_iou}!")
+                      log.error("Failed to retrieve a PGT for PGT IOU #{st.pgt_iou}!")
                     end
                   else
-                    log.info("PGT is present in session and PGT IOU #{vr.pgt_iou} matches the saved PGT IOU.  Not retrieving new PGT.")
+                    log.info("PGT is present in session and PGT IOU #{st.pgt_iou} matches the saved PGT IOU.  Not retrieving new PGT.")
                   end
 
                 end
                 
+                log.debug '################'
+                log.debug '################'
+                controller.session.each do |v|
+                  log.debug v.inspect
+                end
+                log.debug '################'
+                log.debug '################'
+              
+                
                 return true
               else
-                log.warn("Ticket #{st.ticket.inspect} failed validation -- #{vr.failure_code}: #{vr.failure_message}")
-                unauthorized!(controller, vr)
+                log.warn("Ticket #{st.ticket.inspect} failed validation -- #{st.failure_code}: #{st.failure_message}")
+                unauthorized!(controller, st)
                 return false
               end
             else # no service ticket was present in the request
diff --git a/lib/casclient/tickets.rb b/lib/casclient/tickets.rb
@@ -2,7 +2,7 @@ module CASClient
   # Represents a CAS service ticket.
   class ServiceTicket
     attr_reader :ticket, :service, :renew
-    attr_accessor :response
+    attr_accessor :user, :extra_attributes, :pgt_iou, :success, :failure_code, :failure_message
     
     def initialize(ticket, service, renew = false)
       @ticket = ticket
@@ -11,11 +11,11 @@ def initialize(ticket, service, renew = false)
     end
     
     def is_valid?
-      response.is_success?
+      success
     end
     
     def has_been_validated?
-      not response.nil?
+      not user.nil?
     end
   end
   
diff --git a/test/units/casclient/frameworks/rails/filter_test.rb b/test/units/casclient/frameworks/rails/filter_test.rb
@@ -20,6 +20,26 @@
     )
   end
   
+  context "fake user without attributes" do
+    setup { CASClient::Frameworks::Rails::Filter.fake('tester@test.com') }
+    should 'set the session user on #filter' do
+      setup { Hash.new }
+      CASClient::Frameworks::Rails::Filter.filter(controller_with_session(topic,nil))
+      topic
+    end.equals :cas_user => 'tester@test.com', :casfilteruser => 'tester@test.com'
+    teardown { CASClient::Frameworks::Rails::Filter.fake(nil,nil) }
+  end
+  
+  context "fake user with attributes" do
+    setup { CASClient::Frameworks::Rails::Filter.fake('tester@test.com', {:test => 'stuff', :this => 'that'}) }
+    should 'set the session user and attributes on #filter' do
+      setup { Hash.new }
+      CASClient::Frameworks::Rails::Filter.filter(controller_with_session(topic,nil))
+      topic
+    end.equals :cas_user => 'tester@test.com', :casfilteruser => 'tester@test.com', :cas_extra_attributes => {:test => 'stuff', :this => 'that' }
+    teardown { CASClient::Frameworks::Rails::Filter.fake(nil,nil) }
+  end
+  
   context "new service ticket successfully" do
      should("return successfully from filter") do
       setup { Hash.new }
@@ -150,42 +170,15 @@
       mock_request = ActionController::Request.new({})
       mock(mock_request).post? {true}
       
-      pgt = CASClient::ProxyGrantingTicket.new(
-      "PGT-1308586001r9573FAD5A8C62E134A4AA93273F226BD3F0C3A983DCCCD176",
-      "PGTIOU-1308586001r29DC1F852C95930FE6694C1EFC64232A3359798893BC0B")
-      
-      raw_text = "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">
-                    <cas:authenticationSuccess>
-                      <cas:user>rich.yarger@vibes.com</cas:user>
-                      <cas:proxyGrantingTicket>PGTIOU-1308586001r29DC1F852C95930FE6694C1EFC64232A3359798893BC0B</cas:proxyGrantingTicket>
-                    </cas:authenticationSuccess>
-                  </cas:serviceResponse>"
-      response = CASClient::ValidationResponse.new(raw_text)
-      
-      any_instance_of(CASClient::Client, :request_cas_response => response)
-      any_instance_of(CASClient::Client, :retrieve_proxy_granting_ticket => pgt)
+      mock_client = CASClient::Client.new()
+      mock(mock_client).request_cas_response().never
+      mock(mock_client).retrieve_proxy_granting_ticket().never
+      CASClient::Frameworks::Rails::Filter.send(:class_variable_set, :@@client, mock_client)
       
-      topic[:cas_last_valid_ticket] = CASClient::ServiceTicket.new("bogusticket",'bogusurl')
+      topic[:cas_last_valid_ticket] = 'bogusticket'
+      topic[:cas_last_valid_ticket_service] = 'bogusurl'
       controller = controller_with_session(topic,mock_request)
       CASClient::Frameworks::Rails::Filter.filter(controller)
      end.equals(true)
   end
-  
-  context "fake user without attributes" do
-    setup { CASClient::Frameworks::Rails::Filter.fake('tester@test.com') }
-    should 'set the session user on #filter' do
-      setup { Hash.new }
-      CASClient::Frameworks::Rails::Filter.filter(controller_with_session(topic,nil))
-      topic
-    end.equals :cas_user => 'tester@test.com', :casfilteruser => 'tester@test.com'
-  end
-  
-  context "fake user with attributes" do
-    setup { CASClient::Frameworks::Rails::Filter.fake('tester@test.com', {:test => 'stuff', :this => 'that'}) }
-    should 'set the session user and attributes on #filter' do
-      setup { Hash.new }
-      CASClient::Frameworks::Rails::Filter.filter(controller_with_session(topic,nil))
-      topic
-    end.equals :cas_user => 'tester@test.com', :casfilteruser => 'tester@test.com', :cas_extra_attributes => {:test => 'stuff', :this => 'that' }
-  end
 end
