From 0bf0e9dcb292c2f4aad6c0d79ec59a73b8f9f4eb Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 9 Jan 2020 10:36:27 -0500 Subject: [PATCH 01/60] Remove unnecessary if statement that was preventing TO total value from being displayed. --- templates/task_orders/fragments/task_order_view.html | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/templates/task_orders/fragments/task_order_view.html b/templates/task_orders/fragments/task_order_view.html index cfa2fcb3..31e95519 100644 --- a/templates/task_orders/fragments/task_order_view.html +++ b/templates/task_orders/fragments/task_order_view.html @@ -23,14 +23,9 @@ Total Task Order value {{ Tooltip(("task_orders.review.tooltip.total_value" | translate), title="", classes="summary-item__header-icon") }} - {% set earliest_pop_start_date, latest_pop_end_date = portfolio.funding_duration %} - {% if earliest_pop_start_date and latest_pop_end_date %} -

- {{ contract_amount | dollars }} -

- {% else %} -

-

- {% endif %} +

+ {{ contract_amount | dollars }} +

From 34cb9daca62bccbcee52b72299d37ffb43cde6b3 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 9 Jan 2020 10:37:54 -0500 Subject: [PATCH 02/60] Only fake expended funds if the TO is active --- atst/models/task_order.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/atst/models/task_order.py b/atst/models/task_order.py index 36ba905e..789a7e3f 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -140,7 +140,10 @@ class TaskOrder(Base, mixins.TimestampsMixin): @property def invoiced_funds(self): # TODO: implement this using reporting data from the CSP - return self.total_obligated_funds * Decimal(0.75) + if self.is_active: + return self.total_obligated_funds * Decimal(0.75) + else: + return 0 @property def display_status(self): From ca409cae6c6f1614802c6f2596b0436d7eb35bb7 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 9 Jan 2020 10:47:42 -0500 Subject: [PATCH 03/60] Display TO number on Step 4 of TO builder --- templates/task_orders/form_header.html | 10 ++++++---- templates/task_orders/step_4.html | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/templates/task_orders/form_header.html b/templates/task_orders/form_header.html index 05f9116a..3356a8c9 100644 --- a/templates/task_orders/form_header.html +++ b/templates/task_orders/form_header.html @@ -1,4 +1,4 @@ -{% macro TOFormStepHeader(description, title=None, to_number=None) %} +{% macro TOFormStepHeader(description=None, title=None, to_number=None) %}
{% if title -%}
@@ -10,8 +10,10 @@ Task Order Number: {{ to_number }}

{% endif %} -

- {{ description }} -

+ {% if description %} +

+ {{ description }} +

