]> git.mxchange.org Git - quix0rs-gnu-social.git/commitdiff
Better use of Nickname validation functions
authorMikael Nordfeldth <mmn@hethane.se>
Wed, 16 Oct 2013 12:58:22 +0000 (14:58 +0200)
committerMikael Nordfeldth <mmn@hethane.se>
Wed, 16 Oct 2013 13:38:54 +0000 (15:38 +0200)
Nickname verifications on registration and updates for profiles (not yet
groups) have been improved.

Minor bugs in RegisterAction were also fixed, where multiple forms would
be outputed because the function did not return after showForm(). This
will be solved more permanently with throwing exceptions in the future.

actions/apiaccountregister.php
actions/apichecknickname.php
actions/profilesettings.php
actions/register.php
lib/installer.php
lib/nickname.php
lib/util.php
plugins/FacebookBridge/actions/facebookfinishlogin.php
plugins/OpenID/actions/finishopenidlogin.php
plugins/TwitterBridge/actions/twitterauthorization.php

index 9ce7bcfb685e236229854f9372172fca42784698..be66710d80ef1d3fb60519612c1f2f004fa9b611 100644 (file)
@@ -114,22 +114,17 @@ class ApiAccountRegisterAction extends ApiAction
 
         // Input scrubbing
         try {
-            $nickname = Nickname::normalize($nickname);
+            $nickname = Nickname::normalize($nickname, true);
         } catch (NicknameException $e) {
             // clientError handles Api exceptions with various formats and stuff
-               $this->clientError(_('Not a valid nickname.'), 400);
+               $this->clientError($e->getMessage(), $e->getCode());
         }
+
         $email = common_canonical_email($email);
 
            if ($email && !Validate::email($email, common_config('email', 'check_domain'))) {
             // TRANS: Form validation error displayed when trying to register without a valid e-mail address.
                $this->clientError(_('Not a valid email address.'), 400);
-        } else if ($this->nicknameExists($nickname)) {
-            // TRANS: Form validation error displayed when trying to register with an existing nickname.
-               $this->clientError(_('Nickname already in use. Try another one.'), 400);
-        } else if (!User::allowed_nickname($nickname)) {
-            // TRANS: Form validation error displayed when trying to register with an invalid nickname.
-               $this->clientError(_('Not a valid nickname.'), 400);
         } else if ($this->emailExists($email)) {
             // TRANS: Form validation error displayed when trying to register with an already registered e-mail address.
                $this->clientError(_('Email address already exists.'), 400);
@@ -186,22 +181,6 @@ class ApiAccountRegisterAction extends ApiAction
                }                   
         } 
     }
