diff --git a/.github/workflows/deploy-development.yaml b/.github/workflows/deploy-development.yaml index e62607d95..03994e1de 100644 --- a/.github/workflows/deploy-development.yaml +++ b/.github/workflows/deploy-development.yaml @@ -22,16 +22,9 @@ jobs: - name: Compile USWDS assets working-directory: ./src run: | - docker compose run node bash -c "\ - curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.1/install.sh | bash && \ - export NVM_DIR=\"\$HOME/.nvm\" && \ - [ -s \"\$NVM_DIR/nvm.sh\" ] && \. \"\$NVM_DIR/nvm.sh\" && \ - [ -s \"\$NVM_DIR/bash_completion\" ] && \. \"\$NVM_DIR/bash_completion\" && \ - nvm install 21.7.3 && \ - nvm use 21.7.3 && \ - npm install && \ - npx gulp copyAssets && \ - npx gulp compile" + docker compose run node npm install && + docker compose run node npx gulp copyAssets && + docker compose run node npx gulp compile - name: Collect static assets working-directory: ./src run: docker compose run app python manage.py collectstatic --no-input diff --git a/.github/workflows/deploy-sandbox.yaml b/.github/workflows/deploy-sandbox.yaml index 1c486bdf7..214cf6076 100644 --- a/.github/workflows/deploy-sandbox.yaml +++ b/.github/workflows/deploy-sandbox.yaml @@ -43,16 +43,9 @@ jobs: - name: Compile USWDS assets working-directory: ./src run: | - docker compose run node bash -c "\ - curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.1/install.sh | bash && \ - export NVM_DIR=\"\$HOME/.nvm\" && \ - [ -s \"\$NVM_DIR/nvm.sh\" ] && \. \"\$NVM_DIR/nvm.sh\" && \ - [ -s \"\$NVM_DIR/bash_completion\" ] && \. \"\$NVM_DIR/bash_completion\" && \ - nvm install 21.7.3 && \ - nvm use 21.7.3 && \ - npm install && \ - npx gulp copyAssets && \ - npx gulp compile" + docker compose run node npm install && + docker compose run node npx gulp copyAssets && + docker compose run node npx gulp compile - name: Collect static assets working-directory: ./src run: docker compose run app python manage.py collectstatic --no-input diff --git a/.github/workflows/deploy-stable.yaml b/.github/workflows/deploy-stable.yaml index 9d0573e01..a1b947ca5 100644 --- a/.github/workflows/deploy-stable.yaml +++ b/.github/workflows/deploy-stable.yaml @@ -23,16 +23,9 @@ jobs: - name: Compile USWDS assets working-directory: ./src run: | - docker compose run node bash -c "\ - curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.1/install.sh | bash && \ - export NVM_DIR=\"\$HOME/.nvm\" && \ - [ -s \"\$NVM_DIR/nvm.sh\" ] && \. \"\$NVM_DIR/nvm.sh\" && \ - [ -s \"\$NVM_DIR/bash_completion\" ] && \. \"\$NVM_DIR/bash_completion\" && \ - nvm install 21.7.3 && \ - nvm use 21.7.3 && \ - npm install && \ - npx gulp copyAssets && \ - npx gulp compile" + docker compose run node npm install && + docker compose run node npx gulp copyAssets && + docker compose run node npx gulp compile - name: Collect static assets working-directory: ./src run: docker compose run app python manage.py collectstatic --no-input diff --git a/.github/workflows/deploy-staging.yaml b/.github/workflows/deploy-staging.yaml index 9584985f0..3cf5ad5a1 100644 --- a/.github/workflows/deploy-staging.yaml +++ b/.github/workflows/deploy-staging.yaml @@ -23,16 +23,9 @@ jobs: - name: Compile USWDS assets working-directory: ./src run: | - docker compose run node bash -c "\ - curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.1/install.sh | bash && \ - export NVM_DIR=\"\$HOME/.nvm\" && \ - [ -s \"\$NVM_DIR/nvm.sh\" ] && \. \"\$NVM_DIR/nvm.sh\" && \ - [ -s \"\$NVM_DIR/bash_completion\" ] && \. \"\$NVM_DIR/bash_completion\" && \ - nvm install 21.7.3 && \ - nvm use 21.7.3 && \ - npm install && \ - npx gulp copyAssets && \ - npx gulp compile" + docker compose run node npm install && + docker compose run node npx gulp copyAssets && + docker compose run node npx gulp compile - name: Collect static assets working-directory: ./src run: docker compose run app python manage.py collectstatic --no-input diff --git a/src/.pa11yci b/src/.pa11yci index 12b76cd90..56ea40416 100644 --- a/src/.pa11yci +++ b/src/.pa11yci @@ -19,6 +19,7 @@ "http://localhost:8080/request/other_contacts/", "http://localhost:8080/request/anything_else/", "http://localhost:8080/request/requirements/", - "http://localhost:8080/request/finished/" + "http://localhost:8080/request/finished/", + "http://localhost:8080/user-profile/" ] } diff --git a/src/djangooidc/oidc.py b/src/djangooidc/oidc.py index 6e6c209f0..95ed322f5 100644 --- a/src/djangooidc/oidc.py +++ b/src/djangooidc/oidc.py @@ -15,7 +15,6 @@ from oic.oic.message import AccessTokenResponse from oic.utils.authn.client import CLIENT_AUTHN_METHOD from oic.utils import keyio - from . import exceptions as o_e __author__ = "roland" @@ -85,7 +84,6 @@ class Client(oic.Client): def create_authn_request( self, session, - do_step_up_auth=False, extra_args=None, ): """Step 2: Construct a login URL at OP's domain and send the user to it.""" @@ -102,11 +100,10 @@ class Client(oic.Client): "state": session["state"], "nonce": session["nonce"], "redirect_uri": self.registration_response["redirect_uris"][0], + # acr_value may be passed in session if overriding, as in the case + # of step up auth, otherwise get from settings.py + "acr_values": session.get("acr_value") or self.behaviour.get("acr_value"), } - if do_step_up_auth: - self._set_args_for_biometric_auth_request(session, request_args) - else: - request_args["acr_values"] = self.behaviour.get("acr_value") if extra_args is not None: request_args.update(extra_args) @@ -117,35 +114,6 @@ class Client(oic.Client): logger.debug("request args: %s" % request_args) - url, headers = self._prepare_authn_request(request_args, state) # C901 too complex - - try: - # create the redirect object - response = HttpResponseRedirect(str(url)) - # add headers to the object, if any - if headers: - for key, value in headers.items(): - response[key] = value - - except Exception as err: - logger.error(err) - logger.error("Failed to create redirect object for %s" % state) - raise o_e.InternalError(locator=state) - - return response - - def _set_args_for_biometric_auth_request(self, session, request_args): - if "acr_value" in session: - session.pop("acr_value") - request_args["vtr"] = session.get("vtr") - request_args["vtm"] = session.get("vtm") - - def _prepare_authn_request(self, request_args, state): - """ - Constructs an authorization request. Then, assembles the url, body, headers, and cis. - - Returns the assembled url and associated header information: `(url, headers)` - """ try: # prepare the request for sending cis = self.construct_AuthorizationRequest(request_args=request_args) @@ -158,7 +126,6 @@ class Client(oic.Client): method="GET", request_args=request_args, ) - logger.debug("body: %s" % body) logger.debug("URL: %s" % url) logger.debug("headers: %s" % headers) @@ -167,7 +134,19 @@ class Client(oic.Client): logger.error("Failed to prepare request for %s" % state) raise o_e.InternalError(locator=state) - return (url, headers) + try: + # create the redirect object + response = HttpResponseRedirect(str(url)) + # add headers to the object, if any + if headers: + for key, value in headers.items(): + response[key] = value + except Exception as err: + logger.error(err) + logger.error("Failed to create redirect object for %s" % state) + raise o_e.InternalError(locator=state) + + return response def callback(self, unparsed_response, session): """Step 3: Receive OP's response, request an access token, and user info.""" @@ -245,18 +224,9 @@ class Client(oic.Client): if isinstance(info_response, ErrorResponse): logger.error("Unable to get user info (%s) for %s" % (info_response.get("error", ""), state)) raise o_e.AuthenticationFailed(locator=state) - info_response_dict = info_response.to_dict() - # Define vtm/vtr information on the user dictionary so we can track this in one location. - # If a user has this information, then they are bumped up in terms of verification level. - if session.get("needs_step_up_auth") is True: - if "ial" in info_response_dict: - info_response_dict.pop("ial") - info_response_dict["vtm"] = session.get("vtm", "") - info_response_dict["vtr"] = session.get("vtr", "") - - logger.debug("user info: %s" % info_response_dict) - return info_response_dict + logger.debug("user info: %s" % info_response) + return info_response.to_dict() def _request_token(self, state, code, session): """Request a token from OP to allow us to then request user info.""" @@ -315,20 +285,14 @@ class Client(oic.Client): super(Client, self).store_response(resp, info) def get_default_acr_value(self): - """Returns the acr_value from settings. - This helper function is called from djangooidc views.""" + """returns the acr_value from settings + this helper function is called from djangooidc views""" return self.behaviour.get("acr_value") - def get_vtm_value(self): - """Returns the vtm value from settings. - This helper function is called from djangooidc views.""" - return self.behaviour.get("vtm") - - def get_vtr_value(self, cleaned=True): - """Returns the vtr value from settings. - This helper function is called from djangooidc views.""" - vtr = self.behaviour.get("vtr") - return json.dumps(vtr) if cleaned else vtr + def get_step_up_acr_value(self): + """returns the step_up_acr_value from settings + this helper function is called from djangooidc views""" + return self.behaviour.get("step_up_acr_value") def __repr__(self): return "Client {} {} {}".format( diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py index 4ca2fda40..bdd61b346 100644 --- a/src/djangooidc/tests/test_views.py +++ b/src/djangooidc/tests/test_views.py @@ -397,31 +397,25 @@ class ViewsTest(TestCase): """Invoke login_callback passing it a request when _requires_step_up_auth returns True and assert that session is updated and create_authn_request (mock) is called.""" with less_console_noise(): + # MOCK + # Configure the mock to return an expected value for get_step_up_acr_value + mock_client.return_value.get_step_up_acr_value.return_value = "step_up_acr_value" # Create a mock request request = self.factory.get("/some-url") request.session = {"acr_value": ""} - # Ensure that the CLIENT instance used in login_callback is the mock # patch _requires_step_up_auth to return True with patch("djangooidc.views._requires_step_up_auth", return_value=True), patch( - "djangooidc.views.CLIENT.create_authn_request" + "djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock() ) as mock_create_authn_request: - with patch("djangooidc.views.CLIENT.get_vtm_value") as mock_get_vtm_value, patch( - "djangooidc.views.CLIENT.get_vtr_value" - ) as mock_get_vtr_value: - mock_get_vtm_value.return_value = "test_vtm" - mock_get_vtr_value.return_value = "test_vtr" - # TEST - # test the login callback - login_callback(request) - + # TEST + # test the login callback + login_callback(request) # ASSERTIONS - # create_authn_request only gets called when _requires_step_up_auth is True. - # The acr_value should be blank here - self.assertEqual(request.session["acr_value"], "") - self.assertEqual(request.session["vtm"], "test_vtm") - self.assertEqual(request.session["vtr"], "test_vtr") - + # create_authn_request only gets called when _requires_step_up_auth is True + # and it changes this acr_value in request.session + # Assert that acr_value is no longer empty string + self.assertNotEqual(request.session["acr_value"], "") # And create_authn_request was called again mock_create_authn_request.assert_called_once() diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index bb2cebd38..815df4ecf 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -91,21 +91,12 @@ def login_callback(request): _initialize_client() query = parse_qs(request.GET.urlencode()) userinfo = CLIENT.callback(query, request.session) - # test for need for identity verification and if it is satisfied - # if not satisfied, redirect user to login requiring biometric auth - - # Tests for the presence of the vtm/vtr values in the userinfo object. - # If they are there, then we can set a flag in our session for tracking purposes. - needs_step_up_auth = _requires_step_up_auth(userinfo) - request.session["needs_step_up_auth"] = needs_step_up_auth - - # Return a redirect request to a new auth url that does biometric validation - if needs_step_up_auth: - request.session["vtm"] = CLIENT.get_vtm_value() - request.session["vtr"] = CLIENT.get_vtr_value() - return CLIENT.create_authn_request(request.session, do_step_up_auth=True) - + # if not satisfied, redirect user to login with stepped up acr_value + if _requires_step_up_auth(userinfo): + # add acr_value to request.session + request.session["acr_value"] = CLIENT.get_step_up_acr_value() + return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) if user: @@ -147,27 +138,14 @@ def login_callback(request): return error_page(request, err) -def _requires_step_up_auth(userinfo) -> bool: - """ - Checks for the presence of the key 'vtm' and 'vtr' in the provided `userinfo` object. - - If they are not found, then we call `User.needs_identity_verification()`. - - Args: - userinfo (dict): A dictionary of data from the returned user object. - - Return Conditions: - If the provided user does not exist in any tables which would preclude them from doing - biometric authentication, then we return True. Otherwise, we return False. - - Alternatively, if 'vtm' and 'vtr' already exist on `userinfo`, then we return False. - - """ +def _requires_step_up_auth(userinfo): + """if User.needs_identity_verification and step_up_acr_value not in + ial returned from callback, return True""" + step_up_acr_value = CLIENT.get_step_up_acr_value() + acr_value = userinfo.get("ial", "") uuid = userinfo.get("sub", "") email = userinfo.get("email", "") - # This value is returned after successful auth - user_verified = userinfo.get("vot", "") - if not userinfo.get("vtm") or not userinfo.get("vtr") or not user_verified: + if acr_value != step_up_acr_value: # The acr of this attempt is not at the highest level # so check if the user needs the higher level return User.needs_identity_verification(email, uuid) diff --git a/src/node.Dockerfile b/src/node.Dockerfile index cf0b6acc6..0fcab7d92 100644 --- a/src/node.Dockerfile +++ b/src/node.Dockerfile @@ -1,5 +1,4 @@ FROM docker.io/cimg/node:current-browsers -FROM node:21.7.3 WORKDIR /app # Install app dependencies @@ -7,6 +6,4 @@ WORKDIR /app # where available (npm@5+) COPY --chown=circleci:circleci package*.json ./ - -RUN npm install -g npm@10.5.0 RUN npm install \ No newline at end of file diff --git a/src/package-lock.json b/src/package-lock.json index ee2dada01..2ff464d5e 100644 --- a/src/package-lock.json +++ b/src/package-lock.json @@ -15,10 +15,6 @@ }, "devDependencies": { "@uswds/compile": "^1.0.0-beta.3" - }, - "engines": { - "node": "21.7.3", - "npm": "10.5.0" } }, "node_modules/@bufbuild/protobuf": { diff --git a/src/package.json b/src/package.json index 487d0dad6..58ae3a4ed 100644 --- a/src/package.json +++ b/src/package.json @@ -3,11 +3,6 @@ "version": "1.0.0", "description": "========================", "main": "index.js", - "engines": { - "node": "21.7.3", - "npm": "10.5.0" - }, - "engineStrict": true, "scripts": { "pa11y-ci": "pa11y-ci", "test": "echo \"Error: no test specified\" && exit 1" diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 41d3a56fb..2d6559570 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1126,7 +1126,6 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "Type of organization", { "fields": [ - "generic_org_type", "is_election_board", "organization_type", ] @@ -1173,7 +1172,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "generic_org_type", "is_election_board", "federal_agency") + readonly_fields = ("other_contacts", "is_election_board", "federal_agency") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ @@ -1387,7 +1386,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "Type of organization", { "fields": [ - "generic_org_type", "is_election_board", "organization_type", ] @@ -1438,7 +1436,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "other_contacts", "current_websites", "alternative_domains", - "generic_org_type", "is_election_board", "federal_agency", ) diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 212df992f..dc115d69e 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -46,6 +46,35 @@ body { margin-top:units(1); } +.usa-nav__primary-username { + display: inline-block; + padding: units(1) units(2); + max-width: 208px; + overflow: hidden; + text-overflow: ellipsis; + @include at-media(desktop) { + padding: units(2); + max-width: 500px; + } +} + +@include at-media(desktop) { + .usa-nav__primary-item:not(:first-child) { + position: relative; + } + + .usa-nav__primary-item:not(:first-child)::before { + content: ''; + position: absolute; + top: 50%; + left: 0; + width: 0; /* No width since it's a border */ + height: 50%; + border-left: solid 1px color('base-light'); + transform: translateY(-50%); + } +} + .section--outlined { background-color: color('white'); border: 1px solid color('base-lighter'); diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index a0b35a6ca..9f31ffc2c 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -562,11 +562,7 @@ OIDC_PROVIDERS = { "scope": ["email", "profile:name", "phone"], "user_info_request": ["email", "first_name", "last_name", "phone"], "acr_value": "http://idmanagement.gov/ns/assurance/ial/1", - # "P1" is the current IdV option; "Pb" stands for 'biometric' - "vtr": ["Pb", "P1"], - # The url that biometric authentication takes place at. - # A similar analog is the url for acr_value. - "vtm": "https://developer.login.gov/vot-trust-framework", + "step_up_acr_value": "http://idmanagement.gov/ns/assurance/ial/2", }, "client_registration": { "client_id": "cisa_dotgov_registrar", @@ -584,11 +580,7 @@ OIDC_PROVIDERS = { "scope": ["email", "profile:name", "phone"], "user_info_request": ["email", "first_name", "last_name", "phone"], "acr_value": "http://idmanagement.gov/ns/assurance/ial/1", - # "P1" is the current IdV option; "Pb" stands for 'biometric' - "vtr": ["Pb", "P1"], - # The url that biometric authentication takes place at. - # A similar analog is the url for acr_value. - "vtm": "https://developer.login.gov/vot-trust-framework", + "step_up_acr_value": "http://idmanagement.gov/ns/assurance/ial/2", }, "client_registration": { "client_id": ("urn:gov:cisa:openidconnect.profiles:sp:sso:cisa:dotgov_registrar"), diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 720034150..158f8e812 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -178,6 +178,11 @@ urlpatterns = [ views.DomainAddUserView.as_view(), name="domain-users-add", ), + path( + "user-profile", + views.UserProfileView.as_view(), + name="user-profile", + ), path( "invitation//delete", views.DomainInvitationDeleteView.as_view(http_method_names=["post"]), @@ -206,6 +211,7 @@ urlpatterns = [ # Rather than dealing with that, we keep everything centralized in one location. # This way, we can share a view for djangooidc, and other pages as we see fit. handler500 = "registrar.views.utility.error_views.custom_500_error_view" +handler403 = "registrar.views.utility.error_views.custom_403_error_view" # we normally would guard these with `if settings.DEBUG` but tests run with # DEBUG = False even when these apps have been loaded because settings.DEBUG diff --git a/src/registrar/forms/user_profile.py b/src/registrar/forms/user_profile.py new file mode 100644 index 000000000..11b5cc069 --- /dev/null +++ b/src/registrar/forms/user_profile.py @@ -0,0 +1,63 @@ +from django import forms + +from registrar.models.contact import Contact + +from django.core.validators import MaxLengthValidator +from phonenumber_field.widgets import RegionalPhoneNumberWidget +from registrar.models.utility.domain_helper import DomainHelper + + +class UserProfileForm(forms.ModelForm): + """Form for updating user profile.""" + + class Meta: + model = Contact + fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"] + widgets = { + "first_name": forms.TextInput, + "middle_name": forms.TextInput, + "last_name": forms.TextInput, + "title": forms.TextInput, + "email": forms.EmailInput, + "phone": RegionalPhoneNumberWidget, + } + + # the database fields have blank=True so ModelForm doesn't create + # required fields by default. Use this list in __init__ to mark each + # of these fields as required + required = ["first_name", "last_name", "title", "email", "phone"] + + def __init__(self, *args, **kwargs): + """Override the inerited __init__ method to update the fields.""" + + super().__init__(*args, **kwargs) + # take off maxlength attribute for the phone number field + # which interferes with out input_with_errors template tag + self.fields["phone"].widget.attrs.pop("maxlength", None) + + # Define a custom validator for the email field with a custom error message + email_max_length_validator = MaxLengthValidator(320, message="Response must be less than 320 characters.") + self.fields["email"].validators.append(email_max_length_validator) + + for field_name in self.required: + self.fields[field_name].required = True + + # Set custom form label + self.fields["first_name"].label = "First name / given name" + self.fields["middle_name"].label = "Middle name (optional)" + self.fields["last_name"].label = "Last name / family name" + self.fields["title"].label = "Title or role in your organization" + self.fields["email"].label = "Organizational email" + + # Set custom error messages + self.fields["first_name"].error_messages = {"required": "Enter your first name / given name."} + self.fields["last_name"].error_messages = {"required": "Enter your last name / family name."} + self.fields["title"].error_messages = { + "required": "Enter your title or role in your organization (e.g., Chief Information Officer)" + } + self.fields["email"].error_messages = { + "required": "Enter your email address in the required format, like name@example.com." + } + self.fields["phone"].error_messages["required"] = "Enter your phone number." + + DomainHelper.disable_field(self.fields["email"], disable_required=True) diff --git a/src/registrar/templates/base.html b/src/registrar/templates/base.html index 4f2025cc7..b5e3a6ada 100644 --- a/src/registrar/templates/base.html +++ b/src/registrar/templates/base.html @@ -134,7 +134,7 @@ {% block usa_overlay %}
{% endblock %} {% block banner %} -
+
{% block logo %} @@ -147,19 +147,25 @@
{% block usa_nav %} -
{% endif %} -{% if widget.attrs.maxlength %} +{% if widget.attrs.maxlength and not do_not_show_max_chars %} -
-
- Your profile -

