diff --git a/.drone/auth1-setup.sh b/.drone/auth1-setup.sh index 53f52f2..d79bf69 100755 --- a/.drone/auth1-setup.sh +++ b/.drone/auth1-setup.sh @@ -72,6 +72,7 @@ addprinc -pw krb5 sysadmin/admin addprinc -pw krb5 ctdalek addprinc -pw krb5 exec1 addprinc -pw krb5 regular1 +addprinc -pw krb5 office1 addprinc -randkey host/auth1.csclub.internal addprinc -randkey ldap/auth1.csclub.internal ktadd host/auth1.csclub.internal diff --git a/.drone/common.sh b/.drone/common.sh index 26183fd..b798322 100644 --- a/.drone/common.sh +++ b/.drone/common.sh @@ -6,6 +6,9 @@ sed -E '/^(domain|search)[[:space:]]+csclub.uwaterloo.ca/d' /etc/resolv.conf > / cp /tmp/resolv.conf /etc/resolv.conf rm /tmp/resolv.conf +# normally systemd creates /run/ceod for us +mkdir -p /run/ceod + # mock out systemctl ln -s /bin/true /usr/local/bin/systemctl # mock out acme.sh @@ -40,8 +43,8 @@ sync_with() { port=$2 fi synced=false - # give it 5 minutes - for i in {1..60}; do + # give it 20 minutes (can be slow if you're using e.g. NFS or Ceph) + for i in {1..240}; do if nc -vz $host $port ; then synced=true break diff --git a/.drone/data.ldif b/.drone/data.ldif index 2bab1d4..0cb6012 100644 --- a/.drone/data.ldif +++ b/.drone/data.ldif @@ -61,6 +61,7 @@ objectClass: posixGroup gidNumber: 10003 cn: office uniqueMember: uid=ctdalek,ou=People,dc=csclub,dc=internal +uniqueMember: uid=office1,ou=People,dc=csclub,dc=internal dn: cn=src,ou=Group,dc=csclub,dc=internal objectClass: top @@ -160,3 +161,28 @@ objectClass: posixGroup cn: exec gidNumber: 10013 uniqueMember: uid=exec1,ou=People,dc=csclub,dc=internal + +dn: uid=office1,ou=People,dc=csclub,dc=internal +cn: Office One +givenName: Office +sn: One +userPassword: {SASL}office1@CSCLUB.INTERNAL +loginShell: /bin/bash +homeDirectory: /users/office1 +uid: office1 +uidNumber: 20004 +gidNumber: 20004 +objectClass: top +objectClass: account +objectClass: posixAccount +objectClass: shadowAccount +objectClass: member +program: MAT/Mathematics Computer Science +term: f2021 + +dn: cn=office1,ou=Group,dc=csclub,dc=internal +objectClass: top +objectClass: group +objectClass: posixGroup +cn: office1 +gidNumber: 20004 diff --git a/ceod/api/app_factory.py b/ceod/api/app_factory.py index 98d30eb..84bf7c5 100644 --- a/ceod/api/app_factory.py +++ b/ceod/api/app_factory.py @@ -78,15 +78,10 @@ def register_services(app): cfg = Config(config_file) component.provideUtility(cfg, IConfig) - # KerberosService hostname = socket.gethostname() - fqdn = socket.getfqdn() - # Only ceod_admin_host has the ceod/admin key in its keytab - if hostname == cfg.get('ceod_admin_host'): - principal = cfg.get('ldap_admin_principal') - else: - principal = f'ceod/{fqdn}' - krb_srv = KerberosService(principal) + + # KerberosService + krb_srv = KerberosService() component.provideUtility(krb_srv, IKerberosService) # LDAPService diff --git a/ceod/api/members.py b/ceod/api/members.py index a4f135f..7013782 100644 --- a/ceod/api/members.py +++ b/ceod/api/members.py @@ -1,4 +1,4 @@ -from flask import Blueprint, request, json +from flask import Blueprint, g, json, request from zope import component from .utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ @@ -28,6 +28,10 @@ def create_user(): if not body.get(attr): raise BadRequest(f"Attribute '{attr}' is missing or empty") + # We need to use the admin creds here because office members may not + # directly create new LDAP records. + g.need_admin_creds = True + txn = AddMemberTransaction( uid=body['uid'], cn=body['cn'], @@ -82,6 +86,11 @@ def renew_user(username: str): if (terms and non_member_terms) or not (terms or non_member_terms): raise BadRequest('Must specify either terms or non-member terms') + # We need to use the admin creds here because office members should + # not be able to directly modify the shadowExpire field; this could + # prevent syscom members from logging into the machines. + g.need_admin_creds = True + ldap_srv = component.getUtility(ILDAPService) user = ldap_srv.get_user(username) if body.get('terms'): diff --git a/ceod/model/KerberosService.py b/ceod/model/KerberosService.py index 01ee2d7..0322707 100644 --- a/ceod/model/KerberosService.py +++ b/ceod/model/KerberosService.py @@ -2,6 +2,7 @@ import os import subprocess from typing import List +import gssapi from zope import component from zope.interface import implementer @@ -10,20 +11,53 @@ from ceo_common.interfaces import IKerberosService, IConfig @implementer(IKerberosService) class KerberosService: - def __init__( - self, - admin_principal: str, - ): + def __init__(self): cfg = component.getUtility(IConfig) - self.admin_principal = admin_principal - self.realm = cfg.get('ldap_sasl_realm') - # We don't need a credentials cache because the client forwards - # their credentials to us + # For creating new members and renewing memberships, we use the + # admin credentials + self.admin_principal = cfg.get('ldap_admin_principal') + self.admin_principal_name = gssapi.Name(self.admin_principal) + ccache_file = cfg.get('ldap_admin_principal_ccache') + self.admin_principal_ccache = 'FILE:' + ccache_file + self.admin_principal_store = {'ccache': self.admin_principal_ccache} + # For everything else, the clients forwards (delegates) their + # credentials to us. Set KRB5CCNAME to /dev/null to mitigate the + # risk of the admin creds getting accidentally used instead. os.environ['KRB5CCNAME'] = 'FILE:/dev/null' - def _run(self, args: List[str]): - subprocess.run(args, check=True) + def _run(self, args: List[str], **kwargs): + subprocess.run(args, check=True, **kwargs) + + def _kinit_admin_creds(self): + env = {'KRB5CCNAME': self.admin_principal_ccache} + self._run([ + 'kinit', '-k', '-p', self.admin_principal + ], env=env) + + def _get_admin_creds_token_impl(self) -> bytes: + creds = gssapi.Credentials( + usage='initiate', name=self.admin_principal_name, + store=self.admin_principal_store) + # this will raise a gssapi.raw.exceptions.ExpiredCredentialsError + # if the ticket has expired + creds.inquire() + return creds.export() + + def get_admin_creds_token(self) -> bytes: + """ + Returns a serialized GSSAPI credential which can be used to + authenticate to the CSC LDAP server with administrative privileges. + """ + try: + return self._get_admin_creds_token_impl() + except gssapi.raw.misc.GSSError: + # Either the ccache file does not exist, or the ticket has + # expired. + # Run kinit again. + self._kinit_admin_creds() + # This time should work + return self._get_admin_creds_token_impl() def addprinc(self, principal: str, password: str): self._run([ diff --git a/ceod/model/LDAPService.py b/ceod/model/LDAPService.py index 5d65fc3..24cf814 100644 --- a/ceod/model/LDAPService.py +++ b/ceod/model/LDAPService.py @@ -10,7 +10,7 @@ from zope.interface import implementer from ceo_common.errors import UserNotFoundError, GroupNotFoundError, \ UserAlreadyExistsError, GroupAlreadyExistsError from ceo_common.interfaces import ILDAPService, IConfig, \ - IUser, IGroup, IUWLDAPService + IUser, IGroup, IUWLDAPService, IKerberosService from ceo_common.model import Term import ceo_common.utils as ceo_common_utils from .User import User @@ -31,16 +31,26 @@ class LDAPService: self.club_min_id = cfg.get('clubs_min_id') self.club_max_id = cfg.get('clubs_max_id') + self.krb_srv = component.getUtility(IKerberosService) + def _get_ldap_conn(self) -> ldap3.Connection: if 'ldap_conn' in g: return g.ldap_conn kwargs = {'auto_bind': True, 'raise_exceptions': True} - if 'client_token' in g: + + # Use GSSAPI authentication if creds are available + creds_token = None + if g.get('need_admin_creds', False): + creds_token = self.krb_srv.get_admin_creds_token() + elif 'client_token' in g: + creds_token = g.client_token + if creds_token is not None: kwargs['authentication'] = ldap3.SASL kwargs['sasl_mechanism'] = ldap3.KERBEROS - creds = gssapi.Credentials(token=g.client_token) + creds = gssapi.Credentials(token=creds_token) # see https://github.com/cannatag/ldap3/blob/master/ldap3/protocol/sasl/kerberos.py kwargs['sasl_credentials'] = (None, None, creds) + conn = ldap3.Connection(self.ldap_server_url, **kwargs) # cache the connection for a single request g.ldap_conn = conn diff --git a/etc/ceod.ini b/etc/ceod.ini index 1a738a3..f55f33c 100644 --- a/etc/ceod.ini +++ b/etc/ceod.ini @@ -17,6 +17,7 @@ port = 9987 [ldap] admin_principal = ceod/admin +admin_principal_ccache = /run/ceod/admin_ccache server_url = ldaps://auth1.csclub.uwaterloo.ca sasl_realm = CSCLUB.UWATERLOO.CA users_base = ou=People,dc=csclub,dc=uwaterloo,dc=ca diff --git a/tests/ceo/cli/test_cloud.py b/tests/ceo/cli/test_cloud.py index 4ab3b36..c44fe9c 100644 --- a/tests/ceo/cli/test_cloud.py +++ b/tests/ceo/cli/test_cloud.py @@ -22,7 +22,7 @@ def test_cloud_account_activate(cli_setup, mock_cloud_server, new_user, cfg): assert result.output == expected -def test_cloud_accounts_purge(cli_setup, mock_cloud_server): +def test_cloud_accounts_purge(cli_setup, mock_cloud_server, mock_harbor_server): mock_cloud_server.clear() runner = CliRunner() diff --git a/tests/ceod/api/test_members.py b/tests/ceod/api/test_members.py index ea9f2b5..4006316 100644 --- a/tests/ceod/api/test_members.py +++ b/tests/ceod/api/test_members.py @@ -1,3 +1,6 @@ +import os +import subprocess +import time from unittest.mock import patch import ldap3 @@ -84,7 +87,7 @@ def test_api_create_user(cfg, create_user_resp, mock_mail_server): def test_api_next_uid(cfg, client, create_user_result): min_uid = cfg.get('members_min_id') _, data = client.post('/api/members', json={ - 'uid': 'test_2', + 'uid': 'test2', 'cn': 'Test Two', 'given_name': 'Test', 'sn': 'Two', @@ -97,7 +100,7 @@ def test_api_next_uid(cfg, client, create_user_result): assert result['uid_number'] == min_uid + 1 assert result['gid_number'] == min_uid + 1 finally: - client.delete('/api/members/test_2') + client.delete('/api/members/test2') def test_api_get_user(cfg, client, create_user_result): @@ -321,3 +324,44 @@ def test_expire_syscom_member(client, new_user, syscom_group, g_admin_ctx, ldap_ assert data == [] else: assert data == [uid] + + +def test_office_member(cfg, client): + admin_principal = cfg.get('ldap_admin_principal') + ccache_file = cfg.get('ldap_admin_principal_ccache') + if os.path.isfile(ccache_file): + os.unlink(ccache_file) + body = { + 'uid': 'test3', + 'cn': 'Test Three', + 'given_name': 'Test', + 'sn': 'Three', + 'program': 'Math', + 'terms': ['w2022'], + 'forwarding_addresses': ['test3@uwaterloo.internal'], + } + status, data = client.post('/api/members', json=body, principal='office1') + assert status == 200 and data[-1]['status'] == 'completed' + + # Make sure new admin creds were obtained + assert os.path.isfile(ccache_file) + os.unlink(ccache_file) + + # Obtain new creds which expire + subprocess.run([ + 'kinit', '-k', '-p', admin_principal, '-l', '1' + ], check=True, env={'KRB5CCNAME': 'FILE:' + ccache_file}) + old_mtime = os.stat(ccache_file).st_mtime + # Wait for the ticket to expire + time.sleep(1) + + status, _ = client.post( + '/api/members/test3/renew', + json={'terms': ['s2022']}, principal='office1') + assert status == 200 + + # Make sure new admin creds were obtained + assert os.stat(ccache_file).st_mtime > old_mtime + + status, _ = client.delete('/api/members/test3') + assert status == 200 diff --git a/tests/ceod_dev.ini b/tests/ceod_dev.ini index 86633da..e6941c6 100644 --- a/tests/ceod_dev.ini +++ b/tests/ceod_dev.ini @@ -14,6 +14,7 @@ port = 9987 [ldap] admin_principal = ceod/admin +admin_principal_ccache = /run/ceod/admin_ccache server_url = ldap://auth1.csclub.internal sasl_realm = CSCLUB.INTERNAL users_base = ou=People,dc=csclub,dc=internal diff --git a/tests/ceod_test_local.ini b/tests/ceod_test_local.ini index 0171a4f..e933a92 100644 --- a/tests/ceod_test_local.ini +++ b/tests/ceod_test_local.ini @@ -14,6 +14,7 @@ port = 9988 [ldap] admin_principal = ceod/admin +admin_principal_ccache = /run/ceod/admin_ccache server_url = ldap://auth1.csclub.internal sasl_realm = CSCLUB.INTERNAL users_base = ou=TestPeople,dc=csclub,dc=internal diff --git a/tests/conftest.py b/tests/conftest.py index 549eb67..c6d5f99 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -76,12 +76,11 @@ def krb_srv(cfg): # TODO: create temporary Kerberos database using kdb5_util. # We need to be root to read the keytab assert os.geteuid() == 0 - # this dance again... ugh - if socket.gethostname() == cfg.get('ceod_admin_host'): - principal = 'ceod/admin' - else: - principal = 'ceod/' + socket.getfqdn() - krb = KerberosService(principal) + # Delete the admin creds file, if it exists + ccache_file = cfg.get('ldap_admin_principal_ccache') + if os.path.isfile(ccache_file): + os.unlink(ccache_file) + krb = KerberosService() component.getGlobalSiteManager().registerUtility(krb, IKerberosService) delete_test_princs(krb) diff --git a/tests/utils.py b/tests/utils.py index c0b5f6f..871906a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,8 +1,6 @@ import ceod.utils as ceod_utils import contextlib import os -import grp -import pwd import subprocess from subprocess import DEVNULL import tempfile @@ -60,12 +58,5 @@ def gen_password_mock_ctx(): @contextlib.contextmanager def mocks_for_create_user_ctx(): - with gen_password_mock_ctx(), \ - patch.object(pwd, 'getpwuid') as getpwuid_mock, \ - patch.object(grp, 'getgrgid') as getgrgid_mock: - # Normally, if getpwuid or getgrgid do *not* raise a KeyError, - # then LDAPService will skip that UID. Therefore, by raising a - # KeyError, we are making sure that the UID will *not* be skipped. - getpwuid_mock.side_effect = KeyError() - getgrgid_mock.side_effect = KeyError() + with gen_password_mock_ctx(): yield