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

Feature: Added an account disabled page #3391

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
41 changes: 41 additions & 0 deletions pontoon/base/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from raygun4py.middleware.django import Provider

from django.conf import settings
from django.contrib.auth.middleware import get_user
from django.contrib.auth.models import User
from django.core.cache import cache
from django.core.exceptions import PermissionDenied
from django.http import Http404, HttpResponseForbidden
Expand Down Expand Up @@ -149,3 +151,42 @@ def __call__(self, request):
cache.set(observed_key, (1, now), self.observation_period)

return response


class AccountDisabledMiddleware:
def __init__(self, get_response):
self.get_response = get_response

def __call__(self, request):
# Manually fetch user from the session
request.user = get_user(request)
user = request.user

# If user is authenticated, check if they are inactive
if user.is_authenticated:
if not user.is_active:
return render(
request,
"account_disabled.html",
{"DEFAULT_FROM_EMAIL": settings.DEFAULT_FROM_EMAIL},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this line? I didn't check, but the context_processor should pass settings to all templates:
https://github.com/mozilla/pontoon/blob/08710a279ab2f2c52b7f1a1a1035b6778c5f7f95/pontoon/base/context_processors.py

We'd need to change DEFAULT_FROM_EMAIL to settings.DEFAULT_FROM_EMAIL in account_disabled.html though.

status=403,
)
else:
# For non-authenticated users, check the session manually
user_id = request.session.get("_auth_user_id")
if user_id:
try:
user = User.objects.get(pk=user_id)
if not user.is_active:
return render(
request,
"account_disabled.html",
{"DEFAULT_FROM_EMAIL": settings.DEFAULT_FROM_EMAIL},
status=403,
)
except User.DoesNotExist:
pass # If the user ID is invalid, ignore it
Comment on lines +175 to +188
Copy link
Collaborator

@mathjazz mathjazz Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly does this code do? If the user is not authenticated, we shouldn't do anything.

Copy link
Contributor Author

@RafaelJohn9 RafaelJohn9 Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathjazz , I was having trouble running the code without this patch, mainly when the user is not active and wishes to login, it does not pass through the AuthMiddleware,

did some digging and found that one of the requirements for a user to be authenticated is that the user must be active

I tried checking if both is_active and is_authenticated is false, however, this sort of thing affects AnonymousUser's meaning they can't browse any pages if you aren't authenticated.

So I updated the patch to have two parts just in case:

  • User is_authenticated and not active
  • User is not active but is an existing user in the database

the path highlighted presents the second part of the patch,


# Continue processing the request
response = self.get_response(request)
return response
6 changes: 6 additions & 0 deletions pontoon/base/templates/account_disabled.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% extends "404.html" %}

{% block title %}Account Disabled{% endblock %}
{% block description %}
Your account has been disabled. If you believe this is a mistake or need further assistance, please contact us at {{ DEFAULT_FROM_EMAIL }}.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the email address a mailto: hyperlink.

{% endblock %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 changes: 17 additions & 0 deletions pontoon/base/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,20 @@ def test_throttle(client, settings):
# Make another request after block duration
response = client.get(url, REMOTE_ADDR=ip_address)
assert response.status_code == 200


@pytest.mark.django_db
def test_AccountDisabledMiddleware(client, member, settings):
member.user.is_active = False
member.user.save()

response = member.client.get("/")
assert response.status_code == 403
assert "account_disabled.html" in [t.name for t in response.templates]

# Ensure the user is authenticated and active
member.user.is_active = True
member.user.save()

response = member.client.get("/")
assert response.status_code == 200
11 changes: 9 additions & 2 deletions pontoon/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ def _default_from_email():
"corsheaders.middleware.CorsMiddleware",
"django.middleware.common.CommonMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
"pontoon.base.middleware.AccountDisabledMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
"pontoon.base.middleware.ThrottleIpMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
Expand Down Expand Up @@ -753,7 +754,10 @@ def _default_from_email():
# cache.
if os.environ.get("MEMCACHE_SERVERS") is not None:
CACHES = {
"default": {"BACKEND": "django_bmemcached.memcached.BMemcached", "OPTIONS": {}}
"default": {
"BACKEND": "django_bmemcached.memcached.BMemcached",
"OPTIONS": {},
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Please restore this change. :)

}
else:
CACHES = {
Expand Down Expand Up @@ -1168,7 +1172,10 @@ def account_username(user):
)
# Used for Community Builder badge
BADGES_PROMOTION_THRESHOLDS = list(
map(int, os.environ.get("BADGES_PROMOTION_THRESHOLDS", "1, 2, 5").split(","))
map(
int,
os.environ.get("BADGES_PROMOTION_THRESHOLDS", "1, 2, 5").split(","),
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Please restore this change. :)

)

DEFAULT_AUTO_FIELD = "django.db.models.AutoField"
Expand Down
2 changes: 2 additions & 0 deletions pontoon/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class LocaleConverter(StringConverter):
page_not_found_view = TemplateView.as_view(template_name="404.html")
too_many_requests_view = TemplateView.as_view(template_name="429.html")
server_error_view = TemplateView.as_view(template_name="500.html")
account_disabled_view = TemplateView.as_view(template_name="account_disabled.html")

urlpatterns = [
# Accounts
Expand All @@ -34,6 +35,7 @@ class LocaleConverter(StringConverter):
path("404/", page_not_found_view),
path("429/", too_many_requests_view),
path("500/", server_error_view),
path("account_disabled/", account_disabled_view),
# Robots.txt
path(
"robots.txt",
Expand Down
Loading