diff --git a/.gitignore b/.gitignore index b49b30639..5540ec869 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,8 @@ docs/research/data/** **/assets/* !**/assets/src/ -!**/assets/sass/ +!**/assets/src/js/ +!**/assets/src/sass/ !**/assets/img/registrar/ public/ credentials* diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index a234d882b..0863aa0b7 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -893,22 +893,28 @@ Example: `cf ssh getgov-za` [Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. #### Step 5: Running the script -```./manage.py create_federal_portfolio "{federal_agency_name}" --both``` - +To create a specific portfolio: +```./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` Example (only requests): `./manage.py create_federal_portfolio "AMTRAK" --parse_requests` +To create a portfolios for all federal agencies in a branch: +```./manage.py create_federal_portfolio --branch "{executive|legislative|judicial}" --both``` +Example (only requests): `./manage.py create_federal_portfolio --branch "executive" --parse_requests` + ### Running locally #### Step 1: Running the script -```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --both``` +```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` ##### Parameters | | Parameter | Description | |:-:|:-------------------------- |:-------------------------------------------------------------------------------------------| -| 1 | **federal_agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | -| 2 | **both** | If True, runs parse_requests and parse_domains. | -| 3 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | -| 4 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | +| 1 | **agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | +| 2 | **branch** | Creates a portfolio for each federal agency in a branch: executive, legislative, judicial | +| 3 | **both** | If True, runs parse_requests and parse_domains. | +| 4 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | +| 5 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | -Note: Regarding parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, +- Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both. +- Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. diff --git a/src/docker-compose.yml b/src/docker-compose.yml index 8cb2bd60f..f20aa61e4 100644 --- a/src/docker-compose.yml +++ b/src/docker-compose.yml @@ -85,6 +85,7 @@ services: volumes: - .:/app working_dir: /app + entrypoint: /app/node_entrypoint.sh stdin_open: true tty: true command: ./run_node_watch.sh diff --git a/src/node.Dockerfile b/src/node.Dockerfile index 0fcab7d92..3e4f5d1ba 100644 --- a/src/node.Dockerfile +++ b/src/node.Dockerfile @@ -1,9 +1,9 @@ FROM docker.io/cimg/node:current-browsers WORKDIR /app +USER root + # Install app dependencies # A wildcard is used to ensure both package.json AND package-lock.json are copied # where available (npm@5+) -COPY --chown=circleci:circleci package*.json ./ - -RUN npm install \ No newline at end of file +COPY --chown=circleci:circleci package*.json ./ \ No newline at end of file diff --git a/src/node_entrypoint.sh b/src/node_entrypoint.sh new file mode 100755 index 000000000..113e51c30 --- /dev/null +++ b/src/node_entrypoint.sh @@ -0,0 +1,24 @@ +#!/bin/bash + +# Get UID and GID of the /app directory owner +HOST_UID=$(stat -c '%u' /app) +HOST_GID=$(stat -c '%g' /app) + +# Check if the circleci user exists +if id "circleci" &>/dev/null; then + echo "circleci user exists. Updating UID and GID to match host UID:GID ($HOST_UID:$HOST_GID)" + + # Update circleci user's UID and GID + groupmod -g "$HOST_GID" circleci + usermod -u "$HOST_UID" circleci + + echo "Updating ownership of /app recursively to circleci:circleci" + chown -R circleci:circleci /app + + # Switch to circleci user and execute the command + echo "Switching to circleci user and running command: $@" + su -s /bin/bash -c "$*" circleci +else + echo "circleci user does not exist. Running command as the current user." + exec "$@" +fi diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index 7a67158ea..ac0b7cffe 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -157,15 +157,15 @@ export function initAddNewMemberPageListeners() { // Populate permission details based on access level if (selectedAccess && selectedAccess.value === 'admin') { - populatePermissionDetails('new-member-admin-permissions') + populatePermissionDetails('new-member-admin-permissions'); } else { - populatePermissionDetails('new-member-basic-permissions') + populatePermissionDetails('new-member-basic-permissions'); } //------- Show the modal let modalTrigger = document.querySelector("#invite_member_trigger"); if (modalTrigger) { - modalTrigger.click() + modalTrigger.click(); } } diff --git a/src/registrar/assets/src/js/getgov/urbanization.js b/src/registrar/assets/src/js/getgov/urbanization.js index ea38a31e7..75e87628e 100644 --- a/src/registrar/assets/src/js/getgov/urbanization.js +++ b/src/registrar/assets/src/js/getgov/urbanization.js @@ -1,12 +1,18 @@ +import { hideElement, showElement } from './helpers.js'; + function setupUrbanizationToggle(stateTerritoryField) { - var urbanizationField = document.getElementById('urbanization-field'); + let urbanizationField = document.getElementById('urbanization-field'); + if (!urbanizationField) { + console.error("Cannot find expect field: #urbanization-field"); + return; + } function toggleUrbanizationField() { // Checking specifically for Puerto Rico only if (stateTerritoryField.value === 'PR') { - urbanizationField.style.display = 'block'; + showElement(urbanizationField); } else { - urbanizationField.style.display = 'none'; + hideElement(urbanizationField); } } diff --git a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py index 22d6a0c0e..15265cfa8 100644 --- a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py +++ b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py @@ -16,7 +16,7 @@ logger = logging.getLogger(__name__) class UserPortfolioPermissionFixture: """Create user portfolio permissions for each user. - Each user will be admin on 2 portfolios. + Each user will be admin on only one portfolio. Depends on fixture_portfolios""" diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d05a2911b..9cf4d36ea 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -13,16 +13,29 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" + def __init__(self, *args, **kwargs): + """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" + super().__init__(*args, **kwargs) + self.updated_portfolios = set() + self.skipped_portfolios = set() + self.failed_portfolios = set() + def add_arguments(self, parser): """Add three arguments: 1. agency_name => the value of FederalAgency.agency 2. --parse_requests => if true, adds the given portfolio to each related DomainRequest 3. --parse_domains => if true, adds the given portfolio to each related DomainInformation """ - parser.add_argument( - "agency_name", + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument( + "--agency_name", help="The name of the FederalAgency to add", ) + group.add_argument( + "--branch", + choices=["executive", "legislative", "judicial"], + help="The federal branch to process. Creates a portfolio for each FederalAgency in this branch.", + ) parser.add_argument( "--parse_requests", action=argparse.BooleanOptionalAction, @@ -39,7 +52,9 @@ class Command(BaseCommand): help="Adds portfolio to both requests and domains", ) - def handle(self, agency_name, **options): + def handle(self, **options): + agency_name = options.get("agency_name") + branch = options.get("branch") parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") @@ -51,84 +66,94 @@ class Command(BaseCommand): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") - federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first() - if not federal_agency: - raise ValueError( - f"Cannot find the federal agency '{agency_name}' in our database. " - "The value you enter for `agency_name` must be " - "prepopulated in the FederalAgency table before proceeding." - ) + federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} + agencies = FederalAgency.objects.filter(**federal_agency_filter) + if not agencies or agencies.count() < 1: + if agency_name: + raise CommandError( + f"Cannot find the federal agency '{agency_name}' in our database. " + "The value you enter for `agency_name` must be " + "prepopulated in the FederalAgency table before proceeding." + ) + else: + raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - portfolio = self.create_or_modify_portfolio(federal_agency) - self.create_suborganizations(portfolio, federal_agency) + for federal_agency in agencies: + message = f"Processing federal agency '{federal_agency.agency}'..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + try: + # C901 'Command.handle' is too complex (12) + self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + except Exception as exec: + self.failed_portfolios.add(federal_agency) + logger.error(exec) + message = f"Failed to create portfolio '{federal_agency.agency}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message) + + TerminalHelper.log_script_run_summary( + self.updated_portfolios, + self.failed_portfolios, + self.skipped_portfolios, + debug=False, + skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----", + display_as_str=True, + ) + + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): + """Attempts to create a portfolio. If successful, this function will + also create new suborganizations""" + portfolio, created = self.create_portfolio(federal_agency) + if created: + self.create_suborganizations(portfolio, federal_agency) + if parse_domains or both: + self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: self.handle_portfolio_requests(portfolio, federal_agency) - if parse_domains or both: - self.handle_portfolio_domains(portfolio, federal_agency) + def create_portfolio(self, federal_agency): + """Creates a portfolio if it doesn't presently exist. + Returns portfolio, created.""" + # Get the org name / senior official + org_name = federal_agency.agency + so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None - def create_or_modify_portfolio(self, federal_agency): - """Creates or modifies a portfolio record based on a federal agency.""" - portfolio_args = { - "federal_agency": federal_agency, - "organization_name": federal_agency.agency, - "organization_type": DomainRequest.OrganizationChoices.FEDERAL, - "creator": User.get_default_user(), - "notes": "Auto-generated record", - } + # First just try to get an existing portfolio + portfolio = Portfolio.objects.filter(organization_name=org_name).first() + if portfolio: + self.skipped_portfolios.add(portfolio) + TerminalHelper.colorful_logger( + logger.info, + TerminalColors.YELLOW, + f"Portfolio with organization name '{org_name}' already exists. Skipping create.", + ) + return portfolio, False - if federal_agency.so_federal_agency.exists(): - portfolio_args["senior_official"] = federal_agency.so_federal_agency.first() - - portfolio, created = Portfolio.objects.get_or_create( - organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args + # Create new portfolio if it doesn't exist + portfolio = Portfolio.objects.create( + organization_name=org_name, + federal_agency=federal_agency, + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=User.get_default_user(), + notes="Auto-generated record", + senior_official=so, ) - if created: - message = f"Created portfolio '{portfolio}'" + self.updated_portfolios.add(portfolio) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'") + + # Log if the senior official was added or not. + if portfolio.senior_official: + message = f"Added senior official '{portfolio.senior_official}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - - if portfolio_args.get("senior_official"): - message = f"Added senior official '{portfolio_args['senior_official']}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - else: - message = ( - f"No senior official added to portfolio '{portfolio}'. " - "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) else: - proceed = TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f""" - The given portfolio '{federal_agency.agency}' already exists in our DB. - If you cancel, the rest of the script will still execute but this record will not update. - """, - prompt_title="Do you wish to modify this record?", + message = ( + f"No senior official added to portfolio '{org_name}'. " + "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" ) - if proceed: + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - # Don't override the creator and notes fields - if portfolio.creator: - portfolio_args.pop("creator") - - if portfolio.notes: - portfolio_args.pop("notes") - - # Update everything else - for key, value in portfolio_args.items(): - setattr(portfolio, key, value) - - portfolio.save() - message = f"Modified portfolio '{portfolio}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - - if portfolio_args.get("senior_official"): - message = f"Added/modified senior official '{portfolio_args['senior_official']}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - - return portfolio + return portfolio, True def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" @@ -146,10 +171,11 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.warning, TerminalColors.FAIL, message) return - # Check if we need to update any existing suborgs first. This step is optional. + # Check for existing suborgs on the current portfolio existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): - self._update_existing_suborganizations(portfolio, existing_suborgs) + message = f"Some suborganizations already exist for portfolio '{portfolio}'." + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] @@ -175,29 +201,6 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def _update_existing_suborganizations(self, portfolio, orgs_to_update): - """ - Update existing suborganizations with new portfolio. - Prompts for user confirmation before proceeding. - """ - proceed = TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f"""Some suborganizations already exist in our DB. - If you cancel, the rest of the script will still execute but these records will not update. - - ==Proposed Changes== - The following suborgs will be updated: {[org.name for org in orgs_to_update]} - """, - prompt_title="Do you wish to modify existing suborganizations?", - ) - if proceed: - for org in orgs_to_update: - org.portfolio = portfolio - - Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) - message = f"Updated {len(orgs_to_update)} suborganizations." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. @@ -208,12 +211,17 @@ class Command(BaseCommand): DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( + status__in=invalid_states + ) if not domain_requests.exists(): message = f""" - Portfolios not added to domain requests: no valid records found. + Portfolio '{portfolio}' not added to domain requests: no valid records found. This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. Excluded statuses: STARTED, INELIGIBLE, REJECTED. + Filter info: DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( + status__in=invalid_states + ) """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None @@ -224,6 +232,7 @@ class Command(BaseCommand): domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." @@ -234,11 +243,12 @@ class Command(BaseCommand): Associate portfolio with domains for a federal agency. Updates all relevant domain information records. """ - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) if not domain_infos.exists(): message = f""" - Portfolios not added to domains: no valid records found. - This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Portfolio '{portfolio}' not added to domains: no valid records found. + The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Filter info: DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None @@ -251,5 +261,5 @@ class Command(BaseCommand): domain_info.sub_organization = suborgs.get(domain_info.organization_name) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index fa7cde683..eed1027f7 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -192,7 +192,7 @@ class PopulateScriptTemplate(ABC): class TerminalHelper: @staticmethod def log_script_run_summary( - to_update, failed_to_update, skipped, debug: bool, log_header=None, display_as_str=False + to_update, failed_to_update, skipped, debug: bool, log_header=None, skipped_header=None, display_as_str=False ): """Prints success, failed, and skipped counts, as well as all affected objects.""" @@ -203,8 +203,21 @@ class TerminalHelper: if log_header is None: log_header = "============= FINISHED ===============" + if skipped_header is None: + skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----" + + # Give the user the option to see failed / skipped records if any exist. + display_detailed_logs = False + if not debug and update_failed_count > 0 or update_skipped_count > 0: + display_detailed_logs = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"You will see {update_failed_count} failed and {update_skipped_count} skipped records.", + verify_message="** Some records were skipped, or some failed to update. **", + prompt_title="Do you wish to see the full list of failed, skipped and updated records?", + ) + # Prepare debug messages - if debug: + if debug or display_detailed_logs: updated_display = [str(u) for u in to_update] if display_as_str else to_update skipped_display = [str(s) for s in skipped] if display_as_str else skipped failed_display = [str(f) for f in failed_to_update] if display_as_str else failed_to_update @@ -217,7 +230,7 @@ class TerminalHelper: # Print out a list of everything that was changed, if we have any changes to log. # Otherwise, don't print anything. TerminalHelper.print_conditional( - debug, + True, f"{debug_messages.get('success') if update_success_count > 0 else ''}" f"{debug_messages.get('skipped') if update_skipped_count > 0 else ''}" f"{debug_messages.get('failed') if update_failed_count > 0 else ''}", @@ -236,7 +249,7 @@ class TerminalHelper: f"""{TerminalColors.YELLOW} {log_header} Updated {update_success_count} entries - ----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) ----- + {skipped_header} Skipped updating {update_skipped_count} entries {TerminalColors.ENDC} """ @@ -368,7 +381,9 @@ class TerminalHelper: logger.info(print_statement) @staticmethod - def prompt_for_execution(system_exit_on_terminate: bool, prompt_message: str, prompt_title: str) -> bool: + def prompt_for_execution( + system_exit_on_terminate: bool, prompt_message: str, prompt_title: str, verify_message=None + ) -> bool: """Create to reduce code complexity. Prompts the user to inspect the given string and asks if they wish to proceed. @@ -380,6 +395,9 @@ class TerminalHelper: if system_exit_on_terminate: action_description_for_selecting_no = "exit" + if verify_message is None: + verify_message = "*** IMPORTANT: VERIFY THE FOLLOWING LOOKS CORRECT ***" + # Allow the user to inspect the command string # and ask if they wish to proceed proceed_execution = TerminalHelper.query_yes_no_exit( @@ -387,7 +405,7 @@ class TerminalHelper: ===================================================== {prompt_title} ===================================================== - *** IMPORTANT: VERIFY THE FOLLOWING LOOKS CORRECT *** + {verify_message} {prompt_message} {TerminalColors.FAIL} diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index a1738cc76..8b3832d62 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -10,18 +10,21 @@ from .host import Host from .domain_invitation import DomainInvitation from .user_domain_role import UserDomainRole from .public_contact import PublicContact + +# IMPORTANT: UserPortfolioPermission must be before PortfolioInvitation. +# PortfolioInvitation imports from UserPortfolioPermission, so you will get a circular import otherwise. +from .user_portfolio_permission import UserPortfolioPermission +from .portfolio_invitation import PortfolioInvitation from .user import User from .user_group import UserGroup from .website import Website from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .waffle_flag import WaffleFlag -from .portfolio_invitation import PortfolioInvitation from .portfolio import Portfolio from .domain_group import DomainGroup from .suborganization import Suborganization from .senior_official import SeniorOfficial -from .user_portfolio_permission import UserPortfolioPermission from .allowed_email import AllowedEmail diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 61a6b7397..11c564c36 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -1,16 +1,18 @@ """People are invited by email to administer domains.""" import logging -from django.contrib.auth import get_user_model from django.db import models from django_fsm import FSMField, transition -from registrar.models.domain_invitation import DomainInvitation -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from .utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices # type: ignore +from django.contrib.auth import get_user_model +from registrar.models import DomainInvitation, UserPortfolioPermission +from .utility.portfolio_helper import ( + UserPortfolioPermissionChoices, + UserPortfolioRoleChoices, + validate_portfolio_invitation, +) # type: ignore from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField - logger = logging.getLogger(__name__) @@ -108,3 +110,8 @@ class PortfolioInvitation(TimeStampedModel): if self.additional_permissions and len(self.additional_permissions) > 0: user_portfolio_permission.additional_permissions = self.additional_permissions user_portfolio_permission.save() + + def clean(self): + """Extends clean method to perform additional validation, which can raise errors in django admin.""" + super().clean() + validate_portfolio_invitation(self) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 6c9c37c92..bdfc6f804 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -1,15 +1,13 @@ import logging -from django.apps import apps from django.contrib.auth.models import AbstractUser from django.db import models from django.db.models import Q -from registrar.models import DomainInformation, UserDomainRole +from registrar.models import DomainInformation, UserDomainRole, PortfolioInvitation, UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .domain_invitation import DomainInvitation -from .portfolio_invitation import PortfolioInvitation from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .domain import Domain @@ -501,8 +499,6 @@ class User(AbstractUser): def is_only_admin_of_portfolio(self, portfolio): """Check if the user is the only admin of the given portfolio.""" - UserPortfolioPermission = apps.get_model("registrar", "UserPortfolioPermission") - admin_permission = UserPortfolioRoleChoices.ORGANIZATION_ADMIN admins = UserPortfolioPermission.objects.filter(portfolio=portfolio, roles__contains=[admin_permission]) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 319f15d67..a149a9476 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -1,12 +1,11 @@ from django.db import models -from django.forms import ValidationError from registrar.models.user_domain_role import UserDomainRole -from registrar.utility.waffle import flag_is_active_for_user from registrar.models.utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay, + validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField @@ -22,18 +21,29 @@ class UserPortfolioPermission(TimeStampedModel): UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MEMBERS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, # Domain: field specific permissions UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], + # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here. UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, ], } + # Determines which roles are forbidden for certain role types to possess. + # Used to throw a ValidationError on clean() for UserPortfolioPermission and PortfolioInvitation. + FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS = { + UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + ], + } + user = models.ForeignKey( "registrar.User", null=False, @@ -142,30 +152,30 @@ class UserPortfolioPermission(TimeStampedModel): else: return MemberPermissionDisplay.NONE + @classmethod + def get_forbidden_permissions(cls, roles, additional_permissions): + """Some permissions are forbidden for certain roles, like member. + This checks for conflicts between the current permission list and forbidden perms.""" + + # Get the portfolio permissions that the user currently possesses + portfolio_permissions = set(cls.get_portfolio_permissions(roles, additional_permissions)) + + # Get intersection of forbidden permissions across all roles. + # This is because if you have roles ["admin", "member"], then they can have the + # so called "forbidden" ones. But just member on their own cannot. + # The solution to this is to only grab what is only COMMONLY "forbidden". + # This will scale if we add more roles in the future. + # This is thes same as applying the `&` operator across all sets for each role. + common_forbidden_perms = set.intersection( + *[set(cls.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) for role in roles] + ) + + # Check if the users current permissions overlap with any forbidden permissions + # by getting the intersection between current user permissions, and forbidden ones. + # This is the same as portfolio_permissions & common_forbidden_perms. + return portfolio_permissions.intersection(common_forbidden_perms) + def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() - - # Check if portfolio is set without accessing the related object. - has_portfolio = bool(self.portfolio_id) - if not has_portfolio and self._get_portfolio_permissions(): - raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") - - if has_portfolio and not self._get_portfolio_permissions(): - raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") - - # Check if a user is set without accessing the related object. - has_user = bool(self.user_id) - if has_user: - existing_permission_pks = UserPortfolioPermission.objects.filter(user=self.user).values_list( - "pk", flat=True - ) - if ( - not flag_is_active_for_user(self.user, "multiple_portfolios") - and existing_permission_pks.exists() - and self.pk not in existing_permission_pks - ): - raise ValidationError( - "This user is already assigned to a portfolio. " - "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." - ) + validate_user_portfolio_permission(self) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index f1a6cec7a..3768aa77a 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -1,5 +1,9 @@ from registrar.utility import StrEnum from django.db import models +from django.apps import apps +from django.forms import ValidationError +from registrar.utility.waffle import flag_is_active_for_user +from django.contrib.auth import get_user_model class UserPortfolioRoleChoices(models.TextChoices): @@ -69,3 +73,131 @@ class MemberPermissionDisplay(StrEnum): MANAGER = "Manager" VIEWER = "Viewer" NONE = "None" + + +def validate_user_portfolio_permission(user_portfolio_permission): + """ + Validates a UserPortfolioPermission instance. Located in portfolio_helper to avoid circular imports + between PortfolioInvitation and UserPortfolioPermission models. + + Used in UserPortfolioPermission.clean() for model validation. + + Validates: + 1. A portfolio must be assigned if roles or additional permissions are specified, and vice versa. + 2. Assigned roles do not include any forbidden permissions. + 3. If the 'multiple_portfolios' flag is inactive for the user, + they must not have existing portfolio permissions or invitations. + + Raises: + ValidationError: If any of the validation rules are violated. + """ + PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation") + UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") + + has_portfolio = bool(user_portfolio_permission.portfolio_id) + portfolio_permissions = set(user_portfolio_permission._get_portfolio_permissions()) + + # == Validate required fields == # + if not has_portfolio and portfolio_permissions: + raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") + + if has_portfolio and not portfolio_permissions: + raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") + + # == Validate role permissions. Compares existing permissions to forbidden ones. == # + roles = user_portfolio_permission.roles if user_portfolio_permission.roles is not None else [] + bad_perms = user_portfolio_permission.get_forbidden_permissions( + roles, user_portfolio_permission.additional_permissions + ) + if bad_perms: + readable_perms = [ + UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm) for perm in bad_perms + ] + readable_roles = [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in roles] + raise ValidationError( + f"These permissions cannot be assigned to {', '.join(readable_roles)}: <{', '.join(readable_perms)}>" + ) + + # == Validate the multiple_porfolios flag. == # + if not flag_is_active_for_user(user_portfolio_permission.user, "multiple_portfolios"): + existing_permissions = UserPortfolioPermission.objects.exclude(id=user_portfolio_permission.id).filter( + user=user_portfolio_permission.user + ) + if existing_permissions.exists(): + raise ValidationError( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ) + + existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email) + if existing_invitations.exists(): + raise ValidationError( + "This user is already assigned to a portfolio invitation. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ) + + +def validate_portfolio_invitation(portfolio_invitation): + """ + Validates a PortfolioInvitation instance. Located in portfolio_helper to avoid circular imports + between PortfolioInvitation and UserPortfolioPermission models. + + Used in PortfolioInvitation.clean() for model validation. + + Validates: + 1. A portfolio must be assigned if roles or additional permissions are specified, and vice versa. + 2. Assigned roles do not include any forbidden permissions. + 3. If the 'multiple_portfolios' flag is inactive for the user, + they must not have existing portfolio permissions or invitations. + + Raises: + ValidationError: If any of the validation rules are violated. + """ + PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation") + UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") + User = get_user_model() + + has_portfolio = bool(portfolio_invitation.portfolio_id) + portfolio_permissions = set(portfolio_invitation.get_portfolio_permissions()) + + # == Validate required fields == # + if not has_portfolio and portfolio_permissions: + raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") + + if has_portfolio and not portfolio_permissions: + raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") + + # == Validate role permissions. Compares existing permissions to forbidden ones. == # + roles = portfolio_invitation.roles if portfolio_invitation.roles is not None else [] + bad_perms = UserPortfolioPermission.get_forbidden_permissions(roles, portfolio_invitation.additional_permissions) + if bad_perms: + readable_perms = [ + UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm) for perm in bad_perms + ] + readable_roles = [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in roles] + raise ValidationError( + f"These permissions cannot be assigned to {', '.join(readable_roles)}: <{', '.join(readable_perms)}>" + ) + + # == Validate the multiple_porfolios flag. == # + user = User.objects.filter(email=portfolio_invitation.email).first() + # If user returns None, then we check for global assignment of multiple_portfolios. + # Otherwise we just check on the user. + if not flag_is_active_for_user(user, "multiple_portfolios"): + existing_permissions = UserPortfolioPermission.objects.filter(user=user) + + existing_invitations = PortfolioInvitation.objects.exclude(id=portfolio_invitation.id).filter( + email=portfolio_invitation.email + ) + + if existing_permissions.exists(): + raise ValidationError( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ) + + if existing_invitations.exists(): + raise ValidationError( + "This user is already assigned to a portfolio invitation. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ) diff --git a/src/registrar/templates/domain_request_org_contact.html b/src/registrar/templates/domain_request_org_contact.html index d39fb9f78..53e45dd16 100644 --- a/src/registrar/templates/domain_request_org_contact.html +++ b/src/registrar/templates/domain_request_org_contact.html @@ -37,12 +37,9 @@ {% input_with_errors forms.0.zipcode %} {% endwith %} -