Commits

Anonymous committed fa50b7b

AccountManagerPlugin: Finish registration check rewrite, refs #874, #2707, #2897, #4651, #5295, #7577, #8076.

Now moving check execution into AccountManager, and `_create_user()` is gone,
replaced by more clearly structured and modularized code.

Registration checking for requests from the admin panel is re-activated and
minor improvements related to RegistrationError processing done too.

Note: The test, that detected admin requests to skip additional checking of
existing permissions for each new username, has been done differently.
In the future each check has to decide on its own, like this is done in the
`UsernamePermCheck` now.

Comments (0)

Files changed (5)

accountmanagerplugin/trunk/acct_mgr/admin.py

 from acct_mgr.model     import del_user_attribute, email_verified, \
                                get_user_attribute, last_seen, \
                                set_user_attribute
-from acct_mgr.register  import _create_user, EmailVerificationModule
+from acct_mgr.register  import EmailVerificationModule, RegistrationError
 from acct_mgr.web_ui    import AccountModule
 from acct_mgr.util      import is_enabled, get_pretty_dateinfo
 
         if req.method == 'POST':
             if req.args.get('add'):
                 if create_enabled:
+                    # Check request and prime account on success.
                     try:
-                        _create_user(req, env, check_permissions=False)
-                    except TracError, e:
-                        data['editor_error'] = e.message
-                        data['account'] = getattr(e, 'account', '')
+                        acctmgr.validate_registration(req)
+                        # User editor form clean-up.
+                        data['account'] = {}
+                    except RegistrationError, e:
+                        data['editor_error'] = Markup(e.message)
+                        account = {
+                            'username': acctmgr.handle_username_casing(
+                                            req.args.get('username').strip()),
+                                'name': req.args.get('name').strip(),
+                               'email': req.args.get('email').strip()
+                        }
+                        data['account'].update(account)
                 else:
                     data['editor_error'] = _(
                         "The password store does not support creating users.")

accountmanagerplugin/trunk/acct_mgr/api.py

                 pass
         return string
 
-from acct_mgr.model import delete_user, get_user_attribute, set_user_attribute
+from acct_mgr.model import delete_user, get_user_attribute, \
+                           prime_auth_session, set_user_attribute
 
 
 class IPasswordStore(Interface):
         'account-manager', 'password_store', IPasswordStore,
         include_missing=False)
     _password_format = Option('account-manager', 'password_format')
+    _register_check = OrderedExtensionsOption(
+        'account-manager', 'register_check', IAccountRegistrationInspector,
+        default='BasicCheck, EmailCheck, UsernamePermCheck',
+        include_missing=False,
+        doc="""Ordered list of IAccountRegistrationInspector's to use for
+        registration checks.""")
     stores = ExtensionPoint(IPasswordStore)
     change_listeners = ExtensionPoint(IAccountChangeListener)
     allow_delete_account = BoolOption(
         ignore_auth_case = self.config.getbool('trac', 'ignore_auth_case')
         return ignore_auth_case and user.lower() or user
 
+    def validate_registration(self, req):
+        """Run configured registration checks and prime account on success."""
+        for inspector in self._register_check:
+            inspector.validate_registration(req)
+
+        username = self.handle_username_casing(
+            req.args.get('username').strip())
+        name = req.args.get('name').strip()
+        email = req.args.get('email').strip()
+        # Create the user in the configured (primary) password store.
+        self.set_password(username, req.args.get('password'))
+        # Output of a successful account creation request is a made-up
+        # authenticated session, that a new user can refer to later on.
+        prime_auth_session(self.env, username)
+        # Save attributes for the user with reference to that session ID.
+        for attribute in ('name', 'email'):
+            value = req.args.get(attribute)
+            if not value:
+                continue
+            set_user_attribute(self.env, username, attribute, value)
+
     def _maybe_update_hash(self, user, password):
         if not get_user_attribute(self.env, 1, user, 'password_refreshed', 1):
             self.log.debug("Refresh password for user: %s" % user)

accountmanagerplugin/trunk/acct_mgr/model.py

                             'id': {authenticated: m.hexdigest()}}
     return res
 
+def prime_auth_session(env, username, db=None):
+    """Prime session for registered users before initial login.
+
+    These days there's no distinct user object in Trac, but users consist
+    in terms of anonymous or authenticated sessions and related session
+    attributes.  So INSERT new sid, needed as foreign key in some db schemata
+    later on, at least for PostgreSQL.
+    """
+    db = _get_db(env, db)
+    cursor = db.cursor()
+    cursor.execute("""
+        SELECT COUNT(*)
+          FROM session
+         WHERE sid=%s
+        """, (username,))
+    exists = cursor.fetchone()
+    if not exists[0]:
+        cursor.execute("""
+            INSERT INTO session
+                    (sid,authenticated,last_visit)
+            VALUES  (%s,1,0)
+            """, (username,))
+        db.commit()
+
 def set_user_attribute(env, username, attribute, value, db=None):
     """Set or update a Trac user attribute within an atomic db transaction."""
     db = _get_db(env, db)

