Mercurial > p > roundup > code
changeset 5976:71c68961d9f4
- 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 | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sat, 09 Nov 2019 16:33:42 -0500 |
| parents | 59842a3e8108 |
| children | 1fc40205c5d0 |
| files | roundup/cgi/actions.py roundup/configuration.py roundup/roundupdb.py test/test_cgi.py |
| diffstat | 4 files changed, 72 insertions(+), 4 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/cgi/actions.py Sat Nov 09 00:41:53 2019 -0500 +++ b/roundup/cgi/actions.py Sat Nov 09 16:33:42 2019 -0500 @@ -1094,6 +1094,18 @@ # 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:
--- a/roundup/configuration.py Sat Nov 09 00:41:53 2019 -0500 +++ b/roundup/configuration.py Sat Nov 09 16:33:42 2019 -0500 @@ -803,6 +803,12 @@ "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
--- a/roundup/roundupdb.py Sat Nov 09 00:41:53 2019 -0500 +++ b/roundup/roundupdb.py Sat Nov 09 16:33:42 2019 -0500 @@ -119,8 +119,24 @@ 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()
--- a/test/test_cgi.py Sat Nov 09 00:41:53 2019 -0500 +++ b/test/test_cgi.py Sat Nov 09 16:33:42 2019 -0500 @@ -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 @@ 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 @@ 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':'iexist@bork.bork'}, + 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':'i-do@not.exist', + 'password':'secret', + '@confirm@password':'secret', 'address':'iexist@bork.bork'}, + nodeid=None, userid='2') + self.assertRaises(Redirect, actions.RegisterAction(cl).handle) + + def testserve_static_files(self): # make a client instance cl = self._make_client({})