From: Jonatan Schlag Date: Tue, 24 Oct 2017 12:54:41 +0000 (+0200) Subject: Refactor user email handling X-Git-Url: http://git.ipfire.org/?p=people%2Fjschlag%2Fpbs.git;a=commitdiff_plain;h=26fe80dfb32b5b5f4534fe4dd9efc6b8452304c6 Refactor user email handling Signed-off-by: Jonatan Schlag --- diff --git a/src/buildservice/users.py b/src/buildservice/users.py index da9a8a5..104100c 100644 --- a/src/buildservice/users.py +++ b/src/buildservice/users.py @@ -95,8 +95,9 @@ class Users(base.Object): return user def register(self, name, password, email, realname, locale=None): - user = User.new(self.pakfire, name, email, realname, locale) + user = User.new(self.pakfire, name, realname, locale) user.passphrase = password + user.add_email(email) return user def name_is_used(self, name): @@ -108,7 +109,8 @@ class Users(base.Object): return False def email_is_used(self, email): - users = self.db.query("SELECT id FROM users_emails WHERE email = %s", email) + users = self.db.query("SELECT id FROM users_emails \ + WHERE email = %s AND activated IS TRUE", email) if users: return True @@ -244,25 +246,19 @@ class User(base.Object): return cmp(self.realname, other.realname) @classmethod - def new(cls, pakfire, name , email, realname, locale=None): - id = pakfire.db.execute("INSERT INTO users(name, realname) \ - VALUES(%s, %s)", name, realname) - - # Add email address. - pakfire.db.execute("INSERT INTO users_emails(user_id, email, primary) \ - VALUES(%s, %s, 'Y')", id, email) + def new(cls, pakfire, name, realname, locale=None): + res = pakfire.db.get("INSERT INTO users(name, realname) \ + VALUES(%s, %s) RETURNING id", name, realname) # Create row in permissions table. - pakfire.db.execute("INSERT INTO users_permissions(user_id) VALUES(%s)", id) + pakfire.db.execute("INSERT INTO users_permissions(user_id) VALUES(%s)", res.id) - user = cls(pakfire, id) + user = cls(pakfire, res.id) # If we have a guessed locale, we save it (for sending emails). if locale: user.locale = locale - user.send_activation_mail() - return user @property @@ -296,9 +292,6 @@ class User(base.Object): passphrase = property(lambda x: None, set_passphrase) - @property - def activation_code(self): - return self.data.activation_code def get_realname(self): if not self.data.realname: @@ -328,34 +321,91 @@ class User(base.Object): return firstname - def get_email(self): + @property + def email(self): if self._emails is None: self._emails = self.db.query("SELECT * FROM users_emails WHERE user_id = %s", self.id) assert self._emails for email in self._emails: - if not email.primary == "Y": + if not email.primary == True: continue return email.email - def set_email(self, email): - if email == self.email: - return + def set_primary_email(self, email): + if not email in self.emails: + raise ValueError("Email address does not belong to user") - self.db.execute("UPDATE users_emails SET email = %s \ - WHERE user_id = %s AND primary = 'Y'", email, self.id) + # Mark previous primary email as non-primary + self.db.execute("UPDATE users_emails SET \"primary\" = FALSE \ + WHERE user_id = %s AND \"primary\" IS TRUE" % self.id) - self.db.execute("UPDATE users SET activated 'N' WHERE id = %s", - email, self.id) + # Mark new primary email + self.db.execute("UPDATE users_emails SET \"primary\" = TRUE \ + WHERE user_id = %s AND email = %s AND activated IS TRUE", + self.id, email) - # Reset cache. - self._data = self._emails = None + @property + def emails(self): + res = self.db.query("SELECT email FROM users_emails \ + WHERE user_id = %s AND activated IS TRUE ORDER BY email", self.id) + + return (row.email for row in res) + + def get_email_activation_code(self, email): + return self.db.query("SELECT activation_code FROM users_emails WHERE user_id = %s AND \ + email = %s", self.id, email) + + # XXX We need to check if the email is already activated + def activate_email(self, code): + if self.db.query("SELECT * FROM users_emails WHERE user_id = %s AND \ + activation_code = %s AND activated = FALSE", self.id, code): + # We activate the email and the user to + self.db.execute("UPDATE users_emails SET activated = TRUE \ + WHERE user_id = %s AND activation_code = NULL", self.id) + self.activate() + + def has_email_address(self): + emails = self.db.query("SELECT * FROM users_emails WHERE user_id = %s", self.id) + + if not emails: + return False + + return True + + # Te activated flag is useful for LDAP users + def add_email(self, email, activated=False): + # Check if the email is in use + if self.users.email_is_used(email): + raise ValueError("Email %s is already in use" % email) + + activation_code = None + if not activated: + activation_code = generate_random_string(64) + + if not self.user.has_email_address(): + # The user has no email address in the moment do we can safely guess that he has new + # registered + self.db.execute("INSERT INTO users_emails(user_id, email, \ + 'primary', activation_code, activated) VALUES(%s, %s, TRUE, %s, %s)", + self.id, email, activation_code, activated) + self.send_activation_mail() + return + + # Add just another email address. + self.db.execute("INSERT INTO users_emails(user_id, email, 'primary') \ + VALUES(%s, %s, FALSE, %s)", id, email, activation_code, activated) + self.send_email_activation_mail(email) + return - # Inform the user, that he or she has to re-activate the account. - self.send_activation_mail() + def remove_email(self, email): + # We delete this mail if the emial address is not primary and belong to this user + self.db.execute("DELETE FROM users_emails \ + WHERE id = %s AND email = %s AND 'primary' = FALSE", + self.id, email) - email = property(get_email, set_email) + self._data = self._emails = None def get_state(self): return self.data.state @@ -400,7 +450,7 @@ class User(base.Object): @property def activated(self): - return self.data.activated == "Y" + return self.data.activated @property def registered(self): @@ -446,12 +496,6 @@ class User(base.Object): def send_activation_mail(self): logging.debug("Sending activation mail to %s" % self.email) - # Generate a random activation code. - source = string.ascii_letters + string.digits - self.data["activation_code"] = "".join(random.sample(source * 20, 20)) - self.db.execute("UPDATE users SET activation_code = %s WHERE id = %s", - self.activation_code, self.id) - # Get the saved locale from the user. locale = tornado.locale.get(self.locale) _ = locale.translate @@ -470,6 +514,27 @@ class User(base.Object): self.pakfire.messages.add("%s <%s>" % (self.realname, self.email), subject, message) + def send_email_activation_mail(self, email): + logging.debug("Sending email address activation mail to %s" % email) + + # Get the saved locale from the user. + locale = tornado.locale.get(self.locale) + _ = locale.translate + + subject = _("Email address Activation") + + message = _("You, or somebody using your email address, has add this email address to an account on the Pakfire Build Service.") + message += "\n"*2 + message += _("To activate your this email address account, please click on the link below.") + message += "\n"*2 + message += " %(baseurl)s/user/%(name)s/activate?code=%(activation_code)s" \ + % { "baseurl" : self.settings.get("baseurl"), "name" : self.name, + "activation_code" : self.get_email_activation_code(email), } + message += "\n"*2 + message += "Sincerely,\n The Pakfire Build Service" + + self.pakfire.messages.add("%s <%s>" % (self.realname, email), subject, message) + # Some testing code. if __name__ == "__main__": diff --git a/src/database.sql b/src/database.sql index bb400d5..294925a 100644 --- a/src/database.sql +++ b/src/database.sql @@ -1899,13 +1899,12 @@ CREATE TABLE users ( id integer NOT NULL, name text NOT NULL, realname text, - passphrase text NOT NULL, - state users_state NOT NULL, + passphrase text, + state users_state DEFAULT 'user'::users_state NOT NULL, locale text, timezone text, - activated users_activated DEFAULT 'N'::users_activated NOT NULL, - activation_code text, - deleted users_deleted DEFAULT 'N'::users_deleted NOT NULL, + activated boolean DEFAULT false NOT NULL, + deleted boolean DEFAULT false NOT NULL, registered timestamp without time zone DEFAULT now() NOT NULL ); @@ -1920,7 +1919,9 @@ CREATE TABLE users_emails ( id integer NOT NULL, user_id integer NOT NULL, email text NOT NULL, - "primary" users_emails_primary DEFAULT 'N'::users_emails_primary NOT NULL + "primary" boolean DEFAULT false NOT NULL, + activated boolean DEFAULT false NOT NULL, + activation_code text ); diff --git a/src/templates/user-profile-edit.html b/src/templates/user-profile-edit.html index abe39cb..f3533c2 100644 --- a/src/templates/user-profile-edit.html +++ b/src/templates/user-profile-edit.html @@ -55,18 +55,26 @@ + + +
+ {{ _("Email Adresses") }}
- +
- + - {{ _("If the email address is changed, your account will be disabled until you confirm the new email address.") }} + {{ _("This email address will be used for all messages to you.") }}
+
{% if current_user.is_admin() %} diff --git a/src/web/handlers_auth.py b/src/web/handlers_auth.py index f28c1f1..38bfea9 100644 --- a/src/web/handlers_auth.py +++ b/src/web/handlers_auth.py @@ -105,8 +105,7 @@ class ActivationHandler(BaseHandler): code = self.get_argument("code") # Check if the activation code matches and then activate the account. - if user.activation_code == code: - user.activate() + if user.activate_email(code): # If an admin activated another account, he impersonates it. if self.current_user and self.current_user.is_admin(): diff --git a/src/web/handlers_users.py b/src/web/handlers_users.py index 344d1f0..56db347 100644 --- a/src/web/handlers_users.py +++ b/src/web/handlers_users.py @@ -158,51 +158,52 @@ class UserEditHandler(BaseHandler): if not user: raise tornado.web.HTTPError(404) - email = self.get_argument("email", user.email) - realname = self.get_argument("realname", user.realname) - pass1 = self.get_argument("pass1", None) - pass2 = self.get_argument("pass2", None) - locale = self.get_argument("locale", "") - - # Only an admin can alter the state of a user. - if self.current_user.is_admin(): - state = self.get_argument("state", user.state) - else: - state = user.state - - # Collect error messages - msgs = [] - - if not email: - msgs.append(_("No email address provided.")) - elif not "@" in email: - msgs.append(_("Email address is invalid.")) - - # Check if the passphrase is okay. - if pass1 and not len(pass1) >= 8: - msgs.append(_("Password has less than 8 characters.")) - elif not pass1 == pass2: - msgs.append(_("Passwords do not match.")) - - if msgs: - self.render("user-profile-edit-fail.html", messages=msgs) - return - - # Everything is okay, we can save the new settings. - user.locale = locale - user.email = email - user.realname = realname - if pass1: - user.passphrase = pass1 - user.state = state - - # Get the timezone settings. - tz = self.get_argument("timezone", None) - user.timezone = tz - - if not user.activated: - self.render("user-profile-need-activation.html", user=user) - return + with self.db.transaction(): + email = self.get_argument("primary_email_address") + realname = self.get_argument("realname", user.realname) + pass1 = self.get_argument("pass1", None) + pass2 = self.get_argument("pass2", None) + locale = self.get_argument("locale", "") + + # Only an admin can alter the state of a user. + if self.current_user.is_admin(): + state = self.get_argument("state", user.state) + else: + state = user.state + + # Collect error messages + msgs = [] + + if not email: + msgs.append(_("No email address provided.")) + elif not "@" in email: + msgs.append(_("Email address is invalid.")) + + # Check if the passphrase is okay. + if pass1 and not len(pass1) >= 8: + msgs.append(_("Password has less than 8 characters.")) + elif not pass1 == pass2: + msgs.append(_("Passwords do not match.")) + + if msgs: + self.render("user-profile-edit-fail.html", messages=msgs) + return + + # Everything is okay, we can save the new settings. + user.locale = locale + user.set_primary_email(email) + user.realname = realname + if pass1: + user.passphrase = pass1 + user.state = state + + # Get the timezone settings. + tz = self.get_argument("timezone", None) + user.timezone = tz + + if not user.activated: + self.render("user-profile-need-activation.html", user=user) + return self.redirect("/user/%s" % user.name)