diff --git a/dd-sso/admin/src/admin/lib/admin.py b/dd-sso/admin/src/admin/lib/admin.py index a426ccd..30a0eb0 100644 --- a/dd-sso/admin/src/admin/lib/admin.py +++ b/dd-sso/admin/src/admin/lib/admin.py @@ -1553,7 +1553,7 @@ class Admin: internaluser : DDUser = [u for u in self.internal["users"] if u["id"] == user_id][0] cohorts = self.moodle.get_cohorts() for group in mdelete: - cohort = [c for c in cohorts if c["name"] == group[0]] + cohort = [c for c in cohorts if c["name"] == group[0]][0] try: self.moodle.delete_user_in_cohort( internaluser["moodle_id"], cohort["id"] @@ -1599,12 +1599,12 @@ class Admin: try: self.moodle.create_user(email, username, password, first_name, last_name) ev.update_text(str({"name": "Added to moodle", "data": []})) - except UserExists: + except UserExists as ex: log.error(" -->> User already exists") - error = Events(self.app, "User already exists.", str(se), type="error") - except SystemError as se: - log.error("Moodle create user error: " + str(se)) - error = Events(self.app, "Moodle create user error", str(se), type="error") + error = Events(self.app, "User already exists.", str(ex), type="error") + except SystemError as ex: + log.error("Moodle create user error: " + str(ex)) + error = Events(self.app, "Moodle create user error", str(ex), type="error") except: log.error(" -->> Error creating on moodle the user: " + username) print(traceback.format_exc()) @@ -1793,12 +1793,12 @@ class Admin: u["last"], )[0]["id"] ev.increment({"name": "Added to moodle", "data": []}) - except UserExists: + except UserExists as ex: log.error(" -->> User already exists") - error = Events(self.app, "User already exists.", str(se), type="error") - except SystemError as se: - log.error("Moodle create user error: " + str(se)) - error = Events(self.app, "Moodle create user error", str(se), type="error") + error = Events(self.app, "User already exists.", str(ex), type="error") + except SystemError as ex: + log.error("Moodle create user error: " + str(ex)) + error = Events(self.app, "Moodle create user error", str(ex), type="error") except: log.error(" -->> Error creating on moodle the user: " + u["username"]) print(traceback.format_exc()) @@ -1900,7 +1900,7 @@ class Admin: try: self.keycloak.delete_group(group_id) except: - log.error("KEYCLOAK GROUPS: Could no delete group " + group["path"]) + log.error("KEYCLOAK GROUPS: Could no delete group " + group_id) return cohorts = self.moodle.get_cohorts() diff --git a/dd-sso/admin/src/admin/lib/events.py b/dd-sso/admin/src/admin/lib/events.py index da7205d..c1d4605 100644 --- a/dd-sso/admin/src/admin/lib/events.py +++ b/dd-sso/admin/src/admin/lib/events.py @@ -61,9 +61,9 @@ class Events: title : str text : str total : int - table : bool + table : str type : str - def __init__(self, app : "AdminFlaskApp", title : str, text : str="", total : int=0, table : bool=False, type : str="info") -> None: + def __init__(self, app : "AdminFlaskApp", title : str, text : str="", total : int=0, table : str="", type : str="info") -> None: self.app = app # notice, info, success, and error self.eid = str(base64.b64encode(os.urandom(32))[:8]) @@ -195,7 +195,7 @@ class Events: self.app.socketio.emit("reload", json.dumps({}), namespace="/sio", room="admin") sleep(0.0001) - def table(self, event : str, table : bool, data : Dict[str, Any]={}) -> None: + def table(self, event : str, table : str, data : Dict[str, Any]={}) -> None: # refresh, add, delete, update self.app.socketio.emit( "table_" + event, diff --git a/dd-sso/admin/src/admin/lib/moodle.py b/dd-sso/admin/src/admin/lib/moodle.py index 03d798f..972b563 100644 --- a/dd-sso/admin/src/admin/lib/moodle.py +++ b/dd-sso/admin/src/admin/lib/moodle.py @@ -197,9 +197,9 @@ class Moodle: ) return attempts - def get_cohorts(self) -> Any: + def get_cohorts(self) -> List[Dict[str, Any]]: cohorts = self.call("core_cohort_get_cohorts") - return cohorts + return cast(List[Dict[str, Any]], cohorts) def add_system_cohort(self, name : str, description : str ="", visible : bool=True) -> Any: bit_visible = 1 if visible else 0 diff --git a/dd-sso/admin/src/admin/lib/nextcloud.py b/dd-sso/admin/src/admin/lib/nextcloud.py index 80ed9bb..1a28d13 100644 --- a/dd-sso/admin/src/admin/lib/nextcloud.py +++ b/dd-sso/admin/src/admin/lib/nextcloud.py @@ -327,9 +327,11 @@ class Nextcloud: return True if result["ocs"]["meta"]["statuscode"] == 102: raise ProviderItemExists - if result["ocs"]["meta"]["statuscode"] == 104: - self.add_group(group) - # raise ProviderGroupNotExists + # TODO: It is unclear what status code 104 is, it certainly + # shouldn't the group if it doesn't exist + #if result["ocs"]["meta"]["statuscode"] == 104: + # self.add_group(group) + # # raise ProviderGroupNotExists log.error("Get Nextcloud provider user add error: " + str(result)) raise ProviderOpError except: diff --git a/dd-sso/admin/src/admin/lib/nextcloud_exc.py b/dd-sso/admin/src/admin/lib/nextcloud_exc.py index ec1f290..51ff233 100644 --- a/dd-sso/admin/src/admin/lib/nextcloud_exc.py +++ b/dd-sso/admin/src/admin/lib/nextcloud_exc.py @@ -1,5 +1,6 @@ # # Copyright © 2021,2022 IsardVDI S.L. +# Copyright © 2022 Evilham # # This file is part of DD # @@ -46,6 +47,10 @@ class ProviderItemNotExists(Exception): pass +class ProviderUserNotExists(Exception): + pass + + class ProviderGroupNotExists(Exception): pass diff --git a/dd-sso/admin/src/admin/views/ApiViews.py b/dd-sso/admin/src/admin/views/ApiViews.py index 046b740..13481de 100644 --- a/dd-sso/admin/src/admin/views/ApiViews.py +++ b/dd-sso/admin/src/admin/views/ApiViews.py @@ -188,8 +188,8 @@ def setup_api_views(app : "AdminFlaskApp") -> None: if not app.validators["user"].validate(data): raise Error( "bad_request", - "Data validation for user failed: ", - +str(app.validators["user"].errors), + "Data validation for user failed: " + + str(app.validators["user"].errors), traceback.format_exc(), ) @@ -237,7 +237,7 @@ def setup_api_views(app : "AdminFlaskApp") -> None: @app.json_route("/ddapi/username//", methods=["PUT"]) @has_token def ddapi_username(old_user_ddid : str, new_user_did : str) -> OptionalJsonResponse: - user = app.admin.get_user_username(user_ddid) + user = app.admin.get_user_username(old_user_ddid) if not user: raise Error("not_found", "User id not found") # user = app.admin.update_user_username(old_user_ddid,new_user_did) diff --git a/dd-sso/admin/src/admin/views/AppViews.py b/dd-sso/admin/src/admin/views/AppViews.py index ed221e3..5a3dfbc 100644 --- a/dd-sso/admin/src/admin/views/AppViews.py +++ b/dd-sso/admin/src/admin/views/AppViews.py @@ -21,6 +21,7 @@ import concurrent.futures import json import logging as log +from operator import itemgetter import os import re import sys @@ -34,14 +35,15 @@ from uuid import uuid4 from flask import Response, jsonify, redirect, render_template, request, url_for from flask_login import current_user, login_required -from typing import TYPE_CHECKING, cast, Any, Dict, Optional +from typing import TYPE_CHECKING, cast, Any, Callable, Dict, List, Optional, Tuple if TYPE_CHECKING: from admin.flaskapp import AdminFlaskApp from ..lib.helpers import system_group from .decorators import login_or_token, OptionalJsonResponse -threads = {"external": None} +# TODO: this is quirky and non-trivial to manage +threads : Dict[str, threading.Thread] = {} # q = Queue.Queue() from keycloak.exceptions import KeycloakGetError @@ -52,6 +54,39 @@ from ..lib.exceptions import UserExists, UserNotFound from ..lib.legal import get_legal, gen_legal_if_not_exists, new_legal +def run_in_thread( + op : Callable[..., Any], + args : Tuple = tuple(), + err_msg : str = "Something went wrong", + err_code : int = 500, + busy_err_msg : str ="Precondition failed: already operating users" + ) -> OptionalJsonResponse: + if threads.get("external", None) is not None: + if threads["external"].is_alive(): + return ( + json.dumps( + {"msg": busy_err_msg} + ), + 412, + {"Content-Type": "application/json"}, + ) + else: + del threads["external"] + try: + threads["external"] = threading.Thread( + target=op, args=args + ) + # TODO: this probably returns immediately and client gets no real feedback + threads["external"].start() + return json.dumps({}), 200, {"Content-Type": "application/json"} + except: + log.error(traceback.format_exc()) + return ( + json.dumps({"msg": err_msg}), + err_code, + {"Content-Type": "application/json"}, + ) + def setup_app_views(app : "AdminFlaskApp") -> None: dashboard = Dashboard(app) @app.json_route("/sysadmin/api/resync") @@ -109,32 +144,7 @@ def setup_app_views(app : "AdminFlaskApp") -> None: if current_user.role != "admin": return json.dumps({}), 301, {"Content-Type": "application/json"} - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return ( - json.dumps( - {"msg": "Precondition failed: already working with users"} - ), - 412, - {"Content-Type": "application/json"}, - ) - else: - threads["external"] = None - try: - threads["external"] = threading.Thread( - target=app.admin.update_users_from_keycloak, args=() - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} - except: - log.error(traceback.format_exc()) - return ( - json.dumps({"msg": "Add user error."}), - 500, - {"Content-Type": "application/json"}, - ) - - # return json.dumps(app.admin.update_users_from_keycloak()), 200, {'Content-Type': 'application/json'} + return run_in_thread(app.admin.update_users_from_keycloak, err_msg="Add user error.") users = app.admin.get_mix_users() if current_user.role != "admin": @@ -151,80 +161,12 @@ def setup_app_views(app : "AdminFlaskApp") -> None: data = request.get_json(force=True) if request.method == "PUT": if action == "enable": - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return ( - json.dumps( - {"msg": "Precondition failed: already operating users"} - ), - 412, - {"Content-Type": "application/json"}, - ) - else: - threads["external"] = None - try: - threads["external"] = threading.Thread( - target=app.admin.enable_users, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} - except: - log.error(traceback.format_exc()) - return ( - json.dumps({"msg": "Enable users error."}), - 500, - {"Content-Type": "application/json"}, - ) + return run_in_thread(app.admin.enable_users, args=(data,), err_msg="Enable users error.") if action == "disable": - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return ( - json.dumps( - {"msg": "Precondition failed: already operating users"} - ), - 412, - {"Content-Type": "application/json"}, - ) - else: - threads["external"] = None - try: - threads["external"] = threading.Thread( - target=app.admin.disable_users, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} - except: - log.error(traceback.format_exc()) - return ( - json.dumps({"msg": "Disabling users error."}), - 500, - {"Content-Type": "application/json"}, - ) + return run_in_thread(app.admin.disable_users, args=(data,), err_msg="Disabling users error.") if action == "delete": - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return ( - json.dumps( - {"msg": "Precondition failed: already operating users"} - ), - 412, - {"Content-Type": "application/json"}, - ) - else: - threads["external"] = None - try: - threads["external"] = threading.Thread( - target=app.admin.delete_users, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} - except: - log.error(traceback.format_exc()) - return ( - json.dumps({"msg": "Deleting users error."}), - 500, - {"Content-Type": "application/json"}, - ) + return run_in_thread(app.admin.delete_users, args=(data,), err_msg="Deleting users error.") + return json.dumps({}), 405, {"Content-Type": "application/json"} @@ -278,28 +220,7 @@ def setup_app_views(app : "AdminFlaskApp") -> None: data["enabled"] = data.get("enabled", False) in [True, "on"] data["quota"] = data["quota"] if data["quota"] != "false" else False data["groups"] = data["groups"] if data.get("groups", False) else [] - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return ( - json.dumps({"msg": "Precondition failed: already adding users"}), - 412, - {"Content-Type": "application/json"}, - ) - else: - threads["external"] = None - try: - threads["external"] = threading.Thread( - target=app.admin.add_user, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} - except: - log.error(traceback.format_exc()) - return ( - json.dumps({"msg": "Add user error."}), - 500, - {"Content-Type": "application/json"}, - ) + return run_in_thread(app.admin.add_user, args=(data,), err_msg="Add user error") if request.method == "PUT": data = request.get_json(force=True) @@ -331,7 +252,7 @@ def setup_app_views(app : "AdminFlaskApp") -> None: @app.json_route("/api/roles") @login_required def roles() -> OptionalJsonResponse: - sorted_roles = sorted(app.admin.get_roles(), key=lambda k: k["name"]) + sorted_roles = sorted(app.admin.get_roles(), key=itemgetter("name")) if current_user.role != "admin": sorted_roles = [sr for sr in sorted_roles if sr["name"] != "admin"] return json.dumps(sorted_roles), 200, {"Content-Type": "application/json"} @@ -396,37 +317,19 @@ def setup_app_views(app : "AdminFlaskApp") -> None: @app.json_route("/api/external", methods=["POST", "PUT", "GET", "DELETE"]) @login_required def external() -> OptionalJsonResponse: - if "external" in threads.keys(): - if threads["external"] is not None and threads["external"].is_alive(): - return json.dumps({}), 301, {"Content-Type": "application/json"} - else: - threads["external"] = None - if request.method == "POST": data = request.get_json(force=True) if data["format"] == "json-ga": - threads["external"] = threading.Thread( - target=app.admin.upload_json_ga, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} + return run_in_thread(app.admin.upload_json_ga, args=(data,)) if data["format"] == "csv-ug": valid = check_upload_errors(data) if valid["pass"]: - threads["external"] = threading.Thread( - target=app.admin.upload_csv_ug, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} + return run_in_thread(app.admin.upload_csv_ug, args=(data,)) else: return json.dumps(valid), 422, {"Content-Type": "application/json"} if request.method == "PUT": data = request.get_json(force=True) - threads["external"] = threading.Thread( - target=app.admin.sync_external, args=(data,) - ) - threads["external"].start() - return json.dumps({}), 200, {"Content-Type": "application/json"} + return run_in_thread(app.admin.sync_external, args=(data,)) if request.method == "DELETE": print("RESET") app.admin.reset_external()