+ {% endif %}
{% endmacro %} diff --git a/templates/task_orders/step_4.html b/templates/task_orders/step_4.html index c862ad94..770a40c4 100644 --- a/templates/task_orders/step_4.html +++ b/templates/task_orders/step_4.html @@ -1,6 +1,7 @@ {% extends "task_orders/builder_base.html" %} {% from "task_orders/fragments/task_order_view.html" import TaskOrderView %} +{% from 'task_orders/form_header.html' import TOFormStepHeader %} {% set action = url_for('task_orders.form_step_five_confirm_signature', task_order_id=task_order_id) %} {% set previous_button_link = url_for("task_orders.form_step_three_add_clins", task_order_id=task_order_id) %} @@ -16,5 +17,6 @@ {% endblock %} {% block to_builder_form_field %} + {{ TOFormStepHeader(to_number=task_order.number) }} {{ TaskOrderView(task_order, portfolio, builder_mode=True) }} {% endblock %} From c2ea17b8d1216d4dc1449b21c2c8f763083bf92f Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Jan 2020 06:42:35 -0500 Subject: [PATCH 04/60] Clean up unused PKI test files. Previously these files were being used to integration testing of mutual TLS authentication. They're not any longer and can be removed. --- .secrets.baseline | 11 +---------- ssl/certificate-authority/ca.crt | 25 ------------------------- ssl/certificate-authority/ca.key | 27 --------------------------- ssl/certificate-authority/ca.srl | 1 - ssl/make-certs.sh | 18 ------------------ 5 files changed, 1 insertion(+), 81 deletions(-) delete mode 100644 ssl/certificate-authority/ca.crt delete mode 100644 ssl/certificate-authority/ca.key delete mode 100644 ssl/certificate-authority/ca.srl delete mode 100755 ssl/make-certs.sh diff --git a/.secrets.baseline b/.secrets.baseline index 857d311a..75272b46 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-12-18T22:26:52Z", + "generated_at": "2020-01-09T11:35:03Z", "plugins_used": [ { "base64_limit": 4.5, @@ -111,15 +111,6 @@ "type": "Secret Keyword" } ], - "ssl/certificate-authority/ca.key": [ - { - "hashed_secret": "be4fc4886bd949b369d5e092eb87494f12e57e5b", - "is_secret": false, - "is_verified": false, - "line_number": 1, - "type": "Private Key" - } - ], "ssl/client-certs/atat.mil.key": [ { "hashed_secret": "be4fc4886bd949b369d5e092eb87494f12e57e5b", diff --git a/ssl/certificate-authority/ca.crt b/ssl/certificate-authority/ca.crt deleted file mode 100644 index 60ed3ff2..00000000 --- a/ssl/certificate-authority/ca.crt +++ /dev/null @@ -1,25 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIEJDCCAwygAwIBAgIJAK4JGo3BBGhVMA0GCSqGSIb3DQEBCwUAMGkxCzAJBgNV -BAYTAlVTMRUwEwYDVQQIEwxQZW5uc3lsdmFuaWExFTATBgNVBAcTDFBoaWxhZGVs -cGhpYTEMMAoGA1UEChMDRG9EMQwwCgYDVQQLEwNERFMxEDAOBgNVBAMTB0FUQVQg -Q0EwHhcNMTgwNjAxMTk0NjIyWhcNMzgwNTI3MTk0NjIyWjBpMQswCQYDVQQGEwJV -UzEVMBMGA1UECBMMUGVubnN5bHZhbmlhMRUwEwYDVQQHEwxQaGlsYWRlbHBoaWEx -DDAKBgNVBAoTA0RvRDEMMAoGA1UECxMDRERTMRAwDgYDVQQDEwdBVEFUIENBMIIB -IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzYU7UbstArnnVliaC/TB6Vir -kVWMnAEYMUZA1BKP8DZaNEKbzFH2+mMw7O0BY7Ph9x0hEZ1kXLr6U93xcKyUWNPo -13i5EwUUCSh2MdPfS8ZZt8DUIIKC7XzFnKyKSKQmr0Mt9dC44rryPKTBvmI60rQ8 -VZkFEgvs8FCP0M4Ar6/gtJ24ZLEtilu5dQBSlru4nPGXg07r2C2JgEZWshtMBtbH -LkOM2gtp/pkYCCG0zqeU+0s3H8IqDq0uYkONOfVeCumbg1/AtjgrZu7aOVPKyibk -aI6sTTooXE5aSZkfkx0z6+fKM2nPSe30HgiBODtb7G+44ln08d0isjpQ67OvGQID -AQABo4HOMIHLMB0GA1UdDgQWBBSl7CUAWPbx8XqotKKKAufPh0wn4DCBmwYDVR0j -BIGTMIGQgBSl7CUAWPbx8XqotKKKAufPh0wn4KFtpGswaTELMAkGA1UEBhMCVVMx -FTATBgNVBAgTDFBlbm5zeWx2YW5pYTEVMBMGA1UEBxMMUGhpbGFkZWxwaGlhMQww -CgYDVQQKEwNEb0QxDDAKBgNVBAsTA0REUzEQMA4GA1UEAxMHQVRBVCBDQYIJAK4J -Go3BBGhVMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBABguwdFk42YP -8U6Du5HQ6Is1jfc1KEOowdh0d2MCH8q0KNktqiu6kWzjH1gRjRwc07bAkAWqXPB6 -6gkRGYe/FRgi2Rn+Uo5UC5ahI4cXkE8OitCIEP3Br9fUw+vj/3Iiov0QZ6Hv81Kl -ZTZhLiZbjAg5maL/vufnUp+n15qzm67APh3/2hcgO93UlE9o9vXohWy1lHs8u12o -hPLxghSmGc9eKalEWEs61OrohpOtCHUEd1isq76WhaiXSwSUrBxgy89Z517A7ffC -BjzLo5AVo6a9ou+ONVeZk8qw6YR6X9J7axy8YuTWt+Z82WFvOF0ubkqjm72d001M -7R9zCOQ3O+g= ------END CERTIFICATE----- diff --git a/ssl/certificate-authority/ca.key b/ssl/certificate-authority/ca.key deleted file mode 100644 index 529761e9..00000000 --- a/ssl/certificate-authority/ca.key +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -MIIEpAIBAAKCAQEAzYU7UbstArnnVliaC/TB6VirkVWMnAEYMUZA1BKP8DZaNEKb -zFH2+mMw7O0BY7Ph9x0hEZ1kXLr6U93xcKyUWNPo13i5EwUUCSh2MdPfS8ZZt8DU -IIKC7XzFnKyKSKQmr0Mt9dC44rryPKTBvmI60rQ8VZkFEgvs8FCP0M4Ar6/gtJ24 -ZLEtilu5dQBSlru4nPGXg07r2C2JgEZWshtMBtbHLkOM2gtp/pkYCCG0zqeU+0s3 -H8IqDq0uYkONOfVeCumbg1/AtjgrZu7aOVPKyibkaI6sTTooXE5aSZkfkx0z6+fK -M2nPSe30HgiBODtb7G+44ln08d0isjpQ67OvGQIDAQABAoIBAHR4EInc3UEyQVu5 -knM8Hbgzu+b86FZweFlUSuDkNBYZdz0ukkRUHvb+x3c9SRBLnL8CDv+AhqPWgo6M -tIr6Aofkb4vMqnWQ5y3ZdEIApAa5PZbY/F4AGFql3wdO8H8CJ7ojBCTOSDiVYTnk -1Lcjy9okshyAP1Ne1sPJo/bdB56HtXs+wqok1NntIQwiXjjD9xUuc1EZk0J4M97L -vBUjUGNX942UjtRiey5zwhRp3bTPasTduHcA01NaIbOVYlRFwc2W+cflz0l6ml2p -14TNEEvIMMMCNKnlPrpGI23n0psAvE4nbuxZQGVYAFvXrWn+Gyvz0Yag2EoMUCEs -ziLED9ECgYEA6IByu+xqIuIAhj/PwIIxV4+lkuV4TXIlfAFLR4JuokOVfbRsmu2e -9EfeOUD9LfQ4KsG5mu4Abpja0k/VKRKRGRjV6Oe2C6VK942HFP6Kpn0hgIuomZkD -eVv8naDezZjAvVace38zjRWB2GXTpapwBAgf/YflPPsDZ8bi/weqZCMCgYEA4kqx -Ka489Rr7+cSXpMeS5lLufhlaE5OVQc5HVFREDAI5vXU8BM2sLiHTC/BHjis2JvLm -aRJ0UsxUoIUURl2KjTbx3zns4HDVkzBrSpoDXWxBjAo0oEg7JVc+6+qEqbDHHS1L -/UJ6mlUegsE42MkFWG3YJQuHxyLZqPXIwNAyhZMCgYEA5cxnGnSt5rJoAEi7xzMn -H7s71Hf3stw6TlldFV3GiZyw+aDFo09vR1RtQTuJwczbYu88yvOn+6gax7neHo1a -WmrgqiWzGcmS0iDRPZ/kXG/bGBlxV/cTpvSTNx0UejMbdUhQvANaaXyzbLYgPWK6 -+lEphUW2/tG+aOj73UOvVu8CgYA5L8sJz4CUKJeZDTeNauoSzs56i4mZ/OfxU2Hv -S8ROjJlu6ZubUya6Gc4t7DEJGp56xVO5JfLDoeOZFUiEZ8tF2KbTVN4p8hnnMotK -tRU4nM0LyOB3yQk5bIz4LbIM+CG5m+LiQ9Sb//rP7GijUFnLeSbwZbOQfZwn+MUd -BQBfhQKBgQDmuX8tJdPkjE133IhQhZHbHHt6AEQA3aXkFdvPvbYD9VbGTZ8wnpFO -VJrDDWnIKAgO2FerIX9oq+H9a5fggYtTMeAX1cOA6b9SnLmFjt0utxrQKxf7p5I+ -n+EsmcAWfb+KRQwoB0L/mE9Ool14AeJ15kHyNIrCrMPv0J4zoC0Jdg== ------END RSA PRIVATE KEY----- diff --git a/ssl/certificate-authority/ca.srl b/ssl/certificate-authority/ca.srl deleted file mode 100644 index a23185d3..00000000 --- a/ssl/certificate-authority/ca.srl +++ /dev/null @@ -1 +0,0 @@ -F4D74F1607DD3C83 diff --git a/ssl/make-certs.sh b/ssl/make-certs.sh deleted file mode 100755 index 8f1afd19..00000000 --- a/ssl/make-certs.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/bash -# Generate the root (GIVE IT A PASSWORD IF YOU'RE NOT AUTOMATING SIGNING!): -echo 'MAKING CA' -openssl genrsa -out certificate-authority/ca.key 2048 -openssl req -new -x509 -days 7300 -key certificate-authority/ca.key -sha256 -extensions v3_ca -out certificate-authority/ca.crt - -# Generate the domain key: -openssl genrsa -out server-certs/dev.cac.atat.codes.key 2048 - -echo 'MAKING CSR' -# Generate the certificate signing request -openssl req -nodes -sha256 -new -key server-certs/dev.cac.atat.codes.key -out server-certs/dev.cac.atat.codes.csr -reqexts SAN -config <(cat req.cnf <(printf "[SAN]\nsubjectAltName=DNS.1:dev.cac.atat.codes,DNS.2:cac.atat.codes,DNS.3:backend")) - -# Sign the request with your root key -openssl x509 -sha256 -req -in server-certs/dev.cac.atat.codes.csr -CA certificate-authority/ca.crt -CAkey certificate-authority/ca.key -CAcreateserial -out server-certs/dev.cac.atat.codes.crt -days 7300 -extfile <(cat req.cnf <(printf "[SAN]\nsubjectAltName=DNS.1:dev.cac.atat.codes,DNS.2:cac.atat.codes,DNS.3:backend")) -extensions SAN - -# Check your homework: -openssl verify -CAfile certificate-authority/ca.crt server-certs/dev.cac.atat.codes.crt From a0b96402f2daa5f3bde14b62f04f9d543f76916e Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Jan 2020 10:05:10 -0500 Subject: [PATCH 05/60] Remove user.provisional column. This is leftover from a previous iteration of ATAT where inviting a user to a portfolio would create a pending entry in the users table. This is no longer used. --- .../5d7198d34b91_remove_users_provisional.py | 28 +++++++++++++++++++ atst/domain/users.py | 9 ------ atst/models/user.py | 4 +-- atst/routes/__init__.py | 3 -- 4 files changed, 29 insertions(+), 15 deletions(-) create mode 100644 alembic/versions/5d7198d34b91_remove_users_provisional.py diff --git a/alembic/versions/5d7198d34b91_remove_users_provisional.py b/alembic/versions/5d7198d34b91_remove_users_provisional.py new file mode 100644 index 00000000..aeb80bdc --- /dev/null +++ b/alembic/versions/5d7198d34b91_remove_users_provisional.py @@ -0,0 +1,28 @@ +"""remove users.provisional + +Revision ID: 5d7198d34b91 +Revises: 02ac8bdcf16f +Create Date: 2020-01-09 08:42:34.512191 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '5d7198d34b91' # pragma: allowlist secret +down_revision = '02ac8bdcf16f' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'provisional') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('provisional', sa.BOOLEAN(), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/atst/domain/users.py b/atst/domain/users.py index 1db198cf..5e09ce22 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -117,12 +117,3 @@ class Users(object): user.last_session_id = session_id db.session.add(user) db.session.commit() - - @classmethod - def finalize(cls, user): - user.provisional = False - - db.session.add(user) - db.session.commit() - - return user diff --git a/atst/models/user.py b/atst/models/user.py index 29b377d6..a45760d0 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -1,4 +1,4 @@ -from sqlalchemy import String, ForeignKey, Column, Date, Boolean, Table, TIMESTAMP +from sqlalchemy import String, ForeignKey, Column, Date, Table, TIMESTAMP from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.event import listen @@ -67,8 +67,6 @@ class User( last_login = Column(TIMESTAMP(timezone=True), nullable=True) last_session_id = Column(UUID(as_uuid=True), nullable=True) - provisional = Column(Boolean) - cloud_id = Column(String) REQUIRED_FIELDS = [ diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 9665ef9b..46ce2e2c 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -98,9 +98,6 @@ def login_redirect(): auth_context.authenticate() user = auth_context.get_user() - if user.provisional: - Users.finalize(user) - current_user_setup(user) return redirect(redirect_after_login_url()) From 1ab0c2636524b141af0e1184fa50785f78729918 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Jan 2020 15:45:47 -0500 Subject: [PATCH 06/60] Log details about user login and logout. To satisfy security requirements, we need to explicitly track: - when a user attempts to log in, successful or not - when a user logs out - whether or not the user associated with a request is logged in The first two are satisfied by extra log statements and the last is a new boolean field on the JSON logs. --- atst/domain/auth.py | 4 +++- atst/routes/__init__.py | 23 ++++++++++++++++++----- atst/utils/logging.py | 13 ++++++++++--- tests/test_auth.py | 37 +++++++++++++++---------------------- tests/utils/test_logging.py | 2 ++ 5 files changed, 48 insertions(+), 31 deletions(-) diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 0f99af09..73bb1db1 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -1,4 +1,4 @@ -from flask import g, redirect, url_for, session, request +from flask import g, redirect, url_for, session, request, current_app as app from atst.domain.users import Users @@ -59,8 +59,10 @@ def get_last_login(): def logout(): if session.get("user_id"): # pragma: no branch + dod_id = g.current_user.dod_id del session["user_id"] del session["last_login"] + app.logger.info(f"user with EDIPI {dod_id} has logged out") def _unprotected_route(request): diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 46ce2e2c..4e366a08 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -19,6 +19,7 @@ from werkzeug.exceptions import NotFound from atst.domain.users import Users from atst.domain.authnid import AuthenticationContext from atst.domain.auth import logout as _logout +from atst.domain.exceptions import UnauthenticatedError from atst.utils.flash import formatted_flash as flash @@ -64,11 +65,15 @@ def catch_all(path): raise NotFound() +def _client_s_dn(): + return request.environ.get("HTTP_X_SSL_CLIENT_S_DN") + + def _make_authentication_context(): return AuthenticationContext( crl_cache=app.crl_cache, auth_status=request.environ.get("HTTP_X_SSL_CLIENT_VERIFY"), - sdn=request.environ.get("HTTP_X_SSL_CLIENT_S_DN"), + sdn=_client_s_dn(), cert=request.environ.get("HTTP_X_SSL_CLIENT_CERT"), ) @@ -89,16 +94,24 @@ def current_user_setup(user): session["user_id"] = user.id session["last_login"] = user.last_login app.session_limiter.on_login(user) + app.logger.info(f"authentication succeeded for user with EDIPI {user.dod_id}") Users.update_last_login(user) @bp.route("/login-redirect") def login_redirect(): - auth_context = _make_authentication_context() - auth_context.authenticate() - user = auth_context.get_user() + try: + auth_context = _make_authentication_context() + auth_context.authenticate() + + user = auth_context.get_user() + current_user_setup(user) + except UnauthenticatedError as err: + app.logger.info( + f"authentication failed for subject distinguished name {_client_s_dn()}" + ) + raise err - current_user_setup(user) return redirect(redirect_after_login_url()) diff --git a/atst/utils/logging.py b/atst/utils/logging.py index 94e7aab6..f0b16dff 100644 --- a/atst/utils/logging.py +++ b/atst/utils/logging.py @@ -2,16 +2,22 @@ import datetime import json import logging -from flask import g, request, has_request_context +from flask import g, request, has_request_context, session class RequestContextFilter(logging.Filter): def filter(self, record): if has_request_context(): if getattr(g, "current_user", None): - record.user_id = str(g.current_user.id) record.dod_edipi = g.current_user.dod_id + user_id = session.get("user_id") + if user_id: + record.user_id = str(user_id) + record.logged_in = True + else: + record.logged_in = False + if request.environ.get("HTTP_X_REQUEST_ID"): record.request_id = request.environ.get("HTTP_X_REQUEST_ID") @@ -30,6 +36,7 @@ class JsonFormatter(logging.Formatter): ("request_id", lambda r: r.__dict__.get("request_id")), ("user_id", lambda r: r.__dict__.get("user_id")), ("dod_edipi", lambda r: r.__dict__.get("dod_edipi")), + ("logged_in", lambda r: r.__dict__.get("logged_in")), ("severity", lambda r: r.levelname), ("tags", lambda r: r.__dict__.get("tags")), ("audit_event", lambda r: r.__dict__.get("audit_event")), @@ -44,7 +51,7 @@ class JsonFormatter(logging.Formatter): for field, func in self._DEFAULT_RECORD_FIELDS: result = func(record) - if result: + if result is not None: message_dict[field] = result if record.args: diff --git a/tests/test_auth.py b/tests/test_auth.py index 23dc46d2..f47c8750 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -15,7 +15,7 @@ from atst.domain.authnid.crl import CRLCache from .factories import UserFactory from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS -from .utils import make_crl_list +from .utils import make_crl_list, FakeLogger MOCK_USER = {"id": "438567dd-25fa-4d83-a8cc-8aa8366cb24a"} @@ -36,7 +36,7 @@ def _login(client, verify="SUCCESS", sdn=DOD_SDN, cert="", **url_query_args): ) -def test_successful_login_redirect_non_ccpo(client, monkeypatch): +def test_successful_login_redirect(client, monkeypatch, mock_logger): monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True ) @@ -51,30 +51,19 @@ def test_successful_login_redirect_non_ccpo(client, monkeypatch): assert "home" in resp.headers["Location"] assert session["user_id"] - -def test_successful_login_redirect_ccpo(client, monkeypatch): - monkeypatch.setattr( - "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True - ) - role = PermissionSets.get(PermissionSets.VIEW_AUDIT_LOG) - monkeypatch.setattr( - "atst.domain.authnid.AuthenticationContext.get_user", - lambda *args: UserFactory.create(), - ) - - resp = _login(client) - - assert resp.status_code == 302 - assert "home" in resp.headers["Location"] - assert session["user_id"] + login_msg = mock_logger.messages[-1] + assert "succeeded" in login_msg -def test_unsuccessful_login_redirect(client, monkeypatch): +def test_unsuccessful_login_redirect(client, monkeypatch, mock_logger): resp = client.get(url_for("atst.login_redirect")) assert resp.status_code == 401 assert "user_id" not in session + login_msg = mock_logger.messages[0] + assert "failed" in login_msg + # checks that all of the routes in the app are protected by auth def is_unprotected(rule): @@ -221,13 +210,13 @@ def test_creates_new_user_without_email_on_login( assert user.email == None -def test_logout(app, client, monkeypatch): +def test_logout(app, client, monkeypatch, mock_logger): + user = UserFactory.create() monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda s: True ) monkeypatch.setattr( - "atst.domain.authnid.AuthenticationContext.get_user", - lambda s: UserFactory.create(), + "atst.domain.authnid.AuthenticationContext.get_user", lambda s: user, ) # create a real session resp = _login(client) @@ -240,6 +229,10 @@ def test_logout(app, client, monkeypatch): assert resp_failure.status_code == 302 destination = urlparse(resp_failure.headers["Location"]).path assert destination == url_for("atst.root") + # verify that logout is noted in the logs + logout_msg = mock_logger.messages[-1] + assert user.dod_id in logout_msg + assert "logged out" in logout_msg def test_logging_out_creates_a_flash_message(app, client, monkeypatch): diff --git a/tests/utils/test_logging.py b/tests/utils/test_logging.py index 415c89d6..1cff9a1f 100644 --- a/tests/utils/test_logging.py +++ b/tests/utils/test_logging.py @@ -72,9 +72,11 @@ def test_request_context_filter(logger, log_stream_content, request_ctx, monkeyp user.dod_id = "5678901234" monkeypatch.setattr("atst.utils.logging.g", Mock(current_user=user)) + monkeypatch.setattr("atst.utils.logging.session", {"user_id": user_uuid}) request_ctx.request.environ["HTTP_X_REQUEST_ID"] = request_uuid logger.info("this user is doing something") log = json.loads(log_stream_content()) assert log["user_id"] == str(user_uuid) assert log["dod_edipi"] == str(user.dod_id) assert log["request_id"] == request_uuid + assert log["logged_in"] == True From ff3e585dfe90098b40f42d7e79afaaed2b0bf260 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 23 Dec 2019 16:15:10 -0500 Subject: [PATCH 07/60] Initial formatting and styling of portfolio managers table. Deleted unused css --- atst/routes/portfolios/admin.py | 78 +++++----- styles/components/_portfolio_layout.scss | 95 +----------- templates/portfolios/admin.html | 2 +- .../fragments/portfolio_members.html | 136 ++++++------------ translations.yaml | 13 ++ 5 files changed, 91 insertions(+), 233 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 1b4ac711..14ba4b65 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -17,63 +17,51 @@ from atst.utils.flash import formatted_flash as flash from atst.domain.exceptions import UnauthorizedError -def permission_str(member, edit_perm_set, view_perm_set): - if member.has_permission_set(edit_perm_set): - return edit_perm_set - else: - return view_perm_set - - -def serialize_member_form_data(member): - return { - "member_name": member.full_name, - "member_id": member.id, - "perms_app_mgmt": permission_str( - member, - PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, - PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, +def filter_perm_sets_data(member): + perm_sets_data = { + "perms_portfolio_mgmt": bool( + member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_ADMIN) ), - "perms_funding": permission_str( - member, - PermissionSets.EDIT_PORTFOLIO_FUNDING, - PermissionSets.VIEW_PORTFOLIO_FUNDING, + "perms_app_mgmt": bool( + member.has_permission_set( + PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT + ) ), - "perms_reporting": permission_str( - member, - PermissionSets.EDIT_PORTFOLIO_REPORTS, - PermissionSets.VIEW_PORTFOLIO_REPORTS, + "perms_funding": bool( + member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_FUNDING) ), - "perms_portfolio_mgmt": permission_str( - member, - PermissionSets.EDIT_PORTFOLIO_ADMIN, - PermissionSets.VIEW_PORTFOLIO_ADMIN, + "perms_reporting": bool( + member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_REPORTS) ), } + return perm_sets_data -def get_members_data(portfolio): - members = sorted( - [serialize_member_form_data(member) for member in portfolio.members], - key=lambda member: member["member_name"], - ) - for member in members: - if member["member_id"] == portfolio.owner_role.id: - ppoc = member - members.remove(member) - members.insert(0, ppoc) - return members + +def filter_members_data(members_list): + members_data = [] + for member in members_list: + members_data.append( + { + "role_id": member.id, + "user_name": member.user_name, + "permission_sets": filter_perm_sets_data(member), + # add in stuff here for forms + } + ) + + return sorted(members_data, key=lambda member: member["user_name"]) def render_admin_page(portfolio, form=None): pagination_opts = Paginator.get_pagination_opts(http_request) audit_events = AuditLog.get_portfolio_events(portfolio, pagination_opts) - members_data = get_members_data(portfolio) portfolio_form = PortfolioForm(obj=portfolio) - member_perms_form = member_forms.MembersPermissionsForm( - data={"members_permissions": members_data} - ) - + ppoc = filter_members_data([portfolio.owner_role])[0] + member_list = portfolio.members + member_list.remove(portfolio.owner_role) assign_ppoc_form = member_forms.AssignPPOCForm() + for pf_role in portfolio.roles: if pf_role.user != portfolio.owner and pf_role.is_active: assign_ppoc_form.role_id.choices += [(pf_role.id, pf_role.full_name)] @@ -87,13 +75,13 @@ def render_admin_page(portfolio, form=None): "portfolios/admin.html", form=form, portfolio_form=portfolio_form, - member_perms_form=member_perms_form, + ppoc=ppoc, + members=filter_members_data(member_list), member_form=member_forms.NewForm(), assign_ppoc_form=assign_ppoc_form, portfolio=portfolio, audit_events=audit_events, user=g.current_user, - ppoc_id=members_data[0].get("member_id"), current_member_id=current_member_id, applications_count=len(portfolio.applications), ) diff --git a/styles/components/_portfolio_layout.scss b/styles/components/_portfolio_layout.scss index d7583b80..fffc468f 100644 --- a/styles/components/_portfolio_layout.scss +++ b/styles/components/_portfolio_layout.scss @@ -5,13 +5,6 @@ } margin-left: 2 * $gap; - - .line { - box-sizing: border-box; - height: 2px; - width: 100%; - border: 1px solid $color-gray-lightest; - } } .portfolio-header { @@ -40,36 +33,6 @@ } } - &__budget { - font-size: $small-font-size; - align-items: center; - - .icon-tooltip { - margin-left: -$gap / 2; - } - - button { - margin: 0; - padding: 0; - } - - &--dollars { - font-size: $h2-font-size; - font-weight: bold; - } - - &--amount { - white-space: nowrap; - } - - &--cents { - font-size: 2rem; - margin-top: 0.75rem; - margin-left: -0.7rem; - font-weight: bold; - } - } - .links { justify-content: center; font-size: $small-font-size; @@ -109,22 +72,6 @@ } } } - - .column-left { - width: 12.5rem; - float: left; - } - - .column-right { - margin-left: -0.4rem; - } - - .unfunded { - color: $color-red; - .icon { - @include icon-color($color-red); - } - } } @mixin subheading { @@ -138,6 +85,10 @@ .portfolio-content { margin: (4 * $gap) $gap 0 $gap; + .panel { + padding-bottom: 2rem; + } + a.add-new-button { display: inherit; margin-left: auto; @@ -157,44 +108,6 @@ } } - input.usa-button.usa-button-primary { - width: 9rem; - height: 4rem; - } - - select { - padding-left: 1.2rem; - } - - .members-table-ppoc { - select::-ms-expand { - display: none; - color: $color-gray; - } - - select { - -webkit-appearance: none; - -moz-appearance: none; - appearance: none; - display: block; - width: 100%; - float: right; - margin: 5px 0px; - padding: 0px 24px; - background-image: none; - -ms-word-break: normal; - word-break: normal; - padding-right: 3rem; - padding-left: 1.2rem; - color: $color-gray; - } - - select:hover { - box-shadow: none; - color: $color-gray; - } - } - a.modal-link.icon-link { float: right; diff --git a/templates/portfolios/admin.html b/templates/portfolios/admin.html index 75d31873..95278f86 100644 --- a/templates/portfolios/admin.html +++ b/templates/portfolios/admin.html @@ -9,7 +9,7 @@ {{ StickyCTA(text="Settings") }} -
+
diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 4c29b509..074f3b9e 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -3,103 +3,47 @@ {% from "components/modal.html" import Modal %} {% from "components/alert.html" import Alert %} -
-
- {% if g.matchesPath("portfolio-members") %} - {% include "fragments/flash.html" %} - {% endif %} - -
- {{ member_perms_form.csrf_token }} +