-      
-
-    /**
-     * Does the given nickname already exist?
-     *
-     * Checks a canonical nickname against the database.
-     *
-     * @param string $nickname nickname to check
-     *
-     * @return boolean true if the nickname already exists
-     */
-    function nicknameExists($nickname)
-    {
-        $user = User::staticGet('nickname', $nickname);
-        return is_object($user);
-    }
 
     /**
      * Does the given email address already exist?
index 4f80a7f29ea7e1bb92b846bd09cbc8457aa52d30..6867a428cf55334a34300e4b2084130c240310aa 100644 (file)
@@ -52,8 +52,8 @@ class ApiCheckNicknameAction extends ApiAction
         $nickname = $this->trimmed('nickname');
 
         try {
-            Nickname::normalize($nickname);
-            $nickname_ok = $this->nicknameExists($nickname) ? 0 : 1;
+            Nickname::normalize($nickname, true);
+            $nickname_ok = 1;
         } catch (NicknameException $e) {
             $nickname_ok = 0;
         }
index ef62eb9c8f3b91da9737597c159ad4c56de93c9e..b2c9a3c01490a27def8513fdf16a26f7781f3811 100644 (file)
@@ -239,8 +239,16 @@ class ProfilesettingsAction extends SettingsAction
 
         if (Event::handle('StartProfileSaveForm', array($this))) {
 
+            $nickname = $this->trimmed('nickname');
             try {
-                $nickname = Nickname::normalize($this->trimmed('nickname'));
+                $nickname = Nickname::normalize($nickname, true);
+            } catch (NicknameTakenException $e) {
+                // Abort only if the nickname is occupied by another user
+                if ($e->byuser->id != $this->scoped->id) {
+                    $this->showForm($e->getMessage());
+                    return;
+                }
+                $nickname = Nickname::normalize($nickname); // without in-use check this time
             } catch (NicknameException $e) {
                 $this->showForm($e->getMessage());
                 return;
@@ -258,11 +266,7 @@ class ProfilesettingsAction extends SettingsAction
             $tagstring = $this->trimmed('tags');
 
             // Some validation
-            if (!User::allowed_nickname($nickname)) {
-                // TRANS: Validation error in form for profile settings.
-                $this->showForm(_('Not a valid nickname.'));
-                return;
-            } else if (!is_null($homepage) && (strlen($homepage) > 0) &&
+            if (!is_null($homepage) && (strlen($homepage) > 0) &&
                        !common_valid_http_url($homepage)) {
                 // TRANS: Validation error in form for profile settings.
                 $this->showForm(_('Homepage is not a valid URL.'));
@@ -288,10 +292,6 @@ class ProfilesettingsAction extends SettingsAction
                 // TRANS: Validation error in form for profile settings.
                 $this->showForm(_('Timezone not selected.'));
                 return;
-            } else if ($this->nicknameExists($nickname)) {
-                // TRANS: Validation error in form for profile settings.
-                $this->showForm(_('Nickname already in use. Try another one.'));
-                return;
             } else if (!is_null($language) && strlen($language) > 50) {
                 // TRANS: Validation error in form for profile settings.
                 $this->showForm(_('Language is too long (maximum 50 characters).'));
@@ -468,17 +468,6 @@ class ProfilesettingsAction extends SettingsAction
         }
     }
 
-    function nicknameExists($nickname)
-    {
-        $user = common_current_user();
-        $other = User::getKV('nickname', $nickname);
-        if (!$other) {
-            return false;
-        } else {
-            return $other->id != $user->id;
-        }
-    }
-
     function showAside() {
         $user = common_current_user();
 
index 661936d5af3f50acc89bb8b38d7b1170009424a9..7c372892df8c27b57e8eacb3cf839885619b7527 100644 (file)
@@ -192,9 +192,10 @@ class RegisterAction extends Action
 
             // Input scrubbing
             try {
-                $nickname = Nickname::normalize($nickname);
+                $nickname = Nickname::normalize($nickname, true);
             } catch (NicknameException $e) {
                 $this->showForm($e->getMessage());
+                return;
             }
             $email    = common_canonical_email($email);
 
@@ -205,12 +206,6 @@ class RegisterAction extends Action
             } else if ($email && !Validate::email($email, common_config('email', 'check_domain'))) {
                 // TRANS: Form validation error displayed when trying to register without a valid e-mail address.
                 $this->showForm(_('Not a valid email address.'));
-            } else if ($this->nicknameExists($nickname)) {
-                // TRANS: Form validation error displayed when trying to register with an existing nickname.
-                $this->showForm(_('Nickname already in use. Try another one.'));
-            } else if (!User::allowed_nickname($nickname)) {
-                // TRANS: Form validation error displayed when trying to register with an invalid nickname.
-                $this->showForm(_('Not a valid nickname.'));
             } else if ($this->emailExists($email)) {
                 // TRANS: Form validation error displayed when trying to register with an already registered e-mail address.
                 $this->showForm(_('Email address already exists.'));
@@ -218,11 +213,9 @@ class RegisterAction extends Action
                        !common_valid_http_url($homepage)) {
                 // TRANS: Form validation error displayed when trying to register with an invalid homepage URL.
                 $this->showForm(_('Homepage is not a valid URL.'));
-                return;
             } else if (!is_null($fullname) && mb_strlen($fullname) > 255) {
                 // TRANS: Form validation error displayed when trying to register with a too long full name.
                 $this->showForm(_('Full name is too long (maximum 255 characters).'));
-                return;
             } else if (Profile::bioTooLong($bio)) {
                 // TRANS: Form validation error on registration page when providing too long a bio text.
                 // TRANS: %d is the maximum number of characters for bio; used for plural.
@@ -230,15 +223,12 @@ class RegisterAction extends Action
                                            'Bio is too long (maximum %d characters).',
                                            Profile::maxBio()),
                                         Profile::maxBio()));
-                return;
             } else if (!is_null($location) && mb_strlen($location) > 255) {
                 // TRANS: Form validation error displayed when trying to register with a too long location.
                 $this->showForm(_('Location is too long (maximum 255 characters).'));
-                return;
             } else if (strlen($password) < 6) {
                 // TRANS: Form validation error displayed when trying to register with too short a password.
                 $this->showForm(_('Password must be 6 or more characters.'));
-                return;
             } else if ($password != $confirm) {
                 // TRANS: Form validation error displayed when trying to register with non-matching passwords.
                 $this->showForm(_('Passwords do not match.'));
@@ -250,7 +240,7 @@ class RegisterAction extends Action
                                                     'bio' => $bio,
                                                     'location' => $location,
                                                     'code' => $code))) {
-                if (!$user) {
+                if (!($user instanceof User)) {
                     // TRANS: Form validation error displayed when trying to register with an invalid username or password.
                     $this->showForm(_('Invalid username or password.'));
                     return;
@@ -259,7 +249,6 @@ class RegisterAction extends Action
                 if (!common_set_user($user)) {
                     // TRANS: Server error displayed when saving fails during user registration.
                     $this->serverError(_('Error setting user.'));
-                    return;
                 }
                 // this is a real login
                 common_real_login(true);
@@ -281,21 +270,6 @@ class RegisterAction extends Action
         }
     }
 
-    /**
-     * Does the given nickname already exist?
-     *
-     * Checks a canonical nickname against the database.
-     *
-     * @param string $nickname nickname to check
-     *
-     * @return boolean true if the nickname already exists
-     */
-    function nicknameExists($nickname)
-    {
-        $user = User::getKV('nickname', $nickname);
-        return is_object($user);
-    }
-
     /**
      * Does the given email address already exist?
      *
index e3135764295adf59be7328dd46b9bdbe4d127d5a..bf30d00786261c7f72d48a9263f9f40c0177f30b 100644 (file)
@@ -235,7 +235,7 @@ abstract class Installer
                          '" is invalid; should be plain letters and numbers no longer than 64 characters.', true);
             $fail = true;
         }
-        // @fixme hardcoded list; should use User::allowed_nickname()
+        // @fixme hardcoded list; should use Nickname::isValid()
         // if/when it's safe to have loaded the infrastructure here
         $blacklist = array('main', 'panel', 'twitter', 'settings', 'rsd.xml', 'favorited', 'featured', 'favoritedrss', 'featuredrss', 'rss', 'getfile', 'api', 'groups', 'group', 'peopletag', 'tag', 'user', 'message', 'conversation', 'bookmarklet', 'notice', 'attachment', 'search', 'index.php', 'doc', 'opensearch', 'robots.txt', 'xd_receiver.html', 'facebook');
         if (in_array($this->adminNick, $blacklist)) {
index 2b8249d59ab4b6ab7ff01099386cf7c94b55d491..2792d32fd50494900a01e4cbf09a55617d1fab5c 100644 (file)
@@ -110,30 +110,25 @@ class Nickname
      */
     public static function normalize($str, $checkuse=false)
     {
-        if (mb_strlen($str) > self::MAX_LEN) {
-            // Display forms must also fit!
-            throw new NicknameTooLongException();
-        }
-
+        // We should also have UTF-8 normalization (å to a etc.)
         $str = trim($str);
         $str = str_replace('_', '', $str);
         $str = mb_strtolower($str);
 
-        if (mb_strlen($str) < 1) {
+        if (mb_strlen($str) > self::MAX_LEN) {
+            // Display forms must also fit!
+            throw new NicknameTooLongException();
+        } elseif (mb_strlen($str) < 1) {
             throw new NicknameEmptyException();
-        }
-        if (!self::isCanonical($str)) {
+        } elseif (!self::isCanonical($str)) {
             throw new NicknameInvalidException();
-        }
-        if (self::isBlacklisted($str)) {
+        } elseif (self::isBlacklisted($str)) {
             throw new NicknameBlacklistedException();
-        }
-        if (self::isSystemPath($str)) {
+        } elseif (self::isSystemPath($str)) {
             throw new NicknamePathCollisionException();
-        }
-        if ($checkuse && $user = self::isTaken($str)) {
+        } elseif ($checkuse && $user = self::isTaken($str)) {
             if ($user instanceof User) {
-                throw new NicknameTakenException();
+                throw new NicknameTakenException($user);
             }
         }
 
index 8c6ff6718a804fecd83ed7a5415347a6554a4203..81643f9a192e8d9deb741a1e44ac9cf87f5767ab 100644 (file)
@@ -237,7 +237,7 @@ function common_check_user($nickname, $password)
         if (common_is_email($nickname)) {
             $user = User::getKV('email', common_canonical_email($nickname));
         } else {
-            $user = User::getKV('nickname', common_canonical_nickname($nickname));
+            $user = User::getKV('nickname', Nickname::normalize($nickname));
         }
 
         if (!empty($user)) {
@@ -2283,8 +2283,11 @@ function common_url_to_nickname($url)
 
 function common_nicknamize($str)
 {
-    $str = preg_replace('/\W/', '', $str);
-    return strtolower($str);
+    try {
+        return Nickname::normalize($str);
+    } catch (NicknameException $e) {
+        return null;
+    }
 }
 
 function common_perf_counter($key, $val=null)
index 2abbf9c905bbc313437312c7a1fb1472f614c32a..483460baad03542ac859b3f17c68cf028c995a34 100644 (file)
@@ -355,24 +355,12 @@ class FacebookfinishloginAction extends Action
         }
 
         try {
-            $nickname = Nickname::normalize($this->trimmed('newname'));
+            $nickname = Nickname::normalize($this->trimmed('newname'), true);
         } catch (NicknameException $e) {
             $this->showForm($e->getMessage());
             return;
         }
 
-        if (!User::allowed_nickname($nickname)) {
-            // TRANS: Form validation error displayed when picking a nickname that is not allowed.
-            $this->showForm(_m('Nickname not allowed.'));
-            return;
-        }
-
-        if (User::getKV('nickname', $nickname)) {
-            // TRANS: Form validation error displayed when picking a nickname that is already in use.
-            $this->showForm(_m('Nickname already in use. Try another one.'));
-            return;
-        }
-
         $args = array(
             'nickname' => $nickname,
             'fullname' => $this->fbuser->name,
@@ -603,58 +591,23 @@ class FacebookfinishloginAction extends Action
 
     function bestNewNickname()
     {
-        if (!empty($this->fbuser->username)) {
-            $nickname = $this->nicknamize($this->fbuser->username);
-            if ($this->isNewNickname($nickname)) {
-                return $nickname;
-            }
+        try {
+            $nickname = Nickname::normalize($this->fbuser->username, true);
+            return $nickname;
+        } catch (NicknameException $e) {
+            // Failed to normalize nickname, but let's try the full name
         }
 
-        // Try the full name
-
-        $fullname = $this->fbuser->name;
-
-        if (!empty($fullname)) {
-            $fullname = $this->nicknamize($fullname);
-            if ($this->isNewNickname($fullname)) {
-                return $fullname;
-            }
+        try {
+            $nickname = Nickname::normalize($this->fbuser->name, true);
+            return $nickname;
+        } catch (NicknameException $e) {
+            // Any more ideas? Nope.
         }
 
         return null;
     }
 
-     /**
-      * Given a string, try to make it work as a nickname
-      */
-     function nicknamize($str)
-     {
-         $str = preg_replace('/\W/', '', $str);
-         return strtolower($str);
-     }
-
-     /*
-      * Is the desired nickname already taken?
-      *
-      * @return boolean result
-      */
-     function isNewNickname($str)
-     {
-        if (!Nickname::isValid($str)) {
-            return false;
-        }
-
-        if (!User::allowed_nickname($str)) {
-            return false;
-        }
-
-        if (User::getKV('nickname', $str)) {
-            return false;
-        }
-
-        return true;
-    }
-
     /*
      * Do we already have a user record with this email?
      * (emails have to be unique but they can change)
index 4326f1951461a954017aebcb30dceb7ca3a6b78e..372c1e12b5d961eeda2ab07c8e31339b91b2fe40 100644 (file)
@@ -345,24 +345,12 @@ class FinishopenidloginAction extends Action
         }
 
         try {
-            $nickname = Nickname::normalize($this->trimmed('newname'));
+            $nickname = Nickname::normalize($this->trimmed('newname'), true);
         } catch (NicknameException $e) {
             $this->showForm($e->getMessage());
             return;
         }
 
-        if (!User::allowed_nickname($nickname)) {
-            // TRANS: OpenID plugin message. The entered new user name is blacklisted.
-            $this->showForm(_m('Nickname not allowed.'));
-            return;
-        }
-
-        if (User::getKV('nickname', $nickname)) {
-            // TRANS: OpenID plugin message. The entered new user name is already used.
-            $this->showForm(_m('Nickname already in use. Try another one.'));
-            return;
-        }
-
         list($display, $canonical, $sreg) = $this->getSavedValues();
 
         if (!$display || !$canonical) {
@@ -500,8 +488,8 @@ class FinishopenidloginAction extends Action
         // Try the passed-in nickname
 
         if (!empty($sreg['nickname'])) {
-            $nickname = $this->nicknamize($sreg['nickname']);
-            if ($this->isNewNickname($nickname)) {
+            $nickname = common_nicknamize($sreg['nickname']);
+            if (Nickname::isValid($nickname, true)) {
                 return $nickname;
             }
         }
@@ -509,8 +497,8 @@ class FinishopenidloginAction extends Action
         // Try the full name
 
         if (!empty($sreg['fullname'])) {
-            $fullname = $this->nicknamize($sreg['fullname']);
-            if ($this->isNewNickname($fullname)) {
+            $fullname = common_nicknamize($sreg['fullname']);
+            if (Nickname::isValid($fullname, true)) {
                 return $fullname;
             }
         }
@@ -519,7 +507,7 @@ class FinishopenidloginAction extends Action
 
         $from_url = $this->openidToNickname($display);
 
-        if ($from_url && $this->isNewNickname($from_url)) {
+        if ($from_url && Nickname::isValid($from_url, true)) {
             return $from_url;
         }
 
@@ -528,20 +516,6 @@ class FinishopenidloginAction extends Action
         return null;
     }
 
-    function isNewNickname($str)
-    {
-        if (!Nickname::isValid($str)) {
-            return false;
-        }
-        if (!User::allowed_nickname($str)) {
-            return false;
-        }
-        if (User::getKV('nickname', $str)) {
-            return false;
-        }
-        return true;
-    }
-
     function openidToNickname($openid)
     {
         if (Auth_Yadis_identifierScheme($openid) == 'XRI') {
@@ -570,7 +544,7 @@ class FinishopenidloginAction extends Action
             // =evan.prodromou
             // or @gratis*evan.prodromou
             $parts = explode('*', substr($base, 1));
-            return $this->nicknamize(array_pop($parts));
+            return common_nicknamize(array_pop($parts));
         }
     }
 
@@ -582,10 +556,4 @@ class FinishopenidloginAction extends Action
             return $xri;
         }
     }
-
-    // Given a string, try to make it work as a nickname
-    function nicknamize($str)
-    {
-        return common_nicknamize($str);
-    }
 }
index 6a86c81cb724ea01092b9503b1e532a1dd3d08f2..653326e1f823eced353dc2d5e19577163bb3214d 100644 (file)
@@ -519,24 +519,12 @@ class TwitterauthorizationAction extends Action
         }
 
         try {
-            $nickname = Nickname::normalize($this->trimmed('newname'));
+            $nickname = Nickname::normalize($this->trimmed('newname'), true);
         } catch (NicknameException $e) {
             $this->showForm($e->getMessage());
             return;
         }
 
-        if (!User::allowed_nickname($nickname)) {
-            // TRANS: Client error displayed when trying to create a new user with an invalid username.
-            $this->showForm(_m('Nickname not allowed.'));
-            return;
-        }
-
-        if (User::getKV('nickname', $nickname)) {
-            // TRANS: Client error displayed when trying to create a new user with a username that is already in use.
-            $this->showForm(_m('Nickname already in use. Try another one.'));
-            return;
-        }
-
         $fullname = trim($this->tw_fields['fullname']);
 
         $args = array('nickname' => $nickname, 'fullname' => $fullname);
@@ -688,36 +676,10 @@ class TwitterauthorizationAction extends Action
 
     function bestNewNickname()
     {
-        if (!empty($this->tw_fields['fullname'])) {
-            $nickname = $this->nicknamize($this->tw_fields['fullname']);
-            if ($this->isNewNickname($nickname)) {
-                return $nickname;
-            }
-        }
-
-        return null;
-    }
-
-     // Given a string, try to make it work as a nickname
-
-     function nicknamize($str)
-     {
-         $str = preg_replace('/\W/', '', $str);
-         $str = str_replace(array('-', '_'), '', $str);
-         return strtolower($str);
-     }
-
-    function isNewNickname($str)
-    {
-        if (!Nickname::isValid($str)) {
-            return false;
-        }
-        if (!User::allowed_nickname($str)) {
-            return false;
-        }
-        if (User::getKV('nickname', $str)) {
-            return false;
+        try {
+            return Nickname::normalize($this->tw_fields['fullname'], true);
+        } catch (NicknameException $e) {
+            return null
         }
-        return true;
     }
 }