diff --git a/freeipa_community_portal/app.py b/freeipa_community_portal/app.py index a140d61..c702e30 100644 --- a/freeipa_community_portal/app.py +++ b/freeipa_community_portal/app.py @@ -68,10 +68,12 @@ def GET(self): # pylint: disable=invalid-name def POST(self, **kwargs): # pylint: disable=invalid-name """POST /user""" + errors = [] user = User(kwargs) - errors = check_captcha(kwargs) + errors.extend(user.validate()) + errors.extend(check_captcha(kwargs)) if not errors: - errors = user.save() + errors.extend(user.save()) if not errors: # email the admin that the user has signed up SignUpMailer(user).mail() @@ -100,11 +102,14 @@ def GET(self): def POST(self, **kwargs): """accepts a username and initiates a reset""" - errors = check_captcha(kwargs) - if not errors and not kwargs['username']: - errors = "Username is required" + errors = [] + if not kwargs['username']: + errors.append("Username is required.") + errors.extend(check_captcha(kwargs)) if errors: - return render('request_reset.html', errors=errors, captcha=CaptchaHelper()) + return render('request_reset.html', + errors=errors, + captcha=CaptchaHelper()) r = PasswordReset(kwargs['username']) r.save() if r.check_valid(): @@ -153,9 +158,9 @@ def render(template, **args): def check_captcha(args): if not check_response(args['response'], args['solution']): - return "Incorrect Captcha response" + return ["Incorrect Captcha response"] else: - return None + return [] conf = { diff --git a/freeipa_community_portal/model/user.py b/freeipa_community_portal/model/user.py index da13ee7..a3b8571 100644 --- a/freeipa_community_portal/model/user.py +++ b/freeipa_community_portal/model/user.py @@ -42,6 +42,20 @@ def __init__(self, args=None): self.username = self.given_name[0] + self.family_name self.email = args.get("email", "") + def validate(self): + err = [] + if not self.given_name: + err.append('Given name is required.') + if not self.family_name: + err.append('Family name is required.') + if not self.email or "@" not in self.email: + err.append("Invalid email address.") + try: + self.check_available() + except errors.DuplicateEntry as exc: + err.append(exc.msg) + return err + def save(self): """Save the model @@ -51,14 +65,42 @@ def save(self): If there are validation errors, returns the error message. Otherwise returns None """ - error = None + err = [] try: self._call_api() except (errors.ValidationError, errors.RequirementError, - errors.DuplicateEntry) as err: - error = err.msg - return error + errors.DuplicateEntry) as exc: + err.append(exc.msg) + return err + + def check_available(self): + """Checks if the user name is still available. + + For a better user experience, this check is done in advanced. + """ + api_connect() + message = 'active user with name "%(user)s" already exists' % { + 'user': self.username + } + # Check if the username conflicts with an existing user. The check + # is not perfect. A user might be created before the stage user is + # activated. The code also suffers from a race condition. It's as + # good as it can get without a better API, though. + # see https://fedorahosted.org/freeipa/ticket/5186 + try: + api.Command.user_show(uid=self.username) + except errors.NotFound: + pass + else: + raise errors.DuplicateEntry(message=message) + + try: + api.Command.stageuser_show(uid=self.username) + except errors.NotFound: + pass + else: + raise errors.DuplicateEntry(message=message) def _call_api(self): """performs the actual API call. seperate method for testing purposes @@ -69,5 +111,5 @@ def _call_api(self): givenname=self.given_name, sn=self.family_name, uid=self.username, - mail=self.email + mail=self.email, ) diff --git a/freeipa_community_portal/templates/new_user.html b/freeipa_community_portal/templates/new_user.html index 51330c3..8745801 100644 --- a/freeipa_community_portal/templates/new_user.html +++ b/freeipa_community_portal/templates/new_user.html @@ -7,9 +7,11 @@
{{ errors }}
-{{ errors }}
-