Portfolio Managers

+
+
+
+ + + + + + + + + + + + + {% for member in members -%} + + + + + {%- endfor %} + +
NamePortfolio Permissions
{{ ppoc.user_name }} + {% for perm, value in ppoc.permission_sets.items() -%} +
+ {{ ("portfolios.admin.members.{}.{}".format(perm, value)) | translate }} +
+ {%-endfor %} +
{{ member.user_name }} + {% for perm, value in member.permission_sets.items() -%} +
+ {{ ("portfolios.admin.members.{}.{}".format(perm, value)) | translate }} +
+ {%-endfor %} +
+
+
-
-
-
-
-
{{ "portfolios.admin.portfolio_members_title" | translate }}
-
- {{ "portfolios.admin.portfolio_members_subheading" | translate }} -
-
- - - {{ Icon('info') }} - {{ "portfolios.admin.settings_info" | translate }} - -
-
- - {% if not portfolio.members %} -

{{ "portfolios.admin.no_members" | translate }}

- {% else %} - - - - - - - - - - - - - - - {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} - {% include "portfolios/fragments/members_edit.html" %} - {% elif user_can(permissions.VIEW_PORTFOLIO_USERS) %} - {% include "portfolios/fragments/members_view.html" %} - {% endif %} - - -
{{ "portfolios.members.permissions.name" | translate }}{{ "portfolios.members.permissions.app_mgmt" | translate }}{{ "portfolios.members.permissions.funding" | translate }}{{ "portfolios.members.permissions.reporting" | translate }}{{ "portfolios.members.permissions.portfolio_mgmt" | translate }}
-
- {% endif %} - -
-
- {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} - {{ SaveButton(text=('common.save' | translate), element="input", form="member-perms") }} - {% endif %} - - {% if user_can(permissions.CREATE_PORTFOLIO_USERS) %} - - {{ "portfolios.admin.add_new_member" | translate }} - {{ Icon("plus") }} - - {% endif %} -
-
- - - {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} - {% for subform in member_perms_form.members_permissions %} - {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, subform.member_id.data) %} - {% call Modal(name=modal_id, dismissable=False) %} -

