- issue2550920 - Optionally detect duplicate username at registration.

Added config option to allow detection of duplicate username when the
user tries to register. Previously user was rejected when dupliate
name found at confirmation step.

Optional as it can make username guessing easier.

Testing is in place for this.

Also attempted to make the unfriendly error message:

  'node with key "username" exists'

into a translatable friendly error:

  "Username 'username' already exists."

This is missing any test. It is also fragile as I capture the
ValueError exception and see that the exception matches:

  'node with key "username" exists'

If it does reassert the friendly message. Otherwise just re-raise
existing exception. If the "node with key..." message is translated
the friendly override will not trigger.

Author: rouilj

roundup/cgi/actions.py

@@ -1094,6 +1094,18 @@ def handle(self):
 
         # generate the one-time-key and store the props for later
         user_props = props[('user', None)]
+        # check that admin has requested username available check
+        check_user = self.db.config['WEB_REGISTRATION_PREVALIDATE_USERNAME']
+        if check_user:
+            try:
+                user_found = self.db.user.lookup(user_props['username'])
+                # if user is found reject the request.
+                raise Reject(
+                    _("Username '%s' is already used.")%user_props['username'])
+            except KeyError:
+                # user not found this is what we want.
+                pass
+                
         for propname, proptype in self.db.user.getprops().items():
             value = user_props.get(propname, None)
             if value is None:

roundup/configuration.py

@@ -803,6 +803,12 @@ def str2value(self, value):
             "registration form. This limits the rate at which bots\n"
             "can attempt to sign up. Limit can be disabled by setting\n"
             "the value to 0."),
+        (BooleanOption, 'registration_prevalidate_username', "no",
+            "When registering a user, check that the username\n"
+            "is available before sending confirmation email.\n"
+            "Usually a username conflict is detected when\n"
+            "confirming the registration. Disabled by default as\n"
+            "it can be used for guessing existing usernames.\n" ),
         (SameSiteSettingOption, 'samesite_cookie_setting', "Lax",
             """Set the mode of the SameSite cookie option for
 the session cookie. Choices are 'Lax' or

roundup/roundupdb.py

@@ -119,8 +119,24 @@ def confirm_registration(self, otk):
         cl = self.user
 
         props['roles'] = self.config.NEW_WEB_USER_ROLES
-        userid = cl.create(**props)
-        # clear the props from the otk database
+        try:
+            # ASSUME:: ValueError raised during create due to key value
+            # conflict. I an use message in exception to determine
+            # when I should intercept the exception with a more
+            # friendly error message. If i18n is used to translate
+            # original exception message this will fail and translated
+            # text (probably unfriendly) will be used.
+            userid = cl.create(**props)
+        except ValueError as e:
+            username = props['username']
+            # Try to make error message less cryptic to the user.
+            if str(e) == 'node with key "%s" exists' % username:
+                raise ValueError(
+                    _("Username '%s' already exists."%username))
+            else:
+                raise
+
+            # clear the props from the otk database
         self.getOTKManager().destroy(otk)
         # commit cl.create (and otk changes)
         self.commit()

test/test_cgi.py

@@ -15,7 +15,7 @@
 
 from roundup.cgi import client, actions, exceptions
 from roundup.cgi.exceptions import FormError, NotFound, Redirect
-from roundup.exceptions import UsageError
+from roundup.exceptions import UsageError, Reject
 from roundup.cgi.templating import HTMLItem, HTMLRequest, NoTemplate
 from roundup.cgi.templating import HTMLProperty, _HTMLItem, anti_csrf_nonce
 from roundup.cgi.form_parser import FormParser
@@ -1542,7 +1542,7 @@ def testEditCSVRestore(self):
         k = self.db.keyword.getnode('2')
         self.assertEqual(k.name, 'newkey2')
 
-    def testRegisterAction(self):
+    def testRegisterActionDelay(self):
         from roundup.cgi.timestamp import pack_timestamp
 
         # need to set SENDMAILDEBUG to prevent
@@ -1608,6 +1608,40 @@ def testRegisterAction(self):
         if os.path.exists(SENDMAILDEBUG):
             os.remove(SENDMAILDEBUG)
 
+    def testRegisterActionUnusedUserCheck(self):
+        # need to set SENDMAILDEBUG to prevent
+        # downstream issue when email is sent on successful
+        # issue creation. Also delete the file afterwards
+        # just tomake sure that someother test looking for
+        # SENDMAILDEBUG won't trip over ours.
+        if 'SENDMAILDEBUG' not in os.environ:
+            os.environ['SENDMAILDEBUG'] = 'mail-test1.log'
+        SENDMAILDEBUG = os.environ['SENDMAILDEBUG']
+
+        nodeid = self.db.user.create(username='iexist',
+            password=password.Password('foo'))
+
+        # enable check and remove delay time
+        self.db.config.WEB_REGISTRATION_PREVALIDATE_USERNAME = 1
+        self.db.config.WEB_REGISTRATION_DELAY = 0
+
+        # Make a request with existing user. Use iexist.
+        # do not need opaqueregister as we have disabled the delay check
+        cl = self._make_client({'username':'iexist', 'password':'secret',
+                 '@confirm@password':'secret', 'address':'[email protected]'},
+                               nodeid=None, userid='2')
+        with self.assertRaises(Reject) as cm:
+            actions.RegisterAction(cl).handle()
+        self.assertEqual(cm.exception.args,
+                    ("Username 'iexist' is already used.",))
+
+        cl = self._make_client({'username':'[email protected]',
+                                'password':'secret',
+                '@confirm@password':'secret', 'address':'[email protected]'},
+                               nodeid=None, userid='2')
+        self.assertRaises(Redirect, actions.RegisterAction(cl).handle)
+        
+
     def testserve_static_files(self):
         # make a client instance
         cl = self._make_client({})