Use inetOrgPerson instead of account (#29)
continuous-integration/drone/push Build is passing Details

Closes #25.

Co-authored-by: Max Erenberg <>
Reviewed-on: #29
Co-authored-by: Max Erenberg <merenber@csclub.uwaterloo.ca>
Co-committed-by: Max Erenberg <merenber@csclub.uwaterloo.ca>
This commit is contained in:
Max Erenberg 2021-10-23 23:21:09 -04:00
parent e3c50d867a
commit 23f40c74f9
21 changed files with 259 additions and 43 deletions

View File

@ -80,6 +80,8 @@ uniqueMember: uid=ctdalek,ou=People,dc=csclub,dc=internal
dn: uid=ctdalek,ou=People,dc=csclub,dc=internal
cn: Calum Dalek
givenName: Calum
sn: Dalek
userPassword: {SASL}ctdalek@CSCLUB.INTERNAL
loginShell: /bin/bash
homeDirectory: /users/ctdalek
@ -87,7 +89,9 @@ uid: ctdalek
uidNumber: 20001
gidNumber: 20001
objectClass: top
objectClass: account
objectClass: person
objectClass: organizationalPerson
objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: shadowAccount
objectClass: member
@ -103,6 +107,8 @@ gidNumber: 20001
dn: uid=regular1,ou=People,dc=csclub,dc=internal
cn: Regular One
givenName: Regular
sn: One
userPassword: {SASL}regular1@CSCLUB.INTERNAL
loginShell: /bin/bash
homeDirectory: /users/regular1
@ -110,7 +116,9 @@ uid: regular1
uidNumber: 20002
gidNumber: 20002
objectClass: top
objectClass: account
objectClass: person
objectClass: organizationalPerson
objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: shadowAccount
objectClass: member
@ -126,6 +134,8 @@ gidNumber: 20002
dn: uid=exec1,ou=People,dc=csclub,dc=internal
cn: Exec One
givenName: Exec
sn: One
userPassword: {SASL}exec1@CSCLUB.INTERNAL
loginShell: /bin/bash
homeDirectory: /users/exec1
@ -133,7 +143,9 @@ uid: exec1
uidNumber: 20003
gidNumber: 20003
objectClass: top
objectClass: account
objectClass: person
objectClass: organizationalPerson
objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: shadowAccount
objectClass: member

View File

@ -219,6 +219,6 @@ Here's an example of making a request to an endpoint which writes to LDAP:
kinit
# Make the request
curl --negotiate -u : --service-name ceod --delegation always \
-d '{"uid":"test_1","cn":"Test One","program":"Math","terms":["s2021"]}' \
-d '{"uid":"test_1","cn":"Test One","given_name":"Test","sn":"One","program":"Math","terms":["s2021"]}' \
-X POST http://phosphoric-acid:9987/api/members
```

View File

@ -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

View File

@ -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

View File

@ -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:

View File

@ -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')

View File

@ -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,

View File

@ -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:

View File

@ -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'),

View File

@ -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,

View File

@ -52,6 +52,10 @@ paths:
$ref: "#/components/schemas/UID"
cn:
$ref: "#/components/schemas/UserCN"
sn:
$ref: "#/components/schemas/UserSN"
given_name:
$ref: "#/components/schemas/UserGivenName"
program:
$ref: "#/components/schemas/Program"
terms:
@ -76,7 +80,7 @@ paths:
{"status": "in progress", "operation": "send_welcome_message"}
{"status": "in progress", "operation": "subscribe_to_mailing_list"}
{"status": "in progress", "operation": "announce_new_user"}
{"status": "completed", "result": {"cn": "Calum Dalek", "uid": "ctdalek", "uid_number": 20001, "gid_number": 20001, "login_shell": "/bin/bash", "home_directory": "/users/ctdalek", "is_club": false, "program": "MAT/Mathematics Computer Science", "terms": ["f2021"], "forwarding_addresses": ["ctdalek@uwaterloo.ca"], "password": "Wlw1wOTofERTEBlXWzR6/MZL"}}
{"status": "completed", "result": {"cn": "Calum Dalek", "given_name": "Calum", "sn": "Dalek", "uid": "ctdalek", "uid_number": 20001, "gid_number": 20001, "login_shell": "/bin/bash", "home_directory": "/users/ctdalek", "is_club": false, "program": "MAT/Mathematics Computer Science", "terms": ["f2021"], "forwarding_addresses": ["ctdalek@uwaterloo.ca"], "password": "Wlw1wOTofERTEBlXWzR6/MZL"}}
/members/{username}:
get:
tags: ['members']
@ -718,6 +722,14 @@ components:
type: string
description: Full name
example: Calum Dalek
UserSN:
type: string
description: Last name
example: Dalek
UserGivenName:
type: string
description: First name
example: Calum
UID:
type: string
description: Username
@ -753,6 +765,10 @@ components:
properties:
cn:
$ref: "#/components/schemas/UserCN"
sn:
$ref: "#/components/schemas/UserSN"
given_name:
$ref: "#/components/schemas/UserGivenName"
uid:
$ref: "#/components/schemas/UID"
uid_number:
@ -858,4 +874,3 @@ components:
DBConnectionOrPermissionErrorResponse:
<<: *ErrorResponse
description: Unable to connect to database or action failed due to permissions

File diff suppressed because one or more lines are too long

View File

@ -0,0 +1,67 @@
#!/usr/bin/env python3
"""
This is a script which converts each user record in CSC LDAP from the
'account' to the 'inetOrgPerson' objectClass. It pulls first/last name
information from UWLDAP.
GSSAPI is used for LDAP authentication, so make sure to run `kinit` first.
Also, make sure to run this script from the top-level of the git directory
(see the sys.path hack below).
"""
import sys
import traceback
import ldap3
# modify as necessary
LDAP_URI = "ldap://auth1.csclub.uwaterloo.ca"
LDAP_MEMBERS_BASE = "ou=People,dc=csclub,dc=uwaterloo,dc=ca"
UWLDAP_URI = "ldap://auth1.csclub.uwaterloo.ca"
UWLDAP_MEMBERS_BASE = "ou=UWLDAP,dc=csclub,dc=uwaterloo,dc=ca"
csc_conn = ldap3.Connection(
LDAP_URI, authentication=ldap3.SASL, sasl_mechanism=ldap3.KERBEROS,
auto_bind=True, raise_exceptions=True)
uw_conn = ldap3.Connection(UWLDAP_URI, auto_bind=True, raise_exceptions=True)
csc_conn.search(LDAP_MEMBERS_BASE, '(&(objectClass=member)(objectClass=account))',
attributes=ldap3.ALL_ATTRIBUTES)
total_records_updated = 0
for csc_entry in csc_conn.entries:
uid = csc_entry.uid.value
cn = csc_entry.cn.value
sn = None
given_name = None
try:
uw_conn.search(
f'uid={uid},{UWLDAP_MEMBERS_BASE}', '(objectClass=*)',
attributes=ldap3.ALL_ATTRIBUTES, search_scope=ldap3.BASE)
uw_entry = uw_conn.entries[0]
sn = uw_entry.sn.value
given_name = uw_entry.givenName.value
except ldap3.core.exceptions.LDAPNoSuchObjectResult:
pass
if given_name is None or sn is None:
print(f'WARNING: could not retrieve first and last names for {uid}; inferring from whitespace instead')
words = cn.split()
given_name, sn = words[0], words[-1]
old_object_classes = csc_entry.objectClass.values.copy()
old_object_classes.remove('account')
new_object_classes = old_object_classes + [
'person', 'organizationalPerson', 'inetOrgPerson',
]
attrs = csc_entry.entry_attributes_as_dict.copy()
attrs['objectClass'] = new_object_classes
attrs['givenName'] = [given_name]
attrs['sn'] = [sn]
csc_conn.delete(csc_entry.entry_dn)
try:
csc_conn.add(csc_entry.entry_dn, attributes=attrs)
except Exception:
print(traceback.format_exc())
print(f"!!! ERROR !!! We weren't able to create a new record for {uid}.")
print('You need to add the old record back in. Here it is:')
print(csc_entry)
sys.exit(1)
print(f'Created new record for {uid}')
total_records_updated += 1
print(f'Total records updated: {total_records_updated}')

View File

@ -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"

View File

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

View File

@ -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={})

View File

@ -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={})

View File

@ -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'

View File

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

View File

@ -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'],
)

View File

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