From 23f40c74f9c5fe46a434f63ba1ac4fa8d59591c1 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sat, 23 Oct 2021 23:21:09 -0400 Subject: [PATCH] Use inetOrgPerson instead of account (#29) Closes #25. Co-authored-by: Max Erenberg <> Reviewed-on: https://git.csclub.uwaterloo.ca/public/pyceo/pulls/29 Co-authored-by: Max Erenberg Co-committed-by: Max Erenberg --- .drone/data.ldif | 18 ++++- README.md | 2 +- ceo/cli/members.py | 33 +++++++-- ceo/tui/members/AddUserView.py | 16 ++++- ceo/utils.py | 8 ++- ceo_common/interfaces/IUser.py | 4 +- ceod/api/members.py | 5 ++ ceod/model/LDAPService.py | 14 +++- ceod/model/User.py | 9 +++ .../members/AddMemberTransaction.py | 8 ++- docs/openapi.yaml | 19 +++++- docs/redoc-static.html | 10 +-- one_time_scripts/inetorgperson.py | 67 +++++++++++++++++++ tests/ceo/cli/test_members.py | 16 +++-- tests/ceo/cli/test_positions.py | 32 +++++++-- tests/ceod/api/test_db_mysql.py | 6 +- tests/ceod/api/test_db_psql.py | 6 +- tests/ceod/api/test_members.py | 12 +++- tests/ceod/api/test_positions.py | 3 +- tests/ceod/model/test_user.py | 12 ++++ tests/conftest.py | 2 + 21 files changed, 259 insertions(+), 43 deletions(-) create mode 100644 one_time_scripts/inetorgperson.py diff --git a/.drone/data.ldif b/.drone/data.ldif index 2863b81..59f6931 100644 --- a/.drone/data.ldif +++ b/.drone/data.ldif @@ -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 diff --git a/README.md b/README.md index 8a9998b..35fa773 100644 --- a/README.md +++ b/README.md @@ -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 ``` diff --git a/ceo/cli/members.py b/ceo/cli/members.py index 0411463..89bc724 100644 --- a/ceo/cli/members.py +++ b/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 diff --git a/ceo/tui/members/AddUserView.py b/ceo/tui/members/AddUserView.py index 10e81ea..0ba7090 100644 --- a/ceo/tui/members/AddUserView.py +++ b/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 diff --git a/ceo/utils.py b/ceo/utils.py index 44f96dd..b6490a2 100644 --- a/ceo/utils.py +++ b/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: diff --git a/ceo_common/interfaces/IUser.py b/ceo_common/interfaces/IUser.py index 2ebdcb9..5c0c5a2 100644 --- a/ceo_common/interfaces/IUser.py +++ b/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') diff --git a/ceod/api/members.py b/ceod/api/members.py index 1502d88..0025287 100644 --- a/ceod/api/members.py +++ b/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, diff --git a/ceod/model/LDAPService.py b/ceod/model/LDAPService.py index d94e234..5708ce1 100644 --- a/ceod/model/LDAPService.py +++ b/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: diff --git a/ceod/model/User.py b/ceod/model/User.py index d5db444..dcef636 100644 --- a/ceod/model/User.py +++ b/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'), diff --git a/ceod/transactions/members/AddMemberTransaction.py b/ceod/transactions/members/AddMemberTransaction.py index f472807..b2143d2 100644 --- a/ceod/transactions/members/AddMemberTransaction.py +++ b/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, diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 4ba25bd..769224a 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -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 - diff --git a/docs/redoc-static.html b/docs/redoc-static.html index 00ee29e..a038043 100644 --- a/docs/redoc-static.html +++ b/docs/redoc-static.html @@ -467,24 +467,26 @@ curl --negotiate

Create a new user

Creates a new member or club rep. If terms is specified, a member is created; if non_member_terms is specified, a club rep is created.

Authorizations:
GSSAPIAuth
Request Body schema: application/json
uid
string (UID)

Username

cn
string (UserCN)

Full name

+
sn
string (UserSN)

Last name

+
given_name
string (UserGivenName)

First name

program
string (Program)

Academic program

terms
Array of strings (Terms)

Terms for which this user was a member

non_member_terms
Array of strings (NonMemberTerms)

Terms for which this user was a club rep

forwarding_addresses
Array of strings <email> (ForwardingAddresses)

Forwarding addresses in ~/.forward

Responses

Request samples

Content type
application/json
{
  • "uid": "ctdalek",
  • "cn": "Calum Dalek",
  • "program": "MAT/Mathematics Computer Science",
  • "terms": [
    ],
  • "non_member_terms": [
    ],
  • "forwarding_addresses": [
    ]
}

Response samples

Content type
text/plain
{"status": "in progress", "operation": "add_user_to_ldap"}
+

Request samples

Content type
application/json
{
  • "uid": "ctdalek",
  • "cn": "Calum Dalek",
  • "sn": "Dalek",
  • "given_name": "Calum",
  • "program": "MAT/Mathematics Computer Science",
  • "terms": [
    ],
  • "non_member_terms": [
    ],
  • "forwarding_addresses": [
    ]
}

Response samples

Content type
text/plain
{"status": "in progress", "operation": "add_user_to_ldap"}
 {"status": "in progress", "operation": "add_group_to_ldap"}
 {"status": "in progress", "operation": "add_user_to_kerberos"}
 {"status": "in progress", "operation": "create_home_dir"}
 {"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"}}

Get information about a user

Returns information about a member or club rep. The forwarding_addresses field will only be present if the client is an authenticated syscom member.

Authorizations:
GSSAPIAuth
path Parameters
username
required
string

username of the user to return

Responses

Response samples

Content type
application/json
{
  • "cn": "Calum Dalek",
  • "uid": "ctdalek",
  • "uid_number": 20001,
  • "gid_number": 20001,
  • "home_directory": "/users/ctdalek",
  • "is_club": false,
  • "login_shell": "/bin/bash",
  • "program": "MAT/Mathematics Computer Science",
  • "positions": [
    ],
  • "terms": [
    ],
  • "non_member_terms": [
    ],
  • "forwarding_addresses": [
    ]
}

Modify a user

Replace the login shell and/or forwarding addresses of a user

+

Response samples

Content type
application/json
{
  • "cn": "Calum Dalek",
  • "sn": "Dalek",
  • "given_name": "Calum",
  • "uid": "ctdalek",
  • "uid_number": 20001,
  • "gid_number": 20001,
  • "home_directory": "/users/ctdalek",
  • "is_club": false,
  • "login_shell": "/bin/bash",
  • "program": "MAT/Mathematics Computer Science",
  • "positions": [
    ],
  • "terms": [
    ],
  • "non_member_terms": [
    ],
  • "forwarding_addresses": [
    ]
}

Modify a user

Replace the login shell and/or forwarding addresses of a user

Authorizations:
GSSAPIAuth
path Parameters
username
required
string

username of the user to modify

Request Body schema: application/json
login_shell
string (LoginShell)

Login shell

forwarding_addresses
Array of strings <email> (ForwardingAddresses)

Forwarding addresses in ~/.forward

@@ -597,7 +599,7 @@ The JSON request body may be omitted.

{"status": "completed", "result": "OK"}