Add minimum_valid_iat_time to subscriber table, and hook up the res…

…t of the fxa webhook events 🛢 (#222)

* URI encode the email address for fxa login (d'oh.)

* Add `minimum_valid_iat_time` to subscriber so we can easily block old access tokens.

* Don't set a minimum_valid_iat_time if the user is logging out normally.

* Fix unit test from failing due to missing l10n context.

* Add some initial fxa webhook tests, and fix a small issue with subscriber factory not setting email correctly.

* Hook up delete account fxa webhook event and add a test for it.

* Call get_profile on the fxa update-user webhook as noted in the docs.

* Wrap minimum_valid_iat_time with our standard db encryption

backend/pyproject.toml

@@ -25,6 +25,7 @@ test = [
     "Faker==20.1.0",
     "httpx==0.25.1",
     "pytest==7.4.3",
+    "freezegun==1.4.0",
 ]
 deploy = ['appointment[cli]', 'appointment[db]']
 

backend/src/appointment/controller/auth.py

@@ -6,8 +6,23 @@
 import os
 import hashlib
 import hmac
+import datetime
 
-from ..database import repo, schemas
+from sqlalchemy.orm import Session
+
+from .apis.fxa_client import FxaClient
+from ..database import repo, schemas, models
+
+
+def logout(db: Session, subscriber: models.Subscriber, fxa_client: FxaClient | None, deny_previous_tokens=True):
+    """Sets a minimum valid issued at time (time). This prevents access tokens issued earlier from working."""
+    if deny_previous_tokens:
+        subscriber.minimum_valid_iat_time = datetime.datetime.now(datetime.UTC)
+        db.add(subscriber)
+        db.commit()
+
+    if os.getenv('AUTH_SCHEME') == 'fxa':
+        fxa_client.logout()
 
 
 def sign_url(url: str):

backend/src/appointment/database/models.py

@@ -92,6 +92,9 @@ class Subscriber(Base):
     google_state_expires_at = Column(DateTime)
     short_link_hash = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), index=False)
 
+    # Only accept the times greater than the one specified in the `iat` claim of the jwt token
+    minimum_valid_iat_time = Column('minimum_valid_iat_time', StringEncryptedType(DateTime, secret, AesEngine, "pkcs5", length=255))
+
     calendars = relationship("Calendar", cascade="all,delete", back_populates="owner")
     slots = relationship("Slot", cascade="all,delete", back_populates="subscriber")
     external_connections = relationship("ExternalConnections", cascade="all,delete", back_populates="owner")

backend/src/appointment/database/repo.py

