implement renewal reminders (#61)

Closes #55.

Once this is merged and deployed, a cron job will be used to automatically run `ceo members remindexpire` at the beginning of every term.

Reviewed-on: #61

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.")

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.
+        """

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.
+        """

ceod/api/members.py

@@ -1,11 +1,13 @@
-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 ceo_common.logger_factory import logger_factory
 from ceod.transactions.members import (
     AddMemberTransaction,
     ModifyMemberTransaction,
@@ -14,6 +16,7 @@ from ceod.transactions.members import (
 import ceod.utils as utils
 
 bp = Blueprint('members', __name__)
+logger = logger_factory(__name__)
 
 
 @bp.route('/', methods=['POST'], strict_slashes=False)
@@ -141,7 +144,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 +155,20 @@ 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:
+            logger.info(f'Sending renewal reminder to {member.uid}')
+            mail_srv.send_membership_renewal_reminder(member)
+
+    return jsonify([member.uid for member in members])

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)

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 <ceo+memberships@{self.base_domain}>',
+            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)

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

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

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

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)

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()

tests/ceod/model/test_user.py

@@ -4,6 +4,7 @@ import subprocess
 import pytest
 
 from ceo_common.errors import UserNotFoundError, UserAlreadyExistsError
+from ceo_common.model import Term
 from ceod.model import User
 
 
@@ -107,13 +108,14 @@ def test_user_forwarding_addresses(cfg, ldap_user):
 
 def test_user_terms(ldap_user, ldap_srv):
     user = ldap_user
+    term = Term(user.terms[0])
 
-    user.add_terms(['f2021'])
+    user.add_terms([str(term + 1)])
-    assert user.terms == ['s2021', 'f2021']
+    assert user.terms == [str(term), str(term + 1)]
     assert ldap_srv.get_user(user.uid).terms == user.terms
 
-    user.add_non_member_terms(['w2022', 's2022'])
+    user.add_non_member_terms([str(term), str(term + 1)])
-    assert user.non_member_terms == ['w2022', 's2022']
+    assert user.non_member_terms == [str(term), str(term + 1)]
     assert ldap_srv.get_user(user.uid).non_member_terms == user.non_member_terms
 
 

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()

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)