Merge pull request #8 from CatalystIT-AU/sso_redirect

SSO redirect

auth.php

@@ -57,6 +57,7 @@ class auth_plugin_userkey extends auth_plugin_base {
         'keylifetime' => 60,
         'iprestriction' => 0,
         'redirecturl' => '',
+        'ssourl' => '',
         // TODO: use this field when implementing user creation. 'createuser' => 0.
     );
 
@@ -69,6 +70,52 @@ class auth_plugin_userkey extends auth_plugin_base {
         $this->userkeymanager = new core_userkey_manager($this->config);
     }
 
+    /**
+     * 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;
+
+        // If we previously tried to skip SSO on, but then navigated
+        // away, and come in from another deep link while SSO only is
+        // on, then reset the previous session memory of forcing SSO.
+        if (isset($SESSION->enrolkey_skipsso)) {
+            unset($SESSION->enrolkey_skipsso);
+        }
+
+        return $this->loginpage_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()) {
+            $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);
+    }
+
     /**
      * Don't allow login using login form.
      *
@@ -82,9 +129,7 @@ class auth_plugin_userkey extends auth_plugin_base {
     }
 
     /**
-     * Login user using userkey and return URL to redirect after.
+     * Logs a user in using userkey and redirects after.
-     *
-     * @return string URL to redirect.
      *
      * @throws \moodle_exception If something went wrong.
      */
@@ -104,10 +149,12 @@ class auth_plugin_userkey extends auth_plugin_base {
         $SESSION->userkey = true;
 
         if (!empty($wantsurl)) {
-            return $wantsurl;
+            $redirecturl = $wantsurl;
         } else {
-            return $CFG->wwwroot;
+            $redirecturl = $CFG->wwwroot;
         }
+
+        $this->redirect($redirecturl);
     }
 
     /**
@@ -168,9 +215,37 @@ class auth_plugin_userkey extends auth_plugin_base {
             $err['keylifetime'] = get_string('incorrectkeylifetime', 'auth_userkey');
         }
 
-        if (!empty($form->redirecturl) && filter_var($form->redirecturl, FILTER_VALIDATE_URL) === false) {
+        if (!$this->is_valid_url($form->redirecturl)) {
             $err['redirecturl'] = get_string('incorrectredirecturl', 'auth_userkey');
         }
+
+        if (!$this->is_valid_url($form->ssourl)) {
+            $err['ssourl'] = get_string('incorrectssourl', 'auth_userkey');
+        }
+
+    }
+
+    /**
+     * Check if provided url is correct.
+     *
+     * @param string $url URL to check.
+     *
+     * @return bool
+     */
+    protected function is_valid_url($url) {
+        if (empty($url)) {
+            return true;
+        }
+
+        if (filter_var($url, FILTER_VALIDATE_URL) === false) {
+            return false;
+        }
+
+        if (!preg_match("/^(http|https):/", $url)) {
+            return false;
+        }
+
+        return true;
     }
 
     /**
@@ -447,12 +522,40 @@ class auth_plugin_userkey extends auth_plugin_base {
         return $parameters;
     }
 
+    /**
+     * Check if we should redirect a user as part of login.
+     *
+     * @return bool
+     */
+    protected function should_login_redirect() {
+        global $SESSION;
+
+        $skipsso = optional_param('enrolkey_skipsso', 0, PARAM_BOOL);
+
+        // Check whether we've skipped SSO already.
+        // This is here because loginpage_hook is called again during form
+        // submission (all of login.php is processed) and ?skipsso=on is not
+        // preserved forcing us to the SSO.
+        if ((isset($SESSION->enrolkey_skipsso) && $SESSION->enrolkey_skipsso == 1)) {
+            return false;
+        }
+
+        $SESSION->enrolkey_skipsso = $skipsso;
+
+        // If SSO only is set and user is not passing the skip param
+        // or has it already set in their session then redirect to the SSO URL.
+        if (isset($this->config->ssourl) && $this->config->ssourl != '' && !$skipsso) {
+            return true;
+        }
+
+    }
+
     /**
      * Check if we should redirect a user after logout.
      *
      * @return bool
      */
