major security fix: a check in the userform processing was not working as expected and could be abused for ACL and superuser priviledge escalation. New: establish a session after user creation, no need to immediately enter same name/password again.
authorThomas Waldmann <tw AT waldmann-edv DOT de>
Sun, 20 Apr 2008 18:37:03 +0200
changeset 2630f405012e67af
parent 2629 fa7e99b19a43
child 2631 b70e98cb53b8
major security fix: a check in the userform processing was not working as expected and could be abused for ACL and superuser priviledge escalation. New: establish a session after user creation, no need to immediately enter same name/password again.
MoinMoin/userform.py
docs/CHANGES
     1.1 --- a/MoinMoin/userform.py	Sun Apr 20 18:08:43 2008 +0200
     1.2 +++ b/MoinMoin/userform.py	Sun Apr 20 18:37:03 2008 +0200
     1.3 @@ -10,6 +10,7 @@
     1.4  import time
     1.5  from MoinMoin import user, util, wikiutil
     1.6  from MoinMoin.widget import html
     1.7 +from MoinMoin.auth import moin_session
     1.8  
     1.9  _debug = 0
    1.10  
    1.11 @@ -95,37 +96,31 @@
    1.12              form.has_key('create_and_mail')):
    1.13              if self.request.request_method != 'POST':
    1.14                  return _("Use UserPreferences to change your settings or create an account.", formatted=False)
    1.15 -            
    1.16 +
    1.17              from MoinMoin.security.textcha import TextCha
    1.18              if not TextCha(self.request).check_answer_from_form():
    1.19                  return _('TextCha: Wrong answer! Go back and try again...', formatted=False)
    1.20  
    1.21              # Create user profile
    1.22 -            if form.has_key('create'):
    1.23 -                theuser = self.request.get_user_from_form()
    1.24 -            else:
    1.25 -                theuser = user.User(self.request, auth_method="request:152")
    1.26 +            theuser = user.User(self.request, auth_method='new-user')
    1.27  
    1.28              # Require non-empty name
    1.29 -            try:
    1.30 -                theuser.name = form['name'][0]
    1.31 -            except KeyError:
    1.32 +            name = form.get('name', [''])[0]
    1.33 +            if not name:
    1.34                  return _("Empty user name. Please enter a user name.", formatted=False)
    1.35  
    1.36              # Don't allow users with invalid names
    1.37 -            if not user.isValidName(self.request, theuser.name):
    1.38 +            if not user.isValidName(self.request, name):
    1.39                  return _("""Invalid user name {{{'%s'}}}.
    1.40  Name may contain any Unicode alpha numeric character, with optional one
    1.41 -space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
    1.42 +space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
    1.43  
    1.44 -            # Is this an existing user trying to change information or a new user?
    1.45 -            # Name required to be unique. Check if name belong to another user.
    1.46 -            newuser = 1
    1.47 -            if user.getUserId(self.request, theuser.name):
    1.48 -                if theuser.name != self.request.user.name:
    1.49 -                    return _("This user name already belongs to somebody else.", formatted=False)
    1.50 -                else:
    1.51 -                    newuser = 0
    1.52 +            # new user
    1.53 +            if user.getUserId(self.request, name): # a userid for new name exists
    1.54 +                return _("This user name already belongs to somebody else.", formatted=False)
    1.55 +
    1.56 +            # all checks ok, assign name
    1.57 +            theuser.name = name
    1.58  
    1.59              # try to get the password and pw repeat
    1.60              password = form.get('password', [''])[0]
    1.61 @@ -135,14 +130,13 @@
    1.62              if password != password2:
    1.63                  return _("Passwords don't match!", formatted=False)
    1.64  
    1.65 -            if newuser:
    1.66 -                if not password:
    1.67 -                    return _("Please specify a password!", formatted=False)
    1.68 -                pw_checker = self.request.cfg.password_checker
    1.69 -                if pw_checker:
    1.70 -                    pw_error = pw_checker(theuser.name, password)
    1.71 -                    if pw_error:
    1.72 -                        return _("Password not acceptable: %s", formatted=False) % pw_error
    1.73 +            if not password:
    1.74 +                return _("Please specify a password!", formatted=False)
    1.75 +            pw_checker = self.request.cfg.password_checker
    1.76 +            if pw_checker:
    1.77 +                pw_error = pw_checker(theuser.name, password)
    1.78 +                if pw_error:
    1.79 +                    return _("Password not acceptable: %s", formatted=False) % pw_error
    1.80  
    1.81              # Encode password
    1.82              if password and not password.startswith('{SHA}'):
    1.83 @@ -154,21 +148,28 @@
    1.84  
    1.85              # try to get the (required) email
    1.86              email = wikiutil.clean_input(form.get('email', [''])[0])
    1.87 -            theuser.email = email.strip()
    1.88 -            if not theuser.email:
    1.89 +            email = email.strip()
    1.90 +
    1.91 +            if not email:
    1.92                  return _("Please provide your email address. If you lose your"
    1.93                           " login information, you can get it by email.", formatted=False)
    1.94  
    1.95              # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
    1.96 -            if theuser.email and self.request.cfg.user_email_unique:
    1.97 -                if user.get_by_email_address(self.request, theuser.email):
    1.98 +            if self.request.cfg.user_email_unique:
    1.99 +                email_user = user.get_by_email_address(self.request, email)
   1.100 +                if email_user:
   1.101                      return _("This email already belongs to somebody else.", formatted=False)
   1.102  
   1.103 +            theuser.email = email
   1.104 +
   1.105              # save data
   1.106              theuser.save()
   1.107              if form.has_key('create_and_mail'):
   1.108                  theuser.mailAccountData()
   1.109  
   1.110 +            self.request.user = theuser
   1.111 +            moin_session(self.request, user_obj=theuser) # establish session (saves having to do extra login)
   1.112 +
   1.113              result = _("User account created! You can use this account to login now...", formatted=False)
   1.114              if _debug:
   1.115                  result = result + util.dumpFormData(form)
   1.116 @@ -191,10 +192,11 @@
   1.117                      self.request.user = self.request._setuid_real_user
   1.118                      self.request._setuid_real_user = None
   1.119                  else:
   1.120 +                    if self.request._setuid_real_user is None:
   1.121 +                        self.request._setuid_real_user = self.request.user
   1.122                      theuser = user.User(self.request, uid)
   1.123                      theuser.disabled = None
   1.124                      self.request.session['setuid'] = uid
   1.125 -                    self.request._setuid_real_user = self.request.user
   1.126                      # now continue as the other user
   1.127                      self.request.user = theuser
   1.128                  return  _("Use UserPreferences to change settings of the selected user account", formatted=False)
   1.129 @@ -204,28 +206,31 @@
   1.130          if form.has_key('save'): # Save user profile
   1.131              if self.request.request_method != 'POST':
   1.132                  return _("Use UserPreferences to change your settings or create an account.", formatted=False)
   1.133 -            theuser = self.request.get_user_from_form()
   1.134 +            theuser = self.request.user                 # this is either just the currently logged in (normal) user OR
   1.135 +                                                        # the (normal) user that the superuser su'd to
   1.136 +            newuser = False
   1.137 +            if not 'name' in theuser.auth_attribs:
   1.138 +                name = form.get('name', [theuser.name])[0]
   1.139 +                # Require non-empty name
   1.140 +                if not name:
   1.141 +                    return _("Empty user name. Please enter a user name.", formatted=False)
   1.142  
   1.143 -            if not 'name' in theuser.auth_attribs:
   1.144 -                # Require non-empty name
   1.145 -                theuser.name = form.get('name', [theuser.name])[0]
   1.146 -            if not theuser.name:
   1.147 -                return _("Empty user name. Please enter a user name.", formatted=False)
   1.148 +                # Don't allow users with invalid names
   1.149 +                if not user.isValidName(self.request, name):
   1.150 +                    return _("""Invalid user name {{{'%s'}}}.
   1.151 +Name may contain any Unicode alpha numeric character, with optional one
   1.152 +space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
   1.153  
   1.154 -            # Don't allow users with invalid names
   1.155 -            if not user.isValidName(self.request, theuser.name):
   1.156 -                return _("""Invalid user name {{{'%s'}}}.
   1.157 -Name may contain any Unicode alpha numeric character, with optional one
   1.158 -space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
   1.159 +                # Is this an existing user trying to change information or a new user?
   1.160 +                # Name required to be unique. Check if name belongs to another user.
   1.161 +                if name != theuser.name:
   1.162 +                    # either new user or user wants to change his name (edited the name form field)
   1.163 +                    if user.getUserId(self.request, name): # a userid for new name exists
   1.164 +                        return _("This user name already belongs to somebody else.", formatted=False)
   1.165 +                    newuser = not user.getUserId(self.request, theuser.name)
   1.166  
   1.167 -            # Is this an existing user trying to change information or a new user?
   1.168 -            # Name required to be unique. Check if name belong to another user.
   1.169 -            newuser = 1
   1.170 -            if user.getUserId(self.request, theuser.name):
   1.171 -                if theuser.name != self.request.user.name:
   1.172 -                    return _("This user name already belongs to somebody else.", formatted=False)
   1.173 -                else:
   1.174 -                    newuser = 0
   1.175 +                # all checks ok, assign name
   1.176 +                theuser.name = name
   1.177  
   1.178              if not 'password' in theuser.auth_attribs:
   1.179                  # try to get the password and pw repeat                
   1.180 @@ -253,25 +258,21 @@
   1.181                          # Should never happen
   1.182                          return "Can't encode password: %s" % str(err)
   1.183  
   1.184 +            # try to get the (required) email
   1.185              if not 'email' in theuser.auth_attribs:
   1.186 -                # try to get the email
   1.187                  email = wikiutil.clean_input(form.get('email', [theuser.email])[0])
   1.188 -                theuser.email = email.strip()
   1.189 +                email = email.strip()
   1.190 +                if not email:
   1.191 +                    return _("Please provide your email address. If you lose your"
   1.192 +                             " login information, you can get it by email.", formatted=False)
   1.193  
   1.194 -            # Require email
   1.195 -            if not theuser.email:
   1.196 -                return _("Please provide your email address. If you lose your"
   1.197 -                         " login information, you can get it by email.", formatted=False)
   1.198 +                # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
   1.199 +                if email and self.request.cfg.user_email_unique:
   1.200 +                    email_user = user.get_by_email_address(self.request, email)
   1.201 +                    if email_user and email_user.id != theuser.id:
   1.202 +                        return _("This email already belongs to somebody else.", formatted=False)
   1.203  
   1.204 -            # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
   1.205 -            if theuser.email and self.request.cfg.user_email_unique:
   1.206 -                users = user.getUserList(self.request)
   1.207 -                for uid in users:
   1.208 -                    if uid == theuser.id:
   1.209 -                        continue
   1.210 -                    thisuser = user.User(self.request, uid, auth_method='userform:283')
   1.211 -                    if thisuser.email == theuser.email:
   1.212 -                        return _("This email already belongs to somebody else.", formatted=False)
   1.213 +                theuser.email = email
   1.214  
   1.215              if not 'aliasname' in theuser.auth_attribs:
   1.216                  # aliasname
   1.217 @@ -358,7 +359,6 @@
   1.218  
   1.219              # save data
   1.220              theuser.save()
   1.221 -            self.request.user = theuser
   1.222  
   1.223              result = _("User preferences saved!", formatted=False)
   1.224              if _debug:
   1.225 @@ -563,7 +563,7 @@
   1.226                          if key == 'password':
   1.227                              name = 'password1'
   1.228                          else:
   1.229 -                            name = key                            
   1.230 +                            name = key
   1.231                          self.make_row(_(label, formatted=False),
   1.232                                    [html.INPUT(type=type, size=length, name=name, value=getattr(self.request.user, key)), ' ', _(textafter, formatted=False), ])
   1.233  
     2.1 --- a/docs/CHANGES	Sun Apr 20 18:08:43 2008 +0200
     2.2 +++ b/docs/CHANGES	Sun Apr 20 18:37:03 2008 +0200
     2.3 @@ -30,6 +30,10 @@
     2.4  
     2.5  Version 1.6.current:
     2.6    Fixes:
     2.7 +    * Security fix: a check in the user form processing was not working as
     2.8 +      expected, leading to a major ACL and superuser priviledge escalation
     2.9 +      problem. If you use ACL entries other than "Known:" or "All:" and/or
    2.10 +      a non-empty superuser list, you need to urgently install this upgrade.
    2.11      * Security fix: if acl_hierarchic=True was used (False is the default),
    2.12        ACL processing was wrong for some cases, see
    2.13        MoinMoinBugs/AclHierarchicPageAclSupercededByAclRightsAfter
    2.14 @@ -55,6 +59,10 @@
    2.15    Other changes:
    2.16      * Added 'notes' to config.url_schemas, so you can use notes://notessrv/...
    2.17        to invoke your Lotus Notes client.
    2.18 +    * After creating a new user profile via UserPreferences, you are logged
    2.19 +      in with that user (no need to immediately enter the same name/password
    2.20 +      again for logging in).
    2.21 +
    2.22  
    2.23  Version 1.6.2:
    2.24    Fixes: