From 57525fc2b6a1ca51205c3ccf7b8df5793fef3f0f Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 14 Jul 2021 18:56:34 +0200 Subject: [PATCH] Include project code into project authentication token Fix #780 The token used for shared links (aka authentication) includes the project password (aka code). This code is checked when reading the token. If the code is changed, the token is invalid. Test is included for that. Also, reset token are now using URLSafeTimedSerializer instead of the deprecated JWS provided by itsdangerous. The main change is that age is checked when verifying. The coolest effect is that warnings disappeared from tests. --- ihatemoney/api/common.py | 2 +- ihatemoney/models.py | 51 ++++++++++++++------ ihatemoney/templates/password_reminder.en.j2 | 2 +- ihatemoney/templates/password_reminder.fr.j2 | 2 +- ihatemoney/tests/budget_test.py | 33 +++++++++++++ ihatemoney/web.py | 4 +- 6 files changed, 73 insertions(+), 21 deletions(-) diff --git a/ihatemoney/api/common.py b/ihatemoney/api/common.py index ede76e46..70a706c4 100644 --- a/ihatemoney/api/common.py +++ b/ihatemoney/api/common.py @@ -35,7 +35,7 @@ def need_auth(f): auth_token = auth_header.split(" ")[1] except IndexError: abort(401) - project_id = Project.verify_token(auth_token, token_type="non_timed_token") + project_id = Project.verify_token(auth_token, token_type="auth") if auth_token and project_id: project = Project.query.get(project_id) if project: diff --git a/ihatemoney/models.py b/ihatemoney/models.py index 68d1fd84..aa64e4ac 100644 --- a/ihatemoney/models.py +++ b/ihatemoney/models.py @@ -7,8 +7,8 @@ from flask_sqlalchemy import BaseQuery, SQLAlchemy from itsdangerous import ( BadSignature, SignatureExpired, - TimedJSONWebSignatureSerializer, URLSafeSerializer, + URLSafeTimedSerializer, ) import sqlalchemy from sqlalchemy import orm @@ -339,41 +339,60 @@ class Project(db.Model): db.session.delete(self) db.session.commit() - def generate_token(self, expiration=0): + def generate_token(self, token_type="auth"): """Generate a timed and serialized JsonWebToken - :param expiration: Token expiration time (in seconds) + :param token_type: Either "auth" for authentication (invalidated when project code changed), + or "reset" for password reset (invalidated after expiration) """ - if expiration: - serializer = TimedJSONWebSignatureSerializer( - current_app.config["SECRET_KEY"], expiration + + if token_type == "reset": + serializer = URLSafeTimedSerializer( + current_app.config["SECRET_KEY"], salt=token_type ) - token = serializer.dumps({"project_id": self.id}).decode("utf-8") - else: - serializer = URLSafeSerializer(current_app.config["SECRET_KEY"]) token = serializer.dumps({"project_id": self.id}) + else: + serializer = URLSafeSerializer( + current_app.config["SECRET_KEY"], salt=token_type + ) + token = serializer.dumps({"project_id": self.id, "password": self.password}) + return token @staticmethod - def verify_token(token, token_type="timed_token"): + def verify_token(token, token_type="auth", max_age=3600): """Return the project id associated to the provided token, None if the provided token is expired or not valid. :param token: Serialized TimedJsonWebToken + :param token_type: Either "auth" for authentication (invalidated when project code changed), + or "reset" for password reset (invalidated after expiration) + :param max_age: Token expiration time (in seconds). Only used with token_type "reset" """ - if token_type == "timed_token": - serializer = TimedJSONWebSignatureSerializer( - current_app.config["SECRET_KEY"] + loads_kwargs = {} + if token_type == "reset": + serializer = URLSafeTimedSerializer( + current_app.config["SECRET_KEY"], salt=token_type ) + loads_kwargs["max_age"] = max_age else: - serializer = URLSafeSerializer(current_app.config["SECRET_KEY"]) + serializer = URLSafeSerializer( + current_app.config["SECRET_KEY"], salt=token_type + ) try: - data = serializer.loads(token) + data = serializer.loads(token, **loads_kwargs) except SignatureExpired: return None except BadSignature: return None - return data["project_id"] + + password = data.get("password", None) + project_id = data["project_id"] + if password is not None: + project = Project.query.get(project_id) + if project is None or project.password != password: + return None + return project_id def __str__(self): return self.name diff --git a/ihatemoney/templates/password_reminder.en.j2 b/ihatemoney/templates/password_reminder.en.j2 index c6543546..845ff790 100644 --- a/ihatemoney/templates/password_reminder.en.j2 +++ b/ihatemoney/templates/password_reminder.en.j2 @@ -1,7 +1,7 @@ Hi, You requested to reset the password of the following project: "{{ project.name }}". -You can reset it here: {{ url_for(".reset_password", _external=True, token=project.generate_token(expiration=3600)) }}. +You can reset it here: {{ url_for(".reset_password", _external=True, token=project.generate_token(token_type="reset")) }}. This link is only valid for one hour. Hope this helps, diff --git a/ihatemoney/templates/password_reminder.fr.j2 b/ihatemoney/templates/password_reminder.fr.j2 index 17c52c4d..4603a963 100644 --- a/ihatemoney/templates/password_reminder.fr.j2 +++ b/ihatemoney/templates/password_reminder.fr.j2 @@ -1,7 +1,7 @@ Salut, Vous avez demandé à réinitialiser le mot de passe du projet suivant : "{{ project.name }}". -Vous pouvez le réinitialiser ici : {{ url_for(".reset_password", _external=True, token=project.generate_token(expiration=3600)) }}. +Vous pouvez le réinitialiser ici : {{ url_for(".reset_password", _external=True, token=project.generate_token(token_type="reset")) }}. Ce lien est seulement valide pendant 1 heure. Faites-en bon usage ! diff --git a/ihatemoney/tests/budget_test.py b/ihatemoney/tests/budget_test.py index bd424433..dc862f02 100644 --- a/ihatemoney/tests/budget_test.py +++ b/ihatemoney/tests/budget_test.py @@ -92,6 +92,39 @@ class BudgetTestCase(IhatemoneyTestCase): resp = self.client.get("/authenticate?token=token") self.assertIn("You either provided a bad token", resp.data.decode("utf-8")) + def test_invite_expiration_with_code(self): + """Test that invitation link expire after code change""" + self.login("raclette") + self.post_project("raclette") + response = self.client.get("/raclette/invite").data.decode("utf-8") + base_index = response.find("share the following link") + start = response.find('href="', base_index) + 6 + end = response.find('">', base_index) + link = response[start:end] + + self.client.get("/exit") + response = self.client.get(link) + # Link is valid + assert response.status_code == 302 + + response = self.client.post( + "/raclette/edit", + data={ + "name": "raclette", + "contact_email": "zorglub@notmyidea.org", + "password": "didoudida", + "default_currency": "XXX", + }, + follow_redirects=True, + ) + assert response.status_code == 200 + assert "alert-danger" not in response.data.decode("utf-8") + + self.client.get("/exit") + response = self.client.get(link, follow_redirects=True) + # Link is invalid + self.assertIn("You either provided a bad token", response.data.decode("utf-8")) + def test_password_reminder(self): # test that it is possible to have an email containing the password of a # project in case people forget it (and it happens!) diff --git a/ihatemoney/web.py b/ihatemoney/web.py index 712d2b0e..b2d72f55 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -202,7 +202,7 @@ def authenticate(project_id=None): # Try to get project_id from token first token = request.args.get("token") if token: - project_id = Project.verify_token(token, token_type="non_timed_token") + project_id = Project.verify_token(token, token_type="auth") token_auth = True else: if not form.id.data and request.args.get("project_id"): @@ -381,7 +381,7 @@ def reset_password(): return render_template( "reset_password.html", form=form, error=_("No token provided") ) - project_id = Project.verify_token(token) + project_id = Project.verify_token(token, token_type="reset") if not project_id: return render_template( "reset_password.html", form=form, error=_("Invalid token")