From 7c67a07200132085f8010826b3da0a81cb2960a3 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Tue, 3 Aug 2021 03:20:11 +0000 Subject: [PATCH] use create_sync_response --- ceod/api/app_factory.py | 3 ++ ceod/api/mailman.py | 8 ++- ceod/api/members.py | 6 +-- ceod/api/utils.py | 52 ++++++------------- ceod/api/uwldap.py | 17 ++++++ ceod/model/UWLDAPRecord.py | 9 +++- ceod/model/UWLDAPService.py | 3 +- .../mailman/SubscribeMemberTransaction.py | 6 +-- .../mailman/UnsubscribeMemberTransaction.py | 6 +-- 9 files changed, 53 insertions(+), 57 deletions(-) create mode 100644 ceod/api/uwldap.py diff --git a/ceod/api/app_factory.py b/ceod/api/app_factory.py index cad835e..9bbe40c 100644 --- a/ceod/api/app_factory.py +++ b/ceod/api/app_factory.py @@ -65,6 +65,9 @@ def create_app(flask_config={}): uwldap_srv = UWLDAPService() component.provideUtility(uwldap_srv, IUWLDAPService) + from ceod.api import uwldap + app.register_blueprint(uwldap.bp, url_prefix='/api/uwldap') + @app.route('/ping') def ping(): """Health check""" diff --git a/ceod/api/mailman.py b/ceod/api/mailman.py index 554c536..313bf2f 100644 --- a/ceod/api/mailman.py +++ b/ceod/api/mailman.py @@ -1,6 +1,6 @@ from flask import Blueprint -from .utils import authz_restrict_to_staff +from .utils import authz_restrict_to_staff, create_sync_response from ceod.transactions.mailman import SubscribeMemberTransaction, UnsubscribeMemberTransaction bp = Blueprint('mailman', __name__) @@ -10,13 +10,11 @@ bp = Blueprint('mailman', __name__) @authz_restrict_to_staff def subscribe(mailing_list, username): txn = SubscribeMemberTransaction(username, mailing_list) - txn.execute() - return {'message': f"{username} successfully subscribed to {mailing_list}"} + return create_sync_response(txn) @bp.route('//', methods=['DELETE']) @authz_restrict_to_staff def unsubscribe(mailing_list, username): txn = UnsubscribeMemberTransaction(username, mailing_list) - txn.execute() - return {'message': f"{username} successfully unsubscribed from {mailing_list}"} + return create_sync_response(txn) diff --git a/ceod/api/members.py b/ceod/api/members.py index ef9d56f..de44f05 100644 --- a/ceod/api/members.py +++ b/ceod/api/members.py @@ -3,7 +3,7 @@ from zope import component from .utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ - create_streaming_response + create_streaming_response, create_sync_response from ceo_common.errors import UserNotFoundError from ceo_common.interfaces import ILDAPService from ceod.transactions.members import ( @@ -71,11 +71,11 @@ def renew_user(username: str): terms=body.get('terms'), non_member_terms=body.get('non_member_terms'), ) - return create_streaming_response(txn) + return create_sync_response(txn) @bp.route('//pwreset', methods=['POST']) @authz_restrict_to_syscom def reset_user_password(username: str): txn = ResetPasswordTransaction(username) - return create_streaming_response(txn) + return create_sync_response(txn) diff --git a/ceod/api/utils.py b/ceod/api/utils.py index e124422..d8cb15a 100644 --- a/ceod/api/utils.py +++ b/ceod/api/utils.py @@ -3,55 +3,18 @@ import grp import json import os import pwd -import socket import traceback from typing import Callable, List from flask import current_app from flask_kerberos import requires_authentication -from zope import component from ceo_common.logger_factory import logger_factory -from ceo_common.interfaces import IConfig from ceod.transactions import AbstractTransaction logger = logger_factory(__name__) -def restrict_host(role: str) -> Callable[[Callable], Callable]: - """ - This is a function which returns a decorator. - It returns a 400 if the client makes a request to an endpoint - which is restricted to a different host. - - :param role: a key in the app's config (e.g. 'ceod_admin_host') - which maps to a specific hostname - - Example: - @app.route('//', methods=['POST']) - @restrict_host('mailman_host') - def subscribe(mailing_list, username): - .... - """ - - hostname = socket.gethostname() - cfg = component.getUtility(IConfig) - desired_hostname = cfg.get(role) - - def identity(f: Callable): - return f - - def error_decorator(f: Callable): - @functools.wraps(f) - def wrapper(*args, **kwargs): - return {'error': f'Wrong host! Use {desired_hostname} instead'}, 400 - return wrapper - - if hostname == desired_hostname: - return identity - return error_decorator - - def requires_authentication_no_realm(f: Callable) -> Callable: """ Like requires_authentication, but strips the realm out of the principal string. @@ -140,3 +103,18 @@ def create_streaming_response(txn: AbstractTransaction): }) + '\n' return current_app.response_class(generate(), mimetype='text/plain') + + +def create_sync_response(txn: AbstractTransaction): + """ + Runs the transaction synchronously and returns a JSON response. + """ + try: + txn.execute() + return {'result': txn.result} + except Exception as err: + logger.warning('Transaction failed:\n' + traceback.format_exc()) + txn.rollback() + return { + 'error': str(err), + }, 500 diff --git a/ceod/api/uwldap.py b/ceod/api/uwldap.py new file mode 100644 index 0000000..39c2e03 --- /dev/null +++ b/ceod/api/uwldap.py @@ -0,0 +1,17 @@ +from flask import Blueprint +from zope import component + +from ceo_common.interfaces import IUWLDAPService + +bp = Blueprint('uwldap', __name__) + + +@bp.route('/') +def get_user(username: str): + uwldap_srv = component.getUtility(IUWLDAPService) + record = uwldap_srv.get(username) + if record is None: + return { + 'error': 'user not found', + }, 404 + return record.to_dict() diff --git a/ceod/model/UWLDAPRecord.py b/ceod/model/UWLDAPRecord.py index e5328d5..90aceda 100644 --- a/ceod/model/UWLDAPRecord.py +++ b/ceod/model/UWLDAPRecord.py @@ -17,7 +17,7 @@ class UWLDAPRecord: self.mail_local_addresses = mail_local_addresses @staticmethod - def deserialize_from_ldap(self, data: Dict[str, List[bytes]]): + def deserialize_from_ldap(data: Dict[str, List[bytes]]): """ Deserializes a dict returned from ldap.search_s() into a UWLDAPRecord. @@ -28,3 +28,10 @@ class UWLDAPRecord: program=data.get('ou', [None])[0], mail_local_addresses=data['mailLocalAddress'], ) + + def to_dict(self): + return { + 'uid': self.uid, + 'program': self.program, + 'mail_local_addresses': self.mail_local_addresses, + } diff --git a/ceod/model/UWLDAPService.py b/ceod/model/UWLDAPService.py index 89d0b65..f1caec4 100644 --- a/ceod/model/UWLDAPService.py +++ b/ceod/model/UWLDAPService.py @@ -20,4 +20,5 @@ class UWLDAPService: results = conn.search_s(self.uwldap_base, ldap.SCOPE_SUBTREE, f'uid={username}') if not results: return None - return UWLDAPRecord.deserialize_from_ldap(results[0]) + _, data = results[0] # discard the dn + return UWLDAPRecord.deserialize_from_ldap(data) diff --git a/ceod/transactions/mailman/SubscribeMemberTransaction.py b/ceod/transactions/mailman/SubscribeMemberTransaction.py index f53dff4..daeb2a3 100644 --- a/ceod/transactions/mailman/SubscribeMemberTransaction.py +++ b/ceod/transactions/mailman/SubscribeMemberTransaction.py @@ -26,8 +26,4 @@ class SubscribeMemberTransaction(AbstractTransaction): self.mailman_srv.subscribe(self.address, self.mailing_list) yield 'subscribe_to_mailing_list' - self.finish('success') - - def rollback(self): - # nothing to do, since there was only one operation - pass + self.finish('OK') diff --git a/ceod/transactions/mailman/UnsubscribeMemberTransaction.py b/ceod/transactions/mailman/UnsubscribeMemberTransaction.py index f954602..5b217e7 100644 --- a/ceod/transactions/mailman/UnsubscribeMemberTransaction.py +++ b/ceod/transactions/mailman/UnsubscribeMemberTransaction.py @@ -26,8 +26,4 @@ class UnsubscribeMemberTransaction(AbstractTransaction): self.mailman_srv.unsubscribe(self.address, self.mailing_list) yield 'unsubscribe_to_mailing_list' - self.finish('success') - - def rollback(self): - # nothing to do, since there was only one operation - pass + self.finish('OK')