Skip to content

Commit

Permalink
check staff group rather than is_staff for elevated perms (mozilla#5947)
Browse files Browse the repository at this point in the history
* check staff group rather than is_staff for elevated perms

* move the registration one-liners to the top of the file

* fix comment
  • Loading branch information
escattone authored Apr 18, 2024
1 parent 1365233 commit c402587
Show file tree
Hide file tree
Showing 17 changed files with 45 additions and 47 deletions.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{% if user.is_staff and user.has_perm('announcements.add_announcement') %}
{% if in_staff_group(user) and user.has_perm('announcements.add_announcement') %}
<a class="add" href="{{ url('admin:announcements_announcement_add') }}">{{ _('Add announcement') }}</a>
{% endif %}
{% if announcements %}
<ul id="doc-content" class="announcements">
{% for a in announcements %}
<li>
{% if user.is_staff and user.has_perm('announcements.change_announcement') %}
{% if in_staff_group(user) and user.has_perm('announcements.change_announcement') %}
<a class="edit" href="{{ url('admin:announcements_announcement_change', a.id) }}">{{ _('Edit') }}</a>
{% endif %}
{{ a.content|wiki_to_html }}
Expand Down
2 changes: 1 addition & 1 deletion kitsune/groups/jinja2/groups/list.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

{% block content %}
<article id="group-list" class="main sumo-page-section">
{% if user.is_staff and user.has_perm('groups.add_groupprofile') %}
{% if in_staff_group(user) and user.has_perm('groups.add_groupprofile') %}
<div class="sumo-button-wrap">
<a class="sumo-button primary-button add" href="{{ url('admin:groups_groupprofile_add') }}">{{ _('Add group profile') }}</a>
</div>
Expand Down
2 changes: 1 addition & 1 deletion kitsune/groups/jinja2/groups/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{% endif %}
</section>
<section id="main-area">
{% if user.is_staff and user.has_perm('groups.change_groupprofile') %}
{% if in_staff_group(user) and user.has_perm('groups.change_groupprofile') %}
<a class="edit" href="{{ url('admin:groups_groupprofile_change', profile.id) }}">{{ _('Edit in admin') }}</a>
{% endif %}
<h1>{{ profile.group.name }}</h1>
Expand Down
5 changes: 3 additions & 2 deletions kitsune/kbadge/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.urls import reverse

from kitsune.kbadge.signals import badge_will_be_awarded, badge_was_awarded
from kitsune.sumo.utils import in_staff_group


IMG_MAX_SIZE = getattr(settings, "BADGER_IMG_MAX_SIZE", (256, 256))
Expand Down Expand Up @@ -266,10 +267,10 @@ def allows_award_to(self, user):
return True
if user.is_anonymous:
return False
if user.is_staff or user.is_superuser:
return True
if user == self.creator:
return True
if user.is_superuser or in_staff_group(user):
return True

# TODO: List of delegates for whom awarding is allowed

Expand Down
2 changes: 1 addition & 1 deletion kitsune/messages/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def __init__(self, *args, **kwargs):
self.user = kwargs.pop("user")
super(MessageForm, self).__init__(*args, **kwargs)

# If the user is_staff, the placholder text needs to be updated
# If the user is a member of the staff group, the placholder text needs to be updated.
if self.user and self.user.profile.in_staff_group:
self.fields["to"].widget.attrs["placeholder"] = "Search for Users or Groups"

Expand Down
2 changes: 1 addition & 1 deletion kitsune/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ def JINJA_CONFIG():
)


# Our group with permissions to see hidden docs and message groups
# Our group with the power to see hidden docs and message groups.
STAFF_GROUP = "Staff"

# CSRF
Expand Down
22 changes: 6 additions & 16 deletions kitsune/sumo/templatetags/jinja_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
from kitsune.products.models import Product
from kitsune.sumo import parser
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import is_trusted_user as is_trusted_user_func
from kitsune.sumo.utils import webpack_static as webpack_static_func
from kitsune.sumo.utils import in_staff_group
from kitsune.sumo.utils import in_staff_group, is_trusted_user, webpack_static
from kitsune.users.models import Profile
from kitsune.wiki.showfor import showfor_data as _showfor_data

Expand All @@ -47,6 +45,11 @@ class DateTimeFormatError(Exception):
pass


library.global_function(webpack_static)
library.global_function(is_trusted_user)
library.global_function(in_staff_group)


