From 5e8f1b5ba5104d3503cce98f0a46e9103b6afed7 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 23 Jan 2023 02:26:13 -0500 Subject: [PATCH] Implement TUI support for multiple users in each position (#80) Co-authored-by: Justin Chung <20733699+justin13888@users.noreply.github.com> Co-authored-by: Max Erenberg Reviewed-on: https://git.csclub.uwaterloo.ca/public/pyceo/pulls/80 Co-authored-by: Justin Chung Co-committed-by: Justin Chung --- ceo/cli/positions.py | 15 +++- ceo/tui/controllers/GetPositionsController.py | 4 +- ceo/tui/controllers/SetPositionsController.py | 6 +- ceo/utils.py | 3 + ceod/api/positions.py | 43 ++++++---- .../UpdateMemberPositionsTransaction.py | 20 +++-- docker-compose.yml | 2 + docs/openapi.yaml | 57 +++++++------ docs/redoc-static.html | 27 ++++--- requirements.txt | 2 +- tests/ceo/cli/test_db_mysql.py | 14 +++- tests/ceo/cli/test_members.py | 4 +- tests/ceo/cli/test_positions.py | 80 +++++++++++++++++-- tests/ceod/api/test_positions.py | 53 +++++++----- 14 files changed, 228 insertions(+), 102 deletions(-) diff --git a/ceo/cli/positions.py b/ceo/cli/positions.py index 733e8c0..b302a16 100644 --- a/ceo/cli/positions.py +++ b/ceo/cli/positions.py @@ -16,13 +16,22 @@ def positions(): def get(): resp = http_get('/api/positions') result = handle_sync_response(resp) - print_colon_kv(result.items()) + print_colon_kv([ + (position, ', '.join(usernames)) + for position, usernames in result.items() + ]) @positions.command(short_help='Update positions') def set(**kwargs): - body = {k.replace('_', '-'): v for k, v in kwargs.items()} - print_body = {k: v or '' for k, v in body.items()} + body = { + k.replace('_', '-'): v.replace(' ', '').split(',') if v else None + for k, v in kwargs.items() + } + print_body = { + k: ', '.join(v) if v else '' + for k, v in body.items() + } click.echo('The positions will be updated:') print_colon_kv(print_body.items()) click.confirm('Do you want to continue?', abort=True) diff --git a/ceo/tui/controllers/GetPositionsController.py b/ceo/tui/controllers/GetPositionsController.py index 3c40ec1..9c89410 100644 --- a/ceo/tui/controllers/GetPositionsController.py +++ b/ceo/tui/controllers/GetPositionsController.py @@ -19,8 +19,8 @@ class GetPositionsController(Controller): positions = tui_utils.handle_sync_response(resp, self) except Controller.RequestFailed: return - for pos, username in positions.items(): - self.model.positions[pos] = username + for pos, usernames in positions.items(): + self.model.positions[pos] = ','.join(usernames) def target(): self.view.flash_text.set_text('') diff --git a/ceo/tui/controllers/SetPositionsController.py b/ceo/tui/controllers/SetPositionsController.py index 17f8000..2669e7c 100644 --- a/ceo/tui/controllers/SetPositionsController.py +++ b/ceo/tui/controllers/SetPositionsController.py @@ -17,7 +17,7 @@ class SetPositionsController(Controller): body = {} for pos, field in self.view.position_fields.items(): if field.edit_text != '': - body[pos] = field.edit_text + body[pos] = field.edit_text.replace(' ', '').split(',') model = TransactionModel( UpdateMemberPositionsTransaction.operations, 'POST', '/api/positions', json=body @@ -37,8 +37,8 @@ class SetPositionsController(Controller): positions = tui_utils.handle_sync_response(resp, self) except Controller.RequestFailed: return - for pos, username in positions.items(): - self.model.positions[pos] = username + for pos, usernames in positions.items(): + self.model.positions[pos] = ','.join(usernames) def target(): self.view.flash_text.set_text('') diff --git a/ceo/utils.py b/ceo/utils.py index abc96bb..b053f7d 100644 --- a/ceo/utils.py +++ b/ceo/utils.py @@ -87,6 +87,9 @@ def space_colon_kv(pairs: List[Tuple[str, str]]) -> List[str]: maxlen = max(len(key) for key, val in pairs) for key, val in pairs: if key != '': + if not val: + lines.append(key + ':') + continue prefix = key + ': ' else: # assume this is a continuation from the previous line diff --git a/ceod/api/positions.py b/ceod/api/positions.py index a194565..14bb9c0 100644 --- a/ceod/api/positions.py +++ b/ceod/api/positions.py @@ -2,6 +2,7 @@ from flask import Blueprint, request from zope import component from .utils import authz_restrict_to_syscom, create_streaming_response +from ceo_common.errors import BadRequest from ceo_common.interfaces import ILDAPService, IConfig from ceod.transactions.members import UpdateMemberPositionsTransaction @@ -15,7 +16,9 @@ def get_positions(): positions = {} for user in ldap_srv.get_users_with_positions(): for position in user.positions: - positions[position] = user.uid + if position not in positions: + positions[position] = [] + positions[position].append(user.uid) return positions @@ -29,23 +32,31 @@ def update_positions(): required = cfg.get('positions_required') available = cfg.get('positions_available') - # remove falsy values - body = { - positions: username for positions, username in body.items() - if username - } - - for position in body.keys(): + # remove falsy values and parse multiple users in each position + # Example: "user1,user2, user3" -> ["user1","user2","user3"] + position_to_usernames = {} + for position, usernames in body.items(): + if not usernames: + continue + if type(usernames) is list: + position_to_usernames[position] = usernames + elif type(usernames) is str: + position_to_usernames[position] = usernames.replace(' ', '').split(',') + else: + raise BadRequest('usernames must be a list or comma-separated string') + + # check for duplicates (i.e. one username specified twice in the same list) + for usernames in position_to_usernames.values(): + if len(usernames) != len(set(usernames)): + raise BadRequest('username may only be specified at most once for a position') + + for position in position_to_usernames.keys(): if position not in available: - return { - 'error': f'unknown position: {position}' - }, 400 + raise BadRequest(f'unknown position: {position}') for position in required: - if position not in body: - return { - 'error': f'missing required position: {position}' - }, 400 + if position not in position_to_usernames: + raise BadRequest(f'missing required position: {position}') - txn = UpdateMemberPositionsTransaction(body) + txn = UpdateMemberPositionsTransaction(position_to_usernames) return create_streaming_response(txn) diff --git a/ceod/transactions/members/UpdateMemberPositionsTransaction.py b/ceod/transactions/members/UpdateMemberPositionsTransaction.py index 32ac77e..8221f6a 100644 --- a/ceod/transactions/members/UpdateMemberPositionsTransaction.py +++ b/ceod/transactions/members/UpdateMemberPositionsTransaction.py @@ -1,5 +1,5 @@ from collections import defaultdict -from typing import Dict +from typing import Dict, List from zope import component @@ -20,15 +20,19 @@ class UpdateMemberPositionsTransaction(AbstractTransaction): 'subscribe_to_mailing_lists', ] - def __init__(self, positions_reversed: Dict[str, str]): + def __init__(self, position_to_usernames: Dict[str, List[str]]): # positions_reversed is position -> username super().__init__() self.ldap_srv = component.getUtility(ILDAPService) # Reverse the dict so it's easier to use (username -> positions) self.positions = defaultdict(list) - for position, username in positions_reversed.items(): - self.positions[username].append(position) + for position, usernames in position_to_usernames.items(): + if isinstance(usernames, list): + for username in usernames: + self.positions[username].append(position) + else: + raise TypeError("Username(s) under each position must be a list") # a cached Dict of the Users who need to be modified (username -> User) self.users: Dict[str, IUser] = {} @@ -42,7 +46,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction): mailing_lists = cfg.get('auxiliary mailing lists_exec') # position -> username - new_positions_reversed = {} # For returning result + new_position_to_usernames = {} # For returning result # retrieve User objects and cache them for username in self.positions: @@ -64,7 +68,9 @@ class UpdateMemberPositionsTransaction(AbstractTransaction): self.old_positions[username] = old_positions for position in new_positions: - new_positions_reversed[position] = username + if position not in new_position_to_usernames: + new_position_to_usernames[position] = [] + new_position_to_usernames[position].append(username) yield 'update_positions_ldap' # update exec group in LDAP @@ -97,7 +103,7 @@ class UpdateMemberPositionsTransaction(AbstractTransaction): else: yield 'subscribe_to_mailing_lists' - self.finish(new_positions_reversed) + self.finish(new_position_to_usernames) def rollback(self): if 'update_exec_group_ldap' in self.finished_operations: diff --git a/docker-compose.yml b/docker-compose.yml index 59d246c..4db5345 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,6 +4,8 @@ x-common: &common image: python:3.9-bullseye volumes: - .:$PWD:z + security_opt: + - label:disable environment: FLASK_APP: ceod.api FLASK_ENV: development diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 3bbe59d..c98a8e4 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -61,9 +61,9 @@ paths: program: $ref: "#/components/schemas/Program" terms: - $ref: "#/components/schemas/Terms" + $ref: "#/components/schemas/TermsOrNumTerms" non_member_terms: - $ref: "#/components/schemas/NonMemberTerms" + $ref: "#/components/schemas/TermsOrNumTerms" forwarding_addresses: $ref: "#/components/schemas/ForwardingAddresses" responses: @@ -161,17 +161,11 @@ paths: - type: object properties: terms: - type: array - description: Terms for which this user will be a member - items: - $ref: "#/components/schemas/Term" + $ref: "#/components/schemas/TermsOrNumTerms" - type: object properties: non_member_terms: - type: array - description: Terms for which this user will be a club rep - items: - $ref: "#/components/schemas/Term" + $ref: "#/components/schemas/TermsOrNumTerms" example: {"terms": ["f2021"]} responses: "200": @@ -383,11 +377,14 @@ paths: schema: type: object additionalProperties: - type: string + type: array + description: list of usernames + items: + type: string example: - president: user0 - vice-president: user1 - sysadmin: user2 + president: ["user1"] + vice-president: ["user2", "user3"] + sysadmin: ["user4"] treasurer: post: tags: ['positions'] @@ -404,11 +401,18 @@ paths: schema: type: object additionalProperties: - type: string + oneOf: + - type: string + description: username or comma-separated list of usernames + - type: array + description: list of usernames + items: + type: string example: - president: user0 - vice-president: user1 - sysadmin: user2 + president: user1 + vice-president: user2, user3 + secretary: ["user4", "user5"] + sysadmin: ["user6"] treasurer: responses: "200": @@ -422,7 +426,7 @@ paths: {"status": "in progress", "operation": "update_positions_ldap"} {"status": "in progress", "operation": "update_exec_group_ldap"} {"status": "in progress", "operation": "subscribe_to_mailing_list"} - {"status": "completed", "result": "OK"} + {"status": "completed", "result": {"president": ["user1"],"vice-president": ["user2", "user3"],"secretary": ["user4". "user5"],"sysadmin": ["user6"]}} "400": description: Failed content: @@ -883,14 +887,15 @@ components: example: MAT/Mathematics Computer Science Terms: type: array - description: Terms for which this user was a member - items: - $ref: "#/components/schemas/Term" - NonMemberTerms: - type: array - description: Terms for which this user was a club rep + description: List of terms items: $ref: "#/components/schemas/Term" + TermsOrNumTerms: + oneOf: + - type: integer + description: number of additional terms to add + example: 1 + - $ref: "#/components/schemas/Terms" LoginShell: type: string description: Login shell @@ -939,7 +944,7 @@ components: terms: $ref: "#/components/schemas/Terms" non_member_terms: - $ref: "#/components/schemas/NonMemberTerms" + $ref: "#/components/schemas/Terms" forwarding_addresses: $ref: "#/components/schemas/ForwardingAddresses" groups: diff --git a/docs/redoc-static.html b/docs/redoc-static.html index 23f935e..0aeeea6 100644 --- a/docs/redoc-static.html +++ b/docs/redoc-static.html @@ -1854,13 +1854,14 @@ h1:hover > .sc-crzoAE::before,h2:hover > .iUxAWq::before,.iUxAWq:hover::before{v data-styled.g14[id="sc-crzoAE"]{content:"iUxAWq,"}/*!sc*/ .dvcDrG{height:18px;width:18px;min-width:18px;vertical-align:middle;float:right;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(-90deg);-ms-transform:rotateZ(-90deg);transform:rotateZ(-90deg);}/*!sc*/ .iPqByX{height:1.3em;width:1.3em;min-width:1.3em;vertical-align:middle;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(-90deg);-ms-transform:rotateZ(-90deg);transform:rotateZ(-90deg);}/*!sc*/ +.hGHhhO{height:18px;width:18px;min-width:18px;vertical-align:middle;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(-90deg);-ms-transform:rotateZ(-90deg);transform:rotateZ(-90deg);}/*!sc*/ .dqYXmg{height:1.5em;width:1.5em;min-width:1.5em;vertical-align:middle;float:left;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(-90deg);-ms-transform:rotateZ(-90deg);transform:rotateZ(-90deg);}/*!sc*/ .dqYXmg polygon{fill:#1d8127;}/*!sc*/ .bRmrKA{height:20px;width:20px;min-width:20px;vertical-align:middle;float:right;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(0);-ms-transform:rotateZ(0);transform:rotateZ(0);}/*!sc*/ .bRmrKA polygon{fill:white;}/*!sc*/ .dVWHLw{height:1.5em;width:1.5em;min-width:1.5em;vertical-align:middle;float:left;-webkit-transition:-webkit-transform 0.2s ease-out;-webkit-transition:transform 0.2s ease-out;transition:transform 0.2s ease-out;-webkit-transform:rotateZ(-90deg);-ms-transform:rotateZ(-90deg);transform:rotateZ(-90deg);}/*!sc*/ .dVWHLw polygon{fill:#d41f1c;}/*!sc*/ -data-styled.g15[id="sc-dIsUp"]{content:"dvcDrG,iPqByX,dqYXmg,bRmrKA,dVWHLw,"}/*!sc*/ +data-styled.g15[id="sc-dIsUp"]{content:"dvcDrG,iPqByX,hGHhhO,dqYXmg,bRmrKA,dVWHLw,"}/*!sc*/ .iwcKgn{border-left:1px solid #7c7cbb;box-sizing:border-box;position:relative;padding:10px 10px 10px 0;}/*!sc*/ @media screen and (max-width:50rem){.iwcKgn{display:block;overflow:hidden;}}/*!sc*/ tr:first-of-type > .sc-hBMUJo,tr.last > .iwcKgn{border-left-width:0;background-position:top left;background-repeat:no-repeat;background-size:1px 100%;}/*!sc*/ @@ -2068,6 +2069,11 @@ data-styled.g53[id="sc-cOifOu"]{content:"hlhNtL,"}/*!sc*/ data-styled.g54[id="sc-Arkif"]{content:"jojbRz,"}/*!sc*/ .dSaTNC{margin-top:15px;}/*!sc*/ data-styled.g57[id="sc-jgPyTC"]{content:"dSaTNC,"}/*!sc*/ +.hTttpy button{background-color:transparent;border:0;outline:0;font-size:13px;font-family:Courier,monospace;cursor:pointer;padding:0;color:#333333;}/*!sc*/ +.hTttpy button:focus{font-weight:600;}/*!sc*/ +.hTttpy .sc-dIsUp{height:1.1em;width:1.1em;}/*!sc*/ +.hTttpy .sc-dIsUp polygon{fill:#666;}/*!sc*/ +data-styled.g58[id="sc-gSYDnn"]{content:"hTttpy,"}/*!sc*/ .jWaWWE{vertical-align:middle;font-size:13px;line-height:20px;}/*!sc*/ data-styled.g59[id="sc-laZMeE"]{content:"jWaWWE,"}/*!sc*/ .jrLlAa{color:rgba(102,102,102,0.9);}/*!sc*/ @@ -2259,11 +2265,9 @@ in real time.

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

+
integer or Array of Terms (strings) (TermsOrNumTerms)
integer or Array of Terms (strings) (TermsOrNumTerms)
forwarding_addresses
Array of strings <email> (ForwardingAddresses)

Forwarding addresses in ~/.forward

Responses

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"}
+

Request samples

Content type
application/json
{
  • "uid": "ctdalek",
  • "cn": "Calum Dalek",
  • "sn": "Dalek",
  • "given_name": "Calum",
  • "program": "MAT/Mathematics Computer Science",
  • "terms": 1,
  • "non_member_terms": 1,
  • "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"}
@@ -2286,8 +2290,7 @@ in real time.

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

Renew a user

Add member or non-member terms to a user

Authorizations:
GSSAPIAuth
path Parameters
username
required
string

username of the user to renew

-
Request Body schema: application/json
One of
terms
Array of strings (Term)

Terms for which this user will be a member

-

Responses

Request Body schema: application/json
One of
integer or Array of Terms (strings) (TermsOrNumTerms)

Responses

Request samples

Content type
application/json
{
  • "terms": [
    ]
}

Response samples

Content type
application/json
{
  • "terms_added": [
    ]
}

Reset a user's password

Sets a user's password to a randomly generated string, and returns it. The user will be prompted to set a new password on their next login.

Authorizations:
GSSAPIAuth
path Parameters
username
required
string

username of the user whose password will be reset

@@ -2402,17 +2405,17 @@ If the namespace already exists, the certificate inside the kubeconfig will be r
Authorizations:
GSSAPIAuth

Responses

Response samples

Content type
application/json
{
  • "status": "OK",
  • "kubeconfig": "string"
}

positions

Show current positions

Shows the list of positions and members holding them.

Authorizations:
GSSAPIAuth

Responses

Response samples

Content type
application/json
{
  • "president": "user0",
  • "vice-president": "user1",
  • "sysadmin": "user2",
  • "treasurer": null
}

Update positions

Update members for each positions. Members not specified in the parameters will be removed from the position and unsubscribed from the exec's mailing list. New position holders will be subscribed to the mailing list.

+

Response samples

Content type
application/json
{
  • "president": [
    ],
  • "vice-president": [
    ],
  • "sysadmin": [
    ],
  • "treasurer": null
}

Update positions

Update members for each positions. Members not specified in the parameters will be removed from the position and unsubscribed from the exec's mailing list. New position holders will be subscribed to the mailing list.

Authorizations:
GSSAPIAuth
Request Body schema: application/json

New position holders

-
property name*
additional property
string

Responses

additional property
string or Array of strings

Responses

Request samples

Content type
application/json
{
  • "president": "user0",
  • "vice-president": "user1",
  • "sysadmin": "user2",
  • "treasurer": null
}

Response samples

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

Request samples

Content type
application/json
{
  • "president": "user1",
  • "vice-president": "user2, user3",
  • "secretary": [
    ],
  • "sysadmin": [
    ],
  • "treasurer": null
}

Response samples

Content type
text/plain
{"status": "in progress", "operation": "update_positions_ldap"}
 {"status": "in progress", "operation": "update_exec_group_ldap"}
 {"status": "in progress", "operation": "subscribe_to_mailing_list"}
-{"status": "completed", "result": "OK"}
+{"status": "completed", "result": {"president": ["user1"],"vice-president": ["user2", "user3"],"secretary": ["user4". "user5"],"sysadmin": ["user6"]}}