Skip to content

Commit

Permalink
Add rolespermissions module, refractor the group and delete submissio…
Browse files Browse the repository at this point in the history
…n to use it (#4151)

Updates group management:

- **Centralized Group Definition:** Instead of creating groups within
migration files, which can be cumbersome to maintain, group definitions
are now managed through the AbstractRoles model. These roles are
synchronized using the `python manage.py sync_roles` command. This
ensures no existing groups or their associated permissions are deleted.

- **Module Renaming**: The `hypha.apply.users.groups` module has been
renamed to `hypha.apply.users.roles` to reflect the shift from
group-based to role-based permissions. This aligns with upcoming changes
utilizing the `rolepermissions` module.

- **Streamlined Group Descriptions**: The GroupDesc model is removed.
Instead, help text can be directly defined within the role itself. This
simplifies management and allows for translation of group descriptions.

**This is the first of a series of pull requests aimed at refactoring
the permissions system.**

As a sample implementation, converts the delete submission to use this
role-permissions. See:
8239c21

## Test Steps

 - make sure the migrations run fine.
- groups are create correctly via `python manage.py sync_roles`, it
should also keep the existing groups.
 -  groups description is rendered in the admin.
 - ensure delete submission/application is working as expected
  • Loading branch information
theskumar authored Nov 7, 2024
1 parent d340e87 commit 8f16bd2
Show file tree
Hide file tree
Showing 60 changed files with 315 additions and 479 deletions.
2 changes: 1 addition & 1 deletion Procfile
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
release: python manage.py migrate --noinput && python manage.py clear_cache --cache=default
release: python manage.py migrate --noinput && python manage.py clear_cache --cache=default && python manage.py sync_roles
web: gunicorn hypha.wsgi:application --log-file -
10 changes: 10 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# This file is used to setup the django environment for pytest
import pytest
from django.core.management import call_command


@pytest.fixture(scope="session")
def django_db_setup(django_db_setup, django_db_blocker):
"""Create initial groups before running tests."""
with django_db_blocker.unblock():
call_command("sync_roles")
5 changes: 3 additions & 2 deletions docs/setup/deployment/development/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ pg_restore --verbose --clean --if-exists --no-acl --no-owner --dbname=hypha --us
After restoring the sandbox db run the migrate command inside the py container.

```shell
docker-compose exec py bash
python3 manage.py migrate
docker-compose exec py python3 manage.py migrate
docker-compose exec py python3 manage.py sync_roles

```
2 changes: 2 additions & 0 deletions docs/setup/deployment/development/stand-alone.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ There are two ways to about it, you can either load demo data from `/public/san

```shell
python3 manage.py migrate
python3 manage.py sync_roles
```

=== "From Scratch"
Expand All @@ -209,6 +210,7 @@ There are two ways to about it, you can either load demo data from `/public/san

```text
python3 manage.py migrate
python3 manage.py sync_roles
```

!!! tip "Tips"
Expand Down
3 changes: 2 additions & 1 deletion docs/setup/deployment/production/heroku.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,13 @@ python3 -c "from django.core.management.utils import get_random_secret_key; prin

```shell
heroku run python3 manage.py migrate -a [name-of-app]
heroku run python3 manage.py sync_roles -a [name-of-app]
heroku run python3 manage.py createcachetable -a [name-of-app]
heroku run python3 manage.py createsuperuser -a [name-of-app]
heroku run python3 manage.py wagtailsiteupdate [the-public-address] [the-apply-address] 443 -a [name-of-app]
```

7. Now add the "release" step back to the "Procfile" and deploy again.
7. Now add the "release" step back to the `Procfile` and deploy again.

You should now have a running site.

Expand Down
1 change: 1 addition & 0 deletions docs/setup/deployment/production/stand-alone.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ npm run build
python manage.py collectstatic --noinput
python manage.py createcachetable
python manage.py migrate --noinput
python manage.py sync_roles
python manage.py clear_cache --cache=default
python manage.py createsuperuser
python manage.py wagtailsiteupdate apply.server.domain 80
Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/activity/adapters/emails.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
from hypha.apply.activity.models import ALL, APPLICANT_PARTNERS, PARTNER
from hypha.apply.projects.models.payment import CHANGES_REQUESTED_BY_STAFF, DECLINED
from hypha.apply.projects.templatetags.project_tags import display_project_status
from hypha.apply.users.groups import (
from hypha.apply.users.models import User
from hypha.apply.users.roles import (
CONTRACTING_GROUP_NAME,
FINANCE_GROUP_NAME,
STAFF_GROUP_NAME,
)
from hypha.apply.users.models import User
from hypha.core.mail import (
language,
remove_extra_empty_lines,
Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/activity/adapters/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
RESUBMITTED,
SUBMITTED,
)
from hypha.apply.users.groups import (
from hypha.apply.users.models import User
from hypha.apply.users.roles import (
CONTRACTING_GROUP_NAME,
FINANCE_GROUP_NAME,
STAFF_GROUP_NAME,
)
from hypha.apply.users.models import User


def link_to(target, request):
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
)
from hypha.apply.review.models import Review, ReviewOpinion
from hypha.apply.review.options import RECOMMENDATION_CHOICES
from hypha.apply.users.groups import PARTNER_GROUP_NAME, STAFF_GROUP_NAME
from hypha.apply.users.roles import PARTNER_GROUP_NAME, STAFF_GROUP_NAME

User = get_user_model()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{% load i18n dashboard_statusbar_tags statusbar_tags workflow_tags heroicons submission_tags %}
{% load can from permission_tags %}

{% for submission in page.object_list %}
<div class="wrapper wrapper--status-bar-outer">
Expand Down Expand Up @@ -28,7 +29,7 @@ <h4 class="heading mb-0 font-bold line-clamp-3 hover:line-clamp-none">
{% endif %}
</a>
{% endif %}
{% user_can_delete_submission submission request.user as can_delete_submission %}
{% can "delete_submission" submission as can_delete_submission %}
{% if can_delete_submission %}
<a class="button button--white" href="{% url 'funds:submissions:delete' submission.id %}" hx-get="{% url 'funds:submissions:delete' submission.id %}" hx-target="#htmx-modal">
{% heroicon_micro "trash" class="inline me-1 mt-1 fill-red-600" aria_hidden=true %}
Expand Down
10 changes: 5 additions & 5 deletions hypha/apply/determinations/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_can_access_form_if_lead(self):
def test_cant_access_wrong_status(self):
submission = ApplicationSubmissionFactory(status="rejected")
response = self.get_page(submission, "form")
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
self.assertRedirects(response, submission.get_absolute_url())

def test_cant_resubmit_determination(self):
submission = ApplicationSubmissionFactory(
Expand All @@ -81,7 +81,7 @@ def test_cant_resubmit_determination(self):
response = self.post_page(
submission, {"data": "value", "outcome": determination.outcome}, "form"
)
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
self.assertRedirects(response, submission.get_absolute_url())

def test_can_edit_draft_determination(self):
submission = ApplicationSubmissionFactory(
Expand Down Expand Up @@ -119,7 +119,7 @@ def test_can_edit_draft_determination_if_not_lead(self):
submission, {"data": "value", "outcome": determination.outcome}, "form"
)
self.assertContains(response, "Approved")
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
self.assertRedirects(response, submission.get_absolute_url())

def test_can_edit_draft_determination_if_not_lead_with_projects(self):
submission = ApplicationSubmissionFactory(status="in_discussion")
Expand All @@ -130,7 +130,7 @@ def test_can_edit_draft_determination_if_not_lead_with_projects(self):
submission, {"data": "value", "outcome": determination.outcome}, "form"
)
self.assertContains(response, "Approved")
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
self.assertRedirects(response, submission.get_absolute_url())

def test_sends_message_if_requires_more_info(self):
submission = ApplicationSubmissionFactory(
Expand Down Expand Up @@ -172,7 +172,7 @@ def test_can_progress_stage_via_determination(self):
# Cannot use self.url() as that uses a different base.
url = submission_next.get_absolute_url()
self.assertRedirects(
response, self.factory.get(url, secure=True).build_absolute_uri(url)
response, self.factory.get(url, secure=False).get_full_path()
)
self.assertEqual(submission_original.status, "invited_to_proposal")
self.assertEqual(submission_next.status, "draft_proposal")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from hypha.apply.funds.models import ApplicationForm, LabType
from hypha.apply.funds.models.forms import LabBaseForm, LabBaseReviewForm
from hypha.apply.review.models import ReviewForm
from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME
from hypha.home.models import ApplyHomePage

CL_FUND_TITLE = "Community lab (archive fund)"
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/funds/management/commands/seed_concept_note.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ApplicationBaseReviewForm,
)
from hypha.apply.review.models import ReviewForm
from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME
from hypha.home.models import ApplyHomePage

CN_ROUND_TITLE = "Internet Freedom Fund (archive round)"
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/funds/management/commands/seed_fellowship.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ApplicationBaseReviewForm,
)
from hypha.apply.review.models import ReviewForm
from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME
from hypha.home.models import ApplyHomePage

FS_ROUND_TITLE = "Fellowship (archive round)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ApplicationBaseReviewForm,
)
from hypha.apply.review.models import ReviewForm
from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME
from hypha.home.models import ApplyHomePage