@library.filter
def paginator(pager):
"""Render list of pages."""
Expand Down Expand Up @@ -467,11 +470,6 @@ def static(path):
return ""


@library.global_function
def webpack_static(source_path):
return webpack_static_func(source_path)


@library.global_function
def now():
return datetime.datetime.now()
Expand Down Expand Up @@ -563,11 +561,3 @@ def show_header_fx_download(context):
return product.slug != "firefox"
else:
return True


@library.global_function
def is_trusted_user(user):
return is_trusted_user_func(user)


library.global_function(in_staff_group)
18 changes: 9 additions & 9 deletions kitsune/sumo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,20 +353,20 @@ def webpack_static(source_path):
return url


def is_trusted_user(user: User) -> bool:
def is_trusted_user(user: User | None) -> bool:
"""Given a user ID, checks for group membership.
If a user belongs to one of the trusted groups as defined in the project
settings, then is considered a trusted user.
"""
if not user or not user.is_authenticated:
return False
return any(
[
user.groups.filter(name__in=settings.TRUSTED_GROUPS).exists(),
user.is_superuser,
user.is_staff,
]
return bool(
user
and user.is_authenticated
and (
user.is_superuser
or user.profile.in_staff_group
or user.groups.filter(name__in=settings.TRUSTED_GROUPS).exists()
)
)


Expand Down
2 changes: 1 addition & 1 deletion kitsune/users/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def update_user(self, user, claims):

# There is a change in the email in Mozilla accounts. Let's update user's email
# unless we have a superuser
if email and (email != user.email) and not user.is_staff:
if email and (email != user.email) and not user.profile.in_staff_group:
if User.objects.exclude(id=user.id).filter(email=email).exists():
if request:
msg = _(
Expand Down
2 changes: 1 addition & 1 deletion kitsune/wiki/jinja2/wiki/edit_metadata.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ <h1 class="sumo-page-heading">{{ _('<em>Editing Metadata For:</em><br>{title}')|
{{ errorlist(document_form) }}
<form action="" method="post" data-json-url="{{ url('wiki.json') }}" data-document-id="{{ document.id }}">
{% csrf_token %}
{% if request.user.is_staff %}
{% if in_staff_group(request.user) %}
<div class="mzp-c-emphasis-box field">
<fieldset>
<legend>{{ _("Restrict Visibility") }}</legend>
Expand Down
2 changes: 1 addition & 1 deletion kitsune/wiki/jinja2/wiki/new_document.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ <h1 class="sumo-page-heading">{{ _('Create a New Knowledge Base Article') }}</h1
<form action="" method="post" data-json-url="{{ url('wiki.json') }}" id="id_new_document_form">
{% csrf_token %}
<ul>
{% if request.user.is_staff %}
{% if in_staff_group(request.user) %}
<li class="mzp-c-emphasis-box field has-large-textarea">
<fieldset>
<legend>{{ _("Restrict Visibility") }}</legend>
Expand Down
7 changes: 4 additions & 3 deletions kitsune/wiki/managers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.db import models
from django.db.models import Exists, OuterRef, Q

from kitsune.sumo.utils import in_staff_group
from kitsune.wiki.permissions import can_delete_documents_or_review_revisions


Expand Down Expand Up @@ -32,7 +33,7 @@ def unrestricted(self, user=None, **kwargs):
restricted. A translation (i.e., a document with a parent), follows its
parent's restrictions.
"""
if user and (user.is_staff or user.is_superuser):
if user and (user.is_superuser or in_staff_group(user)):
# Staff and superusers are never restricted.
return self.filter(**kwargs)

Expand Down Expand Up @@ -89,8 +90,8 @@ def visible(self, user=None, **kwargs):
return qs.filter(**{f"{prefix}current_revision__isnull": False})

if not (
user.is_staff
or user.is_superuser
user.is_superuser
or in_staff_group(user)
or can_delete_documents_or_review_revisions(user, locale=locale)
):
# Authenticated users without permission to see documents that
Expand Down
5 changes: 3 additions & 2 deletions kitsune/wiki/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from kitsune.sumo.i18n import split_into_language_and_path
from kitsune.sumo.models import LocaleField, ModelBase
from kitsune.sumo.urlresolvers import reverse
from kitsune.sumo.utils import in_staff_group
from kitsune.tags.models import BigVocabTaggableMixin
from kitsune.tidings.models import NotificationsMixin
from kitsune.wiki.config import (
Expand Down Expand Up @@ -720,8 +721,8 @@ def is_visible_for(self, user, use_cache=True):
"""
return self.is_unrestricted_for(user, use_cache=use_cache) and bool(
self.current_revision
or user.is_staff
or user.is_superuser
or in_staff_group(user)
or (
user.is_authenticated
and (
Expand Down Expand Up @@ -762,7 +763,7 @@ def is_unrestricted_for(self, user, use_cache=True):
# Translations follow their parent's restrictions.
return self.parent.is_unrestricted_for(user, use_cache=use_cache)

if user.is_staff or user.is_superuser:
if user.is_superuser or in_staff_group(user):
return True

is_restricted = self.is_restricted if use_cache else self.get_is_restricted()
Expand Down
6 changes: 4 additions & 2 deletions kitsune/wiki/tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from django.conf import settings

from kitsune.sumo.tests import TestCase
from kitsune.sumo.urlresolvers import reverse
from kitsune.users.tests import GroupFactory, UserFactory
Expand All @@ -9,9 +11,9 @@ def setUp(self):
super().setUp()
self.group1 = GroupFactory(name="group1")
self.group2 = GroupFactory(name="group2")
self.group3 = GroupFactory(name="staff")
self.group3 = GroupFactory(name="group3")
self.user1 = UserFactory(groups=[self.group1, self.group2])
self.staff = UserFactory(is_staff=True, groups=[self.group3])
self.staff = UserFactory(groups=[GroupFactory(name=settings.STAFF_GROUP)])
self.doc1 = DocumentFactory()
self.doc2 = ApprovedRevisionFactory().document
self.doc3 = ApprovedRevisionFactory(document__restrict_to_groups=[self.group3]).document
Expand Down
3 changes: 2 additions & 1 deletion kitsune/wiki/tests/test_facets.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.conf import settings
from django.contrib.auth.models import AnonymousUser

from kitsune.products.tests import ProductFactory, TopicFactory
Expand All @@ -22,7 +23,7 @@ def setUp(self):
self.group4 = GroupFactory(name="group4")
self.user1 = UserFactory(groups=[self.group1, self.group4])
self.user2 = UserFactory(groups=[self.group2, self.group3])
self.staff = UserFactory(is_staff=True)
self.staff = UserFactory(groups=[GroupFactory(name=settings.STAFF_GROUP)])
self.anonymous = AnonymousUser()

# Create products
Expand Down
3 changes: 2 additions & 1 deletion kitsune/wiki/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import urllib.parse
from datetime import datetime

from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from django.core.exceptions import ValidationError
from taggit.models import TaggedItem
Expand Down Expand Up @@ -412,7 +413,7 @@ def setUp(self):
self.anonymous = AnonymousUser()
self.user1 = UserFactory(groups=[self.group1, self.group4])
self.user2 = UserFactory(groups=[self.group2, self.group3])
self.staff = UserFactory(is_staff=True)
self.staff = UserFactory(groups=[GroupFactory(name=settings.STAFF_GROUP)])
self.superuser = UserFactory(is_superuser=True)
self.reviewer = UserFactory()
add_permission(self.reviewer, Revision, "review_revision")
Expand Down
5 changes: 3 additions & 2 deletions kitsune/wiki/tests/test_notifications.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
from django.conf import settings
from django.core import mail

from kitsune.products.tests import ProductFactory
Expand Down Expand Up @@ -111,7 +112,7 @@ def test_when_restricted(self):
when the revision is for a restricted document.
"""
rw = _set_up_ready_watcher()
creator = UserFactory(is_staff=True)
creator = UserFactory(groups=[GroupFactory(name=settings.STAFF_GROUP)])
self._review_revision(
is_ready=True,
creator=creator,
Expand Down Expand Up @@ -337,7 +338,7 @@ def setUp(self):
add_permission(self.watcher2, Revision, "review_revision")
ReviewableRevisionInLocaleEvent.notify(self.watcher1, locale="en-US")
ReviewableRevisionInLocaleEvent.notify(self.watcher2, locale="en-US")
creator = UserFactory(is_staff=True)
creator = UserFactory(groups=[GroupFactory(name=settings.STAFF_GROUP)])
self.client.login(username=creator.username, password="testpass")

def test_when_restricted(self):
Expand Down

0 comments on commit c402587

Please sign in to comment.