Skip to content

Commit

Permalink
Merge pull request #4709 from mozilla/code-to-deactivate-user-mpp-3817
Browse files Browse the repository at this point in the history
for MPP-3817: prevent all Relay operations when user.is_active = False
  • Loading branch information
groovecoder authored May 29, 2024
2 parents e59445d + f937381 commit 9ddaf25
Show file tree
Hide file tree
Showing 16 changed files with 321 additions and 20 deletions.
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?"
)

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
2 changes: 2 additions & 0 deletions frontend/pendingTranslations.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,5 @@ upsell-banner-4-masks-us-cta = Upgrade to { -brand-name-relay-premium }
-brand-name-mozilla-monitor = Mozilla Monitor
moz-monitor = { -brand-name-mozilla-monitor }
api-error-account-is-inactive = Your account is not active.
6 changes: 6 additions & 0 deletions frontend/src/pages/accounts/account_inactive.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@import "~@mozilla-protocol/core/protocol/css/includes/lib";

.error {
background-color: $color-red-60;
color: $color-white;
}
18 changes: 18 additions & 0 deletions frontend/src/pages/accounts/account_inactive.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import type { NextPage } from "next";

import { Layout } from "../../components/layout/Layout";
import { useL10n } from "../../hooks/l10n";
import styles from "./account_inactive.module.scss";

const AccountInactive: NextPage = () => {
const l10n = useL10n();
return (
<Layout>
<div className={styles["error"]}>
{l10n.getString("api-error-account-is-inactive")}
</div>
</Layout>
);
};

export default AccountInactive;
5 changes: 4 additions & 1 deletion privaterelay/allauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from urllib.parse import urlencode, urlparse

from django.http import Http404
from django.shortcuts import resolve_url
from django.shortcuts import redirect, resolve_url
from django.urls import resolve

from allauth.account.adapter import DefaultAccountAdapter
Expand Down Expand Up @@ -54,3 +54,6 @@ def is_safe_url(self, url: str | None) -> bool:
# The path is invalid
logger.error("No matching URL for '%s'", url)
return False

def respond_user_inactive(self, request, user):
return redirect("/accounts/account_inactive/")
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:
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

0 comments on commit 9ddaf25

Please sign in to comment.