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

Remove global state from users bans #1143

Merged
merged 3 commits into from
Dec 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion misago/threads/api/postendpoints/edits.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def revert_post_endpoint(request, post):
add_acl(request.user, post)

if post.poster:
make_users_status_aware(request.user, [post.poster])
make_users_status_aware(request, [post.poster])

return Response(PostSerializer(post, context={'user': request.user}).data)

Expand Down
4 changes: 2 additions & 2 deletions misago/threads/api/threadposts.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def create(self, request, thread_pk):
post.is_new = True
post.poster.posts = user_posts + 1

make_users_status_aware(request.user, [post.poster])
make_users_status_aware(request, [post.poster])

return Response(PostSerializer(post, context={'user': request.user}).data)
else:
Expand Down Expand Up @@ -141,7 +141,7 @@ def update(self, request, thread_pk, pk=None):
post.edits = post_edits + 1

if post.poster:
make_users_status_aware(request.user, [post.poster])
make_users_status_aware(request, [post.poster])

return Response(PostSerializer(post, context={'user': request.user}).data)
else:
Expand Down
2 changes: 1 addition & 1 deletion misago/threads/viewmodels/posts.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def __init__(self, request, thread, page):
if post.poster:
posters.append(post.poster)

make_users_status_aware(request.user, posters)
make_users_status_aware(request, posters)

if thread.category.acl['can_see_posts_likes']:
add_likes_to_posts(request.user, posts)
Expand Down
2 changes: 1 addition & 1 deletion misago/users/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def change_forgotten_password(request, pk, token):

