don't allow SAML SLO requests to go to non-SAML endpoints

our SP metadata explicitly says they should only go to /saml_logout
via a GET request. so don't even allow a POST or DELETE to /saml_logout

Change-Id: I5543f043b618801b780a71ed3257637cc49b93ff
Reviewed-on: https://gerrit.instructure.com/51295
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Jenkins
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>

app/controllers/pseudonym_sessions_controller.rb

@@ -17,7 +17,7 @@
 #
 
 class PseudonymSessionsController < ApplicationController
-  protect_from_forgery :except => [:create, :destroy, :saml_logout, :saml_consume, :oauth2_token, :oauth2_logout, :cas_logout]
+  protect_from_forgery :except => [:create, :saml_logout, :saml_consume, :oauth2_token, :oauth2_logout, :cas_logout]
   before_filter :forbid_on_files_domain, :except => [ :clear_file_session ]
   before_filter :require_password_session, :only => [ :otp_login, :disable_otp_login ]
   before_filter :require_user, :only => [ :otp_login ]
@@ -218,25 +218,7 @@ class PseudonymSessionsController < ApplicationController
 
   # DELETE /logout
   def destroy
-    # Only allow DELETE method for all logout requests except for SAML.
-    if saml_response? || saml_request?
-      saml_logout
-    else
-      # We can't verify the authenticity token for saml, so we do it in
-      # this branch rather than in the before filter.
-      #
-      # This also allows us to show the logout confirmation screen rather than
-      # an error if the token is invalid -- this can happen, for example, if
-      # they log out and back in on a 2nd tab, and then click logout on the 1st
-      # tab.
-      begin
-        return unless verify_authenticity_token
-      rescue ActionController::InvalidAuthenticityToken
-        return redirect_to(logout_url)
-      end
-
-      logout_user_action
-    end
+    logout_user_action
   end
 
   # GET /logout
@@ -477,7 +459,7 @@ class PseudonymSessionsController < ApplicationController
     end
   end
 
-  # POST /logout
+  # GET /saml_logout
   def saml_logout
     if saml_response?
       increment_saml_stat("logout_response_received")

config/routes.rb

@@ -581,7 +581,6 @@ CanvasRails::Application.routes.draw do
   get 'login' => 'pseudonym_sessions#new'
   post 'login' => 'pseudonym_sessions#create'
   delete 'logout' => 'pseudonym_sessions#destroy'
-  post 'logout' => 'pseudonym_sessions#saml_logout'
   get 'logout' => 'pseudonym_sessions#logout_confirm'
   get 'login/cas' => 'pseudonym_sessions#new', as: :cas_login
   post 'login/cas' => 'pseudonym_sessions#cas_logout', as: :cas_logout
@@ -710,7 +709,7 @@ CanvasRails::Application.routes.draw do
 
   post 'object_snippet' => 'context#object_snippet'
   post 'saml_consume' => 'pseudonym_sessions#saml_consume'
-  match 'saml_logout' => 'pseudonym_sessions#saml_logout', via: [:get, :post, :delete]
+  get 'saml_logout' => 'pseudonym_sessions#saml_logout'
   get 'saml_meta_data' => 'accounts#saml_meta_data'
 
   # Routes for course exports

spec/controllers/pseudonym_sessions_controller_spec.rb

@@ -575,11 +575,9 @@ describe PseudonymSessionsController do
     end
 
     context "/logout" do
-      it "should redirect to logout confirmation if the authenticity token is invalid" do
-        controller.expects(:verify_authenticity_token).raises(ActionController::InvalidAuthenticityToken)
+      it "should not logout if the authenticity token is invalid" do
         delete 'destroy'
-        expect(response).to be_redirect
-        expect(response['Location']).to match %r{/logout}
+        expect(response).to_not be_success
       end
     end