From 145d29401873eb570f8ce90d82ee6026a7eb25df Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sat, 25 Jun 2022 17:21:28 -0400 Subject: [PATCH] implement renewal reminders --- ceo/cli/members.py | 20 ++++++ ceo_common/interfaces/ILDAPService.py | 9 ++- ceo_common/interfaces/IMailService.py | 18 +++++ ceod/api/members.py | 24 +++++-- ceod/model/LDAPService.py | 22 +++++- ceod/model/MailService.py | 13 ++++ .../templates/membership_renewal_reminder.j2 | 29 ++++++++ requirements.txt | 9 +-- setup.cfg | 4 +- tests/ceo/cli/test_members.py | 44 ++++++++++-- tests/ceod/api/test_members.py | 68 +++++++++++++++++++ tests/conftest.py | 30 ++++++-- tests/utils.py | 10 +++ 13 files changed, 279 insertions(+), 21 deletions(-) create mode 100644 ceod/model/templates/membership_renewal_reminder.j2 diff --git a/ceo/cli/members.py b/ceo/cli/members.py index db64729..7972df3 100644 --- a/ceo/cli/members.py +++ b/ceo/cli/members.py @@ -202,3 +202,23 @@ def expire(dry_run): click.echo("The following members has been marked as expired:") for username in result: click.echo(username) + + +@members.command(short_help="Send renewal reminder emails to expiring members") +@click.option('--dry-run', is_flag=True, default=False) +def remindexpire(dry_run): + url = '/api/members/remindexpire' + if dry_run: + url += '?dry_run=true' + resp = http_post(url) + result = handle_sync_response(resp) + + if len(result) > 0: + if dry_run: + click.echo("The following members will be sent membership renewal reminders:") + else: + click.echo("The following members were sent membership renewal reminders:") + for username in result: + click.echo(username) + else: + click.echo("No members are pending expiration.") diff --git a/ceo_common/interfaces/ILDAPService.py b/ceo_common/interfaces/ILDAPService.py index 870cebd..4b928ed 100644 --- a/ceo_common/interfaces/ILDAPService.py +++ b/ceo_common/interfaces/ILDAPService.py @@ -88,8 +88,15 @@ class ILDAPService(Interface): described above. """ - def get_expiring_users(self) -> List[IUser]: + def get_nonflagged_expired_users(self) -> List[IUser]: """ Retrieves members whose term or nonMemberTerm does not contain the current or the last term. """ + + def get_expiring_users(self) -> List[IUser]: + """ + Retrieves members whose membership will expire in less than a month. + This is used to send membership renewal reminders at the beginning + of a term, during the one-month grace period. + """ diff --git a/ceo_common/interfaces/IMailService.py b/ceo_common/interfaces/IMailService.py index b2149d5..53b2125 100644 --- a/ceo_common/interfaces/IMailService.py +++ b/ceo_common/interfaces/IMailService.py @@ -23,3 +23,21 @@ class IMailService(Interface): `operations` is a list of the operations which were performed during the transaction. """ + + def send_membership_renewal_reminder(self, user: IUser): + """ + Send a reminder to the user that their membership will expire + soon. + """ + + def send_cloud_account_will_be_deleted_message(self, user: IUser): + """ + Send a warning message to the user that their cloud resources + will be deleted if they do not renew their membership. + """ + + def send_cloud_account_has_been_deleted_message(self, user: IUser): + """ + Send a message to the user that their cloud resources have + been deleted. + """ diff --git a/ceod/api/members.py b/ceod/api/members.py index ee34bb2..b96e9ca 100644 --- a/ceod/api/members.py +++ b/ceod/api/members.py @@ -1,11 +1,12 @@ -from flask import Blueprint, g, json, request +from flask import Blueprint, g, request +from flask.json import jsonify from zope import component from .utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ create_streaming_response, development_only, is_truthy from ceo_common.errors import BadRequest, UserAlreadySubscribedError, UserNotSubscribedError -from ceo_common.interfaces import ILDAPService, IConfig +from ceo_common.interfaces import ILDAPService, IConfig, IMailService from ceod.transactions.members import ( AddMemberTransaction, ModifyMemberTransaction, @@ -141,7 +142,7 @@ def expire_users(): ldap_srv = component.getUtility(ILDAPService) cfg = component.getUtility(IConfig) - members = ldap_srv.get_expiring_users() + members = ldap_srv.get_nonflagged_expired_users() member_list = cfg.get('mailman3_new_member_list') if not dry_run: @@ -152,4 +153,19 @@ def expire_users(): except UserNotSubscribedError: pass - return json.jsonify([member.uid for member in members]) + return jsonify([member.uid for member in members]) + + +@bp.route('/remindexpire', methods=['POST']) +@authz_restrict_to_syscom +def remind_users_of_expiration(): + dry_run = is_truthy(request.args.get('dry_run', 'false')) + ldap_srv = component.getUtility(ILDAPService) + members = ldap_srv.get_expiring_users() + + if not dry_run: + mail_srv = component.getUtility(IMailService) + for member in members: + mail_srv.send_membership_renewal_reminder(member) + + return jsonify([member.uid for member in members]) diff --git a/ceod/model/LDAPService.py b/ceod/model/LDAPService.py index 24cf814..8c54c65 100644 --- a/ceod/model/LDAPService.py +++ b/ceod/model/LDAPService.py @@ -245,7 +245,7 @@ class LDAPService: except ldap3.core.exceptions.LDAPEntryAlreadyExistsResult: raise GroupAlreadyExistsError() - def get_expiring_users(self) -> List[IUser]: + def get_nonflagged_expired_users(self) -> List[IUser]: syscom_members = self.get_group('syscom').members clauses = [] @@ -275,6 +275,26 @@ class LDAPService: if entry.uid.value not in syscom_members ] + def get_expiring_users(self) -> List[IUser]: + term = Term.current() + dt = ceo_common_utils.get_current_datetime() + if dt.month != term.start_month(): + # We only send membership renewal reminders at the + # start of a term + return [] + last_term = term - 1 + query = f'(&(term={last_term})(!(term={term})))' + conn = self._get_ldap_conn() + conn.search( + self.ldap_users_base, + query, + attributes=ldap3.ALL_ATTRIBUTES, + search_scope=ldap3.LEVEL) + return [ + User.deserialize_from_ldap(entry) + for entry in conn.entries + ] + @contextlib.contextmanager def entry_ctx_for_group(self, group: IGroup): entry = self._get_writable_entry_for_group(group) diff --git a/ceod/model/MailService.py b/ceod/model/MailService.py index 8a07d41..7439336 100644 --- a/ceod/model/MailService.py +++ b/ceod/model/MailService.py @@ -97,6 +97,19 @@ class MailService: body, ) + def send_membership_renewal_reminder(self, user: IUser): + template = self.jinja_env.get_template('membership_renewal_reminder.j2') + body = template.render(user=user) + self.send( + f'Computer Science Club ', + f'{user.cn} <{user.uid}@{self.base_domain}>', + { + 'Subject': 'Computer Science Club membership renewal reminder', + 'Reply-To': f'syscom@{self.base_domain}', + }, + body, + ) + def send_cloud_account_will_be_deleted_message(self, user: IUser): template = self.jinja_env.get_template('cloud_account_will_be_deleted.j2') body = template.render(user=user) diff --git a/ceod/model/templates/membership_renewal_reminder.j2 b/ceod/model/templates/membership_renewal_reminder.j2 new file mode 100644 index 0000000..e2fa105 --- /dev/null +++ b/ceod/model/templates/membership_renewal_reminder.j2 @@ -0,0 +1,29 @@ +Hello {{ user.given_name }}, + +This is an automated message from ceo, the CSC Electronic Office. + +You are receiving this message because your CSC membership will expire +at the end of this month. If you do not wish to renew your membership, +you may safely ignore this message. + +When your CSC membership expires, the following will happen: + +* You will lose access to most CSC resources, including the + general-use machines +* Your CSC email address will be unsubscribed from the csc-general + mailing list +* If you have a CSC cloud account, all of your cloud resources will be + permanently deleted + +Note that even if you are not a member, you are still welcome to +participate in CSC events and interact with CSC on social media, +including Facebook, Instagram, Twitch, Discord and IRC. + +If you wish to renew your membership, please follow the instructions +here: https://csclub.uwaterloo.ca/get-involved/ + +If you have any questions or concerns, please contact the Systems +Committee: syscom@csclub.uwaterloo.ca + +Best regards, +ceo diff --git a/requirements.txt b/requirements.txt index 6622c75..2be9a85 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,10 +5,11 @@ gssapi==1.6.14 gunicorn==20.1.0 Jinja2==3.1.2 ldap3==2.9.1 -requests==2.26.0 -requests-gssapi==1.2.3 -zope.component==5.0.1 -zope.interface==5.4.0 mysql-connector-python==8.0.26 psycopg2==2.9.1 +requests==2.26.0 +requests-gssapi==1.2.3 urwid==2.1.2 +werkzeug==2.1.2 +zope.component==5.0.1 +zope.interface==5.4.0 diff --git a/setup.cfg b/setup.cfg index 767fbf2..00e65d9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -5,5 +5,7 @@ ignore = # unable to detect undefined names F403, # name may be undefined or or defined from star imports - F405 + F405, + # line break before binary operator + W503 exclude = .git,.vscode,venv,__pycache__,__init__.py,build,dist,one_time_scripts diff --git a/tests/ceo/cli/test_members.py b/tests/ceo/cli/test_members.py index cd3174d..09b5acb 100644 --- a/tests/ceo/cli/test_members.py +++ b/tests/ceo/cli/test_members.py @@ -1,14 +1,13 @@ import os import re import shutil -from datetime import datetime +from datetime import datetime, timedelta from click.testing import CliRunner -from unittest.mock import patch from ceo.cli import cli from ceo_common.model import Term -import ceo_common.utils +from tests.utils import set_datetime_in_app_process, restore_datetime_in_app_process def test_members_get(cli_setup, ldap_user): @@ -147,12 +146,13 @@ def test_members_pwreset(cli_setup, ldap_user, krb_user): assert expected_pat.match(result.output) is not None -def test_members_expire(cli_setup, ldap_user, syscom_group): +def test_members_expire(cli_setup, app_process, ldap_user, syscom_group): runner = CliRunner() - with patch.object(ceo_common.utils, 'get_current_datetime') as datetime_mock: + try: # use a time that we know for sure will expire - datetime_mock.return_value = datetime(4000, 4, 1) + test_date = datetime(4000, 4, 1) + set_datetime_in_app_process(app_process, test_date) result = runner.invoke(cli, ['members', 'expire', '--dry-run']) assert result.exit_code == 0 @@ -168,3 +168,35 @@ def test_members_expire(cli_setup, ldap_user, syscom_group): result = runner.invoke(cli, ['members', 'expire', '--dry-run']) assert result.exit_code == 0 assert result.output == '' + finally: + restore_datetime_in_app_process(app_process) + + +def test_members_remindexpire(cli_setup, app_process, ldap_user): + runner = CliRunner() + term = Term(ldap_user.terms[0]) + + try: + test_date = (term + 1).to_datetime() + set_datetime_in_app_process(app_process, test_date) + result = runner.invoke(cli, ['members', 'remindexpire', '--dry-run']) + assert result.exit_code == 0 + assert result.output == ( + "The following members will be sent membership renewal reminders:\n" + f"{ldap_user.uid}\n" + ) + + result = runner.invoke(cli, ['members', 'remindexpire']) + assert result.exit_code == 0 + assert result.output == ( + "The following members were sent membership renewal reminders:\n" + f"{ldap_user.uid}\n" + ) + + test_date = (term + 1).to_datetime() + timedelta(days=40) + set_datetime_in_app_process(app_process, test_date) + result = runner.invoke(cli, ['members', 'remindexpire']) + assert result.exit_code == 0 + assert result.output == "No members are pending expiration.\n" + finally: + restore_datetime_in_app_process(app_process) diff --git a/tests/ceod/api/test_members.py b/tests/ceod/api/test_members.py index 4006316..7818353 100644 --- a/tests/ceod/api/test_members.py +++ b/tests/ceod/api/test_members.py @@ -365,3 +365,71 @@ def test_office_member(cfg, client): status, _ = client.delete('/api/members/test3') assert status == 200 + + +def test_membership_renewal_reminder(client, mock_mail_server): + uids = ['test3', 'test4'] + # fast-forward by one term so that we don't clash with the other users + # created by other tests + term = Term.current() + 1 + with patch.object(ceo_common.utils, 'get_current_datetime') as datetime_mock: + datetime_mock.return_value = term.to_datetime() + for uid in uids: + body = { + 'uid': uid, + 'cn': 'John Doe', + 'given_name': 'John', + 'sn': 'Doe', + 'program': 'Math', + 'terms': [str(term)], + 'forwarding_addresses': [uid + '@uwaterloo.internal'], + } + status, data = client.post('/api/members', json=body) + assert status == 200 and data[-1]['status'] == 'completed' + mock_mail_server.messages.clear() + + # Members were freshly created - nobody should be expirable + status, data = client.post('/api/members/remindexpire') + assert status == 200 + assert data == [] + + next_term = term + 1 + datetime_mock.return_value = next_term.to_datetime() + datetime.timedelta(days=7) + status, data = client.post('/api/members/remindexpire?dry_run=true') + assert status == 200 + assert sorted(uids) == sorted(data) + # dry run - no messages should have been sent + assert len(mock_mail_server.messages) == 0 + + status, data = client.post('/api/members/remindexpire') + assert status == 200 + assert len(mock_mail_server.messages) == len(uids) + assert ( + [uid + '@csclub.internal' for uid in sorted(uids)] + == sorted([msg['to'] for msg in mock_mail_server.messages]) + ) + + # Renew only one of the expiring users + status, _ = client.post(f'/api/members/{uids[0]}/renew', json={'terms': [str(next_term)]}) + assert status == 200 + status, data = client.post('/api/members/remindexpire?dry_run=true') + assert status == 200 + assert len(data) == len(uids) - 1 + + datetime_mock.return_value = next_term.to_datetime() + datetime.timedelta(days=40) + status, data = client.post('/api/members/remindexpire') + assert status == 200 + # one-month grace period has passed - no messages should be sent out + assert data == [] + + next_next_term = next_term + 1 + datetime_mock.return_value = next_next_term.to_datetime() + status, data = client.post('/api/members/remindexpire?dry_run=true') + assert status == 200 + # only the user who renewed last term should get a reminder + assert data == [uids[0]] + + for uid in uids: + status, _ = client.delete(f'/api/members/{uid}') + assert status == 200 + mock_mail_server.messages.clear() diff --git a/tests/conftest.py b/tests/conftest.py index c6d5f99..599e2fc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,11 +1,13 @@ import contextlib import importlib.resources +import multiprocessing from multiprocessing import Process import os import shutil import subprocess from subprocess import DEVNULL import sys +import threading import time from unittest.mock import Mock @@ -28,6 +30,7 @@ from ceo_common.interfaces import IConfig, IKerberosService, ILDAPService, \ IDatabaseService, ICloudStackService, IKubernetesService, IVHostManager, \ ICloudResourceManager, IContainerRegistryService from ceo_common.model import Config, HTTPClient, Term +import ceo_common.utils from ceod.api import create_app from ceod.db import MySQLService, PostgreSQLService from ceod.model import KerberosService, LDAPService, FileService, User, \ @@ -365,7 +368,7 @@ def simple_user(): given_name='John', sn='Doe', program='Math', - terms=['s2021'], + terms=[str(Term.current())], ) @@ -508,13 +511,32 @@ def uwldap_user(cfg, uwldap_srv, ldap_conn): def app_process(cfg, app, http_client): port = cfg.get('ceod_port') hostname = socket.gethostname() + # The parent process may want to mock the datetime + # function in the child process, so we need IPC + mock_datetime_value = None + orig_get_datetime = ceo_common.utils.get_current_datetime - def server_start(): + def mock_get_datetime(): + if mock_datetime_value is not None: + return mock_datetime_value + else: + return orig_get_datetime() + + def ipc_thread_start(pipe): + nonlocal mock_datetime_value + ceo_common.utils.get_current_datetime = mock_get_datetime + while True: + mock_datetime_value = pipe.recv() + pipe.send(None) # ACK + + def server_start(pipe): sys.stdout = open('/dev/null', 'w') sys.stderr = sys.stdout + threading.Thread(target=ipc_thread_start, args=(pipe,)).start() app.run(debug=False, host='0.0.0.0', port=port) - proc = Process(target=server_start) + parent_conn, child_conn = multiprocessing.Pipe() + proc = Process(target=server_start, args=(child_conn,)) proc.start() try: @@ -526,7 +548,7 @@ def app_process(cfg, app, http_client): continue break assert i != 5, 'Timed out' - yield + yield parent_conn finally: proc.terminate() proc.join() diff --git a/tests/utils.py b/tests/utils.py index 871906a..2a57cce 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -60,3 +60,13 @@ def gen_password_mock_ctx(): def mocks_for_create_user_ctx(): with gen_password_mock_ctx(): yield + + +def set_datetime_in_app_process(app_process, value): + pipe = app_process + pipe.send(value) + pipe.recv() # wait for ACK + + +def restore_datetime_in_app_process(app_process): + set_datetime_in_app_process(app_process, None)