use inetOrgPerson instead of account

ceo/cli/members.py

@@ -20,7 +20,9 @@ def members():
 
 @members.command(short_help='Add a new member or club rep')
 @click.argument('username')
-@click.option('--cn', help='Full name', prompt='Full name')
+@click.option('--cn', help='Full name', required=False)
+@click.option('--given-name', help='First name', required=False)
+@click.option('--sn', help='Last name', required=False)
 @click.option('--program', required=False, help='Academic program')
 @click.option('--terms', 'num_terms', type=click.IntRange(1, 100),
               help='Number of terms to add', default=1)
@@ -30,19 +32,40 @@ def members():
               help=('Forwarding address to set in ~/.forward. '
                     'Default is UW address. '
                     'Set to the empty string to disable forwarding.'))
-def add(username, cn, program, num_terms, clubrep, forwarding_address):
+def add(username, cn, given_name, sn, program, num_terms, clubrep, forwarding_address):
     cfg = component.getUtility(IConfig)
     uw_domain = cfg.get('uw_domain')
 
-    terms = get_terms_for_new_user(num_terms)
-
-    # TODO: get email address from UWLDAP
+    # Try to get info from UWLDAP
+    resp = http_get('/api/uwldap/' + username)
+    if resp.ok:
+        result = handle_sync_response(resp)
+        if cn is None and result.get('cn'):
+            cn = result['cn']
+        if given_name is None and result.get('given_name'):
+            given_name = result['given_name']
+        if sn is None and result.get('sn'):
+            sn = result['sn']
+        if program is None and result.get('program'):
+            program = result['program']
+        if forwarding_address is None:
+            forwarding_address = result['mail_local_addresses'][0]
+    if cn is None:
+        cn = click.prompt('Full name')
+    if given_name is None:
+        given_name = click.prompt('First name')
+    if sn is None:
+        sn = click.prompt('Last name')
     if forwarding_address is None:
         forwarding_address = username + '@' + uw_domain
 
+    terms = get_terms_for_new_user(num_terms)
+
     body = {
         'uid': username,
         'cn': cn,
+        'given_name': given_name,
+        'sn': sn,
     }
     if program is not None:
         body['program'] = program

ceo/tui/members/AddUserView.py

@@ -25,6 +25,10 @@ class AddUserView(CeoFrame):
         layout.add_widget(self._username)
         self._full_name = Text("Full name:", "cn")
         layout.add_widget(self._full_name)
+        self._first_name = Text("First name:", "given_name")
+        layout.add_widget(self._first_name)
+        self._last_name = Text("Last name:", "sn")
+        layout.add_widget(self._last_name)
         self._program = Text("Program:", "program")
         layout.add_widget(self._program)
         self._forwarding_address = Text("Forwarding address:", "forwarding_address")
@@ -69,8 +73,14 @@ class AddUserView(CeoFrame):
                 return
             data = resp.json()
             self._status_label.text = ''
-            self._full_name.value = data['cn']
-            self._program.value = data.get('program', '')
+            if data.get('cn'):
+                self._full_name.value = data['cn']
+            if data.get('given_name'):
+                self._first_name.value = data['given_name']
+            if data.get('sn'):
+                self._last_name.value = data['sn']
+            if data.get('program'):
+                self._program.value = data.get('program', '')
             if data.get('mail_local_addresses'):
                 self._forwarding_address.value = data['mail_local_addresses'][0]
         finally:
@@ -80,6 +90,8 @@ class AddUserView(CeoFrame):
         body = {
             'uid': self._username.value,
             'cn': self._full_name.value,
+            'given_name': self._first_name.value,
+            'sn': self._last_name.value,
         }
         if self._program.value:
             body['program'] = self._program.value

ceo/utils.py