@@ -50,7 +50,7 @@ def delete_attendees_by_subscriber(db: Session, subscriber_id: int):
 """
 
 
-def get_subscriber(db: Session, subscriber_id: int):
+def get_subscriber(db: Session, subscriber_id: int) -> models.Subscriber | None:
     """retrieve subscriber by id"""
     return db.get(models.Subscriber, subscriber_id)
 
@@ -610,7 +610,7 @@ def delete_external_connections_by_type_id(db: Session, subscriber_id: int, type
     return True
 
 
-def get_external_connections_by_type(db: Session, subscriber_id: int, type: models.ExternalConnectionType, type_id: str | None):
+def get_external_connections_by_type(db: Session, subscriber_id: int, type: models.ExternalConnectionType, type_id: str | None) -> models.ExternalConnections | None:
     """Return a subscribers external connections by type, and optionally type id"""
     query = (
         db.query(models.ExternalConnections)

backend/src/appointment/dependencies/auth.py

@@ -18,13 +18,22 @@ def get_user_from_token(db, token: str):
     try:
         payload = jwt.decode(token, os.getenv('JWT_SECRET'), algorithms=[os.getenv('JWT_ALGO')])
         sub = payload.get("sub")
+        iat = payload.get("iat")
         if sub is None:
             raise InvalidTokenException()
     except JWTError:
         raise InvalidTokenException()
 
     id = sub.replace('uid-', '')
-    return repo.get_subscriber(db, int(id))
+    subscriber = repo.get_subscriber(db, int(id))
+
+    # Token has been expired by us - temp measure to avoid spinning a refresh system, or a deny list for this issue
+    if subscriber.minimum_valid_iat_time and not iat:
+        raise InvalidTokenException()
+    elif subscriber.minimum_valid_iat_time and subscriber.minimum_valid_iat_time.timestamp() > int(iat):
+        raise InvalidTokenException()
+
+    return subscriber
 
 
 def get_subscriber(

backend/src/appointment/migrations/versions/2024_01_09_1652-ad7cc2de5ff8_add_minimum_valid_iat_time_to_subscribers.py

@@ -0,0 +1,31 @@
+"""add minimum_valid_iat_time to subscribers
+
+Revision ID: ad7cc2de5ff8
+Revises: 0dc429ca07f5
+Create Date: 2024-01-09 16:52:20.941572
+
+"""
+import os
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy_utils import StringEncryptedType
+from sqlalchemy_utils.types.encrypted.encrypted_type import AesEngine
+
+
+def secret():
+    return os.getenv("DB_SECRET")
+
+
+# revision identifiers, used by Alembic.
+revision = 'ad7cc2de5ff8'
+down_revision = '0dc429ca07f5'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.add_column('subscribers', sa.Column('minimum_valid_iat_time', StringEncryptedType(sa.DateTime, secret, AesEngine, "pkcs5", length=255)))
+
+
+def downgrade() -> None:
+    op.drop_column('subscribers', 'minimum_valid_iat_time')

backend/src/appointment/routes/auth.py

@@ -19,6 +19,7 @@
 from ..dependencies.database import get_db
 from ..dependencies.auth import get_subscriber
 
+from ..controller import auth
 from ..controller.apis.fxa_client import FxaClient
 from ..dependencies.fxa import get_fxa_client
 from ..exceptions.fxa_api import NotInAllowListException
@@ -42,7 +43,10 @@ def create_access_token(data: dict, expires_delta: timedelta | None = None):
         expire = datetime.now(UTC) + expires_delta
     else:
         expire = datetime.now(UTC) + timedelta(minutes=15)
-    to_encode.update({"exp": expire})
+    to_encode.update({
+        "exp": expire,
+        "iat": int(datetime.now(UTC).timestamp())
+    })
     encoded_jwt = jwt.encode(to_encode, os.getenv('JWT_SECRET'), algorithm=os.getenv('JWT_ALGO'))
     return encoded_jwt
 
@@ -194,15 +198,14 @@ def token(
 
 
 @router.get('/logout')
-def logout(subscriber: Subscriber = Depends(get_subscriber), fxa_client: FxaClient = Depends(get_fxa_client)):
+def logout(db: Session = Depends(get_db), subscriber: Subscriber = Depends(get_subscriber), fxa_client: FxaClient = Depends(get_fxa_client)):
     """Logout a given subscriber session"""
 
-    # We don't actually have to do anything for non-fxa schemes
-    if os.getenv('AUTH_SCHEME') != 'fxa':
-        return True
+    if os.getenv('AUTH_SCHEME') == 'fxa':
+        fxa_client.setup(subscriber.id, subscriber.get_external_connection(ExternalConnectionType.fxa).token)
 
-    fxa_client.setup(subscriber.id, subscriber.get_external_connection(ExternalConnectionType.fxa).token)
-    fxa_client.logout()
+    # Don't set a minimum_valid_iat_time here.
+    auth.logout(db, subscriber, fxa_client, deny_previous_tokens=False)
 
     return True
 

backend/src/appointment/routes/webhooks.py

@@ -5,18 +5,19 @@
 from fastapi import APIRouter, Depends, Request
 from sqlalchemy.orm import Session
 
+from ..controller import auth, data
 from ..controller.apis.fxa_client import FxaClient
-from ..database import repo, models
+from ..database import repo, models, schemas
 from ..dependencies.database import get_db
 from ..dependencies.fxa import get_webhook_auth, get_fxa_client
+from ..exceptions.account_api import AccountDeletionSubscriberFail
 from ..exceptions.fxa_api import MissingRefreshTokenException
 
 router = APIRouter()
 
 
 @router.post("/fxa-process")
 def fxa_process(
-    request: Request,
     db: Session = Depends(get_db),
     decoded_token: dict = Depends(get_webhook_auth),
     fxa_client: FxaClient = Depends(get_fxa_client)
@@ -42,20 +43,41 @@ def fxa_process(
                     break
 
                 try:
-                    fxa_client.logout()
+                    auth.logout(db, subscriber, fxa_client)
                 except MissingRefreshTokenException:
                     logging.warning("Subscriber doesn't have refresh token.")
                 except requests.exceptions.HTTPError as ex:
                     logging.error(f"Error logging out user: {ex.response}")
             case 'https://schemas.accounts.firefox.com/event/profile-change':
                 if event_data.get('email') is not None:
-                    # Update the subscriber's email (and username for now)
+                    # Update the subscriber's email, we do this first in case there's a problem with get_profile()
                     subscriber.email = event_data.get('email')
-                    subscriber.username = subscriber.email
                     db.add(subscriber)
                     db.commit()
+
+                try:
+                    profile = fxa_client.get_profile()
+                    # Update profile with fxa info
+                    repo.update_subscriber(db, schemas.SubscriberIn(
+                        avatar_url=profile['avatar'],
+                        name=profile['displayName'] if 'displayName' in profile else profile['email'].split('@')[0],
+                        username=subscriber.username
+                    ), subscriber.id)
+                except Exception as ex:
+                    logging.error(f"Error updating user: {ex}")
+
+                # Finally log the subscriber out
+                try:
+                    auth.logout(db, subscriber, fxa_client)
+                except MissingRefreshTokenException:
+                    logging.warning("Subscriber doesn't have refresh token.")
+                except requests.exceptions.HTTPError as ex:
+                    logging.error(f"Error logging out user: {ex.response}")
             case 'https://schemas.accounts.firefox.com/event/delete-user':
-                # TODO: We have a delete function, but it's not up-to-date
-                logging.warning(f"Deletion request came in for {subscriber.id}")
+                try:
+                    data.delete_account(db, subscriber)
+                except AccountDeletionSubscriberFail as ex:
+                    logging.error(f"Account deletion webhook failed: {ex.message}")
+
             case _:
                 logging.warning(f"Ignoring event {event}")

backend/test/factory/subscriber_factory.py

@@ -14,7 +14,7 @@ def _make_subscriber(level, name=FAKER_RANDOM_VALUE, username=FAKER_RANDOM_VALUE
             subscriber = repo.create_subscriber(db, schemas.SubscriberBase(
                 name=name if factory_has_value(name) else fake.name(),
                 username=username if factory_has_value(username) else fake.name(),
-                email=email if factory_has_value(FAKER_RANDOM_VALUE) else fake.email(),
+                email=email if factory_has_value(email) else fake.email(),
                 level=level,
                 timezone='America/Vancouver'
             ))

backend/test/integration/test_webhooks.py

@@ -0,0 +1,162 @@
+import datetime
+
+from freezegun import freeze_time
+from appointment.database import models, repo
+
+from appointment.dependencies.fxa import get_webhook_auth
+from defines import FXA_CLIENT_PATCH
+
+
+class TestFXAWebhooks:
+    def test_fxa_process_change_password(self, with_db, with_client, make_pro_subscriber, make_external_connections):
+        """Ensure the change password event is handled correctly"""
+        FXA_USER_ID = 'abc-123'
+
+        def override_get_webhook_auth():
+            return {
+                "iss": "https://accounts.firefox.com/",
+                "sub": FXA_USER_ID,
+                "aud": "REMOTE_SYSTEM",
+                "iat": 1565720808,
+                "jti": "e19ed6c5-4816-4171-aa43-56ffe80dbda1",
+                "events": {
+                    "https://schemas.accounts.firefox.com/event/password-change": {
+                        "changeTime": 1565721242227
+                    }
+                }
+            }
+
+        # Override get_webhook_auth so we don't have to mock up a valid jwt token
+        with_client.app.dependency_overrides[get_webhook_auth] = override_get_webhook_auth
+
+        # make a guy
+        subscriber = make_pro_subscriber()
+        subscriber_id = subscriber.id
+        make_external_connections(subscriber_id, type=models.ExternalConnectionType.fxa, type_id=FXA_USER_ID)
+
+        assert subscriber.minimum_valid_iat_time is None
+
+        # Freeze time to before the changeTime timestamp and test the password change works correctly
+        with freeze_time('Aug 13th 2019'):
+            # Update the external connection time to match our freeze_time
+            with with_db() as db:
+                fxa_connection = repo.get_external_connections_by_type(db, subscriber_id, models.ExternalConnectionType.fxa, FXA_USER_ID)[0]
+                fxa_connection.time_updated = datetime.datetime.now()
+                db.add(fxa_connection)
+                db.commit()
+
+            response = with_client.post(
+                "/webhooks/fxa-process",
+            )
+            assert response.status_code == 200, response.text
+
+            with with_db() as db:
+                subscriber = repo.get_subscriber(db, subscriber_id)
+                assert subscriber.minimum_valid_iat_time is not None
+
+        # Update the external connection time to match our current time
+        # This will make the change password event out of date
+        with with_db() as db:
+            fxa_connection = repo.get_external_connections_by_type(db, subscriber_id, models.ExternalConnectionType.fxa, FXA_USER_ID)[0]
+            fxa_connection.time_updated = datetime.datetime.now()
+            db.add(fxa_connection)
+            db.commit()
+
+            # Reset our minimum_valid_iat_time, so we can ensure it stays None
+            subscriber = repo.get_subscriber(db, subscriber_id)
+            subscriber.minimum_valid_iat_time = None
+            db.add(subscriber)
+            db.commit()
+
+        # Finally test that minimum_valid_iat_time stays the same due to an outdated password change event
+        response = with_client.post(
+            "/webhooks/fxa-process",
+        )
+        assert response.status_code == 200, response.text
+        with with_db() as db:
+            subscriber = repo.get_subscriber(db, subscriber_id)
+            assert subscriber.minimum_valid_iat_time is None
+
+    def test_fxa_process_change_primary_email(self, with_db, with_client, make_pro_subscriber, make_external_connections):
+        """Ensure the change primary email event is handled correctly"""
+
+        FXA_USER_ID = 'abc-456'
+        OLD_EMAIL = '[email protected]'
+        NEW_EMAIL = '[email protected]'
+
+        def override_get_webhook_auth():
+            return {
+                "iss": "https://accounts.firefox.com/",
+                "sub": FXA_USER_ID,
+                "aud": "REMOTE_SYSTEM",
+                "iat": 1565720808,
+                "jti": "e19ed6c5-4816-4171-aa43-56ffe80dbda1",
+                "events": {
+                    "https://schemas.accounts.firefox.com/event/profile-change": {
+                        "email": NEW_EMAIL
+                    }
+                }
+            }
+
+        # Override get_webhook_auth so we don't have to mock up a valid jwt token
+        with_client.app.dependency_overrides[get_webhook_auth] = override_get_webhook_auth
+
+        # Make a guy with a middleschool-era email they would like to change
+        subscriber = make_pro_subscriber(email=OLD_EMAIL)
+        subscriber_id = subscriber.id
+        make_external_connections(subscriber_id, type=models.ExternalConnectionType.fxa, type_id=FXA_USER_ID)
+
+        assert subscriber.minimum_valid_iat_time is None
+        assert subscriber.email == OLD_EMAIL
+        assert subscriber.avatar_url != FXA_CLIENT_PATCH.get('subscriber_avatar_url')
+        assert subscriber.name != FXA_CLIENT_PATCH.get('subscriber_display_name')
+
+        response = with_client.post(
+            "/webhooks/fxa-process",
+        )
+        assert response.status_code == 200, response.text
+
+        # Refresh the subscriber and test minimum_valid_iat_time (they should be logged out), and email address
+        with with_db() as db:
+            subscriber = repo.get_subscriber(db, subscriber_id)
+            assert subscriber.email == NEW_EMAIL
+            assert subscriber.minimum_valid_iat_time is not None
+
+            # Ensure our profile update occured
+            assert subscriber.avatar_url == FXA_CLIENT_PATCH.get('subscriber_avatar_url')
+            assert subscriber.name == FXA_CLIENT_PATCH.get('subscriber_display_name')
+
+    def test_fxa_process_delete_user(self, with_db, with_client, make_pro_subscriber, make_external_connections, make_appointment, make_caldav_calendar):
+        """Ensure the delete user event is handled correctly"""
+        FXA_USER_ID = 'abc-789'
+
+        def override_get_webhook_auth():
+            return {
+                "iss": "https://accounts.firefox.com/",
+                "sub": FXA_USER_ID,
+                "aud": "REMOTE_SYSTEM",
+                "iat": 1565720810,
+                "jti": "1b3d623a-300a-4ab8-9241-855c35586809",
+                "events": {
+                    "https://schemas.accounts.firefox.com/event/delete-user": {}
+                }
+            }
+
+        # Override get_webhook_auth so we don't have to mock up a valid jwt token
+        with_client.app.dependency_overrides[get_webhook_auth] = override_get_webhook_auth
+
+        subscriber = make_pro_subscriber()
+        make_external_connections(subscriber.id, type=models.ExternalConnectionType.fxa, type_id=FXA_USER_ID)
+        calendar = make_caldav_calendar(subscriber.id)
+        appointment = make_appointment(calendar_id=calendar.id)
+
+        response = with_client.post(
+            "/webhooks/fxa-process",
+        )
+        assert response.status_code == 200, response.text
+
+        with with_db() as db:
+            # Make sure everything we created is gone. A more exhaustive check is done in the delete account test
+            assert repo.get_subscriber(db, subscriber.id) is None
+            assert repo.get_calendar(db, calendar.id) is None
+            assert repo.get_appointment(db, appointment.id) is None