From 34149de04d346ef29b5227500f62c7e44130e178 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 29 Mar 2019 14:05:26 -0400 Subject: [PATCH 01/38] basic json log formatter --- atst/utils/logging.py | 38 +++++++++++++++++++++++++ tests/utils/test_logging.py | 57 +++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 atst/utils/logging.py create mode 100644 tests/utils/test_logging.py diff --git a/atst/utils/logging.py b/atst/utils/logging.py new file mode 100644 index 00000000..0f097b0b --- /dev/null +++ b/atst/utils/logging.py @@ -0,0 +1,38 @@ +import datetime +import json +import logging + + +class ContextFilter(logging.Filter): + # this should impart the request_id and user_id if available + pass + + +def epoch_to_iso8601(ts): + dt = datetime.datetime.utcfromtimestamp(ts) + return dt.replace(tzinfo=datetime.timezone.utc).isoformat() + + +class JsonFormatter(logging.Formatter): + _DEFAULT_RECORD_FIELDS = [ + ("timestamp", lambda r: epoch_to_iso8601(r.created)), + ("version", lambda r: 1), + ("request_id", lambda r: r.__dict__.get("request_id")), + ("user_id", lambda r: r.__dict__.get("user_id")), + ("severity", lambda r: r.levelname), + ("tags", lambda r: r.__dict__.get("tags")), + ("message", lambda r: r.msg), + ] + + def format(self, record): + message_dict = {} + for field, func in self._DEFAULT_RECORD_FIELDS: + message_dict[field] = func(record) + + if record.__dict__.get("exc_info") is not None: + message_dict["details"] = { + "backtrace": self.formatException(record.exc_info), + "exception": str(record.exc_info[1]), + } + + return json.dumps(message_dict) diff --git a/tests/utils/test_logging.py b/tests/utils/test_logging.py new file mode 100644 index 00000000..79a30c8c --- /dev/null +++ b/tests/utils/test_logging.py @@ -0,0 +1,57 @@ +from io import StringIO +import json +import logging + +import pytest + +from atst.utils.logging import JsonFormatter + + +@pytest.fixture +def log_stream(): + return StringIO() + + +@pytest.fixture +def log_stream_content(log_stream): + def _log_stream_content(): + log_stream.seek(0) + return log_stream.read() + + return _log_stream_content + + +@pytest.fixture +def logger(log_stream): + logger = logging.getLogger() + for handler in logger.handlers: + logger.removeHandler(handler) + + logHandler = logging.StreamHandler(log_stream) + formatter = JsonFormatter() + logHandler.setFormatter(formatter) + logger.addHandler(logHandler) + + return logger + + +def test_json_formatter(logger, log_stream_content): + logger.warning("do or do not", extra={"tags": ["wisdom", "jedi"]}) + + log = json.loads(log_stream_content()) + + assert log["tags"] == ["wisdom", "jedi"] + assert log["message"] == "do or do not" + assert log["severity"] == "WARNING" + assert log.get("details") is None + + +def test_json_formatter_for_exceptions(logger, log_stream_content): + try: + raise Exception() + except Exception: + logger.exception("you found the ventilation shaft!") + + log = json.loads(log_stream_content()) + assert log["severity"] == "ERROR" + assert log.get("details") From 5d05c146d6a084713262b73a2d397d42320bdddd Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 29 Mar 2019 15:47:44 -0400 Subject: [PATCH 02/38] context filter for adding additional data to logs --- atst/utils/logging.py | 14 +++++++++++--- tests/utils/test_logging.py | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/atst/utils/logging.py b/atst/utils/logging.py index 0f097b0b..9c165610 100644 --- a/atst/utils/logging.py +++ b/atst/utils/logging.py @@ -2,10 +2,18 @@ import datetime import json import logging +from flask import g, request -class ContextFilter(logging.Filter): - # this should impart the request_id and user_id if available - pass + +class RequestContextFilter(logging.Filter): + def filter(self, record): + if getattr(g, "current_user", None): + record.user_id = str(g.current_user.id) + + if request.environ.get("HTTP_X_REQUEST_ID"): + record.request_id = request.environ.get("HTTP_X_REQUEST_ID") + + return True def epoch_to_iso8601(ts): diff --git a/tests/utils/test_logging.py b/tests/utils/test_logging.py index 79a30c8c..cb8198d3 100644 --- a/tests/utils/test_logging.py +++ b/tests/utils/test_logging.py @@ -1,10 +1,13 @@ from io import StringIO import json import logging +from uuid import uuid4 import pytest -from atst.utils.logging import JsonFormatter +from atst.utils.logging import JsonFormatter, RequestContextFilter + +from tests.factories import UserFactory @pytest.fixture @@ -30,6 +33,8 @@ def logger(log_stream): logHandler = logging.StreamHandler(log_stream) formatter = JsonFormatter() logHandler.setFormatter(formatter) + logger.setLevel(logging.INFO) + logger.addFilter(RequestContextFilter()) logger.addHandler(logHandler) return logger @@ -55,3 +60,15 @@ def test_json_formatter_for_exceptions(logger, log_stream_content): log = json.loads(log_stream_content()) assert log["severity"] == "ERROR" assert log.get("details") + + +def test_request_context_filter(logger, log_stream_content, request_ctx): + user = UserFactory.create() + uuid = str(uuid4()) + + request_ctx.g.current_user = user + request_ctx.request.environ["HTTP_X_REQUEST_ID"] = uuid + logger.info("this user is doing something") + log = json.loads(log_stream_content()) + assert log["user_id"] == str(user.id) + assert log["request_id"] == uuid From d73b082471d679196c07255f24981823846086a6 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Apr 2019 12:34:53 -0400 Subject: [PATCH 03/38] configurable json logging for atst app --- atst/app.py | 25 +++++++++++++++++++++++++ atst/utils/logging.py | 11 ++++++----- config/base.ini | 1 + 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/atst/app.py b/atst/app.py index adf018ed..13c801f8 100644 --- a/atst/app.py +++ b/atst/app.py @@ -29,11 +29,16 @@ from atst.utils.form_cache import FormCache from atst.utils.json import CustomJSONEncoder from atst.queue import queue +from logging.config import dictConfig +from atst.utils.logging import JsonFormatter, RequestContextFilter + ENV = os.getenv("FLASK_ENV", "dev") def make_app(config): + if ENV == "prod" or config.get("LOG_JSON"): + apply_json_logger() parent_dir = Path().parent @@ -143,6 +148,7 @@ def map_config(config): "RQ_QUEUES": [config["default"]["RQ_QUEUES"]], "DISABLE_CRL_CHECK": config.getboolean("default", "DISABLE_CRL_CHECK"), "CRL_FAIL_OPEN": config.getboolean("default", "CRL_FAIL_OPEN"), + "LOG_JSON": config.getboolean("default", "LOG_JSON"), } @@ -228,3 +234,22 @@ def make_mailer(app): ) sender = app.config.get("MAIL_SENDER") app.mailer = mailer.Mailer(mailer_connection, sender) + + +def apply_json_logger(): + dictConfig( + { + "version": 1, + "formatters": {"default": {"()": lambda *a, **k: JsonFormatter()}}, + "filters": {"requests": {"()": lambda *a, **k: RequestContextFilter()}}, + "handlers": { + "wsgi": { + "class": "logging.StreamHandler", + "stream": "ext://flask.logging.wsgi_errors_stream", + "formatter": "default", + "filters": ["requests"], + } + }, + "root": {"level": "INFO", "handlers": ["wsgi"]}, + } + ) diff --git a/atst/utils/logging.py b/atst/utils/logging.py index 9c165610..1ef84d0a 100644 --- a/atst/utils/logging.py +++ b/atst/utils/logging.py @@ -2,16 +2,17 @@ import datetime import json import logging -from flask import g, request +from flask import g, request, has_request_context class RequestContextFilter(logging.Filter): def filter(self, record): - if getattr(g, "current_user", None): - record.user_id = str(g.current_user.id) + if has_request_context(): + if getattr(g, "current_user", None): + record.user_id = str(g.current_user.id) - if request.environ.get("HTTP_X_REQUEST_ID"): - record.request_id = request.environ.get("HTTP_X_REQUEST_ID") + if request.environ.get("HTTP_X_REQUEST_ID"): + record.request_id = request.environ.get("HTTP_X_REQUEST_ID") return True diff --git a/config/base.ini b/config/base.ini index 37f5df0a..7e41e672 100644 --- a/config/base.ini +++ b/config/base.ini @@ -10,6 +10,7 @@ DISABLE_CRL_CHECK = false CRL_FAIL_OPEN = false DEBUG = true ENVIRONMENT = dev +LOG_JSON = false PERMANENT_SESSION_LIFETIME = 600 PE_NUMBER_CSV_URL = http://c95e1ebb198426ee57b8-174bb05a294821bedbf46b6384fe9b1f.r31.cf5.rackcdn.com/penumbers.csv PGAPPNAME = atst From 19438ab83e541cd39ff4f8c9b849f861858c6316 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Apr 2019 16:48:39 -0400 Subject: [PATCH 04/38] small improvements in existing log statements: - log exceptions as exception so that stack trace is recorded - use tags where appropriate for JSON logs - use different log levels in CRL checker --- atst/domain/authnid/crl/__init__.py | 23 ++++++++++++++--------- atst/domain/authz/decorator.py | 10 ++++++---- atst/routes/errors.py | 2 +- tests/utils.py | 9 ++++++--- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/atst/domain/authnid/crl/__init__.py b/atst/domain/authnid/crl/__init__.py index 3451e30e..6a5addd2 100644 --- a/atst/domain/authnid/crl/__init__.py +++ b/atst/domain/authnid/crl/__init__.py @@ -2,6 +2,7 @@ import sys import os import re import hashlib +import logging from flask import current_app as app from datetime import datetime from OpenSSL import crypto, SSL @@ -30,9 +31,9 @@ class CRLInterface: def __init__(self, *args, logger=None, **kwargs): self.logger = logger - def _log_info(self, message): + def _log(self, message, level=logging.INFO): if self.logger: - self.logger.info(message) + self.logger.log(level, message, extras={"tags": ["authorization", "crl"]}) def crl_check(self, cert): raise NotImplementedError() @@ -50,7 +51,7 @@ class NoOpCRLCache(CRLInterface): def crl_check(self, cert): cn = self._get_cn(cert) - self._log_info( + self._log( "Did not perform CRL validation for certificate with Common Name '{}'".format( cn ) @@ -111,11 +112,14 @@ class CRLCache(CRLInterface): try: return crypto.load_crl(crypto.FILETYPE_ASN1, crl_file.read()) except crypto.Error: - self._log_info("Could not load CRL at location {}".format(crl_location)) + self._log( + "Could not load CRL at location {}".format(crl_location), + level=logging.WARNING, + ) def _build_store(self, issuer): store = self.store_class() - self._log_info("STORE ID: {}. Building store.".format(id(store))) + self._log("STORE ID: {}. Building store.".format(id(store))) store.set_flags(crypto.X509StoreFlags.CRL_CHECK) crl_info = self.crl_cache.get(issuer.der(), {}) issuer_name = get_common_name(issuer) @@ -132,7 +136,7 @@ class CRLCache(CRLInterface): crl = self._load_crl(crl_info["location"]) store.add_crl(crl) - self._log_info( + self._log( "STORE ID: {}. Adding CRL with issuer Common Name {}".format( id(store), issuer_name ) @@ -158,7 +162,7 @@ class CRLCache(CRLInterface): def _add_certificate_chain_to_store(self, store, issuer): ca = self.certificate_authorities.get(issuer.der()) store.add_cert(ca) - self._log_info( + self._log( "STORE ID: {}. Adding CA with subject {}".format( id(store), ca.get_subject() ) @@ -182,10 +186,11 @@ class CRLCache(CRLInterface): except crypto.X509StoreContextError as err: if err.args[0][0] == CRL_EXPIRED_ERROR_CODE: if app.config.get("CRL_FAIL_OPEN"): - self._log_info( + self._log( "Encountered expired CRL for certificate with CN {} and issuer CN {}, failing open.".format( parsed.get_subject().CN, parsed.get_issuer().CN - ) + ), + level=logging.WARNING, ) return True else: diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index e9efac3a..980ee04c 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -45,17 +45,19 @@ def user_can_access_decorator(permission, message=None, override=None): try: check_access(permission, message, override, *args, **kwargs) app.logger.info( - "[access] User {} accessed {} {}".format( + "User {} accessed {} {}".format( g.current_user.id, request.method, request.path - ) + ), + extra={"tags": ["access", "success"]}, ) return f(*args, **kwargs) except UnauthorizedError as err: app.logger.warning( - "[access] User {} denied access {} {}".format( + "User {} denied access {} {}".format( g.current_user.id, request.method, request.path - ) + ), + extra={"tags": ["access", "failure"]}, ) raise (err) diff --git a/atst/routes/errors.py b/atst/routes/errors.py index d00c7f0a..c129e08d 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -15,7 +15,7 @@ from atst.utils.flash import formatted_flash as flash def log_error(e): error_message = e.message if hasattr(e, "message") else str(e) - current_app.logger.error(error_message) + current_app.logger.exception(error_message) def handle_error(e, message="Not Found", code=404): diff --git a/tests/utils.py b/tests/utils.py index 0615aec7..636f08b9 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -20,11 +20,14 @@ class FakeLogger: def __init__(self): self.messages = [] - def info(self, msg): + def log(self, _lvl, msg, *args, **kwargs): self.messages.append(msg) - def warning(self, msg): + def info(self, msg, *args, **kwargs): self.messages.append(msg) - def error(self, msg): + def warning(self, msg, *args, **kwargs): + self.messages.append(msg) + + def error(self, msg, *args, **kwargs): self.messages.append(msg) From a152c66e617d728cf166293d4974e82e004051c6 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 28 Mar 2019 10:30:48 -0400 Subject: [PATCH 05/38] Add hidden input for passing dod id in form --- atst/forms/portfolio_member.py | 3 ++- atst/routes/portfolios/index.py | 11 +++++++++++ templates/fragments/admin/members_edit.html | 1 + templates/fragments/admin/portfolio_members.html | 15 ++++++++------- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py index 2293deed..a4ed18e3 100644 --- a/atst/forms/portfolio_member.py +++ b/atst/forms/portfolio_member.py @@ -1,6 +1,6 @@ -from wtforms.fields import StringField, FormField, FieldList from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import Required, Email, Length, Optional +from wtforms.fields import StringField, FormField, FieldList, HiddenField from atst.domain.permission_sets import PermissionSets from .forms import BaseForm @@ -11,6 +11,7 @@ from atst.utils.localization import translate class PermissionsForm(BaseForm): member = StringField() + user_id = HiddenField() perms_app_mgmt = SelectField( None, choices=[ diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 78f0ba68..9a65d318 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -34,6 +34,7 @@ def permission_str(member, edit_perm_set, view_perm_set): def serialize_member_form_data(member): return { "member": member.user.full_name, + "user_id": member.user_id, "perms_app_mgmt": permission_str( member, PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, @@ -86,6 +87,16 @@ def portfolio_admin(portfolio_id): return render_admin_page(portfolio) +@portfolios_bp.route("/portfolios//admin", methods=["POST"]) +@user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page") +def edit_portfolio_members(portfolio_id): + portfolio = Portfolios.get_for_update(portfolio_id) + member_perms_form = MembersPermissionsForm( + http_request.form + ) + return render_admin_page(portfolio) + + @portfolios_bp.route("/portfolios//edit", methods=["POST"]) @user_can(Permissions.EDIT_PORTFOLIO_NAME, message="edit portfolio") def edit_portfolio(portfolio_id): diff --git a/templates/fragments/admin/members_edit.html b/templates/fragments/admin/members_edit.html index c64dafcb..0d873622 100644 --- a/templates/fragments/admin/members_edit.html +++ b/templates/fragments/admin/members_edit.html @@ -16,5 +16,6 @@ + {{ subform.user_id() }} {% endfor %} diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 63e44680..452cdfcc 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -6,13 +6,14 @@ {% if g.matchesPath("portfolio-members") %} {% include "fragments/flash.html" %} {% endif %} -
-
-
-
{{ "portfolios.admin.portfolio_members_title" | translate }}
-
- {{ "portfolios.admin.portfolio_members_subheading" | translate }} -
+ + {{ member_perms_form.csrf_token }} + +
+
+
{{ "portfolios.admin.portfolio_members_title" | translate }}
+
+ {{ "portfolios.admin.portfolio_members_subheading" | translate }}
{{ Icon('info') }} From 766e3b6f47e482c910cc2625cbf45f59e8f19bfc Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 28 Mar 2019 13:58:38 -0400 Subject: [PATCH 06/38] Update permissions --- atst/routes/portfolios/index.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 9a65d318..6832099b 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -5,6 +5,7 @@ from flask import render_template, request as http_request, g, redirect, url_for from . import portfolios_bp from atst.domain.reports import Reports from atst.domain.portfolios import Portfolios +from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.audit_log import AuditLog from atst.domain.common import Paginator from atst.forms.portfolio import PortfolioForm @@ -91,9 +92,15 @@ def portfolio_admin(portfolio_id): @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page") def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) - member_perms_form = MembersPermissionsForm( - http_request.form - ) + member_perms_form = MembersPermissionsForm(http_request.form) + + for subform in member_perms_form.members_permissions: + new_perm_set = subform.data["permission_sets"] + user_id = subform.user_id.data + portfolio_role = PortfolioRoles.get(portfolio.id, user_id) + if portfolio_role.permission_sets != new_perm_set: + PortfolioRoles.update(portfolio_role, new_perm_set) + return render_admin_page(portfolio) From bfff2a94b8452204baf549b84b9205e40b6b10f7 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 28 Mar 2019 16:45:19 -0400 Subject: [PATCH 07/38] Add tests --- tests/routes/portfolios/test_admin.py | 74 +++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 7f141fe8..1adeaa73 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -26,3 +26,77 @@ def test_member_table_access(client, user_session): user_session(rando) view_resp = client.get(url) assert " Date: Thu, 28 Mar 2019 17:14:24 -0400 Subject: [PATCH 08/38] Success banner --- atst/routes/portfolios/index.py | 15 ++++++++++++++- atst/utils/flash.py | 7 +++++++ templates/fragments/admin/portfolio_members.html | 11 ++++++----- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 6832099b..66bcfa82 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -2,6 +2,8 @@ from datetime import date, timedelta from flask import render_template, request as http_request, g, redirect, url_for +from atst.utils.flash import formatted_flash as flash + from . import portfolios_bp from atst.domain.reports import Reports from atst.domain.portfolios import Portfolios @@ -92,7 +94,7 @@ def portfolio_admin(portfolio_id): @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page") def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) - member_perms_form = MembersPermissionsForm(http_request.form) + member_perms_form = member_forms.MembersPermissionsForm(http_request.form) for subform in member_perms_form.members_permissions: new_perm_set = subform.data["permission_sets"] @@ -101,6 +103,17 @@ def edit_portfolio_members(portfolio_id): if portfolio_role.permission_sets != new_perm_set: PortfolioRoles.update(portfolio_role, new_perm_set) + flash("update_portfolio_members", portfolio=portfolio) + + return redirect( + url_for( + "portfolios.portfolio_admin", + portfolio_id=portfolio.id, + fragment="portfolio-members", + _anchor="portfolio-members", + ) + ) + return render_admin_page(portfolio) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 69239977..11b042e8 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -21,6 +21,13 @@ MESSAGES = { """, "category": "success", }, + "update_portfolio_members": { + "title_template": "Success!", + "message_template": """ +

You have successfully updated access permissions for members of {{ portfolio.name }}.

+ """, + "category": "success", + }, "new_portfolio_member": { "title_template": "Success!", "message_template": """ diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 452cdfcc..08e856e0 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -6,7 +6,7 @@ {% if g.matchesPath("portfolio-members") %} {% include "fragments/flash.html" %} {% endif %} - + {{ member_perms_form.csrf_token }}
@@ -15,10 +15,11 @@
{{ "portfolios.admin.portfolio_members_subheading" | translate }}
- - {{ Icon('info') }} - {{ "portfolios.admin.settings_info" | translate }} - +
+ + {{ Icon('info') }} + {{ "portfolios.admin.settings_info" | translate }} +
{% if not portfolio.members %} From d70332836b7c7bdfbedde677118530408f266e68 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 29 Mar 2019 12:01:15 -0400 Subject: [PATCH 09/38] Only flash when permissions change --- atst/routes/portfolios/index.py | 6 +++++- templates/fragments/admin/portfolio_members.html | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 66bcfa82..8dc37ade 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -95,6 +95,7 @@ def portfolio_admin(portfolio_id): def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) member_perms_form = member_forms.MembersPermissionsForm(http_request.form) + has_changed = False for subform in member_perms_form.members_permissions: new_perm_set = subform.data["permission_sets"] @@ -102,8 +103,10 @@ def edit_portfolio_members(portfolio_id): portfolio_role = PortfolioRoles.get(portfolio.id, user_id) if portfolio_role.permission_sets != new_perm_set: PortfolioRoles.update(portfolio_role, new_perm_set) + has_changed = True - flash("update_portfolio_members", portfolio=portfolio) + if has_changed: + flash("update_portfolio_members", portfolio=portfolio) return redirect( url_for( @@ -111,6 +114,7 @@ def edit_portfolio_members(portfolio_id): portfolio_id=portfolio.id, fragment="portfolio-members", _anchor="portfolio-members", + has_changed=has_changed, ) ) diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 08e856e0..6739d8e2 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -3,7 +3,7 @@
- {% if g.matchesPath("portfolio-members") %} + {% if g.matchesPath("portfolio-members") and has_changed %} {% include "fragments/flash.html" %} {% endif %} From 25563cf06a72516da37ed7bbd5b64f42d9cb25bd Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 29 Mar 2019 14:30:25 -0400 Subject: [PATCH 10/38] Add helper function --- atst/routes/portfolios/index.py | 14 ++++++++++++++ tests/routes/portfolios/test_admin.py | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 8dc37ade..f34fe316 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -90,6 +90,20 @@ def portfolio_admin(portfolio_id): return render_admin_page(portfolio) +def permission_set_has_changed(old_perm_set_names, new_perm_set_names): + has_changed = False + for perm_name in new_perm_set_names: + base = perm_name[4:] + if perm_name.split("_")[0] == "edit": + if perm_name not in old_perm_set_names: + has_changed = True + elif perm_name.split("_")[0] == "view": + edit_version = "edit" + base + if edit_version in old_perm_set_names: + has_changed = True + return has_changed + + @portfolios_bp.route("/portfolios//admin", methods=["POST"]) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page") def edit_portfolio_members(portfolio_id): diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 1adeaa73..8096e62d 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -1,6 +1,7 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets +from atst.routes.portfolios.index import permission_set_has_changed from tests.factories import PortfolioFactory, PortfolioRoleFactory, UserFactory @@ -28,6 +29,32 @@ def test_member_table_access(client, user_session): assert " Date: Fri, 29 Mar 2019 14:33:31 -0400 Subject: [PATCH 11/38] Only update when permissions are different --- atst/routes/portfolios/index.py | 13 +++++++------ templates/fragments/admin/portfolio_members.html | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index f34fe316..ae870410 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -109,17 +109,19 @@ def permission_set_has_changed(old_perm_set_names, new_perm_set_names): def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) member_perms_form = member_forms.MembersPermissionsForm(http_request.form) - has_changed = False + have_any_perms_changed = False for subform in member_perms_form.members_permissions: new_perm_set = subform.data["permission_sets"] user_id = subform.user_id.data portfolio_role = PortfolioRoles.get(portfolio.id, user_id) - if portfolio_role.permission_sets != new_perm_set: - PortfolioRoles.update(portfolio_role, new_perm_set) - has_changed = True + old_perm_set = [perm.name for perm in portfolio_role.permission_sets] - if has_changed: + if permission_set_has_changed(old_perm_set, new_perm_set): + PortfolioRoles.update(portfolio_role, new_perm_set) + have_any_perms_changed = True + + if have_any_perms_changed: flash("update_portfolio_members", portfolio=portfolio) return redirect( @@ -128,7 +130,6 @@ def edit_portfolio_members(portfolio_id): portfolio_id=portfolio.id, fragment="portfolio-members", _anchor="portfolio-members", - has_changed=has_changed, ) ) diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 6739d8e2..08e856e0 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -3,7 +3,7 @@
- {% if g.matchesPath("portfolio-members") and has_changed %} + {% if g.matchesPath("portfolio-members") %} {% include "fragments/flash.html" %} {% endif %} From ba2a63bffc744f18b278c53f1631a8c0b115a8d0 Mon Sep 17 00:00:00 2001 From: dandds <38955503+dandds@users.noreply.github.com> Date: Tue, 2 Apr 2019 09:23:44 -0400 Subject: [PATCH 12/38] Update test to use get_many Co-Authored-By: montana-mil <42577527+montana-mil@users.noreply.github.com> --- tests/routes/portfolios/test_admin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 8096e62d..068be561 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -68,7 +68,9 @@ def test_update_member_permissions(client, user_session): PortfolioRoleFactory.create( user=user, portfolio=portfolio, - permission_sets=[PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_ADMIN)], + permission_sets=PermissionSets.get_many( + [PermissionSets.EDIT_PORTFOLIO_ADMIN, PermissionSets.VIEW_PORTFOLIO_ADMIN] + ), ) user_session(user) From 7109860553ecc569d552d23e3430d5fc29d63b80 Mon Sep 17 00:00:00 2001 From: rachel-dtr Date: Tue, 2 Apr 2019 10:34:13 -0400 Subject: [PATCH 13/38] Continuing to clean up capitalization --- atst/forms/portfolio_member.py | 16 +-- atst/services/invitation.py | 2 +- atst/utils/flash.py | 6 +- .../admin/add_new_portfolio_member.html | 8 +- templates/navigation/global_sidenav.html | 2 +- templates/portfolios/task_orders/index.html | 2 +- templates/portfolios/task_orders/show.html | 4 +- translations.yaml | 106 +++++++++--------- 8 files changed, 73 insertions(+), 73 deletions(-) diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py index 2293deed..bee61c09 100644 --- a/atst/forms/portfolio_member.py +++ b/atst/forms/portfolio_member.py @@ -14,29 +14,29 @@ class PermissionsForm(BaseForm): perms_app_mgmt = SelectField( None, choices=[ - (PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, "View Only"), - (PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, "View only"), + (PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, "Edit access"), ], ) perms_funding = SelectField( None, choices=[ - (PermissionSets.VIEW_PORTFOLIO_FUNDING, "View Only"), - (PermissionSets.EDIT_PORTFOLIO_FUNDING, "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_FUNDING, "View only"), + (PermissionSets.EDIT_PORTFOLIO_FUNDING, "Edit access"), ], ) perms_reporting = SelectField( None, choices=[ - (PermissionSets.VIEW_PORTFOLIO_REPORTS, "View Only"), - (PermissionSets.EDIT_PORTFOLIO_REPORTS, "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_REPORTS, "View only"), + (PermissionSets.EDIT_PORTFOLIO_REPORTS, "Edit access"), ], ) perms_portfolio_mgmt = SelectField( None, choices=[ - (PermissionSets.VIEW_PORTFOLIO_ADMIN, "View Only"), - (PermissionSets.EDIT_PORTFOLIO_ADMIN, "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_ADMIN, "View only"), + (PermissionSets.EDIT_PORTFOLIO_ADMIN, "Edit access"), ], ) diff --git a/atst/services/invitation.py b/atst/services/invitation.py index 0953d42a..b7c3c217 100644 --- a/atst/services/invitation.py +++ b/atst/services/invitation.py @@ -52,7 +52,7 @@ class Invitation: inviter, member, email, - subject="{} has invited you to a JEDI Cloud Portfolio", + subject="{} has invited you to a JEDI cloud portfolio", email_template="emails/invitation.txt", ): self.inviter = inviter diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 69239977..a6f6f5ba 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -10,7 +10,7 @@ MESSAGES = { "task_order_draft": { "title_template": translate("task_orders.form.draft_alert_title"), "message_template": """ -

Please complete your Task Order before submitting it for approval.

+

Please complete your task order before submitting it for approval.

""", "category": "warning", }, @@ -73,7 +73,7 @@ MESSAGES = { "portfolio_member_dod_id_error": { "title_template": "CAC ID Error", "message_template": """ - The member attempted to accept this invite, but their CAC ID did not match the CAC ID you specified on the invite. Please confirm that the DOD ID is accurate. + The member attempted to accept this invite, but their CAC ID did not match the CAC ID you specified on the invite. Please confirm that the DoD ID is accurate. """, "category": "error", }, @@ -127,7 +127,7 @@ MESSAGES = { "task_order_incomplete": { "title_template": "Task Order Incomplete", "message_template": """ - You must complete your Task Order form before submitting. + You must complete your task order form before submitting. """, "category": "error", }, diff --git a/templates/fragments/admin/add_new_portfolio_member.html b/templates/fragments/admin/add_new_portfolio_member.html index 1e09dce6..fdbf7b2c 100644 --- a/templates/fragments/admin/add_new_portfolio_member.html +++ b/templates/fragments/admin/add_new_portfolio_member.html @@ -17,7 +17,7 @@ {% set step_one %}
-

Invite New Portfolio Member

+

Invite new portfolio member

@@ -48,14 +48,14 @@ v-on:click="next()" v-bind:disabled="invalid" class='action-group__action usa-button' - value='Next Step'> + value='Next'> Cancel
{% endset %} {% set step_two %}
-

Assign Member Permissions

+

Assign member permissions

{{ Icon('info') }} {{ "portfolios.admin.permissions_info" | translate }} @@ -70,7 +70,7 @@ type="submit" class='action-group__action usa-button' form="add-port-mem" - value='Invite Member'> + value='Invite member'> Cancel
diff --git a/templates/navigation/global_sidenav.html b/templates/navigation/global_sidenav.html index 7844f6c2..c494622f 100644 --- a/templates/navigation/global_sidenav.html +++ b/templates/navigation/global_sidenav.html @@ -31,7 +31,7 @@
- Fund a New Portfolio + Fund a new portfolio {{ Icon("plus", classes="sidenav__link-icon icon--circle") }}
diff --git a/templates/portfolios/task_orders/index.html b/templates/portfolios/task_orders/index.html index c26bd969..1d92eca2 100644 --- a/templates/portfolios/task_orders/index.html +++ b/templates/portfolios/task_orders/index.html @@ -93,7 +93,7 @@
- Start a New Task Order + Start a new task order
{% for task_order in pending_task_orders %} diff --git a/templates/portfolios/task_orders/show.html b/templates/portfolios/task_orders/show.html index 990f3589..dc787007 100644 --- a/templates/portfolios/task_orders/show.html +++ b/templates/portfolios/task_orders/show.html @@ -27,7 +27,7 @@ {% if complete %} Completed {% else %} - Not Started + Not started {% endif %}
@@ -118,7 +118,7 @@
-
Task Order Value
+
Task order value
{{ task_order.budget | dollars }}
diff --git a/translations.yaml b/translations.yaml index c4ab5a67..261fc66c 100644 --- a/translations.yaml +++ b/translations.yaml @@ -48,7 +48,7 @@ components: footer: about_link_text: Joint Enterprise Defense Infrastructure browser_support: JEDI Cloud supported on these web browsers - jedi_help_link_text: Questions? Contact your CCPO Representative + jedi_help_link_text: Questions? Contact your CCPO representative forms: ccpo_review: comment_description: Provide instructions or notes for additional information that is necessary to approve the request here. The requestor may then re-submit the updated request or initiate contact outside of AT-AT if further discussion is required. This message will be shared with the person making the JEDI request.. @@ -146,18 +146,18 @@ forms: average_daily_traffic_label: Average Daily Traffic (Number of Requests) cloud_native_description: Are your software systems being developed cloud native? data_transfers_description: How much data is being transferred to the cloud? - dod_component_description: Identify the DoD component that is requesting access to the JEDI Cloud + dod_component_description: Identify the DoD component that is requesting access to the JEDI cloud dod_component_label: DoD Component dodid_poc_label: DoD ID - dollar_value_description: What is your total expected budget for this JEDI Cloud Request? + dollar_value_description: What is your total expected budget for this JEDI cloud Request? dollar_value_label: Total Spend email_poc_label: Email Address engineering_assessment_description: Have you completed an engineering assessment of your systems for cloud readiness? estimated_monthly_spend_description: 'Use the JEDI CSP Calculator to estimate your monthly cloud resource usage and enter the dollar amount below. Note these estimates are for initial approval only. After the request is approved, you will be asked to provide a valid task order number with specific CLIN amounts for cloud services.' estimated_monthly_spend_label: Estimated Monthly Spend - expected_completion_date_description: When do you expect to complete your migration to the JEDI Cloud? + expected_completion_date_description: When do you expect to complete your migration to the JEDI cloud? fname_poc_label: First Name - jedi_migration_description: Are you using the JEDI Cloud to migrate existing systems? + jedi_migration_description: Are you using the JEDI cloud to migrate existing systems? jedi_migration_label: JEDI Migration jedi_usage_description: Your answer will help us provide tangible examples to DoD leadership how and why commercial cloud resources are accelerating the Department's missions jedi_usage_label: JEDI Usage @@ -165,14 +165,14 @@ forms: name_description: This name serves as a reference for your initial request and the associated portfolio that will be created once this request is approved. You may edit this name later. name_label: Name Your Request name_length_validation_message: Request names must be at least 4 and not more than 100 characters - num_software_systems_description: Estimate the number of software systems that will be supported by this JEDI Cloud access request + num_software_systems_description: Estimate the number of software systems that will be supported by this JEDI cloud access request num_software_systems_label: Number of Software Systems number_user_sessions_description: How many user sessions do you expect on these systems each day? organization_providing_assistance_description: 'If you are receiving migration assistance, what is the type of organization providing assistance?' rationalization_software_systems_description: Have you completed a “rationalization” of your software systems to move to the cloud? reviewed_label: I have reviewed this data and it is correct. start_date_date_range_validation_message: Must be a date in the future. - start_date_label: When do you expect to start using the JEDI Cloud (not for billing purposes)? + start_date_label: When do you expect to start using the JEDI cloud (not for billing purposes)? technical_support_team_description: Are you working with a technical support team experienced in cloud migrations? application: description_label: Description @@ -181,13 +181,13 @@ forms: environment_names_unique_validation_message: Environment names must be unique. name_label: Name task_order: - portfolio_name_label: Organization Portfolio Name + portfolio_name_label: Portfolio name portfolio_name_description: The name of your office or organization. You can add multiple applications to your portfolio. Your task orders are used to pay for these applications and their environments. - scope_label: Cloud Project Scope + scope_label: Cloud project scope scope_description: Your team's plan for using the cloud, such as migrating an existing application or creating a prototype. - defense_component_label: Department of Defense Component + defense_component_label: Department of Defense component app_migration: - label: App Migration + label: App migration description: Do you plan to migrate one or more existing application(s) to the cloud? on_premise: Yes, migrating from an on-premise data center cloud: Yes, migrating from another cloud provider @@ -195,43 +195,43 @@ forms: none: Not planning to migrate any applications not_sure: "Not sure" native_apps: - label: Native Apps + label: Native apps description: Do you plan to develop any applications natively in the cloud? 'yes': Yes, planning to develop natively in the cloud 'no': No, not planning to develop natively in the cloud not_sure: Not sure, unsure if planning to develop natively in the cloud complexity: - label: Project Complexity + label: Project complexity description: Which of these describes how complex your team's use of the cloud will be? Select all that apply. storage: Storage - data_analytics: Data Analytics - conus: CONUS Access - oconus: OCONUS Access - tactical_edge: Tactical Edge Access + data_analytics: Data analytics + conus: CONUS access + oconus: OCONUS access + tactical_edge: Tactical edge access not_sure: Not sure other: Other complexity_other_label: Project Complexity Other dev_team: - label: Development Team + label: Development team description: Who will be completing the development work for your cloud application(s)? Select all that apply. - civilians: Government Civilians + civilians: Government civilians military: Military contractor: Contractor other: "Other (E.g. University or other partner)" dev_team_other_label: Development Team Other team_experience: - label: Team Experience - description: How much experience does your team have with development in the cloud? + label: Team experience + description: How much experience does your team have with development in the cloud? none: No previous experience planned: Researched or planned a cloud build or migration built_1: Built or migrated 1-2 applications built_3: Built or migrated 3-5 applications built_many: Built or migrated many applications, or consulted on several such projects performance_length: - label: Period of Performance length + label: Period of performance length start_date_label: Start Date end_date_label: End Date - csp_estimate_label: Upload a copy of your CSP Cost Estimate Research + csp_estimate_label: Upload a copy of your CSP cost estimate research csp_estimate_description: Upload a PDF or screenshot of your usage estimate from the calculator. file_format_not_allowed: Only PDF or PNG files can be uploaded. clin_01_label: 'CLIN 01 : Unclassified' @@ -273,7 +273,7 @@ forms: phone_number_message: Please enter a valid 5 or 10 digit phone number. is_required: This field is required. portfolio: - name_label: Portfolio Name + name_label: Portfolio name name_length_validation_message: Portfolio names can be between 4-100 characters officers: contracting_officer_invite: Invite KO to Task Order Builder @@ -287,15 +287,15 @@ fragments: date_last_training_tooltip: When was the last time you completed the IA training?
Information Assurance (IA) training is an important step in cyber awareness. save_details_button: Save Details pending_ccpo_acceptance_alert: - learn_more_link_text: Learn more about the JEDI Cloud task order and the financial verification process. + learn_more_link_text: Learn more about the JEDI cloud task order and the financial verification process. paragraph_1: The CCPO will review and respond to your request in 3 business days. You’ll be notified via email or phone. Please note if your request is for over $1M of JEDI cloud resources it will require a manual review by the CCPO. - paragraph_2: 'While your request is being reviewed, your next step is to create a task order (TO) associated with the JEDI Cloud. Please contact a Contracting Officer (KO), Contracting Officer Representative (COR), or a Financial Manager to help with this step.' + paragraph_2: 'While your request is being reviewed, your next step is to create a task order (TO) associated with the JEDI cloud. Please contact a Contracting Officer (KO), Contracting Officer Representative (COR), or a Financial Manager to help with this step.' pending_ccpo_approval_modal: paragraph_1: The CCPO will review and respond to your Financial Verification submission in 3 business days. You will be notified via email or phone. paragraph_2: Once the financial verification is approved you will be invited to create your JEDI Portfolio and set-up your applications. Click here for more details. pending_financial_verification: - learn_more_link_text: Learn more about the JEDI Cloud task order and the financial verification process. - paragraph_1: 'The next step is to create a task order associated with JEDI Cloud. Please contact a Contracting Officer (KO), Contracting Officer Representative (COR), or a Financial Manager to help with this step.' + learn_more_link_text: Learn more about the JEDI cloud task order and the financial verification process. + paragraph_1: 'The next step is to create a task order associated with JEDI cloud. Please contact a Contracting Officer (KO), Contracting Officer Representative (COR), or a Financial Manager to help with this step.' paragraph_2: 'Once the task order has been created, you will be asked to provide details about the task order in the Financial Verification step.' ko_review_message: title: Steps @@ -314,13 +314,13 @@ login: learn_more: Learn more message: 'When you are prompted to select a certificate, please select Email Certificate from the provided choices.' title: Certificate Selection - h1_title: Access the JEDI Cloud + h1_title: Access the JEDI cloud login_button: Sign in with CAC - title_tag: Sign in | JEDI Cloud + title_tag: Sign in | JEDI cloud navigation: topbar: jedi_cloud_link_text: JEDI - logout_link_title: Log out of JEDI Cloud + logout_link_title: Log out of JEDI cloud named_portfolio: 'Portfolio {portfolio}' no_other_active_portfolios: You have no other active JEDI portfolios. other_active_portfolios: Other Active Portfolios @@ -329,7 +329,7 @@ navigation: add_new_member_label: Add new member add_new_application_label: Add new application budget_report: Budget Report - activity_log: Activity Log + activity_log: Activity log members: Members applications: Applications portfolio_funding: Funding @@ -456,44 +456,44 @@ task_orders: project_title: About your project team_title: About your team market_research_title: Market research - market_research_paragraph: 'The JEDI Cloud Computing Program Office (CCPO) has completed the market research requirements for all related task orders. The Department of Defense CIO has approved this research.
View JEDI Market Research Memo' + market_research_paragraph: 'The JEDI Cloud Computing Program Office (CCPO) has completed the market research requirements for all related task orders. The Department of Defense CIO has approved this research.
View JEDI market research memo' funding: section_title: Funding - performance_period_title: Period of Performance + performance_period_title: Period of performance performance_period_description: Choose the length of time your task order will cover. performance_period_paragraph: Be aware that your funds will be lost if you don’t use them. Because of this, we strongly recommend submitting small, short-duration task orders, usually around a three month period. We’ll notify you when your period of performance is nearing its end so you can request your next set of funds with a new task order. - estimate_usage_title: Estimate Your Cloud Usage + estimate_usage_title: Estimate your cloud usage estimate_usage_description: Calculate how much your cloud usage will cost. A technical representative from your team should help you complete this calculation. These calculations will become your CLINs. estimate_usage_paragraph: This is only an estimation tool to help you make an informed evaluation of what you expect to use. While you're tied to the dollar amount you specify in your task order, you're not obligated by the resources you indicate in the calculator. - cloud_calculations_title: Cloud Usage Calculations + cloud_calculations_title: Cloud usage calculations cloud_calculations_paragraph: Enter the results of your cloud usage calculations below. - cloud_offerings_title: Cloud Offerings + cloud_offerings_title: Cloud offerings cloud_offerings_paragraph: Infrastructure as a Service (IaaS) and Platform as a Service (PaaS) offerings - support_assistance_title: Cloud Support and Assistance + support_assistance_title: Cloud support and assistance support_assistance_paragraph: Technical guidance from the cloud service provider, including architecture, configuration of IaaS and PaaS, integration, troubleshooting assistance, and other services. - total: 'Total Task Order Value:' + total: 'Total task order value:' oversight: section_title: Oversight ko_info_title: Contracting Officer (KO) Information - ko_info_paragraph: Your KO will need to approve funding for this task order by logging into the JEDI Cloud Portal, submitting the task order documents within their official system of record, and then finally providing a digital signature. It might be helpful to work with your program's Financial Manager to get your TO documents moving. + ko_info_paragraph: Your KO will need to approve funding for this task order by logging into the JEDI cloud portal, submitting the task order documents within their official system of record, and then finally providing a digital signature. It might be helpful to work with your program's Financial Manager to get your TO documents moving. skip_ko_label: "Skip for now (We'll remind you to enter one later)" dod_id_tooltip: "The DoD ID is needed to verify the identity of the indicated officer or representative." cor_info_title: Contracting Officer Representative (COR) Information - cor_info_paragraph: Your COR may assist in submitting the task order documents within their official system of record. They may also be invited to log in and manage the Task Order entry within the JEDI Cloud portal. + cor_info_paragraph: Your COR may assist in submitting the task order documents within their official system of record. They may also be invited to log in and manage the Task Order entry within the JEDI cloud portal. so_info_title: Security Officer Information so_info_paragraph: Your Security Officer will need to answer some security configuration questions in order to generate a DD-254 document, then electronically sign. review: section_title: Review Your Task Order app_info: What you're making portfolio: Portfolio - dod: DoD Component - scope: Scope (Statement of Work) + dod: DoD component + scope: Scope (statement of work) reporting: Reporting complexity: Project complexity team: Development team funding: Funding performance_period: Period of performance - usage_est_link: View Usage Estimate + usage_est_link: View usage estimate to_value: Task order value clin_1: 'CLIN #1: Unclassified Cloud' clin_2: 'CLIN #2: Classified Cloud' @@ -546,7 +546,7 @@ portfolios: index: empty: title: You have no apps yet - start_button: Start a New JEDI Portfolio + start_button: Start a new JEDI portfolio applications: add_application_text: Add a new application app_settings_text: App settings @@ -558,7 +558,7 @@ portfolios: environments_heading: Environments environments_description: Each environment created within an application is logically separated from one another for easier management and security. update_button_text: Save - create_button_text: Create Application + create_button_text: Create team_management: title: '{application_name} Team Management' subheading: Team Management @@ -566,20 +566,20 @@ portfolios: portfolio_members_title: Portfolio Members portfolio_members_subheading: These members have different levels of access to the portfolio. settings_info: Learn more about these settings - add_member: Add a New Member + add_member: Add a new member permissions_info: Learn more about these permissions - activity_log_title: Activity Log + activity_log_title: Activity log add_new_member: Add a new member members: - archive_button: Archive User + archive_button: Archive user permissions: name: Name - app_mgmt: App Mgmt + app_mgmt: App management funding: Funding reporting: Reporting - portfolio_mgmt: Portfolio Mgmt - view_only: View Only - edit_access: Edit Access + portfolio_mgmt: Portfolio management + view_only: View only + edit_access: Edit access testing: example_string: Hello World example_with_variables: 'Hello, {name}!' From c46746d43ddadd26ec897825168af30566d7b516 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 2 Apr 2019 09:35:57 -0400 Subject: [PATCH 14/38] No need to manually check for update or flash --- atst/routes/portfolios/index.py | 24 ++---------------------- tests/routes/portfolios/test_admin.py | 27 --------------------------- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index ae870410..dfee5004 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -90,39 +90,19 @@ def portfolio_admin(portfolio_id): return render_admin_page(portfolio) -def permission_set_has_changed(old_perm_set_names, new_perm_set_names): - has_changed = False - for perm_name in new_perm_set_names: - base = perm_name[4:] - if perm_name.split("_")[0] == "edit": - if perm_name not in old_perm_set_names: - has_changed = True - elif perm_name.split("_")[0] == "view": - edit_version = "edit" + base - if edit_version in old_perm_set_names: - has_changed = True - return has_changed - - @portfolios_bp.route("/portfolios//admin", methods=["POST"]) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page") def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) member_perms_form = member_forms.MembersPermissionsForm(http_request.form) - have_any_perms_changed = False for subform in member_perms_form.members_permissions: new_perm_set = subform.data["permission_sets"] user_id = subform.user_id.data portfolio_role = PortfolioRoles.get(portfolio.id, user_id) - old_perm_set = [perm.name for perm in portfolio_role.permission_sets] + PortfolioRoles.update(portfolio_role, new_perm_set) - if permission_set_has_changed(old_perm_set, new_perm_set): - PortfolioRoles.update(portfolio_role, new_perm_set) - have_any_perms_changed = True - - if have_any_perms_changed: - flash("update_portfolio_members", portfolio=portfolio) + flash("update_portfolio_members", portfolio=portfolio) return redirect( url_for( diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 068be561..4a5f2a32 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -1,7 +1,6 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets -from atst.routes.portfolios.index import permission_set_has_changed from tests.factories import PortfolioFactory, PortfolioRoleFactory, UserFactory @@ -29,32 +28,6 @@ def test_member_table_access(client, user_session): assert " Date: Tue, 2 Apr 2019 10:16:46 -0400 Subject: [PATCH 15/38] Validate the form --- atst/routes/portfolios/index.py | 31 ++++++++++++++------------- tests/routes/portfolios/test_admin.py | 28 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index dfee5004..541a8194 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -96,24 +96,25 @@ def edit_portfolio_members(portfolio_id): portfolio = Portfolios.get_for_update(portfolio_id) member_perms_form = member_forms.MembersPermissionsForm(http_request.form) - for subform in member_perms_form.members_permissions: - new_perm_set = subform.data["permission_sets"] - user_id = subform.user_id.data - portfolio_role = PortfolioRoles.get(portfolio.id, user_id) - PortfolioRoles.update(portfolio_role, new_perm_set) + if member_perms_form.validate(): + for subform in member_perms_form.members_permissions: + new_perm_set = subform.data["permission_sets"] + user_id = subform.user_id.data + portfolio_role = PortfolioRoles.get(portfolio.id, user_id) + PortfolioRoles.update(portfolio_role, new_perm_set) - flash("update_portfolio_members", portfolio=portfolio) + flash("update_portfolio_members", portfolio=portfolio) - return redirect( - url_for( - "portfolios.portfolio_admin", - portfolio_id=portfolio.id, - fragment="portfolio-members", - _anchor="portfolio-members", + return redirect( + url_for( + "portfolios.portfolio_admin", + portfolio_id=portfolio.id, + fragment="portfolio-members", + _anchor="portfolio-members", + ) ) - ) - - return render_admin_page(portfolio) + else: + return render_admin_page(portfolio) @portfolios_bp.route("/portfolios//edit", methods=["POST"]) diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 4a5f2a32..522dcb0c 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -102,3 +102,31 @@ def test_no_update_member_permissions_without_edit_access(client, user_session): assert not rando_pf_role.has_permission_set( PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT ) + + +def test_rerender_admin_page_if_member_perms_form_does_not_validate( + client, user_session +): + portfolio = PortfolioFactory.create() + user = UserFactory.create() + PortfolioRoleFactory.create( + user=user, + portfolio=portfolio, + permission_sets=[PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_ADMIN)], + ) + user_session(user) + form_data = { + "members_permissions-0-user_id": user.id, + "members_permissions-0-perms_app_mgmt": "bad input", + "members_permissions-0-perms_funding": "view_portfolio_funding", + "members_permissions-0-perms_reporting": "view_portfolio_reports", + "members_permissions-0-perms_portfolio_mgmt": "view_portfolio_admin", + } + + response = client.post( + url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id), + data=form_data, + follow_redirects=True, + ) + assert response.status_code == 200 + assert "Portfolio Administration" in response.data.decode() From 1f5763cb6ae4519570e3666be5c3538ced8fab16 Mon Sep 17 00:00:00 2001 From: rachel-dtr Date: Tue, 2 Apr 2019 10:56:10 -0400 Subject: [PATCH 16/38] Moving scope examples with rest of scope description --- templates/task_orders/new/app_info.html | 1 - translations.yaml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/templates/task_orders/new/app_info.html b/templates/task_orders/new/app_info.html index 1833ea1f..3f9c0f87 100644 --- a/templates/task_orders/new/app_info.html +++ b/templates/task_orders/new/app_info.html @@ -23,7 +23,6 @@ {% endif %} {{ TextInput(form.scope, paragraph=True) }} -

{{ "task_orders.new.app_info.sample_scope" | translate | safe }}

{% if portfolio %} diff --git a/translations.yaml b/translations.yaml index c4ab5a67..bf5e77d6 100644 --- a/translations.yaml +++ b/translations.yaml @@ -184,7 +184,7 @@ forms: portfolio_name_label: Organization Portfolio Name portfolio_name_description: The name of your office or organization. You can add multiple applications to your portfolio. Your task orders are used to pay for these applications and their environments. scope_label: Cloud Project Scope - scope_description: Your team's plan for using the cloud, such as migrating an existing application or creating a prototype. + scope_description: Your team's plan for using the cloud, such as migrating an existing application or creating a prototype.

Not sure how to describe your scope? Read some examples to get some inspiration.

defense_component_label: Department of Defense Component app_migration: label: App Migration From fe4f0e63ac8a8cacbffffb294d5a497b43d56c39 Mon Sep 17 00:00:00 2001 From: rachel-dtr Date: Tue, 2 Apr 2019 14:39:30 -0400 Subject: [PATCH 17/38] Updating the AT-AT user profile content --- atst/forms/edit_user.py | 4 +--- templates/user/edit.html | 3 +-- translations.yaml | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index ec9db68a..835db7e3 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -29,16 +29,14 @@ USER_FIELDS = { translate("forms.edit_user.service_branch_label"), choices=SERVICE_BRANCHES ), "citizenship": RadioField( - description=translate("forms.edit_user.citizenship_description"), choices=[ ("United States", "United States"), ("Foreign National", "Foreign National"), ("Other", "Other"), - ], + ] ), "designation": RadioField( translate("forms.edit_user.designation_label"), - description=translate("forms.edit_user.designation_description"), choices=[ ("military", "Military"), ("civilian", "Civilian"), diff --git a/templates/user/edit.html b/templates/user/edit.html index f1091963..fc4ac64f 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -9,8 +9,7 @@

{{ user.first_name }} {{ user.last_name }}
-
DOD ID: {{ user.dod_id }}
-
Edit user details
+
DoD ID: {{ user.dod_id }}

diff --git a/translations.yaml b/translations.yaml index c4ab5a67..427447d3 100644 --- a/translations.yaml +++ b/translations.yaml @@ -81,7 +81,7 @@ forms: website. date_latest_training_label: Latest Information Assurance (IA) training completion date designation_description: What is your designation within the DoD? - designation_label: Designation of Person + designation_label: Designation of person email_label: Email Address first_name_label: First Name last_name_label: Last Name From 73cd907d49431f3bf54d9c2fed7260b6d741f290 Mon Sep 17 00:00:00 2001 From: rachel-dtr Date: Tue, 2 Apr 2019 15:03:04 -0400 Subject: [PATCH 18/38] Updates TO view page content --- templates/portfolios/task_orders/show.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/portfolios/task_orders/show.html b/templates/portfolios/task_orders/show.html index 990f3589..742241c4 100644 --- a/templates/portfolios/task_orders/show.html +++ b/templates/portfolios/task_orders/show.html @@ -12,9 +12,9 @@ {% macro officer_name(officer) -%} {%- if not officer -%} - Not specified + not yet invited {%- elif officer == g.current_user -%} - You + you {%- else -%} {{ officer.full_name }} {%- endif -%} @@ -91,7 +91,7 @@
{{ officer_info.phone_number | usPhone }}
{% else %} - Not specified + not yet invited {% endif %}
@@ -118,7 +118,7 @@
-
Task Order Value
+
Task order value
{{ task_order.budget | dollars }}
From 3f5b2abcd5355c1ec7fb182d507b1cb179a7b0ab Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 2 Apr 2019 16:38:34 -0400 Subject: [PATCH 19/38] add uwsgi logfile plugin for additional loggers --- script/alpine_setup | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/alpine_setup b/script/alpine_setup index a06b5694..9b8613b5 100755 --- a/script/alpine_setup +++ b/script/alpine_setup @@ -10,7 +10,7 @@ APP_USER="atst" APP_UID="8010" # Add additional packages required by app dependencies -ADDITIONAL_PACKAGES="postgresql-libs python3 rsync uwsgi uwsgi-python3" +ADDITIONAL_PACKAGES="postgresql-libs python3 rsync uwsgi uwsgi-python3 uwsgi-logfile" # add sync-crl cronjob for atst user echo "1 */6 * * * /opt/atat/atst/script/sync-crls tests/crl-tmp" >> /etc/crontabs/atst From 92302b2a2245ddeef71f47045674ead027e06b03 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 2 Apr 2019 13:46:45 -0400 Subject: [PATCH 20/38] If user is MO and an officer of the TO, do not render resend invite link --- atst/routes/portfolios/task_orders.py | 1 + .../portfolios/task_orders/invitations.html | 32 ++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index 553d226d..a3c77631 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -231,6 +231,7 @@ def task_order_invitations(portfolio_id, task_order_id): portfolio=portfolio, task_order=task_order, form=form, + user=g.current_user, ) else: raise NotFoundError("task_order") diff --git a/templates/portfolios/task_orders/invitations.html b/templates/portfolios/task_orders/invitations.html index b5d46386..92955268 100644 --- a/templates/portfolios/task_orders/invitations.html +++ b/templates/portfolios/task_orders/invitations.html @@ -99,21 +99,23 @@ {{ Link("Update", "edit", onClick="edit") }} {% set invite_type = [prefix + "_invite"] %} - {{ - ConfirmationButton( - btn_text="Resend Invitation", - confirm_btn=('task_orders.invitations.resend_btn' | translate), - confirm_msg=('task_orders.invitations.resend_confirmation_message' | translate), - action=url_for( - "portfolios.resend_invite", - portfolio_id=portfolio.id, - task_order_id=task_order.id, - invite_type=invite_type, - ), - btn_icon=Icon('avatar'), - btn_class="icon-link", - ) - }} + {% if not (user == task_order.creator and user == task_order[officer_type]) %} + {{ + ConfirmationButton( + btn_text="Resend Invitation", + confirm_btn=('task_orders.invitations.resend_btn' | translate), + confirm_msg=('task_orders.invitations.resend_confirmation_message' | translate), + action=url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + invite_type=invite_type, + ), + btn_icon=Icon('avatar'), + btn_class="icon-link", + ) + }} + {% endif %} {{ Link("Remove", "trash", classes="remove") }} From dd4231760f4c88c7e7870d4ab09f5957483a0721 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 3 Apr 2019 09:43:16 -0400 Subject: [PATCH 21/38] Test that resend links show properly --- tests/routes/portfolios/test_task_orders.py | 153 ++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 48aaefd0..0d77ae1e 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -289,6 +289,159 @@ class TestTaskOrderInvitations: assert response.status_code == 404 assert time_updated == other_task_order.time_updated + def test_does_not_render_resend_invite_if_user_is_mo_and_user_is_ko( + self, client, user_session + ): + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + ko_first_name=self.portfolio.owner.first_name, + ko_last_name=self.portfolio.owner.last_name, + ko_email=self.portfolio.owner.email, + ko_phone_number=self.portfolio.owner.phone_number, + ko_dod_id=self.portfolio.owner.dod_id, + ko_invite=True, + ) + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" not in response.data.decode() + + def test_renders_resend_invite_if_user_is_mo_and_user_is_not_ko( + self, client, user_session + ): + ko = UserFactory.create() + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + contracting_officer=ko, + ko_invite=True, + ) + portfolio_role = PortfolioRoleFactory.create(portfolio=self.portfolio, user=ko) + invitation = InvitationFactory.create( + inviter=self.portfolio.owner, + portfolio_role=portfolio_role, + user=ko, + status=InvitationStatus.PENDING, + ) + + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" in response.data.decode() + + def test_does_not_render_resend_invite_if_user_is_mo_and_user_is_cor( + self, client, user_session + ): + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + cor_first_name=self.portfolio.owner.first_name, + cor_last_name=self.portfolio.owner.last_name, + cor_email=self.portfolio.owner.email, + cor_phone_number=self.portfolio.owner.phone_number, + cor_dod_id=self.portfolio.owner.dod_id, + cor_invite=True, + ) + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" not in response.data.decode() + + def test_renders_resend_invite_if_user_is_mo_and_user_is_not_cor( + self, client, user_session + ): + cor = UserFactory.create() + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + contracting_officer_representative=cor, + cor_invite=True, + ) + portfolio_role = PortfolioRoleFactory.create(portfolio=self.portfolio, user=cor) + invitation = InvitationFactory.create( + inviter=self.portfolio.owner, + portfolio_role=portfolio_role, + user=cor, + status=InvitationStatus.PENDING, + ) + + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" in response.data.decode() + + def test_does_not_render_resend_invite_if_user_is_mo_and_user_is_so( + self, client, user_session + ): + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + so_first_name=self.portfolio.owner.first_name, + so_last_name=self.portfolio.owner.last_name, + so_email=self.portfolio.owner.email, + so_phone_number=self.portfolio.owner.phone_number, + so_dod_id=self.portfolio.owner.dod_id, + so_invite=True, + ) + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" not in response.data.decode() + + def test_renders_resend_invite_if_user_is_mo_and_user_is_not_so( + self, client, user_session + ): + so = UserFactory.create() + task_order = TaskOrderFactory.create( + portfolio=self.portfolio, + creator=self.portfolio.owner, + security_officer=so, + so_invite=True, + ) + portfolio_role = PortfolioRoleFactory.create(portfolio=self.portfolio, user=so) + invitation = InvitationFactory.create( + inviter=self.portfolio.owner, + portfolio_role=portfolio_role, + user=so, + status=InvitationStatus.PENDING, + ) + + user_session(self.portfolio.owner) + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=task_order.id, + ) + ) + assert "Resend Invitation" in response.data.decode() + def test_ko_can_view_task_order(client, user_session, portfolio, user): PortfolioRoleFactory.create( From 6f1eb43de4f3b6ad33736fe52ab9cd4263e4712a Mon Sep 17 00:00:00 2001 From: George Drummond Date: Tue, 26 Mar 2019 15:45:32 -0400 Subject: [PATCH 22/38] Remove Portfolio User --- atst/domain/portfolio_roles.py | 9 ++++ atst/routes/portfolios/index.py | 28 ++++++++++ atst/utils/flash.py | 5 ++ styles/components/_portfolio_layout.scss | 4 ++ templates/fragments/admin/members_edit.html | 9 +++- .../fragments/admin/portfolio_members.html | 31 +++++++++++ tests/domain/test_portfolio_roles.py | 8 +++ .../portfolios/test_portfolios_index.py | 52 +++++++++++++++++++ 8 files changed, 145 insertions(+), 1 deletion(-) diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index 153d4707..cb123b4a 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -121,6 +121,15 @@ class PortfolioRoles(object): ) return PermissionSets.get_many(perms_set_names) + @classmethod + def disable(cls, portfolio_role): + portfolio_role.status = PortfolioRoleStatus.DISABLED + + db.session.add(portfolio_role) + db.session.commit() + + return portfolio_role + @classmethod def update(cls, portfolio_role, set_names): new_permission_sets = PortfolioRoles._permission_sets_for_names(set_names) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 541a8194..767f56aa 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -15,6 +15,7 @@ import atst.forms.portfolio_member as member_forms from atst.models.permissions import Permissions from atst.domain.permission_sets import PermissionSets from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.utils.flash import formatted_flash as flash @portfolios_bp.route("/portfolios") @@ -174,3 +175,30 @@ def portfolio_reports(portfolio_id): expiration_date=expiration_date, remaining_days=remaining_days, ) + + +@portfolios_bp.route( + "/portfolios//members//delete", methods=["POST"] +) +@user_can(Permissions.EDIT_PORTFOLIO_USERS, message="update portfolio members") +def remove_member(portfolio_id, member_id): + if member_id == str(g.current_user.id): + raise UnauthorizedError( + user=user, message="you cant remove yourself from the portfolio" + ) + + portfolio = Portfolios.get(g.current_user, portfolio_id) + portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=member_id) + + PortfolioRoles.disable(portfolio_role=portfolio_role) + + flash("portfolio_member_removed", member_name=portfolio_role.user.full_name) + + return redirect( + url_for( + "portfolios.portfolio_admin", + portfolio_id=portfolio.id, + _anchor="portfolio-members", + fragment="portfolio-members", + ) + ) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 89f81132..1fc6a740 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -138,6 +138,11 @@ MESSAGES = { """, "category": "error", }, + "portfolio_member_removed": { + "title_template": "Portfolio Member Removed", + "message_template": "You have successfully removed {{ member_name }} from the portfolio.", + "category": "success", + }, } diff --git a/styles/components/_portfolio_layout.scss b/styles/components/_portfolio_layout.scss index 539ef47b..7a33345b 100644 --- a/styles/components/_portfolio_layout.scss +++ b/styles/components/_portfolio_layout.scss @@ -291,6 +291,10 @@ height: 4rem; } + .usa-button-danger { + background: $color-red; + } + .members-table-footer { float: right; padding: 3 * $gap; diff --git a/templates/fragments/admin/members_edit.html b/templates/fragments/admin/members_edit.html index 0d873622..04d2ed21 100644 --- a/templates/fragments/admin/members_edit.html +++ b/templates/fragments/admin/members_edit.html @@ -1,3 +1,7 @@ +{% from "components/confirmation_button.html" import ConfirmationButton %} + +{% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, user.id) %} + {% for subform in member_perms_form.members_permissions %} {{ subform.member.data }} @@ -14,7 +18,10 @@ {{ OptionsInput(subform.perms_reporting, label=False) }} {{ OptionsInput(subform.perms_portfolio_mgmt, label=False) }} - + + + {{ "portfolios.members.archive_button" | translate }} + {{ subform.user_id() }} diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 08e856e0..da980c3d 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -1,6 +1,9 @@ {% from "components/icon.html" import Icon %} {% from "components/options_input.html" import OptionsInput %} +{% from "components/modal.html" import Modal %} +{% from "components/alert.html" import Alert %} +
{% if g.matchesPath("portfolio-members") %} @@ -51,6 +54,34 @@ {% endif %} + + {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} + {% for member in portfolio.members %} + {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, user.id) %} + {% call Modal(name=modal_id, dismissable=False) %} +

