From 28c1e47c59b073fd97b7d689e4e00cdbc832f91f Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Thu, 1 Feb 2024 11:09:02 +0100 Subject: [PATCH 01/26] fix: f-string in `RelationalBone` introduced in #950 (#1054) follow-up fix for #950 --- src/viur/core/bones/relational.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viur/core/bones/relational.py b/src/viur/core/bones/relational.py index 69eca52a4..8407fa247 100644 --- a/src/viur/core/bones/relational.py +++ b/src/viur/core/bones/relational.py @@ -533,7 +533,7 @@ def delete(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str): :raises Warning: If a referenced entry is missing despite the lock. """ if skel.dbEntity.get(f"{name}_outgoingRelationalLocks"): - for refKey in skel.dbEntity[f"_outgoingRelationalLocks" % name]: + for refKey in skel.dbEntity[f"{name}_outgoingRelationalLocks"]: referencedEntry = db.Get(refKey) if not referencedEntry: logging.warning(f"Programming error detected: Entry {refKey} is gone despite lock") From 40bb6f6a710ce28c8e7fb2f026fc5292b309b3b7 Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Thu, 1 Feb 2024 12:33:49 +0100 Subject: [PATCH 02/26] refactor_ value for `tasks_custom_environment_handler` to new abstract class `CustomEnvironmentHandler` (#946) tuples are still accepted for backward compatibility Resolves #920 --- src/viur/core/config.py | 45 ++++++++++++++++++++++++++++--------- src/viur/core/tasks.py | 50 +++++++++++++++++++++++++++-------------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/src/viur/core/config.py b/src/viur/core/config.py index 8b96bfbda..666211289 100644 --- a/src/viur/core/config.py +++ b/src/viur/core/config.py @@ -14,6 +14,7 @@ from viur.core.email import EmailTransport from viur.core.skeleton import SkeletonInstance from viur.core.module import Module + from viur.core.tasks import CustomEnvironmentHandler # Construct an alias with a generic type to be able to write Multiple[str] # TODO: Backward compatible implementation, refactor when viur-core @@ -683,17 +684,39 @@ class Conf(ConfigType): ] """Priority, in which skeletons are loaded""" - tasks_custom_environment_handler: tuple[t.Callable[[], t.Any], t.Callable[[t.Any], None]] = None - """ - Preserve additional environment in deferred tasks. - - If set, it must be a tuple of two functions (serialize_env, restore_env) - for serializing/restoring environment data. - The `serialize_env` function must not require any parameters and must - return a JSON serializable object with the desired information. - The function `restore_env` will receive this object and should write - the information it contains to the environment of the deferred request. - """ + _tasks_custom_environment_handler: t.Optional["CustomEnvironmentHandler"] = None + + @property + def tasks_custom_environment_handler(self) -> t.Optional["CustomEnvironmentHandler"]: + """ + Preserve additional environment in deferred tasks. + + If set, it must be an instance of CustomEnvironmentHandler + for serializing/restoring environment data. + """ + return self._tasks_custom_environment_handler + + @tasks_custom_environment_handler.setter + def tasks_custom_environment_handler(self, value: "CustomEnvironmentHandler") -> None: + from .tasks import CustomEnvironmentHandler + if isinstance(value, CustomEnvironmentHandler) or value is None: + self._tasks_custom_environment_handler = value + elif isinstance(value, tuple): + if len(value) != 2: + raise ValueError(f"Expected a (serialize_env_func, restore_env_func) pair") + warnings.warn( + f"tuple is deprecated, please provide a CustomEnvironmentHandler object!", + DeprecationWarning, stacklevel=2, + ) + # Construct an CustomEnvironmentHandler class on the fly to be backward compatible + cls = type("ProjectCustomEnvironmentHandler", (CustomEnvironmentHandler,), + # serialize and restore will be bound methods. + # Therefore, consume the self argument with lambda. + {"serialize": lambda self: value[0](), + "restore": lambda self, obj: value[1](obj)}) + self._tasks_custom_environment_handler = cls() + else: + raise ValueError(f"Invalid type {type(value)}. Expected a CustomEnvironmentHandler object.") valid_application_ids: list[str] = [] """Which application-ids we're supposed to run on""" diff --git a/src/viur/core/tasks.py b/src/viur/core/tasks.py index 3090a787d..831e9696d 100644 --- a/src/viur/core/tasks.py +++ b/src/viur/core/tasks.py @@ -1,25 +1,49 @@ +import abc import base64 -import grpc +import json import logging import os -import pytz -import requests import sys import traceback +import typing as t from datetime import datetime, timedelta from functools import wraps -import typing as t +from time import sleep +from typing import TypeVar + +import grpc +import pytz +import requests from google.cloud import tasks_v2 from google.cloud.tasks_v2.services.cloud_tasks.transports import CloudTasksGrpcTransport from google.protobuf import timestamp_pb2 -from time import sleep - -import json from viur.core import current, db, errors, utils from viur.core.config import conf from viur.core.decorators import exposed, skey from viur.core.module import Module +CUSTOM_OBJ = TypeVar("CUSTOM_OBJ") # A JSON serializable object + + +class CustomEnvironmentHandler(abc.ABC): + @abc.abstractmethod + def serialize(self) -> CUSTOM_OBJ: + """Serialize custom environment data + + This function must not require any parameters and must + return a JSON serializable object with the desired information. + """ + ... + + @abc.abstractmethod + def restore(self, obj: CUSTOM_OBJ) -> None: + """Restore custom environment data + + This function will receive the object from :meth:`serialize` and should write + the information it contains to the environment of the deferred request. + """ + ... + # class JsonKeyEncoder(json.JSONEncoder): def preprocessJsonObject(o): @@ -232,11 +256,7 @@ def deferred(self, *args, **kwargs): logging.info(f"""Executing task, transaction {env["transactionMarker"]} did succeed""") if "custom" in env and conf.tasks_custom_environment_handler: # Check if we need to restore additional environmental data - assert isinstance(conf.tasks_custom_environment_handler, tuple) \ - and len(conf.tasks_custom_environment_handler) == 2 \ - and callable(conf.tasks_custom_environment_handler[1]), \ - "Your customEnvironmentHandler must be a tuple of two callable if set!" - conf.tasks_custom_environment_handler[1](env["custom"]) + conf.tasks_custom_environment_handler.restore(env["custom"]) if cmd == "rel": caller = conf.main_app pathlist = [x for x in funcPath.split("/") if x] @@ -579,11 +599,7 @@ def task(): if conf.tasks_custom_environment_handler: # Check if this project relies on additional environmental variables and serialize them too - assert isinstance(conf.tasks_custom_environment_handler, tuple) \ - and len(conf.tasks_custom_environment_handler) == 2 \ - and callable(conf.tasks_custom_environment_handler[0]), \ - "Your customEnvironmentHandler must be a tuple of two callable if set!" - env["custom"] = conf.tasks_custom_environment_handler[0]() + env["custom"] = conf.tasks_custom_environment_handler.serialize() # Create task description task = tasks_v2.Task( From 83cdf42c0065ef94421354b25ef6f45bb0c20159 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Fri, 2 Feb 2024 18:33:02 +0100 Subject: [PATCH 03/26] feat: `UserAuthentication`s with skeleton patch capability (#983) `UserAuthentication` allows for a `@classmethod` to dynamically extend the UserSkel based on the authentication options which are configured. This e.g. only adds the fields for Google login to the UserSkel when `GoogleLogin` is configured, otherwise the fields are simply not there. --------- Co-authored-by: Sven Eberth --- src/viur/core/modules/user.py | 186 +++++++++++++++++++++------------- 1 file changed, 117 insertions(+), 69 deletions(-) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index a17a21267..9e9f46971 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -55,8 +55,8 @@ def __lt__(self, other): class UserSkel(skeleton.Skeleton): - kindName = "user" - # Properties required by google and custom auth + kindName = "user" # this assignment is required, as this Skeleton is defined in viur-core (see #604) + name = EmailBone( descr="E-Mail", required=True, @@ -76,38 +76,6 @@ class UserSkel(skeleton.Skeleton): searchable=True, ) - # Properties required by custom auth - password = PasswordBone( - readOnly=True, - visible=False, - ) - - # Properties required by google auth - uid = StringBone( - descr="Google's UserID", - required=False, - readOnly=True, - unique=UniqueValue(UniqueLockMethod.SameValue, False, "UID already in use"), - ) - - sync = BooleanBone( - descr="Sync user data with OAuth-based services", - defaultValue=True, - params={ - "tooltip": - "If set, user data like firstname and lastname is automatically kept synchronous with the information " - "stored at the OAuth service provider (e.g. Google Login)." - } - ) - - gaeadmin = BooleanBone( - descr="Is GAE Admin", - defaultValue=False, - readOnly=True, - ) - - # Generic properties - roles = SelectBone( descr=i18n.translate("viur.user.bone.roles", defaultText="Roles"), values=conf.user.roles, @@ -146,32 +114,28 @@ class UserSkel(skeleton.Skeleton): readOnly=True, ) - # One-Time Password Verification - otp_serial = StringBone( - descr="OTP serial", - searchable=True, - ) - - otp_secret = CredentialBone( - descr="OTP secret", - ) - - otp_timedrift = NumericBone( - descr="OTP time drift", - readOnly=True, - defaultValue=0, - ) - # Authenticator OTP Apps (like Authy) - otp_app_secret = CredentialBone( - descr="OTP Secret (App-Key)", - - ) - admin_config = JsonBone( # This bone stores settings from the vi descr="Config for the User", visible=False ) + def __new__(cls): + """ + Constructor for the UserSkel-class, with the capability + to dynamically add bones required for the configured + authentication methods. + """ + for provider in conf.main_app.user.authenticationProviders: + assert issubclass(provider, UserPrimaryAuthentication) + provider.patch_user_skel(cls) + + for provider in conf.main_app.user.secondFactorProviders: + assert issubclass(provider, UserSecondFactorAuthentication) + provider.patch_user_skel(cls) + + cls.__boneMap__ = skeleton.MetaBaseSkel.generate_bonemap(cls) + return super().__new__(cls) + @classmethod def toDB(cls, skel, *args, **kwargs): # Roles @@ -212,7 +176,7 @@ def toDB(cls, skel, *args, **kwargs): return super().toDB(skel, *args, **kwargs) -class UserAuthentication(Module): +class UserAuthentication(Module, abc.ABC): def __init__(self, moduleName, modulePath, userModule): super().__init__(moduleName, modulePath) self._user_module = userModule @@ -220,6 +184,15 @@ def __init__(self, moduleName, modulePath, userModule): def can_handle(self, user: db.Entity) -> bool: return True + @classmethod + def patch_user_skel(cls, skel_cls: skeleton.Skeleton) -> skeleton.Skeleton: + """ + Allows for an UserAuthentication to patch the UserSkel + class with additional bones which are required for + the implemented authentication method. + """ + ... + class UserPrimaryAuthentication(UserAuthentication, abc.ABC): """Abstract class for all primary authentication methods.""" @@ -258,6 +231,19 @@ class UserPassword(UserPrimaryAuthentication): def getAuthMethodName(*args, **kwargs): return "X-VIUR-AUTH-User-Password" + @classmethod + def patch_user_skel(cls, skel_cls): + """ + Modifies the UserSkel to be equipped by a PasswordBone. + """ + skel_cls.password = PasswordBone( + readOnly=True, + visible=False, + params={ + "category": "Authentication", + } + ) + class LoginSkel(skeleton.RelSkel): name = EmailBone( descr="E-Mail", @@ -591,6 +577,33 @@ class GoogleAccount(UserPrimaryAuthentication): def getAuthMethodName(*args, **kwargs): return "X-VIUR-AUTH-Google-Account" + @classmethod + def patch_user_skel(cls, skel_cls): + """ + Modifies the UserSkel to be equipped by a bones required by Google Auth + """ + skel_cls.uid = StringBone( + descr="Google UserID", + required=False, + readOnly=True, + unique=UniqueValue(UniqueLockMethod.SameValue, False, "UID already in use"), + params={ + "category": "Authentication", + } + ) + + skel_cls.sync = BooleanBone( + descr="Sync user data with OAuth-based services", + defaultValue=True, + params={ + "category": "Authentication", + "tooltip": + "If set, user data like firstname and lastname is automatically kept" + "synchronous with the information stored at the OAuth service provider" + "(e.g. Google Login)." + } + ) + @exposed @force_ssl @skey(allow_empty=True) @@ -643,7 +656,6 @@ def login(self, token: str | None = None, *args, **kwargs): # Take user information from Google, if wanted! if userSkel["sync"]: - for target, source in { "name": email, "firstname": userInfo.get("given_name"), @@ -655,15 +667,6 @@ def login(self, token: str | None = None, *args, **kwargs): update = True if update: - # TODO: Get access from IAM or similar - # if users.is_current_user_admin(): - # if not userSkel["access"]: - # userSkel["access"] = [] - # if not "root" in userSkel["access"]: - # userSkel["access"].append("root") - # userSkel["gaeadmin"] = True - # else: - # userSkel["gaeadmin"] = False assert userSkel.toDB() return self._user_module.continueAuthenticationFlow(self, userSkel["key"]) @@ -727,6 +730,38 @@ class OtpSkel(skeleton.RelSkel): def get2FactorMethodName(*args, **kwargs): # fixme: What is the purpose of this function? Why not just a member? return "X-VIUR-2FACTOR-TimeBasedOTP" + @classmethod + def patch_user_skel(cls, skel_cls): + """ + Modifies the UserSkel to be equipped by a bones required by Timebased OTP + """ + # One-Time Password Verification + skel_cls.otp_serial = StringBone( + descr="OTP serial", + searchable=True, + params={ + "category": "Second Factor Authentication", + } + ) + + skel_cls.otp_secret = CredentialBone( + descr="OTP secret", + params={ + "category": "Second Factor Authentication", + } + ) + + skel.otp_timedrift = NumericBone( + descr="OTP time drift", + readOnly=True, + defaultValue=0, + params={ + "category": "Second Factor Authentication", + } + ) + + return skel + def get_config(self, user: db.Entity) -> OtpConfig | None: """ Returns an instance of self.OtpConfig with a provided token configuration, @@ -973,6 +1008,19 @@ def can_handle(self, user: db.Entity) -> bool: def get2FactorMethodName(*args, **kwargs) -> str: return "X-VIUR-2FACTOR-AuthenticatorOTP" + @classmethod + def patch_user_skel(cls, skel_cls): + """ + Modifies the UserSkel to be equipped by bones required by Authenticator App + """ + # Authenticator OTP Apps (like Authy) + skel_cls.otp_app_secret = CredentialBone( + descr="OTP Secret (App-Key)", + params={ + "category": "Second Factor Authentication", + } + ) + @classmethod def set_otp_app_secret(cls, otp_app_secret=None): """ @@ -1158,7 +1206,7 @@ class User(List): def __init__(self, moduleName, modulePath): for provider in self.authenticationProviders: - assert issubclass(provider, UserAuthentication) + assert issubclass(provider, UserPrimaryAuthentication) name = f"auth_{provider.__name__.lower()}" setattr(self, name, provider(name, f"{modulePath}/{name}", self)) @@ -1179,7 +1227,7 @@ def get_role_defaults(self, role: str) -> set[str]: return set() def addSkel(self): - skel = super(User, self).addSkel().clone() + skel = super().addSkel().clone() user = current.user.get() if not (user and user["access"] and (f"{self.moduleName}-add" in user["access"] or "root" in user["access"])): skel.status.readOnly = True @@ -1205,7 +1253,7 @@ def addSkel(self): return skel def editSkel(self, *args, **kwargs): - skel = super(User, self).editSkel().clone() + skel = super().editSkel().clone() if "password" in skel: skel.password.required = False From 5e351561dfd3ed12891f62b05d6449f086018430 Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Mon, 5 Feb 2024 21:15:20 +0100 Subject: [PATCH 04/26] fix: patch_user_skel (fix for #983) (#1060) Fix for #983 --------- Co-authored-by: Jan Max Meyer --- src/viur/core/modules/user.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 9e9f46971..3d5d681aa 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -185,7 +185,7 @@ def can_handle(self, user: db.Entity) -> bool: return True @classmethod - def patch_user_skel(cls, skel_cls: skeleton.Skeleton) -> skeleton.Skeleton: + def patch_user_skel(cls, skel_cls: skeleton.Skeleton): """ Allows for an UserAuthentication to patch the UserSkel class with additional bones which are required for @@ -751,7 +751,7 @@ def patch_user_skel(cls, skel_cls): } ) - skel.otp_timedrift = NumericBone( + skel_cls.otp_timedrift = NumericBone( descr="OTP time drift", readOnly=True, defaultValue=0, @@ -760,8 +760,6 @@ def patch_user_skel(cls, skel_cls): } ) - return skel - def get_config(self, user: db.Entity) -> OtpConfig | None: """ Returns an instance of self.OtpConfig with a provided token configuration, From b754a492b30aa0c22d81ea5b9469bf532559df21 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Thu, 8 Feb 2024 10:34:53 +0100 Subject: [PATCH 05/26] feat: Improve `UserBone` to additional defaults (#1055) Also includes refactoring of old/unused stuff. Removed docstring content which is not bone-specific. --- src/viur/core/bones/user.py | 46 +++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/viur/core/bones/user.py b/src/viur/core/bones/user.py index 18ced9441..0df29eb10 100644 --- a/src/viur/core/bones/user.py +++ b/src/viur/core/bones/user.py @@ -1,3 +1,4 @@ +import typing as t from viur.core import current from viur.core.bones.relational import RelationalBone @@ -5,19 +6,31 @@ class UserBone(RelationalBone): """ A specialized relational bone for handling user references. Extends the functionality of - :class:`viur.core.bones.relational.RelationalBone` to include support for creation and update magic. - - :param bool creationMagic: If True, the bone will automatically store the creating user when a new entry is added. - :param bool updateMagic: If True, the bone will automatically store the last user who updated the entry. - :param bool visible: If True, the bone will be visible in the skeleton. - :param bool readOnly: If True, the bone will be read-only and its value cannot be changed through user input. - :param dict kwargs: Additional keyword arguments passed to the parent class constructor. - :raises ValueError: If the bone has multiple set to True and a creation/update magic is set. + :class:`viur.core.bones.relational.RelationalBone` to include support for creation and update magic, + and comes with a predefined descr, format, kind and refKeys setting. """ - kind = "user" - datafields = ["name"] - def __init__(self, *, creationMagic=False, updateMagic=False, visible=None, readOnly=False, **kwargs): + def __init__( + self, + *, + creationMagic: bool = False, + descr: str = "User", + format: str = "$(dest.lastname), $(dest.firstname) ($(dest.name))", + kind: str = "user", + readOnly: bool = False, + refKeys: t.Iterable[str] = ("key", "name", "firstname", "lastname"), + updateMagic: bool = False, + visible: t.Optional[bool] = None, + **kwargs + ): + """ + Initializes a new UserBone. + + :param creationMagic: If True, the bone will automatically store the creating user when a new entry is added. + :param updateMagic: If True, the bone will automatically store the last user who updated the entry. + + :raises ValueError: If the bone is multiple=True and creation/update magic is set. + """ if creationMagic or updateMagic: readOnly = False if visible is None: @@ -25,7 +38,15 @@ def __init__(self, *, creationMagic=False, updateMagic=False, visible=None, read elif visible is None: visible = True - super().__init__(visible=visible, readOnly=readOnly, **kwargs) + super().__init__( + kind=kind, + descr=descr, + format=format, + refKeys=refKeys, + visible=visible, + readOnly=readOnly, + **kwargs + ) self.creationMagic = creationMagic self.updateMagic = updateMagic @@ -51,5 +72,6 @@ def performMagic(self, skel, key, isAdd, *args, **kwargs): if self.updateMagic or (self.creationMagic and isAdd): if user := current.user.get(): return self.setBoneValue(skel, key, user["key"], False) + skel[key] = None return True From f32984a659a892c82ec84b3dfc08c61b39f0194b Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Fri, 9 Feb 2024 20:06:28 +0100 Subject: [PATCH 06/26] refactor: `RelationalBone`s `refKeys` and `parentKeys` as set (#1058) Tested. Might collide with #1022. --- src/viur/core/bones/file.py | 4 +--- src/viur/core/bones/relational.py | 40 +++++++++++++------------------ 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/viur/core/bones/file.py b/src/viur/core/bones/file.py index b6298dbc6..73057b02a 100644 --- a/src/viur/core/bones/file.py +++ b/src/viur/core/bones/file.py @@ -172,9 +172,7 @@ def __init__( """ super().__init__(**kwargs) - if "dlkey" not in self.refKeys: - self.refKeys.append("dlkey") - + self.refKeys.add("dlkey") self.derive = derive self.validMimeTypes = validMimeTypes self.maxFileSize = maxFileSize diff --git a/src/viur/core/bones/relational.py b/src/viur/core/bones/relational.py index 8407fa247..1800e5e1f 100644 --- a/src/viur/core/bones/relational.py +++ b/src/viur/core/bones/relational.py @@ -147,16 +147,6 @@ class RelationalBone(BaseBone): relational updates this will cascade. If Entity A references B with CascadeDeletion set, and B references C also with CascadeDeletion; if C gets deleted, both B and A will be deleted as well. - """ - refKeys = ["key", "name"] # todo: turn into a tuple, as it should not be mutable. - """ - A list of properties to include from the referenced property. These properties will be available in the template - without having to fetch the referenced property. Filtering is also only possible by properties named here! - """ - parentKeys = ["key", "name"] # todo: turn into a tuple, as it should not be mutable. - """ - A list of properties from the current skeleton to include. If mixing filtering by relational properties and - properties of the class itself, these must be named here. """ type = "relational" kind = None @@ -168,8 +158,8 @@ def __init__( format: str = "$(dest.name)", kind: str = None, module: t.Optional[str] = None, - parentKeys: t.Optional[list[str]] = None, - refKeys: t.Optional[list[str]] = None, + parentKeys: t.Optional[t.Iterable[str]] = {"name"}, + refKeys: t.Optional[t.Iterable[str]] = {"name"}, updateLevel: RelationalUpdateLevel = RelationalUpdateLevel.Always, using: t.Optional['viur.core.skeleton.RelSkel'] = None, **kwargs @@ -183,11 +173,11 @@ def __init__( Name of the module which should be used to select entities of kind "type". If not set, the value of "type" will be used (the kindName must match the moduleName) :param refKeys: - A list of properties to include from the referenced property. These properties will be - available in the template without having to fetch the referenced property. Filtering is also only possible - by properties named here! + An iterable of properties to include from the referenced property. These properties will be + available in the template without having to fetch the referenced property. Filtering is also only + possible by properties named here! :param parentKeys: - A list of properties from the current skeleton to include. If mixing filtering by + An iterable of properties from the current skeleton to include. If mixing filtering by relational properties and properties of the class itself, these must be named here. :param multiple: If True, allow referencing multiple Elements of the given class. (Eg. n:n-relation). @@ -265,17 +255,19 @@ def __init__( if self.kind is None or self.module is None: raise NotImplementedError("Type and Module of RelationalBone must not be None") + # Referenced keys + self.refKeys = {"key"} if refKeys: - if not "key" in refKeys: - refKeys.append("key") - self.refKeys = refKeys + self.refKeys |= set(refKeys) + # Parent keys + self.parentKeys = {"key"} if parentKeys: - if not "key" in parentKeys: - parentKeys.append("key") - self.parentKeys = parentKeys + self.parentKeys |= set(parentKeys) self.using = using + + # FIXME: Remove in VIUR4!! if isinstance(updateLevel, int): msg = f"parameter updateLevel={updateLevel} in RelationalBone is deprecated. " \ f"Please use the RelationalUpdateLevel enum instead" @@ -600,7 +592,7 @@ def postSavedHandler(self, skel, boneName, key): dbObj["viur_delayed_update_tag"] = time() dbObj["viur_relational_updateLevel"] = self.updateLevel.value dbObj["viur_relational_consistency"] = self.consistency.value - dbObj["viur_foreign_keys"] = self.refKeys + dbObj["viur_foreign_keys"] = list(self.refKeys) dbObj["viurTags"] = srcEntity.get("viurTags") # Copy tags over so we can still use our searchengine db.Put(dbObj) values.remove(data) @@ -619,7 +611,7 @@ def postSavedHandler(self, skel, boneName, key): dbObj["viur_dest_kind"] = self.kind dbObj["viur_relational_updateLevel"] = self.updateLevel.value dbObj["viur_relational_consistency"] = self.consistency.value - dbObj["viur_foreign_keys"] = self.refKeys + dbObj["viur_foreign_keys"] = list(self.refKeys) db.Put(dbObj) def postDeletedHandler(self, skel, boneName, key): From 01359b8fac1eda328e8b4ec086bb9c135fbf4e6c Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Fri, 9 Feb 2024 20:12:00 +0100 Subject: [PATCH 07/26] Bump version 3.6.0.beta2 --- src/viur/core/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viur/core/version.py b/src/viur/core/version.py index 49d6d82f8..8eb8c85c4 100644 --- a/src/viur/core/version.py +++ b/src/viur/core/version.py @@ -3,7 +3,7 @@ # This will mark it as a pre-release as well on PyPI. # See CONTRIBUTING.md for further information. -__version__ = "3.6.0.beta1" +__version__ = "3.6.0.beta2" assert __version__.count(".") >= 2 and "".join(__version__.split(".", 3)[:3]).isdigit(), \ "Semantic __version__ expected!" From 4f08b6880e953b000970196dca117b69ec997eaa Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Mon, 12 Feb 2024 15:32:26 +0100 Subject: [PATCH 08/26] fix: correctly set `refKeys` to get merged in (#1066) --- src/viur/core/bones/file.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/viur/core/bones/file.py b/src/viur/core/bones/file.py index 73057b02a..dfe312069 100644 --- a/src/viur/core/bones/file.py +++ b/src/viur/core/bones/file.py @@ -130,11 +130,6 @@ class FileBone(TreeLeafBone): """The kind of this bone is 'file'""" type = "relational.tree.leaf.file" """The type of this bone is 'relational.tree.leaf.file'.""" - refKeys = ["name", "key", "mimetype", "dlkey", "size", "width", "height", "derived"] - """ - The list of reference keys for this bone includes "name", "key", "mimetype", "dlkey", "size", "width", - "height", and "derived". - """ def __init__( self, @@ -142,6 +137,7 @@ def __init__( derive: None | dict[str, t.Any] = None, maxFileSize: None | int = None, validMimeTypes: None | list[str] = None, + refKeys: t.Optional[t.Iterable[str]] = ("name", "mimetype", "size", "width", "height", "derived"), **kwargs ): r""" @@ -170,7 +166,7 @@ def __init__( validMimeTypes=["application/pdf", "image/*"] """ - super().__init__(**kwargs) + super().__init__(refKeys=refKeys, **kwargs) self.refKeys.add("dlkey") self.derive = derive From e663c98b46ad9df379e84c9efd96e75bf60acf8a Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Mon, 12 Feb 2024 15:42:22 +0100 Subject: [PATCH 09/26] fix: Use `__getattribute__` instead of `__getattr__` for super call in utils (#1065) It caused ```py 'super' object has no attribute '__getattr__' Traceback (most recent call last): File "/layers/google.python.pip/pip/lib/python3.11/site-packages/viur/core/request.py", line 298, in _process self._route(path) File "/layers/google.python.pip/pip/lib/python3.11/site-packages/viur/core/request.py", line 554, in _route res = caller(*self.args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/layers/google.python.pip/pip/lib/python3.11/site-packages/viur/core/module.py", line 299, in __call__ return self._func(self._instance, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/workspace/modules/file.py", line 112, in static signed_url = utils.downloadUrlFor( ^^^^^^^^^^^^^^^^^^^^ File "/layers/google.python.pip/pip/lib/python3.11/site-packages/viur/core/utils/__init__.py", line 154, in __getattr__ return super(__import__(__name__).__class__).__getattr__(attr) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: 'super' object has no attribute '__getattr__' ``` but should cause: ```py AttributeError: 'super' object has no attribute 'downloadUrlFor' ``` --- src/viur/core/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viur/core/utils/__init__.py b/src/viur/core/utils/__init__.py index e88372f3c..0fff028b0 100644 --- a/src/viur/core/utils/__init__.py +++ b/src/viur/core/utils/__init__.py @@ -151,4 +151,4 @@ def __getattr__(attr): logging.warning(msg, stacklevel=3) return replace[1] - return super(__import__(__name__).__class__).__getattr__(attr) + return super(__import__(__name__).__class__).__getattribute__(attr) From 42d290df6c0ff799a305f9b73920d12632ec3bba Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Mon, 12 Feb 2024 15:56:45 +0100 Subject: [PATCH 10/26] fix: `current.user` unset in deferred task calls (#1067) Already released as v3.5.14 and tested in a live environment. --- CHANGELOG.md | 4 ++++ src/viur/core/modules/user.py | 6 ++---- src/viur/core/tasks.py | 4 ++++ src/viur/core/version.py | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c0b9667a..029244c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ This file documents any relevant changes done to ViUR-core since version 3. +## [3.5.14] + +- fix: `current.user` unset in deferred task calls (#1067) + ## [3.5.13] - fix: `RelationalBone` locking bug (#1052) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index fdfcf700b..cfb2cd868 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -1217,11 +1217,9 @@ def secondFactorProviderByClass(self, cls) -> UserSecondFactorAuthentication: return getattr(self, "f2_%s" % cls.__name__.lower()) def getCurrentUser(self): - # May be a deferred task - if not (session := current.session.get()): - return None + session = current.session.get() - if user := session.get("user"): + if session and (user := session.get("user")): skel = self.baseSkel() skel.setEntity(user) return skel diff --git a/src/viur/core/tasks.py b/src/viur/core/tasks.py index 8e0944af7..c65660581 100644 --- a/src/viur/core/tasks.py +++ b/src/viur/core/tasks.py @@ -222,6 +222,10 @@ def deferred(self, *args, **kwargs): if env: if "user" in env and env["user"]: current.session.get()["user"] = env["user"] + + # Load current user into context variable if user module is there. + if user_mod := getattr(conf["viur.mainApp"], "user", None): + current.user.set(user_mod.getCurrentUser()) if "lang" in env and env["lang"]: current.language.set(env["lang"]) if "transactionMarker" in env: diff --git a/src/viur/core/version.py b/src/viur/core/version.py index 719825c99..ee03f99ad 100644 --- a/src/viur/core/version.py +++ b/src/viur/core/version.py @@ -3,7 +3,7 @@ # This will mark it as a pre-release as well on PyPI. # See CONTRIBUTING.md for further information. -__version__ = "3.5.13" +__version__ = "3.5.14" assert __version__.count(".") >= 2 and "".join(__version__.split(".", 3)[:3]).isdigit(), \ "Semantic __version__ expected!" From b77679c199cdfc07a6945dd2b0e5e2192c2e738a Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Mon, 12 Feb 2024 17:14:41 +0100 Subject: [PATCH 11/26] refactor: `tasks` module (#1016) Refactoring the tasks module. - Restructured `imports` - Renamed preprocessJsonObject into _preprocess_json_object - Renamed jsonDecodeObjectHook into _decode_object_hook - Concise use of datetime-ISO-format - (addresses issue #486 as well!) - breaking change between viur-core 3.5 and viur-core 3.6 when applied to a project where tasks are still open --------- Co-authored-by: Sven Eberth --- src/viur/core/tasks.py | 128 +++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 62 deletions(-) diff --git a/src/viur/core/tasks.py b/src/viur/core/tasks.py index 7da56259f..d00dba6c3 100644 --- a/src/viur/core/tasks.py +++ b/src/viur/core/tasks.py @@ -1,28 +1,26 @@ import abc import base64 +import datetime +import functools +import grpc import json import logging import os +import pytz +import requests import sys +import time import traceback import typing as t -from datetime import datetime, timedelta -from functools import wraps -from time import sleep -from typing import TypeVar - -import grpc -import pytz -import requests +from google import protobuf from google.cloud import tasks_v2 -from google.cloud.tasks_v2.services.cloud_tasks.transports import CloudTasksGrpcTransport -from google.protobuf import timestamp_pb2 from viur.core import current, db, errors, utils from viur.core.config import conf from viur.core.decorators import exposed, skey from viur.core.module import Module -CUSTOM_OBJ = TypeVar("CUSTOM_OBJ") # A JSON serializable object + +CUSTOM_OBJ = t.TypeVar("CUSTOM_OBJ") # A JSON serializable object class CustomEnvironmentHandler(abc.ABC): @@ -45,45 +43,49 @@ def restore(self, obj: CUSTOM_OBJ) -> None: ... -# class JsonKeyEncoder(json.JSONEncoder): -def preprocessJsonObject(o): +def _preprocess_json_object(obj): """ - Add support for Keys, Datetime, Bytes and db.Entities in deferred tasks. - This is not a subclass of json.JSONEncoder anymore, as db.Entites are a subclass of dict, which - is always handled from the json module itself. + Add support for db.Key, datetime, bytes and db.Entity in deferred tasks, + and converts the provided obj into a special dict with JSON-serializable values. """ - if isinstance(o, db.Key): - return {".__key__": db.encodeKey(o)} - elif isinstance(o, datetime): - return {".__datetime__": o.astimezone(pytz.UTC).strftime("%d.%m.%Y %H:%M:%S")} - elif isinstance(o, bytes): - return {".__bytes__": base64.b64encode(o).decode("ASCII")} - elif isinstance(o, db.Entity): - return {".__entity__": preprocessJsonObject(dict(o)), ".__ekey__": db.encodeKey(o.key) if o.key else None} - elif isinstance(o, dict): - return {preprocessJsonObject(k): preprocessJsonObject(v) for k, v in o.items()} - elif isinstance(o, (list, tuple, set)): - return [preprocessJsonObject(x) for x in o] - else: - return o - - -def jsonDecodeObjectHook(obj): + if isinstance(obj, db.Key): + return {".__key__": db.encodeKey(obj)} + elif isinstance(obj, datetime.datetime): + return {".__datetime__": obj.astimezone(pytz.UTC).isoformat()} + elif isinstance(obj, bytes): + return {".__bytes__": base64.b64encode(obj).decode("ASCII")} + elif isinstance(obj, db.Entity): + # TODO: Support Skeleton instances as well? + return { + ".__entity__": _preprocess_json_object(dict(obj)), + ".__ekey__": db.encodeKey(obj.key) if obj.key else None + } + elif isinstance(obj, dict): + return {_preprocess_json_object(k): _preprocess_json_object(v) for k, v in obj.items()} + elif isinstance(obj, (list, tuple, set)): + return [_preprocess_json_object(x) for x in obj] + + return obj + + +def _decode_object_hook(obj): """ - Inverse to JsonKeyEncoder: Check if the object matches a custom ViUR type and recreate it accordingly + Inverse for _preprocess_json_object, which is an object-hook for json.loads. + Check if the object matches a custom ViUR type and recreate it accordingly. """ if len(obj) == 1: - if ".__key__" in obj: - return db.Key.from_legacy_urlsafe(obj[".__key__"]) - elif ".__datetime__" in obj: - value = datetime.strptime(obj[".__datetime__"], "%d.%m.%Y %H:%M:%S") - return datetime(value.year, value.month, value.day, value.hour, value.minute, value.second, tzinfo=pytz.UTC) - elif ".__bytes__" in obj: - return base64.b64decode(obj[".__bytes__"]) + if key := obj.get(".__key__"): + return db.Key.from_legacy_urlsafe(key) + elif date := obj.get(".__datetime__"): + return datetime.datetime.fromisoformat(date) + elif buf := obj.get(".__bytes__"): + return base64.b64decode(buf) + elif len(obj) == 2 and ".__entity__" in obj and ".__ekey__" in obj: - r = db.Entity(db.Key.from_legacy_urlsafe(obj[".__ekey__"]) if obj[".__ekey__"] else None) - r.update(obj[".__entity__"]) - return r + entity = db.Entity(db.Key.from_legacy_urlsafe(obj[".__ekey__"]) if obj[".__ekey__"] else None) + entity.update(obj[".__entity__"]) + return entity + return obj @@ -115,7 +117,9 @@ def jsonDecodeObjectHook(obj): taskClient = tasks_v2.CloudTasksClient() else: taskClient = tasks_v2.CloudTasksClient( - transport=CloudTasksGrpcTransport(channel=grpc.insecure_channel(os.getenv("TASKS_EMULATOR"))) + transport=tasks_v2.services.cloud_tasks.transports.CloudTasksGrpcTransport( + channel=grpc.insecure_channel(os.getenv("TASKS_EMULATOR")) + ) ) queueRegion = "local" @@ -171,7 +175,7 @@ def execute(self): """ The actual code that should be run goes here. """ - raise NotImplemented() + raise NotImplementedError() class TaskHandler(Module): @@ -217,7 +221,7 @@ def queryIter(self, *args, **kwargs): """ req = current.request.get().request self._validate_request() - data = json.loads(req.body, object_hook=jsonDecodeObjectHook) + data = json.loads(req.body, object_hook=_decode_object_hook) if data["classID"] not in MetaQueryIter._classCache: logging.error(f"""Could not continue queryIter - {data["classID"]} not known on this instance""") MetaQueryIter._classCache[data["classID"]]._qryStep(data) @@ -238,7 +242,7 @@ def deferred(self, *args, **kwargs): f"""Task {req.headers.get("X-Appengine-Taskname", "")} is retried for the {retryCount}th time.""" ) - cmd, data = json.loads(req.body, object_hook=jsonDecodeObjectHook) + cmd, data = json.loads(req.body, object_hook=_decode_object_hook) funcPath, args, kwargs, env = data logging.debug(f"Call task {funcPath} with {cmd=} {args=} {kwargs=} {env=}") @@ -277,7 +281,7 @@ def deferred(self, *args, **kwargs): logging.exception(e) raise errors.RequestTimeout() # Task-API should retry elif cmd == "unb": - if not funcPath in _deferred_tasks: + if funcPath not in _deferred_tasks: logging.error(f"Missed deferred task {funcPath=} ({args=},{kwargs=})") # We call the deferred function *directly* (without walking through the mkDeferred lambda), so ensure # that any hit to another deferred function will defer again @@ -305,7 +309,7 @@ def cron(self, cronName="default", *args, **kwargs): periodicTaskName = task.periodicTaskName.lower() if interval: # Ensure this task doesn't get called to often lastCall = db.Get(db.Key("viur-task-interval", periodicTaskName)) - if lastCall and utils.utcNow() - lastCall["date"] < timedelta(minutes=interval): + if lastCall and utils.utcNow() - lastCall["date"] < datetime.timedelta(minutes=interval): logging.debug(f"Task {periodicTaskName!r} has already run recently - skipping.") continue res = self.findBoundTask(task, conf.main_app) @@ -408,7 +412,7 @@ def execute(self, taskID, *args, **kwargs): TaskHandler.html = True -## Decorators ## +# Decorators def retry_n_times(retries: int, email_recipients: None | str | list[str] = None, tpl: None | str = None) -> t.Callable: @@ -429,7 +433,7 @@ def retry_n_times(retries: int, email_recipients: None | str | list[str] = None, 
{{traceback|escape}}
""" def outer_wrapper(func): - @wraps(func) + @functools.wraps(func) def inner_wrapper(*args, **kwargs): try: retry_count = int(current.request.get().request.headers.get("X-Appengine-Taskretrycount", -1)) @@ -480,7 +484,7 @@ def inner_wrapper(*args, **kwargs): def noRetry(f): """Prevents a deferred Function from being called a second time""" - logging.warning(f"Use of `@noRetry` is deprecated; Use `@retry_n_times(0)` instead!", stacklevel=2) + logging.warning("Use of `@noRetry` is deprecated; Use `@retry_n_times(0)` instead!", stacklevel=2) return retry_n_times(0)(f) @@ -537,7 +541,7 @@ def make_deferred(func, self=__undefinedFlag_, *args, **kwargs): # Run tasks inline logging.debug(f"{func=} will be executed inline") - @wraps(func) + @functools.wraps(func) def task(): if self is __undefinedFlag_: return func(*args, **kwargs) @@ -608,7 +612,7 @@ def task(): # Create task description task = tasks_v2.Task( app_engine_http_request=tasks_v2.AppEngineHttpRequest( - body=json.dumps(preprocessJsonObject((command, (funcPath, args, kwargs, env)))).encode("UTF-8"), + body=json.dumps(_preprocess_json_object((command, (funcPath, args, kwargs, env)))).encode("UTF-8"), http_method=tasks_v2.HttpMethod.POST, relative_uri=taskargs["url"], app_engine_routing=tasks_v2.AppEngineRouting( @@ -622,10 +626,10 @@ def task(): # Set a schedule time in case eta (absolut) or countdown (relative) was set. eta = taskargs.get("eta") if seconds := taskargs.get("countdown"): - eta = utils.utcNow() + timedelta(seconds=seconds) + eta = utils.utcNow() + datetime.timedelta(seconds=seconds) if eta: # We must send a Timestamp Protobuf instead of a date-string - timestamp = timestamp_pb2.Timestamp() + timestamp = protobuf.timestamp_pb2.Timestamp() timestamp.FromDatetime(eta) task.schedule_time = timestamp @@ -639,7 +643,7 @@ def task(): global _deferred_tasks _deferred_tasks[f"{func.__name__}.{func.__module__}"] = func - @wraps(func) + @functools.wraps(func) def wrapper(*args, **kwargs): return make_deferred(func, *args, **kwargs) @@ -652,7 +656,7 @@ def callDeferred(func): """ import logging, warnings - msg = f"Use of @callDeferred is deprecated, use @CallDeferred instead." + msg = "Use of @callDeferred is deprecated, use @CallDeferred instead." logging.warning(msg, stacklevel=3) warnings.warn(msg, stacklevel=3) @@ -674,7 +678,7 @@ def mkDecorator(fn): if fn.__name__.startswith("_"): raise RuntimeError("Periodic called methods cannot start with an underscore! " f"Please rename {fn.__name__!r}") - if not cronName in _periodicTasks: + if cronName not in _periodicTasks: _periodicTasks[cronName] = {} _periodicTasks[cronName][fn] = interval fn.periodicTaskName = f"{fn.__module__}_{fn.__qualname__}".replace(".", "_").lower() @@ -783,7 +787,7 @@ def _requeueStep(cls, qryDict: dict[str, t.Any]) -> None: parent=taskClient.queue_path(conf.instance.project_id, queueRegion, cls.queueName), task=tasks_v2.Task( app_engine_http_request=tasks_v2.AppEngineHttpRequest( - body=json.dumps(preprocessJsonObject(qryDict)).encode("UTF-8"), + body=json.dumps(_preprocess_json_object(qryDict)).encode("UTF-8"), http_method=tasks_v2.HttpMethod.POST, relative_uri="/_tasks/queryIter", app_engine_routing=tasks_v2.AppEngineRouting( @@ -815,7 +819,7 @@ def _qryStep(cls, qryDict: dict[str, t.Any]) -> None: try: cls.handleEntry(item, qryDict["customData"]) except: # First exception - we'll try another time (probably/hopefully transaction collision) - sleep(5) + time.sleep(5) try: cls.handleEntry(item, qryDict["customData"]) except Exception as e: # Second exception - call error_handler From b52b92fdf2922df742491b32f1d33022bf9a1226 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Thu, 15 Feb 2024 13:18:06 +0100 Subject: [PATCH 12/26] fix: new icon naming scheme in modules (#1069) --- src/viur/core/config.py | 10 +++++----- src/viur/core/modules/file.py | 2 +- src/viur/core/modules/page.py | 2 +- src/viur/core/modules/user.py | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/viur/core/config.py b/src/viur/core/config.py index 666211289..d468d1dab 100644 --- a/src/viur/core/config.py +++ b/src/viur/core/config.py @@ -271,7 +271,7 @@ class Admin(ConfigType): """secondary color for viur-admin""" module_groups: dict[str, dict[t.Literal["name", "icon", "sortindex"], str | int]] = {} - """Module Groups for the VI + """Module Groups for the admin tool Group modules in the sidebar in categories (groups). @@ -279,12 +279,12 @@ class Admin(ConfigType): conf.admin.module_groups = { "content": { "name": "Content", - "icon": "text-file", + "icon": "file-text-fill", "sortindex": 10, }, "shop": { "name": "Shop", - "icon": "cart", + "icon": "cart-fill", "sortindex": 20, }, } @@ -654,13 +654,13 @@ class Conf(ConfigType): """Upper limit of the amount of parameters we accept per request. Prevents Hash-Collision-Attacks""" moduleconf_admin_info: dict[str, t.Any] = { - "icon": "icon-settings", + "icon": "gear-fill", "display": "hidden", } """Describing the internal ModuleConfig-module""" script_admin_info: dict[str, t.Any] = { - "icon": "icon-hashtag", + "icon": "file-code-fill", "display": "hidden", } """Describing the Script module""" diff --git a/src/viur/core/modules/file.py b/src/viur/core/modules/file.py index d3d300a8c..60b6b2e50 100644 --- a/src/viur/core/modules/file.py +++ b/src/viur/core/modules/file.py @@ -386,7 +386,7 @@ class File(Tree): handler = "tree.simple.file" adminInfo = { - "icon": "file-system", + "icon": "folder-fill", "handler": handler, # fixme: Use static handler; Remove with VIUR4! } diff --git a/src/viur/core/modules/page.py b/src/viur/core/modules/page.py index b7fb3e403..b20bc0388 100644 --- a/src/viur/core/modules/page.py +++ b/src/viur/core/modules/page.py @@ -23,7 +23,7 @@ class Page(Tree): adminInfo = { "name": "Pages", "handler": "tree.nodeonly.page", - "icon": "cloud", + "icon": "cloud-fill", "columns": ["name", "language", "isactive"], "preview": { "Web": "/{{module}}/view/{{key}}" diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 9f775160f..6539d6c24 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -1149,7 +1149,7 @@ class User(List): secondFactorTimeWindow = datetime.timedelta(minutes=10) adminInfo = { - "icon": "users", + "icon": "person-fill", "actions": [ "trigger_kick", "trigger_takeover", @@ -1161,7 +1161,7 @@ class User(List): defaultText="Kick user", hint="Title of the kick user function" ), - "icon": "trash", + "icon": "trash2-fill", "access": ["root"], "action": "fetch", "url": "/vi/{{module}}/trigger/kick/{{key}}?skey={{skey}}", @@ -1180,7 +1180,7 @@ class User(List): defaultText="Take-over user", hint="Title of the take user over function" ), - "icon": "interface", + "icon": "file-person-fill", "access": ["root"], "action": "fetch", "url": "/vi/{{module}}/trigger/takeover/{{key}}?skey={{skey}}", From 93aff4a5a03ce766f572dd8fe6465b2f84f24732 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Mon, 19 Feb 2024 16:24:28 +0100 Subject: [PATCH 13/26] fix: Remove default `P.html = True` from prototypes (#1037) This pull requests removes the inconsequential `Prototype.html = True` from the `List` and `Singleton` prototype. The `Tree` prototype was always not by default visible to the html-render. This is a security issue, which fixes that modules are not visible to the HTML-renderer by default. --- src/viur/core/prototypes/list.py | 1 - src/viur/core/prototypes/singleton.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/viur/core/prototypes/list.py b/src/viur/core/prototypes/list.py index 98faa4b48..4ca63ed0a 100644 --- a/src/viur/core/prototypes/list.py +++ b/src/viur/core/prototypes/list.py @@ -667,5 +667,4 @@ def onCloned(self, skel: SkeletonInstance, src_skel: SkeletonInstance): List.admin = True -List.html = True List.vi = True diff --git a/src/viur/core/prototypes/singleton.py b/src/viur/core/prototypes/singleton.py index 2c5d36ea6..984870bb0 100644 --- a/src/viur/core/prototypes/singleton.py +++ b/src/viur/core/prototypes/singleton.py @@ -293,5 +293,4 @@ def onView(self, skel: SkeletonInstance): Singleton.admin = True -Singleton.html = True Singleton.vi = True From d9cea574abbfa689551e825d42c8e8a21c59b0ab Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Mon, 19 Feb 2024 16:30:09 +0100 Subject: [PATCH 14/26] fix: Several improvements on `ModuleConf` (#1073) - Refactoring - `listFilter` that falls back to any modules the current user is allowed to see - `canEdit` which evaluates the `{module}-manage` access right - Moved `read_all_modules` as staticmethod into ModuleConf --- CHANGELOG.md | 4 ++ src/viur/core/modules/moduleconf.py | 88 +++++++++++++++++++---------- src/viur/core/version.py | 2 +- 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 029244c2b..35772163b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ This file documents any relevant changes done to ViUR-core since version 3. +## [3.5.15] + +- fix: Several improvements on `ModuleConf` (#1073) + ## [3.5.14] - fix: `current.user` unset in deferred task calls (#1067) diff --git a/src/viur/core/modules/moduleconf.py b/src/viur/core/modules/moduleconf.py index e18f0bfd9..b3500a114 100644 --- a/src/viur/core/modules/moduleconf.py +++ b/src/viur/core/modules/moduleconf.py @@ -1,14 +1,14 @@ import logging +from viur.core import Module, conf, db, current, i18n, tasks, skeleton from viur.core.bones import StringBone, TextBone, SelectBone, TreeLeafBone from viur.core.bones.text import _defaultTags -from viur.core.tasks import StartupTask -from viur.core import Module, conf, db -from viur.core.i18n import translate -from viur.core.skeleton import Skeleton, SkeletonInstance, RelSkel from viur.core.prototypes import List -class ScriptRelSkel(RelSkel): +MODULECONF_KINDNAME = "viur-module-conf" + + +class ModuleConfScriptSkel(skeleton.RelSkel): name = StringBone( descr="Label", @@ -44,7 +44,7 @@ class ScriptRelSkel(RelSkel): access = SelectBone( descr="Required access rights", values=lambda: { - right: translate(f"server.modules.user.accessright.{right}", defaultText=right) + right: i18n.translate(f"server.modules.user.accessright.{right}", defaultText=right) for right in sorted(conf["viur.accessRights"]) }, multiple=True, @@ -56,8 +56,8 @@ class ScriptRelSkel(RelSkel): ) -class ModuleConfSkel(Skeleton): - kindName = "viur-module-conf" +class ModuleConfSkel(skeleton.Skeleton): + kindName = MODULECONF_KINDNAME _valid_tags = ['b', 'a', 'i', 'u', 'span', 'div', 'p', 'ol', 'ul', 'li', 'abbr', 'sub', 'sup', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'br', 'hr', 'strong', 'blockquote', 'em'] @@ -65,30 +65,30 @@ class ModuleConfSkel(Skeleton): _valid_html["validTags"] = _valid_tags name = StringBone( - descr=translate("modulename"), + descr=i18n.translate("modulename"), readOnly=True, ) help_text = TextBone( - descr=translate("module helptext"), + descr=i18n.translate("module helptext"), validHtml=_valid_html, ) help_text_add = TextBone( - descr=translate("add helptext"), + descr=i18n.translate("add helptext"), validHtml=_valid_html, ) help_text_edit = TextBone( - descr=translate("edit helptext"), + descr=i18n.translate("edit helptext"), validHtml=_valid_html, ) scripts = TreeLeafBone( - descr=translate("scriptor scripts"), + descr=i18n.translate("scriptor scripts"), module="script", kind="viur-script-leaf", - using=ScriptRelSkel, + using=ModuleConfScriptSkel, refKeys=[ "key", "name", @@ -103,7 +103,8 @@ class ModuleConf(List): This module is for ViUR internal purposes only. It lists all other modules to be able to provide them with help texts. """ - kindName = "viur-module-conf" + MODULES = set() # will be filled by read_all_modules + kindName = MODULECONF_KINDNAME accessRights = ["edit"] def adminInfo(self): @@ -112,33 +113,58 @@ def adminInfo(self): def canAdd(self): return False - def canDelete(self, skel: SkeletonInstance) -> bool: + def canDelete(self, skel): return False + def canEdit(self, skel): + if super().canEdit(skel): + return True + + # Check for "manage"-flag on current user + return (cuser := current.user.get()) and f"""{skel["name"]}-manage""" in cuser["access"] + + def listFilter(self, query): + original_query = query + + # when super-call does not satisfy... + if not (query := super().listFilter(query)): + if cuser := current.user.get(): + # ... then, list modules the user is allowed to use! + user_modules = set(right.split("-", 1)[0] for right in cuser["access"] if "-" in right) + + query = original_query + query.filter("name IN", tuple(user_modules)) + + return query + @classmethod - def get_by_module_name(cls, module_name: str) -> None | SkeletonInstance: - db_key = db.Key("viur-module-conf", module_name) + def get_by_module_name(cls, module_name: str) -> None | skeleton.SkeletonInstance: + db_key = db.Key(MODULECONF_KINDNAME, module_name) skel = conf["viur.mainApp"]._moduleconf.viewSkel() if not skel.fromDB(db_key): - logging.error(f"module({module_name}) not found in viur-module-conf") + logging.error(f"module({module_name}) not found") return None return skel + @tasks.StartupTask + @staticmethod + def read_all_modules(): + db_module_names = (m["name"] for m in db.Query(MODULECONF_KINDNAME).run(999)) + module_names = dir(conf["viur.mainApp"].vi) + + for module_name in module_names: + module = getattr(conf["viur.mainApp"].vi, module_name) + if isinstance(module, Module): + ModuleConf.MODULES.add(module_name) -@StartupTask -def read_all_modules(): - db_module_names = [m["name"] for m in db.Query("viur-module-conf").run(999)] - module_names = dir(conf["viur.mainApp"].vi) + if module_name not in db_module_names: + skel = conf["viur.mainApp"]._moduleconf.addSkel() + skel["key"] = db.Key(MODULECONF_KINDNAME, module_name) + skel["name"] = module_name + skel.toDB() - for module_name in module_names: - module = getattr(conf["viur.mainApp"].vi, module_name) - if isinstance(module, Module): - if module_name not in db_module_names: - skel = conf["viur.mainApp"]._moduleconf.addSkel() - skel["key"] = db.Key("viur-module-conf", module_name) - skel["name"] = module_name - skel.toDB() + # TODO: Remove entries from MODULECONF_KINDNAME which are in db_module_names but not in ModuleConf.MODULES ModuleConf.json = True diff --git a/src/viur/core/version.py b/src/viur/core/version.py index ee03f99ad..7dc3c0e05 100644 --- a/src/viur/core/version.py +++ b/src/viur/core/version.py @@ -3,7 +3,7 @@ # This will mark it as a pre-release as well on PyPI. # See CONTRIBUTING.md for further information. -__version__ = "3.5.14" +__version__ = "3.5.15" assert __version__.count(".") >= 2 and "".join(__version__.split(".", 3)[:3]).isdigit(), \ "Semantic __version__ expected!" From 11a7e51281c0c6de52ba729239fd8d52bfe5bcc1 Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Thu, 22 Feb 2024 00:33:54 +0100 Subject: [PATCH 15/26] fix: Make ViUR runable again after the extremely security-relevant PR #1037 (#1075) Make ViUR, especially the user system runable again after the _extremely_ security-relevant and _non-breaking_ PR #1037 --- src/viur/core/bones/text.py | 4 ++-- src/viur/core/email.py | 4 ++-- src/viur/core/modules/moduleconf.py | 2 +- src/viur/core/modules/user.py | 6 +++--- src/viur/core/request.py | 2 +- src/viur/core/session.py | 2 +- src/viur/core/tasks.py | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/viur/core/bones/text.py b/src/viur/core/bones/text.py index 55812e323..d31d96704 100644 --- a/src/viur/core/bones/text.py +++ b/src/viur/core/bones/text.py @@ -70,7 +70,7 @@ def handle_starttag(self, tag, attrs): if tag in ["a", "img"]: for k, v in attrs: if k == "src": - file = getattr(conf.main_app, "file", None) + file = getattr(conf.main_app.vi, "file", None) if file and (filepath := file.parse_download_url(v)): self.blobs.add(filepath.dlkey) @@ -179,7 +179,7 @@ def handle_starttag(self, tag, attrs): if not (checker.startswith("http://") or checker.startswith("https://") or checker.startswith("/")): continue - file = getattr(conf.main_app, "file", None) + file = getattr(conf.main_app.vi, "file", None) if file and (filepath := file.parse_download_url(v)): v = file.create_download_url( filepath.dlkey, diff --git a/src/viur/core/email.py b/src/viur/core/email.py index 3f811c8ba..5ac3d0c6e 100644 --- a/src/viur/core/email.py +++ b/src/viur/core/email.py @@ -272,8 +272,8 @@ def sendEMailToAdmins(subject: str, body: str, *args, **kwargs) -> bool: users = [] if conf.email.admin_recipients is not None: users = normalize_to_list(conf.email.admin_recipients) - elif "user" in dir(conf.main_app): - for user_skel in conf.main_app.user.viewSkel().all().filter("access =", "root").fetch(): + elif "user" in dir(conf.main_app.vi): + for user_skel in conf.main_app.vi.user.viewSkel().all().filter("access =", "root").fetch(): users.append(user_skel["name"]) # Prefix the instance's project_id to subject diff --git a/src/viur/core/modules/moduleconf.py b/src/viur/core/modules/moduleconf.py index f9e1fc6c6..423e1eb57 100644 --- a/src/viur/core/modules/moduleconf.py +++ b/src/viur/core/modules/moduleconf.py @@ -140,7 +140,7 @@ def listFilter(self, query): @classmethod def get_by_module_name(cls, module_name: str) -> None | skeleton.SkeletonInstance: db_key = db.Key(MODULECONF_KINDNAME, module_name) - skel = conf.main_app._moduleconf.viewSkel() + skel = conf.main_app.vi._moduleconf.viewSkel() if not skel.fromDB(db_key): logging.error(f"module({module_name}) not found") return None diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 6539d6c24..9b3a32fea 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -125,11 +125,11 @@ def __new__(cls): to dynamically add bones required for the configured authentication methods. """ - for provider in conf.main_app.user.authenticationProviders: + for provider in conf.main_app.vi.user.authenticationProviders: assert issubclass(provider, UserPrimaryAuthentication) provider.patch_user_skel(cls) - for provider in conf.main_app.user.secondFactorProviders: + for provider in conf.main_app.vi.user.secondFactorProviders: assert issubclass(provider, UserSecondFactorAuthentication) provider.patch_user_skel(cls) @@ -1522,7 +1522,7 @@ def createNewUserIfNotExists(): """ Create a new Admin user, if the userDB is empty """ - userMod = getattr(conf.main_app, "user", None) + userMod = getattr(conf.main_app.vi, "user", None) if (userMod # We have a user module and isinstance(userMod, User) and "addSkel" in dir(userMod) diff --git a/src/viur/core/request.py b/src/viur/core/request.py index dca1051e0..82e19320c 100644 --- a/src/viur/core/request.py +++ b/src/viur/core/request.py @@ -288,7 +288,7 @@ def _process(self): current.session.get().load(self) # Load current user into context variable if user module is there. - if user_mod := getattr(conf.main_app, "user", None): + if user_mod := getattr(conf.main_app.vi, "user", None): current.user.set(user_mod.getCurrentUser()) path = self._select_language(path)[1:] diff --git a/src/viur/core/session.py b/src/viur/core/session.py index 4b7002e3c..e010bf510 100644 --- a/src/viur/core/session.py +++ b/src/viur/core/session.py @@ -98,7 +98,7 @@ def save(self, req): # Get the current user's key try: # Check for our custom user-api - user_key = conf.main_app.user.getCurrentUser()["key"] + user_key = conf.main_app.vi.user.getCurrentUser()["key"] except Exception: user_key = Session.GUEST_USER # this is a guest diff --git a/src/viur/core/tasks.py b/src/viur/core/tasks.py index d00dba6c3..a3d705978 100644 --- a/src/viur/core/tasks.py +++ b/src/viur/core/tasks.py @@ -251,7 +251,7 @@ def deferred(self, *args, **kwargs): current.session.get()["user"] = env["user"] # Load current user into context variable if user module is there. - if user_mod := getattr(conf.main_app, "user", None): + if user_mod := getattr(conf.main_app.vi, "user", None): current.user.set(user_mod.getCurrentUser()) if "lang" in env and env["lang"]: current.language.set(env["lang"]) From 5232286b060dd7ba00a009a3fc3675e9d4d25612 Mon Sep 17 00:00:00 2001 From: Sven Eberth Date: Thu, 22 Feb 2024 00:36:11 +0100 Subject: [PATCH 16/26] fix: `read_all_modules` after #1073 and #1037 and merge 2a2b76ec16 (#1074) fix `read_all_modules` after #1073 and #1037 and merge 2a2b76ec16 --- src/viur/core/modules/moduleconf.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/viur/core/modules/moduleconf.py b/src/viur/core/modules/moduleconf.py index 423e1eb57..bce8e598a 100644 --- a/src/viur/core/modules/moduleconf.py +++ b/src/viur/core/modules/moduleconf.py @@ -151,6 +151,7 @@ def get_by_module_name(cls, module_name: str) -> None | skeleton.SkeletonInstanc @staticmethod def read_all_modules(): db_module_names = (m["name"] for m in db.Query(MODULECONF_KINDNAME).run(999)) + visited_modules = set() def collect_modules(parent, depth: int = 0, prefix: str = "") -> None: """Recursively collects all routable modules for the vi renderer""" @@ -162,18 +163,24 @@ def collect_modules(parent, depth: int = 0, prefix: str = "") -> None: module = getattr(parent, module_name, None) if not isinstance(module, Module): continue - + if module in visited_modules: + # Some modules reference other modules as parents, this will + # lead to infinite recursion. We can avoid reaching the + # maximum recursion limit by remembering already seen modules. + if conf.debug.trace: + logging.debug(f"Already visited and added {module=}") + continue module_name = f"{prefix}{module_name}" - + visited_modules.add(module) ModuleConf.MODULES.add(module_name) if module_name not in db_module_names: - skel = conf.main_app._moduleconf.addSkel() + skel = conf.main_app.vi._moduleconf.addSkel() skel["key"] = db.Key(MODULECONF_KINDNAME, module_name) skel["name"] = module_name skel.toDB() - # Collect children - collect_modules(module, depth=depth + 1, prefix=f"{module_name}.") + # Collect children + collect_modules(module, depth=depth + 1, prefix=f"{module_name}.") collect_modules(conf.main_app.vi) # TODO: Remove entries from MODULECONF_KINDNAME which are in db_module_names but not in ModuleConf.MODULES From 72b6da3a14f43f68246af6cd9ec529df12d946b0 Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Mon, 26 Feb 2024 22:25:45 +0100 Subject: [PATCH 17/26] fix: Remove `self` from `create_src_set` (#1079) --- src/viur/core/modules/file.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/viur/core/modules/file.py b/src/viur/core/modules/file.py index 60b6b2e50..cecb79bc7 100644 --- a/src/viur/core/modules/file.py +++ b/src/viur/core/modules/file.py @@ -502,7 +502,6 @@ def parse_download_url(url) -> t.Optional[FilePath]: @staticmethod def create_src_set( - self, file: t.Union["SkeletonInstance", dict, str], expires: t.Optional[datetime.timedelta | int] = datetime.timedelta(hours=1), width: t.Optional[int] = None, From 471bee591af7c1620357bf1a7c24f03abb1bfffc Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Tue, 27 Feb 2024 12:01:49 +0100 Subject: [PATCH 18/26] feat: securitykey create duration allow timedelta (#1078) --- src/viur/core/securitykey.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/viur/core/securitykey.py b/src/viur/core/securitykey.py index 9b0400384..b32129b52 100644 --- a/src/viur/core/securitykey.py +++ b/src/viur/core/securitykey.py @@ -33,7 +33,7 @@ def create( - duration: None | int = None, + duration: None | int | datetime.timedelta = None, session_bound: bool = True, key_length: int = 13, indexed: bool = True, @@ -44,7 +44,7 @@ def create( The custom data (given as **custom_data) that can be stored with the key. Any data provided must be serializable by the datastore. - :param duration: Make this CSRF-token valid for a fixed timeframe of seconds. + :param duration: Make this CSRF-token valid for a fixed timeframe. :param session_bound: Bind this CSRF-token to the current session. :param indexed: Indexes all values stored with the security-key (default), set False to not index. :param custom_data: Any other data is stored with the CSRF-token, for later re-use. @@ -56,14 +56,17 @@ def create( if not duration: duration = conf.user.session_life_time if session_bound else SECURITYKEY_DURATION - key = utils.string.random(key_length) entity = db.Entity(db.Key(SECURITYKEY_KINDNAME, key)) entity |= custom_data entity["viur_session"] = current.session.get().cookie_key if session_bound else None - entity["viur_until"] = utils.utcNow() + datetime.timedelta(seconds=int(duration)) + if isinstance(duration, datetime.timedelta): + entity["viur_until"] = utils.utcNow() + duration + else: + entity["viur_until"] = utils.utcNow() + datetime.timedelta(seconds=int(duration)) + if not indexed: entity.exclude_from_indexes = [k for k in entity.keys() if not k.startswith("viur_")] From dc77806839707fbcf7d6d0a217a35297e2eb8e10 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Tue, 27 Feb 2024 19:47:00 +0100 Subject: [PATCH 19/26] feat: Provide `default_order` for `List` and `Tree` (#1076) To avoid the creation of unsecure and error-prone `listFilter`s, this approach tries to fix #1011 and improves the ViUR system generally: 1. Trees default to sort by "sortindex" 2. The `File`-module defaults sorts by "name" 3. The `User`-module defaults to sort by "name" as well --- src/viur/core/modules/file.py | 2 ++ src/viur/core/modules/user.py | 2 ++ src/viur/core/prototypes/list.py | 31 +++++++++++++++++++------ src/viur/core/prototypes/skelmodule.py | 13 ++++++++++- src/viur/core/prototypes/tree.py | 32 +++++++++++++++++++------- 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/viur/core/modules/file.py b/src/viur/core/modules/file.py index cecb79bc7..1885f3b80 100644 --- a/src/viur/core/modules/file.py +++ b/src/viur/core/modules/file.py @@ -396,6 +396,8 @@ class File(Tree): "admin": "*", } + default_order = "name" + # Helper functions currently resist here @staticmethod diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 9b3a32fea..2b9dac771 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -1148,6 +1148,8 @@ class User(List): secondFactorTimeWindow = datetime.timedelta(minutes=10) + default_order = "name.idx" + adminInfo = { "icon": "person-fill", "actions": [ diff --git a/src/viur/core/prototypes/list.py b/src/viur/core/prototypes/list.py index 4ca63ed0a..f9bd4cc86 100644 --- a/src/viur/core/prototypes/list.py +++ b/src/viur/core/prototypes/list.py @@ -4,7 +4,7 @@ from viur.core.decorators import * from viur.core.cache import flushCache from viur.core.skeleton import SkeletonInstance -from .skelmodule import SkelModule +from .skelmodule import SkelModule, DEFAULT_ORDER_TYPE class List(SkelModule): @@ -14,11 +14,20 @@ class List(SkelModule): The list module prototype handles datasets in a flat list. It can be extended to filters and views to provide various use-cases. - Definitely, it is the mostly-used prototype in any ViUR project. + It is undoubtedly the most frequently used prototype in any ViUR project. """ handler = "list" accessRights = ("add", "edit", "view", "delete", "manage") + default_order: DEFAULT_ORDER_TYPE = \ + lambda _self, query: next((prop for prop in ("sortindex", "name") if prop in query.srcSkel), None) + """ + Allows to specify a default order for this module, which is applied when no other order is specified. + + Setting a default_order might result in the requirement of additional indexes, which are being raised + and must be specified. + """ + def viewSkel(self, *args, **kwargs) -> SkeletonInstance: """ Retrieve a new instance of a :class:`viur.core.skeleton.SkeletonInstance` that is used by the application @@ -174,11 +183,19 @@ def list(self, *args, **kwargs) -> t.Any: :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions. """ - query = self.listFilter(self.viewSkel().all().mergeExternalFilter(kwargs)) # Access control - if query is None: - raise errors.Unauthorized() - res = query.fetch() - return self.render.list(res) + # The general access control is made via self.listFilter() + if query := self.listFilter(self.viewSkel().all().mergeExternalFilter(kwargs)): + # Apply default order when specified + if self.default_order and not query.queries.orders and not current.request.get().kwargs.get("search"): + if callable(default_order := self.default_order): + default_order = default_order(query) + + if default_order: + query.order(default_order) + + return self.render.list(query.fetch()) + + raise errors.Unauthorized() @force_ssl @exposed diff --git a/src/viur/core/prototypes/skelmodule.py b/src/viur/core/prototypes/skelmodule.py index 6a92c7780..ae5296a04 100644 --- a/src/viur/core/prototypes/skelmodule.py +++ b/src/viur/core/prototypes/skelmodule.py @@ -1,8 +1,19 @@ -from viur.core import Module +from viur.core import Module, db from viur.core.skeleton import skeletonByKind, Skeleton, SkeletonInstance import typing as t +ORDER_TYPE = str | tuple[str, db.SortOrder] | None +""" +Type for sort order definitions. +""" + +DEFAULT_ORDER_TYPE = ORDER_TYPE | t.Callable[[db.Query], ORDER_TYPE] +""" +Type for default sort order definitions. +""" + + class SkelModule(Module): """ This is the extended module prototype used by any other ViUR module prototype. diff --git a/src/viur/core/prototypes/tree.py b/src/viur/core/prototypes/tree.py index 9eedf79cd..5f7480833 100644 --- a/src/viur/core/prototypes/tree.py +++ b/src/viur/core/prototypes/tree.py @@ -6,7 +6,7 @@ from viur.core.cache import flushCache from viur.core.skeleton import Skeleton, SkeletonInstance from viur.core.tasks import CallDeferred -from .skelmodule import SkelModule +from .skelmodule import SkelModule, DEFAULT_ORDER_TYPE SkelType = t.Literal["node", "leaf"] @@ -47,6 +47,14 @@ class Tree(SkelModule): nodeSkelCls = None leafSkelCls = None + default_order: DEFAULT_ORDER_TYPE = "sortindex" + """ + Allows to specify a default order for this module, which is applied when no other order is specified. + + Setting a default_order might result in the requirement of additional indexes, which are being raised + and must be specified. + """ + def __init__(self, moduleName, modulePath, *args, **kwargs): assert self.nodeSkelCls, f"Need to specify at least nodeSkelCls for {self.__class__.__name__!r}" super().__init__(moduleName, modulePath, *args, **kwargs) @@ -281,13 +289,21 @@ def list(self, skelType: SkelType, *args, **kwargs) -> t.Any: :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions. """ if not (skelType := self._checkSkelType(skelType)): - raise errors.NotAcceptable(f"Invalid skelType provided.") - skel = self.viewSkel(skelType) - query = self.listFilter(skel.all().mergeExternalFilter(kwargs)) # Access control - if query is None: - raise errors.Unauthorized() - res = query.fetch() - return self.render.list(res) + raise errors.NotAcceptable("Invalid skelType provided.") + + # The general access control is made via self.listFilter() + if query := self.listFilter(self.viewSkel(skelType).all().mergeExternalFilter(kwargs)): + # Apply default order when specified + if self.default_order and not query.queries.orders and not current.request.get().kwargs.get("search"): + if callable(default_order := self.default_order): + default_order = default_order(query) + + if default_order: + query.order(default_order) + + return self.render.list(query.fetch()) + + raise errors.Unauthorized() @exposed def structure(self, skelType: SkelType, *args, **kwargs) -> t.Any: From 50089ab3b2f51dab5279e4b4a68980f75ac30351 Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Tue, 27 Feb 2024 22:00:48 +0100 Subject: [PATCH 20/26] feat: Allow `None` in `skel setBoneValue` (#1053) Resolves #1047. @sveneberth I hope I have understood your issue correctly. This PR sets the value of the bones to `None`. --------- Co-authored-by: Jan Max Meyer Co-authored-by: Sven Eberth --- src/viur/core/skeleton.py | 44 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/viur/core/skeleton.py b/src/viur/core/skeleton.py index 0d924ad02..38c386f15 100644 --- a/src/viur/core/skeleton.py +++ b/src/viur/core/skeleton.py @@ -391,24 +391,50 @@ def setSystemInitialized(cls): bone.setSystemInitialized() @classmethod - def setBoneValue(cls, skelValues: t.Any, boneName: str, value: t.Any, - append: bool = False, language: t.Optional[str] = None) -> bool: + def setBoneValue( + cls, + skelValues: SkeletonInstance, + boneName: str, + value: t.Any, + append: bool = False, + language: t.Optional[str] = None + ) -> bool: """ - Allow setting a bones value without calling fromClient or assigning to valuesCache directly. - Santy-Checks are performed; if the value is invalid, that bone flips back to its original + Allows for setting a bones value without calling fromClient or assigning a value directly. + Sanity-Checks are performed; if the value is invalid, that bone flips back to its original (default) value and false is returned. - :param boneName: The Bone which should be modified + :param boneName: The name of the bone to be modified :param value: The value that should be assigned. It's type depends on the type of that bone - :param append: If true, the given value is appended to the values of that bone instead of + :param append: If True, the given value is appended to the values of that bone instead of replacing it. Only supported on bones with multiple=True - :param language: Set/append which language + :param language: Language to set + :return: Wherever that operation succeeded or not. """ bone = getattr(skelValues, boneName, None) + if not isinstance(bone, BaseBone): - raise ValueError(f"{boneName} is no valid bone on this skeleton ({skelValues})") - skelValues[boneName] # FIXME, ensure this bone is unserialized first + raise ValueError(f"{boneName!r} is no valid bone on this skeleton ({skelValues!r})") + + if language: + if not bone.languages: + raise ValueError("The bone {boneName!r} has no language setting") + elif language not in bone.languages: + raise ValueError("The language {language!r} is not available for bone {boneName!r}") + + if value is None: + if append: + raise ValueError("Cannot append None-value to bone {boneName!r}") + + if language: + skelValues[boneName][language] = [] if bone.multiple else None + else: + skelValues[boneName] = [] if bone.multiple else None + + return True + + _ = skelValues[boneName] # ensure the bone is being unserialized first return bone.setBoneValue(skelValues, boneName, value, append, language) @classmethod From 703e25d3e6802a09c73349216c7e01a70bb2d3ec Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Wed, 28 Feb 2024 16:14:41 +0100 Subject: [PATCH 21/26] feat: Require abstract `METHOD_NAME` for any `UserAuthentication` (#1059) - Replaces the `getAuthMethodName()` and `get2FactorMethodName()`-functions by a unified `METHOD_NAME`-abstract property. - Mark `User.getAuthMethods()` as deprecated, as `User.login()` is the correct function to retrieve any login methods. The code is also duplicated between both functions. --------- Co-authored-by: Sven Eberth --- src/viur/core/modules/user.py | 52 ++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 2b9dac771..f6b2d26dd 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -177,6 +177,14 @@ def toDB(cls, skel, *args, **kwargs): class UserAuthentication(Module, abc.ABC): + @property + @abc.abstractstaticmethod + def METHOD_NAME() -> str: + """ + Define a unique method name for this authentication. + """ + ... + def __init__(self, moduleName, modulePath, userModule): super().__init__(moduleName, modulePath) self._user_module = userModule @@ -202,11 +210,9 @@ class UserPrimaryAuthentication(UserAuthentication, abc.ABC): def login(self, *args, kwargs): ... - @abc.abstractmethod - def getAuthMethodName(self, *args, **kwargs) -> str: pass - class UserPassword(UserPrimaryAuthentication): + METHOD_NAME = "X-VIUR-AUTH-User-Password" registrationEmailVerificationRequired = True registrationAdminVerificationRequired = True @@ -227,10 +233,6 @@ class UserPassword(UserPrimaryAuthentication): # Limit (invalid) login-retries to once per 5 seconds loginRateLimit = RateLimit("user.login", 12, 1, "ip") - @classmethod - def getAuthMethodName(*args, **kwargs): - return "X-VIUR-AUTH-User-Password" - @classmethod def patch_user_skel(cls, skel_cls): """ @@ -572,10 +574,7 @@ def add(self, *args, **kwargs): class GoogleAccount(UserPrimaryAuthentication): - - @classmethod - def getAuthMethodName(*args, **kwargs): - return "X-VIUR-AUTH-Google-Account" + METHOD_NAME = "X-VIUR-AUTH-Google-Account" @classmethod def patch_user_skel(cls, skel_cls): @@ -698,6 +697,7 @@ def __init__(self, moduleName, modulePath, _user_module): class TimeBasedOTP(UserSecondFactorAuthentication): + METHOD_NAME = "X-VIUR-2FACTOR-TimeBasedOTP" WINDOW_SIZE = 5 ACTION_NAME = "otp" NAME = "Time based Otp" @@ -726,10 +726,6 @@ class OtpSkel(skeleton.RelSkel): min=0, ) - @classmethod - def get2FactorMethodName(*args, **kwargs): # fixme: What is the purpose of this function? Why not just a member? - return "X-VIUR-2FACTOR-TimeBasedOTP" - @classmethod def patch_user_skel(cls, skel_cls): """ @@ -949,10 +945,14 @@ class AuthenticatorOTP(UserSecondFactorAuthentication): """ This class handles the second factor for apps like authy and so on """ + METHOD_NAME = "X-VIUR-2FACTOR-AuthenticatorOTP" + second_factor_add_template = "user_secondfactor_add" """Template to configure (add) a new TOPT""" + ACTION_NAME = "authenticator_otp" """Action name provided for *otp_template* on login""" + NAME = "Authenticator App" @exposed @@ -1002,9 +1002,6 @@ def can_handle(self, user: db.Entity) -> bool: """ return bool(user.get("otp_app_secret", "")) - @classmethod - def get2FactorMethodName(*args, **kwargs) -> str: - return "X-VIUR-2FACTOR-AuthenticatorOTP" @classmethod def patch_user_skel(cls, skel_cls): @@ -1394,9 +1391,10 @@ def logout(self, *args, **kwargs): @exposed def login(self, *args, **kwargs): - authMethods = [(x.getAuthMethodName(), y.get2FactorMethodName() if y else None) - for x, y in self.validAuthenticationMethods] - return self.render.loginChoices(authMethods) + return self.render.loginChoices([ + (primary.METHOD_NAME, secondary.METHOD_NAME if secondary else None) + for primary, secondary in self.validAuthenticationMethods + ]) def onLogin(self, skel: skeleton.SkeletonInstance): """ @@ -1474,10 +1472,14 @@ def edit(self, key: db.Key | int | str = "self", *args, **kwargs): @exposed def getAuthMethods(self, *args, **kwargs): """Inform tools like Viur-Admin which authentication to use""" - res = [] - - for auth, secondFactor in self.validAuthenticationMethods: - res.append([auth.getAuthMethodName(), secondFactor.get2FactorMethodName() if secondFactor else None]) + # FIXME: This is the same code as in index()... + # FIXME: VIUR4: The entire function should be removed! + logging.warning("DEPRECATED!!! Use of 'User.getAuthMethods' is deprecated! Use 'User.login'-method instead!") + + res = [ + (primary.METHOD_NAME, secondary.METHOD_NAME if secondary else None) + for primary, secondary in self.validAuthenticationMethods + ] return json.dumps(res) From e2d74a029f6f0a2a8f1ab235cd651a0c4146c676 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Thu, 29 Feb 2024 16:23:50 +0100 Subject: [PATCH 22/26] refactor: Remove `extjson`, fallback to `json` (#1084) This pull request was separated from #1072 --- src/viur/core/bones/record.py | 18 ++++++++---------- src/viur/core/bones/relational.py | 15 ++++----------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/viur/core/bones/record.py b/src/viur/core/bones/record.py index b9038fe8d..1cc5bec9b 100644 --- a/src/viur/core/bones/record.py +++ b/src/viur/core/bones/record.py @@ -1,12 +1,6 @@ - +import json from viur.core.bones.base import BaseBone, ReadFromClientError, ReadFromClientErrorSeverity -try: - import extjson -except ImportError: - # FIXME: That json will not read datetime objects - import json as extjson - class RecordBone(BaseBone): """ @@ -53,16 +47,20 @@ def singleValueUnserialize(self, val): """ if isinstance(val, str): try: - value = extjson.loads(val) - except: + value = json.loads(val) + except ValueError: value = None else: value = val + if not value: return None - elif isinstance(value, list) and value: + + if isinstance(value, list) and value: value = value[0] + assert isinstance(value, dict), f"Read something from the datastore thats not a dict: {type(value)}" + usingSkel = self.using() usingSkel.unserialize(value) return usingSkel diff --git a/src/viur/core/bones/relational.py b/src/viur/core/bones/relational.py index 1800e5e1f..7f2dfa01d 100644 --- a/src/viur/core/bones/relational.py +++ b/src/viur/core/bones/relational.py @@ -2,23 +2,16 @@ This module contains the RelationalBone to create and manage relationships between skeletons and enums to parameterize it. """ +import json import logging +import typing as t import warnings from enum import Enum -import typing as t - from itertools import chain from time import time - from viur.core import db, utils from viur.core.bones.base import BaseBone, ReadFromClientError, ReadFromClientErrorSeverity, getSystemInitialized -try: - import extjson -except ImportError: - # FIXME: That json will not read datetime objects - import json as extjson - class RelationalConsistency(Enum): """ @@ -363,14 +356,14 @@ def fixFromDictToEntry(inDict): if isinstance(val, str): # ViUR2 compatibility try: - value = extjson.loads(val) + value = json.loads(val) if isinstance(value, list): value = [fixFromDictToEntry(x) for x in value] elif isinstance(value, dict): value = fixFromDictToEntry(value) else: value = None - except: + except ValueError: value = None else: value = val From b871e6c2a4f108fa4b51822f03d2e31d71ef4256 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Thu, 29 Feb 2024 17:55:35 +0100 Subject: [PATCH 23/26] feat: `utils.json` module / improving `JsonBone` (#1072) Proposal to fix #1064. I'm still a little unhappy with this, as it bites with #1000, but after all it's a different use case. Maybe we can bring both together in some way. --------- Co-authored-by: Sven Eberth --- src/viur/core/bones/json.py | 29 ++++++++----- src/viur/core/tasks.py | 56 ++---------------------- src/viur/core/utils/__init__.py | 2 +- src/viur/core/utils/json.py | 75 +++++++++++++++++++++++++++++++++ tests/main.py | 2 + tests/test_utils.py | 44 +++++++++++++++++++ 6 files changed, 144 insertions(+), 64 deletions(-) create mode 100644 src/viur/core/utils/json.py diff --git a/src/viur/core/bones/json.py b/src/viur/core/bones/json.py index 4aeae0458..621f984ab 100644 --- a/src/viur/core/bones/json.py +++ b/src/viur/core/bones/json.py @@ -1,11 +1,10 @@ import ast import json -import typing as t - import jsonschema - +import typing as t from viur.core.bones.base import ReadFromClientError, ReadFromClientErrorSeverity from viur.core.bones.raw import RawBone +from viur.core import utils class JsonBone(RawBone): @@ -23,9 +22,14 @@ class JsonBone(RawBone): type = "raw.json" - def __init__(self, indexed: bool = False, multiple: bool = False, languages: bool = None, schema: t.Mapping = {}, - *args, - **kwargs): + def __init__( + self, + indexed: bool = False, + multiple: bool = False, + languages: bool = None, + schema: t.Mapping = {}, + *args, **kwargs + ): super().__init__(*args, **kwargs) assert not multiple assert not languages @@ -36,7 +40,7 @@ def __init__(self, indexed: bool = False, multiple: bool = False, languages: boo def serialize(self, skel: 'SkeletonInstance', name: str, parentIndexed: bool) -> bool: if name in skel.accessedValues: - skel.dbEntity[name] = json.dumps(skel.accessedValues[name]) + skel.dbEntity[name] = utils.json.dumps(skel.accessedValues[name]) # Ensure this bone is NOT indexed! skel.dbEntity.exclude_from_indexes.add(name) @@ -47,7 +51,7 @@ def serialize(self, skel: 'SkeletonInstance', name: str, parentIndexed: bool) -> def unserialize(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str) -> bool: if data := skel.dbEntity.get(name): - skel.accessedValues[name] = json.loads(data) + skel.accessedValues[name] = utils.json.loads(data) return True return False @@ -59,7 +63,7 @@ def singleValueFromClient(self, value: str | list | dict, skel, bone_name, clien # Try to parse a JSON string try: - value = json.loads(value) + value = utils.json.loads(value) except json.decoder.JSONDecodeError as e: # Try to parse a Python dict as fallback @@ -76,8 +80,11 @@ def singleValueFromClient(self, value: str | list | dict, skel, bone_name, clien jsonschema.validate(value, self.schema) except (jsonschema.exceptions.ValidationError, jsonschema.exceptions.SchemaError) as e: return self.getEmptyValue(), [ - ReadFromClientError(ReadFromClientErrorSeverity.Invalid, - f"Invalid JSON for schema supplied: {e!s}")] + ReadFromClientError( + ReadFromClientErrorSeverity.Invalid, + f"Invalid JSON for schema supplied: {e!s}") + ] + return super().singleValueFromClient(value, skel, bone_name, client_data) def structure(self) -> dict: diff --git a/src/viur/core/tasks.py b/src/viur/core/tasks.py index a3d705978..9e0e518e6 100644 --- a/src/viur/core/tasks.py +++ b/src/viur/core/tasks.py @@ -1,12 +1,10 @@ import abc -import base64 import datetime import functools import grpc import json import logging import os -import pytz import requests import sys import time @@ -43,52 +41,6 @@ def restore(self, obj: CUSTOM_OBJ) -> None: ... -def _preprocess_json_object(obj): - """ - Add support for db.Key, datetime, bytes and db.Entity in deferred tasks, - and converts the provided obj into a special dict with JSON-serializable values. - """ - if isinstance(obj, db.Key): - return {".__key__": db.encodeKey(obj)} - elif isinstance(obj, datetime.datetime): - return {".__datetime__": obj.astimezone(pytz.UTC).isoformat()} - elif isinstance(obj, bytes): - return {".__bytes__": base64.b64encode(obj).decode("ASCII")} - elif isinstance(obj, db.Entity): - # TODO: Support Skeleton instances as well? - return { - ".__entity__": _preprocess_json_object(dict(obj)), - ".__ekey__": db.encodeKey(obj.key) if obj.key else None - } - elif isinstance(obj, dict): - return {_preprocess_json_object(k): _preprocess_json_object(v) for k, v in obj.items()} - elif isinstance(obj, (list, tuple, set)): - return [_preprocess_json_object(x) for x in obj] - - return obj - - -def _decode_object_hook(obj): - """ - Inverse for _preprocess_json_object, which is an object-hook for json.loads. - Check if the object matches a custom ViUR type and recreate it accordingly. - """ - if len(obj) == 1: - if key := obj.get(".__key__"): - return db.Key.from_legacy_urlsafe(key) - elif date := obj.get(".__datetime__"): - return datetime.datetime.fromisoformat(date) - elif buf := obj.get(".__bytes__"): - return base64.b64decode(buf) - - elif len(obj) == 2 and ".__entity__" in obj and ".__ekey__" in obj: - entity = db.Entity(db.Key.from_legacy_urlsafe(obj[".__ekey__"]) if obj[".__ekey__"] else None) - entity.update(obj[".__entity__"]) - return entity - - return obj - - _gaeApp = os.environ.get("GAE_APPLICATION") queueRegion = None @@ -221,7 +173,7 @@ def queryIter(self, *args, **kwargs): """ req = current.request.get().request self._validate_request() - data = json.loads(req.body, object_hook=_decode_object_hook) + data = utils.json.loads(req.body) if data["classID"] not in MetaQueryIter._classCache: logging.error(f"""Could not continue queryIter - {data["classID"]} not known on this instance""") MetaQueryIter._classCache[data["classID"]]._qryStep(data) @@ -242,7 +194,7 @@ def deferred(self, *args, **kwargs): f"""Task {req.headers.get("X-Appengine-Taskname", "")} is retried for the {retryCount}th time.""" ) - cmd, data = json.loads(req.body, object_hook=_decode_object_hook) + cmd, data = utils.json.loads(req.body) funcPath, args, kwargs, env = data logging.debug(f"Call task {funcPath} with {cmd=} {args=} {kwargs=} {env=}") @@ -612,7 +564,7 @@ def task(): # Create task description task = tasks_v2.Task( app_engine_http_request=tasks_v2.AppEngineHttpRequest( - body=json.dumps(_preprocess_json_object((command, (funcPath, args, kwargs, env)))).encode("UTF-8"), + body=utils.json.dumps((command, (funcPath, args, kwargs, env))).encode(), http_method=tasks_v2.HttpMethod.POST, relative_uri=taskargs["url"], app_engine_routing=tasks_v2.AppEngineRouting( @@ -787,7 +739,7 @@ def _requeueStep(cls, qryDict: dict[str, t.Any]) -> None: parent=taskClient.queue_path(conf.instance.project_id, queueRegion, cls.queueName), task=tasks_v2.Task( app_engine_http_request=tasks_v2.AppEngineHttpRequest( - body=json.dumps(_preprocess_json_object(qryDict)).encode("UTF-8"), + body=utils.json.dumps(qryDict).encode(), http_method=tasks_v2.HttpMethod.POST, relative_uri="/_tasks/queryIter", app_engine_routing=tasks_v2.AppEngineRouting( diff --git a/src/viur/core/utils/__init__.py b/src/viur/core/utils/__init__.py index 0fff028b0..834ef12c6 100644 --- a/src/viur/core/utils/__init__.py +++ b/src/viur/core/utils/__init__.py @@ -9,7 +9,7 @@ from urllib.parse import quote from viur.core import current, db from viur.core.config import conf -from . import string, parse +from . import string, parse, json def utcNow() -> datetime: diff --git a/src/viur/core/utils/json.py b/src/viur/core/utils/json.py new file mode 100644 index 000000000..aec0aafe8 --- /dev/null +++ b/src/viur/core/utils/json.py @@ -0,0 +1,75 @@ +import base64 +import datetime +import json +import pytz +import typing as t +from viur.core import db + + +class ViURJsonEncoder(json.JSONEncoder): + """ + Adds support for db.Key, db.Entity, datetime, bytes and and converts the provided obj + into a special dict with JSON-serializable values. + """ + def default(self, obj: t.Any) -> t.Any: + if isinstance(obj, bytes): + return {".__bytes__": base64.b64encode(obj).decode("ASCII")} + elif isinstance(obj, datetime.datetime): + return {".__datetime__": obj.astimezone(pytz.UTC).isoformat()} + elif isinstance(obj, datetime.timedelta): + return {".__timedelta__": obj / datetime.timedelta(microseconds=1)} + elif isinstance(obj, set): + return {".__set__": list(obj)} + elif hasattr(obj, "__iter__"): + return tuple(obj) + # cannot be tested in tests... + elif isinstance(obj, db.Key): + return {".__key__": db.encodeKey(obj)} + elif isinstance(obj, db.Entity): + # TODO: Handle SkeletonInstance as well? + return { + ".__entity__": dict(obj), + ".__key__": db.encodeKey(obj.key) if obj.key else None + } + + return super().default(obj) + + +def dumps(obj: t.Any, *, cls=ViURJsonEncoder, **kwargs) -> str: + """ + Wrapper for json.dumps() which converts additional ViUR datatypes. + """ + return json.dumps(obj, cls=cls, **kwargs) + + +def _decode_object_hook(obj: t.Any): + """ + Inverse for _preprocess_json_object, which is an object-hook for json.loads. + Check if the object matches a custom ViUR type and recreate it accordingly. + """ + if len(obj) == 1: + if buf := obj.get(".__bytes__"): + return base64.b64decode(buf) + elif date := obj.get(".__datetime__"): + return datetime.datetime.fromisoformat(date) + elif microseconds := obj.get(".__timedelta__"): + return datetime.timedelta(microseconds=microseconds) + elif key := obj.get(".__key__"): + return db.Key.from_legacy_urlsafe(key) + elif items := obj.get(".__set__"): + return set(items) + + elif len(obj) == 2 and all(k in obj for k in (".__entity__", ".__key__")): + # TODO: Handle SkeletonInstance as well? + entity = db.Entity(db.Key.from_legacy_urlsafe(obj[".__key__"]) if obj[".__key__"] else None) + entity.update(obj[".__entity__"]) + return entity + + return obj + + +def loads(s: str, *, object_hook=_decode_object_hook, **kwargs) -> t.Any: + """ + Wrapper for json.loads() which recreates additional ViUR datatypes. + """ + return json.loads(s, object_hook=object_hook, **kwargs) diff --git a/tests/main.py b/tests/main.py index 8befef005..f74581ef6 100755 --- a/tests/main.py +++ b/tests/main.py @@ -85,8 +85,10 @@ def __init__(self, *args, **kwargs): ) viur_datastore = mock.Mock() + for attr in db_attr: setattr(viur_datastore, attr, mock.MagicMock()) + viur_datastore.config = {} sys.modules["viur.datastore"] = viur_datastore diff --git a/tests/test_utils.py b/tests/test_utils.py index c17ce60e3..4267f1925 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -25,3 +25,47 @@ def test_string_escape(self): self.assertEqual("abcde", utils.string.escape("abcdefghi", max_length=5)) self.assertEqual("<html> &</html>", utils.string.escape("\n&\0")) self.assertEqual(utils.string.escape(S), E) + + def test_json(self): + from viur.core import utils, db + import datetime + + # key = db.Key("test", "hello world") + now = datetime.datetime.fromisoformat("2024-02-28T14:43:17.125207+00:00") + duration = datetime.timedelta(minutes=13, microseconds=37) + + example = { + "datetime": now, + "false": False, + "float": 42.5, + "generator": (x for x in "Hello"), + "int": 1337, + # "key": key, # cannot use in tests + "list": [1, 2, 3], + "none": None, + "set": {1, 2, 3}, + "str": "World", + "timedelta": duration, + "true": True, + "tuple": (1, 2, 3), + } + + # serialize example into string + s = utils.json.dumps(example) + + # check if string is as expected + self.assertEqual( + s, + """{"datetime": {".__datetime__": "2024-02-28T14:43:17.125207+00:00"}, "false": false, "float": 42.5, "generator": ["H", "e", "l", "l", "o"], "int": 1337, "list": [1, 2, 3], "none": null, "set": {".__set__": [1, 2, 3]}, "str": "World", "timedelta": {".__timedelta__": 780000037.0}, "true": true, "tuple": [1, 2, 3]}""", # noqa + ) + + # deserialize string into object again + o = utils.json.loads(s) + + # patch tuple as a list + example["tuple"] = list(example["tuple"]) + example["generator"] = [x for x in "Hello"] + + # self.assertEqual(example, o) + for k, v in example.items(): + self.assertEqual(o[k], v) From b2a868fabbe77aa55e9311c5c097dd551a4267cd Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Thu, 29 Feb 2024 20:19:37 +0100 Subject: [PATCH 24/26] refactor: Replace securitykey duration with `timedelta` (#1083) Since #1078 we can use datetime.timedelta as duration. So we shoud use it. --- src/viur/core/modules/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index f6b2d26dd..4dfef905c 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -392,7 +392,7 @@ def pwrecover(self, recovery_key: str | None = None, skey: str | None = None, *a self.passwordRecoveryRateLimit.decrementQuota() recovery_key = securitykey.create( - duration=15 * 60, + duration=datetime.timedelta(minutes=15), key_length=conf.security.password_recovery_key_length, user_name=skel["name"].lower(), session_bound=False, @@ -563,7 +563,7 @@ def add(self, *args, **kwargs): skel.toDB() if self.registrationEmailVerificationRequired and skel["status"] == Status.WAITING_FOR_EMAIL_VERIFICATION: # The user will have to verify his email-address. Create a skey and send it to his address - skey = securitykey.create(duration=60 * 60 * 24 * 7, session_bound=False, + skey = securitykey.create(duration=datetime.timedelta(days=7), session_bound=False, user_key=utils.normalizeKey(skel["key"]), name=skel["name"]) skel.skey = BaseBone(descr="Skey") From 882d329af58a69c6d076d0200ba7400ecbe6edfb Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Thu, 29 Feb 2024 20:21:32 +0100 Subject: [PATCH 25/26] feat: `UserPrimaryAuthentication.next_or_finish` handler (#997) - Generic replacement for #987 (yes, breaking, but used by one project!) - Refactored UserPassword.login() and GoogleAccount.login() to use UserSkel - Improved code for PEP-8 compliance --------- Co-authored-by: agudermann <47318461+ArneGudermann@users.noreply.github.com> --- src/viur/core/modules/user.py | 160 +++++++++++++++++----------------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/src/viur/core/modules/user.py b/src/viur/core/modules/user.py index 4dfef905c..859bab6e8 100644 --- a/src/viur/core/modules/user.py +++ b/src/viur/core/modules/user.py @@ -189,7 +189,7 @@ def __init__(self, moduleName, modulePath, userModule): super().__init__(moduleName, modulePath) self._user_module = userModule - def can_handle(self, user: db.Entity) -> bool: + def can_handle(self, skel: skeleton.SkeletonInstance) -> bool: return True @classmethod @@ -207,9 +207,17 @@ class UserPrimaryAuthentication(UserAuthentication, abc.ABC): registrationEnabled = False @abc.abstractmethod - def login(self, *args, kwargs): + def login(self, *args, **kwargs): ... + def next_or_finish(self, skel: skeleton.SkeletonInstance): + """ + Hook that is called whenever a part of the authentication was successful. + It allows to perform further steps in custom authentications, + e.g. change a password after first login. + """ + return self._user_module.continueAuthenticationFlow(self, skel["key"]) + class UserPassword(UserPrimaryAuthentication): METHOD_NAME = "X-VIUR-AUTH-User-Password" @@ -306,56 +314,43 @@ def login(self, *, name: str | None = None, password: str | None = None, **kwarg self.loginRateLimit.assertQuotaIsAvailable() + # query for the username. The query might find another user, but the name is being checked for equality below name = name.lower().strip() - query = db.Query(self._user_module.viewSkel().kindName) - user_entry = query.filter("name.idx >=", name).getEntry() or {} # might find another user; always keep a dict - - password_data = user_entry.get("password") or {} - # old password hashes used 1001 iterations - iterations = password_data.get("iterations", 1001) - passwd = encode_password(password, password_data.get("salt", "-invalid-"), iterations)["pwhash"] - - # Check if the username matches - stored_user_name = (user_entry.get("name") or {}).get("idx") or "" - is_okay = secrets.compare_digest(stored_user_name, name) + user_skel = self._user_module.baseSkel() + user_skel = user_skel.all().filter("name.idx >=", name).getSkel() or user_skel - # Check if the password matches - stored_password_hash = password_data.get("pwhash", b"-invalid-") - is_okay &= secrets.compare_digest(stored_password_hash, passwd) + # extract password hash from raw database entity (skeleton access blocks it) + password_data = (user_skel.dbEntity and user_skel.dbEntity.get("password")) or {} + iterations = password_data.get("iterations", 1001) # remember iterations; old password hashes used 1001 + password_hash = encode_password(password, password_data.get("salt", "-invalid-"), iterations)["pwhash"] - status = None + # now check if the username matches + is_okay = secrets.compare_digest((user_skel["name"] or "").lower().strip(), name) - # Verify that this account isn't blocked - if (user_entry.get("status") or 0) < Status.ACTIVE.value: - if is_okay: - # The username and password is valid, in this case we can inform that user about his account status - # (ie account locked or email verification pending) - status = user_entry["status"] + # next, check if the password hash matches + is_okay &= secrets.compare_digest(password_data.get("pwhash", b"-invalid-"), password_hash) - is_okay = False + # next, check if the user account is active + is_okay &= (user_skel["status"] or 0) >= Status.ACTIVE.value if not is_okay: self.loginRateLimit.decrementQuota() # Only failed login attempts will count to the quota skel = self.LoginSkel() - return self._user_module.render.login(skel, loginFailed=True, accountStatus=status) + return self._user_module.render.login( + skel, + loginFailed=True, # FIXME: Is this still being used? + accountStatus=user_skel["status"] # FIXME: Is this still being used? + ) + # check if iterations are below current security standards, and update if necessary. if iterations < PBKDF2_DEFAULT_ITERATIONS: logging.info(f"Update password hash for user {name}.") # re-hash the password with more iterations - skel = self._user_module.editSkel() - skel.setEntity(user_entry) - skel["key"] = user_entry.key - skel["password"] = password # will be hashed on serialize - skel.toDB(update_relations=False) - - return self.on_login(user_entry) + # FIXME: This must be done within a transaction! + user_skel["password"] = password # will be hashed on serialize + user_skel.toDB(update_relations=False) - def on_login(self, user_entry: db.Entity): - """ - Hook that is called whenever the password authentication was successful. - It allows to perform further steps in custom UserPassword authentications. - """ - return self._user_module.continueAuthenticationFlow(self, user_entry.key) + return self.next_or_finish(user_skel) @exposed def pwrecover(self, recovery_key: str | None = None, skey: str | None = None, *args, **kwargs): @@ -610,65 +605,68 @@ def login(self, token: str | None = None, *args, **kwargs): # FIXME: Check if already logged in if not conf.user.google_client_id: raise errors.PreconditionFailed("Please configure conf.user.google_client_id!") + if not token: request = current.request.get() request.response.headers["Content-Type"] = "text/html" if request.response.headers.get("cross-origin-opener-policy") == "same-origin": # We have to allow popups here request.response.headers["cross-origin-opener-policy"] = "same-origin-allow-popups" - # Fixme: Render with Jinja2? - with (conf.instance.core_base_path - .joinpath("viur/core/template/vi_user_google_login.html") - .open() as tpl_file): - tplStr = tpl_file.read() - tplStr = tplStr.replace("{{ clientID }}", conf.user.google_client_id) - extendCsp({"script-src": ["sha256-JpzaUIxV/gVOQhKoDLerccwqDDIVsdn1JclA6kRNkLw="], - "style-src": ["sha256-FQpGSicYMVC5jxKGS5sIEzrRjSJmkxKPaetUc7eamqc="]}) - return tplStr - userInfo = id_token.verify_oauth2_token(token, requests.Request(), conf.user.google_client_id) - if userInfo['iss'] not in {'accounts.google.com', 'https://accounts.google.com'}: - raise ValueError('Wrong issuer.') - # Token looks valid :) - uid = userInfo['sub'] - email = userInfo['email'] - # fixme: use self._user_module.baseSkel() for this later - addSkel = skeleton.skeletonByKind(self._user_module.addSkel().kindName) # Ensure that we have the full skeleton + file_path = conf.instance.core_base_path.joinpath("viur/core/template/vi_user_google_login.html") + with open(file_path) as file: + tpl_string = file.read() + + # FIXME: Use Jinja2 for rendering? + tpl_string = tpl_string.replace("{{ clientID }}", conf.user.google_client_id) + extendCsp({ + "script-src": ["sha256-JpzaUIxV/gVOQhKoDLerccwqDDIVsdn1JclA6kRNkLw="], + "style-src": ["sha256-FQpGSicYMVC5jxKGS5sIEzrRjSJmkxKPaetUc7eamqc="] + }) + return tpl_string + + user_info = id_token.verify_oauth2_token(token, requests.Request(), conf.user.google_client_id) + if user_info["iss"] not in {"accounts.google.com", "https://accounts.google.com"}: + raise ValueError("Invalid issuer") + # Token looks valid :) + uid = user_info["sub"] + email = user_info["email"] + + base_skel = self._user_module.baseSkel() update = False - if not (userSkel := addSkel().all().filter("uid =", uid).getSkel()): + if not (user_skel := base_skel.all().filter("uid =", uid).getSkel()): # We'll try again - checking if there's already an user with that email - if not (userSkel := addSkel().all().filter("name.idx =", email.lower()).getSkel()): + if not (user_skel := base_skel.all().filter("name.idx =", email.lower()).getSkel()): # Still no luck - it's a completely new user if not self.registrationEnabled: - if (domain := userInfo.get("hd")) and domain in conf.user.google_gsuite_domains: + if (domain := user_info.get("hd")) and domain in conf.user.google_gsuite_domains: logging.debug(f"Google user is from allowed {domain} - adding account") else: logging.debug(f"Google user is from {domain} - denying registration") raise errors.Forbidden("Registration for new users is disabled") - userSkel = addSkel() # We'll add a new user - - userSkel["uid"] = uid - userSkel["name"] = email - update = True + user_skel = base_skel + user_skel["uid"] = uid + user_skel["name"] = email + update = True # Take user information from Google, if wanted! - if userSkel["sync"]: + if user_skel["sync"]: for target, source in { "name": email, - "firstname": userInfo.get("given_name"), - "lastname": userInfo.get("family_name"), + "firstname": user_info.get("given_name"), + "lastname": user_info.get("family_name"), }.items(): - if userSkel[target] != source: - userSkel[target] = source + if user_skel[target] != source: + user_skel[target] = source update = True if update: - assert userSkel.toDB() + assert user_skel.toDB() - return self._user_module.continueAuthenticationFlow(self, userSkel["key"]) + return self.next_or_finish(user_skel) class UserSecondFactorAuthentication(UserAuthentication, abc.ABC): @@ -756,22 +754,22 @@ def patch_user_skel(cls, skel_cls): } ) - def get_config(self, user: db.Entity) -> OtpConfig | None: + def get_config(self, skel: skeleton.SkeletonInstance) -> OtpConfig | None: """ Returns an instance of self.OtpConfig with a provided token configuration, or None when there is no appropriate configuration of this second factor handler available. """ - if user.get("otp_secret"): - return self.OtpConfig(secret=user["otp_secret"], timedrift=user.get("otp_timedrift") or 0) + if otp_secret := skel.dbEntity.get("otp_secret"): + return self.OtpConfig(secret=otp_secret, timedrift=skel.dbEntity.get("otp_timedrift") or 0) return None - def can_handle(self, user: db.Entity) -> bool: + def can_handle(self, skel: skeleton.SkeletonInstance) -> bool: """ Specified whether the second factor authentication can be handled by the given user or not. """ - return bool(self.get_config(user)) + return bool(self.get_config(skel)) @exposed def start(self): @@ -996,11 +994,11 @@ def add(self, otp=None): name=i18n.translate(self.NAME), ) - def can_handle(self, user: db.Entity) -> bool: + def can_handle(self, skel: skeleton.SkeletonInstance) -> bool: """ We can only handle the second factor if we have stored an otp_app_secret before. """ - return bool(user.get("otp_app_secret", "")) + return bool(skel.dbEntity.get("otp_app_secret", "")) @classmethod @@ -1283,10 +1281,12 @@ def continueAuthenticationFlow(self, provider: UserPrimaryAuthentication, user_k """ Continue authentication flow when primary authentication succeeded. """ - if not (possible_user := db.Get(user_key)): + skel = self.baseSkel() + + if not skel.fromDB(user_key): raise errors.NotFound("User was not found.") - if not provider.can_handle(possible_user): + if not provider.can_handle(skel): raise errors.Forbidden("User is not allowed to use this primary login method.") session = current.session.get() @@ -1300,7 +1300,7 @@ def continueAuthenticationFlow(self, provider: UserPrimaryAuthentication, user_k if isinstance(provider, auth_provider): if second_factor is not None: second_factor_provider_instance = self.secondFactorProviderByClass(second_factor) - if second_factor_provider_instance.can_handle(possible_user): + if second_factor_provider_instance.can_handle(skel): second_factor_providers.append(second_factor_provider_instance) else: second_factor_providers.append(None) From a03620d2f8bf0ffd0762f621232e4b58653f8f82 Mon Sep 17 00:00:00 2001 From: agudermann <47318461+ArneGudermann@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:45:32 +0100 Subject: [PATCH 26/26] feat: Add closed system (#1085) Resolves #879 --------- Co-authored-by: Jan Max Meyer Co-authored-by: Sven Eberth --- src/viur/core/config.py | 20 ++++++++++++++++++++ src/viur/core/request.py | 9 ++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/viur/core/config.py b/src/viur/core/config.py index d468d1dab..cc72eaeeb 100644 --- a/src/viur/core/config.py +++ b/src/viur/core/config.py @@ -397,6 +397,26 @@ class Security(ConfigType): password_recovery_key_length: int = 42 """Length of the Password recovery key""" + closed_system: bool = False + """If `True` it activates a mode in which only authenticated users can access all routes.""" + + closed_system_allowed_paths: t.Iterable[str] = [ + "", # index site + "json/skey", + "json/user/auth_*", + "json/user/getAuthMethods", + "json/user/login", + "user/auth_*", + "user/getAuthMethods", + "user/login", + "vi", + "vi/settings", + "vi/skey", + "vi/user/auth_*", + "vi/user/getAuthMethods", + "vi/user/login", + ] + """List of URLs that are accessible without authentication in a closed system""" _mapping = { "contentSecurityPolicy": "content_security_policy", diff --git a/src/viur/core/request.py b/src/viur/core/request.py index 82e19320c..19e49d4cb 100644 --- a/src/viur/core/request.py +++ b/src/viur/core/request.py @@ -5,7 +5,7 @@ Additionally, this module defines the RequestValidator interface which provides a very early hook into the request processing (useful for global ratelimiting, DDoS prevention or access control). """ - +import fnmatch import json import logging import os @@ -292,6 +292,13 @@ def _process(self): current.user.set(user_mod.getCurrentUser()) path = self._select_language(path)[1:] + + # Check for closed system + if conf.security.closed_system: + if not current.user.get(): + if not any(fnmatch.fnmatch(path, pat) for pat in conf.security.closed_system_allowed_paths): + raise errors.Unauthorized() + if conf.request_preprocessor: path = conf.request_preprocessor(path)