From 81541e4e673f55c3fbddd686f60964d28ce12fe3 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Tue, 15 Oct 2024 13:04:06 +0200 Subject: [PATCH 01/16] feat: set up UserAuditLog --- app/models.py | 12 ++++++++++ app/user_audit_log_utils.py | 42 +++++++++++++++++++++++++++++++++++ cron.py | 9 ++++++++ crontab.yml | 7 ++++++ tasks/clean_user_audit_log.py | 12 ++++++++++ 5 files changed, 82 insertions(+) create mode 100644 app/user_audit_log_utils.py create mode 100644 tasks/clean_user_audit_log.py diff --git a/app/models.py b/app/models.py index 6bd6be66a..11242e8a3 100644 --- a/app/models.py +++ b/app/models.py @@ -3823,3 +3823,15 @@ class AliasAuditLog(Base, ModelMixin): sa.Index("ix_alias_audit_log_alias_id", "alias_id"), sa.Index("ix_alias_audit_log_alias_email", "alias_email"), ) + + +class UserAuditLog(Base, ModelMixin): + """This model holds an audit log for all the actions performed by a user""" + + __tablename__ = "user_audit_log" + + user_id = sa.Column(sa.Integer, nullable=False) + action = sa.Column(sa.String(255), nullable=False) + message = sa.Column(sa.Text, default=None, nullable=True) + + __table_args__ = (sa.Index("ix_user_audit_log_user_id", "user_id"),) diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py new file mode 100644 index 000000000..82c312efc --- /dev/null +++ b/app/user_audit_log_utils.py @@ -0,0 +1,42 @@ +from enum import Enum + +from app.models import UserAuditLog + + +class UserAuditLogAction(Enum): + Upgrade = "upgrade" + LinkAccount = "link_account" + UnlinkAccount = "unlink_account" + + CreateMailbox = "create_mailbox" + VerifyMailbox = "verify_mailbox" + UpdateMailbox = "update_mailbox" + DeleteMailbox = "delete_mailbox" + + CreateSubdomain = "create_subdomain" + UpdateSubdomain = "update_subdomain" + DeleteSubdomain = "delete_subdomain" + + CreateCustomDomain = "create_custom_domain" + VerifyCustomDomain = "verify_custom_domain" + UpdateCustomDomain = "update_custom_domain" + DeleteCustomDomain = "delete_custom_domain" + + CreateContact = "create_contact" + UpdateContact = "update_contact" + DeleteContact = "delete_contact" + + CreateDirectory = "create_directory" + UpdateDirectory = "update_directory" + DeleteDirectory = "delete_directory" + + +def emit_user_audit_log( + user_id: int, action: UserAuditLogAction, message: str, commit: bool = False +): + UserAuditLog.create( + user_id=user_id, + action=action.value, + message=message, + commit=commit, + ) diff --git a/cron.py b/cron.py index 5cd0cbbe4..8645a8cf1 100644 --- a/cron.py +++ b/cron.py @@ -63,6 +63,7 @@ from app.utils import sanitize_email from server import create_light_app from tasks.clean_alias_audit_log import cleanup_alias_audit_log +from tasks.clean_user_audit_log import cleanup_user_audit_log from tasks.cleanup_old_imports import cleanup_old_imports from tasks.cleanup_old_jobs import cleanup_old_jobs from tasks.cleanup_old_notifications import cleanup_old_notifications @@ -1247,6 +1248,11 @@ def clear_alias_audit_log(): cleanup_alias_audit_log(oldest_valid) +def clear_user_audit_log(): + oldest_valid = arrow.now().shift(days=-config.AUDIT_LOG_MAX_DAYS) + cleanup_user_audit_log(oldest_valid) + + if __name__ == "__main__": LOG.d("Start running cronjob") parser = argparse.ArgumentParser() @@ -1323,3 +1329,6 @@ def clear_alias_audit_log(): elif args.job == "clear_alias_audit_log": LOG.d("Clearing alias audit log") clear_alias_audit_log() + elif args.job == "clear_user_audit_log": + LOG.d("Clearing user audit log") + clear_user_audit_log() diff --git a/crontab.yml b/crontab.yml index 08e773a8e..7812df8f0 100644 --- a/crontab.yml +++ b/crontab.yml @@ -87,3 +87,10 @@ jobs: schedule: "0 * * * *" # Once every hour captureStderr: true concurrencyPolicy: Forbid + + - name: SimpleLogin clear user_audit_log old entries + command: python /code/cron.py -j clear_user_audit_log + shell: /bin/bash + schedule: "0 * * * *" # Once every hour + captureStderr: true + concurrencyPolicy: Forbid diff --git a/tasks/clean_user_audit_log.py b/tasks/clean_user_audit_log.py new file mode 100644 index 000000000..b03e5fc0a --- /dev/null +++ b/tasks/clean_user_audit_log.py @@ -0,0 +1,12 @@ +import arrow + +from app.db import Session +from app.log import LOG +from app.models import UserAuditLog + + +def cleanup_user_audit_log(oldest_allowed: arrow.Arrow): + LOG.i(f"Deleting user_audit_log older than {oldest_allowed}") + count = UserAuditLog.filter(UserAuditLog.created_at < oldest_allowed).delete() + Session.commit() + LOG.i(f"Deleted {count} user_audit_log entries") From a231ebe641480ac28130dfd303954a519c54c62c Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Tue, 15 Oct 2024 16:50:24 +0200 Subject: [PATCH 02/16] refactor: extract payment callbacks into their own files + handle subscription user_audit_log --- app/payments/__init__.py | 0 app/payments/coinbase.py | 119 ++++++++++++ app/payments/paddle.py | 286 +++++++++++++++++++++++++++++ app/user_audit_log_utils.py | 2 + server.py | 355 +----------------------------------- 5 files changed, 411 insertions(+), 351 deletions(-) create mode 100644 app/payments/__init__.py create mode 100644 app/payments/coinbase.py create mode 100644 app/payments/paddle.py diff --git a/app/payments/__init__.py b/app/payments/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/payments/coinbase.py b/app/payments/coinbase.py new file mode 100644 index 000000000..dda059530 --- /dev/null +++ b/app/payments/coinbase.py @@ -0,0 +1,119 @@ +import arrow + +from coinbase_commerce.error import WebhookInvalidPayload, SignatureVerificationError +from coinbase_commerce.webhook import Webhook +from flask import Flask, request + +from app.config import COINBASE_WEBHOOK_SECRET +from app.db import Session +from app.email_utils import send_email, render +from app.log import LOG +from app.models import CoinbaseSubscription, User +from app.subscription_webhook import execute_subscription_webhook +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction + + +def setup_coinbase_commerce(app: Flask): + @app.route("/coinbase", methods=["POST"]) + def coinbase_webhook(): + # event payload + request_data = request.data.decode("utf-8") + # webhook signature + request_sig = request.headers.get("X-CC-Webhook-Signature", None) + + try: + # signature verification and event object construction + event = Webhook.construct_event( + request_data, request_sig, COINBASE_WEBHOOK_SECRET + ) + except (WebhookInvalidPayload, SignatureVerificationError) as e: + LOG.e("Invalid Coinbase webhook") + return str(e), 400 + + LOG.d("Coinbase event %s", event) + + if event["type"] == "charge:confirmed": + if handle_coinbase_event(event): + return "success", 200 + else: + return "error", 400 + + return "success", 200 + + +def handle_coinbase_event(event) -> bool: + server_user_id = event["data"]["metadata"]["user_id"] + try: + user_id = int(server_user_id) + except ValueError: + user_id = int(float(server_user_id)) + + code = event["data"]["code"] + user = User.get(user_id) + if not user: + LOG.e("User not found %s", user_id) + return False + + coinbase_subscription: CoinbaseSubscription = CoinbaseSubscription.get_by( + user_id=user_id + ) + + if not coinbase_subscription: + LOG.d("Create a coinbase subscription for %s", user) + coinbase_subscription = CoinbaseSubscription.create( + user_id=user_id, end_at=arrow.now().shift(years=1), code=code, commit=True + ) + emit_user_audit_log( + user_id=user_id, + action=UserAuditLogAction.Upgrade, + message="Upgraded though Coinbase", + commit=True, + ) + send_email( + user.email, + "Your SimpleLogin account has been upgraded", + render( + "transactional/coinbase/new-subscription.txt", + user=user, + coinbase_subscription=coinbase_subscription, + ), + render( + "transactional/coinbase/new-subscription.html", + user=user, + coinbase_subscription=coinbase_subscription, + ), + ) + else: + if coinbase_subscription.code != code: + LOG.d("Update code from %s to %s", coinbase_subscription.code, code) + coinbase_subscription.code = code + + if coinbase_subscription.is_active(): + coinbase_subscription.end_at = coinbase_subscription.end_at.shift(years=1) + else: # already expired subscription + coinbase_subscription.end_at = arrow.now().shift(years=1) + + emit_user_audit_log( + user_id=user_id, + action=UserAuditLogAction.SubscriptionExtended, + message="Extended coinbase subscription", + ) + Session.commit() + + send_email( + user.email, + "Your SimpleLogin account has been extended", + render( + "transactional/coinbase/extend-subscription.txt", + user=user, + coinbase_subscription=coinbase_subscription, + ), + render( + "transactional/coinbase/extend-subscription.html", + user=user, + coinbase_subscription=coinbase_subscription, + ), + ) + execute_subscription_webhook(user) + + return True diff --git a/app/payments/paddle.py b/app/payments/paddle.py new file mode 100644 index 000000000..68b91eebe --- /dev/null +++ b/app/payments/paddle.py @@ -0,0 +1,286 @@ +import arrow +import json +from dateutil.relativedelta import relativedelta + + +from flask import Flask, request + +from app import paddle_utils, paddle_callback +from app.config import ( + PADDLE_MONTHLY_PRODUCT_ID, + PADDLE_MONTHLY_PRODUCT_IDS, + PADDLE_YEARLY_PRODUCT_IDS, + PADDLE_COUPON_ID, +) +from app.db import Session +from app.email_utils import send_email, render +from app.log import LOG +from app.models import Subscription, PlanEnum, User, Coupon +from app.subscription_webhook import execute_subscription_webhook +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction +from app.utils import random_string + + +def setup_paddle_callback(app: Flask): + @app.route("/paddle", methods=["GET", "POST"]) + def paddle(): + LOG.d(f"paddle callback {request.form.get('alert_name')} {request.form}") + + # make sure the request comes from Paddle + if not paddle_utils.verify_incoming_request(dict(request.form)): + LOG.e("request not coming from paddle. Request data:%s", dict(request.form)) + return "KO", 400 + + if ( + request.form.get("alert_name") == "subscription_created" + ): # new user subscribes + # the passthrough is json encoded, e.g. + # request.form.get("passthrough") = '{"user_id": 88 }' + passthrough = json.loads(request.form.get("passthrough")) + user_id = passthrough.get("user_id") + user = User.get(user_id) + + subscription_plan_id = int(request.form.get("subscription_plan_id")) + + if subscription_plan_id in PADDLE_MONTHLY_PRODUCT_IDS: + plan = PlanEnum.monthly + elif subscription_plan_id in PADDLE_YEARLY_PRODUCT_IDS: + plan = PlanEnum.yearly + else: + LOG.e( + "Unknown subscription_plan_id %s %s", + subscription_plan_id, + request.form, + ) + return "No such subscription", 400 + + sub = Subscription.get_by(user_id=user.id) + + if not sub: + LOG.d(f"create a new Subscription for user {user}") + Subscription.create( + user_id=user.id, + cancel_url=request.form.get("cancel_url"), + update_url=request.form.get("update_url"), + subscription_id=request.form.get("subscription_id"), + event_time=arrow.now(), + next_bill_date=arrow.get( + request.form.get("next_bill_date"), "YYYY-MM-DD" + ).date(), + plan=plan, + ) + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.Upgrade, + message="Upgraded through Paddle", + ) + else: + LOG.d(f"Update an existing Subscription for user {user}") + sub.cancel_url = request.form.get("cancel_url") + sub.update_url = request.form.get("update_url") + sub.subscription_id = request.form.get("subscription_id") + sub.event_time = arrow.now() + sub.next_bill_date = arrow.get( + request.form.get("next_bill_date"), "YYYY-MM-DD" + ).date() + sub.plan = plan + + # make sure to set the new plan as not-cancelled + # in case user cancels a plan and subscribes a new plan + sub.cancelled = False + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.SubscriptionExtended, + message="Extended Paddle subscription", + ) + + execute_subscription_webhook(user) + LOG.d("User %s upgrades!", user) + + Session.commit() + + elif request.form.get("alert_name") == "subscription_payment_succeeded": + subscription_id = request.form.get("subscription_id") + LOG.d("Update subscription %s", subscription_id) + + sub: Subscription = Subscription.get_by(subscription_id=subscription_id) + # when user subscribes, the "subscription_payment_succeeded" can arrive BEFORE "subscription_created" + # at that time, subscription object does not exist yet + if sub: + sub.event_time = arrow.now() + sub.next_bill_date = arrow.get( + request.form.get("next_bill_date"), "YYYY-MM-DD" + ).date() + + Session.commit() + execute_subscription_webhook(sub.user) + + elif request.form.get("alert_name") == "subscription_cancelled": + subscription_id = request.form.get("subscription_id") + + sub: Subscription = Subscription.get_by(subscription_id=subscription_id) + if sub: + # cancellation_effective_date should be the same as next_bill_date + LOG.w( + "Cancel subscription %s %s on %s, next bill date %s", + subscription_id, + sub.user, + request.form.get("cancellation_effective_date"), + sub.next_bill_date, + ) + sub.event_time = arrow.now() + + sub.cancelled = True + emit_user_audit_log( + user_id=sub.user_id, + action=UserAuditLogAction.SubscriptionCancelled, + message="Cancelled Paddle subscription", + ) + Session.commit() + + user = sub.user + + send_email( + user.email, + "SimpleLogin - your subscription is canceled", + render( + "transactional/subscription-cancel.txt", + user=user, + end_date=request.form.get("cancellation_effective_date"), + ), + ) + execute_subscription_webhook(sub.user) + + else: + # user might have deleted their account + LOG.i(f"Cancel non-exist subscription {subscription_id}") + return "OK" + elif request.form.get("alert_name") == "subscription_updated": + subscription_id = request.form.get("subscription_id") + + sub: Subscription = Subscription.get_by(subscription_id=subscription_id) + if sub: + next_bill_date = request.form.get("next_bill_date") + if not next_bill_date: + paddle_callback.failed_payment(sub, subscription_id) + return "OK" + + LOG.d( + "Update subscription %s %s on %s, next bill date %s", + subscription_id, + sub.user, + request.form.get("cancellation_effective_date"), + sub.next_bill_date, + ) + if ( + int(request.form.get("subscription_plan_id")) + == PADDLE_MONTHLY_PRODUCT_ID + ): + plan = PlanEnum.monthly + else: + plan = PlanEnum.yearly + + sub.cancel_url = request.form.get("cancel_url") + sub.update_url = request.form.get("update_url") + sub.event_time = arrow.now() + sub.next_bill_date = arrow.get( + request.form.get("next_bill_date"), "YYYY-MM-DD" + ).date() + sub.plan = plan + + # make sure to set the new plan as not-cancelled + sub.cancelled = False + emit_user_audit_log( + user_id=sub.user_id, + action=UserAuditLogAction.SubscriptionExtended, + message="Extended Paddle subscription", + ) + + Session.commit() + execute_subscription_webhook(sub.user) + else: + LOG.w( + f"update non-exist subscription {subscription_id}. {request.form}" + ) + return "No such subscription", 400 + elif request.form.get("alert_name") == "payment_refunded": + subscription_id = request.form.get("subscription_id") + LOG.d("Refund request for subscription %s", subscription_id) + + sub: Subscription = Subscription.get_by(subscription_id=subscription_id) + + if sub: + user = sub.user + Subscription.delete(sub.id) + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.SubscriptionCancelled, + message="Paddle subscription cancelled as user requested a refund", + ) + Session.commit() + LOG.e("%s requests a refund", user) + execute_subscription_webhook(sub.user) + + elif request.form.get("alert_name") == "subscription_payment_refunded": + subscription_id = request.form.get("subscription_id") + sub: Subscription = Subscription.get_by(subscription_id=subscription_id) + LOG.d( + "Handle subscription_payment_refunded for subscription %s", + subscription_id, + ) + + if not sub: + LOG.w( + "No such subscription for %s, payload %s", + subscription_id, + request.form, + ) + return "No such subscription" + + plan_id = int(request.form["subscription_plan_id"]) + if request.form["refund_type"] == "full": + if plan_id in PADDLE_MONTHLY_PRODUCT_IDS: + LOG.d("subtract 1 month from next_bill_date %s", sub.next_bill_date) + sub.next_bill_date = sub.next_bill_date - relativedelta(months=1) + LOG.d("next_bill_date is %s", sub.next_bill_date) + Session.commit() + elif plan_id in PADDLE_YEARLY_PRODUCT_IDS: + LOG.d("subtract 1 year from next_bill_date %s", sub.next_bill_date) + sub.next_bill_date = sub.next_bill_date - relativedelta(years=1) + LOG.d("next_bill_date is %s", sub.next_bill_date) + Session.commit() + else: + LOG.e("Unknown plan_id %s", plan_id) + else: + LOG.w("partial subscription_payment_refunded, not handled") + execute_subscription_webhook(sub.user) + + return "OK" + + @app.route("/paddle_coupon", methods=["GET", "POST"]) + def paddle_coupon(): + LOG.d("paddle coupon callback %s", request.form) + + if not paddle_utils.verify_incoming_request(dict(request.form)): + LOG.e("request not coming from paddle. Request data:%s", dict(request.form)) + return "KO", 400 + + product_id = request.form.get("p_product_id") + if product_id != PADDLE_COUPON_ID: + LOG.e("product_id %s not match with %s", product_id, PADDLE_COUPON_ID) + return "KO", 400 + + email = request.form.get("email") + LOG.d("Paddle coupon request for %s", email) + + coupon = Coupon.create( + code=random_string(30), + comment="For 1-year coupon", + expires_date=arrow.now().shift(years=1, days=-1), + commit=True, + ) + + return ( + f"Your 1-year coupon is {coupon.code}
" + f"It's valid until {coupon.expires_date.date().isoformat()}" + ) diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py index 82c312efc..d0524331c 100644 --- a/app/user_audit_log_utils.py +++ b/app/user_audit_log_utils.py @@ -5,6 +5,8 @@ class UserAuditLogAction(Enum): Upgrade = "upgrade" + SubscriptionExtended = "subscription_extended" + SubscriptionCancelled = "subscription_cancelled" LinkAccount = "link_account" UnlinkAccount = "unlink_account" diff --git a/server.py b/server.py index 35a3dce8c..de69b9d0e 100644 --- a/server.py +++ b/server.py @@ -1,4 +1,3 @@ -import json import os import time from datetime import timedelta @@ -9,9 +8,7 @@ import flask_profiler import newrelic.agent import sentry_sdk -from coinbase_commerce.error import WebhookInvalidPayload, SignatureVerificationError -from coinbase_commerce.webhook import Webhook -from dateutil.relativedelta import relativedelta + from flask import ( Flask, redirect, @@ -30,7 +27,7 @@ from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration from werkzeug.middleware.proxy_fix import ProxyFix -from app import paddle_utils, config, paddle_callback, constants +from app import config, constants from app.admin_model import ( SLAdminIndexView, UserAdmin, @@ -56,7 +53,6 @@ FLASK_SECRET, SENTRY_DSN, URL, - PADDLE_MONTHLY_PRODUCT_ID, FLASK_PROFILER_PATH, FLASK_PROFILER_PASSWORD, SENTRY_FRONT_END_DSN, @@ -70,22 +66,16 @@ LANDING_PAGE_URL, STATUS_PAGE_URL, SUPPORT_EMAIL, - PADDLE_MONTHLY_PRODUCT_IDS, - PADDLE_YEARLY_PRODUCT_IDS, PGP_SIGNER, - COINBASE_WEBHOOK_SECRET, PAGE_LIMIT, - PADDLE_COUPON_ID, ZENDESK_ENABLED, MAX_NB_EMAIL_FREE_PLAN, MEM_STORE_URI, ) from app.dashboard.base import dashboard_bp -from app.subscription_webhook import execute_subscription_webhook from app.db import Session from app.developer.base import developer_bp from app.discover.base import discover_bp -from app.email_utils import send_email, render from app.extensions import login_manager, limiter from app.fake_data import fake_data from app.internal.base import internal_bp @@ -94,11 +84,8 @@ from app.models import ( User, Alias, - Subscription, - PlanEnum, CustomDomain, Mailbox, - CoinbaseSubscription, EmailLog, Contact, ManualSubscription, @@ -115,10 +102,11 @@ from app.newsletter_utils import send_newsletter_to_user from app.oauth.base import oauth_bp from app.onboarding.base import onboarding_bp +from app.payments.coinbase import setup_coinbase_commerce +from app.payments.paddle import setup_paddle_callback from app.phone.base import phone_bp from app.redis_services import initialize_redis_services from app.sentry_utils import sentry_before_send -from app.utils import random_string if SENTRY_DSN: LOG.d("enable sentry") @@ -446,341 +434,6 @@ def inject_stage_and_region(): ) -def setup_paddle_callback(app: Flask): - @app.route("/paddle", methods=["GET", "POST"]) - def paddle(): - LOG.d(f"paddle callback {request.form.get('alert_name')} {request.form}") - - # make sure the request comes from Paddle - if not paddle_utils.verify_incoming_request(dict(request.form)): - LOG.e("request not coming from paddle. Request data:%s", dict(request.form)) - return "KO", 400 - - if ( - request.form.get("alert_name") == "subscription_created" - ): # new user subscribes - # the passthrough is json encoded, e.g. - # request.form.get("passthrough") = '{"user_id": 88 }' - passthrough = json.loads(request.form.get("passthrough")) - user_id = passthrough.get("user_id") - user = User.get(user_id) - - subscription_plan_id = int(request.form.get("subscription_plan_id")) - - if subscription_plan_id in PADDLE_MONTHLY_PRODUCT_IDS: - plan = PlanEnum.monthly - elif subscription_plan_id in PADDLE_YEARLY_PRODUCT_IDS: - plan = PlanEnum.yearly - else: - LOG.e( - "Unknown subscription_plan_id %s %s", - subscription_plan_id, - request.form, - ) - return "No such subscription", 400 - - sub = Subscription.get_by(user_id=user.id) - - if not sub: - LOG.d(f"create a new Subscription for user {user}") - Subscription.create( - user_id=user.id, - cancel_url=request.form.get("cancel_url"), - update_url=request.form.get("update_url"), - subscription_id=request.form.get("subscription_id"), - event_time=arrow.now(), - next_bill_date=arrow.get( - request.form.get("next_bill_date"), "YYYY-MM-DD" - ).date(), - plan=plan, - ) - else: - LOG.d(f"Update an existing Subscription for user {user}") - sub.cancel_url = request.form.get("cancel_url") - sub.update_url = request.form.get("update_url") - sub.subscription_id = request.form.get("subscription_id") - sub.event_time = arrow.now() - sub.next_bill_date = arrow.get( - request.form.get("next_bill_date"), "YYYY-MM-DD" - ).date() - sub.plan = plan - - # make sure to set the new plan as not-cancelled - # in case user cancels a plan and subscribes a new plan - sub.cancelled = False - - execute_subscription_webhook(user) - LOG.d("User %s upgrades!", user) - - Session.commit() - - elif request.form.get("alert_name") == "subscription_payment_succeeded": - subscription_id = request.form.get("subscription_id") - LOG.d("Update subscription %s", subscription_id) - - sub: Subscription = Subscription.get_by(subscription_id=subscription_id) - # when user subscribes, the "subscription_payment_succeeded" can arrive BEFORE "subscription_created" - # at that time, subscription object does not exist yet - if sub: - sub.event_time = arrow.now() - sub.next_bill_date = arrow.get( - request.form.get("next_bill_date"), "YYYY-MM-DD" - ).date() - - Session.commit() - execute_subscription_webhook(sub.user) - - elif request.form.get("alert_name") == "subscription_cancelled": - subscription_id = request.form.get("subscription_id") - - sub: Subscription = Subscription.get_by(subscription_id=subscription_id) - if sub: - # cancellation_effective_date should be the same as next_bill_date - LOG.w( - "Cancel subscription %s %s on %s, next bill date %s", - subscription_id, - sub.user, - request.form.get("cancellation_effective_date"), - sub.next_bill_date, - ) - sub.event_time = arrow.now() - - sub.cancelled = True - Session.commit() - - user = sub.user - - send_email( - user.email, - "SimpleLogin - your subscription is canceled", - render( - "transactional/subscription-cancel.txt", - user=user, - end_date=request.form.get("cancellation_effective_date"), - ), - ) - execute_subscription_webhook(sub.user) - - else: - # user might have deleted their account - LOG.i(f"Cancel non-exist subscription {subscription_id}") - return "OK" - elif request.form.get("alert_name") == "subscription_updated": - subscription_id = request.form.get("subscription_id") - - sub: Subscription = Subscription.get_by(subscription_id=subscription_id) - if sub: - next_bill_date = request.form.get("next_bill_date") - if not next_bill_date: - paddle_callback.failed_payment(sub, subscription_id) - return "OK" - - LOG.d( - "Update subscription %s %s on %s, next bill date %s", - subscription_id, - sub.user, - request.form.get("cancellation_effective_date"), - sub.next_bill_date, - ) - if ( - int(request.form.get("subscription_plan_id")) - == PADDLE_MONTHLY_PRODUCT_ID - ): - plan = PlanEnum.monthly - else: - plan = PlanEnum.yearly - - sub.cancel_url = request.form.get("cancel_url") - sub.update_url = request.form.get("update_url") - sub.event_time = arrow.now() - sub.next_bill_date = arrow.get( - request.form.get("next_bill_date"), "YYYY-MM-DD" - ).date() - sub.plan = plan - - # make sure to set the new plan as not-cancelled - sub.cancelled = False - - Session.commit() - execute_subscription_webhook(sub.user) - else: - LOG.w( - f"update non-exist subscription {subscription_id}. {request.form}" - ) - return "No such subscription", 400 - elif request.form.get("alert_name") == "payment_refunded": - subscription_id = request.form.get("subscription_id") - LOG.d("Refund request for subscription %s", subscription_id) - - sub: Subscription = Subscription.get_by(subscription_id=subscription_id) - - if sub: - user = sub.user - Subscription.delete(sub.id) - Session.commit() - LOG.e("%s requests a refund", user) - execute_subscription_webhook(sub.user) - - elif request.form.get("alert_name") == "subscription_payment_refunded": - subscription_id = request.form.get("subscription_id") - sub: Subscription = Subscription.get_by(subscription_id=subscription_id) - LOG.d( - "Handle subscription_payment_refunded for subscription %s", - subscription_id, - ) - - if not sub: - LOG.w( - "No such subscription for %s, payload %s", - subscription_id, - request.form, - ) - return "No such subscription" - - plan_id = int(request.form["subscription_plan_id"]) - if request.form["refund_type"] == "full": - if plan_id in PADDLE_MONTHLY_PRODUCT_IDS: - LOG.d("subtract 1 month from next_bill_date %s", sub.next_bill_date) - sub.next_bill_date = sub.next_bill_date - relativedelta(months=1) - LOG.d("next_bill_date is %s", sub.next_bill_date) - Session.commit() - elif plan_id in PADDLE_YEARLY_PRODUCT_IDS: - LOG.d("subtract 1 year from next_bill_date %s", sub.next_bill_date) - sub.next_bill_date = sub.next_bill_date - relativedelta(years=1) - LOG.d("next_bill_date is %s", sub.next_bill_date) - Session.commit() - else: - LOG.e("Unknown plan_id %s", plan_id) - else: - LOG.w("partial subscription_payment_refunded, not handled") - execute_subscription_webhook(sub.user) - - return "OK" - - @app.route("/paddle_coupon", methods=["GET", "POST"]) - def paddle_coupon(): - LOG.d("paddle coupon callback %s", request.form) - - if not paddle_utils.verify_incoming_request(dict(request.form)): - LOG.e("request not coming from paddle. Request data:%s", dict(request.form)) - return "KO", 400 - - product_id = request.form.get("p_product_id") - if product_id != PADDLE_COUPON_ID: - LOG.e("product_id %s not match with %s", product_id, PADDLE_COUPON_ID) - return "KO", 400 - - email = request.form.get("email") - LOG.d("Paddle coupon request for %s", email) - - coupon = Coupon.create( - code=random_string(30), - comment="For 1-year coupon", - expires_date=arrow.now().shift(years=1, days=-1), - commit=True, - ) - - return ( - f"Your 1-year coupon is {coupon.code}
" - f"It's valid until {coupon.expires_date.date().isoformat()}" - ) - - -def setup_coinbase_commerce(app): - @app.route("/coinbase", methods=["POST"]) - def coinbase_webhook(): - # event payload - request_data = request.data.decode("utf-8") - # webhook signature - request_sig = request.headers.get("X-CC-Webhook-Signature", None) - - try: - # signature verification and event object construction - event = Webhook.construct_event( - request_data, request_sig, COINBASE_WEBHOOK_SECRET - ) - except (WebhookInvalidPayload, SignatureVerificationError) as e: - LOG.e("Invalid Coinbase webhook") - return str(e), 400 - - LOG.d("Coinbase event %s", event) - - if event["type"] == "charge:confirmed": - if handle_coinbase_event(event): - return "success", 200 - else: - return "error", 400 - - return "success", 200 - - -def handle_coinbase_event(event) -> bool: - server_user_id = event["data"]["metadata"]["user_id"] - try: - user_id = int(server_user_id) - except ValueError: - user_id = int(float(server_user_id)) - - code = event["data"]["code"] - user = User.get(user_id) - if not user: - LOG.e("User not found %s", user_id) - return False - - coinbase_subscription: CoinbaseSubscription = CoinbaseSubscription.get_by( - user_id=user_id - ) - - if not coinbase_subscription: - LOG.d("Create a coinbase subscription for %s", user) - coinbase_subscription = CoinbaseSubscription.create( - user_id=user_id, end_at=arrow.now().shift(years=1), code=code, commit=True - ) - send_email( - user.email, - "Your SimpleLogin account has been upgraded", - render( - "transactional/coinbase/new-subscription.txt", - user=user, - coinbase_subscription=coinbase_subscription, - ), - render( - "transactional/coinbase/new-subscription.html", - user=user, - coinbase_subscription=coinbase_subscription, - ), - ) - else: - if coinbase_subscription.code != code: - LOG.d("Update code from %s to %s", coinbase_subscription.code, code) - coinbase_subscription.code = code - - if coinbase_subscription.is_active(): - coinbase_subscription.end_at = coinbase_subscription.end_at.shift(years=1) - else: # already expired subscription - coinbase_subscription.end_at = arrow.now().shift(years=1) - - Session.commit() - - send_email( - user.email, - "Your SimpleLogin account has been extended", - render( - "transactional/coinbase/extend-subscription.txt", - user=user, - coinbase_subscription=coinbase_subscription, - ), - render( - "transactional/coinbase/extend-subscription.html", - user=user, - coinbase_subscription=coinbase_subscription, - ), - ) - execute_subscription_webhook(user) - - return True - - def init_extensions(app: Flask): login_manager.init_app(app) From e6978d29d32aaa67495bb2d46d77dc99e7188ad3 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Tue, 15 Oct 2024 16:50:37 +0200 Subject: [PATCH 03/16] feat: handle account linking for user audit log --- app/account_linking.py | 28 +++++++++++++++++++----- app/partner_user_utils.py | 46 +++++++++++++++++++++++++++++++++++++++ app/proton/utils.py | 6 +++++ 3 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 app/partner_user_utils.py diff --git a/app/account_linking.py b/app/account_linking.py index 1e5f297c0..80b42bf52 100644 --- a/app/account_linking.py +++ b/app/account_linking.py @@ -9,6 +9,7 @@ from app.db import Session from app.email_utils import send_welcome_email +from app.partner_user_utils import create_partner_user, create_partner_subscription from app.utils import sanitize_email, canonicalize_email from app.errors import ( AccountAlreadyLinkedToAnotherPartnerException, @@ -23,6 +24,7 @@ User, Alias, ) +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import random_string @@ -66,9 +68,10 @@ def set_plan_for_partner_user(partner_user: PartnerUser, plan: SLPlan): LOG.i( f"Creating partner_subscription [user_id={partner_user.user_id}] [partner_id={partner_user.partner_id}]" ) - PartnerSubscription.create( - partner_user_id=partner_user.id, - end_at=plan.expiration, + create_partner_subscription( + partner_user=partner_user, + expiration=plan.expiration, + msg="Upgraded via partner. User did not have a previous partner subscription", ) agent.record_custom_event("PlanChange", {"plan": "premium", "type": "new"}) else: @@ -80,6 +83,11 @@ def set_plan_for_partner_user(partner_user: PartnerUser, plan: SLPlan): "PlanChange", {"plan": "premium", "type": "extension"} ) sub.end_at = plan.expiration + emit_user_audit_log( + user_id=partner_user.user_id, + action=UserAuditLogAction.SubscriptionExtended, + message="Extended partner subscription", + ) Session.commit() @@ -98,7 +106,7 @@ def ensure_partner_user_exists_for_user( if res and res.partner_id != partner.id: raise AccountAlreadyLinkedToAnotherPartnerException() if not res: - res = PartnerUser.create( + res = create_partner_user( user_id=sl_user.id, partner_id=partner.id, partner_email=link_request.email, @@ -140,7 +148,7 @@ def process(self) -> LinkResult: activated=True, from_partner=self.link_request.from_partner, ) - partner_user = PartnerUser.create( + partner_user = create_partner_user( user_id=new_user.id, partner_id=self.partner.id, external_user_id=self.link_request.external_user_id, @@ -200,7 +208,7 @@ def get_login_strategy( return ExistingUnlinkedUserStrategy(link_request, user, partner) -def check_alias(email: str) -> bool: +def check_alias(email: str): alias = Alias.get_by(email=email) if alias is not None: raise AccountIsUsingAliasAsEmail() @@ -275,6 +283,14 @@ def switch_already_linked_user( LOG.i( f"Deleting previous partner_user:{other_partner_user.id} from user:{current_user.id}" ) + + old_info = f"Old: (external_user_id={other_partner_user.external_user_id} | partner_email={other_partner_user.partner_email})" + new_info = f"New: (external_user_id={partner_user.external_user_id} | partner_email={partner_user.partner_email})" + emit_user_audit_log( + user_id=other_partner_user.user_id, + action=UserAuditLogAction.UnlinkAccount, + message=f"Unlinking from partner, as user will now be tied to another external account. {old_info} | {new_info}", + ) PartnerUser.delete(other_partner_user.id) LOG.i(f"Linking partner_user:{partner_user.id} to user:{current_user.id}") # Link this partner_user to the current user diff --git a/app/partner_user_utils.py b/app/partner_user_utils.py new file mode 100644 index 000000000..921c78ead --- /dev/null +++ b/app/partner_user_utils.py @@ -0,0 +1,46 @@ +from typing import Optional + +from arrow import Arrow + +from app.models import PartnerUser, PartnerSubscription +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction + + +def create_partner_user( + user_id: int, partner_id: int, partner_email: str, external_user_id: str +) -> PartnerUser: + instance = PartnerUser.create( + user_id=user_id, + partner_id=partner_id, + partner_email=partner_email, + external_user_id=external_user_id, + ) + emit_user_audit_log( + user_id=user_id, + action=UserAuditLogAction.LinkAccount, + message=f"Linked account to partner_id={partner_id} | partner_email={partner_email} | external_user_id={external_user_id}", + ) + + return instance + + +def create_partner_subscription( + partner_user: PartnerUser, + expiration: Optional[Arrow], + msg: Optional[str] = None, +) -> PartnerSubscription: + instance = PartnerSubscription.create( + partner_user_id=partner_user.id, + end_at=expiration, + ) + + message = "User upgraded through partner subscription" + if msg: + message += f" | {msg}" + emit_user_audit_log( + user_id=partner_user.user_id, + action=UserAuditLogAction.Upgrade, + message=message, + ) + + return instance diff --git a/app/proton/utils.py b/app/proton/utils.py index a75cd180f..e08650d66 100644 --- a/app/proton/utils.py +++ b/app/proton/utils.py @@ -5,6 +5,7 @@ from app.log import LOG from app.errors import ProtonPartnerNotSetUp from app.models import Partner, PartnerUser, User +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction PROTON_PARTNER_NAME = "Proton" _PROTON_PARTNER: Optional[Partner] = None @@ -32,6 +33,11 @@ def perform_proton_account_unlink(current_user: User): ) if partner_user is not None: LOG.info(f"User {current_user} has unlinked the account from {partner_user}") + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UnlinkAccount, + message=f"User has unlinked the account (email={partner_user.partner_email} | external_user_id={partner_user.external_user_id})", + ) PartnerUser.delete(partner_user.id) Session.commit() agent.record_custom_event("AccountUnlinked", {"partner": proton_partner.name}) From ab84e16adfa65831355853c431834c7090c5e86d Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 08:59:56 +0200 Subject: [PATCH 04/16] chore: user_audit_log for mailboxes --- app/dashboard/views/mailbox.py | 9 ++++- app/dashboard/views/mailbox_detail.py | 57 +++++++++++++++++++++++++-- app/mailbox_utils.py | 15 ++++++- app/user_audit_log_utils.py | 2 + app/user_settings.py | 9 ++++- job_runner.py | 9 ++++- 6 files changed, 93 insertions(+), 8 deletions(-) diff --git a/app/dashboard/views/mailbox.py b/app/dashboard/views/mailbox.py index 36c7f7863..225d40cb1 100644 --- a/app/dashboard/views/mailbox.py +++ b/app/dashboard/views/mailbox.py @@ -1,6 +1,7 @@ import base64 import binascii import json +from typing import Optional from flask import render_template, request, redirect, url_for, flash from flask_login import login_required, current_user @@ -15,6 +16,7 @@ from app.db import Session from app.log import LOG from app.models import Mailbox +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import CSRFValidationForm @@ -151,7 +153,7 @@ def verify_with_signed_secret(request: str): flash("Invalid link. Please delete and re-add your mailbox", "error") return redirect(url_for("dashboard.mailbox_route")) mailbox_id = mailbox_data[0] - mailbox = Mailbox.get(mailbox_id) + mailbox: Optional[Mailbox] = Mailbox.get(mailbox_id) if not mailbox: flash("Invalid link", "error") return redirect(url_for("dashboard.mailbox_route")) @@ -161,6 +163,11 @@ def verify_with_signed_secret(request: str): return redirect(url_for("dashboard.mailbox_route")) mailbox.verified = True + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.VerifyMailbox, + message=f"Verified mailbox {mailbox.id} ({mailbox.email})", + ) Session.commit() LOG.d("Mailbox %s is verified", mailbox) diff --git a/app/dashboard/views/mailbox_detail.py b/app/dashboard/views/mailbox_detail.py index bbf2e9535..12a5e9f1a 100644 --- a/app/dashboard/views/mailbox_detail.py +++ b/app/dashboard/views/mailbox_detail.py @@ -1,4 +1,5 @@ from smtplib import SMTPRecipientsRefused +from typing import Optional from email_validator import validate_email, EmailNotValidError from flask import render_template, request, redirect, url_for, flash @@ -20,6 +21,7 @@ from app.models import Alias, AuthorizedAddress from app.models import Mailbox from app.pgp_utils import PGPException, load_public_key_and_check +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import sanitize_email, CSRFValidationForm @@ -88,8 +90,12 @@ def mailbox_detail_route(mailbox_id): flash("SPF enforcement globally not enabled", "error") return redirect(url_for("dashboard.index")) - mailbox.force_spf = ( - True if request.form.get("spf-status") == "on" else False + force_spf_value = request.form.get("spf-status") == "on" + mailbox.force_spf = force_spf_value + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Set force_spf to {force_spf_value} on mailbox {mailbox_id} ({mailbox.email})", ) Session.commit() flash( @@ -113,6 +119,11 @@ def mailbox_detail_route(mailbox_id): if AuthorizedAddress.get_by(mailbox_id=mailbox.id, email=address): flash(f"{address} already added", "error") else: + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Add authorized address {address} to mailbox {mailbox_id} ({mailbox.email})", + ) AuthorizedAddress.create( user_id=current_user.id, mailbox_id=mailbox.id, @@ -133,6 +144,11 @@ def mailbox_detail_route(mailbox_id): flash("Unknown error. Refresh the page", "warning") else: address = authorized_address.email + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Remove authorized address {address} from mailbox {mailbox_id} ({mailbox.email})", + ) AuthorizedAddress.delete(authorized_address_id) Session.commit() flash(f"{address} has been deleted", "success") @@ -165,6 +181,11 @@ def mailbox_detail_route(mailbox_id): except PGPException: flash("Cannot add the public key, please verify it", "error") else: + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Add PGP Key {mailbox.pgp_finger_print} to mailbox {mailbox_id} ({mailbox.email})", + ) Session.commit() flash("Your PGP public key is saved successfully", "success") return redirect( @@ -172,6 +193,11 @@ def mailbox_detail_route(mailbox_id): ) elif request.form.get("action") == "remove": # Free user can decide to remove their added PGP key + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Remove PGP Key {mailbox.pgp_finger_print} from mailbox {mailbox_id} ({mailbox.email})", + ) mailbox.pgp_public_key = None mailbox.pgp_finger_print = None mailbox.disable_pgp = False @@ -191,9 +217,19 @@ def mailbox_detail_route(mailbox_id): ) else: mailbox.disable_pgp = False + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Enabled PGP for mailbox {mailbox_id} ({mailbox.email})", + ) flash(f"PGP is enabled on {mailbox.email}", "info") else: mailbox.disable_pgp = True + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Disabled PGP for mailbox {mailbox_id} ({mailbox.email})", + ) flash(f"PGP is disabled on {mailbox.email}", "info") Session.commit() @@ -203,6 +239,11 @@ def mailbox_detail_route(mailbox_id): elif request.form.get("form-name") == "generic-subject": if request.form.get("action") == "save": mailbox.generic_subject = request.form.get("generic-subject") + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Set generic subject for mailbox {mailbox_id} ({mailbox.email})", + ) Session.commit() flash("Generic subject is enabled", "success") return redirect( @@ -210,6 +251,11 @@ def mailbox_detail_route(mailbox_id): ) elif request.form.get("action") == "remove": mailbox.generic_subject = None + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Remove generic subject for mailbox {mailbox_id} ({mailbox.email})", + ) Session.commit() flash("Generic subject is disabled", "success") return redirect( @@ -282,7 +328,7 @@ def mailbox_confirm_change_route(): flash("Invalid link", "error") return redirect(url_for("dashboard.index")) else: - mailbox = Mailbox.get(mailbox_id) + mailbox: Optional[Mailbox] = Mailbox.get(mailbox_id) # new_email can be None if user cancels change in the meantime if mailbox and mailbox.new_email: @@ -293,6 +339,11 @@ def mailbox_confirm_change_route(): url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox.id) ) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Change mailbox email for mailbox {mailbox_id} (old={mailbox.email} | new={mailbox.new_email})", + ) mailbox.email = mailbox.new_email mailbox.new_email = None diff --git a/app/mailbox_utils.py b/app/mailbox_utils.py index 9f94b7e92..af4270fc9 100644 --- a/app/mailbox_utils.py +++ b/app/mailbox_utils.py @@ -16,6 +16,7 @@ from app.email_validation import is_valid_email from app.log import LOG from app.models import User, Mailbox, Job, MailboxActivation +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction @dataclasses.dataclass @@ -70,9 +71,14 @@ def create_mailbox( f"User {user} has tried to create mailbox with {email} but email is invalid" ) raise MailboxError("Invalid email") - new_mailbox = Mailbox.create( + new_mailbox: Mailbox = Mailbox.create( email=email, user_id=user.id, verified=verified, commit=True ) + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.CreateMailbox, + message=f"Create mailbox {new_mailbox.id} ({new_mailbox.email}). Verified={verified}", + ) if verified: LOG.i(f"User {user} as created a pre-verified mailbox with {email}") @@ -129,7 +135,7 @@ def delete_mailbox( if not transfer_mailbox.verified: LOG.i(f"User {user} has tried to transfer to a non verified mailbox") - MailboxError("Your new mailbox is not verified") + raise MailboxError("Your new mailbox is not verified") # Schedule delete account job LOG.i( @@ -204,6 +210,11 @@ def verify_mailbox_code(user: User, mailbox_id: int, code: str) -> Mailbox: raise CannotVerifyError("Invalid activation code") LOG.i(f"User {user} has verified mailbox {mailbox_id}") mailbox.verified = True + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.VerifyMailbox, + message=f"Verify mailbox {mailbox_id} ({mailbox.email})", + ) clear_activation_codes_for_mailbox(mailbox) return mailbox diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py index d0524331c..665b218f3 100644 --- a/app/user_audit_log_utils.py +++ b/app/user_audit_log_utils.py @@ -32,6 +32,8 @@ class UserAuditLogAction(Enum): UpdateDirectory = "update_directory" DeleteDirectory = "delete_directory" + DeleteUser = "delete_user" + def emit_user_audit_log( user_id: int, action: UserAuditLogAction, message: str, commit: bool = False diff --git a/app/user_settings.py b/app/user_settings.py index 136a18b32..f616d9918 100644 --- a/app/user_settings.py +++ b/app/user_settings.py @@ -3,6 +3,7 @@ from app.db import Session from app.log import LOG from app.models import User, SLDomain, CustomDomain, Mailbox +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction class CannotSetAlias(Exception): @@ -54,7 +55,7 @@ def set_default_alias_domain(user: User, domain_name: Optional[str]): def set_default_mailbox(user: User, mailbox_id: int) -> Mailbox: - mailbox = Mailbox.get(mailbox_id) + mailbox: Optional[Mailbox] = Mailbox.get(mailbox_id) if not mailbox or mailbox.user_id != user.id: raise CannotSetMailbox("Invalid mailbox") @@ -67,5 +68,11 @@ def set_default_mailbox(user: User, mailbox_id: int) -> Mailbox: LOG.i(f"User {user} has set mailbox {mailbox} as his default one") user.default_mailbox_id = mailbox.id + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.UpdateMailbox, + message=f"Set mailbox {mailbox.id} ({mailbox.email}) as default", + ) + Session.commit() return mailbox diff --git a/job_runner.py b/job_runner.py index 72caef269..4567c845d 100644 --- a/job_runner.py +++ b/job_runner.py @@ -20,6 +20,7 @@ from app.jobs.export_user_data_job import ExportUserDataJob from app.log import LOG from app.models import User, Job, BatchImport, Mailbox, CustomDomain, JobState +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from server import create_light_app @@ -128,7 +129,7 @@ def welcome_proton(user): def delete_mailbox_job(job: Job): mailbox_id = job.payload.get("mailbox_id") - mailbox = Mailbox.get(mailbox_id) + mailbox: Optional[Mailbox] = Mailbox.get(mailbox_id) if not mailbox: return @@ -152,6 +153,12 @@ def delete_mailbox_job(job: Job): mailbox_email = mailbox.email user = mailbox.user + + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.DeleteMailbox, + message=f"Delete mailbox {mailbox.id} ({mailbox.email})", + ) Mailbox.delete(mailbox_id) Session.commit() LOG.d("Mailbox %s %s deleted", mailbox_id, mailbox_email) From 9625718f516d3fc5bcc536484324e92ca5274209 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 09:42:40 +0200 Subject: [PATCH 05/16] chore: user_audit_log for custom domains --- app/custom_domain_utils.py | 12 ++++++++++++ app/custom_domain_validation.py | 27 +++++++++++++++++++++++++++ app/dashboard/views/domain_detail.py | 21 +++++++++++++++++++++ app/dashboard/views/subdomain.py | 7 +++++++ app/user_audit_log_utils.py | 4 ---- job_runner.py | 11 +++++++++++ 6 files changed, 78 insertions(+), 4 deletions(-) diff --git a/app/custom_domain_utils.py b/app/custom_domain_utils.py index d54760b58..937cea2fb 100644 --- a/app/custom_domain_utils.py +++ b/app/custom_domain_utils.py @@ -10,6 +10,7 @@ from app.email_utils import get_email_domain_part from app.log import LOG from app.models import User, CustomDomain, SLDomain, Mailbox, Job, DomainMailbox +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction _ALLOWED_DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(? dict[str, str]: # Original DKIM record is not there, which means the DKIM config is not finished. Proceed with the # rest of the code path, returning the invalid records and clearing the flag custom_domain.dkim_verified = len(invalid_records) == 0 + if custom_domain.dkim_verified: + emit_user_audit_log( + user_id=custom_domain.user_id, + action=UserAuditLogAction.VerifyCustomDomain, + message=f"Verified DKIM records for custom domain {custom_domain.id} ({custom_domain.domain})", + ) Session.commit() return invalid_records @@ -137,6 +144,11 @@ def validate_domain_ownership( if expected_verification_record in txt_records: custom_domain.ownership_verified = True + emit_user_audit_log( + user_id=custom_domain.user_id, + action=UserAuditLogAction.VerifyCustomDomain, + message=f"Verified ownership for custom domain {custom_domain.id} ({custom_domain.domain})", + ) Session.commit() return DomainValidationResult(success=True, errors=[]) else: @@ -155,6 +167,11 @@ def validate_mx_records( ) else: custom_domain.verified = True + emit_user_audit_log( + user_id=custom_domain.user_id, + action=UserAuditLogAction.VerifyCustomDomain, + message=f"Verified MX records for custom domain {custom_domain.id} ({custom_domain.domain})", + ) Session.commit() return DomainValidationResult(success=True, errors=[]) @@ -169,6 +186,11 @@ def validate_spf_records( return DomainValidationResult(success=True, errors=[]) else: custom_domain.spf_verified = False + emit_user_audit_log( + user_id=custom_domain.user_id, + action=UserAuditLogAction.VerifyCustomDomain, + message=f"Verified SPF records for custom domain {custom_domain.id} ({custom_domain.domain})", + ) Session.commit() txt_records = self._dns_client.get_txt_record(custom_domain.domain) cleaned_records = self.__clean_spf_records(txt_records, custom_domain) @@ -183,6 +205,11 @@ def validate_dmarc_records( txt_records = self._dns_client.get_txt_record("_dmarc." + custom_domain.domain) if DMARC_RECORD in txt_records: custom_domain.dmarc_verified = True + emit_user_audit_log( + user_id=custom_domain.user_id, + action=UserAuditLogAction.VerifyCustomDomain, + message=f"Verified DMARC records for custom domain {custom_domain.id} ({custom_domain.domain})", + ) Session.commit() return DomainValidationResult(success=True, errors=[]) else: diff --git a/app/dashboard/views/domain_detail.py b/app/dashboard/views/domain_detail.py index 2b1ac32fb..638a57bdb 100644 --- a/app/dashboard/views/domain_detail.py +++ b/app/dashboard/views/domain_detail.py @@ -20,6 +20,7 @@ AutoCreateRuleMailbox, ) from app.regex_utils import regex_match +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import random_string, CSRFValidationForm @@ -164,6 +165,11 @@ def domain_detail(custom_domain_id): return redirect(request.url) if request.form.get("form-name") == "switch-catch-all": custom_domain.catch_all = not custom_domain.catch_all + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateCustomDomain, + message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) catch all to {custom_domain.catch_all}", + ) Session.commit() if custom_domain.catch_all: @@ -182,6 +188,11 @@ def domain_detail(custom_domain_id): elif request.form.get("form-name") == "set-name": if request.form.get("action") == "save": custom_domain.name = request.form.get("alias-name").replace("\n", "") + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateCustomDomain, + message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) name", + ) Session.commit() flash( f"Default alias name for Domain {custom_domain.domain} has been set", @@ -189,6 +200,11 @@ def domain_detail(custom_domain_id): ) else: custom_domain.name = None + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateCustomDomain, + message=f"Cleared custom domain {custom_domain.id} ({custom_domain.domain}) name", + ) Session.commit() flash( f"Default alias name for Domain {custom_domain.domain} has been removed", @@ -202,6 +218,11 @@ def domain_detail(custom_domain_id): custom_domain.random_prefix_generation = ( not custom_domain.random_prefix_generation ) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateCustomDomain, + message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) random prefix generation to {custom_domain.random_prefix_generation}", + ) Session.commit() if custom_domain.random_prefix_generation: diff --git a/app/dashboard/views/subdomain.py b/app/dashboard/views/subdomain.py index 04f7880a4..a1cfbd983 100644 --- a/app/dashboard/views/subdomain.py +++ b/app/dashboard/views/subdomain.py @@ -11,6 +11,7 @@ from app.errors import SubdomainInTrashError from app.log import LOG from app.models import CustomDomain, Mailbox, SLDomain +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction # Only lowercase letters, numbers, dashes (-) are currently supported _SUBDOMAIN_PATTERN = r"[0-9a-z-]{1,}" @@ -102,6 +103,12 @@ def subdomain_route(): ownership_verified=True, commit=True, ) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.CreateCustomDomain, + message=f"Create subdomain {new_custom_domain.id} ({full_domain})", + commit=True, + ) except SubdomainInTrashError: flash( f"{full_domain} has been used before and cannot be reused", diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py index 665b218f3..7b3c16720 100644 --- a/app/user_audit_log_utils.py +++ b/app/user_audit_log_utils.py @@ -15,10 +15,6 @@ class UserAuditLogAction(Enum): UpdateMailbox = "update_mailbox" DeleteMailbox = "delete_mailbox" - CreateSubdomain = "create_subdomain" - UpdateSubdomain = "update_subdomain" - DeleteSubdomain = "delete_subdomain" - CreateCustomDomain = "create_custom_domain" VerifyCustomDomain = "verify_custom_domain" UpdateCustomDomain = "update_custom_domain" diff --git a/job_runner.py b/job_runner.py index 4567c845d..a7de58a7c 100644 --- a/job_runner.py +++ b/job_runner.py @@ -251,6 +251,7 @@ def process_job(job: Job): if not custom_domain: return + is_subdomain = custom_domain.is_sl_subdomain domain_name = custom_domain.domain user = custom_domain.user @@ -258,6 +259,16 @@ def process_job(job: Job): CustomDomain.delete(custom_domain.id) Session.commit() + if is_subdomain: + message = f"Delete subdomain {custom_domain_id} ({domain_name})" + else: + message = f"Delete custom domain {custom_domain_id} ({domain_name})" + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.DeleteCustomDomain, + message=message, + ) + LOG.d("Domain %s deleted", domain_name) if custom_domain_partner_id is None: From e80cc03c6ed3467a2974f9d798a81ec96b056b7c Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 09:59:19 +0200 Subject: [PATCH 06/16] chore: user_audit_log for contacts --- app/contact_utils.py | 7 +++++++ app/dashboard/views/alias_contact_manager.py | 9 ++++++++- app/dashboard/views/contact_detail.py | 15 ++++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/contact_utils.py b/app/contact_utils.py index 31a8d95d0..04bb3e3ec 100644 --- a/app/contact_utils.py +++ b/app/contact_utils.py @@ -9,6 +9,7 @@ from app.email_validation import is_valid_email from app.log import LOG from app.models import Contact, Alias +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import sanitize_email @@ -100,6 +101,12 @@ def create_contact( invalid_email=email == "", commit=True, ) + emit_user_audit_log( + user_id=alias.user_id, + action=UserAuditLogAction.CreateContact, + message=f"Created contact {contact.id} ({contact.email})", + commit=True, + ) LOG.d( f"Created contact {contact} for alias {alias} with email {email} invalid_email={contact.invalid_email}" ) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 784c481f6..c1d050b14 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from operator import or_ +from typing import Optional from flask import render_template, request, redirect, flash from flask import url_for @@ -22,6 +23,7 @@ ) from app.log import LOG from app.models import Alias, Contact, EmailLog +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import CSRFValidationForm @@ -190,7 +192,7 @@ def get_contact_infos( def delete_contact(alias: Alias, contact_id: int): - contact = Contact.get(contact_id) + contact: Optional[Contact] = Contact.get(contact_id) if not contact: flash("Unknown error. Refresh the page", "warning") @@ -198,6 +200,11 @@ def delete_contact(alias: Alias, contact_id: int): flash("You cannot delete reverse-alias", "warning") else: delete_contact_email = contact.website_email + emit_user_audit_log( + user_id=alias.user_id, + action=UserAuditLogAction.DeleteContact, + message=f"Delete contact {contact_id} ({contact.email})", + ) Contact.delete(contact_id) Session.commit() diff --git a/app/dashboard/views/contact_detail.py b/app/dashboard/views/contact_detail.py index 372aca0be..354196c0e 100644 --- a/app/dashboard/views/contact_detail.py +++ b/app/dashboard/views/contact_detail.py @@ -1,3 +1,5 @@ +from typing import Optional + from flask import render_template, request, redirect, url_for, flash from flask_login import login_required, current_user from flask_wtf import FlaskForm @@ -7,6 +9,7 @@ from app.db import Session from app.models import Contact from app.pgp_utils import PGPException, load_public_key_and_check +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction class PGPContactForm(FlaskForm): @@ -20,7 +23,7 @@ class PGPContactForm(FlaskForm): @dashboard_bp.route("/contact//", methods=["GET", "POST"]) @login_required def contact_detail_route(contact_id): - contact = Contact.get(contact_id) + contact: Optional[Contact] = Contact.get(contact_id) if not contact or contact.user_id != current_user.id: flash("You cannot see this page", "warning") return redirect(url_for("dashboard.index")) @@ -50,6 +53,11 @@ def contact_detail_route(contact_id): except PGPException: flash("Cannot add the public key, please verify it", "error") else: + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateContact, + message=f"Added PGP key {contact.pgp_public_key} for contact {contact_id} ({contact.email})", + ) Session.commit() flash( f"PGP public key for {contact.email} is saved successfully", @@ -62,6 +70,11 @@ def contact_detail_route(contact_id): ) elif pgp_form.action.data == "remove": # Free user can decide to remove contact PGP key + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateContact, + message=f"Removed PGP key {contact.pgp_public_key} for contact {contact_id} ({contact.email})", + ) contact.pgp_public_key = None contact.pgp_finger_print = None Session.commit() From 28690e07d395c56bbe0ecaae2ec54d113ff6d9d0 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 10:11:18 +0200 Subject: [PATCH 07/16] chore: user_audit_log for directories --- app/dashboard/views/directory.py | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/app/dashboard/views/directory.py b/app/dashboard/views/directory.py index 668ed0133..d86385aa5 100644 --- a/app/dashboard/views/directory.py +++ b/app/dashboard/views/directory.py @@ -1,3 +1,5 @@ +from typing import Optional + from flask import render_template, request, redirect, url_for, flash from flask_login import login_required, current_user from flask_wtf import FlaskForm @@ -20,6 +22,7 @@ from app.db import Session from app.errors import DirectoryInTrashError from app.models import Directory, Mailbox, DirectoryMailbox +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction class NewDirForm(FlaskForm): @@ -69,7 +72,9 @@ def directory(): if not delete_dir_form.validate(): flash("Invalid request", "warning") return redirect(url_for("dashboard.directory")) - dir_obj = Directory.get(delete_dir_form.directory_id.data) + dir_obj: Optional[Directory] = Directory.get( + delete_dir_form.directory_id.data + ) if not dir_obj: flash("Unknown error. Refresh the page", "warning") @@ -79,6 +84,11 @@ def directory(): return redirect(url_for("dashboard.directory")) name = dir_obj.name + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.DeleteDirectory, + message=f"Delete directory {dir_obj.id} ({dir_obj.name})", + ) Directory.delete(dir_obj.id) Session.commit() flash(f"Directory {name} has been deleted", "success") @@ -90,7 +100,7 @@ def directory(): flash("Invalid request", "warning") return redirect(url_for("dashboard.directory")) dir_id = toggle_dir_form.directory_id.data - dir_obj = Directory.get(dir_id) + dir_obj: Optional[Directory] = Directory.get(dir_id) if not dir_obj or dir_obj.user_id != current_user.id: flash("Unknown error. Refresh the page", "warning") @@ -103,6 +113,11 @@ def directory(): dir_obj.disabled = True flash(f"On-the-fly is disabled for {dir_obj.name}", "warning") + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateDirectory, + message=f"Updated directory {dir_obj.id} ({dir_obj.name}) set disabled = {dir_obj.disabled}", + ) Session.commit() return redirect(url_for("dashboard.directory")) @@ -112,7 +127,7 @@ def directory(): flash("Invalid request", "warning") return redirect(url_for("dashboard.directory")) dir_id = update_dir_form.directory_id.data - dir_obj = Directory.get(dir_id) + dir_obj: Optional[Directory] = Directory.get(dir_id) if not dir_obj or dir_obj.user_id != current_user.id: flash("Unknown error. Refresh the page", "warning") @@ -143,6 +158,12 @@ def directory(): for mailbox in mailboxes: DirectoryMailbox.create(directory_id=dir_obj.id, mailbox_id=mailbox.id) + mailboxes_as_str = ",".join(map(str, mailbox_ids)) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UpdateDirectory, + message=f"Updated directory {dir_obj.id} ({dir_obj.name}) mailboxes ({mailboxes_as_str})", + ) Session.commit() flash(f"Directory {dir_obj.name} has been updated", "success") @@ -181,6 +202,11 @@ def directory(): new_dir = Directory.create( name=new_dir_name, user_id=current_user.id ) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.CreateDirectory, + message=f"New directory {new_dir.name} ({new_dir.name})", + ) except DirectoryInTrashError: flash( f"{new_dir_name} has been used before and cannot be reused", From a912c094bc5ac1a5749e7d1451022d15a54989d2 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 11:34:21 +0200 Subject: [PATCH 08/16] fix: do not enforce cronjob being defined in choices + enable user deletion --- cron.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/cron.py b/cron.py index 8645a8cf1..7e75731c2 100644 --- a/cron.py +++ b/cron.py @@ -60,6 +60,7 @@ ) from app.pgp_utils import load_public_key_and_check, PGPException from app.proton.utils import get_proton_partner +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction from app.utils import sanitize_email from server import create_light_app from tasks.clean_alias_audit_log import cleanup_alias_audit_log @@ -1220,7 +1221,7 @@ def notify_hibp(): def clear_users_scheduled_to_be_deleted(dry_run=False): - users = User.filter( + users: List[User] = User.filter( and_( User.delete_on.isnot(None), User.delete_on <= arrow.now().shift(days=-DELETE_GRACE_DAYS), @@ -1232,6 +1233,11 @@ def clear_users_scheduled_to_be_deleted(dry_run=False): ) if dry_run: continue + emit_user_audit_log( + user_id=user.id, + action=UserAuditLogAction.DeleteUser, + message=f"Delete user {user.id} ({user.email})", + ) User.delete(user.id) Session.commit() @@ -1261,22 +1267,6 @@ def clear_user_audit_log(): "--job", help="Choose a cron job to run", type=str, - choices=[ - "stats", - "notify_trial_end", - "notify_manual_subscription_end", - "notify_premium_end", - "delete_logs", - "delete_old_data", - "poll_apple_subscription", - "sanity_check", - "delete_old_monitoring", - "check_custom_domain", - "check_hibp", - "notify_hibp", - "cleanup_tokens", - "send_undelivered_mails", - ], ) args = parser.parse_args() # wrap in an app context to benefit from app setup like database cleanup, sentry integration, etc @@ -1325,7 +1315,7 @@ def clear_user_audit_log(): load_unsent_mails_from_fs_and_resend() elif args.job == "delete_scheduled_users": LOG.d("Deleting users scheduled to be deleted") - clear_users_scheduled_to_be_deleted(dry_run=True) + clear_users_scheduled_to_be_deleted() elif args.job == "clear_alias_audit_log": LOG.d("Clearing alias audit log") clear_alias_audit_log() From a8a2d8590e372f58c06b9340dbd57ce86dafa3ce Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 11:34:38 +0200 Subject: [PATCH 09/16] chore: user_audit_log for user deletion --- app/api/views/user.py | 6 ++++++ app/dashboard/views/delete_account.py | 6 ++++++ app/user_audit_log_utils.py | 1 + 3 files changed, 13 insertions(+) diff --git a/app/api/views/user.py b/app/api/views/user.py index 8a98e328e..d0adca2ef 100644 --- a/app/api/views/user.py +++ b/app/api/views/user.py @@ -6,6 +6,7 @@ from app.extensions import limiter from app.log import LOG from app.models import Job, ApiToCookieToken +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction @api_bp.route("/user", methods=["DELETE"]) @@ -16,6 +17,11 @@ def delete_user(): """ # Schedule delete account job + emit_user_audit_log( + user_id=g.user.id, + action=UserAuditLogAction.UserMarkedForDeletion, + message=f"Marked user {g.user.id} ({g.user.email}) for deletion from API", + ) LOG.w("schedule delete account job for %s", g.user) Job.create( name=config.JOB_DELETE_ACCOUNT, diff --git a/app/dashboard/views/delete_account.py b/app/dashboard/views/delete_account.py index b3281b1e5..fa4694847 100644 --- a/app/dashboard/views/delete_account.py +++ b/app/dashboard/views/delete_account.py @@ -8,6 +8,7 @@ from app.dashboard.views.enter_sudo import sudo_required from app.log import LOG from app.models import Subscription, Job +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction class DeleteDirForm(FlaskForm): @@ -33,6 +34,11 @@ def delete_account(): # Schedule delete account job LOG.w("schedule delete account job for %s", current_user) + emit_user_audit_log( + user_id=current_user.id, + action=UserAuditLogAction.UserMarkedForDeletion, + message=f"User {current_user.id} ({current_user.email}) marked for deletion via webapp", + ) Job.create( name=JOB_DELETE_ACCOUNT, payload={"user_id": current_user.id}, diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py index 7b3c16720..8089a324e 100644 --- a/app/user_audit_log_utils.py +++ b/app/user_audit_log_utils.py @@ -28,6 +28,7 @@ class UserAuditLogAction(Enum): UpdateDirectory = "update_directory" DeleteDirectory = "delete_directory" + UserMarkedForDeletion = "user_marked_for_deletion" DeleteUser = "delete_user" From edaeb94af0952f4cb2fd810c840cfdc132875a4e Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 11:49:00 +0200 Subject: [PATCH 10/16] refactor: change emit_user_audit_log function to receive the full user object --- app/account_linking.py | 8 ++++---- app/api/views/user.py | 2 +- app/contact_utils.py | 2 +- app/custom_domain_utils.py | 4 ++-- app/custom_domain_validation.py | 10 +++++----- app/dashboard/views/alias_contact_manager.py | 2 +- app/dashboard/views/contact_detail.py | 4 ++-- app/dashboard/views/delete_account.py | 2 +- app/dashboard/views/directory.py | 8 ++++---- app/dashboard/views/domain_detail.py | 8 ++++---- app/dashboard/views/mailbox.py | 2 +- app/dashboard/views/mailbox_detail.py | 20 ++++++++++---------- app/dashboard/views/subdomain.py | 2 +- app/mailbox_utils.py | 4 ++-- app/models.py | 6 +++++- app/partner_user_utils.py | 10 +++++----- app/payments/coinbase.py | 8 +++++--- app/payments/paddle.py | 10 +++++----- app/proton/utils.py | 2 +- app/user_audit_log_utils.py | 7 ++++--- app/user_settings.py | 2 +- cron.py | 2 +- job_runner.py | 4 ++-- 23 files changed, 68 insertions(+), 61 deletions(-) diff --git a/app/account_linking.py b/app/account_linking.py index 80b42bf52..7be68bba8 100644 --- a/app/account_linking.py +++ b/app/account_linking.py @@ -84,7 +84,7 @@ def set_plan_for_partner_user(partner_user: PartnerUser, plan: SLPlan): ) sub.end_at = plan.expiration emit_user_audit_log( - user_id=partner_user.user_id, + user=partner_user.user, action=UserAuditLogAction.SubscriptionExtended, message="Extended partner subscription", ) @@ -107,7 +107,7 @@ def ensure_partner_user_exists_for_user( raise AccountAlreadyLinkedToAnotherPartnerException() if not res: res = create_partner_user( - user_id=sl_user.id, + user=sl_user, partner_id=partner.id, partner_email=link_request.email, external_user_id=link_request.external_user_id, @@ -149,7 +149,7 @@ def process(self) -> LinkResult: from_partner=self.link_request.from_partner, ) partner_user = create_partner_user( - user_id=new_user.id, + user=new_user, partner_id=self.partner.id, external_user_id=self.link_request.external_user_id, partner_email=self.link_request.email, @@ -287,7 +287,7 @@ def switch_already_linked_user( old_info = f"Old: (external_user_id={other_partner_user.external_user_id} | partner_email={other_partner_user.partner_email})" new_info = f"New: (external_user_id={partner_user.external_user_id} | partner_email={partner_user.partner_email})" emit_user_audit_log( - user_id=other_partner_user.user_id, + user=other_partner_user.user, action=UserAuditLogAction.UnlinkAccount, message=f"Unlinking from partner, as user will now be tied to another external account. {old_info} | {new_info}", ) diff --git a/app/api/views/user.py b/app/api/views/user.py index d0adca2ef..b55d96085 100644 --- a/app/api/views/user.py +++ b/app/api/views/user.py @@ -18,7 +18,7 @@ def delete_user(): """ # Schedule delete account job emit_user_audit_log( - user_id=g.user.id, + user=g.user, action=UserAuditLogAction.UserMarkedForDeletion, message=f"Marked user {g.user.id} ({g.user.email}) for deletion from API", ) diff --git a/app/contact_utils.py b/app/contact_utils.py index 04bb3e3ec..9a97797c8 100644 --- a/app/contact_utils.py +++ b/app/contact_utils.py @@ -102,7 +102,7 @@ def create_contact( commit=True, ) emit_user_audit_log( - user_id=alias.user_id, + user=alias.user, action=UserAuditLogAction.CreateContact, message=f"Created contact {contact.id} ({contact.email})", commit=True, diff --git a/app/custom_domain_utils.py b/app/custom_domain_utils.py index 937cea2fb..6724a47a9 100644 --- a/app/custom_domain_utils.py +++ b/app/custom_domain_utils.py @@ -139,7 +139,7 @@ def create_custom_domain( new_custom_domain.partner_id = partner_id emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.CreateCustomDomain, message=f"Created custom domain {new_custom_domain.id} ({new_domain})", ) @@ -198,7 +198,7 @@ def set_custom_domain_mailboxes( mailboxes_as_str = ",".join(map(str, mailbox_ids)) emit_user_audit_log( - user_id=user_id, + user=custom_domain.user, action=UserAuditLogAction.UpdateCustomDomain, message=f"Updated custom domain {custom_domain.id} mailboxes (domain={custom_domain.domain}) (mailboxes={mailboxes_as_str})", ) diff --git a/app/custom_domain_validation.py b/app/custom_domain_validation.py index 1a957af1a..255756460 100644 --- a/app/custom_domain_validation.py +++ b/app/custom_domain_validation.py @@ -124,7 +124,7 @@ def validate_dkim_records(self, custom_domain: CustomDomain) -> dict[str, str]: custom_domain.dkim_verified = len(invalid_records) == 0 if custom_domain.dkim_verified: emit_user_audit_log( - user_id=custom_domain.user_id, + user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified DKIM records for custom domain {custom_domain.id} ({custom_domain.domain})", ) @@ -145,7 +145,7 @@ def validate_domain_ownership( if expected_verification_record in txt_records: custom_domain.ownership_verified = True emit_user_audit_log( - user_id=custom_domain.user_id, + user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified ownership for custom domain {custom_domain.id} ({custom_domain.domain})", ) @@ -168,7 +168,7 @@ def validate_mx_records( else: custom_domain.verified = True emit_user_audit_log( - user_id=custom_domain.user_id, + user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified MX records for custom domain {custom_domain.id} ({custom_domain.domain})", ) @@ -187,7 +187,7 @@ def validate_spf_records( else: custom_domain.spf_verified = False emit_user_audit_log( - user_id=custom_domain.user_id, + user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified SPF records for custom domain {custom_domain.id} ({custom_domain.domain})", ) @@ -206,7 +206,7 @@ def validate_dmarc_records( if DMARC_RECORD in txt_records: custom_domain.dmarc_verified = True emit_user_audit_log( - user_id=custom_domain.user_id, + user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified DMARC records for custom domain {custom_domain.id} ({custom_domain.domain})", ) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index c1d050b14..656c53d69 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -201,7 +201,7 @@ def delete_contact(alias: Alias, contact_id: int): else: delete_contact_email = contact.website_email emit_user_audit_log( - user_id=alias.user_id, + user=alias.user, action=UserAuditLogAction.DeleteContact, message=f"Delete contact {contact_id} ({contact.email})", ) diff --git a/app/dashboard/views/contact_detail.py b/app/dashboard/views/contact_detail.py index 354196c0e..e84a79eba 100644 --- a/app/dashboard/views/contact_detail.py +++ b/app/dashboard/views/contact_detail.py @@ -54,7 +54,7 @@ def contact_detail_route(contact_id): flash("Cannot add the public key, please verify it", "error") else: emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateContact, message=f"Added PGP key {contact.pgp_public_key} for contact {contact_id} ({contact.email})", ) @@ -71,7 +71,7 @@ def contact_detail_route(contact_id): elif pgp_form.action.data == "remove": # Free user can decide to remove contact PGP key emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateContact, message=f"Removed PGP key {contact.pgp_public_key} for contact {contact_id} ({contact.email})", ) diff --git a/app/dashboard/views/delete_account.py b/app/dashboard/views/delete_account.py index fa4694847..086949fbd 100644 --- a/app/dashboard/views/delete_account.py +++ b/app/dashboard/views/delete_account.py @@ -35,7 +35,7 @@ def delete_account(): # Schedule delete account job LOG.w("schedule delete account job for %s", current_user) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UserMarkedForDeletion, message=f"User {current_user.id} ({current_user.email}) marked for deletion via webapp", ) diff --git a/app/dashboard/views/directory.py b/app/dashboard/views/directory.py index d86385aa5..28d0de7f1 100644 --- a/app/dashboard/views/directory.py +++ b/app/dashboard/views/directory.py @@ -85,7 +85,7 @@ def directory(): name = dir_obj.name emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.DeleteDirectory, message=f"Delete directory {dir_obj.id} ({dir_obj.name})", ) @@ -114,7 +114,7 @@ def directory(): flash(f"On-the-fly is disabled for {dir_obj.name}", "warning") emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateDirectory, message=f"Updated directory {dir_obj.id} ({dir_obj.name}) set disabled = {dir_obj.disabled}", ) @@ -160,7 +160,7 @@ def directory(): mailboxes_as_str = ",".join(map(str, mailbox_ids)) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateDirectory, message=f"Updated directory {dir_obj.id} ({dir_obj.name}) mailboxes ({mailboxes_as_str})", ) @@ -203,7 +203,7 @@ def directory(): name=new_dir_name, user_id=current_user.id ) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.CreateDirectory, message=f"New directory {new_dir.name} ({new_dir.name})", ) diff --git a/app/dashboard/views/domain_detail.py b/app/dashboard/views/domain_detail.py index 638a57bdb..2606f6d17 100644 --- a/app/dashboard/views/domain_detail.py +++ b/app/dashboard/views/domain_detail.py @@ -166,7 +166,7 @@ def domain_detail(custom_domain_id): if request.form.get("form-name") == "switch-catch-all": custom_domain.catch_all = not custom_domain.catch_all emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateCustomDomain, message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) catch all to {custom_domain.catch_all}", ) @@ -189,7 +189,7 @@ def domain_detail(custom_domain_id): if request.form.get("action") == "save": custom_domain.name = request.form.get("alias-name").replace("\n", "") emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateCustomDomain, message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) name", ) @@ -201,7 +201,7 @@ def domain_detail(custom_domain_id): else: custom_domain.name = None emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateCustomDomain, message=f"Cleared custom domain {custom_domain.id} ({custom_domain.domain}) name", ) @@ -219,7 +219,7 @@ def domain_detail(custom_domain_id): not custom_domain.random_prefix_generation ) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateCustomDomain, message=f"Switched custom domain {custom_domain.id} ({custom_domain.domain}) random prefix generation to {custom_domain.random_prefix_generation}", ) diff --git a/app/dashboard/views/mailbox.py b/app/dashboard/views/mailbox.py index 225d40cb1..e7124716e 100644 --- a/app/dashboard/views/mailbox.py +++ b/app/dashboard/views/mailbox.py @@ -164,7 +164,7 @@ def verify_with_signed_secret(request: str): mailbox.verified = True emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.VerifyMailbox, message=f"Verified mailbox {mailbox.id} ({mailbox.email})", ) diff --git a/app/dashboard/views/mailbox_detail.py b/app/dashboard/views/mailbox_detail.py index 12a5e9f1a..357038d41 100644 --- a/app/dashboard/views/mailbox_detail.py +++ b/app/dashboard/views/mailbox_detail.py @@ -93,7 +93,7 @@ def mailbox_detail_route(mailbox_id): force_spf_value = request.form.get("spf-status") == "on" mailbox.force_spf = force_spf_value emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Set force_spf to {force_spf_value} on mailbox {mailbox_id} ({mailbox.email})", ) @@ -120,7 +120,7 @@ def mailbox_detail_route(mailbox_id): flash(f"{address} already added", "error") else: emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Add authorized address {address} to mailbox {mailbox_id} ({mailbox.email})", ) @@ -145,7 +145,7 @@ def mailbox_detail_route(mailbox_id): else: address = authorized_address.email emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Remove authorized address {address} from mailbox {mailbox_id} ({mailbox.email})", ) @@ -182,7 +182,7 @@ def mailbox_detail_route(mailbox_id): flash("Cannot add the public key, please verify it", "error") else: emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Add PGP Key {mailbox.pgp_finger_print} to mailbox {mailbox_id} ({mailbox.email})", ) @@ -194,7 +194,7 @@ def mailbox_detail_route(mailbox_id): elif request.form.get("action") == "remove": # Free user can decide to remove their added PGP key emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Remove PGP Key {mailbox.pgp_finger_print} from mailbox {mailbox_id} ({mailbox.email})", ) @@ -218,7 +218,7 @@ def mailbox_detail_route(mailbox_id): else: mailbox.disable_pgp = False emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Enabled PGP for mailbox {mailbox_id} ({mailbox.email})", ) @@ -226,7 +226,7 @@ def mailbox_detail_route(mailbox_id): else: mailbox.disable_pgp = True emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Disabled PGP for mailbox {mailbox_id} ({mailbox.email})", ) @@ -240,7 +240,7 @@ def mailbox_detail_route(mailbox_id): if request.form.get("action") == "save": mailbox.generic_subject = request.form.get("generic-subject") emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Set generic subject for mailbox {mailbox_id} ({mailbox.email})", ) @@ -252,7 +252,7 @@ def mailbox_detail_route(mailbox_id): elif request.form.get("action") == "remove": mailbox.generic_subject = None emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Remove generic subject for mailbox {mailbox_id} ({mailbox.email})", ) @@ -340,7 +340,7 @@ def mailbox_confirm_change_route(): ) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UpdateMailbox, message=f"Change mailbox email for mailbox {mailbox_id} (old={mailbox.email} | new={mailbox.new_email})", ) diff --git a/app/dashboard/views/subdomain.py b/app/dashboard/views/subdomain.py index a1cfbd983..695204199 100644 --- a/app/dashboard/views/subdomain.py +++ b/app/dashboard/views/subdomain.py @@ -104,7 +104,7 @@ def subdomain_route(): commit=True, ) emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.CreateCustomDomain, message=f"Create subdomain {new_custom_domain.id} ({full_domain})", commit=True, diff --git a/app/mailbox_utils.py b/app/mailbox_utils.py index af4270fc9..413144e60 100644 --- a/app/mailbox_utils.py +++ b/app/mailbox_utils.py @@ -75,7 +75,7 @@ def create_mailbox( email=email, user_id=user.id, verified=verified, commit=True ) emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.CreateMailbox, message=f"Create mailbox {new_mailbox.id} ({new_mailbox.email}). Verified={verified}", ) @@ -211,7 +211,7 @@ def verify_mailbox_code(user: User, mailbox_id: int, code: str) -> Mailbox: LOG.i(f"User {user} has verified mailbox {mailbox_id}") mailbox.verified = True emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.VerifyMailbox, message=f"Verify mailbox {mailbox_id} ({mailbox.email})", ) diff --git a/app/models.py b/app/models.py index 11242e8a3..df9a8b996 100644 --- a/app/models.py +++ b/app/models.py @@ -3831,7 +3831,11 @@ class UserAuditLog(Base, ModelMixin): __tablename__ = "user_audit_log" user_id = sa.Column(sa.Integer, nullable=False) + user_email = sa.Column(sa.String(255), nullable=False) action = sa.Column(sa.String(255), nullable=False) message = sa.Column(sa.Text, default=None, nullable=True) - __table_args__ = (sa.Index("ix_user_audit_log_user_id", "user_id"),) + __table_args__ = ( + sa.Index("ix_user_audit_log_user_id", "user_id"), + sa.Index("ix_user_audit_log_user_email", "user_email"), + ) diff --git a/app/partner_user_utils.py b/app/partner_user_utils.py index 921c78ead..c25f0b0c8 100644 --- a/app/partner_user_utils.py +++ b/app/partner_user_utils.py @@ -2,21 +2,21 @@ from arrow import Arrow -from app.models import PartnerUser, PartnerSubscription +from app.models import PartnerUser, PartnerSubscription, User from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction def create_partner_user( - user_id: int, partner_id: int, partner_email: str, external_user_id: str + user: User, partner_id: int, partner_email: str, external_user_id: str ) -> PartnerUser: instance = PartnerUser.create( - user_id=user_id, + user_id=user.id, partner_id=partner_id, partner_email=partner_email, external_user_id=external_user_id, ) emit_user_audit_log( - user_id=user_id, + user=user, action=UserAuditLogAction.LinkAccount, message=f"Linked account to partner_id={partner_id} | partner_email={partner_email} | external_user_id={external_user_id}", ) @@ -38,7 +38,7 @@ def create_partner_subscription( if msg: message += f" | {msg}" emit_user_audit_log( - user_id=partner_user.user_id, + user=partner_user.user, action=UserAuditLogAction.Upgrade, message=message, ) diff --git a/app/payments/coinbase.py b/app/payments/coinbase.py index dda059530..8d955dc50 100644 --- a/app/payments/coinbase.py +++ b/app/payments/coinbase.py @@ -1,3 +1,5 @@ +from typing import Optional + import arrow from coinbase_commerce.error import WebhookInvalidPayload, SignatureVerificationError @@ -49,7 +51,7 @@ def handle_coinbase_event(event) -> bool: user_id = int(float(server_user_id)) code = event["data"]["code"] - user = User.get(user_id) + user: Optional[User] = User.get(user_id) if not user: LOG.e("User not found %s", user_id) return False @@ -64,7 +66,7 @@ def handle_coinbase_event(event) -> bool: user_id=user_id, end_at=arrow.now().shift(years=1), code=code, commit=True ) emit_user_audit_log( - user_id=user_id, + user=user, action=UserAuditLogAction.Upgrade, message="Upgraded though Coinbase", commit=True, @@ -94,7 +96,7 @@ def handle_coinbase_event(event) -> bool: coinbase_subscription.end_at = arrow.now().shift(years=1) emit_user_audit_log( - user_id=user_id, + user=user, action=UserAuditLogAction.SubscriptionExtended, message="Extended coinbase subscription", ) diff --git a/app/payments/paddle.py b/app/payments/paddle.py index 68b91eebe..df2100080 100644 --- a/app/payments/paddle.py +++ b/app/payments/paddle.py @@ -70,7 +70,7 @@ def paddle(): plan=plan, ) emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.Upgrade, message="Upgraded through Paddle", ) @@ -89,7 +89,7 @@ def paddle(): # in case user cancels a plan and subscribes a new plan sub.cancelled = False emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.SubscriptionExtended, message="Extended Paddle subscription", ) @@ -132,7 +132,7 @@ def paddle(): sub.cancelled = True emit_user_audit_log( - user_id=sub.user_id, + user=sub.user, action=UserAuditLogAction.SubscriptionCancelled, message="Cancelled Paddle subscription", ) @@ -191,7 +191,7 @@ def paddle(): # make sure to set the new plan as not-cancelled sub.cancelled = False emit_user_audit_log( - user_id=sub.user_id, + user=sub.user, action=UserAuditLogAction.SubscriptionExtended, message="Extended Paddle subscription", ) @@ -213,7 +213,7 @@ def paddle(): user = sub.user Subscription.delete(sub.id) emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.SubscriptionCancelled, message="Paddle subscription cancelled as user requested a refund", ) diff --git a/app/proton/utils.py b/app/proton/utils.py index e08650d66..abbd3a92b 100644 --- a/app/proton/utils.py +++ b/app/proton/utils.py @@ -34,7 +34,7 @@ def perform_proton_account_unlink(current_user: User): if partner_user is not None: LOG.info(f"User {current_user} has unlinked the account from {partner_user}") emit_user_audit_log( - user_id=current_user.id, + user=current_user, action=UserAuditLogAction.UnlinkAccount, message=f"User has unlinked the account (email={partner_user.partner_email} | external_user_id={partner_user.external_user_id})", ) diff --git a/app/user_audit_log_utils.py b/app/user_audit_log_utils.py index 8089a324e..b83f19c41 100644 --- a/app/user_audit_log_utils.py +++ b/app/user_audit_log_utils.py @@ -1,6 +1,6 @@ from enum import Enum -from app.models import UserAuditLog +from app.models import User, UserAuditLog class UserAuditLogAction(Enum): @@ -33,10 +33,11 @@ class UserAuditLogAction(Enum): def emit_user_audit_log( - user_id: int, action: UserAuditLogAction, message: str, commit: bool = False + user: User, action: UserAuditLogAction, message: str, commit: bool = False ): UserAuditLog.create( - user_id=user_id, + user_id=user.id, + user_email=user.email, action=action.value, message=message, commit=commit, diff --git a/app/user_settings.py b/app/user_settings.py index f616d9918..e782ebd32 100644 --- a/app/user_settings.py +++ b/app/user_settings.py @@ -69,7 +69,7 @@ def set_default_mailbox(user: User, mailbox_id: int) -> Mailbox: user.default_mailbox_id = mailbox.id emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.UpdateMailbox, message=f"Set mailbox {mailbox.id} ({mailbox.email}) as default", ) diff --git a/cron.py b/cron.py index 7e75731c2..1d65adb09 100644 --- a/cron.py +++ b/cron.py @@ -1234,7 +1234,7 @@ def clear_users_scheduled_to_be_deleted(dry_run=False): if dry_run: continue emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.DeleteUser, message=f"Delete user {user.id} ({user.email})", ) diff --git a/job_runner.py b/job_runner.py index a7de58a7c..e2aefc130 100644 --- a/job_runner.py +++ b/job_runner.py @@ -155,7 +155,7 @@ def delete_mailbox_job(job: Job): user = mailbox.user emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.DeleteMailbox, message=f"Delete mailbox {mailbox.id} ({mailbox.email})", ) @@ -264,7 +264,7 @@ def process_job(job: Job): else: message = f"Delete custom domain {custom_domain_id} ({domain_name})" emit_user_audit_log( - user_id=user.id, + user=user, action=UserAuditLogAction.DeleteCustomDomain, message=message, ) From 93152d60e030c00016f0150ed36902aa23ee7a95 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 12:30:18 +0200 Subject: [PATCH 11/16] feat: add user_audit_log migration --- ...2024_101611_7d7b84779837_user_audit_log.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 migrations/versions/2024_101611_7d7b84779837_user_audit_log.py diff --git a/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py b/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py new file mode 100644 index 000000000..6e53db71a --- /dev/null +++ b/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py @@ -0,0 +1,42 @@ +"""user_audit_log + +Revision ID: 7d7b84779837 +Revises: 91ed7f46dc81 +Create Date: 2024-10-16 11:52:49.128644 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '7d7b84779837' +down_revision = '91ed7f46dc81' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('user_audit_log', + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('created_at', sqlalchemy_utils.types.arrow.ArrowType(), nullable=False), + sa.Column('updated_at', sqlalchemy_utils.types.arrow.ArrowType(), nullable=True), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('user_email', sa.String(length=255), nullable=False), + sa.Column('action', sa.String(length=255), nullable=False), + sa.Column('message', sa.Text(), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + op.create_index('ix_user_audit_log_user_email', 'user_audit_log', ['user_email'], unique=False) + op.create_index('ix_user_audit_log_user_id', 'user_audit_log', ['user_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index('ix_user_audit_log_user_id', table_name='user_audit_log') + op.drop_index('ix_user_audit_log_user_email', table_name='user_audit_log') + op.drop_table('user_audit_log') + # ### end Alembic commands ### From bb95e71daea84d1815b052aabea143e42095ed27 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 12:30:22 +0200 Subject: [PATCH 12/16] test: fix tests --- tests/test_mailbox_utils.py | 28 +++++++++++++++++++++++++++- tests/test_server.py | 2 +- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/tests/test_mailbox_utils.py b/tests/test_mailbox_utils.py index bc3bf39e3..b217d7a40 100644 --- a/tests/test_mailbox_utils.py +++ b/tests/test_mailbox_utils.py @@ -218,7 +218,11 @@ def test_delete_with_transfer(): user, random_email(), use_digit_codes=True, send_link=False ).mailbox transfer_mailbox = mailbox_utils.create_mailbox( - user, random_email(), use_digit_codes=True, send_link=False + user, + random_email(), + use_digit_codes=True, + send_link=False, + verified=True, ).mailbox mailbox_utils.delete_mailbox( user, mailbox.id, transfer_mailbox_id=transfer_mailbox.id @@ -236,6 +240,28 @@ def test_delete_with_transfer(): assert job.payload["transfer_mailbox_id"] is None +def test_cannot_delete_with_transfer_to_unverified_mailbox(): + mailbox = mailbox_utils.create_mailbox( + user, random_email(), use_digit_codes=True, send_link=False + ).mailbox + transfer_mailbox = mailbox_utils.create_mailbox( + user, + random_email(), + use_digit_codes=True, + send_link=False, + verified=False, + ).mailbox + + with pytest.raises(mailbox_utils.MailboxError): + mailbox_utils.delete_mailbox( + user, mailbox.id, transfer_mailbox_id=transfer_mailbox.id + ) + + # Verify mailbox still exists + db_mailbox = Mailbox.get_by(id=mailbox.id) + assert db_mailbox is not None + + def test_verify_non_existing_mailbox(): with pytest.raises(mailbox_utils.MailboxError): mailbox_utils.verify_mailbox_code(user, 999999999, "9999999") diff --git a/tests/test_server.py b/tests/test_server.py index bc69880d6..3d3dc446b 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2,7 +2,7 @@ from app.db import Session from app.models import CoinbaseSubscription -from server import handle_coinbase_event +from app.payments.coinbase import handle_coinbase_event from tests.utils import create_new_user From 0ec67c7c97ba6f777dda641b51388229d72957c1 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 14:56:30 +0200 Subject: [PATCH 13/16] test: add some tests for user_audit_log --- app/account_linking.py | 14 ++++-- app/mailbox_utils.py | 1 + tests/test_account_linking.py | 72 +++++++++++++++++++++++++++++- tests/test_user_audit_log_utils.py | 52 +++++++++++++++++++++ 4 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 tests/test_user_audit_log_utils.py diff --git a/app/account_linking.py b/app/account_linking.py index 7be68bba8..ef7c0bd69 100644 --- a/app/account_linking.py +++ b/app/account_linking.py @@ -284,17 +284,25 @@ def switch_already_linked_user( f"Deleting previous partner_user:{other_partner_user.id} from user:{current_user.id}" ) - old_info = f"Old: (external_user_id={other_partner_user.external_user_id} | partner_email={other_partner_user.partner_email})" - new_info = f"New: (external_user_id={partner_user.external_user_id} | partner_email={partner_user.partner_email})" emit_user_audit_log( user=other_partner_user.user, action=UserAuditLogAction.UnlinkAccount, - message=f"Unlinking from partner, as user will now be tied to another external account. {old_info} | {new_info}", + message=f"Deleting partner_user {other_partner_user.id} (external_user_id={other_partner_user.external_user_id} | partner_email={other_partner_user.partner_email}) from user {current_user.id}, as we received a new link request for the same partner", ) PartnerUser.delete(other_partner_user.id) LOG.i(f"Linking partner_user:{partner_user.id} to user:{current_user.id}") # Link this partner_user to the current user + emit_user_audit_log( + user=partner_user.user, + action=UserAuditLogAction.UnlinkAccount, + message=f"Unlinking from partner, as user will now be tied to another external account. old=(id={partner_user.user.id} | email={partner_user.user.email}) | new=(id={current_user.id} | email={current_user.email})", + ) partner_user.user_id = current_user.id + emit_user_audit_log( + user=current_user, + action=UserAuditLogAction.LinkAccount, + message=f"Linking user {current_user.id} ({current_user.email}) to partner_user:{partner_user.id} (external_user_id={partner_user.external_user_id} | partner_email={partner_user.partner_email})", + ) # Set plan set_plan_for_partner_user(partner_user, link_request.plan) Session.commit() diff --git a/app/mailbox_utils.py b/app/mailbox_utils.py index 413144e60..c3c05e56d 100644 --- a/app/mailbox_utils.py +++ b/app/mailbox_utils.py @@ -78,6 +78,7 @@ def create_mailbox( user=user, action=UserAuditLogAction.CreateMailbox, message=f"Create mailbox {new_mailbox.id} ({new_mailbox.email}). Verified={verified}", + commit=True, ) if verified: diff --git a/tests/test_account_linking.py b/tests/test_account_linking.py index 1ef7222e7..7ae6272cf 100644 --- a/tests/test_account_linking.py +++ b/tests/test_account_linking.py @@ -1,3 +1,5 @@ +from typing import List + import pytest from arrow import Arrow @@ -16,8 +18,9 @@ ) from app.db import Session from app.errors import AccountAlreadyLinkedToAnotherPartnerException -from app.models import Partner, PartnerUser, User +from app.models import Partner, PartnerUser, User, UserAuditLog from app.proton.utils import get_proton_partner +from app.user_audit_log_utils import UserAuditLogAction from app.utils import random_string, canonicalize_email from tests.utils import random_email @@ -91,6 +94,11 @@ def test_login_case_from_partner(): ) assert res.user.activated is True + audit_logs: List[UserAuditLog] = UserAuditLog.filter_by(user_id=res.user.id).all() + assert len(audit_logs) == 1 + assert audit_logs[0].user_id == res.user.id + assert audit_logs[0].action == UserAuditLogAction.LinkAccount.value + def test_login_case_from_partner_with_uppercase_email(): partner = get_proton_partner() @@ -125,6 +133,11 @@ def test_login_case_from_web(): assert 0 == (res.user.flags & User.FLAG_CREATED_FROM_PARTNER) assert res.user.activated is True + audit_logs: List[UserAuditLog] = UserAuditLog.filter_by(user_id=res.user.id).all() + assert len(audit_logs) == 1 + assert audit_logs[0].user_id == res.user.id + assert audit_logs[0].action == UserAuditLogAction.LinkAccount.value + def test_get_strategy_existing_sl_user(): email = random_email() @@ -205,6 +218,10 @@ def test_link_account_with_proton_account_same_address(flask_client): ) assert partner_user.partner_id == get_proton_partner().id assert partner_user.external_user_id == partner_user_id + audit_logs: List[UserAuditLog] = UserAuditLog.filter_by(user_id=res.user.id).all() + assert len(audit_logs) == 1 + assert audit_logs[0].user_id == res.user.id + assert audit_logs[0].action == UserAuditLogAction.LinkAccount.value def test_link_account_with_proton_account_different_address(flask_client): @@ -229,6 +246,11 @@ def test_link_account_with_proton_account_different_address(flask_client): assert partner_user.partner_id == get_proton_partner().id assert partner_user.external_user_id == partner_user_id + audit_logs: List[UserAuditLog] = UserAuditLog.filter_by(user_id=res.user.id).all() + assert len(audit_logs) == 1 + assert audit_logs[0].user_id == res.user.id + assert audit_logs[0].action == UserAuditLogAction.LinkAccount.value + def test_link_account_with_proton_account_same_address_but_linked_to_other_user( flask_client, @@ -248,22 +270,54 @@ def test_link_account_with_proton_account_same_address_but_linked_to_other_user( partner_user_id, email=random_email() ) # User already linked with the proton account + # START Ensure sl_user_2 has a partner_user with the right data + partner_user = PartnerUser.get_by( + partner_id=get_proton_partner().id, user_id=sl_user_2.id + ) + assert partner_user is not None + assert partner_user.partner_id == get_proton_partner().id + assert partner_user.external_user_id == partner_user_id + assert partner_user.partner_email == sl_user_2.email + assert partner_user.user_id == sl_user_2.id + # END Ensure sl_user_2 has a partner_user with the right data + + # Proceed to link sl_user_1 res = process_link_case(link_request, sl_user_1, get_proton_partner()) + + # Check that the result is linking sl_user_1 assert res.user.id == sl_user_1.id assert res.user.email == partner_email assert res.strategy == "Link" + # Ensure partner_user for sl_user_1 exists partner_user = PartnerUser.get_by( partner_id=get_proton_partner().id, user_id=sl_user_1.id ) assert partner_user.partner_id == get_proton_partner().id assert partner_user.external_user_id == partner_user_id + # Ensure partner_user for sl_user_2 does not exist anymore partner_user = PartnerUser.get_by( partner_id=get_proton_partner().id, user_id=sl_user_2.id ) assert partner_user is None + # Ensure audit logs for sl_user_1 show the link action + sl_user_1_audit_logs: List[UserAuditLog] = UserAuditLog.filter_by( + user_id=sl_user_1.id + ).all() + assert len(sl_user_1_audit_logs) == 1 + assert sl_user_1_audit_logs[0].user_id == sl_user_1.id + assert sl_user_1_audit_logs[0].action == UserAuditLogAction.LinkAccount.value + + # Ensure audit logs for sl_user_2 show the unlink action + sl_user_2_audit_logs: List[UserAuditLog] = UserAuditLog.filter_by( + user_id=sl_user_2.id + ).all() + assert len(sl_user_2_audit_logs) == 1 + assert sl_user_2_audit_logs[0].user_id == sl_user_2.id + assert sl_user_2_audit_logs[0].action == UserAuditLogAction.UnlinkAccount.value + def test_link_account_with_proton_account_different_address_and_linked_to_other_user( flask_client, @@ -300,6 +354,22 @@ def test_link_account_with_proton_account_different_address_and_linked_to_other_ ) assert partner_user_2 is None + # Ensure audit logs for sl_user_1 show the link action + sl_user_1_audit_logs: List[UserAuditLog] = UserAuditLog.filter_by( + user_id=sl_user_1.id + ).all() + assert len(sl_user_1_audit_logs) == 1 + assert sl_user_1_audit_logs[0].user_id == sl_user_1.id + assert sl_user_1_audit_logs[0].action == UserAuditLogAction.LinkAccount.value + + # Ensure audit logs for sl_user_2 show the unlink action + sl_user_2_audit_logs: List[UserAuditLog] = UserAuditLog.filter_by( + user_id=sl_user_2.id + ).all() + assert len(sl_user_2_audit_logs) == 1 + assert sl_user_2_audit_logs[0].user_id == sl_user_2.id + assert sl_user_2_audit_logs[0].action == UserAuditLogAction.UnlinkAccount.value + def test_cannot_create_instance_of_base_strategy(): with pytest.raises(Exception): diff --git a/tests/test_user_audit_log_utils.py b/tests/test_user_audit_log_utils.py new file mode 100644 index 000000000..a3c9b374c --- /dev/null +++ b/tests/test_user_audit_log_utils.py @@ -0,0 +1,52 @@ +from typing import List + +from app import config, mailbox_utils +from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction +from app.models import UserAuditLog +from app.utils import random_string +from tests.utils import create_new_user, random_email + + +def setup_module(): + config.SKIP_MX_LOOKUP_ON_CHECK = True + + +def teardown_module(): + config.SKIP_MX_LOOKUP_ON_CHECK = False + + +def test_emit_alias_audit_log_for_random_data(): + user = create_new_user() + + message = random_string() + action = UserAuditLogAction.CreateMailbox + emit_user_audit_log( + user=user, + action=action, + message=message, + commit=True, + ) + + logs_for_user: List[UserAuditLog] = UserAuditLog.filter_by(user_id=user.id).all() + assert len(logs_for_user) == 1 + assert logs_for_user[0].user_id == user.id + assert logs_for_user[0].user_email == user.email + assert logs_for_user[0].action == action.value + assert logs_for_user[0].message == message + + +def test_emit_audit_log_on_mailbox_creation(): + user = create_new_user() + output = mailbox_utils.create_mailbox( + user=user, email=random_email(), verified=True + ) + + logs_for_user: List[UserAuditLog] = UserAuditLog.filter_by(user_id=user.id).all() + assert len(logs_for_user) == 1 + assert logs_for_user[0].user_id == user.id + assert logs_for_user[0].user_email == user.email + assert logs_for_user[0].action == UserAuditLogAction.CreateMailbox.value + assert ( + logs_for_user[0].message + == f"Create mailbox {output.mailbox.id} ({output.mailbox.email}). Verified=True" + ) From 564ce1499fda82e6834c1de41a2e03bdaaf8b87a Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 15:52:04 +0200 Subject: [PATCH 14/16] fix: spf record verification user_audit_log --- app/custom_domain_validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/custom_domain_validation.py b/app/custom_domain_validation.py index 255756460..90bed3bb7 100644 --- a/app/custom_domain_validation.py +++ b/app/custom_domain_validation.py @@ -182,16 +182,16 @@ def validate_spf_records( expected_spf_domain = self.get_expected_spf_domain(custom_domain) if expected_spf_domain in spf_domains: custom_domain.spf_verified = True - Session.commit() - return DomainValidationResult(success=True, errors=[]) - else: - custom_domain.spf_verified = False emit_user_audit_log( user=custom_domain.user, action=UserAuditLogAction.VerifyCustomDomain, message=f"Verified SPF records for custom domain {custom_domain.id} ({custom_domain.domain})", ) Session.commit() + return DomainValidationResult(success=True, errors=[]) + else: + custom_domain.spf_verified = False + Session.commit() txt_records = self._dns_client.get_txt_record(custom_domain.domain) cleaned_records = self.__clean_spf_records(txt_records, custom_domain) return DomainValidationResult( From b676ad0d1cf9add6eec04f5b596a19b2afa757bc Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 16:45:08 +0200 Subject: [PATCH 15/16] chore: add missing index to user_audit_log.created_at --- app/models.py | 1 + migrations/versions/2024_101611_7d7b84779837_user_audit_log.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/models.py b/app/models.py index df9a8b996..517ccdef3 100644 --- a/app/models.py +++ b/app/models.py @@ -3838,4 +3838,5 @@ class UserAuditLog(Base, ModelMixin): __table_args__ = ( sa.Index("ix_user_audit_log_user_id", "user_id"), sa.Index("ix_user_audit_log_user_email", "user_email"), + sa.Index("ix_user_audit_log_created_at", "created_at"), ) diff --git a/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py b/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py index 6e53db71a..9eea91611 100644 --- a/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py +++ b/migrations/versions/2024_101611_7d7b84779837_user_audit_log.py @@ -31,6 +31,7 @@ def upgrade(): ) op.create_index('ix_user_audit_log_user_email', 'user_audit_log', ['user_email'], unique=False) op.create_index('ix_user_audit_log_user_id', 'user_audit_log', ['user_id'], unique=False) + op.create_index('ix_user_audit_log_created_at', 'user_audit_log', ['created_at'], unique=False) # ### end Alembic commands ### @@ -38,5 +39,6 @@ def downgrade(): # ### commands auto generated by Alembic - please adjust! ### op.drop_index('ix_user_audit_log_user_id', table_name='user_audit_log') op.drop_index('ix_user_audit_log_user_email', table_name='user_audit_log') + op.drop_index('ix_user_audit_log_created_at', table_name='user_audit_log') op.drop_table('user_audit_log') # ### end Alembic commands ### From d866e9ae1f430bfa0c80a4c3f466368094e5b3d8 Mon Sep 17 00:00:00 2001 From: Carlos Quintana Date: Wed, 16 Oct 2024 16:47:08 +0200 Subject: [PATCH 16/16] chore: add missing index to alias_audit_log.created_at --- app/models.py | 1 + ...bf12f6_alias_audit_log_index_created_at.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 migrations/versions/2024_101616_32f25cbf12f6_alias_audit_log_index_created_at.py diff --git a/app/models.py b/app/models.py index 517ccdef3..5e3c55080 100644 --- a/app/models.py +++ b/app/models.py @@ -3822,6 +3822,7 @@ class AliasAuditLog(Base, ModelMixin): sa.Index("ix_alias_audit_log_user_id", "user_id"), sa.Index("ix_alias_audit_log_alias_id", "alias_id"), sa.Index("ix_alias_audit_log_alias_email", "alias_email"), + sa.Index("ix_alias_audit_log_created_at", "created_at"), ) diff --git a/migrations/versions/2024_101616_32f25cbf12f6_alias_audit_log_index_created_at.py b/migrations/versions/2024_101616_32f25cbf12f6_alias_audit_log_index_created_at.py new file mode 100644 index 000000000..dd29f37d3 --- /dev/null +++ b/migrations/versions/2024_101616_32f25cbf12f6_alias_audit_log_index_created_at.py @@ -0,0 +1,27 @@ +"""alias_audit_log_index_created_at + +Revision ID: 32f25cbf12f6 +Revises: 7d7b84779837 +Create Date: 2024-10-16 16:45:36.827161 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '32f25cbf12f6' +down_revision = '7d7b84779837' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.get_context().autocommit_block(): + op.create_index('ix_alias_audit_log_created_at', 'alias_audit_log', ['created_at'], unique=False, postgresql_concurrently=True) + + +def downgrade(): + with op.get_context().autocommit_block(): + op.drop_index('ix_alias_audit_log_created_at', table_name='alias_audit_log', postgresql_concurrently=True)