From 7d9226745f3d1ceb25f3dc31fccd222d171694fa Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 13 Oct 2021 22:00:38 +0200 Subject: [PATCH] Change token path authentication to /PROJECT/join/TOKEN (#843) --- ihatemoney/templates/invitation_mail.en.j2 | 2 +- ihatemoney/templates/invitation_mail.fr.j2 | 2 +- ihatemoney/templates/send_invites.html | 4 +- ihatemoney/tests/api_test.py | 4 +- ihatemoney/tests/budget_test.py | 22 +++++---- ihatemoney/web.py | 53 +++++++++++----------- 6 files changed, 44 insertions(+), 43 deletions(-) diff --git a/ihatemoney/templates/invitation_mail.en.j2 b/ihatemoney/templates/invitation_mail.en.j2 index 2b3157b1..bb38b9aa 100644 --- a/ihatemoney/templates/invitation_mail.en.j2 +++ b/ihatemoney/templates/invitation_mail.en.j2 @@ -4,7 +4,7 @@ Someone using the email address {{ g.project.contact_email }} invited you to sha It's as simple as saying what did you pay for, for whom, and how much did it cost you, we are caring about the rest. -You can log in using this link: {{ url_for(".authenticate", _external=True, project_id=g.project.id, token=g.project.generate_token()) }}. +You can log in using this link: {{ url_for(".join_project", _external=True, project_id=g.project.id, token=g.project.generate_token()) }}. Once logged-in, you can use the following link which is easier to remember: {{ url_for(".list_bills", _external=True) }} If your cookie gets deleted or if you log out, you will need to log back in using the first link. diff --git a/ihatemoney/templates/invitation_mail.fr.j2 b/ihatemoney/templates/invitation_mail.fr.j2 index d095cfdb..0cf02f43 100644 --- a/ihatemoney/templates/invitation_mail.fr.j2 +++ b/ihatemoney/templates/invitation_mail.fr.j2 @@ -4,7 +4,7 @@ Quelqu'un dont l'adresse email est {{ g.project.contact_email }} vous a invité Il suffit de renseigner qui a payé pour quoi, pour qui, combien ça a coûté, et on s’occupe du reste. -Vous pouvez vous connecter grâce à ce lien : {{ url_for(".authenticate", _external=True, project_id=g.project.id, token=g.project.generate_token()) }}. +Vous pouvez vous connecter grâce à ce lien : {{ url_for(".join_project", _external=True, project_id=g.project.id, token=g.project.generate_token()) }}. Une fois connecté, vous pourrez utiliser le lien suivant qui est plus facile à mémoriser : {{ url_for(".list_bills", _external=True) }} Si vous êtes déconnecté volontairement ou non, vous devrez utiliser à nouveau le premier lien. diff --git a/ihatemoney/templates/send_invites.html b/ihatemoney/templates/send_invites.html index 8b73b175..d5112926 100644 --- a/ihatemoney/templates/send_invites.html +++ b/ihatemoney/templates/send_invites.html @@ -21,8 +21,8 @@ {{ _("You can directly share the following link via your prefered medium") }}
- - {{ url_for(".authenticate", _external=True, project_id=g.project.id, token=g.project.generate_token()) }} + + {{ url_for(".join_project", _external=True, project_id=g.project.id, token=g.project.generate_token()) }} diff --git a/ihatemoney/tests/api_test.py b/ihatemoney/tests/api_test.py index 83d5aa2a..9c496873 100644 --- a/ihatemoney/tests/api_test.py +++ b/ihatemoney/tests/api_test.py @@ -213,9 +213,7 @@ class APITestCase(IhatemoneyTestCase): "/api/projects/raclette/token", headers=self.get_auth("raclette") ) decoded_resp = json.loads(resp.data.decode("utf-8")) - resp = self.client.get( - f"/authenticate?token={decoded_resp['token']}&project_id=raclette" - ) + resp = self.client.get(f"/raclette/join/{decoded_resp['token']}") # Test that we are redirected. self.assertEqual(302, resp.status_code) diff --git a/ihatemoney/tests/budget_test.py b/ihatemoney/tests/budget_test.py index 1539ece7..159f016a 100644 --- a/ihatemoney/tests/budget_test.py +++ b/ihatemoney/tests/budget_test.py @@ -4,7 +4,7 @@ import json import re from time import sleep import unittest -from urllib.parse import parse_qs, urlencode, urlparse, urlunparse +from urllib.parse import urlparse, urlunparse from flask import session import pytest @@ -91,17 +91,19 @@ class BudgetTestCase(IhatemoneyTestCase): self.client.get("/exit") # Use another project_id parsed_url = urlparse(url) - query = parse_qs(parsed_url.query) - query["project_id"] = "invalid" resp = self.client.get( - urlunparse(parsed_url._replace(query=urlencode(query, doseq=True))) + urlunparse( + parsed_url._replace( + path=parsed_url.path.replace("raclette/", "invalid_project/") + ) + ), + follow_redirects=True, ) - assert "You either provided a bad token" in resp.data.decode("utf-8") + assert "Create a new project" in resp.data.decode("utf-8") - resp = self.client.get("/authenticate") - self.assertIn("You either provided a bad token", resp.data.decode("utf-8")) - resp = self.client.get("/authenticate?token=token") - self.assertIn("You either provided a bad token", resp.data.decode("utf-8")) + # A token MUST have a point between payload and signature + resp = self.client.get("/raclette/join/token.invalid", follow_redirects=True) + self.assertIn("Provided token is invalid", resp.data.decode("utf-8")) def test_invite_code_invalidation(self): """Test that invitation link expire after code change""" @@ -133,7 +135,7 @@ class BudgetTestCase(IhatemoneyTestCase): 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")) + self.assertIn("Provided token is invalid", response.data.decode("utf-8")) def test_password_reminder(self): # test that it is possible to have an email containing the password of a diff --git a/ihatemoney/web.py b/ihatemoney/web.py index 19f88976..f783cd86 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -143,7 +143,8 @@ def pull_project(endpoint, values): raise Redirect303(url_for(".create_project", project_id=project_id)) is_admin = session.get("is_admin") - if session.get(project.id) or is_admin: + is_invitation = endpoint == "main.join_project" + if session.get(project.id) or is_admin or is_invitation: # add project into kwargs and call the original function g.project = project else: @@ -195,6 +196,28 @@ def admin(): ) +@main.route("//join/", methods=["GET"]) +def join_project(token): + project_id = g.project.id + verified_project_id = Project.verify_token( + token, token_type="auth", project_id=project_id + ) + if verified_project_id != project_id: + flash(_("Provided token is invalid"), "danger") + return redirect("/") + + # maintain a list of visited projects + if "projects" not in session: + session["projects"] = [] + # add the project on the top of the list + session["projects"].insert(0, (project_id, g.project.name)) + session[project_id] = True + # Set session to permanent to make language choice persist + session.permanent = True + session.update() + return redirect(url_for(".list_bills")) + + @main.route("/authenticate", methods=["GET", "POST"]) def authenticate(project_id=None): """Authentication form""" @@ -203,26 +226,8 @@ def authenticate(project_id=None): if not form.id.data and request.args.get("project_id"): form.id.data = request.args["project_id"] project_id = form.id.data - # Try to get project_id from token first - token = request.args.get("token") - if token: - verified_project_id = Project.verify_token( - token, token_type="auth", project_id=project_id - ) - if verified_project_id == project_id: - token_auth = True - else: - project_id = None - else: - token_auth = False - if project_id is None: - # User doesn't provide project identifier or a valid token - # return to authenticate form - msg = _("You either provided a bad token or no project identifier.") - form["id"].errors = [msg] - return render_template("authenticate.html", form=form) - project = Project.query.get(project_id) + project = Project.query.get(project_id) if project_id is not None else None if not project: # If the user try to connect to an unexisting project, we will # propose him a link to the creation form. @@ -235,13 +240,9 @@ def authenticate(project_id=None): setattr(g, "project", project) return redirect(url_for(".list_bills")) - # else do form authentication or token authentication + # else do form authentication authentication is_post_auth = request.method == "POST" and form.validate() - if ( - is_post_auth - and check_password_hash(project.password, form.password.data) - or token_auth - ): + if is_post_auth and check_password_hash(project.password, form.password.data): # maintain a list of visited projects if "projects" not in session: session["projects"] = []