refresh CAS ticket in find_user

fixes: CNVS-16438

Found an issue where the refresh_cas_ticket method was being called
before find_user which will fail since @pseudonym_session would not have
been populated yet.  This removes that method from application_controller
and adds the refresh to the CAS expired check.

Test Plan:

  - Setup CAS with Canvas
  - Log in with CAS and take note of the ticket that was used.
  - Open up redis and find a record with a key of cas_ticket:#{ticket}
    from the previous step.
  - The note the ttl of the record in redis.
  - Refresh the page and the ttl should have been extended.
  - Do the same for an ajax call.  The ttl should have been extended.

Change-Id: Ieb25f05765fed2cfb3de00ed6a49966fd3d04698
Reviewed-on: https://gerrit.instructure.com/44062
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Nick Cloward <ncloward@instructure.com>

app/controllers/application_controller.rb

@@ -41,7 +41,6 @@ class ApplicationController < ActionController::Base
   protect_from_forgery
   # load_user checks masquerading permissions, so this needs to be cleared first
   before_filter :clear_cached_contexts
-  before_filter :refresh_cas_ticket
   before_filter :load_account, :load_user
   before_filter ::Filters::AllowAppProfiling
   before_filter :check_pending_otp
@@ -797,12 +796,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def refresh_cas_ticket
-    if session[:cas_session] && @current_pseudonym
-      @current_pseudonym.claim_cas_ticket(session[:cas_session])
-    end
-  end
-
   def require_reacceptance_of_terms
     if session[:require_terms] && !api_request? && request.get?
       render :template => "shared/terms_required", :layout => "application", :status => :unauthorized

app/models/pseudonym.rb

@@ -30,6 +30,7 @@ class Pseudonym < ActiveRecord::Base
   MAX_UNIQUE_ID_LENGTH = 100
 
   CAS_TICKET_EXPIRED = 'expired'
+  CAS_TICKET_TTL = 1.day
 
   EXPORTABLE_ATTRIBUTES = [
     :id, :user_id, :account_id, :workflow_state, :unique_id, :login_count, :failed_login_count, :last_request_at, :last_login_at, :current_login_at, :last_login_ip,
@@ -515,32 +516,39 @@ class Pseudonym < ActiveRecord::Base
     nil
   end
 
+  def self.cas_ticket_key(ticket)
+    "cas_session:#{ticket}"
+  end
+
   def claim_cas_ticket(ticket)
     return unless Canvas.redis_enabled?
 
-    redis_key = "cas_session:#{ticket}"
-    ttl = 1.day
+    redis_key = Pseudonym.cas_ticket_key(ticket)
 
     # Refresh the keys ttl if it exists.
-    unless Canvas.redis.expire(redis_key, ttl)
+    unless Canvas.redis.expire(redis_key, CAS_TICKET_TTL)
       # If it does not exist we need to create it.
-      Canvas.redis.set(redis_key, global_id, ex: ttl, nx: true)
+      Canvas.redis.set(redis_key, global_id, ex: CAS_TICKET_TTL, nx: true)
     end
   end
 
   def cas_ticket_expired?(ticket)
     return unless Canvas.redis_enabled?
-    Canvas.redis.get("cas_session:#{ticket}") != global_id.to_s
+    redis_key = Pseudonym.cas_ticket_key(ticket)
+
+    # Refresh the ttl on the cas ticket before we check its state.
+    Canvas.redis.expire(redis_key, CAS_TICKET_TTL)
+    Canvas.redis.get(redis_key) != global_id.to_s
   end
 
   def self.expire_cas_ticket(ticket)
     return unless Canvas.redis_enabled?
-    redis_key = "cas_session:#{ticket}"
+    redis_key = cas_ticket_key(ticket)
 
-    if id = Canvas.redis.getset(redis_key, Pseudonym::CAS_TICKET_EXPIRED)
-      Canvas.redis.expire(redis_key, 1.day)
+    if id = Canvas.redis.getset(redis_key, CAS_TICKET_EXPIRED)
+      Canvas.redis.expire(redis_key, CAS_TICKET_TTL)
 
-      Pseudonym.where(id: id).exists? if id != Pseudonym::CAS_TICKET_EXPIRED
+      Pseudonym.where(id: id).exists? if id != CAS_TICKET_EXPIRED
     end
   end
 end

spec/integration/pseudonym_sessions_controller_spec.rb

@@ -131,6 +131,25 @@ describe PseudonymSessionsController do
       expect(session[:cas_session]).to eq 'ST-abcd'
     end
 
+    it "should refresh the users CAS ticket on a request" do
+      user = user_with_pseudonym({:active_all => true})
+      cas_ticket = 'ST-abcd'
+      cas_redis_key = "cas_session:#{cas_ticket}"
+
+      stubby("yes\n#{user.pseudonyms.first.unique_id.capitalize}\n")
+
+      get login_url
+      redirect_until(@cas_client.add_service_to_login_url(cas_login_url))
+
+      get cas_login_url :ticket => 'ST-abcd'
+      expect(response).to redirect_to(dashboard_url(:login_success => 1))
+      expect(session[:cas_session]).to eq cas_ticket
+
+      cas_ticket_ttl = Canvas::redis.ttl(cas_redis_key)
+      get dashboard_url(:login_success => 1)
+      expect(Canvas::redis.ttl(cas_redis_key)).to be > cas_ticket_ttl
+    end
+
     context "single sign out" do
       before do
         skip "needs redis" unless Canvas.redis_enabled?