Use hashed passwords for projects (#286)

- Remove all occurences of clear text project passwords.
- Migrate the database to hash the previously stored passwords.
Closes #232

7 files changed, 74 insertions, 31 deletions

diff --git a/ihatemoney/api.py b/ihatemoney/api.py
index a34fa12..82380fd 100644
--- a/ihatemoney/api.py
+++ b/ihatemoney/api.py
@@ -5,6 +5,7 @@ from flask_rest import RESTResource, need_auth
 from ihatemoney.models import db, Project, Person, Bill
 from ihatemoney.forms import (ProjectForm, EditProjectForm, MemberForm,
                               get_billform_for)
+from werkzeug.security import check_password_hash
 
 
 api = Blueprint("api", __name__, url_prefix="/api")
@@ -21,7 +22,7 @@ def check_project(*args, **kwargs):
     if auth and "project_id" in kwargs and \
             auth.username == kwargs["project_id"]:
         project = Project.query.get(auth.username)
-        if project and project.password == auth.password:
+        if project and check_password_hash(project.password, auth.password):
             return project
     return False
 
diff --git a/ihatemoney/forms.py b/ihatemoney/forms.py
index c5e0b54..3966891 100644
--- a/ihatemoney/forms.py
+++ b/ihatemoney/forms.py
@@ -5,6 +5,7 @@ from wtforms.fields.simple import PasswordField, SubmitField, TextAreaField, Str
 from wtforms.validators import Email, Required, ValidationError, EqualTo
 from flask_babel import lazy_gettext as _
 from flask import request
+from werkzeug.security import generate_password_hash
 
 from datetime import datetime
 from jinja2 import Markup
@@ -52,14 +53,14 @@ class EditProjectForm(FlaskForm):
         Returns the created instance
         """
         project = Project(name=self.name.data, id=self.id.data,
-                          password=self.password.data,
+                          password=generate_password_hash(self.password.data),
                           contact_email=self.contact_email.data)
         return project
 
     def update(self, project):
         """Update the project with the information from the form"""
         project.name = self.name.data
-        project.password = self.password.data
+        project.password = generate_password_hash(self.password.data)
         project.contact_email = self.contact_email.data
 
         return project
diff --git a/ihatemoney/migrations/versions/b78f8a8bdb16_hash_project_passwords.py b/ihatemoney/migrations/versions/b78f8a8bdb16_hash_project_passwords.py
new file mode 100644
index 0000000..e32983d
--- /dev/null
+++ b/ihatemoney/migrations/versions/b78f8a8bdb16_hash_project_passwords.py
@@ -0,0 +1,41 @@
+"""hash project passwords
+
+Revision ID: b78f8a8bdb16
+Revises: f629c8ef4ab0
+Create Date: 2017-12-17 11:45:44.783238
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'b78f8a8bdb16'
+down_revision = 'f629c8ef4ab0'
+
+from alembic import op
+import sqlalchemy as sa
+from werkzeug.security import generate_password_hash
+
+project_helper = sa.Table(
+    'project', sa.MetaData(),
+    sa.Column('id', sa.String(length=64), nullable=False),
+    sa.Column('name', sa.UnicodeText(), nullable=True),
+    sa.Column('password', sa.String(length=128), nullable=True),
+    sa.Column('contact_email', sa.String(length=128), nullable=True),
+    sa.PrimaryKeyConstraint('id')
+)
+
+
+def upgrade():
+    connection = op.get_bind()
+    for project in connection.execute(project_helper.select()):
+        connection.execute(
+            project_helper.update().where(
+                project_helper.c.name == project.name
+            ).values(
+                password=generate_password_hash(project.password)
+            )
+        )
+
+
+def downgrade():
+    # Downgrade path is not possible, because information has been lost.
+    pass
diff --git a/ihatemoney/templates/reminder_mail.en b/ihatemoney/templates/reminder_mail.en
index 5f9b7d8..8784d2a 100644
--- a/ihatemoney/templates/reminder_mail.en
+++ b/ihatemoney/templates/reminder_mail.en
@@ -2,8 +2,7 @@ Hi,
 
 You have just (or someone else using your email address) created the project "{{ g.project.name }}" to share your expenses.
 
-You can access it here: {{ url_for(".list_bills", _external=True) }} (the identifier is {{ g.project.id }}),
-and the shared password is "{{ g.project.password }}".
+You can access it here: {{ url_for(".list_bills", _external=True) }} (the identifier is {{ g.project.id }}).
 If you want to share this project with your friends, you can share the identifier and the shared password with them or send them invitations with the following link:
 {{ url_for(".invite", _external=True) }}
 
diff --git a/ihatemoney/templates/reminder_mail.fr b/ihatemoney/templates/reminder_mail.fr
index fbe299a..e73938a 100644
--- a/ihatemoney/templates/reminder_mail.fr
+++ b/ihatemoney/templates/reminder_mail.fr
@@ -2,8 +2,7 @@ Hey,
 
 Vous venez de créer le projet "{{ g.project.name }}" pour partager vos dépenses.
 
-Vous pouvez y accéder ici: {{ url_for(".list_bills", _external=True) }} (l'identifieur est {{ g.project.id }}),
-et le code d'accès "{{ g.project.password }}".
+Vous pouvez y accéder ici: {{ url_for(".list_bills", _external=True) }} (l'identifieur est {{ g.project.id }}).
 Si vous voulez partager ce projet avec vos amis, vous pouvez partager son identifiant et son code d'accès avec eux ou leur envoyer une invitation avec le lien suivant :
 {{ url_for(".invite", _external=True) }}
 
diff --git a/ihatemoney/tests/tests.py b/ihatemoney/tests/tests.py
index a421762..dc46580 100644
--- a/ihatemoney/tests/tests.py
+++ b/ihatemoney/tests/tests.py
@@ -11,7 +11,7 @@ from collections import defaultdict
 import six
 from time import sleep
 
-from werkzeug.security import generate_password_hash
+from werkzeug.security import generate_password_hash, check_password_hash
 from flask import session
 from flask_testing import TestCase
 
@@ -61,7 +61,7 @@ class BaseTestCase(TestCase):
         project = models.Project(
             id=name,
             name=six.text_type(name),
-            password=name,
+            password=generate_password_hash(name),
             contact_email="%s@notmyidea.org" % name)
         models.db.session.add(project)
         models.db.session.commit()
@@ -670,8 +670,9 @@ class BudgetTestCase(IhatemoneyTestCase):
         self.assertEqual(resp.status_code, 200)
         project = models.Project.query.get("raclette")
 
-        for key, value in new_data.items():
-            self.assertEqual(getattr(project, key), value, key)
+        self.assertEqual(project.name, new_data['name'])
+        self.assertEqual(project.contact_email, new_data['contact_email'])
+        self.assertTrue(check_password_hash(project.password, new_data['password']))
 
         # Editing a project with a wrong email address should fail
         new_data['contact_email'] = 'wrong_email'
@@ -1071,11 +1072,12 @@ class APITestCase(IhatemoneyTestCase):
             "name": "raclette",
             "contact_email": "raclette@notmyidea.org",
             "members": [],
-            "password": "raclette",
             "id": "raclette",
             "balance": {},
         }
-        self.assertDictEqual(json.loads(resp.data.decode('utf-8')), expected)
+        decoded_resp = json.loads(resp.data.decode('utf-8'))
+        self.assertTrue(check_password_hash(decoded_resp.pop('password'), 'raclette'))
+        self.assertDictEqual(decoded_resp, expected)
 
         # edit should work
         resp = self.client.put("/api/projects/raclette", data={
@@ -1095,11 +1097,12 @@ class APITestCase(IhatemoneyTestCase):
             "name": "The raclette party",
             "contact_email": "yeah@notmyidea.org",
             "members": [],
-            "password": "raclette",
             "id": "raclette",
             "balance": {},
         }
-        self.assertDictEqual(json.loads(resp.data.decode('utf-8')), expected)
+        decoded_resp = json.loads(resp.data.decode('utf-8'))
+        self.assertTrue(check_password_hash(decoded_resp.pop('password'), 'raclette'))
+        self.assertDictEqual(decoded_resp, expected)
 
         # delete should work
         resp = self.client.delete("/api/projects/raclette",
@@ -1334,11 +1337,12 @@ class APITestCase(IhatemoneyTestCase):
                 {"activated": True, "id": 2, "name": "freddy familly", "weight": 4.0},
                 {"activated": True, "id": 3, "name": "arnaud", "weight": 1.0}
             ],
-            "name": "raclette",
-            "password": "raclette"}
+            "name": "raclette"}
 
         self.assertStatus(200, req)
-        self.assertEqual(expected, json.loads(req.data.decode('utf-8')))
+        decoded_req = json.loads(req.data.decode('utf-8'))
+        self.assertTrue(check_password_hash(decoded_req.pop('password'), 'raclette'))
+        self.assertDictEqual(decoded_req, expected)
 
 
 class ServerTestCase(APITestCase):
diff --git a/ihatemoney/web.py b/ihatemoney/web.py
index c1b1093..e6df385 100644
--- a/ihatemoney/web.py
+++ b/ihatemoney/web.py
@@ -15,7 +15,7 @@ from flask import (
 )
 from flask_mail import Message
 from flask_babel import get_locale, gettext as _
-from werkzeug.security import check_password_hash
+from werkzeug.security import check_password_hash, generate_password_hash
 from smtplib import SMTPRecipientsRefused
 from werkzeug.exceptions import NotFound
 from sqlalchemy import orm
@@ -181,8 +181,7 @@ def authenticate(project_id=None):
 
     # else do form authentication or token authentication
     is_post_auth = request.method == "POST" and form.validate()
-    is_valid_password = form.password.data == project.password
-    if is_post_auth and is_valid_password or token_auth:
+    if is_post_auth and check_password_hash(project.password, form.password.data) or token_auth:
         # maintain a list of visited projects
         if "projects" not in session:
             session["projects"] = []
@@ -192,7 +191,7 @@ def authenticate(project_id=None):
         session.update()
         setattr(g, 'project', project)
         return redirect(url_for(".list_bills"))
-    if is_post_auth and not is_valid_password:
+    if is_post_auth and not check_password_hash(project.password, form.password.data):
         msg = _("This private code is not the right one")
         form.errors['password'] = [msg]
 
@@ -297,13 +296,12 @@ def reset_password():
     if not project:
         return render_template('reset_password.html', form=form, error=_("Unknown project"))
 
-    if request.method == "POST":
-        if form.validate():
-            project.password = form.password.data
-            db.session.add(project)
-            db.session.commit()
-            flash(_("Password successfully reset."))
-            return redirect(url_for(".home"))
+    if request.method == "POST" and form.validate():
+        project.password = generate_password_hash(form.password.data)
+        db.session.add(project)
+        db.session.commit()
+        flash(_("Password successfully reset."))
+        return redirect(url_for(".home"))
     return render_template('reset_password.html', form=form)
 
 
@@ -342,7 +340,6 @@ def edit_project():
                              )
     else:
         edit_form.name.data = g.project.name
-        edit_form.password.data = g.project.password
         edit_form.contact_email.data = g.project.contact_email
 
     return render_template("edit_project.html", edit_form=edit_form, export_form=export_form)
@@ -379,7 +376,8 @@ def demo():
         raise Redirect303(url_for(".create_project",
                                   project_id='demo'))
     if not project and is_demo_project_activated:
-        project = Project(id="demo", name=u"demonstration", password="demo",
+        project = Project(id="demo", name=u"demonstration",
+                          password=generate_password_hash("demo"),
                           contact_email="demo@notmyidea.org")
         db.session.add(project)
         db.session.commit()