Refactor hooks and modify related tests

auth.php

@@ -71,7 +71,9 @@ class auth_plugin_userkey extends auth_plugin_base {
     }
 
     /**
-     * All the checking happens before the login page in this hook
+     * All the checking happens before the login page in this hook.
+     *
+     * It redirects a user if required or return true.
      */
     public function pre_loginpage_hook() {
         global $SESSION;
@@ -82,16 +84,36 @@ class auth_plugin_userkey extends auth_plugin_base {
         if (isset($SESSION->enrolkey_skipsso)) {
             unset($SESSION->enrolkey_skipsso);
         }
-        $this->loginpage_hook();
+
+        return $this->loginpage_hook();
     }
 
     /**
-     * All the checking happens before the login page in this hook
+     * All the checking happens before the login page in this hook.
+     *
+     * It redirects a user if required or return true.
      */
     public function loginpage_hook() {
         if ($this->should_login_redirect()) {
-            redirect($this->config->ssourl);
+            $this->redirect($this->config->ssourl);
         }
+
+        return true;
+    }
+
+    /**
+     * Redirects the user to provided URL.
+     *
+     * @param $url URL to redirect to.
+     *
+     * @throws \moodle_exception If gets running via CLI or AJAX call.
+     */
+    protected function redirect($url) {
+        if (CLI_SCRIPT or AJAX_SCRIPT) {
+            throw new moodle_exception('redirecterrordetected', 'auth_userkey', '', $url);
+        }
+
+        redirect($url);
     }
 
     /**
@@ -505,8 +527,9 @@ class auth_plugin_userkey extends auth_plugin_base {
      *
      * @return bool
      */
-    public function should_login_redirect() {
+    protected function should_login_redirect() {
         global $SESSION;
+
         $skipsso = optional_param('enrolkey_skipsso', 0, PARAM_BOOL);
 
         // Check whether we've skipped SSO already.

lang/en/auth_userkey.php

@@ -42,3 +42,4 @@ $string['userkey:generatekey'] = 'Generate login user key';
 $string['pluginisdisabled'] = 'The userkey authentication plugin is disabled.';
 $string['ssourl'] = 'URL of SSO host';
 $string['ssourl_desc'] = 'URL of the SSO host to redirect users to. If defined users will be redirected here on login instead of the Moodle Login page';
+$string['redirecterrordetected'] = 'Unsupported redirect to {$a} detected, execution terminated.';

tests/auth_plugin_test.php

@@ -740,60 +740,87 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
     }
 
     /**
-     * Test that login page hook redirects correctly.
+     * Test that login hook redirects a user if skipsso not set and ssourl is set.
+     *
+     * @expectedException moodle_exception
+     * @expectedExceptionMessage Unsupported redirect to http://google.com detected, execution terminated.
      */
-    public function test_loginpage_hook_redirects_correctly() {
+    public function test_loginpage_hook_redirects_if_skipsso_not_set_and_ssourl_set() {
         global $SESSION;
 
         $SESSION->enrolkey_skipsso = 0;
         set_config('ssourl', 'http://google.com', 'auth_userkey');
         $this->auth = new auth_plugin_userkey();
 
-        $userredirect = $this->auth->should_login_redirect();
+        $this->auth->loginpage_hook();
-        $this->assertEquals($userredirect, true);
-
     }
 
     /**
-     * Test that Moodle login page is displayed if url param is set correctly.
+     * Test that login hook does not redirect a user if skipsso not set and ssourl is not set.
      */
-    public function test_login_page_displays_correctly_url_param_set() {
+    public function test_loginpage_hook_does_not_redirect_if_skipsso_not_set_and_ssourl_not_set() {
-        global $SESSION;
-
-        $SESSION->enrolkey_skipsso = 1;
-        set_config('ssourl', 'http://google.com', 'auth_userkey');
-        $this->auth = new auth_plugin_userkey();
-        $userredirect = $this->auth->should_login_redirect();
-        $this->assertEquals($userredirect, false);
-
-    }
-
-    /**
-     * Test that Moodle login page is displayed if no redirect url and no param is set.
-     */
-    public function test_login_page_displays_correctly() {
         global $SESSION;
 
         $SESSION->enrolkey_skipsso = 0;
         set_config('ssourl', '', 'auth_userkey');
         $this->auth = new auth_plugin_userkey();
-        $userredirect = $this->auth->should_login_redirect();
-        $this->assertEquals($userredirect, false);
 
+        $this->assertTrue($this->auth->loginpage_hook());
     }
 
     /**
-     * Test that Moodle login page is displayed if no redirect url, but param is set.
+     * Test that login hook does not redirect a user if skipsso is set and ssourl is not set.
      */
-    public function test_login_page_displays_correctly_param_set() {
+    public function test_loginpage_hook_does_not_redirect_if_skipsso_set_and_ssourl_not_set() {
         global $SESSION;
 
         $SESSION->enrolkey_skipsso = 1;
         set_config('ssourl', '', 'auth_userkey');
         $this->auth = new auth_plugin_userkey();
-        $userredirect = $this->auth->should_login_redirect();
-        $this->assertEquals($userredirect, false);
 
+        $this->assertTrue($this->auth->loginpage_hook());
+    }
+
+    /**
+     * Test that pre login hook redirects a user if skipsso not set and ssourl is set.
+     *
+     * @expectedException moodle_exception
+     * @expectedExceptionMessage Unsupported redirect to http://google.com detected, execution terminated.
+     */
+    public function test_pre_loginpage_hook_redirects_if_skipsso_not_set_and_ssourl_set() {
+        global $SESSION;
+
+        $SESSION->enrolkey_skipsso = 0;
+        set_config('ssourl', 'http://google.com', 'auth_userkey');
+        $this->auth = new auth_plugin_userkey();
+
+        $this->auth->pre_loginpage_hook();
+    }
+
+    /**
+     * Test that pre login hook does not redirect a user if skipsso is not set and ssourl is not set.
+     */
+    public function test_pre_loginpage_hook_does_not_redirect_if_skipsso_not_set_and_ssourl_not_set() {
+        global $SESSION;
+
+        $SESSION->enrolkey_skipsso = 0;
+        set_config('ssourl', '', 'auth_userkey');
+        $this->auth = new auth_plugin_userkey();
+
+        $this->assertTrue($this->auth->pre_loginpage_hook());
+    }
+
+    /**
+     * Test that login page hook does not redirect a user if skipsso is set and ssourl is not set.
+     */
+    public function test_pre_loginpage_hook_does_not_redirect_if_skipsso_set_and_ssourl_not_set() {
+        global $SESSION;
+
+        $SESSION->enrolkey_skipsso = 1;
+        set_config('ssourl', '', 'auth_userkey');
+        $this->auth = new auth_plugin_userkey();
+
+        $this->assertTrue($this->auth->pre_loginpage_hook());
     }
 
 }