From fe51d04d9551d69133ba9967fd3247914bb8ee7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20M=C3=BCller?= Date: Sat, 1 Jun 2024 11:52:44 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20Move=20subscriber=20active=20fla?= =?UTF-8?q?g=20to=20time=5Fdeleted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 1 - backend/src/appointment/database/models.py | 14 +++++-- .../appointment/database/repo/subscriber.py | 13 +++++- backend/src/appointment/database/schemas.py | 2 +- backend/src/appointment/dependencies/auth.py | 1 + .../src/appointment/exceptions/validation.py | 27 +++++++++++++ backend/src/appointment/l10n/de/main.ftl | 3 ++ backend/src/appointment/l10n/en/main.ftl | 3 ++ ...5de8f10ab87_add_subscriber_soft_delete.py} | 6 +-- backend/src/appointment/routes/auth.py | 6 +-- backend/src/appointment/routes/subscriber.py | 28 ++++++++----- .../src/views/admin/SubscriberPanelView.vue | 40 ++++++++++--------- 12 files changed, 101 insertions(+), 43 deletions(-) rename backend/src/appointment/migrations/versions/{2024_05_31_1454-d5de8f10ab87_add_subscriber_active_flag.py => 2024_05_31_1454-d5de8f10ab87_add_subscriber_soft_delete.py} (66%) diff --git a/README.md b/README.md index 47b90c689..1c71c6800 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,6 @@ Invite others to grab times on your calendar. Choose a date. Make appointments as easy as it gets. - ## Feedback and Support If you'd like to give feedback or need support, please see our [Topicbox](https://thunderbird.topicbox.com/groups/services). diff --git a/backend/src/appointment/database/models.py b/backend/src/appointment/database/models.py index 334e954b4..5727ce4e0 100644 --- a/backend/src/appointment/database/models.py +++ b/backend/src/appointment/database/models.py @@ -98,7 +98,17 @@ def get_columns(self) -> list: time_updated = Column(DateTime, server_default=func.now(), default=func.now(), onupdate=func.now(), index=True) -class Subscriber(Base): +class HasSoftDelete: + """Mixing in a column to support deletion without removing the record""" + time_deleted = Column(DateTime, nullable=True) + + @property + def is_deleted(self): + """A record is marked deleted if a delete time is set.""" + return self.time_deleted is not None + + +class Subscriber(HasSoftDelete, Base): __tablename__ = "subscribers" id = Column(Integer, primary_key=True, index=True) @@ -106,8 +116,6 @@ class Subscriber(Base): # Encrypted (here) and hashed (by the associated hashing functions in routes/auth) password = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), index=False) - active: bool = Column(Boolean, index=True, default=True) - # Use subscriber.preferred_email for any email, or other user-facing presence. email = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), unique=True, index=True) secondary_email = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), nullable=True, index=True) diff --git a/backend/src/appointment/database/repo/subscriber.py b/backend/src/appointment/database/repo/subscriber.py index 854d0d01a..516eb02e9 100644 --- a/backend/src/appointment/database/repo/subscriber.py +++ b/backend/src/appointment/database/repo/subscriber.py @@ -4,6 +4,7 @@ """ import re +import datetime from sqlalchemy.orm import Session from .. import models, schemas @@ -73,7 +74,11 @@ def update(db: Session, data: schemas.SubscriberIn, subscriber_id: int): def disable(db: Session, subscriber: models.Subscriber): """Disable a given subscriber""" - subscriber.active = False + # mark subscriber deleted = disable them + subscriber.time_deleted = datetime.datetime.now(datetime.UTC) + # invalidate session + # subscriber.minimum_valid_iat_time = datetime.datetime.now(datetime.UTC) + db.add(subscriber) db.commit() db.refresh(subscriber) return subscriber @@ -81,7 +86,11 @@ def disable(db: Session, subscriber: models.Subscriber): def enable(db: Session, subscriber: models.Subscriber): """Enable a given subscriber""" - subscriber.active = True + # mark subscriber deleted = disable them + subscriber.time_deleted = None + # refresh session + # subscriber.minimum_valid_iat_time = datetime.datetime.now(datetime.UTC) + db.add(subscriber) db.commit() db.refresh(subscriber) return subscriber diff --git a/backend/src/appointment/database/schemas.py b/backend/src/appointment/database/schemas.py index 3ddff028c..9092423b2 100644 --- a/backend/src/appointment/database/schemas.py +++ b/backend/src/appointment/database/schemas.py @@ -269,9 +269,9 @@ class Config: class SubscriberAdminOut(Subscriber): - active: bool | None = True invite: Invite | None = None time_created: datetime + time_deleted: datetime | None class Config: from_attributes = True diff --git a/backend/src/appointment/dependencies/auth.py b/backend/src/appointment/dependencies/auth.py index f9636bdd5..f3fa13657 100644 --- a/backend/src/appointment/dependencies/auth.py +++ b/backend/src/appointment/dependencies/auth.py @@ -32,6 +32,7 @@ def get_user_from_token(db, token: str): # Token has been expired by us - temp measure to avoid spinning a refresh system, or a deny list for this issue if any([ subscriber is None, + subscriber.is_deleted, subscriber and subscriber.minimum_valid_iat_time and not iat, subscriber and subscriber.minimum_valid_iat_time and subscriber.minimum_valid_iat_time.timestamp() > int(iat) ]): diff --git a/backend/src/appointment/exceptions/validation.py b/backend/src/appointment/exceptions/validation.py index 74a6bf066..b7cac0361 100644 --- a/backend/src/appointment/exceptions/validation.py +++ b/backend/src/appointment/exceptions/validation.py @@ -213,3 +213,30 @@ class CreateSubscriberAlreadyExistsException(APIException): def get_msg(self): return l10n('subscriber-already-exists') + + +class SubscriberAlreadyDeletedException(APIException): + """Raise when a subscriber failed to be marked deleted because they already are""" + id_code = 'SUBSCRIBER_ALREADY_DELETED' + status_code = 400 + + def get_msg(self): + return l10n('subscriber-already-deleted') + + +class SubscriberAlreadyEnabledException(APIException): + """Raise when a subscriber failed to be marked undeleted because they already are""" + id_code = 'SUBSCRIBER_ALREADY_ENABLED' + status_code = 400 + + def get_msg(self): + return l10n('subscriber-already-enabled') + + +class SubscriberSelfDeleteException(APIException): + """Raise when a subscriber tries to delete themselves where not allowed""" + id_code = 'SUBSCRIBER_SELF_DELETE' + status_code = 403 + + def get_msg(self): + return l10n('subscriber-self-delete') diff --git a/backend/src/appointment/l10n/de/main.ftl b/backend/src/appointment/l10n/de/main.ftl index f454af08f..33f0be710 100644 --- a/backend/src/appointment/l10n/de/main.ftl +++ b/backend/src/appointment/l10n/de/main.ftl @@ -42,6 +42,9 @@ event-could-not-be-accepted = Es ist ein Fehler bei der Annahme der Buchungsdate failed-to-create-subscriber = Es gab einen Fehler beim Anlegen der Person. Bitte später erneut versuchen. subscriber-already-exists = Eine Person mit dieser E-Mail-Adresse existiert bereits. +subscriber-already-deleted = Die Person ist bereits gelöscht. +subscriber-already-enabled = Die Person ist bereits aktiv. +subscriber-self-delete = Die Löschung des eigenen Benutzerkontos ist hier nicht möglich. ## Authentication Exceptions diff --git a/backend/src/appointment/l10n/en/main.ftl b/backend/src/appointment/l10n/en/main.ftl index 5ffd1710a..303c72b51 100644 --- a/backend/src/appointment/l10n/en/main.ftl +++ b/backend/src/appointment/l10n/en/main.ftl @@ -42,6 +42,9 @@ event-could-not-be-accepted = There was an error accepting the booking details. failed-to-create-subscriber = There was an error creating the subscriber. Please try again later. subscriber-already-exists = A subscriber with this email address already exists. +subscriber-already-deleted = The subscriber is already deleted. +subscriber-already-enabled = The subscriber is already enabled. +subscriber-self-delete = You are not allowed to delete yourself here. ## Authentication Exceptions diff --git a/backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_active_flag.py b/backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_soft_delete.py similarity index 66% rename from backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_active_flag.py rename to backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_soft_delete.py index 7471f348c..33da7c215 100644 --- a/backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_active_flag.py +++ b/backend/src/appointment/migrations/versions/2024_05_31_1454-d5de8f10ab87_add_subscriber_soft_delete.py @@ -1,4 +1,4 @@ -"""add subscriber active flag +"""add subscriber soft delete Revision ID: d5de8f10ab87 Revises: 9fe08ba6f2ed @@ -17,8 +17,8 @@ def upgrade() -> None: - op.add_column('subscribers', sa.Column('active', sa.Boolean, index=True, default=True)) + op.add_column('subscribers', sa.Column('time_deleted', sa.DateTime, nullable=True)) def downgrade() -> None: - op.drop_column('subscribers', 'active') + op.drop_column('subscribers', 'time_deleted') diff --git a/backend/src/appointment/routes/auth.py b/backend/src/appointment/routes/auth.py index d0164a619..6039c885f 100644 --- a/backend/src/appointment/routes/auth.py +++ b/backend/src/appointment/routes/auth.py @@ -153,7 +153,7 @@ def fxa_callback( subscriber = fxa_subscriber # Only proceed if user account is enabled (which is the default case for new users) - if not subscriber.active: + if subscriber.is_deleted: raise HTTPException(status_code=403, detail=l10n('disabled-account')) fxa_connections = repo.external_connection.get_by_type(db, subscriber.id, ExternalConnectionType.fxa) @@ -222,7 +222,7 @@ def token( raise HTTPException(status_code=403, detail=l10n('invalid-credentials')) # Only proceed if user account is enabled - if not subscriber.active: + if subscriber.is_deleted: raise HTTPException(status_code=403, detail=l10n('disabled-account')) # Verify the incoming password, and re-hash our password if needed @@ -279,7 +279,7 @@ def me( @router.post("/permission-check") def permission_check(subscriber: Subscriber = Depends(get_admin_subscriber)): """Checks if they have admin permissions""" - return subscriber.active + return subscriber.is_deleted # @router.get('/test-create-account') diff --git a/backend/src/appointment/routes/subscriber.py b/backend/src/appointment/routes/subscriber.py index fe0580288..beac14bf4 100644 --- a/backend/src/appointment/routes/subscriber.py +++ b/backend/src/appointment/routes/subscriber.py @@ -22,22 +22,28 @@ def get_all_subscriber(db: Session = Depends(get_db), _: Subscriber = Depends(ge @router.put("/disable/{email}") -def disable_subscriber(email: str, db: Session = Depends(get_db), _: Subscriber = Depends(get_admin_subscriber)): - """endpoint to disable a subscriber by email, needs admin permissions""" - subscriber = repo.subscriber.get_by_email(db, email) - if not subscriber: +def disable_subscriber(email: str, db: Session = Depends(get_db), subscriber: Subscriber = Depends(get_admin_subscriber)): + """endpoint to mark a subscriber deleted by email, needs admin permissions""" + subscriber_to_delete = repo.subscriber.get_by_email(db, email) + if not subscriber_to_delete: raise validation.SubscriberNotFoundException() - + if subscriber_to_delete.is_deleted: + raise validation.SubscriberAlreadyDeletedException() + if subscriber.email == subscriber_to_delete.email: + raise validation.SubscriberSelfDeleteException() + # Set active flag to false on the subscribers model. - return repo.subscriber.disable(db, subscriber) + return repo.subscriber.disable(db, subscriber_to_delete) @router.put("/enable/{email}") def disable_subscriber(email: str, db: Session = Depends(get_db), _: Subscriber = Depends(get_admin_subscriber)): - """endpoint to disable a subscriber by email, needs admin permissions""" - subscriber = repo.subscriber.get_by_email(db, email) - if not subscriber: + """endpoint to enable a subscriber by email, needs admin permissions""" + subscriber_to_enable = repo.subscriber.get_by_email(db, email) + if not subscriber_to_enable: raise validation.SubscriberNotFoundException() - + if not subscriber_to_enable.is_deleted: + raise validation.SubscriberAlreadyEnabledException() + # Set active flag to true on the subscribers model. - return repo.subscriber.enable(db, subscriber) + return repo.subscriber.enable(db, subscriber_to_enable) diff --git a/frontend/src/views/admin/SubscriberPanelView.vue b/frontend/src/views/admin/SubscriberPanelView.vue index 1a72af88f..b48c6f6cd 100644 --- a/frontend/src/views/admin/SubscriberPanelView.vue +++ b/frontend/src/views/admin/SubscriberPanelView.vue @@ -23,7 +23,7 @@ :columns="columns" :filters="filters" :loading="loading" - @field-click="(_key, field) => toggleSubscriberState(field.email.value, field.active.value)" + @field-click="(_key, field) => toggleSubscriberState(field.email.value, field.timeDeleted.value === '')" >