diff --git a/.prod.env b/.prod.env index 8f52df39..b6b3dc2a 100644 --- a/.prod.env +++ b/.prod.env @@ -61,6 +61,12 @@ SECRET_KEY=fixme #BEARER_TOKEN_EXPIRATION=3600 * 12 # in seconds +#SECURITY_BEARER_SALT=NODEFAULT +SECURITY_BEARER_SALT=fixme + +#SECURITY_EMAIL_SALT=NODEFAULT +SECURITY_EMAIL_SALT=fixme + #SECURITY_PASSWORD_SALT=NODEFAULT SECURITY_PASSWORD_SALT=fixme diff --git a/server/.test.env b/server/.test.env index d83920b8..908db5b9 100644 --- a/server/.test.env +++ b/server/.test.env @@ -20,3 +20,6 @@ GLOBAL_WORKSPACE='mergin' GLOBAL_STORAGE=104857600 COLLECT_STATISTICS=0 GEODIFF_WORKING_DIR=/tmp/geodiff +SECURITY_BEARER_SALT='bearer' +SECURITY_EMAIL_SALT='email' +SECURITY_PASSWORD_SALT='password' diff --git a/server/mergin/.env b/server/mergin/.env index db417d2e..0ff3dc42 100644 --- a/server/mergin/.env +++ b/server/mergin/.env @@ -1,6 +1,8 @@ GEODIFF_LOGGER_LEVEL="2" # only for dev - should be overwritten in production SECRET_KEY='top-secret' +SECURITY_BEARER_SALT='top-secret' +SECURITY_EMAIL_SALT='top-secret' SECURITY_PASSWORD_SALT='top-secret' MAIL_DEFAULT_SENDER='' FLASK_DEBUG=0 diff --git a/server/mergin/app.py b/server/mergin/app.py index 86c72d74..24b1ebc9 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -211,6 +211,7 @@ def load_user_from_header(header_val): # pylint: disable=W0613,W0612 try: data = decode_token( app.app.config["SECRET_KEY"], + app.app.config["SECURITY_BEARER_SALT"], header_val, app.app.config["BEARER_TOKEN_EXPIRATION"], ) diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index cfba8d98..7d3d8e29 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -80,17 +80,15 @@ def authenticate(login, password): return user -def generate_confirmation_token(app, email): +def generate_confirmation_token(app, email, salt): serializer = URLSafeTimedSerializer(app.config["SECRET_KEY"]) - return serializer.dumps(email, salt=app.config["SECURITY_PASSWORD_SALT"]) + return serializer.dumps(email, salt=salt) -def confirm_token(token, expiration=3600 * 24 * 3): +def confirm_token(token, salt, expiration=3600): serializer = URLSafeTimedSerializer(current_app.config["SECRET_KEY"]) try: - email = serializer.loads( - token, salt=current_app.config["SECURITY_PASSWORD_SALT"], max_age=expiration - ) + email = serializer.loads(token, salt=salt, max_age=expiration) except: return return email @@ -103,7 +101,12 @@ def send_confirmation_email(app, user, url, template, header, **kwargs): """ from ..celery import send_email_async - token = generate_confirmation_token(app, user.email) + salt = ( + app.config["SECURITY_EMAIL_SALT"] + if url == "confirm-email" + else app.config["SECURITY_PASSWORD_SALT"] + ) + token = generate_confirmation_token(app, user.email, salt) confirm_url = f"{url}/{token}" html = render_template( template, subject=header, confirm_url=confirm_url, user=user, **kwargs diff --git a/server/mergin/auth/bearer.py b/server/mergin/auth/bearer.py index 117f86d3..1c54a054 100644 --- a/server/mergin/auth/bearer.py +++ b/server/mergin/auth/bearer.py @@ -7,8 +7,7 @@ from flask.sessions import TaggedJSONSerializer -def decode_token(secret_key, token, max_age=None): - salt = "bearer-session" +def decode_token(secret_key, salt, token, max_age=None): serializer = TaggedJSONSerializer() signer_kwargs = {"key_derivation": "hmac", "digest_method": hashlib.sha1} s = URLSafeTimedSerializer( @@ -17,8 +16,7 @@ def decode_token(secret_key, token, max_age=None): return s.loads(token, max_age=max_age) -def encode_token(secret_key, data): - salt = "bearer-session" +def encode_token(secret_key, salt, data): serializer = TaggedJSONSerializer() signer_kwargs = {"key_derivation": "hmac", "digest_method": hashlib.sha1} s = URLSafeTimedSerializer( diff --git a/server/mergin/auth/config.py b/server/mergin/auth/config.py index 4484014e..07b5a05d 100644 --- a/server/mergin/auth/config.py +++ b/server/mergin/auth/config.py @@ -6,6 +6,8 @@ class Configuration(object): + SECURITY_BEARER_SALT = config("SECURITY_BEARER_SALT") + SECURITY_EMAIL_SALT = config("SECURITY_EMAIL_SALT") SECURITY_PASSWORD_SALT = config("SECURITY_PASSWORD_SALT") BEARER_TOKEN_EXPIRATION = config( "BEARER_TOKEN_EXPIRATION", default=3600 * 12, cast=int diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 106d30bd..86fa5610 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -39,6 +39,9 @@ from ..sync.utils import files_size +EMAIL_CONFIRMATION_EXPIRATION = 12 * 3600 + + # public endpoints def user_profile(user, return_all=True): """Return user profile in json format @@ -143,7 +146,11 @@ def login_public(): # noqa: E501 "email": user.email, "expire": str(expire), } - token = encode_token(current_app.config["SECRET_KEY"], token_data) + token = encode_token( + current_app.config["SECRET_KEY"], + current_app.config["SECURITY_BEARER_SALT"], + token_data, + ) data = user_profile(user) data["session"] = {"token": token, "expire": expire} @@ -297,7 +304,7 @@ def password_reset(): # pylint: disable=W0613,W0612 def confirm_new_password(token): # pylint: disable=W0613,W0612 - email = confirm_token(token) + email = confirm_token(token, salt=current_app.config["SECURITY_PASSWORD_SALT"]) if not email: abort(400, "Invalid token") @@ -315,7 +322,11 @@ def confirm_new_password(token): # pylint: disable=W0613,W0612 def confirm_email(token): # pylint: disable=W0613,W0612 - email = confirm_token(token) + email = confirm_token( + token, + expiration=EMAIL_CONFIRMATION_EXPIRATION, + salt=current_app.config["SECURITY_EMAIL_SALT"], + ) if not email: abort(400, "Invalid token") @@ -375,7 +386,9 @@ def register_user(): # pylint: disable=W0613,W0612 if form.validate(): user = User.create(form.username.data, form.email.data, form.password.data) user_created.send(user, source="admin") - token = generate_confirmation_token(current_app, user.email) + token = generate_confirmation_token( + current_app, user.email, current_app.config["SECURITY_EMAIL_SALT"] + ) confirm_url = f"confirm-email/{token}" html = render_template( "email/user_created.html", diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 280626de..18b50ffb 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -4,6 +4,7 @@ from datetime import datetime, timedelta import os +import time import pytest import json from flask import url_for @@ -12,7 +13,7 @@ from unittest.mock import patch from mergin.tests import test_workspace - +from ..auth.app import generate_confirmation_token, confirm_token from ..auth.models import User, UserProfile, LoginHistory from ..auth.tasks import anonymize_removed_users from ..app import db @@ -152,26 +153,20 @@ def test_user_register(client, username, email, pwd, expected): def test_confirm_email(app, client): - serializer = URLSafeTimedSerializer(app.config["SECRET_KEY"]) - token = serializer.dumps( - "mergin@mergin.com", salt=app.config["SECURITY_PASSWORD_SALT"] - ) - resp = client.post(url_for("/.mergin_auth_controller_confirm_email", token=token)) - assert resp.status_code == 200 - user = User.query.filter_by(username="mergin").first() - # tests with old registered user + token = generate_confirmation_token( + app, user.email, app.config["SECURITY_EMAIL_SALT"] + ) user.verified_email = False - user.registration_date = datetime.utcnow() - timedelta(days=1) db.session.commit() - resp = client.post(url_for("/.mergin_auth_controller_confirm_email", token=token)) - assert resp.status_code == 200 - # try again with freshly registered user - user.verified_email = False - user.registration_date = datetime.utcnow() - db.session.add(user) - db.session.commit() + # verify token can't be used in different context + resp = client.post( + url_for("/.mergin_auth_controller_confirm_new_password", token=token), + json={"password": "ilovemergin#0", "confirm": "ilovemergin#0"}, + ) + assert resp.status_code == 400 + resp = client.post(url_for("/.mergin_auth_controller_confirm_email", token=token)) assert resp.status_code == 200 @@ -187,21 +182,35 @@ def test_confirm_email(app, client): resp = client.post( url_for( "/.mergin_auth_controller_confirm_email", - token=serializer.dumps( - "tests@mergin.com", salt=app.config["SECURITY_PASSWORD_SALT"] + token=generate_confirmation_token( + app, "tests@mergin.com", app.config["SECURITY_EMAIL_SALT"] ), ) ) assert resp.status_code == 404 + # test expired token + token = generate_confirmation_token( + app, user.email, app.config["SECURITY_EMAIL_SALT"] + ) + time.sleep(2) + assert not confirm_token( + token=token, expiration=1, salt=app.config["SECURITY_EMAIL_SALT"] + ) + def test_confirm_password(app, client): - serializer = URLSafeTimedSerializer(app.config["SECRET_KEY"]) - token = serializer.dumps( - "mergin@mergin.com", salt=app.config["SECURITY_PASSWORD_SALT"] + user = User.query.filter_by(username="mergin").first() + token = generate_confirmation_token( + app, user.email, app.config["SECURITY_PASSWORD_SALT"] ) form_data = {"password": "ilovemergin#0", "confirm": "ilovemergin#0"} + + # verify token can't be used in different context + resp = client.post(url_for("/.mergin_auth_controller_confirm_email", token=token)) + assert resp.status_code == 400 + resp = client.post( url_for("/.mergin_auth_controller_confirm_new_password", token=token), data=json.dumps(form_data), @@ -221,8 +230,8 @@ def test_confirm_password(app, client): resp = client.post( url_for( "/.mergin_auth_controller_confirm_new_password", - token=serializer.dumps( - "tests@mergin.com", salt=app.config["SECURITY_PASSWORD_SALT"] + token=generate_confirmation_token( + app, "tests@mergin.com", app.config["SECURITY_PASSWORD_SALT"] ), ), data=json.dumps(form_data), @@ -240,8 +249,8 @@ def test_confirm_password(app, client): resp = client.post( url_for( "/.mergin_auth_controller_confirm_new_password", - token=serializer.dumps( - "tests@mergin.com", salt=app.config["SECURITY_PASSWORD_SALT"] + token=generate_confirmation_token( + app, "tests@mergin.com", app.config["SECURITY_PASSWORD_SALT"] ), ) )