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

for MPP-3817: prevent all Relay operations when user.is_active = False #4709

Merged
merged 8 commits into from
May 29, 2024
6 changes: 6 additions & 0 deletions api/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ def authenticate(self, request):
)
user = sa.user

if not user.is_active:
raise PermissionDenied(
"Authenticated user does not have an active Relay account."
" Have they been deactivated?"
)

say-yawn marked this conversation as resolved.
Show resolved Hide resolved
if user:
return (user, token)
else:
Expand Down
21 changes: 21 additions & 0 deletions api/tests/authentication_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,27 @@ def test_200_resp_from_fxa_no_matching_user_raises_APIException(self) -> None:
response = client.get("/api/v1/relayaddresses/")
assert responses.assert_call_count(self.fxa_verify_path, 1) is True

@responses.activate
def test_200_resp_from_fxa_inactive_Relay_user_raises_APIException(self) -> None:
sa: SocialAccount = baker.make(SocialAccount, uid=self.uid, provider="fxa")
sa.user.is_active = False
sa.user.save()
now_time = int(datetime.now().timestamp())
# Note: FXA iat and exp are timestamps in *milliseconds*
exp_time = (now_time + 60 * 60) * 1000
_setup_fxa_response(200, {"active": True, "sub": self.uid, "exp": exp_time})
inactive_user_token = "inactive-user-123"
client = APIClient()
client.credentials(HTTP_AUTHORIZATION=f"Bearer {inactive_user_token}")

response = client.get("/api/v1/relayaddresses/")
assert response.status_code == 403
expected_detail = (
"Authenticated user does not have an active Relay account."
" Have they been deactivated?"
)
assert response.json()["detail"] == expected_detail

