Skip to content

Commit 3aeb976

Browse files
Add data migration to lowercase emails, and only ingest lowercase emails. (#915)
* Add data migration to lowercase emails, and only ingest lowercase emails. * Lint test_waiting_list * Remove redundant calls
1 parent 2fb4fc9 commit 3aeb976

File tree

10 files changed

+168
-122
lines changed

10 files changed

+168
-122
lines changed

backend/src/appointment/database/repo/subscriber.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def get(db: Session, subscriber_id: int) -> models.Subscriber | None:
1919

2020
def get_by_email(db: Session, email: str) -> models.Subscriber | None:
2121
"""retrieve subscriber by email"""
22-
return db.query(models.Subscriber).filter(models.Subscriber.email == email).first()
22+
return db.query(models.Subscriber).filter(models.Subscriber.email == email.lower()).first()
2323

2424

2525
def get_by_username(db: Session, username: str):
@@ -51,6 +51,10 @@ def create(db: Session, subscriber: schemas.SubscriberBase):
5151
"""create new subscriber"""
5252
data = subscriber.model_dump()
5353

54+
data['email'] = data.get('email').lower()
55+
if data.get('secondary_email'):
56+
data['secondary_email'] = data.secondary_email.lower()
57+
5458
# Filter incoming data to just the available model columns
5559
columns = models.Subscriber().get_columns()
5660
data = {k: v for k, v in data.items() if k in columns}
@@ -68,6 +72,9 @@ def update(db: Session, data: schemas.SubscriberIn, subscriber_id: int):
6872
"""update all subscriber attributes, they can edit themselves"""
6973
db_subscriber = get(db, subscriber_id)
7074
for key, value in data:
75+
if (key == 'email' or key == 'secondary_email') and value is not None:
76+
value = value.lower()
77+
7178
if value is not None:
7279
setattr(db_subscriber, key, value)
7380
db.commit()
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
"""data migration clean up email case
2+
3+
Revision ID: 537672c3933d
4+
Revises: 330fdd8cd0f8
5+
Create Date: 2025-03-07 21:09:16.541016
6+
7+
"""
8+
9+
from alembic import op
10+
from sqlalchemy.orm import Session
11+
12+
from appointment.database import models
13+
14+
# revision identifiers, used by Alembic.
15+
revision = '537672c3933d'
16+
down_revision = '330fdd8cd0f8'
17+
branch_labels = None
18+
depends_on = None
19+
20+
21+
def upgrade() -> None:
22+
session = Session(op.get_bind())
23+
subscribers: list[models.Subscriber] = session.query(models.Subscriber).all()
24+
for subscriber in subscribers:
25+
# Lowercase the emails
26+
subscriber.email = subscriber.email.lower()
27+
if subscriber.secondary_email:
28+
subscriber.secondary_email = subscriber.secondary_email.lower()
29+
30+
# Add the subscriber to the database session and commit (update) it
31+
session.add(subscriber)
32+
session.commit()
33+
34+
waiting_list: list[models.WaitingList] = session.query(models.WaitingList).all()
35+
for waiting_list_individual in waiting_list:
36+
# Lowercase the emails
37+
waiting_list_individual.email = waiting_list_individual.email.lower()
38+
39+
# Add the waiting list individual to the database session and commit (update) it
40+
session.add(waiting_list_individual)
41+
session.commit()
42+
43+
44+
def downgrade() -> None:
45+
pass

backend/src/appointment/routes/api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def update_me(
6666
me = repo.subscriber.update(db=db, data=data, subscriber_id=subscriber.id)
6767
return schemas.SubscriberMeOut(
6868
username=me.username,
69-
email=me.email,
69+
email=me.email.lower(),
7070
preferred_email=me.preferred_email,
7171
name=me.name,
7272
level=me.level,

backend/src/appointment/routes/auth.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def can_login(data: schemas.CheckEmail, db: Session = Depends(get_db), fxa_clien
7070
"""Determines if a user can go through the login flow"""
7171
if os.getenv('AUTH_SCHEME') == 'fxa':
7272
# This checks if a subscriber exists, or is in allowed list
73-
return fxa_client.is_in_allow_list(db, data.email)
73+
return fxa_client.is_in_allow_list(db, data.email.lower())
7474

7575
# There's no waiting list setting on password login
7676
return True
@@ -158,7 +158,7 @@ def fxa_callback(
158158
f"{os.getenv('FRONTEND_URL', 'http://localhost:8080')}/login/?error={errors['email-not-in-session']}"
159159
)
160160

161-
email = request.session['fxa_user_email']
161+
email = request.session['fxa_user_email'].lower()
162162
# We only use timezone during subscriber creation, or if their timezone is None
163163
timezone = request.session['fxa_user_timezone']
164164
invite_code = request.session.get('fxa_user_invite_code')
@@ -176,7 +176,7 @@ def fxa_callback(
176176
creds = fxa_client.get_credentials(code)
177177
profile = fxa_client.get_profile()
178178

179-
if profile['email'] != email:
179+
if email == '' or profile.get('email', '').lower() != email:
180180
fxa_client.logout()
181181
return RedirectResponse(
182182
f"{os.getenv('FRONTEND_URL', 'http://localhost:8080')}/login/?error={errors['email-mismatch']}"

backend/src/appointment/routes/google.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ def google_auth(
4747
subscriber: Subscriber = Depends(get_subscriber),
4848
):
4949
"""Starts the google oauth process"""
50+
if email:
51+
email = email.lower()
5052
url, state = google_client.get_redirect_url(email)
5153

5254
request.session[SESSION_OAUTH_STATE] = state
@@ -168,7 +170,7 @@ def disconnect_account(
168170
repo.calendar.delete_by_subscriber_and_provider(db, subscriber.id, provider=models.CalendarProvider.google)
169171

170172
# Unassociated any secondary emails if they're attached to their google connection
171-
if subscriber.secondary_email == google_connection.name:
173+
if subscriber.secondary_email == google_connection.name.lower():
172174
subscriber.secondary_email = None
173175
db.add(subscriber)
174176
db.commit()

backend/src/appointment/routes/waiting_list.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,19 @@ def join_the_waiting_list(
3535
db: Session = Depends(get_db)
3636
):
3737
"""Join the waiting list!"""
38-
added = repo.invite.add_to_waiting_list(db, data.email)
38+
email = data.email.lower()
39+
added = repo.invite.add_to_waiting_list(db, email)
3940

4041
# TODO: Replace our signed functionality with this
4142
# Not timed since we don't have a mechanism to re-send these...
4243
serializer = URLSafeSerializer(os.getenv('SIGNED_SECRET'), 'waiting-list')
43-
confirm_token = serializer.dumps({'email': data.email, 'action': WaitingListAction.CONFIRM_EMAIL.value})
44-
decline_token = serializer.dumps({'email': data.email, 'action': WaitingListAction.LEAVE.value})
44+
confirm_token = serializer.dumps({'email': email, 'action': WaitingListAction.CONFIRM_EMAIL.value})
45+
decline_token = serializer.dumps({'email': email, 'action': WaitingListAction.LEAVE.value})
4546

4647
# If they were added, send the email
4748
if added:
4849
background_tasks.add_task(
49-
send_confirm_email, to=data.email, confirm_token=confirm_token, decline_token=decline_token
50+
send_confirm_email, to=email, confirm_token=confirm_token, decline_token=decline_token
5051
)
5152

5253
return added
@@ -68,6 +69,8 @@ def act_on_waiting_list(data: schemas.TokenForWaitingList, db: Session = Depends
6869
if action is None or email is None:
6970
raise validation.InvalidLinkException()
7071

72+
email = email.lower()
73+
7174
if action == WaitingListAction.CONFIRM_EMAIL.value:
7275
success = repo.invite.confirm_waiting_list_email(db, email)
7376
elif action == WaitingListAction.LEAVE.value:

backend/src/appointment/routes/webhooks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def fxa_process(
5353
case 'https://schemas.accounts.firefox.com/event/profile-change':
5454
if event_data.get('email') is not None:
5555
# Update the subscriber's email, we do this first in case there's a problem with get_profile()
56-
subscriber.email = event_data.get('email')
56+
subscriber.email = event_data.get('email').lower()
5757
db.add(subscriber)
5858
db.commit()
5959

backend/test/integration/test_auth.py

Lines changed: 48 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,27 @@
1313

1414

1515
class TestAuth:
16+
def test_can_login(self, with_db, with_client, make_pro_subscriber, faker):
17+
# Request can-login with a capitalized email
18+
capital_email = faker.email().capitalize()
19+
subscriber = make_pro_subscriber(email=capital_email)
20+
21+
assert subscriber.email != capital_email
22+
assert subscriber.email == capital_email.lower()
23+
24+
with with_db() as db:
25+
# Check we've saved the email as lowercase
26+
subscriber_check = repo.subscriber.get_by_email(db, capital_email.lower())
27+
assert subscriber_check is not None
28+
assert subscriber_check.email != capital_email
29+
assert subscriber_check.email == capital_email.lower()
30+
31+
response = with_client.post('/can-login', json={'email': subscriber.email})
32+
33+
assert response.status_code == 200, response.text
34+
data = response.json()
35+
assert data is True
36+
1637
def test_me(self, with_db, with_client):
1738
response = with_client.get('/me', headers=auth_headers)
1839
assert response.status_code == 200, response.text
@@ -198,21 +219,14 @@ def test_fxa_with_allowlist_and_with_bad_invite_code(self, with_client, with_l10
198219
199220
response = with_client.get(
200221
'/fxa_login',
201-
params={
202-
'email': email,
203-
'invite_code': 'absolute nonsense!'
204-
},
222+
params={'email': email, 'invite_code': 'absolute nonsense!'},
205223
)
206224
assert response.status_code == 404, response.text
207225
data = response.json()
208226
assert data.get('detail') == l10n('invite-code-not-valid')
209227

210228
def test_fxa_with_allowlist_and_with_used_invite_code(
211-
self,
212-
with_client,
213-
with_l10n,
214-
make_invite,
215-
make_pro_subscriber
229+
self, with_client, with_l10n, make_invite, make_pro_subscriber
216230
):
217231
os.environ['AUTH_SCHEME'] = 'fxa'
218232
os.environ['FXA_ALLOW_LIST'] = '@example.org'
@@ -223,10 +237,7 @@ def test_fxa_with_allowlist_and_with_used_invite_code(
223237
224238
response = with_client.get(
225239
'/fxa_login',
226-
params={
227-
'email': email,
228-
'invite_code': invite.code
229-
},
240+
params={'email': email, 'invite_code': invite.code},
230241
)
231242
assert response.status_code == 403, response.text
232243
data = response.json()
@@ -433,17 +444,12 @@ def test_fxa_token_success(self, make_basic_subscriber, with_client):
433444

434445
subscriber = make_basic_subscriber(email='[email protected]')
435446
access_token_expires = timedelta(minutes=float(10))
436-
one_time_access_token = create_access_token(data={
437-
'sub': f'uid-{subscriber.id}',
438-
'jti': secrets.token_urlsafe(16)
439-
}, expires_delta=access_token_expires)
447+
one_time_access_token = create_access_token(
448+
data={'sub': f'uid-{subscriber.id}', 'jti': secrets.token_urlsafe(16)}, expires_delta=access_token_expires
449+
)
440450

441451
# Exchange the one-time token with a long-living token
442-
response = with_client.post(
443-
'/fxa-token', headers={
444-
'Authorization': f'Bearer {one_time_access_token}'
445-
}
446-
)
452+
response = with_client.post('/fxa-token', headers={'Authorization': f'Bearer {one_time_access_token}'})
447453

448454
assert response.status_code == 200, response.text
449455

@@ -454,11 +460,7 @@ def test_fxa_token_success(self, make_basic_subscriber, with_client):
454460
assert data.get('token_type') == 'bearer'
455461

456462
# Test it out!
457-
response = with_client.get(
458-
'/me', headers={
459-
'Authorization': f'Bearer {access_token}'
460-
}
461-
)
463+
response = with_client.get('/me', headers={'Authorization': f'Bearer {access_token}'})
462464

463465
assert response.status_code == 200, response.text
464466
assert response.json().get('email') == subscriber.email
@@ -471,16 +473,15 @@ def test_fxa_token_failed_due_to_non_one_time_token(self, make_basic_subscriber,
471473

472474
subscriber = make_basic_subscriber(email='[email protected]')
473475
access_token_expires = timedelta(minutes=float(10))
474-
regular_access_token = create_access_token(data={
475-
'sub': f'uid-{subscriber.id}',
476-
}, expires_delta=access_token_expires)
477-
478-
response = with_client.post(
479-
'/fxa-token', headers={
480-
'Authorization': f'Bearer {regular_access_token}'
481-
}
476+
regular_access_token = create_access_token(
477+
data={
478+
'sub': f'uid-{subscriber.id}',
479+
},
480+
expires_delta=access_token_expires,
482481
)
483482

483+
response = with_client.post('/fxa-token', headers={'Authorization': f'Bearer {regular_access_token}'})
484+
484485
assert response.status_code == 401, response.text
485486

486487
def test_fxa_token_failed_due_to_empty_auth(self, make_basic_subscriber, with_client):
@@ -489,9 +490,7 @@ def test_fxa_token_failed_due_to_empty_auth(self, make_basic_subscriber, with_cl
489490

490491
del with_client.app.dependency_overrides[auth.get_subscriber]
491492

492-
response = with_client.post(
493-
'/fxa-token'
494-
)
493+
response = with_client.post('/fxa-token')
495494

496495
assert response.status_code == 401, response.text
497496

@@ -504,17 +503,12 @@ def test_fxa_token_failed_due_to_invalid_auth_scheme(self, with_client, make_bas
504503

505504
subscriber = make_basic_subscriber(email='[email protected]')
506505
access_token_expires = timedelta(minutes=float(10))
507-
one_time_access_token = create_access_token(data={
508-
'sub': f'uid-{subscriber.id}',
509-
'jti': secrets.token_urlsafe(16)
510-
}, expires_delta=access_token_expires)
506+
one_time_access_token = create_access_token(
507+
data={'sub': f'uid-{subscriber.id}', 'jti': secrets.token_urlsafe(16)}, expires_delta=access_token_expires
508+
)
511509

512510
# Exchange the one-time token with a long-living token
513-
response = with_client.post(
514-
'/fxa-token', headers={
515-
'Authorization': f'Bearer {one_time_access_token}'
516-
}
517-
)
511+
response = with_client.post('/fxa-token', headers={'Authorization': f'Bearer {one_time_access_token}'})
518512
os.environ['AUTH_SCHEME'] = saved_scheme
519513
assert response.status_code == 405, response.text
520514

@@ -530,17 +524,15 @@ def test_auth(self, with_db, with_client):
530524
assert len(ecs) == 0
531525

532526
with patch('appointment.controller.calendar.Tools.dns_caldav_lookup') as mock:
533-
mock.return_value = "https://example.com", 300
527+
mock.return_value = 'https://example.com', 300
534528

535529
with patch('appointment.controller.calendar.CalDavConnector.sync_calendars') as sync_mock:
536530
sync_mock.return_value = None
537531

538532
response = with_client.post(
539533
'/caldav/auth',
540-
json={'user': '[email protected]',
541-
'url': 'example.com',
542-
'password': 'test'},
543-
headers=auth_headers
534+
json={'user': '[email protected]', 'url': 'example.com', 'password': 'test'},
535+
headers=auth_headers,
544536
)
545537

546538
mock.assert_called()
@@ -559,19 +551,13 @@ def test_disconnect(self, with_db, with_client, make_external_connections, make_
559551
ec = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.caldav, type_id=type_id)
560552
calendar = make_caldav_calendar(subscriber_id=TEST_USER_ID, user=username)
561553

562-
response = with_client.post(
563-
'/caldav/disconnect', json={'type_id': ec.type_id},
564-
headers=auth_headers
565-
)
554+
response = with_client.post('/caldav/disconnect', json={'type_id': ec.type_id}, headers=auth_headers)
566555

567556
assert response.status_code == 200, response.content
568557

569558
with with_db() as db:
570559
ecs = repo.external_connection.get_by_type(
571-
db,
572-
TEST_USER_ID,
573-
models.ExternalConnectionType.caldav,
574-
type_id=type_id
560+
db, TEST_USER_ID, models.ExternalConnectionType.caldav, type_id=type_id
575561
)
576562
assert len(ecs) == 0
577563

@@ -586,19 +572,13 @@ def test_disconnect(self, with_db, with_client, make_external_connections, make_
586572
ec = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.google, type_id=type_id)
587573
calendar = make_google_calendar(subscriber_id=TEST_USER_ID)
588574

589-
response = with_client.post(
590-
'/google/disconnect', json={'type_id': ec.type_id},
591-
headers=auth_headers
592-
)
575+
response = with_client.post('/google/disconnect', json={'type_id': ec.type_id}, headers=auth_headers)
593576

594577
assert response.status_code == 200, response.content
595578

596579
with with_db() as db:
597580
ecs = repo.external_connection.get_by_type(
598-
db,
599-
TEST_USER_ID,
600-
models.ExternalConnectionType.google,
601-
type_id=type_id
581+
db, TEST_USER_ID, models.ExternalConnectionType.google, type_id=type_id
602582
)
603583
assert len(ecs) == 0
604584

0 commit comments

Comments
 (0)