revert some of the tweaks

src/registrar/migrations/0114_alter_user_portfolio_additional_permissions.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.10 on 2024-07-30 02:51
+# Generated by Django 4.2.10 on 2024-07-30 23:58
 
 import django.contrib.postgres.fields
 from django.db import migrations, models
@@ -26,6 +26,7 @@ class Migration(migrations.Migration):
                         ("view_created_requests", "View created requests"),
                         ("edit_requests", "Create and edit requests"),
                         ("edit_portfolio", "Edit organization"),
+                        ("view_portfolio", "View organization"),
                     ],
                     max_length=50,
                 ),
@@ -35,18 +36,4 @@ class Migration(migrations.Migration):
                 size=None,
             ),
         ),
-        migrations.AlterField(
-            model_name="user",
-            name="portfolio_roles",
-            field=django.contrib.postgres.fields.ArrayField(
-                base_field=models.CharField(
-                    choices=[("organization_admin", "Admin"), ("organization_admin_read_only", "Admin read only")],
-                    max_length=50,
-                ),
-                blank=True,
-                help_text="Select one or more roles.",
-                null=True,
-                size=None,
-            ),
-        ),
     ]

src/registrar/models/user.py

@@ -64,15 +64,15 @@ class User(AbstractUser):
 
     class UserPortfolioRoleChoices(models.TextChoices):
         """
-        Roles make it easier for admins to look at
+        Roles make it easier for admins to look at groups of users
         """
 
         ORGANIZATION_ADMIN = "organization_admin", "Admin"
         ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only"
-        # ORGANIZATION_MEMBER is an abstract role where user.portfolio is true
+        ORGANIZATION_MEMBER = "organization_member", "Member"
 
     class UserPortfolioPermissionChoices(models.TextChoices):
-        """ """
+        """We test against permissions to manage access"""
 
         VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports"
         VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains"
@@ -89,8 +89,8 @@ class User(AbstractUser):
         VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests"
         EDIT_REQUESTS = "edit_requests", "Create and edit requests"
 
-        # VIEW_PORTFOLIO is an abstract permission that returns true when user.portfolio is true
         EDIT_PORTFOLIO = "edit_portfolio", "Edit organization"
+        VIEW_PORTFOLIO = "view_portfolio", "View organization"
 
     PORTFOLIO_ROLE_PERMISSIONS = {
         UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [
@@ -106,6 +106,9 @@ class User(AbstractUser):
             UserPortfolioPermissionChoices.VIEW_MEMBER,
             UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS,
         ],
+        UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
+            UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
+        ],
     }
 
     # #### Constants for choice fields ####
@@ -278,8 +281,7 @@ class User(AbstractUser):
     # the methods below are checks for individual portfolio permissions. They are defined here
     # to make them easier to call elsewhere throughout the application
     def has_base_portfolio_permission(self):
-        """Base role/permission, the user is simply linked to a portfolio"""
+        return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO)
-        return self.portfolio is not None
 
     def has_edit_org_portfolio_permission(self):
         return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO)

src/registrar/tests/test_views_portfolio.py

@@ -37,10 +37,25 @@ class TestPortfolio(WebTest):
         User.objects.all().delete()
         super().tearDown()
 
+    @less_console_noise_decorator
+    def test_middleware_does_not_redirect_if_no_permission(self):
+        """Test that user with no portfolio permission is not redirected when attempting to access home"""
+        self.app.set_user(self.user.username)
+        self.user.portfolio = self.portfolio
+        self.user.save()
+        self.user.refresh_from_db()
+        with override_flag("organization_feature", active=True):
+            # This will redirect the user to the portfolio page.
+            # Follow implicity checks if our redirect is working.
+            portfolio_page = self.app.get(reverse("home"))
+            # Assert that we're on the right page
+            self.assertNotContains(portfolio_page, self.portfolio.organization_name)
+
     @less_console_noise_decorator
     def test_middleware_does_not_redirect_if_no_portfolio(self):
         """Test that user with no assigned portfolio is not redirected when attempting to access home"""
         self.app.set_user(self.user.username)
+        self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
         self.user.save()
         self.user.refresh_from_db()
         with override_flag("organization_feature", active=True):
@@ -52,9 +67,10 @@ class TestPortfolio(WebTest):
 
     @less_console_noise_decorator
     def test_middleware_redirects_to_portfolio_organization_page(self):
-        """Test that user with a portfolio is redirected to portfolio organization page"""
+        """Test that user with a portfolio and VIEW_PORTFOLIO is redirected to portfolio organization page"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
+        self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
         self.user.save()
         self.user.refresh_from_db()
         with override_flag("organization_feature", active=True):
@@ -67,10 +83,12 @@ class TestPortfolio(WebTest):
 
     @less_console_noise_decorator
     def test_middleware_redirects_to_portfolio_domains_page(self):
-        """Test that user with a portfolio and VIEW_ALL_DOMAINS is redirected to portfolio domains page"""
+        """Test that user with a portfolio, VIEW_PORTFOLIO, VIEW_ALL_DOMAINS
+        is redirected to portfolio domains page"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
         self.user.portfolio_additional_permissions = [
+            User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS,
         ]
         self.user.save()
@@ -116,12 +134,29 @@ class TestPortfolio(WebTest):
             # Assert the response is a 403 Forbidden
             self.assertEqual(response.status_code, 403)
 
+    @less_console_noise_decorator
+    def test_portfolio_organization_page_403_when_user_not_have_permission(self):
+        """Test that user without proper permission is not allowed access to portfolio organization page"""
+        self.app.set_user(self.user.username)
+        self.user.portfolio = self.portfolio
+        self.user.save()
+        self.user.refresh_from_db()
+        with override_flag("organization_feature", active=True):
+            # This will redirect the user to the portfolio page.
+            # Follow implicity checks if our redirect is working.
+            response = self.app.get(
+                reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}), status=403
+            )
+            # Assert the response is a 403 Forbidden
+            self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_portfolio_organization_page_read_only(self):
         """Test that user with a portfolio can access the portfolio organization page, read only"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
         self.portfolio.city = "Los Angeles"
+        self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
         self.portfolio.save()
         self.user.save()
         self.user.refresh_from_db()
@@ -142,6 +177,7 @@ class TestPortfolio(WebTest):
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
         self.user.portfolio_additional_permissions = [
+            User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
         ]
         self.portfolio.city = "Los Angeles"
@@ -210,6 +246,7 @@ class TestPortfolio(WebTest):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
+                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()
@@ -227,6 +264,7 @@ class TestPortfolio(WebTest):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
+                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()
@@ -245,6 +283,7 @@ class TestPortfolio(WebTest):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
+                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()