From 54239a520e2ca6a10ca22b3c7913b1827cbfee92 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 13:29:35 -0400 Subject: [PATCH 1/8] Add revoke access button and modal, sketch out route function for removing cppo perms from a user --- atst/routes/ccpo.py | 9 +++++++ templates/ccpo/users.html | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index c2846aa1..7c9ed484 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -56,3 +56,12 @@ def confirm_new_user(): Users.give_ccpo_perms(user) flash("ccpo_user_added", user_name=user.full_name) return redirect(url_for("ccpo.users")) + + +@bp.route("/ccpo-users/remove-access/", methods=["POST"]) +@user_can(Permissions.DELETE_CCPO_USER, message="remove ccpo user") +def remove_access(user_id): + user = Users.get(user_id) + # update user to remove perms + # flash alert to confirm removing ccpo perms + return redirect(url_for("ccpo.users")) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index bc77a83f..47ebe17d 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -1,6 +1,9 @@ {% extends "base.html" %} +{% from "components/alert.html" import Alert %} +{% from "components/delete_confirmation.html" import DeleteConfirmation %} {% from "components/icon.html" import Icon %} +{% from "components/modal.html" import Modal %} {% block content %}
@@ -20,10 +23,21 @@ {% for user in users %} + {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} + {% set disable_button_class = 'button-danger-outline' %} + {% if user == g.current_user %} + {% set disable_button_class = "usa-button-disabled" %} + {% endif %} + {{ user.full_name }} {{ user.email }} {{ user.dod_id }} + + + Disable + + {% endfor %} @@ -36,4 +50,42 @@ {% endif %} + {% if user_can(permissions.DELETE_CCPO_USER) %} + {% for user in users %} + {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} + {% set confirmation_text = 'remove' %} + {% call Modal(name=modal_id) %} + {{ + Alert( + title=("components.modal.destructive_title" | translate), + message="Confirm removing CCPO superuser access from {}".format(user.full_name), + level="warning" + ) + }} + + +
+
+ + +
+
+
+ +
+ +
+
+
+ {% endcall %} + {% endfor %} + {% endif %} {% endblock %} From e35399d8f5befb98c0ae22670f05cc390eb91ae0 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 14:09:10 -0400 Subject: [PATCH 2/8] Update route function to revoke CCPO superuser status --- atst/routes/ccpo.py | 4 ++-- atst/utils/flash.py | 5 +++++ tests/routes/test_ccpo.py | 12 +++++++++++- tests/test_access.py | 11 +++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index 7c9ed484..5f799e22 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -62,6 +62,6 @@ def confirm_new_user(): @user_can(Permissions.DELETE_CCPO_USER, message="remove ccpo user") def remove_access(user_id): user = Users.get(user_id) - # update user to remove perms - # flash alert to confirm removing ccpo perms + Users.revoke_ccpo_perms(user) + flash("ccpo_user_removed", user_name=user.full_name) return redirect(url_for("ccpo.users")) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index e4f852ea..b511f4d2 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -40,6 +40,11 @@ MESSAGES = { "message_template": translate("ccpo.form.user_not_found_text"), "category": "info", }, + "ccpo_user_removed": { + "title_template": translate("flash.success"), + "message_template": "You have successfully removed {{ user_name }}'s CCPO permissions.", + "category": "success", + }, "environment_added": { "title_template": translate("flash.success"), "message_template": """ diff --git a/tests/routes/test_ccpo.py b/tests/routes/test_ccpo.py index 618eb9b9..c4a8e2a4 100644 --- a/tests/routes/test_ccpo.py +++ b/tests/routes/test_ccpo.py @@ -1,5 +1,6 @@ from flask import url_for +from atst.domain.users import Users from atst.utils.localization import translate from tests.factories import UserFactory @@ -45,10 +46,19 @@ def test_confirm_new_user(user_session, client): ) assert new_user.dod_id in response.data.decode() - # give person with out ATAT account CCPO permissions + # give person without ATAT account CCPO permissions response = client.post( url_for("ccpo.confirm_new_user"), data={"dod_id": random_dod_id}, follow_redirects=True, ) assert random_dod_id not in response.data.decode() + + +def test_remove_access(user_session, client): + ccpo = UserFactory.create_ccpo() + user = UserFactory.create_ccpo() + user_session(ccpo) + + response = client.post(url_for("ccpo.remove_access", user_id=user.id)) + assert user not in Users.get_ccpo_users() diff --git a/tests/test_access.py b/tests/test_access.py index f459ea06..7da4868e 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -161,6 +161,17 @@ def test_ccpo_confirm_new_user_access(post_url_assert_status): post_url_assert_status(rando, url, 404, data={"dod_id": user.dod_id}) +# ccpo.remove_access +def test_ccpo_remove_access(post_url_assert_status): + ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) + rando = user_with() + user = UserFactory.create_ccpo() + + url = url_for("ccpo.remove_access", user_id=user.id) + post_url_assert_status(rando, url, 404) + post_url_assert_status(ccpo, url, 302) + + # applications.access_environment def test_applications_access_environment_access(get_url_assert_status): dev = UserFactory.create() From 0745539853e235bcfbbee86ff91f21c1a4c780e1 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 14:14:00 -0400 Subject: [PATCH 3/8] Move text into translations --- templates/ccpo/users.html | 6 +++--- translations.yaml | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 47ebe17d..13e4db0b 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -35,7 +35,7 @@ {{ user.dod_id }} - Disable + {{ "common.disable" | translate }} @@ -58,7 +58,7 @@ {{ Alert( title=("components.modal.destructive_title" | translate), - message="Confirm removing CCPO superuser access from {}".format(user.full_name), + message=("ccpo.disable_user.alert_message" | translate("user_name": user.full_name)), level="warning" ) }} @@ -76,7 +76,7 @@
diff --git a/translations.yaml b/translations.yaml index fd97ae8f..f03ed93c 100644 --- a/translations.yaml +++ b/translations.yaml @@ -37,6 +37,9 @@ ccpo: return_link: Return to list of CCPO users user_not_found_title: User not found user_not_found_text: To add someone as a CCPO user, they must already have an ATAT account. + disable_user: + alert_message: "Confirm removing CCPO superuser access from {user_name}" + remove_button: Remove Access common: cancel: Cancel close: Close @@ -46,6 +49,7 @@ common: deactivate: Deactivate delete_confirm: 'Please type the word {word} to confirm:' dod_id: DoD ID + disable: Disable edit: Edit email: Email members: Members From 7c65783d081af7aa00e2d4c427d8301600c5e915 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 14:33:10 -0400 Subject: [PATCH 4/8] Add csrf token to remove ccpo user form --- atst/routes/ccpo.py | 5 ++++- templates/ccpo/users.html | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index 5f799e22..384a6b50 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -26,7 +26,10 @@ def activity_history(): @user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") def users(): users = Users.get_ccpo_users() - return render_template("ccpo/users.html", users=users) + data = {} + for user in users: + data[user] = CCPOUserForm(obj=user) + return render_template("ccpo/users.html", data=data) @bp.route("/ccpo-users/new") diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 13e4db0b..086219e3 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -22,7 +22,7 @@ - {% for user in users %} + {% for user, form in data.items() %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% set disable_button_class = 'button-danger-outline' %} {% if user == g.current_user %} @@ -51,14 +51,14 @@ {% endif %} {% if user_can(permissions.DELETE_CCPO_USER) %} - {% for user in users %} + {% for user, form in data.items() %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% set confirmation_text = 'remove' %} {% call Modal(name=modal_id) %} {{ Alert( title=("components.modal.destructive_title" | translate), - message=("ccpo.disable_user.alert_message" | translate("user_name": user.full_name)), + message=("ccpo.disable_user.alert_message" | translate({"user_name": user.full_name})), level="warning" ) }} @@ -75,6 +75,7 @@
+ {{ form.csrf_token }} From 14978142b1d12aa0291e0ff814affb87899e2e85 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 14:41:14 -0400 Subject: [PATCH 5/8] Use DeleteConfirmation macro --- templates/ccpo/users.html | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 086219e3..5181df5b 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -53,7 +53,6 @@ {% if user_can(permissions.DELETE_CCPO_USER) %} {% for user, form in data.items() %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} - {% set confirmation_text = 'remove' %} {% call Modal(name=modal_id) %} {{ Alert( @@ -62,30 +61,15 @@ level="warning" ) }} - - -
-
- - -
-
- - {{ form.csrf_token }} - - - -
-
-
+ {{ + DeleteConfirmation( + modal_id=modal_id, + delete_text='Remove Access', + delete_action=(url_for('ccpo.remove_access', user_id=user.id)), + form=form, + confirmation_text='remove' + ) + }} {% endcall %} {% endfor %} {% endif %} From 8b23173fef4d53fd734305851bc2b9f67d6d6493 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 14:44:25 -0400 Subject: [PATCH 6/8] Only show disable link if user has delete ccpo user perms --- templates/ccpo/users.html | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 5181df5b..584f2bf8 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -19,6 +19,9 @@ {{ "common.name" | translate }} {{ "common.email" | translate }} {{ "common.dod_id" | translate }} + {% if user_can(permissions.DELETE_CCPO_USER) %} + + {% endif %} @@ -33,11 +36,13 @@ {{ user.full_name }} {{ user.email }} {{ user.dod_id }} - - - {{ "common.disable" | translate }} - - + {% if user_can(permissions.DELETE_CCPO_USER) %} + + + {{ "common.disable" | translate }} + + + {% endif %} {% endfor %} From 87b173b035fefa157f5c9cf0500a994ac6296aa5 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 13 Aug 2019 14:09:04 -0400 Subject: [PATCH 7/8] Use tuple instead of a dict for the user data and form --- atst/routes/ccpo.py | 4 +--- templates/ccpo/users.html | 8 ++++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index 384a6b50..553e2a00 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -26,9 +26,7 @@ def activity_history(): @user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") def users(): users = Users.get_ccpo_users() - data = {} - for user in users: - data[user] = CCPOUserForm(obj=user) + data = [(user, CCPOUserForm(obj=user)) for user in users] return render_template("ccpo/users.html", data=data) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 584f2bf8..7e0d2ff8 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -25,7 +25,9 @@ - {% for user, form in data.items() %} + {% for user_data in data %} + {% set user = user_data[0] %} + {% set form = user_data[1] %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% set disable_button_class = 'button-danger-outline' %} {% if user == g.current_user %} @@ -56,7 +58,9 @@ {% endif %} {% if user_can(permissions.DELETE_CCPO_USER) %} - {% for user, form in data.items() %} + {% for user_data in data %} + {% set user = user_data[0] %} + {% set form = user_data[1] %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% call Modal(name=modal_id) %} {{ From 852046a3e719fead449270df89bded855d110491 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 13 Aug 2019 14:22:43 -0400 Subject: [PATCH 8/8] Change variable name from data to user_info because it was more descriptive --- atst/routes/ccpo.py | 4 ++-- templates/ccpo/users.html | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index 553e2a00..a1a3a87c 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -26,8 +26,8 @@ def activity_history(): @user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") def users(): users = Users.get_ccpo_users() - data = [(user, CCPOUserForm(obj=user)) for user in users] - return render_template("ccpo/users.html", data=data) + users_info = [(user, CCPOUserForm(obj=user)) for user in users] + return render_template("ccpo/users.html", users_info=users_info) @bp.route("/ccpo-users/new") diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 7e0d2ff8..f5ea52a8 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -25,9 +25,7 @@ - {% for user_data in data %} - {% set user = user_data[0] %} - {% set form = user_data[1] %} + {% for user, form in users_info %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% set disable_button_class = 'button-danger-outline' %} {% if user == g.current_user %} @@ -58,9 +56,7 @@ {% endif %} {% if user_can(permissions.DELETE_CCPO_USER) %} - {% for user_data in data %} - {% set user = user_data[0] %} - {% set form = user_data[1] %} + {% for user, form in users_info %} {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} {% call Modal(name=modal_id) %} {{