Are you sure you want to archive this user?

+ + {{ + Alert( + title="Warning! You are about to archive a user from the portfolio admin.", + message="User will be removed from the portfolio, but their log history will be retained.", + level="warning" + ) + }} + +
+
+ {{ member_perms_form.csrf_token }} + +
+ Cancel +
+ {% endcall %} + {% endfor %} + {% endif %} +
{% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 5bdc9bc0..fedd7faf 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -29,3 +29,11 @@ def test_add_portfolio_role_with_permission_sets(): ] actual_names = [prms.name for prms in port_role.permission_sets] assert expected_names == expected_names + + +def test_disable_portfolio_role(): + portfolio_role = PortfolioRoleFactory.create(status=PortfolioRoleStatus.ACTIVE) + assert portfolio_role.status == PortfolioRoleStatus.ACTIVE + + PortfolioRoles.disable(portfolio_role=portfolio_role) + assert portfolio_role.status == PortfolioRoleStatus.DISABLED diff --git a/tests/routes/portfolios/test_portfolios_index.py b/tests/routes/portfolios/test_portfolios_index.py index 7f631c52..d4fdeaba 100644 --- a/tests/routes/portfolios/test_portfolios_index.py +++ b/tests/routes/portfolios/test_portfolios_index.py @@ -2,6 +2,8 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets from atst.models.permissions import Permissions +from atst.domain.portfolio_roles import PortfolioRoles +from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( random_future_date, @@ -81,6 +83,56 @@ def test_portfolio_admin_screen_when_not_ppoc(client, user_session): assert translate("fragments.ppoc.update_btn").encode("utf8") not in response.data +def test_remove_portfolio_member(client, user_session): + portfolio = PortfolioFactory.create() + + user = UserFactory.create() + PortfolioRoleFactory.create(portfolio=portfolio, user=user) + + user_session(portfolio.owner) + + response = client.post( + url_for( + "portfolios.remove_member", portfolio_id=portfolio.id, member_id=user.id + ), + follow_redirects=False, + ) + + assert response.status_code == 302 + assert response.headers["Location"] == url_for( + "portfolios.portfolio_admin", + portfolio_id=portfolio.id, + _anchor="portfolio-members", + fragment="portfolio-members", + _external=True, + ) + assert ( + PortfolioRoles.get(portfolio_id=portfolio.id, user_id=user.id).status + == PortfolioRoleStatus.DISABLED + ) + + +def test_remove_portfolio_member_self(client, user_session): + portfolio = PortfolioFactory.create() + + user_session(portfolio.owner) + + response = client.post( + url_for( + "portfolios.remove_member", + portfolio_id=portfolio.id, + member_id=portfolio.owner.id, + ), + follow_redirects=False, + ) + + assert response.status_code == 500 + assert ( + PortfolioRoles.get(portfolio_id=portfolio.id, user_id=portfolio.owner.id).status + == PortfolioRoleStatus.ACTIVE + ) + + def test_portfolio_reports(client, user_session): portfolio = PortfolioFactory.create( applications=[ From 6d7400b7d2829af00ec713d1e34d8643cd1c41a2 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Fri, 29 Mar 2019 14:14:21 -0400 Subject: [PATCH 23/38] Add in correct modal close call --- templates/fragments/admin/portfolio_members.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index da980c3d..64e0a58b 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -76,7 +76,7 @@ {{ "portfolios.members.archive_button" | translate }} - Cancel + Cancel
{% endcall %} {% endfor %} From bb5b8e212264acb3e13b97fde9745d2369525202 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Fri, 29 Mar 2019 14:19:44 -0400 Subject: [PATCH 24/38] Fix incorrect modal id --- templates/fragments/admin/portfolio_members.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 64e0a58b..d9bdbc81 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -57,7 +57,7 @@ {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} {% for member in portfolio.members %} - {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, user.id) %} + {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, member.user_id) %} {% call Modal(name=modal_id, dismissable=False) %}

Are you sure you want to archive this user?

From a22b416c6a749751341696372932de4a8904ee30 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Fri, 29 Mar 2019 14:25:18 -0400 Subject: [PATCH 25/38] Open correct form id --- templates/fragments/admin/portfolio_members.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index d9bdbc81..5b4ac977 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -76,7 +76,7 @@ {{ "portfolios.members.archive_button" | translate }} - Cancel + Cancel
{% endcall %} {% endfor %} From 5cc8c05dbdd5e7a3084eec7ac735cff81f837adf Mon Sep 17 00:00:00 2001 From: George Drummond Date: Fri, 29 Mar 2019 15:24:06 -0400 Subject: [PATCH 26/38] Return correct error code --- atst/routes/portfolios/index.py | 3 ++- tests/routes/portfolios/test_portfolios_index.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 767f56aa..ecb1f361 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -16,6 +16,7 @@ from atst.models.permissions import Permissions from atst.domain.permission_sets import PermissionSets from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.utils.flash import formatted_flash as flash +from atst.domain.exceptions import UnauthorizedError @portfolios_bp.route("/portfolios") @@ -184,7 +185,7 @@ def portfolio_reports(portfolio_id): def remove_member(portfolio_id, member_id): if member_id == str(g.current_user.id): raise UnauthorizedError( - user=user, message="you cant remove yourself from the portfolio" + user=g.current_user, action="you cant remove yourself from the portfolio" ) portfolio = Portfolios.get(g.current_user, portfolio_id) diff --git a/tests/routes/portfolios/test_portfolios_index.py b/tests/routes/portfolios/test_portfolios_index.py index d4fdeaba..29672f9c 100644 --- a/tests/routes/portfolios/test_portfolios_index.py +++ b/tests/routes/portfolios/test_portfolios_index.py @@ -126,7 +126,7 @@ def test_remove_portfolio_member_self(client, user_session): follow_redirects=False, ) - assert response.status_code == 500 + assert response.status_code == 404 assert ( PortfolioRoles.get(portfolio_id=portfolio.id, user_id=portfolio.owner.id).status == PortfolioRoleStatus.ACTIVE From 358b00a6e2dbbb4b19a49217782866138f506bf4 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Mon, 1 Apr 2019 10:23:19 -0400 Subject: [PATCH 27/38] Import on one line --- tests/routes/portfolios/test_portfolios_index.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/routes/portfolios/test_portfolios_index.py b/tests/routes/portfolios/test_portfolios_index.py index 29672f9c..d7d6b4dd 100644 --- a/tests/routes/portfolios/test_portfolios_index.py +++ b/tests/routes/portfolios/test_portfolios_index.py @@ -2,8 +2,7 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets from atst.models.permissions import Permissions -from atst.domain.portfolio_roles import PortfolioRoles -from atst.models.portfolio_role import Status as PortfolioRoleStatus +from atst.domain.portfolio_roles import PortfolioRoles, Status as PortfolioRoleStatus from tests.factories import ( random_future_date, From dee14b98bea537442c4b51578e91dd96a74be637 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Mon, 1 Apr 2019 10:44:53 -0400 Subject: [PATCH 28/38] Remove portfolio permissions when role is disabled --- atst/domain/authz/__init__.py | 4 +++- tests/domain/test_authz.py | 9 +++++++++ tests/routes/portfolios/test_portfolios_index.py | 3 ++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/atst/domain/authz/__init__.py b/atst/domain/authz/__init__.py index 6e8cdfea..37e06ab8 100644 --- a/atst/domain/authz/__init__.py +++ b/atst/domain/authz/__init__.py @@ -1,6 +1,8 @@ from atst.utils import first_or_none from atst.models.permissions import Permissions from atst.domain.exceptions import UnauthorizedError +from atst.domain.portfolio_roles import PortfolioRoles +from atst.models.portfolio_role import Status as PortfolioRoleStatus class Authorization(object): @@ -9,7 +11,7 @@ class Authorization(object): port_role = first_or_none( lambda pr: pr.portfolio == portfolio, user.portfolio_roles ) - if port_role: + if port_role and port_role.status is not PortfolioRoleStatus.DISABLED: return permission in port_role.permissions else: return False diff --git a/tests/domain/test_authz.py b/tests/domain/test_authz.py index bbd6ecba..37749533 100644 --- a/tests/domain/test_authz.py +++ b/tests/domain/test_authz.py @@ -11,6 +11,7 @@ from atst.domain.authz.decorator import user_can_access_decorator from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import UnauthorizedError from atst.models.permissions import Permissions +from atst.domain.portfolio_roles import PortfolioRoles from tests.utils import FakeLogger @@ -101,6 +102,14 @@ def test_user_can_access(): view_admin, Permissions.EDIT_PORTFOLIO_NAME, portfolio=portfolio ) + # check when portfolio_role is disabled + view_admin_pr = PortfolioRoles.get(portfolio_id=portfolio.id, user_id=view_admin.id) + PortfolioRoles.disable(portfolio_role=view_admin_pr) + with pytest.raises(UnauthorizedError): + user_can_access( + view_admin, Permissions.EDIT_PORTFOLIO_NAME, portfolio=portfolio + ) + @pytest.fixture def set_current_user(request_ctx): diff --git a/tests/routes/portfolios/test_portfolios_index.py b/tests/routes/portfolios/test_portfolios_index.py index d7d6b4dd..29672f9c 100644 --- a/tests/routes/portfolios/test_portfolios_index.py +++ b/tests/routes/portfolios/test_portfolios_index.py @@ -2,7 +2,8 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets from atst.models.permissions import Permissions -from atst.domain.portfolio_roles import PortfolioRoles, Status as PortfolioRoleStatus +from atst.domain.portfolio_roles import PortfolioRoles +from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( random_future_date, From 933d90b203ef31178d3914abd4d27701fd1b0b0f Mon Sep 17 00:00:00 2001 From: George Drummond Date: Mon, 1 Apr 2019 13:32:11 -0400 Subject: [PATCH 29/38] Save PR and don't do an extra lookup --- tests/domain/test_authz.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/domain/test_authz.py b/tests/domain/test_authz.py index 37749533..bfebe562 100644 --- a/tests/domain/test_authz.py +++ b/tests/domain/test_authz.py @@ -76,7 +76,7 @@ def test_user_can_access(): portfolio = PortfolioFactory.create(owner=edit_admin) # factory gives view perms by default - PortfolioRoleFactory.create(user=view_admin, portfolio=portfolio) + view_admin_pr = PortfolioRoleFactory.create(user=view_admin, portfolio=portfolio) # check a site-wide permission assert user_can_access(ccpo, Permissions.VIEW_AUDIT_LOG) @@ -103,7 +103,6 @@ def test_user_can_access(): ) # check when portfolio_role is disabled - view_admin_pr = PortfolioRoles.get(portfolio_id=portfolio.id, user_id=view_admin.id) PortfolioRoles.disable(portfolio_role=view_admin_pr) with pytest.raises(UnauthorizedError): user_can_access( From 270b8d0db6c2a2dc8459d912ca0b5a122efd1e15 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Mon, 1 Apr 2019 13:34:01 -0400 Subject: [PATCH 30/38] We aren't using this import --- atst/domain/authz/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/atst/domain/authz/__init__.py b/atst/domain/authz/__init__.py index 37e06ab8..38e3fa38 100644 --- a/atst/domain/authz/__init__.py +++ b/atst/domain/authz/__init__.py @@ -1,7 +1,6 @@ from atst.utils import first_or_none from atst.models.permissions import Permissions from atst.domain.exceptions import UnauthorizedError -from atst.domain.portfolio_roles import PortfolioRoles from atst.models.portfolio_role import Status as PortfolioRoleStatus From bcfc8ee8e1f31a62f3f6673e5fd4642e98d9c1f9 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 3 Apr 2019 09:48:30 -0400 Subject: [PATCH 31/38] Remove double import --- atst/routes/portfolios/index.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index ecb1f361..cf0a87b0 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -2,8 +2,6 @@ from datetime import date, timedelta from flask import render_template, request as http_request, g, redirect, url_for -from atst.utils.flash import formatted_flash as flash - from . import portfolios_bp from atst.domain.reports import Reports from atst.domain.portfolios import Portfolios From 83b071bf209a97338f7d20ad1ccf363f38feba95 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 3 Apr 2019 10:32:03 -0400 Subject: [PATCH 32/38] Get changes working with merged PRs --- atst/routes/portfolios/index.py | 14 ++++++-------- templates/fragments/admin/members_edit.html | 4 ++-- templates/fragments/admin/portfolio_members.html | 2 +- tests/routes/portfolios/test_portfolios_index.py | 6 ++---- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index cf0a87b0..daf768ae 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -177,18 +177,16 @@ def portfolio_reports(portfolio_id): @portfolios_bp.route( - "/portfolios//members//delete", methods=["POST"] + "/portfolios//members//delete", methods=["POST"] ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="update portfolio members") -def remove_member(portfolio_id, member_id): - if member_id == str(g.current_user.id): +def remove_member(portfolio_id, user_id): + if str(g.current_user.id) == user_id: raise UnauthorizedError( - user=g.current_user, action="you cant remove yourself from the portfolio" + g.current_user, "you cant remove yourself from the portfolio" ) - portfolio = Portfolios.get(g.current_user, portfolio_id) - portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=member_id) - + portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=user_id) PortfolioRoles.disable(portfolio_role=portfolio_role) flash("portfolio_member_removed", member_name=portfolio_role.user.full_name) @@ -196,7 +194,7 @@ def remove_member(portfolio_id, member_id): return redirect( url_for( "portfolios.portfolio_admin", - portfolio_id=portfolio.id, + portfolio_id=portfolio_id, _anchor="portfolio-members", fragment="portfolio-members", ) diff --git a/templates/fragments/admin/members_edit.html b/templates/fragments/admin/members_edit.html index 04d2ed21..2c4df95f 100644 --- a/templates/fragments/admin/members_edit.html +++ b/templates/fragments/admin/members_edit.html @@ -1,8 +1,8 @@ {% from "components/confirmation_button.html" import ConfirmationButton %} -{% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, user.id) %} - {% for subform in member_perms_form.members_permissions %} + {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, subform.user_id.data) %} + {{ subform.member.data }} {% if subform.member.data == user.full_name %} diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 5b4ac977..0763aad0 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -70,7 +70,7 @@ }}
-
+ {{ member_perms_form.csrf_token }}
+ {% if g.last_login %} +
+ Last Login: +
+ {% endif %} From 215c2b4cbcb3499b93eb241cebe6606c09e020d0 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 3 Apr 2019 12:23:42 -0400 Subject: [PATCH 37/38] Updates from PR feedback --- atst/domain/users.py | 2 +- templates/footer.html | 2 +- tests/domain/test_users.py | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/atst/domain/users.py b/atst/domain/users.py index b276a5bd..baf5acb0 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -85,7 +85,7 @@ class Users(object): @classmethod def update_last_login(cls, user): - setattr(user, "last_login", datetime.now()) + user.last_login = datetime.now() db.session.add(user) db.session.commit() diff --git a/templates/footer.html b/templates/footer.html index 340b10cf..39e1bf96 100644 --- a/templates/footer.html +++ b/templates/footer.html @@ -9,7 +9,7 @@
{% if g.last_login %}
- Last Login: + Last Login:
{% endif %} diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index 1c0cdcbc..116e5294 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -69,12 +69,8 @@ def test_update_user_with_dod_id(): def test_update_user_with_last_login(): - new_user = UserFactory.create(last_login=datetime.now()) + new_user = UserFactory.create() Users.update_last_login(new_user) last_login = new_user.last_login - - with pytest.raises(UnauthorizedError): - Users.update(new_user, {"last_login": datetime.now()}) - Users.update_last_login(new_user) assert new_user.last_login > last_login From cc11123eba3065d55ac57ddb64c818530b15c19d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 3 Apr 2019 13:03:17 -0400 Subject: [PATCH 38/38] Simplify get_last_login() --- atst/domain/auth.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 256511f9..1737763f 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -53,11 +53,7 @@ def get_current_user(): def get_last_login(): - last_login = session.get("last_login") - if last_login and session.get("user_id"): - return last_login - else: - return False + return session.get("user_id") and session.get("last_login") def logout():