auth/saml default remoteuser (bug #932909)

Ensure that default behaviour is to match user
to remote user name

Change-Id: Iadabb5c47004786af6fb6e2e6ac0590fb4a887d8
Signed-off-by: Piers Harding <piers@catalyst.net.nz>

htdocs/auth/saml/lang/en.utf8/auth.saml.php

@@ -39,7 +39,7 @@ $string['errorbadconfig'] = 'SimpleSAMLPHP config directory %s is incorrect.';
 $string['errorbadcombo'] = 'You can only choose user auto creation if you have not selected remoteuser';
 $string['errorbadinstitutioncombo'] = 'There is already an existing authinstance with this institutionattribute and institutionvalue combination';
 $string['errormissinguserattributes'] = 'You seem to be authenticated but we did not receive the required user attributes. Please check that your Identity Provider releases these SSO fields for First Name, Surname, and Email to the Service Provider Mahara is running on or inform the webmaster of this server.';
-//$string['idpidentity'] = 'IdP Identity Service';
+$string['errorremoteuser'] = 'Matching on remoteuser is mandatory if usersuniquebyusername is turned off';
 $string['institutionattribute'] = 'Institution attribute (contains "%s")';
 $string['institutionvalue'] = 'Institution value to check against attribute';
 $string['link'] = 'Link accounts';

htdocs/auth/saml/lib.php

@@ -47,7 +47,7 @@ class AuthSaml extends Auth {
         $this->config['institutionregex'] = 0;
         $this->config['institutionvalue'] = '';
         $this->config['updateuserinfoonlogin'] = 1;
-        $this->config['remoteuser'] = false;
+        $this->config['remoteuser'] = true;
         $this->config['loginlink'] = false;
         $this->instanceid = $id;
 
@@ -103,6 +103,7 @@ class AuthSaml extends Auth {
 
         // Retrieve a $user object. If that fails, create a blank one.
         try {
+            $isremote = $this->config['remoteuser'] ? true : false;
             $user = new User;
             if (get_config('usersuniquebyusername')) {
                 // When turned on, this setting means that it doesn't matter
@@ -133,9 +134,15 @@ class AuthSaml extends Auth {
                         . "somewhere else. Please turn this setting on in Site Options");
                     throw new AccessDeniedException();
                 }
-
             }
-            $isremote = $this->config['remoteuser'] ? true : false;
+            else {
+                if (!$isremote){
+                    log_warn("usersuniquebyusername is turned off but remoteuser has not been set on for this institution: $institutionname. "
+                        . "This is a security risk as users from different institutions with different IdPs can hijack "
+                        . "each others accounts.  Fix this in the institution level auth/saml settings.");
+                    throw new AccessDeniedException();
+                }
+            }
             if ($isremote) {
                 $user->find_by_instanceid_username($this->instanceid, $remoteuser, $isremote);
             }
@@ -282,7 +289,7 @@ class PluginAuthSaml extends PluginAuth {
         'institutionattribute'  => '',
         'institutionvalue'      => '',
         'institutionregex'      => 0,
-        'remoteuser'            => 0,
+        'remoteuser'            => 1,
         'loginlink'             => 0,
             );
 
@@ -488,6 +495,10 @@ class PluginAuthSaml extends PluginAuth {
                 $form->set_error('simplesamlphpconfig', get_string('errorbadconfig', 'auth.saml', $values['simplesamlphpconfig']));
             }
         }
+        // only allow remoteuser to be unset if usersuniquebyusername is NOT set
+        if (isset($values['remoteuser']) && !get_config('usersuniquebyusername') && !$values['remoteuser']) {
+            $form->set_error('remoteuser', get_string('errorremoteuser', 'auth.saml'));
+        }
         if (isset($values['weautocreateusers'])) {
             if ($values['weautocreateusers'] && $values['remoteuser']) {
                 $form->set_error('weautocreateusers', get_string('errorbadcombo', 'auth.saml'));