@@ -92,9 +92,13 @@ def user_dict_kv(d: Dict) -> List[Tuple[str]]:
     """Pretty-format a serialized User as (key, value) pairs."""
     pairs = [
         ('uid', d['uid']),
-        ('cn', d['cn']),
-        ('program', d.get('program', 'Unknown')),
+        ('full name', d['cn']),
     ]
+    if 'given_name' in d:
+        pairs.append(('first name', d['given_name']))
+    if 'sn' in d:
+        pairs.append(('last name', d['sn']))
+    pairs.append(('program', d.get('program', 'Unknown')))
     if 'uid_number' in d:
         pairs.append(('UID number', d['uid_number']))
     if 'gid_number' in d:

ceo_common/interfaces/IUser.py

@@ -8,7 +8,9 @@ class IUser(Interface):
 
     # LDAP attributes
     uid = Attribute('user identifier')
-    cn = Attribute('common name')
+    cn = Attribute('full name')
+    given_name = Attribute('first name')
+    sn = Attribute('last name')
     login_shell = Attribute('login shell')
     uid_number = Attribute('uid number')
     gid_number = Attribute('gid number')

ceod/api/members.py

@@ -24,10 +24,15 @@ def create_user():
     non_member_terms = body.get('non_member_terms')
     if (terms and non_member_terms) or not (terms or non_member_terms):
         raise BadRequest('Must specify either terms or non-member terms')
+    for attr in ['uid', 'cn', 'given_name', 'sn']:
+        if not body.get(attr):
+            raise BadRequest(f"Attribute '{attr}' is missing or empty")
 
     txn = AddMemberTransaction(
         uid=body['uid'],
         cn=body['cn'],
+        given_name=body['given_name'],
+        sn=body['sn'],
         program=body.get('program'),
         terms=terms,
         non_member_terms=non_member_terms,

ceod/model/LDAPService.py

@@ -164,13 +164,19 @@ class LDAPService:
         conn.delete(dn)
 
     def add_user(self, user: IUser):
-        object_classes = ['account', 'posixAccount', 'shadowAccount']
         if user.is_club():
             min_id, max_id = self.club_min_id, self.club_max_id
-            object_classes.append('club')
+            object_classes = [
+                'top', 'account', 'posixAccount', 'shadowAccount', 'club',
+            ]
         else:
+            assert user.given_name and user.sn, \
+                'First name and last name must be specified for new members'
             min_id, max_id = self.member_min_id, self.member_max_id
-            object_classes.append('member')
+            object_classes = [
+                'top', 'person', 'organizationalPerson', 'inetOrgPerson',
+                'posixAccount', 'shadowAccount', 'member',
+            ]
         if user.mail_local_addresses:
             object_classes.append('inetLocalMailRecipient')
         conn = self._get_ldap_conn()
@@ -200,6 +206,8 @@ class LDAPService:
         if user.is_club_rep:
             entry.isClubRep = True
         if not user.is_club():
+            entry.givenName = user.given_name
+            entry.sn = user.sn
             entry.userPassword = '{SASL}%s@%s' % (user.uid, self.ldap_sasl_realm)
 
         try:

ceod/model/User.py

@@ -18,6 +18,8 @@ class User:
         self,
         uid: str,
         cn: str,
+        given_name: Union[str, None] = None,
+        sn: Union[str, None] = None,
         program: Union[str, None] = None,
         terms: Union[List[str], None] = None,
         non_member_terms: Union[List[str], None] = None,
@@ -37,6 +39,8 @@ class User:
 
         self.uid = uid
         self.cn = cn
+        self.given_name = given_name
+        self.sn = sn
         self.program = program
         self.terms = terms or []
         self.non_member_terms = non_member_terms or []
@@ -79,6 +83,9 @@ class User:
             'is_club_rep': self.is_club_rep,
             'program': self.program or 'Unknown',
         }
+        if self.sn and self.given_name:
+            data['sn'] = self.sn
+            data['given_name'] = self.given_name
         if self.terms:
             data['terms'] = self.terms
         if self.non_member_terms:
