use ConfigParser

ceo_common/errors.py

@@ -1,18 +1,37 @@
 class UserNotFoundError(Exception):
-    pass
+    def __init__(self):
+        super().__init__('user not found')
 
 
 class GroupNotFoundError(Exception):
-    pass
+    def __init__(self):
+        super().__init__('group not found')
 
 
 class BadRequest(Exception):
     pass
 
 
+class UserAlreadyExistsError(Exception):
+    def __init__(self):
+        super().__init__('user already exists')
+
+
+class GroupAlreadyExistsError(Exception):
+    def __init__(self):
+        super().__init__('group already exists')
+
+
 class UserAlreadySubscribedError(Exception):
-    pass
+    def __init__(self):
+        super().__init__('user is already subscribed')
 
 
 class UserNotSubscribedError(Exception):
-    pass
+    def __init__(self):
+        super().__init__('user is not subscribed')
+
+
+class NoSuchListError(Exception):
+    def __init__(self):
+        super().__init__('mailing list does not exist')

ceo_common/model/Config.py

@@ -1,3 +1,7 @@
+from configparser import ConfigParser
+import os
+from typing import Union
+
 from zope.interface import implementer
 
 from ceo_common.interfaces import IConfig
@@ -5,39 +9,14 @@ from ceo_common.interfaces import IConfig
 
 @implementer(IConfig)
 class Config:
-    # TODO: read from a config file
-    _domain = 'csclub.internal'
-    _ldap_base = ','.join(['dc=' + dc for dc in _domain.split('.')])
-    _config = {
-        'base_domain': _domain,
-        'ldap_admin_principal': 'ceod/admin',
-        'ldap_server_url': 'ldap://ldap-master.' + _domain,
-        'ldap_users_base': 'ou=People,' + _ldap_base,
-        'ldap_groups_base': 'ou=Group,' + _ldap_base,
-        'ldap_sudo_base': 'ou=SUDOers,' + _ldap_base,
-        'ldap_sasl_realm': _domain.upper(),
-        'uwldap_server_url': 'ldap://uwldap.uwaterloo.ca',
-        'uwldap_base': 'dc=uwaterloo,dc=ca',
-        'member_min_id': 20001,
-        'member_max_id': 29999,
-        'club_min_id': 30001,
-        'club_max_id': 39999,
-        'member_home': '/users',
-        'club_home': '/users',
-        'member_home_skel': '/users/skel',
-        'club_home_skel': '/users/skel',
-        'smtp_url': 'smtp://mail.' + _domain,
-        'smtp_starttls': False,
-        'mailman3_api_base_url': 'http://localhost:8001/3.1',
-        'mailman3_api_username': 'restadmin',
-        'mailman3_api_password': 'mailman3',
-        'new_member_list': 'csc-general',
-        'ceod_admin_host': 'phosphoric-acid',
-        'fs_root_host': 'phosphoric-acid',
-        'mailman_host': 'mail',
-        'ceod_use_https': False,
-        'ceod_port': 9987,
-    }
+    def __init__(self, config_file: Union[str, None] = None):
+        if config_file is None:
+            config_file = os.environ.get('CEOD_CONFIG', '/etc/csc/ceod.ini')
+        self.config = ConfigParser()
+        self.config.read(config_file)
 
     def get(self, key: str) -> str:
-        return self._config[key]
+        section, subkey = key.split('_', 1)
+        if section in self.config:
+            return self.config[section][subkey]
+        return self.config['DEFAULT'][key]

ceo_common/model/HTTPClient.py

@@ -20,6 +20,7 @@ class HTTPClient:
         else:
             self.scheme = 'http'
         self.ceod_port = cfg.get('ceod_port')
+        self.base_domain = cfg.get('base_domain')
 
         # Determine which principal to use for SPNEGO
         # TODO: this code is duplicated in app_factory.py. Figure out
@@ -51,6 +52,9 @@ class HTTPClient:
             target_name='ceod',
             creds=self.get_creds(),
         )