if user.requires_activation:
raise PasswordChangeFailed(expired_message)
if get_user_ban(user):
if get_user_ban(user, request.cache_versions):
raise PasswordChangeFailed(expired_message)
except PasswordChangeFailed as e:
return Response(
Expand Down
4 changes: 2 additions & 2 deletions misago/users/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def retrieve(self, request, pk=None):
profile = self.get_user(request, pk)

add_acl(request.user, profile)
profile.status = get_user_status(request.user, profile)
profile.status = get_user_status(request, profile)

serializer = UserProfileSerializer(profile, context={'user': request.user})
profile_json = serializer.data
Expand Down Expand Up @@ -199,7 +199,7 @@ def ban(self, request, pk=None):
profile = self.get_user(request, pk)
allow_see_ban_details(request.user, profile)

ban = get_user_ban(profile)
ban = get_user_ban(profile, request.cache_versions)
if ban:
return Response(BanDetailsSerializer(ban).data)
else:
Expand Down
2 changes: 1 addition & 1 deletion misago/users/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def can_see_ban_details(request, profile):
if request.user.is_authenticated:
if request.user.acl_cache['can_see_ban_details']:
from .bans import get_user_ban
return bool(get_user_ban(profile))
return bool(get_user_ban(profile, request.cache_versions))
else:
return False
else:
Expand Down
19 changes: 8 additions & 11 deletions misago/users/bans.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@
from django.utils import timezone
from django.utils.dateparse import parse_datetime

from misago.core import cachebuster

from .constants import BANS_CACHE
from .models import Ban, BanCache


CACHE_SESSION_KEY = 'misago_ip_check'
VERSION_KEY = 'misago_bans'


def get_username_ban(username, registration_only=False):
Expand All @@ -39,7 +36,7 @@ def get_ip_ban(ip, registration_only=False):
return None


def get_user_ban(user):
def get_user_ban(user, cache_versions):
"""
This function checks if user is banned

Expand All @@ -49,21 +46,21 @@ def get_user_ban(user):
"""
try:
ban_cache = user.ban_cache
if not ban_cache.is_valid:
if not ban_cache.is_valid(cache_versions):
_set_user_ban_cache(user)
except BanCache.DoesNotExist:
user.ban_cache = BanCache(user=user)
user.ban_cache = _set_user_ban_cache(user)
user.ban_cache = _set_user_ban_cache(user, cache_versions)

if user.ban_cache.ban:
return user.ban_cache
else:
return None


def _set_user_ban_cache(user):
def _set_user_ban_cache(user, cache_versions):
ban_cache = user.ban_cache
ban_cache.bans_version = cachebuster.get_version(VERSION_KEY)
ban_cache.cache_version = cache_versions[BANS_CACHE]

try:
user_ban = Ban.objects.get_ban(
Expand Down Expand Up @@ -103,7 +100,7 @@ def get_request_ip_ban(request):
found_ban = get_ip_ban(request.user_ip)

ban_cache = request.session[CACHE_SESSION_KEY] = {
'version': cachebuster.get_version(VERSION_KEY),
'version': request.cache_versions[BANS_CACHE],
'ip': request.user_ip,
}

Expand All @@ -128,7 +125,7 @@ def _get_session_bancache(request):
ban_cache = _hydrate_session_cache(ban_cache)
if ban_cache['ip'] != request.user_ip:
return None
if not cachebuster.is_valid(VERSION_KEY, ban_cache['version']):
if ban_cache['version'] != request.cache_versions[BANS_CACHE]:
return None
if ban_cache.get('expires_on'):
if ban_cache['expires_on'] < timezone.today():
Expand Down
2 changes: 1 addition & 1 deletion misago/users/constants.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
BANS_CACHEBUSTER = 'misago_bans'
BANS_CACHE = "bans"
6 changes: 5 additions & 1 deletion misago/users/forms/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def confirm_user_active(self, user):

def confirm_user_not_banned(self, user):
if not user.is_staff:
self.user_ban = get_user_ban(user)
self.user_ban = get_user_ban(user, self.request.cache_versions)
if self.user_ban:
raise ValidationError('', code='banned')

Expand Down Expand Up @@ -62,6 +62,10 @@ class AuthenticationForm(MisagoAuthMixin, BaseAuthenticationForm):
widget=forms.PasswordInput,
)

def __init__(self, *args, request=None, **kwargs):
self.request = request
super().__init__(*args, **kwargs)

def clean(self):
username = self.cleaned_data.get('username')
password = self.cleaned_data.get('password')
Expand Down
8 changes: 5 additions & 3 deletions misago/users/management/commands/invalidatebans.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from django.core.management.base import BaseCommand
from django.utils import timezone

from misago.core import cachebuster
from misago.cache.versions import get_cache_versions

from misago.users.constants import BANS_CACHE
from misago.users.models import Ban, BanCache


Expand All @@ -28,8 +30,8 @@ def handle_bans_caches(self):
expired_count = queryset.count()
queryset.delete()

bans_version = cachebuster.get_version('misago_bans')
queryset = BanCache.objects.filter(bans_version__lt=bans_version)
cache_versions = get_cache_versions()
queryset = BanCache.objects.exclude(cache_version=cache_versions[BANS_CACHE])

expired_count += queryset.count()
queryset.delete()
Expand Down
5 changes: 4 additions & 1 deletion misago/users/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ def process_request(self, request):
if request.user.is_anonymous:
request.user = AnonymousUser()
elif not request.user.is_staff:
if get_request_ip_ban(request) or get_user_ban(request.user):
if (
get_request_ip_ban(request) or
get_user_ban(request.user, request.cache_versions)
):
logout(request)
request.user = AnonymousUser()

Expand Down
10 changes: 1 addition & 9 deletions misago/users/migrations/0003_bans_version_tracker.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
from django.db import migrations


def register_bans_version_tracker(apps, schema_editor):
from misago.core.migrationutils import cachebuster_register_cache
cachebuster_register_cache(apps, "misago_bans")


class Migration(migrations.Migration):
"""Migration superseded by 0016"""

Expand All @@ -14,7 +9,4 @@ class Migration(migrations.Migration):
('misago_core', '0001_initial'),
]

operations = [
# FIXME: remove this operation
migrations.RunPython(register_bans_version_tracker),
]
operations = []
32 changes: 32 additions & 0 deletions misago/users/migrations/0017_move_bans_to_cache_version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 1.11.16 on 2018-11-29 20:28
from django.db import migrations, models

from misago.users.constants import BANS_CACHE


def populate_cache_version(apps, _):
BanCache = apps.get_model("misago_users", "BanCache")
BanCache.objects.all().delete()


class Migration(migrations.Migration):

dependencies = [
('misago_users', '0016_cache_version'),
]

operations = [
migrations.RemoveField(
model_name='bancache',
name='bans_version',
),
migrations.RunPython(
populate_cache_version,
migrations.RunPython.noop,
),
migrations.AddField(
model_name='bancache',
name='cache_version',
field=models.CharField(max_length=8),
),
]
17 changes: 8 additions & 9 deletions misago/users/models/ban.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
from django.utils import timezone
from django.utils.translation import gettext_lazy as _

