From bb2fa960077a4899ef17d9fa31973cd264e5a7de Mon Sep 17 00:00:00 2001 From: Anuj Gupta <84966248+Anuj-Gupta4@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:55:00 +0545 Subject: [PATCH] feat(backend): warn users and delete user accounts after period of inactivity (#2088) * feat(migrations): add last_active_at column to users table * feat(auth): update user authentication to track last active timestamp * feat(migrations): remove NOT NULL constraint from author_id in projects and update revert migration * feat(users): implement process for identifying and handling inactive users * Revert "feat(auth): update user authentication to track last active timestamp" This reverts commit 6441f36cbc4518897024f26bff799998ad5cfcb3. * feat(users): update user retrieval to include last active timestamp tracking * feat(users): rename last_active_at to last_login_at for clarity * feat(auth): update user creation to set last_login_at on conflict * refactor(migration): rename file names from last active at to last login at * feat(migrations): add last_login_at column to users and remove null constraint from project author * revert: dbuser one method to not update last login at * refactor(user): uncomment super admin check for inactive user deletion --------- Signed-off-by: Anuj Gupta <84966248+Anuj-Gupta4@users.noreply.github.com> --- src/backend/app/auth/auth_routes.py | 3 +- src/backend/app/db/models.py | 7 +- src/backend/app/users/user_crud.py | 99 +++++++++++++++++++ src/backend/app/users/user_routes.py | 16 ++- .../migrations/005-add-user-lastloginat.sql | 38 +++++++ .../migrations/init/fmtm_base_schema.sql | 5 +- .../revert/005-add-user-lastloginat.sql | 35 +++++++ 7 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 src/backend/migrations/005-add-user-lastloginat.sql create mode 100644 src/backend/migrations/revert/005-add-user-lastloginat.sql diff --git a/src/backend/app/auth/auth_routes.py b/src/backend/app/auth/auth_routes.py index 438f2355bf..6a6af478dd 100644 --- a/src/backend/app/auth/auth_routes.py +++ b/src/backend/app/auth/auth_routes.py @@ -133,7 +133,8 @@ async def get_or_create_user( ) ON CONFLICT (id) DO UPDATE SET - profile_img = EXCLUDED.profile_img + profile_img = EXCLUDED.profile_img, + last_login_at = NOW() RETURNING id, username, profile_img, role ) diff --git a/src/backend/app/db/models.py b/src/backend/app/db/models.py index 8653a807cf..cbc7bc409c 100644 --- a/src/backend/app/db/models.py +++ b/src/backend/app/db/models.py @@ -161,6 +161,7 @@ class DbUser(BaseModel): projects_mapped: Optional[list[int]] = None api_key: Optional[str] = None registered_at: Optional[AwareDatetime] = None + last_login_at: Optional[AwareDatetime] = None # Relationships project_roles: Optional[dict[int, ProjectRole]] = None # project:role pairs @@ -208,12 +209,12 @@ async def one(cls, db: Connection, user_identifier: int | str) -> Self: sql, {"user_identifier": user_identifier}, ) - db_project = await cur.fetchone() + db_user = await cur.fetchone() - if db_project is None: + if db_user is None: raise KeyError(f"User ({user_identifier}) not found.") - return db_project + return db_user @classmethod async def all( diff --git a/src/backend/app/users/user_crud.py b/src/backend/app/users/user_crud.py index 38d9293a08..76944c4ee0 100644 --- a/src/backend/app/users/user_crud.py +++ b/src/backend/app/users/user_crud.py @@ -16,3 +16,102 @@ # along with FMTM. If not, see . # """Logic for user routes.""" + +from datetime import datetime, timedelta, timezone +from textwrap import dedent + +from fastapi import Request +from loguru import logger as log +from osm_login_python.core import Auth +from psycopg import Connection +from psycopg.rows import class_row + +from app.auth.providers.osm import get_osm_token, send_osm_message +from app.db.models import DbUser + +WARNING_INTERVALS = [21, 14, 7] # Days before deletion +INACTIVITY_THRESHOLD = 2 * 365 # 2 years approx + + +async def process_inactive_users( + db: Connection, + request: Request, + osm_auth: Auth, +): + """Identify inactive users, send warnings, and delete accounts.""" + now = datetime.now(timezone.utc) + warning_thresholds = [ + (now - timedelta(days=INACTIVITY_THRESHOLD - days)) + for days in WARNING_INTERVALS + ] + deletion_threshold = now - timedelta(days=INACTIVITY_THRESHOLD) + + osm_token = get_osm_token(request, osm_auth) + + async with db.cursor() as cur: + # Users eligible for warnings + for days, warning_date in zip( + WARNING_INTERVALS, warning_thresholds, strict=False + ): + async with db.cursor(row_factory=class_row(DbUser)) as cur: + await cur.execute( + """ + SELECT id, username, last_login_at + FROM users + WHERE last_login_at < %(warning_date)s + AND last_login_at >= %(next_warning_date)s; + """, + { + "warning_date": warning_date, + "next_warning_date": warning_date - timedelta(days=7), + }, + ) + users_to_warn = await cur.fetchall() + + for user in users_to_warn: + await send_warning_email_or_osm(user.id, user.username, days, osm_token) + + # Users eligible for deletion + async with db.cursor(row_factory=class_row(DbUser)) as cur: + await cur.execute( + """ + SELECT id, username + FROM users + WHERE last_login_at < %(deletion_threshold)s; + """, + {"deletion_threshold": deletion_threshold}, + ) + users_to_delete = await cur.fetchall() + + for user in users_to_delete: + log.info(f"Deleting user {user.username} due to inactivity.") + await DbUser.delete(db, user.id) + + +async def send_warning_email_or_osm( + user_id: int, + username: str, + days_remaining: int, + osm_token: str, +): + """Send warning email or OSM message to the user.""" + message_content = dedent(f""" + ## Account Deletion Warning + + Hi {username}, + + Your account has been inactive for a long time. To comply with our policy, your + account will be deleted in {days_remaining} days if you do not log in. + + Please log in to reset your inactivity period and avoid deletion. + + Thank you for being a part of our platform! + """) + + send_osm_message( + osm_token=osm_token, + osm_id=user_id, + title="FMTM account deletion warning", + body=message_content, + ) + log.info(f"Sent warning to {username}: {days_remaining} days remaining.") diff --git a/src/backend/app/users/user_routes.py b/src/backend/app/users/user_routes.py index b91e7327ac..67bbdb0c30 100644 --- a/src/backend/app/users/user_routes.py +++ b/src/backend/app/users/user_routes.py @@ -19,16 +19,18 @@ from typing import Annotated, List -from fastapi import APIRouter, Depends, Response +from fastapi import APIRouter, Depends, Request, Response from loguru import logger as log from psycopg import Connection +from app.auth.providers.osm import init_osm_auth from app.auth.roles import mapper, super_admin from app.db.database import db_conn from app.db.enums import HTTPStatus from app.db.enums import UserRole as UserRoleEnum from app.db.models import DbUser from app.users import user_schemas +from app.users.user_crud import process_inactive_users from app.users.user_deps import get_user router = APIRouter( @@ -83,3 +85,15 @@ async def delete_user_by_identifier( await DbUser.delete(db, user.id) log.info(f"User {user.id} deleted successfully.") return Response(status_code=HTTPStatus.NO_CONTENT) + + +@router.post("/process-inactive-users") +async def delete_inactive_users( + request: Request, + db: Annotated[Connection, Depends(db_conn)], + current_user: Annotated[DbUser, Depends(super_admin)], + osm_auth=Depends(init_osm_auth), +): + """Identify inactive users, send warnings, and delete accounts.""" + await process_inactive_users(db, request, osm_auth) + return Response(status_code=HTTPStatus.NO_CONTENT) diff --git a/src/backend/migrations/005-add-user-lastloginat.sql b/src/backend/migrations/005-add-user-lastloginat.sql new file mode 100644 index 0000000000..f64d4266e1 --- /dev/null +++ b/src/backend/migrations/005-add-user-lastloginat.sql @@ -0,0 +1,38 @@ +-- ## Migration add some extra fields. +-- * Add last_login_at to users. +-- * Remove NOT NULL constraint from author_id in projects. + +-- Related issues: +-- https://github.com/hotosm/fmtm/issues/1999 + +-- Start a transaction + +BEGIN; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_name = 'users' + AND column_name = 'last_login_at' + ) THEN + ALTER TABLE users ADD COLUMN last_login_at TIMESTAMPTZ DEFAULT now(); + END IF; +END $$; + +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_name = 'projects' + AND column_name = 'author_id' + AND is_nullable = 'NO' + ) THEN + ALTER TABLE projects ALTER COLUMN author_id DROP NOT NULL; + END IF; +END $$; + +-- Commit the transaction +COMMIT; diff --git a/src/backend/migrations/init/fmtm_base_schema.sql b/src/backend/migrations/init/fmtm_base_schema.sql index 56a2e35584..eea48da46e 100644 --- a/src/backend/migrations/init/fmtm_base_schema.sql +++ b/src/backend/migrations/init/fmtm_base_schema.sql @@ -247,7 +247,7 @@ CREATE TABLE public.projects ( id integer NOT NULL, organisation_id integer, odkid integer, - author_id integer NOT NULL, + author_id integer, name character varying, short_description character varying, description character varying, @@ -357,7 +357,8 @@ CREATE TABLE public.users ( tasks_invalidated integer NOT NULL DEFAULT 0, projects_mapped integer [], api_key character varying, - registered_at timestamp with time zone DEFAULT now() + registered_at timestamp with time zone DEFAULT now(), + last_login_at timestamp with time zone DEFAULT now() ); ALTER TABLE public.users OWNER TO fmtm; diff --git a/src/backend/migrations/revert/005-add-user-lastloginat.sql b/src/backend/migrations/revert/005-add-user-lastloginat.sql new file mode 100644 index 0000000000..2fc5e4d09b --- /dev/null +++ b/src/backend/migrations/revert/005-add-user-lastloginat.sql @@ -0,0 +1,35 @@ +-- * Remove last_login_at from users. +-- * Restore NOT NULL constraint on author_id in projects. + +-- Start a transaction +BEGIN; + +-- Remove last_login_at column from users +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_name = 'users' + AND column_name = 'last_login_at' + ) THEN + ALTER TABLE users DROP COLUMN last_login_at; + END IF; +END $$; + +-- Restore NOT NULL constraint on author_id in projects +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_name = 'projects' + AND column_name = 'author_id' + AND is_nullable = 'YES' + ) THEN + ALTER TABLE projects ALTER COLUMN author_id SET NOT NULL; + END IF; +END $$; + +-- Commit the transaction +COMMIT;