Merge pull request #1711 from RogerHaase/1707-settings

User Settings > Personal allows creation of invalid names fixes #1707

Author: RogerHaase

docs/user/accounts.rst

@@ -54,7 +54,7 @@ wiki experience, each of these sub-pages are listed below:
 Personal Settings
 -----------------
 
-Personal settings include wiki language and locale, username and alias.
+Personal settings include wiki language and locale, name, alias and display-name.
 
 Name
  Your username, as it will appear on the login form, the history pages of wiki items
@@ -65,8 +65,10 @@ Name
  Alias names are only useful at login.
 
 Display-Name
- The display name can be used to override your username, so you will still log in using your username
- but your display name will be displayed to other users and in your history page.
+ If your wiki has a custom auth method that creates cryptic user names, then
+ the display-name can be created as an alternative. You will still login using your username
+ or alias. The display-name will appear as links in history pages and the footer of items you have edited.
+ Use your display-name to create your home page in the users namespace.
 
 Timezone
  Setting this value will display edit times converted to your local time zone. For

src/moin/apps/admin/templates/admin/userbrowser.html

@@ -1,6 +1,6 @@
 {#
     Display a table of user data collected from userprofiles metadata.
-    The report includes users name(s), email address, group memberships,
+    The report includes users name(s), display_name, email address, group memberships,
     and subscriptions. Action buttons include links to disable/enable a user's account,
     email a password reset, and display a User ACL Report.
 #}
@@ -12,6 +12,7 @@ <h1>{{ _("Users") }}</h1>
         <thead>
             <tr>
                 <th>{{ _("User name") }}</th>
+                <th>{{ _("Display name") }}</th>
                 <th>{{ _("Email address") }}</th>
                 <th class="center" colspan="3" data-sorter="false">{{ _("Actions") }}</th>
                 <th>{{ _("Groups") }}</th>
@@ -22,6 +23,7 @@ <h1>{{ _("Users") }}</h1>
             {% for u in user_accounts %}
                 <tr>
                     <td><a href="{{ url_for('frontend.show_item', item_name=u.fqname) }}">{{ u.name|join(', ') }}</a>{{ u.disabled and " (%s)" % _("disabled") or "" }}</td>
+                    <td>{{ u.display_name }}</td>
                     <td>
                         {%- if u.email -%}
                             <a href="mailto:{{ u.email|e }}" class="mailto">{{ u.email|e }}</a>

src/moin/apps/admin/views.py

@@ -31,6 +31,7 @@
 from moin import user
 from moin.constants.keys import (
     NAME,
+    DISPLAY_NAME,
     ITEMID,
     SIZE,
     EMAIL,
@@ -180,6 +181,7 @@ def userbrowser():
     user_accounts = []
     for rev in revs:
         user_names = rev.meta[NAME]
+        display_name = rev.meta.get(DISPLAY_NAME, "")
         user_groups = member_groups.get(user_names[0], [])
         for name in user_names[1:]:
             user_groups = user_groups + member_groups.get(name, [])
@@ -188,6 +190,7 @@ def userbrowser():
             dict(
                 uid=rev.meta[ITEMID],
                 name=user_names,
+                display_name=display_name,
                 fqname=CompositeName(NAMESPACE_USERS, NAME_EXACT, rev.name),
                 email=rev.meta[EMAIL] if EMAIL in rev.meta else rev.meta[EMAIL_UNVALIDATED],
                 disabled=rev.meta[DISABLED],

src/moin/apps/frontend/views.py

@@ -34,7 +34,7 @@
 
 from werkzeug.utils import secure_filename
 
-from flask import request, url_for, flash, Response, make_response, redirect, abort, jsonify
+from flask import request, url_for, flash, Response, make_response, redirect, abort, jsonify, session
 from flask import current_app as app
 from flask import g as flaskg
 from flask_babel import format_datetime
@@ -95,6 +95,7 @@
 from moin.constants.itemtypes import ITEMTYPE_DEFAULT, ITEMTYPE_TICKET
 from moin.constants.contenttypes import *  # noqa
 from moin.constants.rights import SUPERUSER
+from moin.constants.misc import FLASH_REPEAT
 from moin.utils import crypto, rev_navigation, close_file, show_time, utcfromtimestamp
 from moin.utils.crypto import make_uuid, hash_hexdigest
 from moin.utils.interwiki import url_for_item, split_fqname, CompositeName
@@ -2366,12 +2367,36 @@ def usersettings():
     # TODO: maybe "is_xhr = request.method == 'POST'" would work
     is_xhr = request.accept_mimetypes.best in ("application/json", "text/javascript")
 
+    class ValidUserSettingsPersonal(Validator):
+        """Validator for settings personal change, name, display-name"""
+
+        def validate(self, element, state):
+            invalid_id_in_use_msg = L_("This name is already in use: ")
+            invalid_character_msg = L_("The Display-Name contains invalid characters: ")
+            invalid_character_message = L_("The Username contains invalid characters: ")
+            errors = []
+            if set(form["name"].value) != set(flaskg.user.name):
+                new_names = set(form["name"].value) - set(flaskg.user.name)
+                for name in new_names:
+                    if user.search_users(**{NAME_EXACT: name}):
+                        # duplicate name
+                        errors.append(invalid_id_in_use_msg + name)
+                if not user.normalizeName(name) == name:
+                    errors.append(invalid_character_message + name)
+            display_name = form[DISPLAY_NAME].value
+            if display_name:
+                if not user.normalizeName(display_name) == display_name:
+                    errors.append(invalid_character_msg + display_name)
+            if errors:
+                return self.note_error(element, state, message=", ".join(errors))
+            return True
+
     # these forms can't be global because we need app object, which is only available within a request:
     class UserSettingsPersonalForm(Form):
         form_name = "usersettings_personal"
         name = Names.using(label=L_("Usernames")).with_properties(placeholder=L_("The login usernames you want to use"))
         display_name = OptionalText.using(label=L_("Display-Name")).with_properties(
-            placeholder=L_("Your display name (informational)")
+            placeholder=L_("Your display name (optional, rarely used)")
         )
         # _timezones_keys = sorted(Locale('en').time_zones.keys())
         _timezones_keys = [str(tz) for tz in pytz.common_timezones]
@@ -2382,6 +2407,8 @@ class UserSettingsPersonalForm(Form):
         )
         submit_label = L_("Save")
 
+        validators = [ValidUserSettingsPersonal()]
+
     class UserSettingsUIForm(Form):
         form_name = "usersettings_ui"
         theme_name = RadioChoice.using(label=L_("Theme name")).with_properties(
@@ -2437,24 +2464,6 @@ class UserSettingsUIForm(Form):
                     flaskg.user.save()
                     response["flash"].append((_("Your password has been changed."), "info"))
                 else:
-                    if part == "personal":
-                        if set(form["name"].value) != set(flaskg.user.name):
-                            new_names = set(form["name"].value) - set(flaskg.user.name)
-                            for name in new_names:
-                                if user.search_users(**{NAME_EXACT: name}):
-                                    # duplicate name
-                                    response["flash"].append(
-                                        (_("The username '{name}' is already in use.").format(name=name), "error")
-                                    )
-                                    success = False
-                                if not user.normalizeName(name) == name:
-                                    response["flash"].append(
-                                        (
-                                            _("The username '{name}' contains invalid characters").format(name=name),
-                                            "error",
-                                        )
-                                    )
-                                    success = False
                     if part == "notification":
                         if (
                             form["email"].value != flaskg.user.email
@@ -2513,9 +2522,12 @@ class UserSettingsUIForm(Form):
             else:
                 # validation failed
                 response["flash"].append((_("Nothing saved."), "error"))
+
             if not response["flash"]:
                 # if no flash message was added until here, we add a generic success message
-                response["flash"].append((_("Your changes have been saved."), "info"))
+                msg = _("Your changes have been saved.")
+                response["flash"].append((msg, "info"))
+                repeat_flash_msg(msg, "info")
 
             if response["redirect"] is not None or not is_xhr:
                 # if we redirect or it is no XHR request, we just flash() the messages normally
@@ -2544,6 +2556,15 @@ class UserSettingsUIForm(Form):
     return render_template("usersettings.html", title_name=title_name, form_objs=forms)
 
 
+def repeat_flash_msg(msg, level):
+    """
+    Add a flash message to flask session. The message will be re-flashed by the next transaction.
+    """
+    if FLASH_REPEAT not in session:
+        session[FLASH_REPEAT] = []
+    session[FLASH_REPEAT].append((msg, level))
+
+
 @frontend.route("/+bookmark")
 def bookmark():
     """set bookmark (in time) for recent changes (or delete them)"""

src/moin/config/default.py

@@ -292,11 +292,14 @@ def _default_password_checker(cfg, username, password, min_length=8, min_differe
     """
     # in any case, do a very simple built-in check to avoid the worst passwords
     if len(password) < min_length:
-        return _("For a password a minimum length of {min_length:d} characters is required.", min_length=min_length)
+        return _(
+            "For a password a minimum length of {min_length} characters is required.".format(min_length=min_length)
+        )
     if len(set(password)) < min_different:
         return _(
-            "For a password a minimum of {min_different:d} different characters is required.",
-            min_different=min_different,
+            "For a password a minimum of {min_different:d} different characters is required.".format(
+                min_different=min_different
+            )
         )
 
     username_lower = username.lower()

src/moin/constants/misc.py

@@ -73,3 +73,8 @@
 
 # Valid views allowed for itemlinks
 VALID_ITEMLINK_VIEWS = ["+meta", "+history", "+download", "+highlight"]
+
+# Transient attribute added/removed to/from flask session. Used when a User Settings
+# form creates a flash message but then redirects the page making the flash message a
+# very short flash message.
+FLASH_REPEAT = "flash_repeat"

src/moin/templates/snippets.html

@@ -84,7 +84,7 @@
             {% if cfg.show_interwiki %}
                 {{ cfg.interwikiname }}:
             {% endif %}
-            {{ item_name }} (rev {{ rev.revid | shorten_id }}),
+            {{ item_name }} (rev {{ rev.meta['rev_number'] }}),
             {{ _("modified") }} {{ rev.meta['mtime']|time_datetime }}
             {{ _("by") }} {{ utils.editor_info(rev.meta) }}
             {% if rev.meta['tags'] %}

src/moin/themes/__init__.py

@@ -17,7 +17,7 @@
 
 from flask import current_app as app
 from flask import g as flaskg
-from flask import url_for, request
+from flask import url_for, request, session, flash
 from flask_theme import get_theme, render_theme_template
 
 from babel import Locale
@@ -26,7 +26,7 @@
 from moin import wikiutil, user
 from moin.constants.keys import USERID, ADDRESS, HOSTNAME, REVID, ITEMID, NAME_EXACT, ASSIGNED_TO, NAME, NAMESPACE
 from moin.constants.contenttypes import CONTENTTYPES_MAP, CONTENTTYPE_MARKUP, CONTENTTYPE_TEXT, CONTENTTYPE_MOIN_19
-from moin.constants.misc import VALID_ITEMLINK_VIEWS
+from moin.constants.misc import VALID_ITEMLINK_VIEWS, FLASH_REPEAT
 from moin.constants.namespaces import NAMESPACE_DEFAULT, NAMESPACE_USERS, NAMESPACE_ALL
 from moin.constants.rights import SUPERUSER
 from moin.search import SearchForm
@@ -102,6 +102,10 @@ def __init__(self, cfg):
             self.wiki_root = "/" + request.url_root[len(request.host_url) : -1]
         else:
             self.wiki_root = ""
+        if FLASH_REPEAT in session:
+            for msg, level in session[FLASH_REPEAT]:
+                flash(msg, level)
+            del session[FLASH_REPEAT]
 
     def get_fullname(self, meta):
         """
@@ -655,7 +659,7 @@ def get_editor_info(meta, external=False):
     if userid:
         u = user.User(userid)
         name = u.name0
-        text = name
+        text = u.display_name or name
         display_name = u.display_name or name
         if title:
             # we already have some address info