Skip to content

Commit 8c13caa

Browse files
aapelivclaude
andcommitted
Backend: fix feature-flag gating gaps for SMS and postal verification
Evaluate sms_enabled per-user instead of globally - send_sms always has a user in scope (its only caller, ChangePhone, has the context), so global evaluation skipped feature-usage tracking and would silently fall through to the default if the flag ever became a rollout/experiment. Gate ConfirmPostalAddress on postal_verification_enabled. Previously only the free address-validation step (InitiatePostalVerification) was gated, so a user already past step 1 could still trigger a (paid) postcard with the flag off. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f3e159d commit 8c13caa

5 files changed

Lines changed: 50 additions & 11 deletions

File tree

app/backend/src/couchers/phone/sms.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
import boto3
55
import luhn
66

7-
from couchers import crypto, experimentation
7+
from couchers import crypto
88
from couchers.config import config
9+
from couchers.context import CouchersContext
910
from couchers.db import session_scope
1011
from couchers.models import SMS
1112

@@ -25,14 +26,14 @@ def format_message(token: str) -> str:
2526
return f"{token} is your Couchers.org verification code. If you did not request this, please ignore this message. Best, the Couchers.org team."
2627

2728

28-
def send_sms(number: str, message: str) -> str:
29+
def send_sms(context: CouchersContext, number: str, message: str) -> str:
2930
"""Send SMS to a E.164 formatted phone number with leading +. Return "success" on
3031
success, "unsupported operator" on unsupported operator, and any other
3132
string for any other error."""
3233

3334
assert len(message) <= 140, "Message too long"
3435

35-
if not experimentation.get_global_boolean_value("sms_enabled", default=False):
36+
if not context.get_boolean_value("sms_enabled", default=False):
3637
logger.info(f"SMS not enabled, need to send to {number}: {message}")
3738
return "SMS not enabled."
3839