{{ "portfolios.admin.alert_header" | translate }}

-
- {{ - Alert( - title="portfolios.admin.alert_title" | translate, - message="portfolios.admin.alert_message" | translate, - level="warning" - ) - }} -
-
- {{ member_perms_form.csrf_token }} - -
- {{ "common.cancel" | translate }} -
- {% endcall %} - {% endfor %} - {% endif %} -
{% if user_can(permissions.CREATE_PORTFOLIO_USERS) %} + Add Portfolio Manager {% include "portfolios/fragments/add_new_portfolio_member.html" %} {% endif %} -
+
diff --git a/translations.yaml b/translations.yaml index 5f00a464..925cc07e 100644 --- a/translations.yaml +++ b/translations.yaml @@ -317,6 +317,19 @@ portfolios: portfolio_members_title: Portfolio members settings_info: Learn more about these settings portfolio_name: Portfolio name + members: + perms_portfolio_mgmt: + 'False': View Portfolio + 'True': Edit Portfolio + perms_app_mgmt: + 'False': View Applications + 'True': Edit Applications + perms_funding: + 'False': View Funding + 'True': Edit Funding + perms_reporting: + 'False': View Reporting + 'True': Edit Reporting applications: add_application_text: Add a new application add_environment: Create an Environment From 6e50a8cc1f74de03a9bdcf60e0eaf53b76a4fcf6 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 26 Dec 2019 13:01:34 -0500 Subject: [PATCH 08/60] Generalize macro for adding new member to an application or portfolio --- .../fragments/new_member_modal_content.html | 21 +------ .../components/member_form_template.html | 62 +++++++++++++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 templates/components/member_form_template.html diff --git a/templates/applications/fragments/new_member_modal_content.html b/templates/applications/fragments/new_member_modal_content.html index 14bb00a2..c7b5e3f1 100644 --- a/templates/applications/fragments/new_member_modal_content.html +++ b/templates/applications/fragments/new_member_modal_content.html @@ -1,25 +1,7 @@ {% from "components/icon.html" import Icon %} +{% from "components/member_form_template.html" import MemberFormTemplate %} {% import "applications/fragments/member_form_fields.html" as member_fields %} -{% macro MemberFormTemplate(title=None, next_button=None, previous=True) %} -
- {% if title %}

