From 026189ccd9f8cd881f2b65ca3d103cd308044a30 Mon Sep 17 00:00:00 2001 From: jebus Date: Thu, 24 Jul 2025 09:57:50 +1200 Subject: [PATCH] First Issues / Improvements detected in Beta (#36) Co-authored-by: Tim Lorsbach Reviewed-on: https://git.envipath.com/enviPath/enviPy/pulls/36 --- epdb/logic.py | 51 ++++- epdb/views.py | 232 +++++++++++++++-------- templates/errors/error.html | 4 +- templates/login.html | 46 +++-- templates/login/email_login.html | 14 -- templates/login/email_login_success.html | 5 - templates/objects/pathway.html | 96 +++++++++- utilities/decorators.py | 40 ++++ 8 files changed, 360 insertions(+), 128 deletions(-) delete mode 100644 templates/login/email_login.html delete mode 100644 templates/login/email_login_success.html create mode 100644 utilities/decorators.py diff --git a/epdb/logic.py b/epdb/logic.py index 6d9397ac..62d4f585 100644 --- a/epdb/logic.py +++ b/epdb/logic.py @@ -21,6 +21,7 @@ class UserManager(object): return bool(re.findall(UserManager.user_pattern, url)) @staticmethod + @transaction.atomic def create_user(username, email, password, set_setting=True, add_to_group=True, *args, **kwargs): # avoid circular import :S from .tasks import send_registration_mail @@ -59,19 +60,24 @@ class UserManager(object): def get_user(user_url): pass + @staticmethod + def get_user_by_id(user, user_uuid: str): + if user.uuid != user_uuid and not user.is_superuser: + raise ValueError("Getting user failed!") + return get_user_model().objects.get(uuid=user_uuid) + @staticmethod def get_user_lp(user_url: str): uuid = user_url.strip().split('/')[-1] return get_user_model().objects.get(uuid=uuid) + @staticmethod + def get_users_lp(): + return get_user_model().objects.all() + @staticmethod def get_users(): - return [] - - @staticmethod - def get_user_lp(user_url: str): - uuid = user_url.strip().split('/')[-1] - return get_user_model().objects.get(uuid=uuid) + raise ValueError("") @staticmethod def writable(current_user, user): @@ -102,6 +108,10 @@ class GroupManager(object): uuid = group_url.strip().split('/')[-1] return Group.objects.get(uuid=uuid) + @staticmethod + def get_groups_lp(): + return Group.objects.all() + @staticmethod def get_group_by_url(user, group_url): return GroupManager.get_group_by_id(user, group_url.split('/')[-1]) @@ -173,6 +183,30 @@ class PackageManager(object): return True return False + @staticmethod + def has_package_permission(user: 'User', package: Union[str, 'Package'], permission: str): + + if isinstance(package, str): + package = Package.objects.get(uuid=package) + + groups = GroupManager.get_groups(user) + + perms = { + 'all': ['all'], + 'write': ['all', 'write'], + 'read': ['all', 'write', 'read'] + } + + valid_perms = perms.get(permission) + + if UserPackagePermission.objects.filter(package=package, user=user, permission__in=valid_perms).exists() or \ + GroupPackagePermission.objects.filter(package=package, group__in=groups, + permission__in=valid_perms).exists() or \ + user.is_superuser: + return True + + return False + @staticmethod def get_package_lp(package_url): match = re.findall(PackageManager.package_pattern, package_url) @@ -701,8 +735,9 @@ class SettingManager(object): @staticmethod def get_all_settings(user): - sp = UserSettingPermission.objects.filter(user=user).values('setting').distinct() - return Setting.objects.filter(id__in=sp) + sp = UserSettingPermission.objects.filter(user=user).values('setting') + return (Setting.objects.filter(id__in=sp) | Setting.objects.filter(public=True) | Setting.objects.filter( + global_default=True)).distinct() @staticmethod @transaction.atomic diff --git a/epdb/views.py b/epdb/views.py index 839e27d4..a7f085ce 100644 --- a/epdb/views.py +++ b/epdb/views.py @@ -1,6 +1,5 @@ import json import logging -from functools import wraps from typing import List, Dict, Any from django.conf import settings as s @@ -10,6 +9,7 @@ from django.shortcuts import render, redirect from django.views.decorators.csrf import csrf_exempt from utilities.chem import FormatConverter, IndigoUtils +from utilities.decorators import package_permission_required from .logic import GroupManager, PackageManager, UserManager, SettingManager, SearchManager from .models import Package, GroupPackagePermission, Group, CompoundStructure, Compound, Reaction, Rule, Pathway, Node, \ EPModel, EnviFormer, MLRelativeReasoning, RuleBaseRelativeReasoning, Scenario, SimpleAmbitRule, APIToken, \ @@ -24,6 +24,20 @@ def log_post_params(request): logger.debug(f"{k}\t{v}") +def error(request, message: str, detail: str, code: int = 400): + context = get_base_context(request) + error_context = { + 'error_message': message, + 'error_detail': detail, + } + + if request.headers.get('Accept') == 'application/json': + return JsonResponse(error_context, status=500) + + context.update(**error_context) + return render(request, "errors/error.html", context, status=code) + + def login(request): current_user = _anonymous_or_real(request) context = get_base_context(request) @@ -55,8 +69,11 @@ def login(request): except get_user_model().DoesNotExist: context['message'] = "Login failed!" return render(request, 'login.html', context) - - user = authenticate(username=email, password=password) + try: + user = authenticate(username=email, password=password) + except Exception as e: + context['message'] = "Login failed!" + return render(request, 'login.html', context) if user is not None: login(request, user) @@ -68,15 +85,23 @@ def login(request): elif is_register: username = request.POST.get('username') email = request.POST.get('email') - password = request.POST.get('password') - rpassword = request.POST.get('rpassword') + password = request.POST.get('password', '').strip() + rpassword = request.POST.get('rpassword', '').strip() - if password != rpassword: - pass + if password != rpassword or password == '': + context['message'] = "Registration failed, provided passwords differ!" + return render(request, 'login.html', context) - u = UserManager.create_user(username, email, password) + try: + u = UserManager.create_user(username, email, password) + except Exception: + context['message'] = "Registration failed! Couldn't create User Account." + return render(request, 'login.html', context) - context['message'] = "Your account has been created! An admin will activate it soon!" + if s.ADMIN_APPROVAL_REQUIRED: + context['message'] = "Your account has been created! An admin will activate it soon!" + else: + context['message'] = "Account has been created! You'll receive a mail to activate your account shortly." return render(request, 'login.html', context) @@ -92,25 +117,11 @@ def logout(request): return HttpResponseBadRequest() -def catch_exceptions(view_func): - @wraps(view_func) - def _wrapped_view(request, *args, **kwargs): - try: - return view_func(request, *args, **kwargs) - except Exception as e: - # Optionally return JSON or plain HttpResponse - if request.headers.get('Accept') == 'application/json': - return JsonResponse( - {'error': 'Internal server error. Please try again later.'}, - status=500 - ) - else: - return render(request, 'errors/error.html', get_base_context(request)) - - return _wrapped_view - - def editable(request, user): + + if user.is_superuser: + return True + url = request.build_absolute_uri(request.path) if PackageManager.is_package_url(url): _package = PackageManager.get_package_lp(request.build_absolute_uri()) @@ -129,11 +140,13 @@ def editable(request, user): return False -def get_base_context(request) -> Dict[str, Any]: +def get_base_context(request, for_user=None) -> Dict[str, Any]: current_user = _anonymous_or_real(request) - can_edit = editable(request, current_user) + if for_user: + current_user = for_user + ctx = { 'title': 'enviPath', 'meta': { @@ -183,8 +196,6 @@ def breadcrumbs(first_level_object=None, second_level_namespace=None, second_lev return bread -# @catch_exceptions - def index(request): context = get_base_context(request) context['title'] = 'enviPath - Home' @@ -269,6 +280,9 @@ def compounds(request): default_package = current_user.default_package return package_compounds(request, default_package.uuid) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def rules(request): if request.method == 'GET': @@ -305,6 +319,9 @@ def rules(request): default_package = current_user.default_package return package_rules(request, default_package.uuid) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def reactions(request): if request.method == 'GET': @@ -341,6 +358,9 @@ def reactions(request): default_package = current_user.default_package return package_reactions(request, default_package.uuid) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def pathways(request): if request.method == 'GET': @@ -378,6 +398,9 @@ def pathways(request): default_package = current_user.default_package return package_pathways(request, default_package.uuid) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def scenarios(request): if request.method == 'GET': @@ -408,11 +431,14 @@ def scenarios(request): context['reviewed_objects'] = reviewed_scenario_qs return render(request, 'collections/objects_list.html', context) - # TODO - # elif request.method == 'POST': - # # delegate to default package - # default_package = request.user.default_package - # return package_scenarios(request, default_package.uuid) + + elif request.method == 'POST': + # delegate to default package + default_package = request.user.default_package + return package_scenarios(request, default_package.uuid) + + else: + return HttpResponseNotAllowed(['GET', 'POST']) def models(request): @@ -459,6 +485,9 @@ def models(request): default_package = current_user.default_package return package_models(request, default_package.uuid) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def search(request): current_user = _anonymous_or_real(request) @@ -504,7 +533,11 @@ def search(request): return render(request, 'search.html', context) + else: + return HttpResponseNotAllowed(['GET']) + +@package_permission_required() def package_models(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -587,10 +620,15 @@ def package_models(request, package_uuid): mod = RuleBaseRelativeReasoning() mod.save() - + else: + return error(request, 'Invalid model type.', f'Model type "{model_type}" is not supported."') return redirect(mod.url) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + +@package_permission_required() def package_model(request, package_uuid, model_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -641,9 +679,10 @@ def package_model(request, package_uuid, model_uuid): return HttpResponseBadRequest() else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) +@package_permission_required() def package(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -751,10 +790,10 @@ def package(request, package_uuid): return HttpResponseBadRequest() else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//compound +@package_permission_required() def package_compounds(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -797,8 +836,11 @@ def package_compounds(request, package_uuid): return redirect(c.url) + else: + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//compound/ + +@package_permission_required() def package_compound(request, package_uuid, compound_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -838,11 +880,12 @@ def package_compound(request, package_uuid, compound_uuid): return redirect(current_compound.url) else: return HttpResponseBadRequest() + else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//compound//structure +@package_permission_required() def package_compound_structures(request, package_uuid, compound_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -878,10 +921,10 @@ def package_compound_structures(request, package_uuid, compound_uuid): return redirect(cs.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//compound//structure/ +@package_permission_required() def package_compound_structure(request, package_uuid, compound_uuid, structure_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -899,8 +942,11 @@ def package_compound_structure(request, package_uuid, compound_uuid, structure_u return render(request, 'objects/compound_structure.html', context) + else: + return HttpResponseNotAllowed(['GET', ]) -# https://envipath.org/package//rule + +@package_permission_required() def package_rules(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -965,10 +1011,10 @@ def package_rules(request, package_uuid): return redirect(r.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//rule/ +@package_permission_required() def package_rule(request, package_uuid, rule_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1012,10 +1058,10 @@ def package_rule(request, package_uuid, rule_uuid): return HttpResponseBadRequest() else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//reaction +@package_permission_required() def package_reactions(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1063,10 +1109,10 @@ def package_reactions(request, package_uuid): return redirect(r.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//reaction/ +@package_permission_required() def package_reaction(request, package_uuid, reaction_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1108,10 +1154,10 @@ def package_reaction(request, package_uuid, reaction_uuid): return HttpResponseBadRequest() else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway +@package_permission_required() def package_pathways(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1154,13 +1200,22 @@ def package_pathways(request, package_uuid): pw_mode = request.POST.get('predict', 'predict') smiles = request.POST.get('smiles') - if smiles is None or smiles == '': - return HttpResponseBadRequest() + if smiles is None or smiles.strip() == '': + return error(request, "Pathway prediction failed!", + "Pathway prediction failed due to missing or empty SMILES") - stand_smiles = FormatConverter.standardize(smiles) + smiles = smiles.strip() - if pw_mode not in ['predict', 'build', 'incremental']: - return HttpResponseBadRequest() + try: + stand_smiles = FormatConverter.standardize(smiles) + except ValueError: + return error(request, "Pathway prediction failed!", + f'Pathway prediction failed as standardization of SMILES "{smiles}" failed!') + + modes = ['predict', 'build', 'incremental'] + if pw_mode not in modes: + return error(request, "Pathway prediction failed!", + f'Pathway prediction failed as received mode "{pw_mode}" is none of {modes}') pw = Pathway.create(current_package, stand_smiles, name=name, description=description) # set mode @@ -1185,10 +1240,10 @@ def package_pathways(request, package_uuid): return redirect(pw.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway/ +@package_permission_required() def package_pathway(request, package_uuid, pathway_uuid): current_user: User = _anonymous_or_real(request) current_package: Package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1257,10 +1312,10 @@ def package_pathway(request, package_uuid, pathway_uuid): return HttpResponseBadRequest() else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway//node +@package_permission_required() def package_pathway_nodes(request, package_uuid, pathway_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1313,10 +1368,10 @@ def package_pathway_nodes(request, package_uuid, pathway_uuid): return redirect(current_pathway.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway//node/ +@package_permission_required() def package_pathway_node(request, package_uuid, pathway_uuid, node_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1361,10 +1416,10 @@ def package_pathway_node(request, package_uuid, pathway_uuid, node_uuid): return redirect(current_pathway.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway//edge +@package_permission_required() def package_pathway_edges(request, package_uuid, pathway_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1422,10 +1477,10 @@ def package_pathway_edges(request, package_uuid, pathway_uuid): return redirect(current_pathway.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//pathway//edge/ +@package_permission_required() def package_pathway_edge(request, package_uuid, pathway_uuid, edge_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1440,7 +1495,8 @@ def package_pathway_edge(request, package_uuid, pathway_uuid, edge_uuid): return HttpResponse(svg_data, content_type="image/svg+xml") context = get_base_context(request) - context['title'] = f'enviPath - {current_package.name} - {current_pathway.name} - {current_edge.edge_label.name}' + context[ + 'title'] = f'enviPath - {current_package.name} - {current_pathway.name} - {current_edge.edge_label.name}' context['meta']['current_package'] = current_package context['object_type'] = 'reaction' @@ -1460,10 +1516,10 @@ def package_pathway_edge(request, package_uuid, pathway_uuid, edge_uuid): return redirect(current_pathway.url) else: - return HttpResponseBadRequest() + return HttpResponseNotAllowed(['GET', 'POST']) -# https://envipath.org/package//scenario +@package_permission_required() def package_scenarios(request, package_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1497,8 +1553,11 @@ def package_scenarios(request, package_uuid): return render(request, 'collections/objects_list.html', context) + else: + return HttpResponseNotAllowed(['GET', ]) -# https://envipath.org/package//scenario/ + +@package_permission_required() def package_scenario(request, package_uuid, scenario_uuid): current_user = _anonymous_or_real(request) current_package = PackageManager.get_package_by_id(current_user, package_uuid) @@ -1516,6 +1575,9 @@ def package_scenario(request, package_uuid, scenario_uuid): return render(request, 'objects/scenario.html', context) + else: + return HttpResponseNotAllowed(['GET', ]) + ############## # User/Group # @@ -1536,6 +1598,9 @@ def users(request): return render(request, 'collections/objects_list.html', context) + else: + return HttpResponseNotAllowed(['GET']) + def user(request, user_uuid): current_user = _anonymous_or_real(request) @@ -1546,6 +1611,8 @@ def user(request, user_uuid): if str(current_user.uuid) != user_uuid and not current_user.is_superuser: return HttpResponseBadRequest() + user = UserManager.get_user_by_id(current_user, user_uuid) + context = get_base_context(request) context['title'] = f'enviPath - User' @@ -1556,7 +1623,7 @@ def user(request, user_uuid): {current_user.username: current_user.url} ] - context['user'] = current_user + context['user'] = user model_qs = EPModel.objects.none() for p in PackageManager.get_all_readable_packages(current_user, include_reviewed=True): @@ -1637,6 +1704,9 @@ def user(request, user_uuid): return HttpResponseBadRequest() + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def groups(request): current_user = _anonymous_or_real(request) @@ -1663,6 +1733,9 @@ def groups(request): return redirect(g.url) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def group(request, group_uuid): current_user = _anonymous_or_real(request) @@ -1681,9 +1754,8 @@ def group(request, group_uuid): context['group'] = current_group - # TODO use managers - context['users'] = get_user_model().objects.exclude(id__in=current_group.user_member.all()) - context['groups'] = Group.objects.exclude(id__in=current_group.group_member.all()).exclude(id=current_group.pk) + context['users'] = UserManager.get_users_lp().exclude(id__in=current_group.user_member.all()) + context['groups'] = GroupManager.get_groups_lp().exclude(id__in=current_group.group_member.all()).exclude(id=current_group.pk) context['packages'] = Package.objects.filter( id__in=GroupPackagePermission.objects.filter(group=current_group).values('package').distinct()) @@ -1716,6 +1788,9 @@ def group(request, group_uuid): return redirect(current_group.url) + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def settings(request): current_user = _anonymous_or_real(request) @@ -1775,6 +1850,9 @@ def settings(request): return HttpResponse("Success!") + else: + return HttpResponseNotAllowed(['GET', 'POST']) + def setting(request, setting_uuid): pass diff --git a/templates/errors/error.html b/templates/errors/error.html index 1db057f2..40db5623 100644 --- a/templates/errors/error.html +++ b/templates/errors/error.html @@ -3,10 +3,10 @@ {% block content %} diff --git a/templates/login.html b/templates/login.html index 02a3c11c..067e0367 100644 --- a/templates/login.html +++ b/templates/login.html @@ -1,6 +1,5 @@ {% load static %} - @@ -29,17 +28,6 @@ z-index: -1; } - /* Optional: dim layer */ - .bg-dim { - position: fixed; - top: 0; - left: 0; - width: 100%; - height: 100%; - background-color: rgba(0, 0, 0, 0.4); - z-index: 0; - } - .center-button { position: absolute; top: 50%; @@ -47,6 +35,15 @@ transform: translate(-50%, -50%); z-index: 1; } + + .center-message { + position: absolute; + top: 40%; + left: 50%; + transform: translate(-50%, -50%); + z-index: 1; + } + @@ -55,17 +52,28 @@
-{% if message %} - -{% endif %} -
- +
+
+ {% if message %} + + {% else %} + + {% endif %} +