- Required fields are marked with an asterisk (*). +

+ + + +

+ Back to manage your domains

- {% for field in profile_form %} - - {{ field }} - {% endfor %} -
- - +
+ {# messages block is under the back breadcrumb link #} + {% if messages %} + {% for message in messages %} +
+
+ {{ message }} +
+
+ {% endfor %} + {% endif %} + {% include "includes/form_errors.html" with form=form %} + +

Your profile

+

We require that you maintain accurate contact information. The details you provide will only be used to support the administration of .gov and won’t be made public.

+ +

Contact information

+

Review the details below and update any required information. Note that editing this information won’t affect your Login.gov account information.

+ + {% include "includes/required_fields.html" %} + +
+ {% csrf_token %} + + {% input_with_errors form.first_name %} + + {% input_with_errors form.middle_name %} + + {% input_with_errors form.last_name %} + + {% input_with_errors form.title %} + + {% public_site_url "help/account-management/#get-help-with-login.gov" as login_help_url %} + + {% with link_href=login_help_url %} + {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with your Login.gov account." %} + {% with link_text="Get help with your Login.gov account" %} + {% with target_blank=True %} + {% with do_not_show_max_chars=True %} + {% input_with_errors form.email %} + {% endwith %} + {% endwith %} + {% endwith %} + {% endwith %} + {% endwith %} + + {% with add_class="usa-input--medium" %} + {% input_with_errors form.phone %} + {% endwith %} + + +
{% endblock content %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e65d3d4c4..e2d390471 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2230,7 +2230,6 @@ class TestDomainRequestAdmin(MockEppLib): "other_contacts", "current_websites", "alternative_domains", - "generic_org_type", "is_election_board", "federal_agency", "id", @@ -2285,7 +2284,6 @@ class TestDomainRequestAdmin(MockEppLib): "other_contacts", "current_websites", "alternative_domains", - "generic_org_type", "is_election_board", "federal_agency", "creator", @@ -2314,7 +2312,6 @@ class TestDomainRequestAdmin(MockEppLib): "other_contacts", "current_websites", "alternative_domains", - "generic_org_type", "is_election_board", "federal_agency", ] @@ -3173,7 +3170,6 @@ class TestDomainInformationAdmin(TestCase): expected_fields = [ "other_contacts", - "generic_org_type", "is_election_board", "federal_agency", "creator", diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 217e24b9a..8c8a0fda0 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,11 +1,14 @@ from datetime import date from django.test import Client, TestCase, override_settings from django.contrib.auth import get_user_model +from django_webtest import WebTest # type: ignore +from django.conf import settings from api.tests.common import less_console_noise_decorator from registrar.models.contact import Contact from registrar.models.domain import Domain from registrar.models.draft_domain import DraftDomain +from registrar.models.public_contact import PublicContact from registrar.models.user import User from registrar.models.user_domain_role import UserDomainRole from registrar.views.domain import DomainNameserversView @@ -18,6 +21,7 @@ from registrar.models import ( DomainRequest, DomainInformation, ) +from waffle.testutils import override_flag import logging logger = logging.getLogger(__name__) @@ -505,3 +509,177 @@ class HomeTests(TestWithUser): with less_console_noise(): response = self.client.get("/request/", follow=True) self.assertEqual(response.status_code, 403) + + +class UserProfileTests(TestWithUser, WebTest): + """A series of tests that target your profile functionality""" + + def setUp(self): + super().setUp() + self.client.force_login(self.user) + self.domain, _ = Domain.objects.get_or_create(name="sampledomain.gov", state=Domain.State.READY) + self.role, _ = UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER + ) + + def tearDown(self): + super().tearDown() + PublicContact.objects.filter(domain=self.domain).delete() + self.role.delete() + self.domain.delete() + Contact.objects.all().delete() + DraftDomain.objects.all().delete() + DomainRequest.objects.all().delete() + + @less_console_noise_decorator + def error_500_main_nav_with_profile_feature_turned_on(self): + """test that Your profile is in main nav of 500 error page when profile_feature is on. + + Our treatment of 401 and 403 error page handling with that waffle feature is similar, so we + assume that the same test results hold true for 401 and 403.""" + with override_flag("profile_feature", active=True): + with self.assertRaises(Exception): + response = self.client.get(reverse("home")) + self.assertEqual(response.status_code, 500) + self.assertContains(response, "Your profile") + + @less_console_noise_decorator + def error_500_main_nav_with_profile_feature_turned_off(self): + """test that Your profile is not in main nav of 500 error page when profile_feature is off. + + Our treatment of 401 and 403 error page handling with that waffle feature is similar, so we + assume that the same test results hold true for 401 and 403.""" + with override_flag("profile_feature", active=False): + with self.assertRaises(Exception): + response = self.client.get(reverse("home"), follow=True) + self.assertEqual(response.status_code, 500) + self.assertNotContains(response, "Your profile") + + @less_console_noise_decorator + def test_home_page_main_nav_with_profile_feature_on(self): + """test that Your profile is in main nav of home page when profile_feature is on""" + with override_flag("profile_feature", active=True): + response = self.client.get("/") + self.assertContains(response, "Your profile") + + @less_console_noise_decorator + def test_home_page_main_nav_with_profile_feature_off(self): + """test that Your profile is not in main nav of home page when profile_feature is off""" + with override_flag("profile_feature", active=False): + response = self.client.get("/") + self.assertNotContains(response, "Your profile") + + @less_console_noise_decorator + def test_new_request_main_nav_with_profile_feature_on(self): + """test that Your profile is in main nav of new request when profile_feature is on""" + with override_flag("profile_feature", active=True): + response = self.client.get("/request/") + self.assertContains(response, "Your profile") + + @less_console_noise_decorator + def test_new_request_main_nav_with_profile_feature_off(self): + """test that Your profile is not in main nav of new request when profile_feature is off""" + with override_flag("profile_feature", active=False): + response = self.client.get("/request/") + self.assertNotContains(response, "Your profile") + + @less_console_noise_decorator + def test_user_profile_main_nav_with_profile_feature_on(self): + """test that Your profile is in main nav of user profile when profile_feature is on""" + with override_flag("profile_feature", active=True): + response = self.client.get("/user-profile") + self.assertContains(response, "Your profile") + + @less_console_noise_decorator + def test_user_profile_returns_404_when_feature_off(self): + """test that Your profile returns 404 when profile_feature is off""" + with override_flag("profile_feature", active=False): + response = self.client.get("/user-profile") + self.assertEqual(response.status_code, 404) + + @less_console_noise_decorator + def test_domain_detail_profile_feature_on(self): + """test that domain detail view when profile_feature is on""" + with override_flag("profile_feature", active=True): + response = self.client.get(reverse("domain", args=[self.domain.pk])) + self.assertContains(response, "Your profile") + self.assertNotContains(response, "Your contact information") + + @less_console_noise_decorator + def test_domain_your_contact_information_when_profile_feature_off(self): + """test that Your contact information is accessible when profile_feature is off""" + with override_flag("profile_feature", active=False): + response = self.client.get(f"/domain/{self.domain.id}/your-contact-information") + self.assertContains(response, "Your contact information") + + @less_console_noise_decorator + def test_domain_your_contact_information_when_profile_feature_on(self): + """test that Your contact information is not accessible when profile feature is on""" + with override_flag("profile_feature", active=True): + response = self.client.get(f"/domain/{self.domain.id}/your-contact-information") + self.assertEqual(response.status_code, 404) + + @less_console_noise_decorator + def test_request_when_profile_feature_on(self): + """test that Your profile is in request page when profile feature is on""" + + contact_user, _ = Contact.objects.get_or_create(user=self.user) + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, + requested_domain=site, + status=DomainRequest.DomainRequestStatus.SUBMITTED, + authorizing_official=contact_user, + submitter=contact_user, + ) + with override_flag("profile_feature", active=True): + response = self.client.get(f"/domain-request/{domain_request.id}") + self.assertContains(response, "Your profile") + response = self.client.get(f"/domain-request/{domain_request.id}/withdraw") + self.assertContains(response, "Your profile") + + @less_console_noise_decorator + def test_request_when_profile_feature_off(self): + """test that Your profile is not in request page when profile feature is off""" + + contact_user, _ = Contact.objects.get_or_create(user=self.user) + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, + requested_domain=site, + status=DomainRequest.DomainRequestStatus.SUBMITTED, + authorizing_official=contact_user, + submitter=contact_user, + ) + with override_flag("profile_feature", active=False): + response = self.client.get(f"/domain-request/{domain_request.id}") + self.assertNotContains(response, "Your profile") + response = self.client.get(f"/domain-request/{domain_request.id}/withdraw") + self.assertNotContains(response, "Your profile") + # cleanup + domain_request.delete() + site.delete() + + @less_console_noise_decorator + def test_user_profile_form_submission(self): + """test user profile form submission""" + self.app.set_user(self.user.username) + with override_flag("profile_feature", active=True): + profile_page = self.app.get(reverse("user-profile")) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + profile_form = profile_page.form + profile_page = profile_form.submit() + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # assert that first result contains errors + self.assertContains(profile_page, "Enter your title") + self.assertContains(profile_page, "Enter your phone number") + profile_form = profile_page.form + profile_form["title"] = "sample title" + profile_form["phone"] = "(201) 555-1212" + profile_page = profile_form.submit() + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + profile_page = profile_page.follow() + self.assertEqual(profile_page.status_code, 200) + self.assertContains(profile_page, "Your profile has been updated") diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 5f61181c7..51d61355a 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -32,8 +32,9 @@ def send_templated_email( template_name and subject_template_name are relative to the same template context as Django's HTML templates. context gives additional information that the template may use. + + Raises EmailSendingError if SES client could not be accessed """ - logger.info(f"An email was sent! Template name: {template_name} to {to_address}") template = get_template(template_name) email_body = template.render(context=context) @@ -48,7 +49,9 @@ def send_templated_email( aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY, config=settings.BOTO_CONFIG, ) + logger.info(f"An email was sent! Template name: {template_name} to {to_address}") except Exception as exc: + logger.debug("E-mail unable to send! Could not access the SES client.") raise EmailSendingError("Could not access the SES client.") from exc destination = {"ToAddresses": [to_address]} diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index bd15196d4..4ed5bfa75 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -14,5 +14,6 @@ from .domain import ( DomainInvitationDeleteView, DomainDeleteUserView, ) +from .user_profile import UserProfileView from .health import * from .index import * diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9134080a1..b7ce06cce 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -59,7 +59,7 @@ from epplibwrapper import ( from ..utility.email import send_templated_email, EmailSendingError from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView - +from waffle.decorators import flag_is_active, waffle_flag logger = logging.getLogger(__name__) @@ -102,6 +102,13 @@ class DomainBaseView(DomainPermissionView): domain_pk = "domain:" + str(self.kwargs.get("pk")) self.session[domain_pk] = self.object + def get_context_data(self, **kwargs): + """Extend get_context_data to add has_profile_feature_flag to context""" + context = super().get_context_data(**kwargs) + # This is a django waffle flag which toggles features based off of the "flag" table + context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") + return context + class DomainFormBaseView(DomainBaseView, FormMixin): """ @@ -568,6 +575,10 @@ class DomainYourContactInformationView(DomainFormBaseView): template_name = "domain_your_contact_information.html" form_class = ContactForm + @waffle_flag("!profile_feature") # type: ignore + def dispatch(self, request, *args, **kwargs): # type: ignore + return super().dispatch(request, *args, **kwargs) + def get_form_kwargs(self, *args, **kwargs): """Add domain_info.submitter instance to make a bound form.""" form_kwargs = super().get_form_kwargs(*args, **kwargs) @@ -734,7 +745,10 @@ class DomainAddUserView(DomainFormBaseView): does not make a domain information object email: string- email to send to add_success: bool- default True indicates: - adding a success message to the view if the email sending succeeds""" + adding a success message to the view if the email sending succeeds + + raises EmailSendingError + """ # Set a default email address to send to for staff requestor_email = settings.DEFAULT_FROM_EMAIL @@ -762,33 +776,43 @@ class DomainAddUserView(DomainFormBaseView): "requestor_email": requestor_email, }, ) - except EmailSendingError: - messages.warning(self.request, "Could not send email invitation.") + except EmailSendingError as exc: logger.warn( "Could not sent email invitation to %s for domain %s", email, self.object, exc_info=True, ) + raise EmailSendingError("Could not send email invitation.") from exc else: if add_success: messages.success(self.request, f"{email} has been invited to this domain.") def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" - invitation, created = DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) - if not created: + # Check to see if an invite has already been sent (NOTE: we do not want to create an invite just yet.) + try: + invite = DomainInvitation.objects.get(email=email_address, domain=self.object) # that invitation already existed - messages.warning( - self.request, - f"{email_address} has already been invited to this domain.", - ) - else: - self._send_domain_invitation_email(email=email_address, requestor=requestor) + if invite is not None: + messages.warning( + self.request, + f"{email_address} has already been invited to this domain.", + ) + except DomainInvitation.DoesNotExist: + # Try to send the invitation. If it succeeds, add it to the DomainInvitation table. + try: + self._send_domain_invitation_email(email=email_address, requestor=requestor) + except EmailSendingError: + messages.warning(self.request, "Could not send email invitation.") + else: + # (NOTE: only create a domainInvitation if the e-mail sends correctly) + DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) return redirect(self.get_success_url()) def form_valid(self, form): - """Add the specified user on this domain.""" + """Add the specified user on this domain. + Throws EmailSendingError.""" requested_email = form.cleaned_data["email"] requestor = self.request.user # look up a user with that email @@ -799,7 +823,22 @@ class DomainAddUserView(DomainFormBaseView): return self._make_invitation(requested_email, requestor) else: # if user already exists then just send an email - self._send_domain_invitation_email(requested_email, requestor, add_success=False) + try: + self._send_domain_invitation_email(requested_email, requestor, add_success=False) + except EmailSendingError: + logger.warn( + "Could not send email invitation (EmailSendingError)", + self.object, + exc_info=True, + ) + messages.warning(self.request, "Could not send email invitation.") + except Exception: + logger.warn( + "Could not send email invitation (Other Exception)", + self.object, + exc_info=True, + ) + messages.warning(self.request, "Could not send email invitation.") try: UserDomainRole.objects.create( diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 8fb104005..f1d3d49dc 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -227,6 +227,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): # will NOT be redirected. The purpose of this is to allow code to # send users "to the domain request wizard" without needing to # know which view is first in the list of steps. + context = self.get_context_data() if self.__class__ == DomainRequestWizard: if request.path_info == self.NEW_URL_NAME: context = self.get_context_data() @@ -235,7 +236,6 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): return self.goto(self.steps.first) self.steps.current = current_url - context = self.get_context_data() context["forms"] = self.get_forms() # if pending requests exist and user does not have approved domains, @@ -705,6 +705,13 @@ class Finished(DomainRequestWizard): class DomainRequestStatus(DomainRequestPermissionView): template_name = "domain_request_status.html" + def get_context_data(self, **kwargs): + """Extend get_context_data to add has_profile_feature_flag to context""" + context = super().get_context_data(**kwargs) + # This is a django waffle flag which toggles features based off of the "flag" table + context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") + return context + class DomainRequestWithdrawConfirmation(DomainRequestPermissionWithdrawView): """This page will ask user to confirm if they want to withdraw @@ -715,6 +722,13 @@ class DomainRequestWithdrawConfirmation(DomainRequestPermissionWithdrawView): template_name = "domain_request_withdraw_confirmation.html" + def get_context_data(self, **kwargs): + """Extend get_context_data to add has_profile_feature_flag to context""" + context = super().get_context_data(**kwargs) + # This is a django waffle flag which toggles features based off of the "flag" table + context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") + return context + class DomainRequestWithdrawn(DomainRequestPermissionWithdrawView): # this view renders no template diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py new file mode 100644 index 000000000..b386e6a62 --- /dev/null +++ b/src/registrar/views/user_profile.py @@ -0,0 +1,77 @@ +"""Views for a User Profile. + +""" + +import logging + +from django.contrib import messages +from django.views.generic.edit import FormMixin +from registrar.forms.user_profile import UserProfileForm +from django.urls import reverse +from registrar.models import ( + Contact, +) +from registrar.views.utility.permission_views import UserProfilePermissionView +from waffle.decorators import flag_is_active, waffle_flag + + +logger = logging.getLogger(__name__) + + +class UserProfileView(UserProfilePermissionView, FormMixin): + """ + Base View for the User Profile. Handles getting and setting the User Profile + """ + + model = Contact + template_name = "profile.html" + form_class = UserProfileForm + + def get(self, request, *args, **kwargs): + """Handle get requests by getting user's contact object and setting object + and form to context before rendering.""" + self.object = self.get_object() + form = self.form_class(instance=self.object) + context = self.get_context_data(object=self.object, form=form) + return self.render_to_response(context) + + @waffle_flag("profile_feature") # type: ignore + def dispatch(self, request, *args, **kwargs): # type: ignore + return super().dispatch(request, *args, **kwargs) + + def get_context_data(self, **kwargs): + """Extend get_context_data to include has_profile_feature_flag""" + context = super().get_context_data(**kwargs) + # This is a django waffle flag which toggles features based off of the "flag" table + context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature") + return context + + def get_success_url(self): + """Redirect to the user's profile page.""" + return reverse("user-profile") + + def post(self, request, *args, **kwargs): + """Handle post requests (form submissions)""" + self.object = self.get_object() + form = self.form_class(request.POST, instance=self.object) + + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + + def form_valid(self, form): + """Handle successful and valid form submissions.""" + form.save() + + messages.success(self.request, "Your profile has been updated.") + + # superclass has the redirect + return super().form_valid(form) + + def get_object(self, queryset=None): + """Override get_object to return the logged-in user's contact""" + user = self.request.user # get the logged in user + if hasattr(user, "contact"): # Check if the user has a contact instance + return user.contact + return None diff --git a/src/registrar/views/utility/error_views.py b/src/registrar/views/utility/error_views.py index 48ae628a4..2374277d5 100644 --- a/src/registrar/views/utility/error_views.py +++ b/src/registrar/views/utility/error_views.py @@ -14,19 +14,28 @@ Rather than dealing with that, we keep everything centralized in one location. """ from django.shortcuts import render +from waffle.decorators import flag_is_active def custom_500_error_view(request, context=None): """Used to redirect 500 errors to a custom view""" if context is None: - return render(request, "500.html", status=500) - else: - return render(request, "500.html", context=context, status=500) + context = {} + context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") + return render(request, "500.html", context=context, status=500) def custom_401_error_view(request, context=None): """Used to redirect 401 errors to a custom view""" if context is None: - return render(request, "401.html", status=401) - else: - return render(request, "401.html", context=context, status=401) + context = {} + context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") + return render(request, "401.html", context=context, status=401) + + +def custom_403_error_view(request, exception=None, context=None): + """Used to redirect 403 errors to a custom view""" + if context is None: + context = {} + context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") + return render(request, "403.html", context=context, status=403) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index c7083ce48..78bdf0fd7 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -382,3 +382,18 @@ class DomainInvitationPermission(PermissionsLoginMixin): return False return True + + +class UserProfilePermission(PermissionsLoginMixin): + """Permission mixin that redirects to user profile if user + has access, otherwise 403""" + + def has_permission(self): + """Check if this user has access. + + If the user is authenticated, they have access + """ + if not self.request.user.is_authenticated: + return False + + return True diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index f2752c3b5..d35647af2 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -4,6 +4,7 @@ import abc # abstract base class from django.views.generic import DetailView, DeleteView, TemplateView from registrar.models import Domain, DomainRequest, DomainInvitation +from registrar.models.contact import Contact from registrar.models.user_domain_role import UserDomainRole from .mixins import ( @@ -13,6 +14,7 @@ from .mixins import ( DomainInvitationPermission, DomainRequestWizardPermission, UserDeleteDomainRolePermission, + UserProfilePermission, ) import logging @@ -142,3 +144,22 @@ class UserDomainRolePermissionDeleteView(UserDeleteDomainRolePermission, DeleteV # variable name in template context for the model object context_object_name = "userdomainrole" + + +class UserProfilePermissionView(UserProfilePermission, DetailView, abc.ABC): + """Abstract base view for user profile view that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + # DetailView property for what model this is viewing + model = Contact + # variable name in template context for the model object + context_object_name = "contact" + + # Abstract property enforces NotImplementedError on an attribute. + @property + @abc.abstractmethod + def template_name(self): + raise NotImplementedError