+        # always use the FQDN, for HTTPS purposes
+        if '.' not in host:
+            host = host + '.' + self.base_domain
         return requests.request(
             method,
             f'{self.scheme}://{host}:{self.ceod_port}{api_path}',

ceo_common/model/RemoteMailmanService.py

@@ -8,7 +8,7 @@ from ..interfaces import IMailmanService, IConfig, IHTTPClient
 class RemoteMailmanService:
     def __init__(self):
         cfg = component.getUtility(IConfig)
-        self.mailman_host = cfg.get('mailman_host')
+        self.mailman_host = cfg.get('ceod_mailman_host')
         self.http_client = component.getUtility(IHTTPClient)
 
     def subscribe(self, address: str, mailing_list: str):

ceo_common/test/ceod_dev.ini

@@ -0,0 +1,45 @@
+[DEFAULT]
+base_domain = csclub.internal
+
+[ceod]
+# this is the host with the ceod/admin Kerberos key
+admin_host = phosphoric-acid
+# this is the host with NFS no_root_squash
+fs_root_host = phosphoric-acid
+mailman_host = mail
+use_https = false
+port = 9987
+
+[ldap]
+admin_principal = ceod/admin
+server_url = ldap://ldap-master.csclub.internal
+sasl_realm = CSCLUB.INTERNAL
+users_base = ou=People,dc=csclub,dc=internal
+groups_base = ou=Group,dc=csclub,dc=internal
+sudo_base = ou=SUDOers,dc=csclub,dc=internal
+
+[uwldap]
+server_url = ldap://uwldap.uwaterloo.ca
+base = dc=uwaterloo,dc=ca
+
+[members]
+min_id = 20001
+max_id = 29999
+home = /users
+skel = /users/skel
+
+[clubs]
+min_id = 30001
+max_id = 39999
+home = /users
+skel = /users/skel
+
+[mail]
+smtp_url = smtp://mail.csclub.internal
+smtp_starttls = false
+
+[mailman3]
+api_base_url = http://localhost:8001/3.1
+api_username = restadmin
+api_password = mailman3
+new_member_list = csc-general

ceod/api/app_factory.py

@@ -1,9 +1,12 @@
+import importlib.resources
+import os
 import socket
 
 from flask import Flask
 from flask_kerberos import init_kerberos
 from zope import component
 
+from .error_handlers import register_error_handlers
 from ceo_common.interfaces import IConfig, IKerberosService, ILDAPService, IFileService, \
     IMailmanService, IMailService, IUWLDAPService, IHTTPClient
 from ceo_common.model import Config, HTTPClient, RemoteMailmanService
@@ -15,11 +18,12 @@ def create_app(flask_config={}):
     app = Flask(__name__)
     app.config.from_mapping(flask_config)
 
-    if app.config.get('TESTING') or app.config.get('ENV') == 'development':
-        # TODO: create test config class
-        cfg = Config()
+    if app.config.get('ENV') == 'development' and 'CEOD_CONFIG' not in os.environ:
+        with importlib.resources.path('ceo_common.test', 'ceod_dev.ini') as p:
+            config_file = p.__fspath__()
     else:
-        cfg = Config()
+        config_file = None
+    cfg = Config(config_file)
     component.provideUtility(cfg, IConfig)
 
     init_kerberos(app, service='ceod')
@@ -43,13 +47,15 @@ def create_app(flask_config={}):
     http_client = HTTPClient()
     component.provideUtility(http_client, IHTTPClient)
 
-    # Only instantiate FileService is this host has NFS no_root_squash
-    if hostname == cfg.get('fs_root_host'):
+    # Only instantiate FileService if this host has NFS no_root_squash
+    # If admin_host and fs_root_host become separate, we will need
+    # to create a RemoteFileService
+    if hostname == cfg.get('ceod_fs_root_host'):
         file_srv = FileService()
         component.provideUtility(file_srv, IFileService)
 
     # Only offer mailman API if this host is running Mailman
-    if hostname == cfg.get('mailman_host'):
+    if hostname == cfg.get('ceod_mailman_host'):
         mailman_srv = MailmanService()
         component.provideUtility(mailman_srv, IMailmanService)
 
@@ -68,6 +74,8 @@ def create_app(flask_config={}):
     from ceod.api import uwldap
     app.register_blueprint(uwldap.bp, url_prefix='/api/uwldap')
 
+    register_error_handlers(app)
+
     @app.route('/ping')
     def ping():
         """Health check"""

ceod/api/error_handlers.py

@@ -0,0 +1,23 @@
+import traceback
+
+from flask.app import Flask
+from werkzeug.exceptions import HTTPException
+
+from ceo_common.logger_factory import logger_factory
+
+__all__ = ['register_error_handlers']
+logger = logger_factory(__name__)
+
+
+def register_error_handlers(app: Flask):
+    """Register error handlers for the application."""
+    app.register_error_handler(Exception, generic_error_handler)
+
+
+def generic_error_handler(err: Exception):
+    """Return JSON for internal server errors."""
+    # pass through HTTP errors
+    if isinstance(err, HTTPException):
+        return err
+    logger.error(traceback.format_exc())
+    return {'error': type(err).__name__ + ': ' + str(err)}, 500

ceod/api/mailman.py

@@ -2,7 +2,8 @@ from flask import Blueprint
 from zope import component
 
 from .utils import authz_restrict_to_staff
-from ceo_common.errors import UserAlreadySubscribedError, UserNotSubscribedError
+from ceo_common.errors import UserAlreadySubscribedError, UserNotSubscribedError, \
+    NoSuchListError
 from ceo_common.interfaces import IMailmanService
 
 bp = Blueprint('mailman', __name__)
@@ -16,6 +17,8 @@ def subscribe(mailing_list, username):
         mailman_srv.subscribe(username, mailing_list)
     except UserAlreadySubscribedError as err:
         return {'error': str(err)}, 409
+    except NoSuchListError as err:
+        return {'error': str(err)}, 404
     return {'result': 'OK'}
 
 
@@ -25,6 +28,6 @@ def unsubscribe(mailing_list, username):
     mailman_srv = component.getUtility(IMailmanService)
     try:
         mailman_srv.unsubscribe(username, mailing_list)
-    except UserNotSubscribedError as err:
+    except (UserNotSubscribedError, NoSuchListError) as err:
         return {'error': str(err)}, 404
     return {'result': 'OK'}

ceod/api/members.py

@@ -39,7 +39,8 @@ def get_user(auth_user: str, username: str):
         get_forwarding_addresses = True
     ldap_srv = component.getUtility(ILDAPService)
     try:
-        return ldap_srv.get_user(username).to_dict(get_forwarding_addresses)
+        user = ldap_srv.get_user(username)
+        return user.to_dict(get_forwarding_addresses)
     except UserNotFoundError:
         return {
             'error': 'user not found'

ceod/model/FileService.py

@@ -13,8 +13,8 @@ from ceo_common.interfaces import IFileService, IConfig, IUser
 class FileService:
     def __init__(self):
         cfg = component.getUtility(IConfig)
-        self.member_home_skel = cfg.get('member_home_skel')
-        self.club_home_skel = cfg.get('club_home_skel')
+        self.member_home_skel = cfg.get('members_skel')
+        self.club_home_skel = cfg.get('clubs_skel')
 
     def create_home_dir(self, user: IUser):
         if user.is_club():

ceod/model/LDAPService.py

@@ -8,7 +8,8 @@ import ldap.modlist
 from zope import component
 from zope.interface import implementer
 
-from ceo_common.errors import UserNotFoundError, GroupNotFoundError
+from ceo_common.errors import UserNotFoundError, GroupNotFoundError, \
+    UserAlreadyExistsError, GroupAlreadyExistsError
 from ceo_common.interfaces import ILDAPService, IKerberosService, IConfig, \
     IUser, IGroup, IUWLDAPService
 from .User import User
@@ -25,10 +26,10 @@ class LDAPService:
         self.ldap_server_url = cfg.get('ldap_server_url')
         self.ldap_users_base = cfg.get('ldap_users_base')
         self.ldap_groups_base = cfg.get('ldap_groups_base')
-        self.member_min_id = cfg.get('member_min_id')
-        self.member_max_id = cfg.get('member_max_id')
-        self.club_min_id = cfg.get('club_min_id')
-        self.club_max_id = cfg.get('club_max_id')
+        self.member_min_id = cfg.get('members_min_id')
+        self.member_max_id = cfg.get('members_max_id')
+        self.club_min_id = cfg.get('clubs_min_id')
+        self.club_max_id = cfg.get('clubs_max_id')
 
     def _get_ldap_conn(self, gssapi_bind: bool = True) -> ldap.ldapobject.LDAPObject:
         # TODO: cache the connection
@@ -126,8 +127,10 @@ class LDAPService:
         new_user.gid_number = uid_number
 
         modlist = ldap.modlist.addModlist(new_user.serialize_for_ldap())
-        conn.add_s(new_user.dn, modlist)
-
+        try:
+            conn.add_s(new_user.dn, modlist)
+        except ldap.ALREADY_EXISTS:
+            raise UserAlreadyExistsError()
         return new_user
 
     def modify_user(self, old_user: IUser, new_user: IUser):
@@ -147,7 +150,10 @@ class LDAPService:
         # make sure that the caller initialized the GID number
         assert group.gid_number
         modlist = ldap.modlist.addModlist(group.serialize_for_ldap())
-        conn.add_s(group.dn, modlist)
+        try:
+            conn.add_s(group.dn, modlist)
+        except ldap.ALREADY_EXISTS:
+            raise GroupAlreadyExistsError()
         return group
 
     def modify_group(self, old_group: IGroup, new_group: IGroup):

ceod/model/MailService.py

@@ -17,14 +17,14 @@ smtp_url_re = re.compile(r'^(?P<scheme>smtps?)://(?P<host>[\w.-]+)(:(?P<port>\d+
 class MailService:
     def __init__(self):
         cfg = component.getUtility(IConfig)
-        smtp_url = cfg.get('smtp_url')
+        smtp_url = cfg.get('mail_smtp_url')
         match = smtp_url_re.match(smtp_url)
         if match is None:
             raise Exception('Invalid SMTP URL: %s' % smtp_url)
         self.smtps = match.group('scheme') == 'smtps'
         self.host = match.group('host')
         self.port = int(match.group('port') or 25)
-        self.starttls = cfg.get('smtp_starttls')
+        self.starttls = cfg.get('mail_smtp_starttls')
         assert not (self.smtps and self.starttls)
         self.base_domain = cfg.get('base_domain')
         self.jinja_env = jinja2.Environment(

ceod/model/MailmanService.py

@@ -3,7 +3,8 @@ from requests.auth import HTTPBasicAuth
 from zope import component
 from zope.interface import implementer
 
-from ceo_common.errors import UserAlreadySubscribedError, UserNotSubscribedError
+from ceo_common.errors import UserAlreadySubscribedError, UserNotSubscribedError, \
+    NoSuchListError
 from ceo_common.interfaces import IMailmanService, IConfig
 
 
@@ -13,8 +14,9 @@ class MailmanService:
         cfg = component.getUtility(IConfig)
         self.base_domain = cfg.get('base_domain')
         self.api_base_url = cfg.get('mailman3_api_base_url')
-        self.api_username = cfg.get('mailman3_api_username')
-        self.api_password = cfg.get('mailman3_api_password')
+        api_username = cfg.get('mailman3_api_username')
+        api_password = cfg.get('mailman3_api_password')
+        self.basic_auth = HTTPBasicAuth(api_username, api_password)
 
     def subscribe(self, address: str, mailing_list: str):
         if '@' in mailing_list:
@@ -31,11 +33,15 @@ class MailmanService:
                 'pre_confirmed': 'True',
                 'pre_approved': 'True',
             },
-            auth=HTTPBasicAuth(self.api_username, self.api_password),
+            auth=self.basic_auth,
         )
-        if resp.status_code == 409:
-            raise UserAlreadySubscribedError(resp.json()['description'])
-        resp.raise_for_status()
+        if not resp.ok:
+            desc = resp.json().get('description')
+            if resp.status_code == 409:
+                raise UserAlreadySubscribedError()
+            elif resp.status_code == 400 and desc == 'No such list':
+                raise NoSuchListError()
+            raise Exception(desc)
 
     def unsubscribe(self, address: str, mailing_list: str):
         if '@' not in mailing_list:
@@ -49,8 +55,12 @@ class MailmanService:
                 'pre_approved': 'True',
                 'pre_confirmed': 'True',
             },
-            auth=HTTPBasicAuth(self.api_username, self.api_password),
+            auth=self.basic_auth,
         )
-        if resp.status_code == 404:
-            raise UserNotSubscribedError('user is not subscribed')
-        resp.raise_for_status()
+        if not resp.ok:
+            desc = resp.json().get('description')
+            if resp.status_code == 404:
+                # Unfortunately, a 404 here could mean either the list doesn't
+                # exist, or the member isn't subscribed
+                raise UserNotSubscribedError()
+            raise Exception(desc)

ceod/model/User.py

@@ -41,9 +41,9 @@ class User:
         self.gid_number = gid_number
         if home_directory is None:
             if is_club:
-                home_parent = cfg.get('member_home')
+                home_parent = cfg.get('members_home')
             else:
-                home_parent = cfg.get('club_home')
+                home_parent = cfg.get('clubs_home')
             self.home_directory = os.path.join(home_parent, uid)
         else:
             self.home_directory = home_directory