from misago.core import cachebuster
from misago.users.constants import BANS_CACHEBUSTER
from misago.cache.versions import invalidate_cache
from misago.users.constants import BANS_CACHE


class BansManager(models.Manager):
Expand All @@ -29,7 +29,7 @@ def get_email_ban(self, email, registration_only=False):
)

def invalidate_cache(self):
cachebuster.invalidate(BANS_CACHEBUSTER)
invalidate_cache(BANS_CACHE)

def get_ban(self, username=None, email=None, ip=None, registration_only=False):
checks = []
Expand Down Expand Up @@ -131,7 +131,7 @@ class BanCache(models.Model):
blank=True,
on_delete=models.SET_NULL,
)
bans_version = models.PositiveIntegerField(default=0)
cache_version = models.CharField(max_length=8)
user_message = models.TextField(null=True, blank=True)
staff_message = models.TextField(null=True, blank=True)
expires_on = models.DateTimeField(null=True, blank=True)
Expand All @@ -157,9 +157,8 @@ def get_serialized_message(self):
def is_banned(self):
return bool(self.ban)

@property
def is_valid(self):
version_is_valid = cachebuster.is_valid(BANS_CACHEBUSTER, self.bans_version)
expired = self.expires_on and self.expires_on < timezone.now()
def is_valid(self, cache_versions):
is_versioned = self.cache_version == cache_versions[BANS_CACHE]
is_expired = self.expires_on and self.expires_on < timezone.now()

return version_is_valid and not expired
return is_versioned and not is_expired
47 changes: 24 additions & 23 deletions misago/users/online/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,27 @@
ACTIVITY_CUTOFF = timedelta(minutes=2)


def get_user_status(viewer, user):

def make_users_status_aware(request, users, fetch_state=False):
users_dict = {}
for user in users:
users_dict[user.pk] = user

if fetch_state:
# Fill ban cache on users
for ban_cache in BanCache.objects.filter(user__in=users_dict.keys()):
users_dict[ban_cache.user_id].ban_cache = ban_cache

# Fill user online trackers
for online_tracker in Online.objects.filter(user__in=users_dict.keys()):
users_dict[online_tracker.user_id].online_tracker = online_tracker

# Fill user states
for user in users:
user.status = get_user_status(request, user)


def get_user_status(request, user):
user_status = {
'is_banned': False,
'is_hidden': user.is_hiding_presence,
Expand All @@ -21,14 +41,14 @@ def get_user_status(viewer, user):
'last_click': user.last_login or user.joined_on,
}

user_ban = get_user_ban(user)
user_ban = get_user_ban(user, request.cache_versions)
if user_ban:
user_status['is_banned'] = True
user_status['banned_until'] = user_ban.expires_on

try:
online_tracker = user.online_tracker
is_hidden = user.is_hiding_presence and not viewer.acl_cache['can_see_hidden_users']
is_hidden = user.is_hiding_presence and not request.user.acl_cache['can_see_hidden_users']

if online_tracker and not is_hidden:
if online_tracker.last_click >= timezone.now() - ACTIVITY_CUTOFF:
Expand All @@ -38,7 +58,7 @@ def get_user_status(viewer, user):
pass

if user_status['is_hidden']:
if viewer.acl_cache['can_see_hidden_users']:
if request.user.acl_cache['can_see_hidden_users']:
user_status['is_hidden'] = False
if user_status['is_online']:
user_status['is_online_hidden'] = True
Expand All @@ -55,22 +75,3 @@ def get_user_status(viewer, user):
user_status['is_offline'] = True

return user_status


def make_users_status_aware(viewer, users, fetch_state=False):
users_dict = {}
for user in users:
users_dict[user.pk] = user

if fetch_state:
# Fill ban cache on users
for ban_cache in BanCache.objects.filter(user__in=users_dict.keys()):
users_dict[ban_cache.user_id].ban_cache = ban_cache

# Fill user online trackers
for online_tracker in Online.objects.filter(user__in=users_dict.keys()):
users_dict[online_tracker.user_id].online_tracker = online_tracker

# Fill user states
for user in users:
user.status = get_user_status(viewer, user)
2 changes: 1 addition & 1 deletion misago/users/social/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def validate_user_not_banned(strategy, details, backend, user=None, *args, **kwa
if not user or user.is_staff:
return None

user_ban = get_user_ban(user)
user_ban = get_user_ban(user, strategy.request.cache_versions)
if user_ban:
raise SocialAuthBanned(backend, user_ban)

Expand Down