{{ title }}

{% endif %} - - {{ caller() }} - -
- {{ next_button }} - {% if previous %} - - {% endif %} - {{ "common.cancel" | translate }} -
-{% endmacro %} - {% macro MemberStepOne(member_form) %} {% set next_button %} + {% if title %}

{{ title }}

{% endif %} + + {{ caller() }} + +
+ {{ next_button }} + {% if previous %} + + {% endif %} + {{ "common.cancel" | translate }} +
+{% endmacro %} + +{% macro BasicStep( + title=None, + form=form, + next_button_text=next_button_text, + previous=True, + modal=modal +) %} + {% set next_button %} + + {% endset %} + + {% call MemberFormTemplate(title=title, next_button=next_button, previous=previous, modal=modal) %} + {{ form }} + {% endcall %} +{% endmacro %} + +{% macro SubmitStep( + title=None, + form=form, + submit_text=submit_text, + previous=True, + modal=modal +) %} + {% set next_button %} + + {% endset %} + + {% call MemberFormTemplate(title=title, next_button=next_button, previous=previous, modal=modal) %} + {{ form }} + {% endcall %} +{% endmacro %} From 4d2a175136b826d5220f4ddaf9636e8728fed200 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 26 Dec 2019 13:07:15 -0500 Subject: [PATCH 09/60] Use generalized macro for new member form in application settings --- templates/applications/fragments/members.html | 18 +++++++--- .../fragments/new_member_modal_content.html | 33 ------------------- 2 files changed, 13 insertions(+), 38 deletions(-) delete mode 100644 templates/applications/fragments/new_member_modal_content.html diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index be312351..bac92a3a 100644 --- a/templates/applications/fragments/members.html +++ b/templates/applications/fragments/members.html @@ -1,12 +1,10 @@ -{% from "components/alert.html" import Alert %} {% from "components/icon.html" import Icon %} {% from "components/label.html" import Label %} -{% import "applications/fragments/new_member_modal_content.html" as member_steps %} +{% import "components/member_form_template.html" as member_form %} {% import "applications/fragments/member_form_fields.html" as member_fields %} {% from "components/modal.html" import Modal %} {% from "components/multi_step_modal_form.html" import MultiStepModalForm %} {% from "components/save_button.html" import SaveButton %} -{% from "components/toggle_list.html" import ToggleButton, ToggleSection %} {% macro MemberManagementTemplate( application, @@ -179,8 +177,18 @@ form=new_member_form, form_action=url_for(action_new, application_id=application.id), steps=[ - member_steps.MemberStepOne(new_member_form), - member_steps.MemberStepTwo(new_member_form, application) + member_form.BasicStep( + title="portfolios.applications.members.form.add_member"|translate, + form=member_fields.InfoFields(new_member_form.user_data), + next_button_text="portfolios.applications.members.form.next_button"|translate, + previous=False, + modal=new_member_modal_name, + ), + member_form.SubmitStep( + form=member_fields.PermsFields(form=new_member_form, new=True), + submit_text="portfolios.applications.members.form.add_member"|translate, + modal=new_member_modal_name, + ) ], ) }} {% endif %} diff --git a/templates/applications/fragments/new_member_modal_content.html b/templates/applications/fragments/new_member_modal_content.html deleted file mode 100644 index c7b5e3f1..00000000 --- a/templates/applications/fragments/new_member_modal_content.html +++ /dev/null @@ -1,33 +0,0 @@ -{% from "components/icon.html" import Icon %} -{% from "components/member_form_template.html" import MemberFormTemplate %} -{% import "applications/fragments/member_form_fields.html" as member_fields %} - -{% macro MemberStepOne(member_form) %} - {% set next_button %} - - {% endset %} - - {% call MemberFormTemplate(title="portfolios.applications.members.form.add_member"|translate, next_button=next_button, previous=False) %} - {{ member_fields.InfoFields(member_form.user_data) }} - {% endcall %} -{% endmacro %} - -{% macro MemberStepTwo(member_form, application) %} - {% set next_button %} - - {% endset %} - - {% call MemberFormTemplate(next_button=next_button) %} - {{ member_fields.PermsFields(form=member_form, new=True) }} - {% endcall %} -{% endmacro %} From 79b27738522659171114265b1d4637e6e8def5a8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 3 Jan 2020 16:03:32 -0500 Subject: [PATCH 10/60] Portfolio manager invite updates: - Update the form to use BooleanFields for the permissions and make the form more similar to the Application Members form - Use MemberFormTemplate macro in the portfolio settings template - fix tests affected by the form changes --- atst/domain/portfolios/portfolios.py | 4 +- atst/forms/portfolio_member.py | 77 +++++-------- atst/routes/portfolios/admin.py | 2 +- atst/routes/portfolios/invitations.py | 2 +- templates/applications/fragments/members.html | 1 + .../components/member_form_template.html | 3 +- .../fragments/add_new_portfolio_member.html | 104 ++++++------------ .../fragments/portfolio_members.html | 36 +++++- tests/domain/test_portfolios.py | 2 +- tests/routes/portfolios/test_invitations.py | 8 +- 10 files changed, 106 insertions(+), 133 deletions(-) diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index bb9e7aea..6b8a5c4e 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -75,10 +75,10 @@ class Portfolios(object): permission_sets = PortfolioRoles._permission_sets_for_names( member_data.get("permission_sets", []) ) - role = PortfolioRole(portfolio_id=portfolio.id, permission_sets=permission_sets) + role = PortfolioRole(portfolio=portfolio, permission_sets=permission_sets) invitation = PortfolioInvitations.create( - inviter=inviter, role=role, member_data=member_data + inviter=inviter, role=role, member_data=member_data["user_data"] ) PortfoliosQuery.add_and_commit(role) diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py index 918d1112..a017539f 100644 --- a/atst/forms/portfolio_member.py +++ b/atst/forms/portfolio_member.py @@ -1,76 +1,59 @@ from wtforms.validators import Required -from wtforms.fields import StringField, FormField, FieldList, HiddenField +from wtforms.fields import BooleanField, FormField -from atst.domain.permission_sets import PermissionSets from .forms import BaseForm from .member import NewForm as BaseNewMemberForm +from atst.domain.permission_sets import PermissionSets from atst.forms.fields import SelectField from atst.utils.localization import translate class PermissionsForm(BaseForm): - member_name = StringField() - member_id = HiddenField() - perms_app_mgmt = SelectField( + perms_app_mgmt = BooleanField( translate("forms.new_member.app_mgmt"), - choices=[ - ( - PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, - translate("common.view"), - ), - ( - PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, - translate("common.edit"), - ), - ], + default=False, + description="Add, remove and edit applications in this Portfolio.", ) - perms_funding = SelectField( + perms_funding = BooleanField( translate("forms.new_member.funding"), - choices=[ - (PermissionSets.VIEW_PORTFOLIO_FUNDING, translate("common.view")), - (PermissionSets.EDIT_PORTFOLIO_FUNDING, translate("common.edit")), - ], + default=False, + description="Add and Modify Task Orders to fund this Portfolio.", ) - perms_reporting = SelectField( + perms_reporting = BooleanField( translate("forms.new_member.reporting"), - choices=[ - (PermissionSets.VIEW_PORTFOLIO_REPORTS, translate("common.view")), - (PermissionSets.EDIT_PORTFOLIO_REPORTS, translate("common.edit")), - ], + default=False, + description="View and export reports about this Portfolio's funding.", ) - perms_portfolio_mgmt = SelectField( + perms_portfolio_mgmt = BooleanField( translate("forms.new_member.portfolio_mgmt"), - choices=[ - (PermissionSets.VIEW_PORTFOLIO_ADMIN, translate("common.view")), - (PermissionSets.EDIT_PORTFOLIO_ADMIN, translate("common.edit")), - ], + default=False, + description="Edit this Portfolio's settings.", ) @property def data(self): _data = super().data - _data["permission_sets"] = [] - for field in _data: - if "perms" in field: - _data["permission_sets"].append(_data[field]) + _data.pop("csrf_token", None) + perm_sets = [] + if _data["perms_app_mgmt"]: + perm_sets.append(PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT) + + if _data["perms_funding"]: + perm_sets.append(PermissionSets.EDIT_PORTFOLIO_FUNDING) + + if _data["perms_reporting"]: + perm_sets.append(PermissionSets.EDIT_PORTFOLIO_REPORTS) + + if _data["perms_portfolio_mgmt"]: + perm_sets.append(PermissionSets.EDIT_PORTFOLIO_ADMIN) + + _data["permission_sets"] = perm_sets return _data -class MembersPermissionsForm(BaseForm): - members_permissions = FieldList(FormField(PermissionsForm)) - - -class NewForm(BaseForm): +class NewForm(PermissionsForm): user_data = FormField(BaseNewMemberForm) - permission_sets = FormField(PermissionsForm) - - @property - def update_data(self): - return { - "permission_sets": self.data.get("permission_sets").get("permission_sets"), - **self.data.get("user_data"), - } class AssignPPOCForm(PermissionsForm): diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 14ba4b65..19666907 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -77,7 +77,7 @@ def render_admin_page(portfolio, form=None): portfolio_form=portfolio_form, ppoc=ppoc, members=filter_members_data(member_list), - member_form=member_forms.NewForm(), + new_manager_form=member_forms.NewForm(), assign_ppoc_form=assign_ppoc_form, portfolio=portfolio, audit_events=audit_events, diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 8f2e4701..09a22d1f 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -79,7 +79,7 @@ def invite_member(portfolio_id): if form.validate(): try: - invite = Portfolios.invite(portfolio, g.current_user, form.update_data) + invite = Portfolios.invite(portfolio, g.current_user, form.data) send_portfolio_invitation( invitee_email=invite.email, inviter_name=g.current_user.full_name, diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index bac92a3a..c82daa60 100644 --- a/templates/applications/fragments/members.html +++ b/templates/applications/fragments/members.html @@ -185,6 +185,7 @@ modal=new_member_modal_name, ), member_form.SubmitStep( + name=new_member_modal_name, form=member_fields.PermsFields(form=new_member_form, new=True), submit_text="portfolios.applications.members.form.add_member"|translate, modal=new_member_modal_name, diff --git a/templates/components/member_form_template.html b/templates/components/member_form_template.html index f878df34..f62f90c1 100644 --- a/templates/components/member_form_template.html +++ b/templates/components/member_form_template.html @@ -41,6 +41,7 @@ {% endmacro %} {% macro SubmitStep( + name=name, title=None, form=form, submit_text=submit_text, @@ -51,7 +52,7 @@ {% endset %} diff --git a/templates/portfolios/fragments/add_new_portfolio_member.html b/templates/portfolios/fragments/add_new_portfolio_member.html index 644f5062..ced2ef6c 100644 --- a/templates/portfolios/fragments/add_new_portfolio_member.html +++ b/templates/portfolios/fragments/add_new_portfolio_member.html @@ -1,79 +1,41 @@ +{% from "components/checkbox_input.html" import CheckboxInput %} {% from "components/icon.html" import Icon %} +{% from "components/phone_input.html" import PhoneInput %} {% from "components/text_input.html" import TextInput %} -{% from "components/multi_step_modal_form.html" import MultiStepModalForm %} -{% macro SimpleOptionsInput(field) %} -
-
- -
- {{ field.label | striptags}} -
- - {{ field() }} -
-
-{% endmacro %} - -{% set step_one %} -
-

Invite new portfolio member

-
-
- {{ TextInput(member_form.user_data.first_name, validation='requiredField', optional=False) }} -
-
- {{ TextInput(member_form.user_data.last_name, validation='requiredField', optional=False) }} -
-
-
-
- {{ TextInput(member_form.user_data.email, validation='email', optional=False) }} -
-
- {{ TextInput(member_form.user_data.phone_number, validation='usPhone') }} -
-
-
-
- {{ TextInput(member_form.user_data.dod_id, validation='dodId', optional=False) }} -
-
-
-
-
- - Cancel -
-{% endset %} -{% set step_two %} -
-

Assign member permissions

+{% macro PermsFields(form, member_role_id=None) %} +

Assign member permissions

{{ Icon('info') }} {{ "portfolios.admin.permissions_info" | translate }} - {{ SimpleOptionsInput(member_form.permission_sets.perms_app_mgmt) }} - {{ SimpleOptionsInput(member_form.permission_sets.perms_funding) }} - {{ SimpleOptionsInput(member_form.permission_sets.perms_reporting) }} - {{ SimpleOptionsInput(member_form.permission_sets.perms_portfolio_mgmt) }} -
- - Cancel +
+ {% if new %} + {% set app_mgmt = form.perms_app_mgmt.name %} + {% set funding = form.perms_funding.name %} + {% set reporting = form.perms_reporting.name %} + {% set portfolio_mgmt = form.perms_portfolio_mgmt.name %} + {% else %} + {% set app_mgmt = "perms_app_mgmt-{}".format(member_role_id) %} + {% set funding = "perms_funding-{}".format(member_role_id) %} + {% set reporting = "perms_reporting-{}".format(member_role_id) %} + {% set portfolio_mgmt = "perms_portfolio_mgmt-{}".format(member_role_id) %} + {% endif %} + + {{ CheckboxInput(form.perms_app_mgmt, classes="input__inline-fields", key=app_mgmt, id=app_mgmt, optional=True) }} + {{ CheckboxInput(form.perms_funding, classes="input__inline-fields", key=funding, id=funding, optional=True) }} + {{ CheckboxInput(form.perms_reporting, classes="input__inline-fields", key=reporting, id=reporting, optional=True) }} + {{ CheckboxInput(form.perms_portfolio_mgmt, classes="input__inline-fields", key=portfolio_mgmt, id=portfolio_mgmt, optional=True) }}
-{% endset %} -{{ MultiStepModalForm( - 'add-port-mem', - member_form, - url_for("portfolios.invite_member", portfolio_id=portfolio.id), - [step_one, step_two], - ) }} +{% endmacro %} + +{% macro InfoFields(member_form) %} + +{% endmacro %} diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 074f3b9e..5aee8fd7 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -1,7 +1,10 @@ -{% from "components/icon.html" import Icon %} -{% from 'components/save_button.html' import SaveButton %} -{% from "components/modal.html" import Modal %} {% from "components/alert.html" import Alert %} +{% from "components/icon.html" import Icon %} +{% import "components/member_form_template.html" as member_form %} +{% from "components/modal.html" import Modal %} +{% from "components/multi_step_modal_form.html" import MultiStepModalForm %} +{% from 'components/save_button.html' import SaveButton %} +{% import "portfolios/fragments/add_new_portfolio_member.html" as member_form_fields %}

Portfolio Managers

@@ -43,7 +46,30 @@ {% if user_can(permissions.CREATE_PORTFOLIO_USERS) %} - Add Portfolio Manager - {% include "portfolios/fragments/add_new_portfolio_member.html" %} + {% set new_manager_modal = "add-portfolio-manager" %} + + Add Portfolio Manager + + + {{ MultiStepModalForm( + name=new_manager_modal, + form=new_manager_form, + form_action=url_for("portfolios.invite_member", portfolio_id=portfolio.id), + steps=[ + member_form.BasicStep( + title="Add Manager", + form=member_form_fields.InfoFields(new_manager_form.user_data), + next_button_text="Next: Permissions", + previous=False, + modal=new_manager_modal_name, + ), + member_form.SubmitStep( + name=new_manager_modal, + form=member_form_fields.PermsFields(new_manager_form), + submit_text="Invite member", + modal=new_manager_modal_name, + ) + ], + ) }} {% endif %}
diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 0390f69f..828fa1ba 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -205,7 +205,7 @@ def test_invite(): inviter = UserFactory.create() member_data = UserFactory.dictionary() - invitation = Portfolios.invite(portfolio, inviter, member_data) + invitation = Portfolios.invite(portfolio, inviter, {"user_data": member_data}) assert invitation.role assert invitation.role.portfolio == portfolio diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index 2638a9af..44c72460 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -269,10 +269,10 @@ def test_existing_member_invite_resent_to_email_submitted_in_form( _DEFAULT_PERMS_FORM_DATA = { - "permission_sets-perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, - "permission_sets-perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING, - "permission_sets-perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS, - "permission_sets-perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN, + "permission_sets-perms_app_mgmt": False, + "permission_sets-perms_funding": False, + "permission_sets-perms_reporting": False, + "permission_sets-perms_portfolio_mgmt": False, } From c9d0c64c1f44bc0fe389bb57d528578a1c4c7368 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 6 Jan 2020 13:54:12 -0500 Subject: [PATCH 11/60] Fix and generalize styling for member form macro Only display permissions with 'Edit' value Delete unused files and rename MemberForm macro file --- styles/atat.scss | 1 + styles/components/_member_form.scss | 55 +++++++++++++++++ styles/sections/_application_edit.scss | 60 ------------------- .../fragments/member_form_fields.html | 2 +- templates/applications/fragments/members.html | 2 +- ...er_form_template.html => member_form.html} | 20 ++++--- .../fragments/add_new_portfolio_member.html | 4 +- .../portfolios/fragments/members_edit.html | 38 ------------ .../portfolios/fragments/members_view.html | 26 -------- .../fragments/portfolio_members.html | 6 +- 10 files changed, 75 insertions(+), 139 deletions(-) create mode 100644 styles/components/_member_form.scss rename templates/components/{member_form_template.html => member_form.html} (71%) delete mode 100644 templates/portfolios/fragments/members_edit.html delete mode 100644 templates/portfolios/fragments/members_view.html diff --git a/styles/atat.scss b/styles/atat.scss index 4c8aa263..0134dd89 100644 --- a/styles/atat.scss +++ b/styles/atat.scss @@ -38,6 +38,7 @@ @import "components/dod_login_notice.scss"; @import "components/sticky_cta.scss"; @import "components/error_page.scss"; +@import "components/member_form.scss"; @import "sections/login"; @import "sections/home"; diff --git a/styles/components/_member_form.scss b/styles/components/_member_form.scss new file mode 100644 index 00000000..c734cfd1 --- /dev/null +++ b/styles/components/_member_form.scss @@ -0,0 +1,55 @@ +.member-form { + text-align: left; + min-width: 75rem; + + input[type="checkbox"] + label::before { + margin-left: 0; + } + + .input__inline-fields { + text-align: left; + + .usa-input__choices label { + font-weight: $font-bold; + } + } + + .input__inline-fields { + padding: $gap * 2; + border: 1px solid $color-gray-lighter; + + &.checked { + border: 1px solid $color-blue; + } + + label { + font-weight: $font-bold; + } + + p.usa-input__help { + margin-bottom: 0; + padding-left: 3rem; + } + } + + .user-info { + .usa-input { + width: 45rem; + + input, + label, + .usa-input__message { + max-width: unset; + } + + label .icon-validation { + left: unset; + right: -$gap * 4; + } + + &--validation--phoneExt { + width: 18rem; + } + } + } +} diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index 8ff79b7b..8282cb64 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -23,66 +23,6 @@ } } -#modal--add-app-mem, -.form-content--app-mem { - text-align: left; - - .modal__body { - min-width: 75rem; - } - - input[type="checkbox"] + label::before { - margin-left: 0; - } - - .input__inline-fields { - text-align: left; - - .usa-input__choices label { - font-weight: $font-bold; - } - } - - .input__inline-fields { - padding: $gap * 2; - border: 1px solid $color-gray-lighter; - - &.checked { - border: 1px solid $color-blue; - } - - label { - font-weight: $font-bold; - } - - p.usa-input__help { - margin-bottom: 0; - padding-left: 3rem; - } - } - - .application-member__user-info { - .usa-input { - width: 45rem; - - input, - label, - .usa-input__message { - max-width: unset; - } - - label .icon-validation { - left: unset; - right: -$gap * 4; - } - - &--validation--phoneExt { - width: 18rem; - } - } - } -} - .environment-roles { padding: 0 ($gap * 3) ($gap * 3); diff --git a/templates/applications/fragments/member_form_fields.html b/templates/applications/fragments/member_form_fields.html index 3aa97687..707653fa 100644 --- a/templates/applications/fragments/member_form_fields.html +++ b/templates/applications/fragments/member_form_fields.html @@ -118,7 +118,7 @@ {% endmacro %} {% macro InfoFields(member_form) %} -