Skip to content

Commit

Permalink
Ensure the incoming fxa uid matches all existing ones (there should o…
Browse files Browse the repository at this point in the history
…nly be one!)
  • Loading branch information
MelissaAutumn committed Apr 24, 2024
1 parent bae35b0 commit c94008d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 3 deletions.
8 changes: 6 additions & 2 deletions backend/src/appointment/l10n.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from typing import Union, Dict, Any
from starlette_context import context

from starlette_context import context, errors


def l10n(msg_id: str, args: Union[Dict[str, Any], None] = None) -> str:
"""Helper function to automatically call fluent.format_value from context"""
if 'l10n' not in context:
try:
if 'l10n' not in context:
return msg_id
except errors.ContextDoesNotExistError:
return msg_id

return context['l10n'](msg_id, args)
13 changes: 13 additions & 0 deletions backend/src/appointment/routes/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import argon2.exceptions
from fastapi.security import OAuth2PasswordRequestForm
from jose import jwt
from sentry_sdk import capture_exception
from sqlalchemy.orm import Session

from fastapi import APIRouter, Depends, HTTPException, Request
Expand Down Expand Up @@ -126,6 +127,18 @@ def fxa_callback(
elif not subscriber:
subscriber = fxa_subscriber

fxa_connections = repo.external_connection.get_by_type(db, subscriber.id, ExternalConnectionType.fxa)

# If we have fxa_connections, ensure the incoming one matches our known one.
# This shouldn't occur, but it's a safety check in-case we missed a webhook push.
if any([profile['uid'] != ec.type_id for ec in fxa_connections]):
# Ensure sentry captures the error too!
if os.getenv('SENTRY_DSN') != '':
e = Exception("Invalid Credentials, incoming profile uid does not match existing profile uid")
capture_exception(e)

raise HTTPException(403, l10n('invalid-credentials'))

external_connection_schema = schemas.ExternalConnection(
name=profile['email'],
type=ExternalConnectionType.fxa,
Expand Down
34 changes: 33 additions & 1 deletion backend/test/integration/test_auth.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

from defines import FXA_CLIENT_PATCH
from appointment.l10n import l10n
from defines import FXA_CLIENT_PATCH, TEST_USER_ID
from appointment.database import repo, models


Expand Down Expand Up @@ -89,3 +90,34 @@ def test_fxa_callback(self, with_db, with_client, monkeypatch):
fxa = subscriber.get_external_connection(models.ExternalConnectionType.fxa)
assert fxa
assert fxa.type_id == FXA_CLIENT_PATCH.get('external_connection_type_id')

def test_fxa_callback_with_mismatch_uid(self, with_db, with_client, monkeypatch, make_external_connections, make_basic_subscriber, with_l10n):
"""Test that our fxa callback will throw an invalid-credentials error if the incoming fxa uid doesn't match any existing ones."""
os.environ['AUTH_SCHEME'] = 'fxa'

state = 'a1234'

subscriber = make_basic_subscriber(email=FXA_CLIENT_PATCH.get('subscriber_email'))

mismatch_uid = f"{FXA_CLIENT_PATCH.get('external_connection_type_id')}-not-actually"
make_external_connections(subscriber.id, type=models.ExternalConnectionType.fxa, type_id=mismatch_uid)

monkeypatch.setattr('starlette.requests.HTTPConnection.session', {
'fxa_state': state,
'fxa_user_email': FXA_CLIENT_PATCH.get('subscriber_email'),
'fxa_user_timezone': 'America/Vancouver'
})

response = with_client.get(
"/fxa",
params={
'code': FXA_CLIENT_PATCH.get('credentials_code'),
'state': state
},
follow_redirects=False
)

# This should error out as a 403
assert response.status_code == 403, response.text
# This will just key match due to the lack of context.
assert response.json().get('detail') == l10n('invalid-credentials')

0 comments on commit c94008d

Please sign in to comment.