Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add minimum_valid_iat_time to subscriber table, and hook up the rest of the fxa webhook events 🛢 #222

Merged
merged 9 commits into from
Jan 12, 2024
Merged
1 change: 1 addition & 0 deletions backend/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]']

Expand Down
17 changes: 16 additions & 1 deletion backend/src/appointment/controller/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
3 changes: 3 additions & 0 deletions backend/src/appointment/database/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(DateTime, index=True)

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")
Expand Down
4 changes: 2 additions & 2 deletions backend/src/appointment/database/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
11 changes: 10 additions & 1 deletion backend/src/appointment/dependencies/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
devmount marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
"""add minimum_valid_iat_time to subscribers

Revision ID: ad7cc2de5ff8
Revises: 0dc429ca07f5
Create Date: 2024-01-09 16:52:20.941572

"""
from alembic import op
import sqlalchemy as sa


# 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', sa.DateTime, index=True))
devmount marked this conversation as resolved.
Show resolved Hide resolved


def downgrade() -> None:
op.drop_column('subscribers', 'minimum_valid_iat_time')
17 changes: 10 additions & 7 deletions backend/src/appointment/routes/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
36 changes: 29 additions & 7 deletions backend/src/appointment/routes/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
devmount marked this conversation as resolved.
Show resolved Hide resolved
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}")
2 changes: 1 addition & 1 deletion backend/test/factory/subscriber_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
))
Expand Down
162 changes: 162 additions & 0 deletions backend/test/integration/test_webhooks.py
Original file line number Diff line number Diff line change
@@ -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:
devmount marked this conversation as resolved.
Show resolved Hide resolved
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
Loading