diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5008de6b0..8c94d1fc6 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -654,7 +654,6 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): "user_permissions", "portfolio", "portfolio_roles", - "portfolio_permissions", ) }, ), @@ -685,7 +684,6 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): "groups", "portfolio", "portfolio_roles", - "portfolio_permissions", ) }, ), @@ -717,7 +715,6 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): "date_joined", "portfolio", "portfolio_roles", - "portfolio_permissions", ] list_filter = ( diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 1b8a5005f..4de049be4 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -240,6 +240,7 @@ TEMPLATES = [ "registrar.context_processors.canonical_path", "registrar.context_processors.is_demo_site", "registrar.context_processors.is_production", + "registrar.context_processors.portfolio_permissions", ], }, }, diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index b060d0877..2cb7c0280 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -40,18 +40,10 @@ def is_production(request): return {"IS_PRODUCTION": settings.IS_PRODUCTION} -def has_base_portfolio_permission(request): +def portfolio_permissions(request): """""" - return {"has_base_portfolio_permission": request.user.has_portfolio_permissions(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO)} - -def has_domains_portfolio_permission(request): - """""" - return {"has_domains_portfolio_permission": request.user.has_portfolio_permissions(User.UserPortfolioPermissionChoices.VIEW_DOMAINS)} - -def has_requests_portfolio_permission(request): - """""" - return {"has_requests_portfolio_permission": request.user.has_portfolio_permissions(User.UserPortfolioPermissionChoices.VIEW_REQUESTS)} - -def has_organization_portfolio_permission(request): - """""" - return {"has_organization_portfolio_permission": request.user.has_portfolio_permissions(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO)} + return { + "has_base_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO), + "has_domains_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_DOMAINS), + "has_domain_requests_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_REQUESTS) + } diff --git a/src/registrar/migrations/0113_user_portfolio_user_portfolio_permissions_and_more.py b/src/registrar/migrations/0113_user_portfolio_user_portfolio_roles_and_more.py similarity index 61% rename from src/registrar/migrations/0113_user_portfolio_user_portfolio_permissions_and_more.py rename to src/registrar/migrations/0113_user_portfolio_user_portfolio_roles_and_more.py index cbcad2c67..787ee99c2 100644 --- a/src/registrar/migrations/0113_user_portfolio_user_portfolio_permissions_and_more.py +++ b/src/registrar/migrations/0113_user_portfolio_user_portfolio_roles_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-07-15 22:07 +# Generated by Django 4.2.10 on 2024-07-17 17:12 from django.conf import settings import django.contrib.postgres.fields @@ -24,29 +24,6 @@ class Migration(migrations.Migration): to="registrar.portfolio", ), ), - migrations.AddField( - model_name="user", - name="portfolio_permissions", - field=django.contrib.postgres.fields.ArrayField( - base_field=models.CharField( - choices=[ - ("view_domains", "View all domains and domain reports"), - ("edit_domains", "User is a manager on a domain"), - ("view_member", "View members"), - ("edit_member", "Create and edit members"), - ("view_requests", "View requests"), - ("edit_requests", "Create and edit requests"), - ("view_portfolio", "View organization"), - ("edit_portfolio", "Edit organization"), - ], - max_length=50, - ), - blank=True, - help_text="Select one or more permissions.", - null=True, - size=None, - ), - ), migrations.AddField( model_name="user", name="portfolio_roles", diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 204ead524..6e278e730 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -63,6 +63,7 @@ class User(AbstractUser): class UserPortfolioRoleChoices(models.TextChoices): """ + Roles make it easier for admins to look at """ ORGANIZATION_ADMIN = "organization_admin", "Admin" @@ -149,15 +150,15 @@ class User(AbstractUser): help_text="Select one or more roles.", ) - portfolio_permissions = ArrayField( - models.CharField( - max_length=50, - choices=UserPortfolioPermissionChoices.choices, - ), - null=True, - blank=True, - help_text="Select one or more permissions.", - ) + # portfolio_permissions = ArrayField( + # models.CharField( + # max_length=50, + # choices=UserPortfolioPermissionChoices.choices, + # ), + # null=True, + # blank=True, + # help_text="Select one or more permissions.", + # ) phone = PhoneNumberField( null=True, @@ -252,30 +253,30 @@ class User(AbstractUser): """Do not rely on roles when testing for perms in views""" return role in self.portfolio_roles if self.portfolio_roles else False - def has_portfolio_permissions(self, portfolio_permission): + def has_portfolio_permission(self, portfolio_permission): """The views should only call this guy when testing for perms and not rely on roles""" # TODO: this does not seem to be working - if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): - print(f'portfolio_permission {portfolio_permission}') - return True + # if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): + # return True + + if not self.portfolio: + return False - print(f'portfolio_permission {portfolio_permission}') - - return portfolio_permission in self.portfolio_permissions if self.portfolio_permissions else False + portfolio_permissions = self.get_portfolio_permissions() + + return portfolio_permission in portfolio_permissions - def save(self, *args, **kwargs): - self.update_permissions_from_roles() - super().save(*args, **kwargs) + def get_portfolio_permissions(self): + """ + Retrieve the permissions for the user's portfolio roles. + """ + portfolio_permissions = set() # Use a set to avoid duplicate permissions - def update_permissions_from_roles(self): - print('update permissions when saving') - new_portfolio_permissions = set(self.portfolio_permissions or []) - print(f'new_portfolio_permissions {new_portfolio_permissions}') - for role in self.portfolio_roles or []: - print(f'role {role}') - new_portfolio_permissions.update(self.PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) - self.portfolio_permissions = list(new_portfolio_permissions) + for role in self.portfolio_roles: + if role in self.PORTFOLIO_ROLE_PERMISSIONS: + portfolio_permissions.update(self.PORTFOLIO_ROLE_PERMISSIONS[role]) + return list(portfolio_permissions) # Convert back to list if necessary @classmethod def needs_identity_verification(cls, email, uuid): diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index d58a3082d..e3bc33c8f 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -6,7 +6,7 @@ import logging from urllib.parse import parse_qs from django.urls import reverse from django.http import HttpResponseRedirect -from registrar.context_processors import has_base_portfolio_permission, has_domains_portfolio_permission, has_requests_portfolio_permission +from registrar.context_processors import portfolio_permissions from registrar.models.user import User from waffle.decorators import flag_is_active @@ -148,19 +148,21 @@ class CheckPortfolioMiddleware: if request.user.is_authenticated: # user_portfolios = Portfolio.objects.filter(creator=request.user) - if has_base_portfolio_permission(request): - # print('user has portfolio') + permission_dict = portfolio_permissions(request) + has_portfolio_base_permission = permission_dict['has_base_portfolio_permission'] + + if has_portfolio_base_permission: portfolio = request.user.portfolio - if has_domains_portfolio_permission(request): + permission_dict = portfolio_permissions(request) + has_portfolio_domains_permission = permission_dict['has_domains_portfolio_permission'] + + if has_portfolio_domains_permission: portfolio_redirect = reverse("portfolio-domains", kwargs={"portfolio_id": portfolio.id}) - elif has_requests_portfolio_permission(request): - portfolio_redirect = reverse("portfolio-requests", kwargs={"portfolio_id": portfolio.id}) else: # View organization is the lowest access portfolio_redirect = reverse("portfolio-organization", kwargs={"portfolio_id": portfolio.id}) return HttpResponseRedirect(portfolio_redirect) - # print('user does not have a portfolio') return None diff --git a/src/registrar/templates/portfolio_sidebar.html b/src/registrar/templates/portfolio_sidebar.html index 271bc77c5..49ed9ea88 100644 --- a/src/registrar/templates/portfolio_sidebar.html +++ b/src/registrar/templates/portfolio_sidebar.html @@ -12,7 +12,15 @@ {% endif %} - {% if has_requests_portfolio_permission %} + +
  • + {% url 'portfolio-organization' portfolio.id as url %} + + Organization + +
  • + + {% if has_domain_requests_portfolio_permission %}
  • {% url 'portfolio-domain-requests' portfolio.id as url %} @@ -26,12 +34,7 @@ Members
  • -
  • - {% url 'portfolio-organization' portfolio.id as url %} - - Organization - -
  • +
  • Senior official diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 83f41bb6a..a9f9a3669 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -1,6 +1,6 @@ from django.shortcuts import get_object_or_404, render from registrar.models.portfolio import Portfolio -from registrar.views.utility.permission_views import PortfolioDomainRequestsPermissionView, PortfolioDomainsPermissionView, PortfolioOrganizationssPermissionView +from registrar.views.utility.permission_views import PortfolioDomainRequestsPermissionView, PortfolioDomainsPermissionView, PortfolioBasePermissionView from waffle.decorators import flag_is_active from django.views.generic import View @@ -35,7 +35,7 @@ class PortfolioDomainRequestsView(PortfolioDomainRequestsPermissionView, View): return render(request, "portfolio_requests.html", context) -class PortfolioOrganizationView(PortfolioOrganizationssPermissionView, View): +class PortfolioOrganizationView(PortfolioBasePermissionView, View): template_name = "portfolio_organization.html" diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 18dc3c550..0b2931583 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -2,7 +2,7 @@ from django.contrib.auth.mixins import PermissionRequiredMixin -from registrar.context_processors import has_base_portfolio_permission, has_domains_portfolio_permission, has_organization_portfolio_permission, has_requests_portfolio_permission +from registrar.context_processors import portfolio_permissions from registrar.models import ( Domain, DomainRequest, @@ -402,7 +402,7 @@ class UserProfilePermission(PermissionsLoginMixin): return True -class PortfolioPermission(PermissionsLoginMixin): +class PortfolioBasePermission(PermissionsLoginMixin): """Permission mixin that redirects to portfolio pages if user has access, otherwise 403""" @@ -415,15 +415,15 @@ class PortfolioPermission(PermissionsLoginMixin): if not self.request.user.is_authenticated: return False - # portfolio_id = self.kwargs["pk"] - # portfolio = Portfolio.objects.get(pk=portfolio_id) + permission_dict = portfolio_permissions(self.request) + has_permission = permission_dict['has_base_portfolio_permission'] - if not has_base_portfolio_permission(self.request): + if not has_permission: return False return True -class PortfolioDomainsPermission(PortfolioPermission): +class PortfolioDomainsPermission(PortfolioBasePermission): """ """ @@ -431,18 +431,16 @@ class PortfolioDomainsPermission(PortfolioPermission): """ """ - permission_dict = has_domains_portfolio_permission(self.request) + permission_dict = portfolio_permissions(self.request) has_permission = permission_dict['has_domains_portfolio_permission'] if not has_permission: return False - - print('return true') return True -class PortfolioDomainRequestsPermission(PortfolioPermission): +class PortfolioDomainRequestsPermission(PortfolioBasePermission): """ """ @@ -450,24 +448,8 @@ class PortfolioDomainRequestsPermission(PortfolioPermission): """ """ - permission_dict = has_requests_portfolio_permission(self.request) - has_permission = permission_dict['has_requests_portfolio_permission'] - - if not has_permission: - return False - - return True - -class PortfolioOrganizationssPermission(PortfolioPermission): - """ - """ - - def has_permission(self): - """ - """ - - permission_dict = has_organization_portfolio_permission(self.request) - has_permission = permission_dict['has_organization_portfolio_permission'] + permission_dict = portfolio_permissions(self.request) + has_permission = permission_dict['has_domain_requests_portfolio_permission'] if not has_permission: return False diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index b1247d280..373b4174b 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -15,10 +15,9 @@ from .mixins import ( DomainRequestWizardPermission, PortfolioDomainRequestsPermission, PortfolioDomainsPermission, - PortfolioOrganizationssPermission, UserDeleteDomainRolePermission, UserProfilePermission, - PortfolioPermission, + PortfolioBasePermission, ) import logging @@ -169,7 +168,7 @@ class UserProfilePermissionView(UserProfilePermission, DetailView, abc.ABC): raise NotImplementedError -class PortfolioBasePermissionView(PortfolioPermission, DetailView, abc.ABC): +class PortfolioBasePermissionView(PortfolioBasePermission, DetailView, abc.ABC): """Abstract base view for portfolio views that enforces permissions. This abstract view cannot be instantiated. Actual views must specify @@ -196,8 +195,3 @@ class PortfolioDomainsPermissionView(PortfolioDomainsPermission, PortfolioBasePe class PortfolioDomainRequestsPermissionView(PortfolioDomainRequestsPermission, PortfolioBasePermissionView, abc.ABC): """ """ - -class PortfolioOrganizationssPermissionView(PortfolioOrganizationssPermission, PortfolioBasePermissionView, abc.ABC): - """ - """ -