@responses.activate
def test_200_resp_from_fxa_for_user_returns_user_and_caches(self) -> None:
sa: SocialAccount = baker.make(SocialAccount, uid=self.uid, provider="fxa")
Expand Down
18 changes: 18 additions & 0 deletions api/tests/emails_views_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,24 @@ def test_post_relayaddress_flagged_error(
assert get_glean_event(caplog) is None


def test_post_relayaddress_inactive_user_error(
free_user: User, free_api_client: APIClient, caplog: pytest.LogCaptureFixture
) -> None:
"""An inactive user is unable to create a random mask."""
free_user.is_active = False
free_user.save()

response = free_api_client.post(reverse("relayaddress-list"), {}, format="json")

assert response.status_code == 403
ret_data = response.json()
assert ret_data == {
"detail": "Your account is not active.",
"error_code": "account_is_inactive",
}
assert get_glean_event(caplog) is None


@pytest.mark.parametrize(
"key,value",
[
Expand Down
49 changes: 49 additions & 0 deletions api/tests/phones_views_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,28 @@ def test_inbound_sms_valid_twilio_signature_unknown_number(
assert "Could Not Find Relay Number." in response.data[0].title()


def test_inbound_sms_valid_twilio_signature_good_data_deactivated_user(
phone_user, mocked_twilio_client
):
phone_user.is_active = False
phone_user.save()
_make_real_phone(phone_user, verified=True)
relay_number = _make_relay_number(phone_user)
pre_inbound_remaining_texts = relay_number.remaining_texts
mocked_twilio_client.reset_mock()

client = APIClient()
path = "/api/v1/inbound_sms"
data = {"From": "+15556660000", "To": relay_number.number, "Body": "test body"}
response = client.post(path, data, HTTP_X_TWILIO_SIGNATURE="valid")

assert response.status_code == 200
mocked_twilio_client.messages.create.assert_not_called()
relay_number.refresh_from_db()
assert relay_number.texts_forwarded == 0
assert relay_number.remaining_texts == pre_inbound_remaining_texts


def test_inbound_sms_valid_twilio_signature_good_data(phone_user, mocked_twilio_client):
real_phone = _make_real_phone(phone_user, verified=True)
relay_number = _make_relay_number(phone_user)
Expand Down Expand Up @@ -1854,6 +1876,33 @@ def test_inbound_call_valid_twilio_signature_unknown_number(
assert "Could Not Find Relay Number." in response.data[0].title()


def test_inbound_call_valid_twilio_signature_good_data_deactivated_user(
phone_user, mocked_twilio_client
):
_make_real_phone(phone_user, verified=True)
relay_number = _make_relay_number(phone_user, enabled=True)
phone_user.is_active = False
phone_user.save()
pre_call_calls_forwarded = relay_number.calls_forwarded
caller_number = "+15556660000"
mocked_twilio_client.reset_mock()

client = APIClient()
path = "/api/v1/inbound_call"
data = {"Caller": caller_number, "Called": relay_number.number}
response = client.post(path, data, HTTP_X_TWILIO_SIGNATURE="valid")

assert response.status_code == 200
decoded_content = response.content.decode()
assert "<Response/>" in decoded_content
relay_number.refresh_from_db()
assert relay_number.calls_forwarded == pre_call_calls_forwarded
with pytest.raises(InboundContact.DoesNotExist):
InboundContact.objects.get(
relay_number=relay_number, inbound_number=caller_number
)


def test_inbound_call_valid_twilio_signature_good_data(
phone_user, mocked_twilio_client
):
Expand Down
11 changes: 11 additions & 0 deletions api/views/phones.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,12 @@ def inbound_sms(request):
raise exceptions.ValidationError("Request missing From, To, Or Body.")

relay_number, real_phone = _get_phone_objects(inbound_to)
if not real_phone.user.is_active:
return response.Response(
status=200,
template_name="twiml_empty_response.xml",
)

_check_remaining(relay_number, "texts")

if inbound_from == real_phone.number:
Expand Down Expand Up @@ -939,6 +945,11 @@ def inbound_call(request):
raise exceptions.ValidationError("Call data missing Caller or Called.")

relay_number, real_phone = _get_phone_objects(inbound_to)
if not real_phone.user.is_active:
return response.Response(
status=200,
template_name="twiml_empty_response.xml",
)

number_disabled = _check_disabled(relay_number, "calls")
if number_disabled:
Expand Down
19 changes: 0 additions & 19 deletions emails/management/commands/deactivate_user_by_token.py

This file was deleted.

15 changes: 15 additions & 0 deletions emails/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,9 @@ def custom_domain(self) -> str:

@property
def has_premium(self) -> bool:
if not self.user.is_active:
return False

# FIXME: as we don't have all the tiers defined we are over-defining
# this to mark the user as a premium user as well
if not self.fxa:
Expand Down Expand Up @@ -664,6 +667,12 @@ class AccountIsPausedException(CannotMakeAddressException):
status_code = 403


class AccountIsInactiveException(CannotMakeAddressException):
default_code = "account_is_inactive"
default_detail = "Your account is not active."
status_code = 403


class RelayAddrFreeTierLimitException(CannotMakeAddressException):
default_code = "free_tier_limit"
default_detail_template = (
Expand Down Expand Up @@ -857,6 +866,9 @@ def metrics_id(self) -> str:


def check_user_can_make_another_address(profile: Profile) -> None:
if not profile.user.is_active:
raise AccountIsInactiveException()

if profile.is_flagged:
raise AccountIsPausedException()
# MPP-3021: return early for premium users to avoid at_max_free_aliases DB query
Expand Down Expand Up @@ -910,6 +922,9 @@ def check_user_can_make_domain_address(user_profile: Profile) -> None:
if not user_profile.subdomain:
raise DomainAddrNeedSubdomainException()

if not user_profile.user.is_active:
raise AccountIsInactiveException()

if user_profile.is_flagged:
raise AccountIsPausedException()

Expand Down
11 changes: 11 additions & 0 deletions emails/tests/views_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,17 @@ def test_user_bounce_hard_paused_email_in_s3_deleted(self) -> None:
self.assert_log_incoming_email_dropped(caplog, "hard_bounce_pause")
mm.assert_incr_once("fx.private.relay.email_suppressed_for_hard_bounce")

def test_user_deactivated_email_in_s3_deleted(self) -> None:
self.profile.user.is_active = False
self.profile.user.save()

with self.assertLogs(INFO_LOG) as caplog:
response = _sns_notification(EMAIL_SNS_BODIES["s3_stored"])
self.mock_remove_message_from_s3.assert_called_once_with(self.bucket, self.key)
assert response.status_code == 200
assert response.content == b"Account is deactivated."
self.assert_log_incoming_email_dropped(caplog, "user_deactivated")

@patch("emails.views._reply_allowed")
@patch("emails.views._get_reply_record_from_lookup_key")
def test_reply_not_allowed_email_in_s3_deleted(
Expand Down
8 changes: 8 additions & 0 deletions emails/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ def _sns_message(message_json: AWS_SNSMessageJSON) -> HttpResponse:
"hard_bounce_pause", # The user recently had a hard bounce
"soft_bounce_pause", # The user recently has a soft bounce
"abuse_flag", # The user exceeded an abuse limit, like mails forwarded
"user_deactivated", # The user account is deactivated
"reply_requires_premium", # The email is a reply from a free user
"content_missing", # Could not load the email from storage
"error_from_header", # Error generating the From: header, retryable
Expand Down Expand Up @@ -678,6 +679,10 @@ def _handle_received(message_json: AWS_SNSMessageJSON) -> HttpResponse:
log_email_dropped(reason="abuse_flag", mask=address)
return HttpResponse("Address is temporarily disabled.")

if not user_profile.user.is_active:
log_email_dropped(reason="user_deactivated", mask=address)
return HttpResponse("Account is deactivated.")

# if address is set to block, early return
if not address.enabled:
incr_if_enabled("email_for_disabled_address", 1)
Expand Down Expand Up @@ -1218,6 +1223,9 @@ def _reply_allowed(
):
# This is a Relay user replying to an external sender;

if not reply_record.profile.user.is_active:
return False

if reply_record.profile.is_flagged:
return False

Expand Down
60 changes: 60 additions & 0 deletions privaterelay/management/commands/deactivate_user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
from argparse import ArgumentParser
from typing import Any

from django.contrib.auth.models import User
from django.core.management.base import BaseCommand

from allauth.socialaccount.models import SocialAccount

from emails.models import Profile


class Command(BaseCommand):
help = "Deactivates a user to effectively block all usage of Relay."

def add_arguments(self, parser: ArgumentParser) -> None:
parser.add_argument("--key", type=str, help="User API key")
parser.add_argument("--email", type=str, help="User email address")
parser.add_argument("--uid", type=str, help="User FXA UID")

def handle(self, *args: Any, **options: Any) -> str:
groovecoder marked this conversation as resolved.
Show resolved Hide resolved
api_key: str | None = options.get("key")
email: str | None = options.get("email")
uid: str | None = options.get("uid")

user: User

if api_key:
try:
profile = Profile.objects.get(api_token=api_key)
user = profile.user
user.is_active = False
user.save()
msg = f"SUCCESS: deactivated user with api_token: {api_key}"
except Profile.DoesNotExist:
msg = "ERROR: Could not find user with that API key."
self.stderr.write(msg)
return msg

if email:
try:
user = User.objects.get(email=email)
user.is_active = False
user.save()
msg = f"SUCCESS: deactivated user with email: {email}"
except User.DoesNotExist:
msg = "ERROR: Could not find user with that email address."
self.stderr.write(msg)
return msg

if uid:
try:
user = SocialAccount.objects.get(uid=uid).user
user.is_active = False
user.save()
msg = f"SUCCESS: deactivated user with FXA UID: {uid}"
except SocialAccount.DoesNotExist:
msg = "ERROR: Could not find user with that FXA UID."
self.stderr.write(msg)
return msg
return msg
1 change: 1 addition & 0 deletions privaterelay/pending_locales/en/pending.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

# This is the Django equivalent of frontend/pendingTranslations.ftl
api-error-account-is-inactive = Your account is not active.
Loading