[SUCS Devel] [Git][sucssite/sucs-site][master] 7 commits: Fix signup validation. Emails, names and addresses are now much less strict, and…

Imran Hussain imranh at sucs.org
Wed Sep 27 10:41:08 BST 2017


Imran Hussain pushed to branch master at sucssite / sucs-site


Commits:
620c9b63 by gigosaurus at 2017-09-26T21:59:10+01:00
Fix signup validation. Emails, names and addresses are now much less strict, and more valid postcodes are allowed.

- - - - -
777965c6 by Kit Manners at 2017-09-27T00:07:42+01:00
Fix typo in signup completion
- - - - -
fcfabbae by Kit Manners at 2017-09-27T01:13:07+01:00
Fix retrieving full name from campus ldap
- - - - -
86ccc6b7 by Kit Manners at 2017-09-27T01:20:53+01:00
Add missing space in error message
- - - - -
4063d666 by gigosaurus at 2017-09-27T02:40:47+01:00
Remember the student number if they entered it in a previous form

- - - - -
07f6b98f by gigosaurus at 2017-09-27T02:42:05+01:00
Merge branch 'master' of projects.sucs.org:gigosaurus/sucs-site

- - - - -
ea4cbf7c by Imran Hussain at 2017-09-27T10:41:03+01:00
Merge branch 'gigosaurus/sucs-site-master'

- - - - -


7 changed files:

- components/signup.php
- components/signupajax.php
- components/susignup.php
- lib/validation.php
- lib/validationData.php
- templates/signup.tpl
- templates/susignup.tpl


Changes:

=====================================
components/signup.php
=====================================
--- a/components/signup.php
+++ b/components/signup.php
@@ -50,6 +50,10 @@ if (isset($_REQUEST['signupid']) && isset($_REQUEST['signuppw'])) {
             // pass on the id and passwd and id the validation is overridable
             $smarty->assign("signupid", $signupid);
             $smarty->assign("signuppw", $signuppw);
+            // pass on the student id if it exists
+            if (isset($_REQUEST['signupsid'])) {
+                $smarty->assign("signupsid", $signupsid);
+            }
             $smarty->assign("overridable", $overridable);
             $smarty->assign("usertype", $row[type]);
             // if accepting the form
@@ -80,18 +84,18 @@ if (isset($_REQUEST['signupid']) && isset($_REQUEST['signuppw'])) {
                         $errors['address'] = $error;
                     }
                     $fields['address'] = sanitizeAddress($_POST['address']);
-                    if (!validRealName($_REQUEST['realname'], $override)) {
+                    if (!validName($_REQUEST['realname'], $override)) {
                         $valid = false;
                         $errors['realname'] = $error;
                     }
                     $fields['realname'] = $_REQUEST['realname'];
                 } else {
-                    if (!(validRealName($_REQUEST['contact'], false) || $override)) {
+                    if (!(validName($_REQUEST['contact'], false) || $override)) {
                         $valid = false;
                         $errors['contact'] = $error;
                     }
                     $fields['contact'] = $_REQUEST['contact'];
-                    if (!validSocName($_REQUEST['realname'], $override)) {
+                    if (!validName($_REQUEST['realname'], $override)) {
                         $valid = false;
                         $errors['realname'] = $error;
                     }


=====================================
components/signupajax.php
=====================================
--- a/components/signupajax.php
+++ b/components/signupajax.php
@@ -29,7 +29,7 @@ if (isset($_GET['key'])) {
             break;
         case "realname":
             $realname = $_GET['value'];
-            if (validRealName($realname, false)) {
+            if (validName($realname, false)) {
                 echo "OK";
             } else {
                 echo $error;
@@ -37,7 +37,7 @@ if (isset($_GET['key'])) {
             break;
         case "socname":
             $socname = $_GET['value'];
-            if (validSocName($socname, false)) {
+            if (validName($socname, false)) {
                 echo "OK";
             } else {
                 echo $error;


=====================================
components/susignup.php
=====================================
--- a/components/susignup.php
+++ b/components/susignup.php
@@ -51,6 +51,7 @@ if (!empty($_REQUEST['sid']) && !empty($_REQUEST['transactionID'])) {
             $mode = "form";
             $smarty->assign("id", $signuptmpresult->fields["id"]);
             $smarty->assign("pass", $signuptmpresult->fields["password"]);
+            $smarty->assign("sid", $signuptmpresult->fields["sid"]);
         // else if they aren't in the SUCS DB, then bootstrap signup process
         } else if ($tmpresult->fields == false) {
             $mode = "form";
@@ -59,6 +60,7 @@ if (!empty($_REQUEST['sid']) && !empty($_REQUEST['transactionID'])) {
             $id = $iddata->fields['id'];
             $smarty->assign("id", $id);
             $smarty->assign("pass", $pass);
+            $smarty->assign("sid", $sid);
         } else {
             // they should never get here
             die("You'll see this if there has been a database error. Someone probably knows and is trying to fix it. Sorry.");


=====================================
lib/validation.php
=====================================
--- a/lib/validation.php
+++ b/lib/validation.php
@@ -7,18 +7,22 @@ require_once("sanitization.php");
 function validEmail($email)
 {
     global $error;
-    //split user and domain
-    list($user, $domain) = explode("@", $email);
-    // check for bad characters, and check for zero length user & domain
-    if (!preg_match("/^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$/i", $email) or !$user or !$domain) {
-        $error = 'an invalid email address (syntax)';
+
+    // check for valid syntax
+    if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
+        $error = 'Invalid email address (syntax)';
         return false;
     }
+
     // Syntax OK
 
+    // domain consists of any character after a '@' and cannot contain '@'
+    // therefore any character after the last '@' is part of the domain
+    $domain = substr($email, strrpos($email, '@') + 1);
+
     // Check for an mail server
-    elseif (!getmxrr($domain, $mx) or !gethostbyname($domain)) {
-        $error = "no mail servers listed for '$domain'";
+    if (!getmxrr($domain, $mx) or !gethostbyname($domain)) {
+        $error = "No mail servers listed for '$domain'";
         return false;
     } else {
         // Email address valid from technical point of view
@@ -26,41 +30,6 @@ function validEmail($email)
     }
 }
 
-// test whether a password is considered Strong Enough
-// ideally we'd want to use cracklib or something here, but no RPM for the php bindings :-(
-// dont use this, use weakPassword instead it uses cracklib
-function strongPassword($pass)
-{
-
-    // you call this a password? my cat could bruteforce this.
-    if (strlen($pass) < 6) {
-        return false;
-    }
-
-// start at 0, and increment for certain features
-    $score = 0;
-
-
-// greater than 8 characters
-    if (strlen($pass) > 8) $score++;
-// includes lowercase characters
-    if (preg_match("/[a-z]/", $pass)) $score++;
-// includes uppercase characters
-    if (preg_match("/[A-Z]/", $pass)) $score++;
-// includes digits
-    if (preg_match("/\d/", $pass)) $score++;
-// includes "non-word" characters
-    if (preg_match("/\W/", $pass)) $score++;
-
-// I reckons if it has at least 3 of the above it should be... adequate
-// better if it checked for dictionary words too though
-    if ($score > 3) {
-        return true;
-    } else {
-        return false;
-    }
-}
-
 # Use cracklib to check for weak passwords.
 # returns FALSE if the password is good i.e. not weak
 # otherwise returns a string saying why its weak
@@ -112,7 +81,7 @@ function isAlias($username)
     return $ok;
 }
 
-//check if a user with a sid already exsists
+//check if a user with a sid already exists
 function sidUsed($sid)
 {
     $sucsDB = NewADOConnection('postgres8');
@@ -127,12 +96,12 @@ function sidUsed($sid)
 function validUsername($username)
 {
     global $error;
-    // check if uname is sytactically valid
+    // check if uname is syntactically valid
     $syntax = preg_match("/^[a-z][a-z0-9_]*$/", $username);
     if (!$syntax || (strlen($username) < 2)) {
-        $error = "Usernames must start with a letter, only contain lowercase letter, numbers 0-9 and underscores (_) and be at least 2 characters.";
+        $error = "Usernames must start with a letter, only contain lowercase letters, numbers 0-9 and underscores (_) and be at least 2 characters.";
         return false;
-    } // check if the username already exsists
+    } // check if the username already exists
     elseif (posix_getpwnam($username)) {
         $error = "Username already taken";
         return false;
@@ -163,7 +132,7 @@ function validSID($SID, $override)
             $error = "A user with that student ID already exists, email <a href=\"mailto:admin at sucs.org\">admin at sucs.org</a> if this is an error.";
             return false;
         } elseif (lookupSID($SID) == " ") {
-            $error = "Student not found, email<a href=\"mailto:admin at sucs.org\">admin at sucs.org</a> if this is an error.";
+            $error = "Student not found, email <a href=\"mailto:admin at sucs.org\">admin at sucs.org</a> if this is an error.";
             return false;
         } else {
             return true;
@@ -171,7 +140,7 @@ function validSID($SID, $override)
     }
 }
 
-function validRealName($realName, $override)
+function validName($realName, $override)
 {
     global $error;
     if ($override) {
@@ -182,56 +151,12 @@ function validRealName($realName, $override)
             return true;
         }
     } else {
-        //check for enough names for real name (we insist on at least 2
-        if (count(explode(" ", $realName)) < 2) {
-            $error = "Too few names given, please give at least two.";
-            return false;
-        } //check for a sane realname, see comment below
-        elseif (!preg_match("/^([A-Z]([.]+ +[A-Z])*([\']+[A-Z])*[a-z]+[ -]*)+$/", $realName)) {
-            $error = "Name incorrectly formatted, email <a href=\"mailto:admin at sucs.org\">admin at sucs.org</a> if this is an error.";
-            return false;
-        } /*
- * This should force sane real names, with capitals for the first letter of each word,
- * Whist alowing for complex names such as Robin M. O'Leary
- * 
- * break down of regexp
- * 
- * (
- * [A-Z]                - start with a single capital
- * ([.]+ +[A-Z])*       - zero or more of, (at least one "." followed by at least one space then another single capital)  //we dont expect people to have initals at the end of there names so this is alright
- * ([\']+[A-Z])*        - zero or more of, (at least one "'"s followed by a single capital letter)
- * [a-z]+               - One or more lower case letters, this forces initals to be followed by a "."
- *[ -]*           - zero or more " "s or "-"s so double barreled names are supported
- * )
- * 
- * In its current state 
- * Robin M. O'Leary is valid
- * Robin M O'Leary is not
- * Robin M. OLeary is Not
- * Robin M. O'LeaRy is valid (though its not meant to be.. bad side effect of not requireing at least one space...)
- * BUT... this alows for McSmith's... which is rather nice :)... and of course delibrate
- * RObin M O'Leary is not
- *
- */
-        else {
-            return true;
-        }
-    }
-}
 
-function validSocName($socname, $override)
-{
-    global $error;
-    if ($override) {
-        if ($socname == "") {
-            $error = "You MUST provide some sort of name";
-            return false;
-        } else {
-            return true;
-        }
-    } else {
-        if (!preg_match('/^[A-Z1-9]/', $socname) || strlen($socname) < 2) {
-            $error = "Must start with a capital letter or a number and be more than 1 character";
+        // names can legally be really weird so just check that it is at least 1 visible character
+        // followed by any number of non-control characters
+        $realName = trim($realName);
+        if (!preg_match("/^[[:graph:]][[:print:]]*$/", $realName)) {
+            $error = "Invalid name";
             return false;
         } else {
             return true;
@@ -243,9 +168,11 @@ function validAddress($address)
 {
     global $error;
     $address = sanitizeAddress($address);
-    $regex = "/^([A-Z0-9]([[:alnum:]]|[ .\/'-])*\n)+[A-Z0-9]([[:alnum:]]|[ .\/'-])*$/";
+
+    // check that they at least entered in something. Address doesn't need to be as strict when the postcode is.
+    $regex = "/^.{5,}+$/s";
     if (!preg_match($regex, $address)) {
-        $error = "Please supply at least two valid lines of address.";
+        $error = "Please supply a valid address.";
         return false;
     } else {
         return true;
@@ -255,7 +182,10 @@ function validAddress($address)
 function validPostcode($postcode)
 {
     $postcode = sanitizePostcode($postcode);
-    if (!preg_match('/^[A-Z]{1,2}[0-9]{1,2}[A-Z]{0,1} [0-9][A-Z]{2}$/', $postcode)) {
+
+    // matches all postcodes following the valid format described in a 2012 government published document
+    $postcodeRegex = "/^([A-Z](([0-9][0-9]?)|([A-Z][0-9][0-9]?)|([A-Z]?[0-9][A-Z])) ?[0-9][ABD-HJLNP-UW-Z]{2})$/";
+    if (!preg_match($postcodeRegex, $postcode)) {
         return false;
     } else {
         return $postcode;


=====================================
lib/validationData.php
=====================================
--- a/lib/validationData.php
+++ b/lib/validationData.php
@@ -7,7 +7,7 @@ function lookupSID($sid)
     $sr = ldap_search($ds, "ou=Active,ou=Resources,o=Swansea", "EDUPERSONTARGETEDID=" . $sid);
     $info = ldap_get_entries($ds, $sr);
     ldap_unbind($ds);
-    return ucwords(strtolower($info[0]['givenName'][0] . " " . $info[0]['sn'][0]));
+    return ucwords(strtolower($info[0]['givenname'][0] . " " . $info[0]['sn'][0]));
 }
 
 // lookup addresses from postcodes using the university's website


=====================================
templates/signup.tpl
=====================================
--- a/templates/signup.tpl
+++ b/templates/signup.tpl
@@ -36,7 +36,7 @@
             <div class="row" id="studentiddiv">
                 <label for="studentid">Student Number</label>
                 <span class="textinput"><input type="text" id="studentid" name="studentid" size="30"
-                                               {if $mode=='re-form'}value='{$fields.studentid}'{/if} /></span>
+                                               {if $mode=='re-form'}value='{$fields.studentid}'{elseif isset($signupsid)}value='{$signupsid}'{/if} /></span>
 
                 <div id="studentidmessage"{if $mode=='re-form'}{if isset($errors.studentid)}
                      style="color:red; float:right; clear:right;">{$errors.studentid}{else} style="color:green;
@@ -136,7 +136,7 @@
     {if !$failed}
         <h1>Welcome to SUCS</h1>
         <p>Signup is complete, please see below for your password, a copy has also been send to {$email}, we request you
-            change this immediatley. See our <a href="{$baseurl}/Getting%20Started">Getting Started</a> page for some
+            change this immediately. See our <a href="{$baseurl}/Getting%20Started">Getting Started</a> page for some
             ways you can start using your new SUCS account!</p>
         <p>
             Username: <strong>{$username}</strong><br/>


=====================================
templates/susignup.tpl
=====================================
--- a/templates/susignup.tpl
+++ b/templates/susignup.tpl
@@ -39,6 +39,7 @@
     <form action="{$baseurl}/signup/" method="post">
         <input type=hidden name="signupid" id="id" value="{$id}"/>
         <input type=hidden name="signuppw" id="pass" value="{$pass}"/>
+        <input type=hidden name="signupsid" id="sid" value="{$sid}"/>
         <input type=submit name="submit" value="Proceed"/>
     </form>
 {else}
@@ -49,4 +50,4 @@
     An error occured during signup, please email, with as much information as you can provide,
     <a href='mailto:admin at sucs.org'>admin at sucs.org</a>
     for assistance.
-{/if}
\ No newline at end of file
+{/if}



View it on GitLab: https://projects.sucs.org/sucssite/sucs-site/compare/52510509648083629f5de76d44d017b0a38695b3...ea4cbf7cf205eec0cca2a5c23275fd46771b3fa1

---
View it on GitLab: https://projects.sucs.org/sucssite/sucs-site/compare/52510509648083629f5de76d44d017b0a38695b3...ea4cbf7cf205eec0cca2a5c23275fd46771b3fa1
You're receiving this email because of your account on projects.sucs.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sucs.org/pipermail/devel/attachments/20170927/ba30f71e/attachment-0001.html>


More information about the Devel mailing list