diff --git a/hypha/apply/funds/forms.py b/hypha/apply/funds/forms.py index 0a4d30075794ab215675d854dcb993d3c11e004d..1259a1762d3cade41651e46c8672ba909a27779d 100644 --- a/hypha/apply/funds/forms.py +++ b/hypha/apply/funds/forms.py @@ -315,9 +315,18 @@ class UpdateReviewersForm(ApplicationSubmissionModelForm): class BatchUpdateReviewersForm(forms.Form): submissions = forms.CharField(widget=forms.HiddenInput(attrs={'class': 'js-submissions-id'})) + external_reviewers = forms.ModelMultipleChoiceField( + queryset=User.objects.reviewers().only('pk', 'full_name'), + widget=Select2MultiCheckboxesWidget(attrs={'data-placeholder': 'Reviewers'}), + label=_('External Reviewers'), + required=False, + ) def __init__(self, *args, user=None, round=None, **kwargs): super().__init__(*args, **kwargs) + self.user = user + + self.fields = OrderedDict(self.fields) self.role_fields = {} field_data = make_role_reviewer_fields() @@ -327,6 +336,8 @@ class BatchUpdateReviewersForm(forms.Form): self.fields[field_name] = data['field'] self.role_fields[field_name] = data['role'] + self.fields.move_to_end('external_reviewers') + def clean_submissions(self): value = self.cleaned_data['submissions'] submission_ids = [int(submission) for submission in value.split(',')] @@ -334,6 +345,16 @@ class BatchUpdateReviewersForm(forms.Form): def clean(self): cleaned_data = super().clean() + external_reviewers = self.cleaned_data['external_reviewers'] + submissions = self.cleaned_data['submissions'] + if external_reviewers: + # User needs to be superuser or lead of all selected submissions. + if self.user_cant_alter_submissions_external_reviewers(submissions, self.user): + self.add_error('external_reviewers', _("Only Lead can change the External Reviewers")) + # If user is trying to change the external reviewers for submissions that doesn't have workflow with external_review stage. + elif self.submissions_cant_have_external_reviewers(submissions): + self.add_error('external_reviewers', _('External Reviewers cannot be selected because of the application workflow')) + role_reviewers = [ user for field, user in self.cleaned_data.items() @@ -346,8 +367,21 @@ class BatchUpdateReviewersForm(forms.Form): return cleaned_data + def submissions_cant_have_external_reviewers(self, submissions): + for submission in submissions: + if not submission.stage.has_external_review: + return True + return False + + def user_cant_alter_submissions_external_reviewers(self, submissions, user): + for submission in submissions: + if user != submission.lead and not user.is_superuser: + return True + return False + def save(self): submissions = self.cleaned_data['submissions'] + external_reviewers = self.cleaned_data['external_reviewers'] assigned_roles = { role: self.cleaned_data[field] for field, role in self.role_fields.items() @@ -356,6 +390,12 @@ class BatchUpdateReviewersForm(forms.Form): if reviewer: AssignedReviewers.objects.update_role(role, reviewer, *submissions) + for submission in submissions: + AssignedReviewers.objects.bulk_create_reviewers( + [reviewer for reviewer in external_reviewers], + submission, + ) + return None diff --git a/hypha/apply/users/admin_views.py b/hypha/apply/users/admin_views.py index b1e8d9c6e24bd6ab6b9ce28f35b81bb63bb75555..17c8da5835ebda9c2525cb5ba735abf5247e78a9 100644 --- a/hypha/apply/users/admin_views.py +++ b/hypha/apply/users/admin_views.py @@ -3,7 +3,8 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.core.paginator import Paginator from django.db.models import Q -from django.shortcuts import render +from django.shortcuts import get_object_or_404 +from django.template.response import TemplateResponse from django.utils.translation import gettext as _ from django.views.decorators.vary import vary_on_headers from wagtail.admin.auth import any_permission_required @@ -50,7 +51,7 @@ class UserFilterSet(WagtailFilterSet): @any_permission_required(add_user_perm, change_user_perm, delete_user_perm) @vary_on_headers('X-Requested-With') -def index(request): +def index(request, *args): """ Override wagtail's users index view to filter by full_name https://github.com/wagtail/wagtail/blob/af69cb4a544a1b9be1339546be62ff54b389730e/wagtail/users/views/users.py#L47 @@ -58,9 +59,15 @@ def index(request): q = None is_searching = False + group = None + group_filter = Q() + if args: + group = get_object_or_404(Group, id=args[0]) + group_filter = Q(groups=group) if args else Q() + model_fields = [f.name for f in User._meta.get_fields()] - if request.GET.get('q', None): + if 'q' in request.GET: form = SearchForm(request.GET, placeholder=_("Search users")) if form.is_valid(): q = form.cleaned_data['q'] @@ -84,15 +91,17 @@ def index(request): if 'full_name' in model_fields: conditions |= Q(full_name__icontains=term) - users = User.objects.filter(conditions) + users = User.objects.filter(group_filter & conditions) else: form = SearchForm(placeholder=_("Search users")) if not is_searching: - users = User.objects.all().order_by('-is_active', 'full_name') + users = User.objects.filter(group_filter).order_by('-is_active', 'full_name') - filters = UserFilterSet(request.GET, queryset=users, request=request) - users = filters.qs + filters = None + if not group: + filters = UserFilterSet(request.GET, queryset=users, request=request) + users = filters.qs if 'ordering' in request.GET: ordering = request.GET['ordering'] @@ -105,11 +114,11 @@ def index(request): ordering = 'name' user_count = users.count() - paginator = Paginator(users, per_page=20) + paginator = Paginator(users.select_related('wagtail_userprofile'), per_page=20) users = paginator.get_page(request.GET.get('p')) if request.headers.get('x-requested-with') == 'XMLHttpRequest': - return render(request, "wagtailusers/users/results.html", { + return TemplateResponse(request, "wagtailusers/users/results.html", { 'users': users, 'user_count': user_count, 'is_searching': is_searching, @@ -120,7 +129,8 @@ def index(request): 'model_name': User._meta.model_name, }) else: - return render(request, "wagtailusers/users/index.html", { + return TemplateResponse(request, "wagtailusers/users/index.html", { + 'group': group, 'search_form': form, 'users': users, 'user_count': user_count, diff --git a/hypha/apply/users/templates/two_factor/admin/disable.html b/hypha/apply/users/templates/two_factor/admin/disable.html new file mode 100644 index 0000000000000000000000000000000000000000..a3fc26d465297ccff74832a1c6b19fcd1aa44c13 --- /dev/null +++ b/hypha/apply/users/templates/two_factor/admin/disable.html @@ -0,0 +1,31 @@ +<!-- It is a custom template built on the top of wagtail base, to confirm 2FA disable process with the user's Password --> +{% extends "wagtailadmin/base.html" %} +{% load i18n static wagtailadmin_tags %} + +{% block content %} + + {% trans "Disabling 2FA" as editing_str %} + {% include "wagtailadmin/shared/header.html" with title=editing_str subtitle=user.get_username merged=1 tabbed=1 icon="user" %} + + <form class="form" action="" method="POST" novalidate> + <div class="tab-content"> + {% csrf_token %} + + <section id="account" class="active nice-padding"> + <p> Are you sure you want to disable the Two Factor Authentication for this user? Please type your password to confirm.</p> + + <ul class="fields"> + {% block fields %} + {% include "wagtailadmin/shared/field_as_li.html" with field=form.password %} + {% endblock %} + + <li> + <button class="button button--primary" type="submit">{% trans 'Disable 2FA' %}</button> + </li> + </ul> + </section> + </div> + </form> + + +{% endblock %} diff --git a/hypha/apply/users/templates/wagtailusers/users/edit.html b/hypha/apply/users/templates/wagtailusers/users/edit.html index 6de2d3c9c4d17c60727cf4aaef6f8c11a9ca726b..6ee179e89dfbad85809d3869479de33d985219a4 100644 --- a/hypha/apply/users/templates/wagtailusers/users/edit.html +++ b/hypha/apply/users/templates/wagtailusers/users/edit.html @@ -49,8 +49,12 @@ <a href="{% url 'wagtailusers_users:delete' user.pk %}" class="button button-secondary no">{% trans "Delete user" %}</a> {% endif %} <!-- Add a custom button to user account edit form --> - {% user_2fa_enabled as is_2fa_enabled %} - <a href="{% if is_2fa_enabled %}{% url 'users:two_factor_reset' user.pk %}{% else %}javascript:void(0){% endif %}" class="button {% if not is_2fa_enabled %}disabled{% endif %}" {% if not is_2fa_enabled %} title="{% blocktranslate %} {{ user }}'s 2FA security has been disabled {% endblocktranslate %}" {% endif %}>{% trans "Disable 2FA" %}</a> + {% user_2fa_enabled user as is_2fa_enabled %} + {% if is_2fa_enabled %} + <a href="{% url 'users:two_factor_reset' user.pk %}" class="button">{% trans "Disable 2FA" %}</a> + {% else %} + <button type="button" title="{% trans "This account do not have 2FA enabled." %}" class="button" disabled>{% trans "Disable 2FA" %}</button> + {% endif %} </li> </ul> </section> diff --git a/hypha/apply/users/templates/wagtailusers/users/results.html b/hypha/apply/users/templates/wagtailusers/users/results.html index 745afe7c11cf18048e497ad56d305e2e7758ac34..61fcec4dc56068515889e9a0fd08ac60a8bc8441 100644 --- a/hypha/apply/users/templates/wagtailusers/users/results.html +++ b/hypha/apply/users/templates/wagtailusers/users/results.html @@ -1,5 +1,5 @@ {% load i18n wagtailadmin_tags %} -<div class="users-list users-list--has-filters"> +<div class="users-list{% if filters %} users-list--has-filters{% endif %}"> <div class="users-list__results"> {% if users %} <h2 role="alert"> @@ -33,15 +33,17 @@ {% endif %} {% endif %} </div> - <div class="users-list__filters"> - <h2>{% trans 'Filter' %}</h2> - <form method="get"> - {% for filter in filters.form %} - {{ filter.label_tag }} - {{ filter }} - {{ filter.errors }} - {% endfor %} - <button class="button button-longrunning" type="submit">{% icon name="spinner" %}{% trans 'Apply filters' %}</button> - </form> - </div> + {% if filters %} + <div class="users-list__filters"> + <h2>{% trans 'Filter' %}</h2> + <form method="get"> + {% for filter in filters.form %} + {{ filter.label_tag }} + {{ filter }} + {{ filter.errors }} + {% endfor %} + <button class="button button-longrunning" type="submit">{% icon name="spinner" %}{% trans 'Apply filters' %}</button> + </form> + </div> + {% endif %} </div> diff --git a/hypha/apply/users/templatetags/users_tags.py b/hypha/apply/users/templatetags/users_tags.py index 06d353d10a7e55be2f01c593cabcca5278916f12..3cdc2d9a4ba77fbd0d0ffa9123cc4229bc58116f 100644 --- a/hypha/apply/users/templatetags/users_tags.py +++ b/hypha/apply/users/templatetags/users_tags.py @@ -28,10 +28,9 @@ def can_use_oauth(context): return can_use_oauth_check(user) -@register.simple_tag(takes_context=True) -def user_2fa_enabled(context): +@register.simple_tag +def user_2fa_enabled(user): """Checking if 2FA devices exist for the user""" - user = context.get('user') if len(list(devices_for_user(user))): return True return False diff --git a/hypha/apply/users/tests/test_views.py b/hypha/apply/users/tests/test_views.py index a1bea58c49ca8ad035a910cffa32cff44a35c8e3..52f70dbc6a04496f6f46c8ffa2c638bcf0fe9ade 100644 --- a/hypha/apply/users/tests/test_views.py +++ b/hypha/apply/users/tests/test_views.py @@ -74,9 +74,9 @@ class TestBecome(TestCase): response = self.client.post(url, {'user_pk': target.pk}, follow=True, secure=True) return response - def test_staff_can_become_user(self): + def test_staff_cannot_become_user(self): response = self.become_request(self.staff, self.user) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 403) def test_staff_cannot_become_superuser(self): response = self.become_request(self.staff, self.superuser) diff --git a/hypha/apply/users/urls.py b/hypha/apply/users/urls.py index 31899c76eb957f4d79199a0e560f8e9ec1835f35..ac4a14231e24551bbef53ac535485be2ec9eaadd 100644 --- a/hypha/apply/users/urls.py +++ b/hypha/apply/users/urls.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.contrib.auth import views as auth_views from django.urls import include, path, reverse_lazy @@ -8,10 +9,10 @@ from .views import ( EmailChangeDoneView, EmailChangePasswordView, LoginView, + TWOFAAdminDisableView, TWOFABackupTokensPasswordView, TWOFADisableView, TWOFARequiredMessageView, - TWOFAResetView, become, create_password, oauth, @@ -38,7 +39,6 @@ public_urlpatterns = [ urlpatterns = [ path('account/', include([ path('', AccountView.as_view(), name='account'), - path('become/', become, name='become'), path('password/', include([ path('', EmailChangePasswordView.as_view(), name='email_change_confirm_password'), path( @@ -89,10 +89,15 @@ urlpatterns = [ path('two_factor/required/', TWOFARequiredMessageView.as_view(), name='two_factor_required'), path('two_factor/backup_tokens/password/', TWOFABackupTokensPasswordView.as_view(), name='backup_tokens_password'), path('two_factor/disable/', TWOFADisableView.as_view(), name='disable'), - path('two_factor/reset/<str:user_id>/', TWOFAResetView.as_view(), name='two_factor_reset'), + path('two_factor/admin/disable/<str:user_id>/', TWOFAAdminDisableView.as_view(), name='two_factor_reset'), path('confirmation/done/', EmailChangeDoneView.as_view(), name="confirm_link_sent"), path('confirmation/<uidb64>/<token>/', EmailChangeConfirmationView.as_view(), name="confirm_email"), path('activate/', create_password, name="activate_password"), path('oauth', oauth, name='oauth'), ])), ] + +if settings.HIJACK_ENABLE: + urlpatterns += [ + path('account/become/', become, name='become'), + ] diff --git a/hypha/apply/users/views.py b/hypha/apply/users/views.py index 4df398dbe397072d94331c0f8ce155f7863d6e85..adeb4c98abccb60c7ac3787cb3bb102ea4b57a6d 100644 --- a/hypha/apply/users/views.py +++ b/hypha/apply/users/views.py @@ -91,7 +91,7 @@ class AccountView(UpdateView): return reverse_lazy('users:account') def get_context_data(self, **kwargs): - if self.request.user.is_superuser: + if self.request.user.is_superuser and settings.HIJACK_ENABLE: swappable_form = BecomeUserForm() else: swappable_form = None @@ -161,7 +161,10 @@ class EmailChangeDoneView(TemplateView): @login_required() def become(request): - if not request.user.is_apply_staff: + if not settings.HIJACK_ENABLE: + raise Http404(_('Hijack feature is not enabled.')) + + if not request.user.is_superuser: raise PermissionDenied() id = request.POST.get('user_pk') @@ -309,12 +312,12 @@ class TWOFADisableView(TwoFactorDisableView): @method_decorator(permission_required(change_user_perm, raise_exception=True), name='dispatch') -class TWOFAResetView(FormView): +class TWOFAAdminDisableView(FormView): """ - View for PasswordForm to confirm the Disable 2FA process. + View for PasswordForm to confirm the Disable 2FA process on wagtail admin. """ form_class = TWOFAPasswordForm - template_name = 'two_factor/reset.html' + template_name = 'two_factor/admin/disable.html' user = None def get_form_kwargs(self): @@ -326,7 +329,7 @@ class TWOFAResetView(FormView): return kwargs def get_form(self, form_class=None): - form = super(TWOFAResetView, self).get_form(form_class=form_class) + form = super(TWOFAAdminDisableView, self).get_form(form_class=form_class) form.fields['password'].label = "Password" return form @@ -339,7 +342,7 @@ class TWOFAResetView(FormView): return reverse('wagtailusers_users:edit', kwargs={'user_id': self.user.id}) def get_context_data(self, **kwargs): - ctx = super(TWOFAResetView, self).get_context_data(**kwargs) + ctx = super(TWOFAAdminDisableView, self).get_context_data(**kwargs) ctx['user'] = self.user return ctx diff --git a/hypha/apply/users/wagtail_hooks.py b/hypha/apply/users/wagtail_hooks.py index ea7b6e59b7a2bed712eabf37e0393ad457a6562c..b3dfc4512ec4f8cee46a55d2069980a59fd6c66e 100644 --- a/hypha/apply/users/wagtail_hooks.py +++ b/hypha/apply/users/wagtail_hooks.py @@ -10,6 +10,7 @@ from .admin_views import index def register_admin_urls(): return [ re_path(r'^users/$', index, name='index'), + re_path(r'^groups/(\d+)/users/$', index, name='index'), ] diff --git a/hypha/settings/base.py b/hypha/settings/base.py index 8eb1f01fade095472701ffc1c85bd721e15fb794..845ef28b72e2c0ee0a2f5b115ce16612e3a3ea7b 100644 --- a/hypha/settings/base.py +++ b/hypha/settings/base.py @@ -143,10 +143,10 @@ MIDDLEWARE = [ 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django_referrer_policy.middleware.ReferrerPolicyMiddleware', 'django_otp.middleware.OTPMiddleware', + 'hypha.apply.users.middleware.TwoFactorAuthenticationMiddleware', 'hijack.middleware.HijackUserMiddleware', - 'hypha.apply.users.middleware.TwoFactorAuthenticationMiddleware', 'hypha.apply.users.middleware.SocialAuthExceptionMiddleware', 'wagtail.contrib.redirects.middleware.RedirectMiddleware', @@ -497,6 +497,7 @@ FILE_ALLOWED_EXTENSIONS = ['doc', 'docx', 'odp', 'ods', 'odt', 'pdf', 'ppt', 'pp FILE_ACCEPT_ATTR_VALUE = ', '.join(['.' + ext for ext in FILE_ALLOWED_EXTENSIONS]) # Hijack Settings +HIJACK_ENABLE = env.bool('HIJACK_ENABLE', False) HIJACK_LOGIN_REDIRECT_URL = '/dashboard/' HIJACK_LOGOUT_REDIRECT_URL = '/account/' HIJACK_DECORATOR = 'hypha.apply.users.decorators.superuser_decorator' diff --git a/hypha/settings/test.py b/hypha/settings/test.py index 1d913795abb604266204c36627b4f05d42358e15..e4936bbfb355f67645f8f151eb9ab0b6ba5bd2ca 100644 --- a/hypha/settings/test.py +++ b/hypha/settings/test.py @@ -9,6 +9,8 @@ logging.disable(logging.CRITICAL) SECRET_KEY = 'NOT A SECRET' +HIJACK_ENABLE = True + PROJECTS_ENABLED = True PROJECTS_AUTO_CREATE = True diff --git a/public/sandbox_db.dump b/public/sandbox_db.dump index 8b24fd9194e60996e843bb56a057f7297acde19d..06edaee350b1ae254fff6de5f7f35cb88cddb11c 100644 Binary files a/public/sandbox_db.dump and b/public/sandbox_db.dump differ