app/backend/src/couchers/servicers/account.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ def ChangePhone(
349349
context.abort_with_error_code(grpc.StatusCode.RESOURCE_EXHAUSTED, "reverification_too_early")
350350

351351
token = sms.generate_random_code()
352-
result = sms.send_sms(phone, sms.format_message(token))
352+
result = sms.send_sms(context, phone, sms.format_message(token))
353353

354354
if result == "success":
355355
user.phone = phone

app/backend/src/couchers/servicers/postal_verification.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ def ConfirmPostalAddress(
168168
"""
169169
Step 2: User confirms address, we generate code and send postcard.
170170
"""
171+
# Gate the step that actually commits to sending a (paid) postcard, not just the initial
172+
# address validation - otherwise turning the flag off wouldn't stop postcards mid-flow.
173+
if not context.get_boolean_value("postal_verification_enabled", default=False):
174+
context.abort_with_error_code(grpc.StatusCode.UNAVAILABLE, "postal_verification_disabled")
175+
171176
attempt = session.execute(
172177
select(PostalVerificationAttempt)
173178
.where(PostalVerificationAttempt.id == request.postal_verification_attempt_id)

app/backend/src/tests/test_postal_verification.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from couchers.helpers.postal_verification import generate_postal_verification_code, has_postal_verification
1616
from couchers.jobs.worker import process_job
1717
from couchers.models import User
18-
from couchers.models.postal_verification import PostalVerificationAttempt
18+
from couchers.models.postal_verification import PostalVerificationAttempt, PostalVerificationStatus
1919
from couchers.postal.my_postcard import _generate_back_left_side_png
2020
from couchers.proto import postal_verification_pb2
2121
from couchers.resources import get_postcard_front_image
@@ -60,6 +60,32 @@ def test_postal_verification_disabled(db, feature_flags):
6060
assert e.value.code() == grpc.StatusCode.UNAVAILABLE
6161

6262

63+
def test_postal_verification_confirm_disabled(db, feature_flags):
64+
"""Confirming (which queues the paid postcard) must respect the flag, not just initiation."""
65+
feature_flags.set("postal_verification_enabled", False)
66+
user, token = generate_user()
67+
68+
# Seed a pending attempt directly, since initiation is gated by the same flag.
69+
with session_scope() as session:
70+
attempt = PostalVerificationAttempt(
71+
user_id=user.id,
72+
status=PostalVerificationStatus.pending_address_confirmation,
73+
address_line_1="123 Main St",
74+
city="Test City",
75+
country_code="US",
76+
)
77+
session.add(attempt)
78+
session.flush()
79+
attempt_id = attempt.id
80+
81+
with postal_verification_session(token) as pv:
82+
with pytest.raises(grpc.RpcError) as e:
83+
pv.ConfirmPostalAddress(
84+
postal_verification_pb2.ConfirmPostalAddressReq(postal_verification_attempt_id=attempt_id)
85+
)
86+
assert e.value.code() == grpc.StatusCode.UNAVAILABLE
87+
88+
6389
def test_postal_verification_happy_path(db):
6490
"""Test the complete happy path for postal verification."""
6591
user, token = generate_user()

app/backend/src/tests/test_verification.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import couchers.phone.sms
99
from couchers.config import config
10+
from couchers.context import make_background_user_context
1011
from couchers.crypto import random_hex
1112
from couchers.db import session_scope
1213
from couchers.models import SMS, User
@@ -43,7 +44,7 @@ def test_ChangePhone(db, monkeypatch, push_collector: PushCollector):
4344
assert e.value.code() == grpc.StatusCode.UNIMPLEMENTED
4445

4546
# Test with operator not supported by SMS backend
46-
def deny_operator(phone, message):
47+
def deny_operator(context, phone, message):
4748
assert phone == "+46701740605"
4849
return "unsupported operator"
4950

@@ -54,7 +55,7 @@ def deny_operator(phone, message):
5455
assert e.value.code() == grpc.StatusCode.UNIMPLEMENTED
5556

5657
# Test with successfully sent SMS
57-
def succeed(phone, message):
58+
def succeed(context, phone, message):
5859
assert phone == "+46701740605"
5960
return "success"
6061

@@ -94,7 +95,7 @@ def test_ChangePhone_ratelimit(db, monkeypatch):
9495
user_id = user.id
9596
with account_session(token) as account:
9697

97-
def succeed(phone, message):
98+
def succeed(context, phone, message):
9899
return "success"
99100

100101
monkeypatch.setattr(couchers.phone.sms, "send_sms", succeed)
@@ -166,7 +167,7 @@ def test_phone_uniqueness(monkeypatch):
166167
user2, token2 = generate_user()
167168
with account_session(token1) as account1, account_session(token2) as account2:
168169

169-
def succeed(phone, message):
170+
def succeed(context, phone, message):
170171
return "success"
171172

172173
monkeypatch.setattr(couchers.phone.sms, "send_sms", succeed)
@@ -217,7 +218,10 @@ def test_send_sms(db, monkeypatch):
217218
sns.publish.return_value = {"MessageId": msg_id}
218219
mock.client.return_value = sns
219220

220-
assert couchers.phone.sms.send_sms("+46701740605", "Testing SMS message") == "success"
221+
assert (
222+
couchers.phone.sms.send_sms(make_background_user_context(1), "+46701740605", "Testing SMS message")
223+
== "success"
224+
)
221225

222226
mock.client.assert_called_once_with("sns")
223227
sns.publish.assert_called_once_with(
@@ -239,7 +243,10 @@ def test_send_sms(db, monkeypatch):
239243

240244
def test_send_sms_disabled(db, feature_flags):
241245
feature_flags.set("sms_enabled", False)
242-
assert couchers.phone.sms.send_sms("+46701740605", "Testing SMS message") == "SMS not enabled."
246+
assert (
247+
couchers.phone.sms.send_sms(make_background_user_context(1), "+46701740605", "Testing SMS message")
248+
== "SMS not enabled."
249+
)
243250

244251

245252
def test_sms_verification_no_donation():

0 commit comments

Comments
 (0)