-    protected function should_redirect() {
+    protected function should_logout_redirect() {
         global $SESSION;
 
         if (!isset($SESSION->userkey)) {
@@ -470,6 +573,7 @@ class auth_plugin_userkey extends auth_plugin_base {
         return true;
     }
 
+
     /**
      * Logout page hook.
      *
@@ -480,7 +584,7 @@ class auth_plugin_userkey extends auth_plugin_base {
     public function logoutpage_hook() {
         global $redirect;
 
-        if ($this->should_redirect()) {
+        if ($this->should_logout_redirect()) {
             $redirect = $this->config->redirecturl;
         }
     }

lang/en/auth_userkey.php

@@ -37,5 +37,9 @@ $string['createuser_desc'] = 'If enabled, a new user will be created if fail to
 $string['redirecturl'] = 'Logout redirect URL';
 $string['redirecturl_desc'] = 'Optionally you can redirect users to this URL after they logged out from LMS.';
 $string['incorrectredirecturl'] = 'You should provide valid URL';
+$string['incorrectssourl'] = 'You should provide valid URL';
 $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.';

login.php

@@ -28,5 +28,4 @@ if (!is_enabled_auth('userkey')) {
     print_error(get_string('pluginisdisabled', 'auth_userkey'));
 }
 
-$redirect = get_auth_plugin('userkey')->user_login_userkey();
+get_auth_plugin('userkey')->user_login_userkey();
-redirect($redirect);

settings.html

@@ -56,6 +56,13 @@ $fields = get_auth_plugin('userkey')->get_allowed_mapping_fields();
             <?php if (isset($err[$field])) { echo $OUTPUT->notification($err[$field], 'notifyfailure'); } ?>
             <?php print_string($field.'_desc', 'auth_userkey') ?></td>
     </tr>
+    <tr valign="top">
+        <?php $field = 'ssourl' ?>
+        <td align="right"><label for="<?php echo $field ?>"><?php print_string($field, 'auth_userkey') ?></label></td>
+        <td><input type="text" size="60" name="<?php echo $field ?>" value="<?php print $config->$field ?>" placeholder=""><br>
+            <?php if (isset($err[$field])) { echo $OUTPUT->notification($err[$field], 'notifyfailure'); } ?>
+            <?php print_string($field.'_desc', 'auth_userkey') ?></td>
+    </tr>
     <!--UNCOMMENT FOLLOWING WHEN IMPLEMENT USER CREATION.-->
     <!--<tr valign="top">-->
         <!--<?php $field = 'createuser' ?>-->

tests/auth_plugin_test.php

@@ -403,6 +403,7 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
         $form = new stdClass();
 
         $form->redirecturl = '';
+        $form->ssourl = '';
 
         $form->keylifetime = '';
         $err = array();
@@ -436,52 +437,82 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
     }
 
     /**
-     * Test that we can validate redirecturl for config form correctly.
+     * Data provider for testing URL validation functions.
+     *
+     * @return array First element URL, the second URL is error message. Empty error massage means no errors.
      */
-    public function test_validate_redirecturl_for_config_form() {
+    public function url_data_provider() {
+        return array(
+            array('', ''),
+            array('http://google.com/', ''),
+            array('https://google.com', ''),
+            array('http://some.very.long.and.silly.domain/with/a/path/', ''),
+            array('http://0.255.1.1/numericip.php', ''),
+            array('http://0.255.1.1/numericip.php?test=1&id=2', ''),
+            array('/just/a/path', 'You should provide valid URL'),
+            array('random string', 'You should provide valid URL'),
+            array(123456, 'You should provide valid URL'),
+            array('php://google.com', 'You should provide valid URL'),
+        );
+    }
+
+    /**
+     * Test that we can validate redirecturl for config form correctly.
+     *
+     * @dataProvider url_data_provider
+     */
+
+    /**
+     * Test that we can validate redirecturl for config form correctly.
+     *
+     * @dataProvider url_data_provider
+     *
+     * @param string $url URL to test.
+     * @param string $errortext Expected error text.
+     */
+    public function test_validate_redirecturl_for_config_form($url, $errortext) {
         $form = new stdClass();
 
         $form->keylifetime = 10;
+        $form->ssourl = '';
 
+        $form->redirecturl = $url;
+        $err = array();
+        $this->auth->validate_form($form, $err);
+
+        if (empty($errortext)) {
+            $this->assertFalse(array_key_exists('redirecturl', $err));
+        } else {
+            $this->assertArrayHasKey('redirecturl', $err);
+            $this->assertEquals($errortext, $err['redirecturl']);
+        }
+    }
+
+    /**
+     * Test that we can validate ssourl for config form correctly.
+     *
+     * @dataProvider url_data_provider
+     *
+     * @param string $url URL to test.
+     * @param string $errortext Expected error text.
+     */
+    public function test_validate_ssourl_for_config_form($url, $errortext) {
+        $form = new stdClass();
+
+        $form->keylifetime = 10;
         $form->redirecturl = '';
-        $err = array();
+        $form->ssourl = '';
-        $this->auth->validate_form($form, $err);
-        $this->assertFalse(array_key_exists('redirecturl', $err));
 
-        $form->redirecturl = 'http://google.com/';
+        $form->ssourl = $url;
         $err = array();
         $this->auth->validate_form($form, $err);
-        $this->assertFalse(array_key_exists('redirecturl', $err));
 
-        $form->redirecturl = 'https://google.com';
+        if (empty($errortext)) {
-        $err = array();
+            $this->assertFalse(array_key_exists('ssourl', $err));
-        $this->auth->validate_form($form, $err);
+        } else {
-        $this->assertFalse(array_key_exists('redirecturl', $err));
+            $this->assertArrayHasKey('ssourl', $err);
-
+            $this->assertEquals($errortext, $err['ssourl']);
-        $form->redirecturl = 'http://some.very.long.and.silly.domain/with/a/path/';
+        }
-        $err = array();
-        $this->auth->validate_form($form, $err);
-        $this->assertFalse(array_key_exists('redirecturl', $err));
-
-        $form->redirecturl = 'http://0.255.1.1/numericip.php';
-        $err = array();
-        $this->auth->validate_form($form, $err);
-        $this->assertFalse(array_key_exists('redirecturl', $err));
-
-        $form->redirecturl = '/just/a/path';
-        $err = array();
-        $this->auth->validate_form($form, $err);
-        $this->assertEquals('You should provide valid URL', $err['redirecturl']);
-
-        $form->redirecturl = 'random string';
-        $err = array();
-        $this->auth->validate_form($form, $err);
-        $this->assertEquals('You should provide valid URL', $err['redirecturl']);
-
-        $form->redirecturl = 123456;
-        $err = array();
-        $this->auth->validate_form($form, $err);
-        $this->assertEquals('You should provide valid URL', $err['redirecturl']);
     }
 
     /**
@@ -499,6 +530,7 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
         $formconfig->keylifetime = 100;
         $formconfig->iprestriction = 0;
         $formconfig->redirecturl = 'http://google.com/';
+        $formconfig->ssourl = 'http://google.com/';
 
         $this->auth->process_config($formconfig);
 
@@ -624,18 +656,43 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
         $_POST['key'] = 'RemoveKey';
         $_SERVER['HTTP_CLIENT_IP'] = '192.168.1.1';
 
+        try {
             // Using @ is the only way to test this. Thanks moodle!
             @$this->auth->user_login_userkey();
-
+        } catch (moodle_exception $e) {
             $keyexists = $DB->record_exists('user_private_key', array('value' => 'RemoveKey'));
             $this->assertFalse($keyexists);
         }
+    }
 
     /**
-     * Test that a user loggs in correctly.
+     * Test that a user logs in and gets redirected correctly.
+     *
+     * @expectedException moodle_exception
+     * @expectedExceptionMessage Unsupported redirect to http://www.example.com/moodle detected, execution terminated.
      */
-    public function test_that_user_logged_in() {
+    public function test_that_user_logged_in_and_redirected() {
-        global $DB, $USER, $SESSION, $CFG;
+        global $DB;
+
+        $key = new stdClass();
+        $key->value = 'UserLogin';
+        $key->script = 'auth/userkey';
+        $key->userid = $this->user->id;
+        $key->instance = $this->user->id;
+        $key->iprestriction = null;
+        $key->validuntil    = time() + 300;
+        $key->timecreated   = time();
+        $DB->insert_record('user_private_key', $key);
+
+        $_POST['key'] = 'UserLogin';
+        @$this->auth->user_login_userkey();
+    }
+
+    /**
+     * Test that a user logs in correctly.
+     */
+    public function test_that_user_logged_in_correctly() {
+        global $DB, $USER, $SESSION;
 
         $key = new stdClass();
         $key->value = 'UserLogin';
@@ -649,18 +706,23 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
 
         $_POST['key'] = 'UserLogin';
 
+        try {
             // Using @ is the only way to test this. Thanks moodle!
-        $redirect = @$this->auth->user_login_userkey();
+            @$this->auth->user_login_userkey();
-        $this->assertEquals($CFG->wwwroot, $redirect);
+        } catch (moodle_exception $e) {
             $this->assertEquals($this->user->id, $USER->id);
             $this->assertSame(sesskey(), $USER->sesskey);
             $this->assertObjectHasAttribute('userkey', $SESSION);
         }
+    }
 
     /**
-     * Test that wantsurl URL gets returned after user logged in if wantsurl's set.
+     * Test that a user gets redirected to internal wantsurl URL successful log in.
+     *
+     * @expectedException moodle_exception
+     * @expectedExceptionMessage Unsupported redirect to /course/index.php?id=12&key=134 detected, execution terminated.
      */
-    public function test_that_return_wantsurl() {
+    public function test_that_user_gets_redirected_to_internal_wantsurl() {
         global $DB;
 
         $key = new stdClass();
@@ -677,15 +739,17 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
         $_POST['wantsurl'] = '/course/index.php?id=12&key=134';
 
         // Using @ is the only way to test this. Thanks moodle!
-        $redirect = @$this->auth->user_login_userkey();
+        @$this->auth->user_login_userkey();
-
-        $this->assertEquals('/course/index.php?id=12&key=134', $redirect);
     }
 
     /**
-     * Test that wantsurl URL gets returned after user logged in if wantsurl's set to external URL.
+     * Test that a user gets redirected to external wantsurl URL successful log in.
+     *
+     * @expectedException moodle_exception
+     * @expectedExceptionMessage Unsupported redirect to http://test.com/course/index.php?id=12&key=134 detected,
+     * execution terminated.
      */
-    public function test_that_return_wantsurl_if_it_is_external_url() {
+    public function test_that_user_gets_redirected_to_external_wantsurl() {
         global $DB;
 
         $key = new stdClass();
@@ -702,9 +766,91 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
         $_POST['wantsurl'] = 'http://test.com/course/index.php?id=12&key=134';
 
         // Using @ is the only way to test this. Thanks moodle!
-        $redirect = @$this->auth->user_login_userkey();
+        @$this->auth->user_login_userkey();
+    }
 
-        $this->assertEquals('http://test.com/course/index.php?id=12&key=134', $redirect);
+    /**
+     * 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_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->loginpage_hook();
+    }
+
+    /**
+     * Test that login hook does not redirect a user if skipsso not set and ssourl is not set.
+     */
+    public function test_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->loginpage_hook());
+    }
+
+    /**
+     * Test that login hook does not redirect a user if skipsso is set and ssourl is not 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();
+
+        $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());
     }
 
 }