RR_ROUND_TITLE = "Rapid Response (archive round)"
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/funds/models/submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
from hypha.apply.stream_forms.models import BaseStreamForm
from hypha.apply.todo.options import SUBMISSION_DRAFT
from hypha.apply.todo.views import remove_tasks_for_user
from hypha.apply.users.groups import APPLICANT_GROUP_NAME
from hypha.apply.users.roles import APPLICANT_GROUP_NAME

from ..blocks import NAMED_BLOCKS, ApplicationCustomFormFieldsBlock
from ..workflow import (
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/funds/models/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from hypha.apply.stream_forms.models import AbstractStreamForm
from hypha.apply.todo.options import SUBMISSION_DRAFT
from hypha.apply.todo.views import add_task_to_user
from hypha.apply.users.groups import (
from hypha.apply.users.roles import (
COMMUNITY_REVIEWER_GROUP_NAME,
PARTNER_GROUP_NAME,
REVIEWER_GROUP_NAME,
Expand Down
29 changes: 22 additions & 7 deletions hypha/apply/funds/permissions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from django.conf import settings
from django.core.exceptions import PermissionDenied
from rolepermissions.permissions import register_object_checker

from hypha.apply.funds.models.submissions import DRAFT_STATE

from ..users.groups import STAFF_GROUP_NAME, SUPERADMIN, TEAMADMIN_GROUP_NAME
from ..users.roles import STAFF_GROUP_NAME, SUPERADMIN, TEAMADMIN_GROUP_NAME, StaffAdmin


def has_permission(action, user, object=None, raise_exception=True):
Expand All @@ -26,12 +27,27 @@ def can_edit_submission(user, submission):
return True, ""


def can_delete_submission(user, submission):
@register_object_checker()
def delete_submission(role, user, submission) -> bool:
"""
Determines if a user has permission to delete a submission.
Permissions are granted if:
- User is a Superuser, or StaffAdmin
- User has explicit delete_applicationsubmission permission
- User is the applicant of the submission and it is in draft state
"""
if role == StaffAdmin:
return True

if user.has_perm("funds.delete_applicationsubmission"):
return True, "User can delete submission"
elif user == submission.user and submission.status == DRAFT_STATE:
return True, "Applicant can delete draft submissions"
return False, "Forbidden Error"
return True

# Allow the user to delete their own draft submissions
if user == submission.user and submission.status == DRAFT_STATE:
return True

return False


def can_bulk_delete_submissions(user) -> bool:
Expand Down Expand Up @@ -174,7 +190,6 @@ def can_view_submission_screening(user, submission):
permissions_map = {
"submission_view": is_user_has_access_to_view_submission,
"submission_edit": can_edit_submission,
"submission_delete": can_delete_submission,
"can_view_submission_screening": can_view_submission_screening,
"archive_alter": can_alter_archived_submissions,
}
2 changes: 1 addition & 1 deletion hypha/apply/funds/reviewers/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from django.db.models import Q
from django.db.models.query import QuerySet

from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME

User = get_user_model()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{% extends "base-apply.html" %}
{% load i18n static workflow_tags wagtailcore_tags statusbar_tags archive_tags submission_tags %}
{% load heroicons %}
{% load can from permission_tags %}

{% block title %}{{ object.title_text_display }}{% endblock %}
{% block body_class %}{% endblock %}
Expand Down Expand Up @@ -122,7 +123,7 @@ <h5>{% blocktrans with stage=object.previous.stage %}Your {{ stage }} applicatio
</strong>
</span>
<div class="flex gap-4 justify-end flex-1 items-center">
{% user_can_delete_submission object request.user as can_delete_submission %}
{% can "delete_submission" object as can_delete_submission %}
{% if can_delete_submission %}
<a
class="flex items-center font-bold text-red-600 transition-opacity hover:opacity-70"
Expand Down
12 changes: 0 additions & 12 deletions hypha/apply/funds/templatetags/submission_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from django.utils.safestring import mark_safe

from hypha.apply.funds.models import ApplicationSubmission
from hypha.apply.funds.permissions import has_permission

register = template.Library()

Expand All @@ -30,14 +29,3 @@ def submission_links(value):
value = re.sub(rf"(?<!\w){sid}(?!\w)", link, value)

return mark_safe(value)


@register.simple_tag
def user_can_delete_submission(submission, user):
permission, _ = has_permission(
"submission_delete",
user=user,
object=submission,
raise_exception=False,
)
return permission
2 changes: 1 addition & 1 deletion hypha/apply/funds/tests/factories/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
)
from hypha.apply.funds.workflow import ConceptProposal, Request, RequestExternal
from hypha.apply.stream_forms.testing.factories import FormDataFactory
from hypha.apply.users.groups import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME
from hypha.apply.users.roles import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME
from hypha.apply.users.tests.factories import (
ApplicantFactory,
GroupFactory,
Expand Down
2 changes: 1 addition & 1 deletion hypha/apply/funds/tests/test_admin_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from wagtail.test.utils import WagtailTestUtils

from hypha.apply.funds.models.forms import ApplicationForm
from hypha.apply.users.groups import STAFF_GROUP_NAME
from hypha.apply.users.roles import STAFF_GROUP_NAME
from hypha.apply.users.tests.factories import SuperUserFactory
from hypha.home.factories import ApplyHomePageFactory

Expand Down
6 changes: 2 additions & 4 deletions hypha/apply/funds/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ def test_redirected_to_determination(self):
"funds:submissions:determinations:form",
kwargs={"submission_pk": submission.id},
)
self.assertEqual(
self.absolute_url(response.url), f"{url}?action=invited_to_proposal"
)
assert response.url == f"{url}?action=invited_to_proposal"

def test_new_form_after_progress(self):
submission = ApplicationSubmissionFactory(
Expand Down Expand Up @@ -1760,7 +1758,7 @@ def test_anonymous_can_not_access(self):
submission = ApplicationSubmissionFactory()
response = self.get_page(submission)
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.redirect_chain), 2)
self.assertEqual(len(response.redirect_chain), 1)
for path, _ in response.redirect_chain:
self.assertIn(reverse(settings.LOGIN_URL), path)

Expand Down
Loading

0 comments on commit 8f16bd2

Please sign in to comment.