Merge pull request #4425 from creviera/issue-4251

Add first and last name to journalist table and web app

securedrop/alembic/versions/a9fe328b053a_migrations_for_0_14_0.py

@@ -0,0 +1,28 @@
+"""Migrations for SecureDrop's 0.14.0 release
+
+Revision ID: a9fe328b053a
+Revises: b58139cfdc8c
+Create Date: 2019-05-21 20:23:30.005632
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'a9fe328b053a'
+down_revision = 'b58139cfdc8c'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('journalists', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('first_name', sa.String(length=255), nullable=True))
+        batch_op.add_column(sa.Column('last_name', sa.String(length=255), nullable=True))
+
+
+def downgrade():
+    with op.batch_alter_table('journalists', schema=None) as batch_op:
+        batch_op.drop_column('last_name')
+        batch_op.drop_column('first_name')

securedrop/alembic/versions/b58139cfdc8c_add_checksum_columns_revoke_table.py

@@ -1,9 +1,7 @@
 """add checksum columns and revoke token table
-
 Revision ID: b58139cfdc8c
 Revises: f2833ac34bb6
 Create Date: 2019-04-02 10:45:05.178481
-
 """
 import os
 from alembic import op

securedrop/journalist_app/account.py

@@ -5,8 +5,8 @@
 from flask_babel import gettext
 
 from db import db
-from journalist_app.utils import (make_password, set_diceware_password,
-                                  validate_user, validate_hotp_secret)
+from journalist_app.utils import (make_password, set_diceware_password, set_name, validate_user,
+                                  validate_hotp_secret)
 
 
 def make_blueprint(config):
@@ -18,6 +18,13 @@ def edit():
         return render_template('edit_account.html',
                                password=password)
 
+    @view.route('/change-name', methods=('POST',))
+    def change_name():
+        first_name = request.form.get('first_name')
+        last_name = request.form.get('last_name')
+        set_name(g.user, first_name, last_name)
+        return redirect(url_for('account.edit'))
+
     @view.route('/new-password', methods=('POST',))
     def new_password():
         user = g.user

securedrop/journalist_app/admin.py

@@ -9,10 +9,10 @@
 from sqlalchemy.orm.exc import NoResultFound
 
 from db import db
-from models import Journalist, InvalidUsernameException, PasswordError
+from models import Journalist, InvalidUsernameException, FirstOrLastNameError, PasswordError
 from journalist_app.decorators import admin_required
-from journalist_app.utils import (make_password, commit_account_changes,
-                                  set_diceware_password, validate_hotp_secret)
+from journalist_app.utils import (make_password, commit_account_changes, set_diceware_password,
+                                  validate_hotp_secret)
 from journalist_app.forms import LogoForm, NewUserForm
 
 
@@ -54,6 +54,8 @@ def add_user():
         if form.validate_on_submit():
             form_valid = True
             username = request.form['username']
+            first_name = request.form['first_name']
+            last_name = request.form['last_name']
             password = request.form['password']
             is_admin = bool(request.form.get('is_admin'))
 
@@ -63,6 +65,8 @@ def add_user():
                     otp_secret = request.form.get('otp_secret', '')
                 new_user = Journalist(username=username,
                                       password=password,
+                                      first_name=first_name,
+                                      last_name=last_name,
                                       is_admin=is_admin,
                                       otp_secret=otp_secret)
                 db.session.add(new_user)
@@ -172,6 +176,22 @@ def edit_user(user_id):
                 else:
                     user.username = new_username
 
+            try:
+                first_name = request.form['first_name']
+                Journalist.check_name_acceptable(first_name)
+                user.first_name = first_name
+            except FirstOrLastNameError as e:
+                flash(gettext('Name not updated: {}'.format(e)), "error")
+                return redirect(url_for("admin.edit_user", user_id=user_id))
+
+            try:
+                last_name = request.form['last_name']
+                Journalist.check_name_acceptable(last_name)
+                user.last_name = last_name
+            except FirstOrLastNameError as e:
+                flash(gettext('Name not updated: {}'.format(e)), "error")
+                return redirect(url_for("admin.edit_user", user_id=user_id))
+
             user.is_admin = bool(request.form.get('is_admin'))
 
             commit_account_changes(user)

securedrop/journalist_app/forms.py

@@ -30,11 +30,20 @@ def minimum_length_validation(form, field):
                         num_chars=len(field.data))))
 
 
+def name_length_validation(form, field):
+    if len(field.data) > Journalist.MAX_NAME_LEN:
+        raise ValidationError(gettext(
+            'Field can not be more than {max_chars} characters.'
+            .format(max_chars=Journalist.MAX_NAME_LEN)))
+
+
 class NewUserForm(FlaskForm):
     username = TextField('username', validators=[
         InputRequired(message=gettext('This field is required.')),
         minimum_length_validation
     ])
+    first_name = TextField('first_name', validators=[name_length_validation, Optional()])
+    last_name = TextField('last_name', validators=[name_length_validation, Optional()])
     password = HiddenField('password')
     is_admin = BooleanField('is_admin')
     is_hotp = BooleanField('is_hotp')

securedrop/journalist_app/utils.py

@@ -10,10 +10,9 @@
 import i18n
 
 from db import db
-from models import (get_one_or_else, Source, Journalist,
-                    InvalidUsernameException, WrongPasswordException,
-                    LoginThrottledException, BadTokenException, SourceStar,
-                    PasswordError, Submission, RevokedToken)
+from models import (get_one_or_else, Source, Journalist, InvalidUsernameException,
+                    WrongPasswordException, FirstOrLastNameError, LoginThrottledException,
+                    BadTokenException, SourceStar, PasswordError, Submission, RevokedToken)
 from rm import srm
 from store import add_checksum_for_file
 from worker import rq_worker_queue
@@ -273,6 +272,15 @@ def delete_collection(filesystem_id):
     return job
 
 
+def set_name(user, first_name, last_name):
+    try:
+        user.set_name(first_name, last_name)
+        db.session.commit()
+        flash(gettext('Name updated.'), "success")
+    except FirstOrLastNameError as e:
+        flash(gettext('Name not updated: {}'.format(e)), "error")
+
+
 def set_diceware_password(user, password):
     try:
         user.set_password(password)

securedrop/journalist_templates/admin_add_user.html

@@ -8,23 +8,51 @@
   <p>
     {{ form.password(value=password, id=False) }}
   </p>
-<div class="container flex-end">
-  <div>
-  <p>
-    {{ form.username(placeholder=gettext('Username')) }}
-    <br>
-    {% for error in form.username.errors %}
-      <span class="form-validation-error">{{ error }}</span>
-    {% endfor %}
-  </p>
+  <div class="container flex-end">
+    <div>
+      <p>
+        <label for="first_name">{{ gettext('Username') }}</label>
+        {{ form.username() }}
+        <br>
+        {% for error in form.username.errors %}
+          <span class="form-validation-error">{{ error }}</span>
+        {% endfor %}
+      </p>
+    </div>
+    <div>
+        <ul class="journalist-username__notes">
+            <li>{{gettext("Username can contain spaces")}}</li>
+            <li>{{gettext("Username is case-sensitive")}}</li>
+        </ul>
+    </div>
   </div>
-  <div>
-      <ul class="journalist-username__notes">
-          <li>{{gettext("Username can contain spaces")}}</li>
-          <li>{{gettext("Username is case-sensitive")}}</li>
-      </ul>
+  <div class="container flex-end">
+    <div>
+      <p>
+        <label for="first_name">{{ gettext('First name') }}</label>
+        {{ form.first_name() }}
+        <br>
+        {% for error in form.first_name.errors %}
+          <span class="form-validation-error">{{ error }}</span>
+        {% endfor %}
+      </p>
+    </div>
+    <div>
+      <p>
+        <label for="last_name">{{ gettext('Last name') }}</label>
+        {{ form.last_name() }}
+        <br>
+        {% for error in form.last_name.errors %}
+          <span class="form-validation-error">{{ error }}</span>
+        {% endfor %}
+      </p>
+    </div>
+    <div>
+        <ul class="journalist-username__notes">
+            <li>{{gettext("First name and last name are optional")}}</li>
+        </ul>
+    </div>
   </div>
-</div>
   <p>{{ gettext("The user's password will be:") }} <span class="password" id="password">{{ password }}</span></p>
   <p>
     {{ form.is_admin(id="is-admin") }}

securedrop/journalist_templates/edit_account.html

@@ -4,13 +4,21 @@
 {% if user %}
   {# Only admins may edit usernames and admin status #}
   <h1>{{ gettext('Edit user "{user}"').format(user=user.username) }}</h1>
-  <p><a href="/admin">« {{ gettext('Back to admin interface') }}</a></p>
-  <h2>{{ gettext('Change Username &amp; Admin Status') }}</h2>
+  <p><a href="/admin">{{ gettext('Back to admin interface') }}</a></p>
+  <h2>{{ gettext('Change Name &amp; Admin Status') }}</h2>
   <form method="post">
     <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
     <p>
-      <label for="username">{{ gettext('Change username') }}</label>
-      <input name="username" id="username" type="text" placeholder="{{ user.username }}">
+      <label for="username">{{ gettext('Username') }}</label>
+      <input name="username" id="username" type="text" value="{{ user.username }}">
+    </p>
+    <p>
+      <label for="first_name">{{ gettext('First name') }}</label>
+      {% set first_name = user.first_name or '' %}
+      <input name="first_name" id="first_name" type="text" value="{{ first_name }}">
+      <label for="last_name">{{ gettext('Last name') }}</label>
+      {% set last_name = user.last_name or '' %}
+      <input name="last_name" id="last_name" type="text" value="{{ last_name }}">
     </p>
     <p>
       <input name="is_admin" id="is-admin" type="checkbox" {% if user.is_admin %}checked{% endif %}>
@@ -20,6 +28,20 @@ <h2>{{ gettext('Change Username &amp; Admin Status') }}</h2>
   </form>
 {% else %}
   <h1>{{ gettext('Edit your account') }}</h1>
+  <h2>{{ gettext('Change Name') }}</h2>
+  {% set change_name_url = url_for('account.change_name') %}
+  <form action="{{ change_name_url }}" method="post" id="change-name">
+    <p>
+      <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
+      <label for="first_name">{{ gettext('First name') }}</label>
+      {% set first_name = g.user.first_name or '' %}
+      <input name="first_name" id="first_name" value = "{{ first_name }}">
+      <label for="last_name">{{ gettext('Last name') }}</label>
+      {% set last_name = g.user.last_name or '' %}
+      <input name="last_name" id="last_name" value = "{{ last_name }}"">
+    </p>
+    <button class="sd-button" type="submit" id="change-name">{{ gettext('UPDATE') }}</button>
+  </form>
 {% endif %}
 
 <h2>{{ gettext('Reset Password') }}</h2>
@@ -35,8 +57,7 @@ <h2>{{ gettext('Reset Password') }}</h2>
 {% endif %}
 
 <form action="{{ password_reset_url }}" method="post" id="new-password" class="login-form">
-  <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
-  {% if not user or g.user == user %}
+   {% if not user or g.user == user %}
   <p><input type="password" name="current_password" placeholder="{{ gettext('Current Password') }}"></p>
   <p><input name="token" id="token" type="text" placeholder="{{ gettext('Two-factor Code') }}"></p>
   {% endif %}

securedrop/manage.py

@@ -26,7 +26,7 @@
 import journalist_app
 
 from db import db
-from models import Source, Journalist, PasswordError, InvalidUsernameException
+from models import Source, Journalist, PasswordError, InvalidUsernameException, FirstOrLastNameError
 from management.run import run
 
 logging.basicConfig(format='%(asctime)s %(levelname)s %(message)s')
@@ -125,6 +125,30 @@ def _get_username():
             return username
 
 
+def _get_first_name():
+    while True:
+        first_name = obtain_input('First name: ')
+        if not first_name:
+            return None
+        try:
+            Journalist.check_name_acceptable(first_name)
+            return first_name
+        except FirstOrLastNameError as e:
+            print('Invalid name: ' + str(e))
+
+
+def _get_last_name():
+    while True:
+        last_name = obtain_input('Last name: ')
+        if not last_name:
+            return None
+        try:
+            Journalist.check_name_acceptable(last_name)
+            return last_name
+        except FirstOrLastNameError as e:
+            print('Invalid name: ' + str(e))
+
+
 def _get_yubikey_usage():
     '''Function used to allow for test suite mocking'''
     while True:
@@ -151,6 +175,8 @@ def _make_password():
 def _add_user(is_admin=False):
     with app_context():
         username = _get_username()
+        first_name = _get_first_name()
+        last_name = _get_last_name()
 
         print("Note: Passwords are now autogenerated.")
         password = _make_password()
@@ -175,6 +201,8 @@ def _add_user(is_admin=False):
 
         try:
             user = Journalist(username=username,
+                              first_name=first_name,
+                              last_name=last_name,
                               password=password,
                               is_admin=is_admin,
                               otp_secret=otp_secret)