accountmanagerplugin/trunk/acct_mgr/register.py

 
 from trac import perm, util
 from trac.core import Component, TracError, implements
-from trac.config import Configuration, BoolOption, IntOption, Option, \
-                        OrderedExtensionsOption
+from trac.config import Configuration, BoolOption, IntOption, Option
 from trac.env import open_environment
 from trac.web import auth, chrome
 from trac.web.main import IRequestHandler, IRequestFilter
 from acct_mgr.util import containsAny, is_enabled
 
 
-def _create_user(req, env, check_permissions=True):
-    acctmgr = AccountManager(env)
-    username = acctmgr.handle_username_casing(req.args.get('username').strip())
-    name = req.args.get('name').strip()
-    email = req.args.get('email').strip()
-    acctmgr.set_password(username, req.args.get('password'))
-
-    # INSERT new sid, needed as foreign key in some db schemata later on,
-    # at least for PostgreSQL.
-    db = env.get_db_cnx()
-    cursor = db.cursor()
-    cursor.execute("""
-        SELECT  COUNT(*)
-        FROM    session
-        WHERE   sid=%s
-        """, (username,))
-    exists = cursor.fetchone()
-    if not exists[0]:
-        cursor.execute("""
-            INSERT INTO session
-                    (sid,authenticated,last_visit)
-            VALUES  (%s,1,0)
-            """, (username,))
-
-    for attribute in ('name', 'email'):
-        value = req.args.get(attribute)
-        if not value:
-            continue
-        set_user_attribute(env, username, attribute, value)
-
-
 class RegistrationError(TracError):
     """Exception raised when a registration check fails."""
 
                 else:
                     pretty_blacklist = tag(pretty_blacklist,
                                            ', \'', tag.b(c), '\'')
-            raise RegistrationError(Markup(tag(_(
+            raise RegistrationError(tag(_(
                 "The username must not contain any of these characters:"),
-                pretty_blacklist)))
+                pretty_blacklist))
 
         # Prohibit some user names, that are important for Trac and therefor
         # reserved, even if not in the permission store for some reason.
         if username.lower() in ['anonymous', 'authenticated']:
-            raise RegistrationError(Markup(_("Username %s is not allowed.")
-                                           % tag.b(username)))
+            raise RegistrationError(_("Username %s is not allowed.")
+                                    % tag.b(username))
 
         # NOTE: A user may exist in a password store but not in the permission
         #   store.  I.e. this happens, when the user (from the password store)
         for store_user in acctmgr.get_users():
             # Do it carefully by disregarding case.
             if store_user.lower() == username.lower():
-                raise RegistrationError(Markup(_("""
+                raise RegistrationError(_("""
                     Another account or group already exists, who's name
                     differs from %s only by case or is identical.
-                    """) % tag.b(username)))
+                    """) % tag.b(username))
 
         # Password consistency checks follow.
         password = req.args.get('password')
         for (perm_user, perm_action) in \
                 perm.PermissionSystem(self.env).get_all_permissions():
             if perm_user.lower() == username.lower():
-                raise RegistrationError(Markup(_("""
+                raise RegistrationError(_("""
                     Another account or group already exists, who's name
                     differs from %s only by case or is identical.
-                    """) % tag.b(username)))
+                    """) % tag.b(username))
 
 
 class RegistrationModule(CommonTemplateProvider):
 
     implements(chrome.INavigationContributor, IRequestHandler)
 
-    _register_check = OrderedExtensionsOption(
-        'account-manager', 'register_check', IAccountRegistrationInspector,
-        default='BasicCheck, EmailCheck, UsernamePermCheck',
-        include_missing=False,
-        doc="""Ordered list of IAccountRegistrationInspector's to use for
-        registration checks.""")
-
     def __init__(self):
         self.acctmgr = AccountManager(self.env)
         self._enable_check(log=True)
         data['verify_account_enabled'] = verify_enabled
         if req.method == 'POST' and action == 'create':
             try:
-                for inspector in self._register_check:
-                    inspector.validate_registration(req)
-                _create_user(req, self.env)
+                # Check request and prime account on success.
+                self.acctmgr.validate_registration(req)
             except RegistrationError, e:
-                chrome.add_warning(req, e.message)
+                chrome.add_warning(req, Markup(e.message))
                 account = {
                     'username': self.acctmgr.handle_username_casing(
                                     req.args.get('username').strip()),
                 req.redirect(req.href.login())
         # Collect additional fields from IAccountRegistrationInspector's.
         fragments = dict(required=[], optional=[])
-        for inspector in self._register_check:
+        for inspector in self.acctmgr._register_check:
             try:
                 fragment, f_data = inspector.render_registration_fields(req)
             except TypeError, e:

accountmanagerplugin/trunk/changelog

  * do AccountManager class API cleanup by moving db access to model layer
 
  new features
+ * #874: Add new fields to register form and a registration validation system
  * #9618: HttpAuthStore authentication enhancement
    by allowing a relative URL for `authentication_url` configuration option
  * #9676: Incorporate optional Single-Sign-On functionality