@@ -133,6 +140,8 @@ class User:
         return User(
             uid=attrs['uid'][0],
             cn=attrs['cn'][0],
+            given_name=attrs.get('givenName', [None])[0],
+            sn=attrs.get('sn', [None])[0],
             program=attrs.get('program', [None])[0],
             terms=attrs.get('term'),
             non_member_terms=attrs.get('nonMemberTerm'),

ceod/transactions/members/AddMemberTransaction.py

@@ -31,7 +31,9 @@ class AddMemberTransaction(AbstractTransaction):
         self,
         uid: str,
         cn: str,
-        program: Union[str, None],
+        given_name: str,
+        sn: str,
+        program: Union[str, None] = None,
         terms: Union[List[str], None] = None,
         non_member_terms: Union[List[str], None] = None,
         forwarding_addresses: Union[List[str], None] = None,
@@ -40,6 +42,8 @@ class AddMemberTransaction(AbstractTransaction):
         cfg = component.getUtility(IConfig)
         self.uid = uid
         self.cn = cn
+        self.given_name = given_name
+        self.sn = sn
         self.program = program
         self.terms = terms
         self.non_member_terms = non_member_terms
@@ -53,6 +57,8 @@ class AddMemberTransaction(AbstractTransaction):
         user = User(
             uid=self.uid,
             cn=self.cn,
+            given_name=self.given_name,
+            sn=self.sn,
             program=self.program,
             terms=self.terms,
             non_member_terms=self.non_member_terms,

tests/ceo/cli/test_members.py

@@ -13,7 +13,9 @@ def test_members_get(cli_setup, ldap_user):
     result = runner.invoke(cli, ['members', 'get', ldap_user.uid])
     expected = (
         f"uid:                  {ldap_user.uid}\n"
-        f"cn:                   {ldap_user.cn}\n"
+        f"full name:            {ldap_user.cn}\n"
+        f"first name:           {ldap_user.given_name}\n"
+        f"last name:            {ldap_user.sn}\n"
         f"program:              {ldap_user.program}\n"
         f"UID number:           {ldap_user.uid_number}\n"
         f"GID number:           {ldap_user.gid_number}\n"
@@ -31,13 +33,15 @@ def test_members_get(cli_setup, ldap_user):
 def test_members_add(cli_setup):
     runner = CliRunner()
     result = runner.invoke(cli, [
-        'members', 'add', 'test_1', '--cn', 'Test One', '--program', 'Math',
-        '--terms', '1',
+        'members', 'add', 'test_1', '--cn', 'Test One', '--given-name', 'Test',
+        '--sn', 'One', '--program', 'Math', '--terms', '1',
     ], input='y\n')
     expected_pat = re.compile((
         "^The following user will be created:\n"
         "uid:                  test_1\n"
-        "cn:                   Test One\n"
+        "full name:            Test One\n"
+        "first name:           Test\n"
+        "last name:            One\n"
         "program:              Math\n"
         "forwarding addresses: test_1@uwaterloo.internal\n"
         "member terms:         [sfw]\\d{4}\n"
@@ -52,7 +56,9 @@ def test_members_add(cli_setup):
         "Announce new user to mailing list... Done\n"
         "Transaction successfully completed.\n"
         "uid:                  test_1\n"
-        "cn:                   Test One\n"
+        "full name:            Test One\n"
+        "first name:           Test\n"
+        "last name:            One\n"
         "program:              Math\n"
         "UID number:           \\d{5}\n"
         "GID number:           \\d{5}\n"

tests/ceo/cli/test_positions.py

@@ -1,14 +1,31 @@
 from click.testing import CliRunner
 from ceo.cli import cli
+from ceod.model import User, Group
 
 
-def test_positions(cli_setup):
+def test_positions(cli_setup, g_admin_ctx):
     runner = CliRunner()
 
     # Setup test data
-    for i in range(5):
-        runner.invoke(cli, ['members', 'add', f'test_{i}', '--cn', f'Test {i}', '--program', 'Math', '--terms', '1'], input='y\n')
-    runner.invoke(cli, ['groups', 'add', 'exec', '--description', 'Test Group'], input='y\n')
+    users = []
+    with g_admin_ctx():
+        for i in range(5):
+            user = User(
+                uid=f'test_{i}',
+                cn=f'Test {i}',
+                given_name='Test',
+                sn=str(i),
+                program='Math',
+                terms=['f2021'],
+            )
+            user.add_to_ldap()
+            users.append(user)
+        group = Group(
+            cn='exec',
+            description='Test Group',
+            gid_number=10500,
+        )
+        group.add_to_ldap()
 
     result = runner.invoke(cli, [
         'positions', 'set',
@@ -50,6 +67,7 @@ webmaster:      test_4
 '''[1:]
 
     # Cleanup test data
-    for i in range(5):
-        runner.invoke(cli, ['members', 'delete', f'test_{i}'], input='y\n')
-    runner.invoke(cli, ['groups', 'delete', 'exec'], input='y\n')
+    with g_admin_ctx():
+        for user in users:
+            user.remove_from_ldap()
+        group.remove_from_ldap()

tests/ceod/api/test_db_mysql.py

@@ -9,7 +9,8 @@ def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user, krb_user):
     uid = ldap_user.uid
 
     with g_admin_ctx():
-        user = User(uid='someone_else', cn='Some Name', terms=['s2021'])
+        user = User(uid='someone_else', cn='Some Name', given_name='Some',
+                    sn='Name', terms=['s2021'])
         user.add_to_ldap()
 
     # user should be able to create db for themselves
@@ -75,7 +76,8 @@ def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user, krb_user):
     uid = ldap_user.uid
 
     with g_admin_ctx():
-        user = User(uid='someone_else', cn='Some Name', terms=['s2021'])
+        user = User(uid='someone_else', cn='Some Name', given_name='Some',
+                    sn='Name', terms=['s2021'])
         user.add_to_ldap()
 
     status, data = client.post(f"/api/db/mysql/{uid}", json={})

tests/ceod/api/test_db_psql.py

@@ -8,7 +8,8 @@ def test_api_create_psql_db(cfg, client, g_admin_ctx, ldap_user, krb_user):
     uid = ldap_user.uid
 
     with g_admin_ctx():
-        user = User(uid='someone_else', cn='Some Name', terms=['s2021'])
+        user = User(uid='someone_else', cn='Some Name', given_name='Some',
+                    sn='Name', terms=['s2021'])
         user.add_to_ldap()
 
     # user should be able to create db for themselves
@@ -77,7 +78,8 @@ def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, ldap_user, krb_user):
     uid = ldap_user.uid
 
     with g_admin_ctx():
-        user = User(uid='someone_else', cn='Some Name', terms=['s2021'])
+        user = User(uid='someone_else', cn='Some Name', given_name='Some',
+                    sn='Name', terms=['s2021'])
         user.add_to_ldap()
 
     status, data = client.post(f"/api/db/postgresql/{uid}", json={})

tests/ceod/api/test_members.py

@@ -17,6 +17,8 @@ def create_user_resp(client, mocks_for_create_user, mock_mail_server):
     status, data = client.post('/api/members', json={
         'uid': 'test_1',
         'cn': 'Test One',
+        'given_name': 'Test',
+        'sn': 'One',
         'program': 'Math',
         'terms': ['s2021'],
         'forwarding_addresses': ['test_1@uwaterloo.internal'],
@@ -50,6 +52,8 @@ def test_api_create_user(cfg, create_user_resp, mock_mail_server):
         {"status": "in progress", "operation": "announce_new_user"},
         {"status": "completed", "result": {
             "cn": "Test One",
+            "given_name": "Test",
+            "sn": "One",
             "uid": "test_1",
             "uid_number": min_uid,
             "gid_number": min_uid,
@@ -77,6 +81,8 @@ def test_api_next_uid(cfg, client, create_user_result):
     _, data = client.post('/api/members', json={
         'uid': 'test_2',
         'cn': 'Test Two',
+        'given_name': 'Test',
+        'sn': 'Two',
         'program': 'Math',
         'terms': ['s2021'],
     })
@@ -205,7 +211,8 @@ def test_api_reset_password(client, create_user_result):
 def test_authz_check(client, create_user_result):
     # non-staff members may not create users
     status, data = client.post('/api/members', json={
-        'uid': 'test_1', 'cn': 'Test One', 'terms': ['s2021'],
+        'uid': 'test_1', 'cn': 'Test One', 'given_name': 'Test',
+        'sn': 'One', 'terms': ['s2021'],
     }, principal='regular1')
     assert status == 403
 
@@ -219,6 +226,7 @@ def test_authz_check(client, create_user_result):
 
     # If we're syscom but we don't pass credentials, the request should fail
     _, data = client.post('/api/members', json={
-        'uid': 'test_1', 'cn': 'Test One', 'terms': ['s2021'],
+        'uid': 'test_1', 'cn': 'Test One', 'given_name': 'Test',
+        'sn': 'One', 'terms': ['s2021'],
     }, principal='ctdalek', delegate=False)
     assert data[-1]['status'] == 'aborted'

tests/ceod/api/test_positions.py

@@ -21,7 +21,8 @@ def test_set_positions(cfg, client, g_admin_ctx, mock_mailman_server):
     users = []
     with g_admin_ctx():
         for uid in ['test_1', 'test_2', 'test_3', 'test_4']:
-            user = User(uid=uid, cn='Some Name', terms=['s2021'])
+            user = User(uid=uid, cn='Some Name', given_name='Some', sn='Name',
+                        terms=['s2021'])
             user.add_to_ldap()
             users.append(user)
         exec_group = Group(cn='exec', gid_number=10013)

tests/ceod/model/test_user.py

@@ -37,12 +37,16 @@ def test_get_display_info_for_users(cfg, ldap_srv):
     user1 = User(
         uid='test_1',
         cn='Test One',
+        given_name='Test',
+        sn='One',
         program='Math',
         terms=['s2021'],
     )
     user2 = User(
         uid='test_2',
         cn='Test Two',
+        given_name='Test',
+        sn='Two',
         program='Science',
         non_member_terms=['s2021'],
     )
@@ -145,6 +149,8 @@ def test_user_to_dict(cfg):
     user = User(
         uid='test_jsmith',
         cn='John Smith',
+        given_name='John',
+        sn='Smith',
         program='Math',
         terms=['s2021'],
         uid_number=21000,
@@ -154,6 +160,8 @@ def test_user_to_dict(cfg):
     expected = {
         'uid': user.uid,
         'cn': user.cn,
+        'given_name': user.given_name,
+        'sn': user.sn,
         'program': user.program,
         'terms': user.terms,
         'uid_number': user.uid_number,
@@ -179,6 +187,8 @@ def test_user_is_club_rep(ldap_user, ldap_srv):
     user = User(
         uid='test_jsmith',
         cn='John Smith',
+        given_name='John',
+        sn='Smith',
         program='Math',
         terms=['s2021'],
     )
@@ -186,6 +196,8 @@ def test_user_is_club_rep(ldap_user, ldap_srv):
     club_rep = User(
         uid='test_jdoe',
         cn='John Doe',
+        given_name='John',
+        sn='Doe',
         program='Math',
         non_member_terms=['s2021'],
     )

tests/conftest.py

@@ -292,6 +292,8 @@ def simple_user():
     return User(
         uid='test_jdoe',
         cn='John Doe',
+        given_name='John',
+        sn='Doe',
         program='Math',
         terms=['s2021'],
     )