From 5de1b859db743922d132bc89f997a0b97c5aeb98 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 18 Apr 2019 14:04:09 -0400 Subject: [PATCH 01/16] Create form class for app env settings --- atst/forms/app_settings.py | 15 +++++++++++++++ atst/forms/data.py | 3 +++ 2 files changed, 18 insertions(+) create mode 100644 atst/forms/app_settings.py diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py new file mode 100644 index 00000000..8f8d7d6e --- /dev/null +++ b/atst/forms/app_settings.py @@ -0,0 +1,15 @@ +from wtforms.fields import StringField, HiddenField, RadioField, FieldList, FormField + +from .forms import BaseForm +from .data import ENV_ROLES + + +class EnvMemberRoleForm(BaseForm): + name = StringField() + user_id = HiddenField() + role = RadioField(choices=ENV_ROLES) + + +class EnvironmentForm(BaseForm): + team_roles = FieldList(FormField(EnvMemberRoleForm)) + env_id = HiddenField() diff --git a/atst/forms/data.py b/atst/forms/data.py index 4223de08..a98fbc04 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,4 +1,5 @@ from atst.utils.localization import translate, translate_duration +from atst.models.environment_role import CSPRole SERVICE_BRANCHES = [ @@ -215,3 +216,5 @@ REQUIRED_DISTRIBUTIONS = [ ("administrative_ko", "Administrative Contracting Officer"), ("other", "Other as necessary"), ] + +ENV_ROLES = [(role.value, role.value) for role in CSPRole] From fbd9c9db66e565b0f6c7dc54ea2f789e8c35534e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 18 Apr 2019 15:24:48 -0400 Subject: [PATCH 02/16] Create form in view route for application settings --- atst/routes/applications/settings.py | 23 ++++++++++++ tests/factories.py | 3 +- tests/routes/applications/test_settings.py | 43 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8ef290d6..fe398146 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -4,6 +4,7 @@ from . import applications_bp from atst.domain.environment_roles import EnvironmentRoles from atst.domain.applications import Applications from atst.forms.application import ApplicationForm +from atst.forms.app_settings import EnvironmentForm from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.permissions import Permissions from atst.utils.flash import formatted_flash as flash @@ -23,17 +24,39 @@ def get_environments_obj_for_app(application): return environments_obj +def serialize_env_member_form_data(application): + environments_list = [] + for env in application.environments: + env_info = {"env_id": env.id, "team_roles": []} + for user in env.users: + env_role = EnvironmentRoles.get(user.id, env.id) + env_info["team_roles"].append( + { + "name": user.full_name, + "user_id": user.id, + "role": env_role.displayname, + } + ) + environments_list.append(env_info) + return environments_list + + @applications_bp.route("/applications//settings") @user_can(Permissions.VIEW_APPLICATION, message="view application edit form") def settings(application_id): application = Applications.get(application_id) form = ApplicationForm(name=application.name, description=application.description) + env_data = serialize_env_member_form_data(application) + env_forms = {} + for data in env_data: + env_forms[data["env_id"]] = EnvironmentForm(data=data) return render_template( "portfolios/applications/edit.html", application=application, form=form, environments_obj=get_environments_obj_for_app(application=application), + env_forms=env_forms, ) diff --git a/tests/factories.py b/tests/factories.py index 29dc7fa0..12b4f62b 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -10,6 +10,7 @@ from atst.models import * from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.application_role import Status as ApplicationRoleStatus from atst.models.invitation import Status as InvitationStatus +from atst.models.environment_role import CSPRole from atst.domain.invitations import Invitations from atst.domain.permission_sets import PermissionSets from atst.domain.portfolio_roles import PortfolioRoles @@ -234,7 +235,7 @@ class EnvironmentRoleFactory(Base): model = EnvironmentRole environment = factory.SubFactory(EnvironmentFactory) - role = factory.Faker("name") + role = random.choice([e.value for e in CSPRole]) user = factory.SubFactory(UserFactory) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index f6f2b5db..b0f4d427 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -10,6 +10,8 @@ from tests.factories import ( ) from atst.domain.applications import Applications +from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.models.portfolio_role import Status as PortfolioRoleStatus @@ -65,6 +67,47 @@ def test_edit_application_environments_obj(app, client, user_session): } +def test_edit_app_serialize_env_member_form_data(app, client, user_session): + portfolio = PortfolioFactory.create() + application = Applications.create( + portfolio, + "Snazzy Application", + "A new application for me and my friends", + {"env1", "env2"}, + ) + user1 = UserFactory.create() + user2 = UserFactory.create() + env1 = application.environments[0] + env2 = application.environments[1] + env_role1 = EnvironmentRoleFactory.create(environment=env1, user=user1) + env_role2 = EnvironmentRoleFactory.create(environment=env1, user=user2) + env_role3 = EnvironmentRoleFactory.create(environment=env2, user=user1) + + user_session(portfolio.owner) + + with captured_templates(app) as templates: + response = app.test_client().get( + url_for("applications.settings", application_id=application.id) + ) + + assert response.status_code == 200 + _, context = templates[0] + for env_id in context["env_forms"]: + env = Environments.get(environment_id=env_id) + form_data = {"env_id": env_id, "team_roles": []} + for user in env.users: + env_role = EnvironmentRoles.get(user.id, env.id) + form_data["team_roles"].append( + { + "name": user.full_name, + "user_id": user.id, + "role": env_role.displayname, + } + ) + + assert context["env_forms"][env_id].data == form_data + + def test_user_with_permission_can_update_application(client, user_session): owner = UserFactory.create() portfolio = PortfolioFactory.create( From f6577c0cd663d7327ead31887ae70dfc7c31afd4 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 22 Apr 2019 16:56:09 -0400 Subject: [PATCH 03/16] Update name of env form --- atst/forms/app_settings.py | 2 +- atst/forms/data.py | 4 +++- atst/routes/applications/settings.py | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index 8f8d7d6e..cdc20b0d 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -10,6 +10,6 @@ class EnvMemberRoleForm(BaseForm): role = RadioField(choices=ENV_ROLES) -class EnvironmentForm(BaseForm): +class EnvironmentRolesForm(BaseForm): team_roles = FieldList(FormField(EnvMemberRoleForm)) env_id = HiddenField() diff --git a/atst/forms/data.py b/atst/forms/data.py index a98fbc04..85826ada 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -217,4 +217,6 @@ REQUIRED_DISTRIBUTIONS = [ ("other", "Other as necessary"), ] -ENV_ROLES = [(role.value, role.value) for role in CSPRole] +ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [ + ("No access", "No access") +] diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index fe398146..46919ac4 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -4,7 +4,7 @@ from . import applications_bp from atst.domain.environment_roles import EnvironmentRoles from atst.domain.applications import Applications from atst.forms.application import ApplicationForm -from atst.forms.app_settings import EnvironmentForm +from atst.forms.app_settings import EnvironmentRolesForm from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.permissions import Permissions from atst.utils.flash import formatted_flash as flash @@ -49,7 +49,7 @@ def settings(application_id): env_data = serialize_env_member_form_data(application) env_forms = {} for data in env_data: - env_forms[data["env_id"]] = EnvironmentForm(data=data) + env_forms[data["env_id"]] = EnvironmentRolesForm(data=data) return render_template( "portfolios/applications/edit.html", From c085f27af8d59bffed1523562828c1a2950c992f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 22 Apr 2019 16:06:13 -0400 Subject: [PATCH 04/16] Update env roles by environment --- atst/domain/environments.py | 61 ++++---- tests/domain/test_environments.py | 208 ++++++++++++---------------- tests/factories.py | 1 + tests/models/test_portfolio_role.py | 4 +- 4 files changed, 129 insertions(+), 145 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 5e7102b1..69ba8ab3 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -6,6 +6,7 @@ from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole from atst.models.application import Application from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.users import Users from .exceptions import NotFoundError @@ -62,39 +63,51 @@ class Environments(object): return env @classmethod - def update_environment_roles(cls, portfolio_role, ids_and_roles): + def update_env_role(cls, environment, user, new_role): updated = False - for id_and_role in ids_and_roles: - new_role = id_and_role["role"] - environment = Environments.get(id_and_role["id"]) - - if new_role is None: - role_deleted = EnvironmentRoles.delete( - portfolio_role.user.id, environment.id + if new_role is None or new_role == "No access": + role_deleted = EnvironmentRoles.delete(user.id, environment.id) + if role_deleted: + updated = True + else: + env_role = EnvironmentRoles.get(user.id, environment.id) + if env_role and env_role.role != new_role: + env_role.role = new_role + updated = True + db.session.add(env_role) + elif not env_role: + env_role = EnvironmentRoles.create( + user=user, environment=environment, role=new_role ) - if role_deleted: - updated = True - else: - env_role = EnvironmentRoles.get( - portfolio_role.user.id, id_and_role["id"] - ) - if env_role and env_role.role != new_role: - env_role.role = new_role - updated = True - db.session.add(env_role) - elif not env_role: - env_role = EnvironmentRoles.create( - user=portfolio_role.user, environment=environment, role=new_role - ) - updated = True - db.session.add(env_role) + updated = True + db.session.add(env_role) if updated: db.session.commit() return updated + @classmethod + def update_env_roles_by_environment(cls, environment_id, team_roles): + environment = Environments.get(environment_id) + + for member in team_roles: + new_role = member["role"] + user = Users.get(member["user_id"]) + Environments.update_env_role( + environment=environment, user=user, new_role=new_role + ) + + @classmethod + def update_env_roles_by_member(cls, member, env_roles): + for env_roles in env_roles: + new_role = env_roles["role"] + environment = Environments.get(env_roles["id"]) + Environments.update_env_role( + environment=environment, user=member, new_role=new_role + ) + @classmethod def revoke_access(cls, environment, target_user): EnvironmentRoles.delete(environment.id, target_user.id) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index b572a7e5..18027fd8 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -1,9 +1,11 @@ import pytest +import random from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.exceptions import NotFoundError +from atst.models.environment_role import CSPRole from tests.factories import ( ApplicationFactory, @@ -21,143 +23,111 @@ def test_create_environments(): assert env.cloud_id is not None -def test_create_environment_role_creates_cloud_id(session): - owner = UserFactory.create() - developer = UserFactory.create() +def test_update_env_role(): + env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) + new_role = CSPRole.TECHNICAL_READ.value - portfolio = PortfolioFactory.create( - owner=owner, - members=[{"user": developer, "role_name": "developer"}], - applications=[ - {"name": "application1", "environments": [{"name": "application1 prod"}]} - ], + assert Environments.update_env_role(env_role.environment, env_role.user, new_role) + assert env_role.role == new_role + + +def test_update_env_role_no_access(): + env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) + + assert Environments.update_env_role( + env_role.environment, env_role.user, "No access" + ) + assert not EnvironmentRoles.get(env_role.user.id, env_role.environment.id) + + +def test_update_env_role_no_change(): + env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) + new_role = CSPRole.BASIC_ACCESS.value + + assert not Environments.update_env_role( + env_role.environment, env_role.user, new_role ) - env = portfolio.applications[0].environments[0] - new_role = [{"id": env.id, "role": "developer"}] - portfolio_role = portfolio.members[0] - assert not portfolio_role.user.cloud_id - assert Environments.update_environment_roles(portfolio_role, new_role) - - assert portfolio_role.user.cloud_id is not None +def test_update_env_role_creates_cloud_id_for_new_member(session): + user = UserFactory.create() + env = EnvironmentFactory.create() + assert not user.cloud_id + assert Environments.update_env_role(env, user, CSPRole.TECHNICAL_READ.value) + assert EnvironmentRoles.get(user.id, env.id) + assert user.cloud_id is not None -def test_update_environment_roles(): - owner = UserFactory.create() - developer = UserFactory.create() - - portfolio = PortfolioFactory.create( - owner=owner, - members=[{"user": developer, "role_name": "developer"}], - applications=[ - { - "name": "application1", - "environments": [ - { - "name": "application1 dev", - "members": [{"user": developer, "role_name": "devlops"}], - }, - { - "name": "application1 staging", - "members": [{"user": developer, "role_name": "developer"}], - }, - {"name": "application1 prod"}, - ], - } - ], +def test_update_env_roles_by_environment(): + environment = EnvironmentFactory.create() + env_role_1 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + env_role_2 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.NETWORK_ADMIN.value + ) + env_role_3 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.TECHNICAL_READ.value ) - dev_env = portfolio.applications[0].environments[0] - staging_env = portfolio.applications[0].environments[1] - new_ids_and_roles = [ - {"id": dev_env.id, "role": "billing_admin"}, - {"id": staging_env.id, "role": "developer"}, + team_roles = [ + { + "user_id": env_role_1.user.id, + "name": env_role_1.user.full_name, + "role": CSPRole.BUSINESS_READ.value, + }, + { + "user_id": env_role_2.user.id, + "name": env_role_2.user.full_name, + "role": CSPRole.NETWORK_ADMIN.value, + }, + { + "user_id": env_role_3.user.id, + "name": env_role_3.user.full_name, + "role": "No access", + }, ] - portfolio_role = portfolio.members[0] - assert Environments.update_environment_roles(portfolio_role, new_ids_and_roles) - new_dev_env_role = EnvironmentRoles.get(portfolio_role.user.id, dev_env.id) - staging_env_role = EnvironmentRoles.get(portfolio_role.user.id, staging_env.id) - - assert new_dev_env_role.role == "billing_admin" - assert staging_env_role.role == "developer" + Environments.update_env_roles_by_environment(environment.id, team_roles) + assert env_role_1.role == CSPRole.BUSINESS_READ.value + assert env_role_2.role == CSPRole.NETWORK_ADMIN.value + assert not EnvironmentRoles.get(env_role_3.user.id, environment.id) -def test_remove_environment_role(): - owner = UserFactory.create() - developer = UserFactory.create() - portfolio = PortfolioFactory.create( - owner=owner, - members=[{"user": developer, "role_name": "developer"}], - applications=[ +def test_update_env_roles_by_member(): + user = UserFactory.create() + application = ApplicationFactory.create( + environments=[ { - "name": "application1", - "environments": [ - { - "name": "application1 dev", - "members": [{"user": developer, "role_name": "devops"}], - }, - { - "name": "application1 staging", - "members": [{"user": developer, "role_name": "developer"}], - }, - { - "name": "application1 uat", - "members": [ - {"user": developer, "role_name": "financial_auditor"} - ], - }, - {"name": "application1 prod"}, - ], - } - ], + "name": "dev", + "members": [{"user": user, "role_name": CSPRole.BUSINESS_READ.value}], + }, + { + "name": "staging", + "members": [{"user": user, "role_name": CSPRole.BUSINESS_READ.value}], + }, + {"name": "prod"}, + { + "name": "testing", + "members": [{"user": user, "role_name": CSPRole.BUSINESS_READ.value}], + }, + ] ) - application = portfolio.applications[0] - now_ba = application.environments[0].id - now_none = application.environments[1].id - still_fa = application.environments[2].id - - new_environment_roles = [ - {"id": now_ba, "role": "billing_auditor"}, - {"id": now_none, "role": None}, + dev, staging, prod, testing = application.environments + env_roles = [ + {"id": dev.id, "role": CSPRole.NETWORK_ADMIN.value}, + {"id": staging.id, "role": CSPRole.BUSINESS_READ.value}, + {"id": prod.id, "role": CSPRole.TECHNICAL_READ.value}, + {"id": testing.id, "role": "No access"}, ] - portfolio_role = PortfolioRoles.get(portfolio.id, developer.id) - assert Environments.update_environment_roles(portfolio_role, new_environment_roles) + Environments.update_env_roles_by_member(user, env_roles) - assert portfolio_role.num_environment_roles == 2 - assert EnvironmentRoles.get(developer.id, now_ba).role == "billing_auditor" - assert EnvironmentRoles.get(developer.id, now_none) is None - assert EnvironmentRoles.get(developer.id, still_fa).role == "financial_auditor" - - -def test_no_update_to_environment_roles(): - owner = UserFactory.create() - developer = UserFactory.create() - - portfolio = PortfolioFactory.create( - owner=owner, - members=[{"user": developer, "role_name": "developer"}], - applications=[ - { - "name": "application1", - "environments": [ - { - "name": "application1 dev", - "members": [{"user": developer, "role_name": "devops"}], - } - ], - } - ], - ) - - dev_env = portfolio.applications[0].environments[0] - new_ids_and_roles = [{"id": dev_env.id, "role": "devops"}] - - portfolio_role = PortfolioRoles.get(portfolio.id, developer.id) - assert not Environments.update_environment_roles(portfolio_role, new_ids_and_roles) + assert EnvironmentRoles.get(user.id, dev.id).role == CSPRole.NETWORK_ADMIN.value + assert EnvironmentRoles.get(user.id, staging.id).role == CSPRole.BUSINESS_READ.value + assert EnvironmentRoles.get(user.id, prod.id).role == CSPRole.TECHNICAL_READ.value + assert not EnvironmentRoles.get(user.id, testing.id) def test_get_scoped_environments(db): diff --git a/tests/factories.py b/tests/factories.py index 12b4f62b..a9ac984e 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -194,6 +194,7 @@ class EnvironmentFactory(Base): model = Environment name = factory.Faker("domain_word") + application = factory.SubFactory(ApplicationFactory) @classmethod def _create(cls, model_class, *args, **kwargs): diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 3d9ac0ec..4b5e6601 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -123,8 +123,8 @@ def test_has_env_role_history(session): env_role = EnvironmentRoleFactory.create( user=user, environment=environment, role="developer" ) - Environments.update_environment_roles( - portfolio_role, [{"role": "admin", "id": environment.id}] + Environments.update_env_roles_by_member( + user, [{"role": "admin", "id": environment.id}] ) changed_events = ( session.query(AuditEvent) From 9b426bbde43ba8b96ca19a81749b120741a15f19 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 23 Apr 2019 14:16:29 -0400 Subject: [PATCH 05/16] Add post route for env roles form --- atst/routes/applications/settings.py | 33 +++++++++ tests/routes/applications/test_settings.py | 86 ++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 46919ac4..2e117e88 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -2,6 +2,7 @@ from flask import redirect, render_template, request as http_request, url_for from . import applications_bp from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.environments import Environments from atst.domain.applications import Applications from atst.forms.application import ApplicationForm from atst.forms.app_settings import EnvironmentRolesForm @@ -76,11 +77,43 @@ def update(application_id): ) ) else: + env_data = serialize_env_member_form_data(application) + env_forms = {} + for data in env_data: + env_forms[data["env_id"]] = EnvironmentRolesForm(data=data) + return render_template( "portfolios/applications/edit.html", application=application, form=form, environments_obj=get_environments_obj_for_app(application=application), + env_forms=env_forms, + ) + + +@applications_bp.route( + "/applications//update_env_roles", methods=["POST"] +) +@user_can(Permissions.ASSIGN_ENVIRONMENT_MEMBER, message="update application") +def update_env_roles(application_id): + application = Applications.get(application_id) + env_roles_form = EnvironmentRolesForm(http_request.form) + + if env_roles_form.validate(): + env_data = env_roles_form.data + Environments.update_env_roles_by_environment( + environment_id=env_data["env_id"], team_roles=env_data["team_roles"] + ) + return redirect(url_for("applications.settings", application_id=application.id)) + else: + return render_template( + "portfolios/applications/edit.html", + application=application, + form=ApplicationForm( + name=application.name, description=application.description + ), + environments_obj=get_environments_obj_for_app(application=application), + env_forms=env_roles_form, ) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index b0f4d427..ff8025b0 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -7,12 +7,15 @@ from tests.factories import ( EnvironmentRoleFactory, EnvironmentFactory, ApplicationFactory, + ApplicationRoleFactory, ) from atst.domain.applications import Applications from atst.domain.environment_roles import EnvironmentRoles from atst.domain.environments import Environments +from atst.domain.permission_sets import PermissionSets from atst.domain.portfolios import Portfolios +from atst.models.environment_role import CSPRole from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.utils import captured_templates @@ -163,6 +166,89 @@ def test_user_without_permission_cannot_update_application(client, user_session) assert application.description == "Cool stuff happening here!" +def test_user_with_permission_can_update_team_env_roles(client, user_session): + environment = EnvironmentFactory.create() + application = environment.application + env_role_1 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + env_role_2 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + env_role_3 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + app_role = ApplicationRoleFactory.create(application=application) + form_data = { + "env_id": environment.id, + "team_roles-0-user_id": env_role_1.user.id, + "team_roles-0-name": env_role_1.user.full_name, + "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, + "team_roles-1-user_id": env_role_2.user.id, + "team_roles-1-name": env_role_2.user.full_name, + "team_roles-1-role": CSPRole.BASIC_ACCESS.value, + "team_roles-2-user_id": env_role_3.user.id, + "team_roles-2-name": env_role_3.user.full_name, + "team_roles-2-role": "No access", + "team_roles-3-user_id": app_role.user.id, + "team_roles-3-name": app_role.user.full_name, + "team_roles-3-role": CSPRole.TECHNICAL_READ.value, + } + + user_session(application.portfolio.owner) + response = client.post( + url_for("applications.update_env_roles", application_id=application.id), + data=form_data, + follow_redirects=True, + ) + + assert response.status_code == 200 + assert env_role_1.role == CSPRole.NETWORK_ADMIN.value + assert env_role_2.role == CSPRole.BASIC_ACCESS.value + assert not EnvironmentRoles.get(env_role_3.user.id, environment.id) + assert EnvironmentRoles.get(app_role.user.id, environment.id) + + +def test_user_without_permission_cannot_update_team_env_roles(client, user_session): + environment = EnvironmentFactory.create() + application = environment.application + app_role_without_perms = ApplicationRoleFactory.create(application=application) + env_role_1 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + env_role_2 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + app_role = ApplicationRoleFactory.create(application=application) + form_data = { + "env_id": environment.id, + "team_roles-0-user_id": env_role_1.user.id, + "team_roles-0-name": env_role_1.user.full_name, + "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, + "team_roles-1-user_id": env_role_2.user.id, + "team_roles-1-name": env_role_2.user.full_name, + "team_roles-1-role": "No access", + "team_roles-2-user_id": app_role.user.id, + "team_roles-2-name": app_role.user.full_name, + "team_roles-2-role": CSPRole.TECHNICAL_READ.value, + } + + user_session(app_role_without_perms.user) + response = client.post( + url_for("applications.update_env_roles", application_id=application.id), + data=form_data, + follow_redirects=True, + ) + + assert response.status_code == 404 + assert env_role_1.role == CSPRole.BASIC_ACCESS.value + assert ( + EnvironmentRoles.get(env_role_2.user.id, environment.id).role + == CSPRole.BASIC_ACCESS.value + ) + assert not EnvironmentRoles.get(app_role.user.id, environment.id) + + def test_user_can_only_access_apps_in_their_portfolio(client, user_session): portfolio = PortfolioFactory.create() other_portfolio = PortfolioFactory.create( From 5e415edaef5bcffd235fe1a27c21c631172392fa Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 23 Apr 2019 15:39:31 -0400 Subject: [PATCH 06/16] Update form to handle No access --- atst/domain/environments.py | 2 +- atst/forms/app_settings.py | 7 +++++++ atst/forms/data.py | 4 +--- tests/domain/test_environments.py | 8 +++----- tests/routes/applications/test_settings.py | 4 ++-- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 69ba8ab3..3e2024a8 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -66,7 +66,7 @@ class Environments(object): def update_env_role(cls, environment, user, new_role): updated = False - if new_role is None or new_role == "No access": + if new_role is None: role_deleted = EnvironmentRoles.delete(user.id, environment.id) if role_deleted: updated = True diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index cdc20b0d..4c9b4c3c 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -9,6 +9,13 @@ class EnvMemberRoleForm(BaseForm): user_id = HiddenField() role = RadioField(choices=ENV_ROLES) + @property + def data(self): + _data = super().data + if _data["role"] == "": + _data["role"] = None + return _data + class EnvironmentRolesForm(BaseForm): team_roles = FieldList(FormField(EnvMemberRoleForm)) diff --git a/atst/forms/data.py b/atst/forms/data.py index 85826ada..20f9a2ff 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -217,6 +217,4 @@ REQUIRED_DISTRIBUTIONS = [ ("other", "Other as necessary"), ] -ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [ - ("No access", "No access") -] +ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [("", "No access")] diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 18027fd8..7d959874 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -34,9 +34,7 @@ def test_update_env_role(): def test_update_env_role_no_access(): env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) - assert Environments.update_env_role( - env_role.environment, env_role.user, "No access" - ) + assert Environments.update_env_role(env_role.environment, env_role.user, None) assert not EnvironmentRoles.get(env_role.user.id, env_role.environment.id) @@ -84,7 +82,7 @@ def test_update_env_roles_by_environment(): { "user_id": env_role_3.user.id, "name": env_role_3.user.full_name, - "role": "No access", + "role": None, }, ] @@ -119,7 +117,7 @@ def test_update_env_roles_by_member(): {"id": dev.id, "role": CSPRole.NETWORK_ADMIN.value}, {"id": staging.id, "role": CSPRole.BUSINESS_READ.value}, {"id": prod.id, "role": CSPRole.TECHNICAL_READ.value}, - {"id": testing.id, "role": "No access"}, + {"id": testing.id, "role": None}, ] Environments.update_env_roles_by_member(user, env_roles) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index ff8025b0..9bc2953a 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -189,7 +189,7 @@ def test_user_with_permission_can_update_team_env_roles(client, user_session): "team_roles-1-role": CSPRole.BASIC_ACCESS.value, "team_roles-2-user_id": env_role_3.user.id, "team_roles-2-name": env_role_3.user.full_name, - "team_roles-2-role": "No access", + "team_roles-2-role": "", "team_roles-3-user_id": app_role.user.id, "team_roles-3-name": app_role.user.full_name, "team_roles-3-role": CSPRole.TECHNICAL_READ.value, @@ -227,7 +227,7 @@ def test_user_without_permission_cannot_update_team_env_roles(client, user_sessi "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, "team_roles-1-user_id": env_role_2.user.id, "team_roles-1-name": env_role_2.user.full_name, - "team_roles-1-role": "No access", + "team_roles-1-role": "", "team_roles-2-user_id": app_role.user.id, "team_roles-2-name": app_role.user.full_name, "team_roles-2-role": CSPRole.TECHNICAL_READ.value, From e7903ed000a77dff0eda1463e2dc6a6c935cd382 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:15:10 -0400 Subject: [PATCH 07/16] Move remove_empty_string filter to BaseForm and use the filter to coerce the role to be None --- atst/forms/app_settings.py | 9 +------ atst/forms/data.py | 2 +- atst/forms/forms.py | 4 +++ atst/forms/task_order.py | 50 ++++++++++++++++++-------------------- 4 files changed, 29 insertions(+), 36 deletions(-) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index 4c9b4c3c..d63cf4b1 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -7,14 +7,7 @@ from .data import ENV_ROLES class EnvMemberRoleForm(BaseForm): name = StringField() user_id = HiddenField() - role = RadioField(choices=ENV_ROLES) - - @property - def data(self): - _data = super().data - if _data["role"] == "": - _data["role"] = None - return _data + role = RadioField(choices=ENV_ROLES, coerce=BaseForm.remove_empty_string) class EnvironmentRolesForm(BaseForm): diff --git a/atst/forms/data.py b/atst/forms/data.py index 20f9a2ff..23ab9c4a 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -217,4 +217,4 @@ REQUIRED_DISTRIBUTIONS = [ ("other", "Other as necessary"), ] -ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [("", "No access")] +ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [(None, "No access")] diff --git a/atst/forms/forms.py b/atst/forms/forms.py index 31e62476..bb0d086f 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -35,3 +35,7 @@ class BaseForm(FlaskForm): if not valid: flash("form_errors") return valid + + @classmethod + def remove_empty_string(cls, value): + return value or None diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index b5041d67..997d71ff 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -27,10 +27,6 @@ from .data import ( from atst.utils.localization import translate -def remove_empty_string(value): - return value or None - - class AppInfoWithExistingPortfolioForm(BaseForm): scope = TextAreaField( translate("forms.task_order.scope_label"), @@ -55,28 +51,28 @@ class AppInfoWithExistingPortfolioForm(BaseForm): description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, default=None, - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) complexity_other = StringField( translate("forms.task_order.complexity_other_label"), default=None, - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) dev_team = SelectMultipleField( translate("forms.task_order.dev_team.label"), description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, default=None, - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) dev_team_other = StringField( translate("forms.task_order.dev_team_other_label"), default=None, - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) team_experience = RadioField( translate("forms.task_order.team_experience.label"), @@ -91,7 +87,7 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): portfolio_name = StringField( translate("forms.task_order.portfolio_name_label"), description=translate("forms.task_order.portfolio_name_description"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[ Required(), Length( @@ -105,7 +101,7 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES, default="", - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) @@ -147,36 +143,36 @@ class FundingForm(BaseForm): class UnclassifiedFundingForm(FundingForm): clin_02 = StringField( translate("forms.task_order.unclassified_clin_02_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) clin_04 = StringField( translate("forms.task_order.unclassified_clin_04_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) class OversightForm(BaseForm): ko_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) ko_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) ko_email = StringField( translate("forms.task_order.oversight_email_label"), validators=[Optional(), Email()], - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) ko_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), validators=[Optional(), PhoneNumber()], - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) ko_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[ RequiredIf(lambda form: form._fields.get("ko_invite").data), Length(min=10), @@ -187,20 +183,20 @@ class OversightForm(BaseForm): am_cor = BooleanField(translate("forms.task_order.oversight_am_cor_label")) cor_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) cor_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) cor_email = StringField( translate("forms.task_order.oversight_email_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[Optional(), Email()], ) cor_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[ RequiredIf(lambda form: not form._fields.get("am_cor").data), Optional(), @@ -209,7 +205,7 @@ class OversightForm(BaseForm): ) cor_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[ RequiredIf( lambda form: not form._fields.get("am_cor").data @@ -222,25 +218,25 @@ class OversightForm(BaseForm): so_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) so_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], ) so_email = StringField( translate("forms.task_order.oversight_email_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[Optional(), Email()], ) so_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[Optional(), PhoneNumber()], ) so_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[remove_empty_string], + filters=[BaseForm.remove_empty_string], validators=[ RequiredIf(lambda form: form._fields.get("so_invite").data), Length(min=10), From 1f7b5469de290361da9b7741119acee042286a6c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:19:01 -0400 Subject: [PATCH 08/16] remove unnecessary if statement --- atst/domain/environments.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 3e2024a8..44eaad9a 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -67,9 +67,7 @@ class Environments(object): updated = False if new_role is None: - role_deleted = EnvironmentRoles.delete(user.id, environment.id) - if role_deleted: - updated = True + updated = EnvironmentRoles.delete(user.id, environment.id) else: env_role = EnvironmentRoles.get(user.id, environment.id) if env_role and env_role.role != new_role: From c6cf4d76411373a1b6f92f771450f4e7bd00d96e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:24:41 -0400 Subject: [PATCH 09/16] change variable name to be more clear --- atst/routes/applications/settings.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 2e117e88..ac739d3b 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -45,12 +45,13 @@ def serialize_env_member_form_data(application): @applications_bp.route("/applications//settings") @user_can(Permissions.VIEW_APPLICATION, message="view application edit form") def settings(application_id): + # refactor like portfolio admin render function application = Applications.get(application_id) form = ApplicationForm(name=application.name, description=application.description) - env_data = serialize_env_member_form_data(application) + app_envs_data = serialize_env_member_form_data(application) env_forms = {} - for data in env_data: - env_forms[data["env_id"]] = EnvironmentRolesForm(data=data) + for env_data in app_envs_data: + env_forms[env_data["env_id"]] = EnvironmentRolesForm(data=env_data) return render_template( "portfolios/applications/edit.html", From addd1c07c39bd934504684978566f8093682aba3 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:30:33 -0400 Subject: [PATCH 10/16] Add TODO about handling form validation failures --- atst/routes/applications/settings.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index ac739d3b..419bbb29 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -107,6 +107,9 @@ def update_env_roles(application_id): ) return redirect(url_for("applications.settings", application_id=application.id)) else: + # TODO: Create a better pattern to handle when a form doesn't validate + # if a user is submitting the data via the web page then they + # should never have any form validation errors return render_template( "portfolios/applications/edit.html", application=application, From 8680c1001749623beb0003652a20700eb22fab13 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:35:11 -0400 Subject: [PATCH 11/16] Change file name to settings --- atst/routes/applications/settings.py | 6 +++--- .../portfolios/applications/{edit.html => settings.html} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename templates/portfolios/applications/{edit.html => settings.html} (100%) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 419bbb29..9a1db545 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -54,7 +54,7 @@ def settings(application_id): env_forms[env_data["env_id"]] = EnvironmentRolesForm(data=env_data) return render_template( - "portfolios/applications/edit.html", + "portfolios/applications/settings.html", application=application, form=form, environments_obj=get_environments_obj_for_app(application=application), @@ -84,7 +84,7 @@ def update(application_id): env_forms[data["env_id"]] = EnvironmentRolesForm(data=data) return render_template( - "portfolios/applications/edit.html", + "portfolios/applications/settings.html", application=application, form=form, environments_obj=get_environments_obj_for_app(application=application), @@ -111,7 +111,7 @@ def update_env_roles(application_id): # if a user is submitting the data via the web page then they # should never have any form validation errors return render_template( - "portfolios/applications/edit.html", + "portfolios/applications/settings.html", application=application, form=ApplicationForm( name=application.name, description=application.description diff --git a/templates/portfolios/applications/edit.html b/templates/portfolios/applications/settings.html similarity index 100% rename from templates/portfolios/applications/edit.html rename to templates/portfolios/applications/settings.html From 73e4057c3da972fe65703b4d70de2cc35191446d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 11:36:26 -0400 Subject: [PATCH 12/16] Remove unused imports --- tests/domain/test_environments.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 7d959874..af8bb2a4 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -1,9 +1,7 @@ import pytest -import random from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles -from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.exceptions import NotFoundError from atst.models.environment_role import CSPRole From 99a3534d030ce232407386c14bfba6110560f888 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 16:37:58 -0400 Subject: [PATCH 13/16] Update route to include environment_id --- atst/domain/authz/decorator.py | 6 ++++++ atst/routes/applications/settings.py | 13 ++++++------- atst/utils/context_processors.py | 10 +++++++++- tests/routes/applications/test_settings.py | 4 ++-- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index 90f3f78e..8c02718a 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -7,6 +7,7 @@ from atst.domain.portfolios import Portfolios from atst.domain.task_orders import TaskOrders from atst.domain.applications import Applications from atst.domain.invitations import Invitations +from atst.domain.environments import Environments from atst.domain.exceptions import UnauthorizedError @@ -31,6 +32,11 @@ def check_access(permission, message, override, *args, **kwargs): g.current_user, kwargs["portfolio_id"] ) + elif "environment_id" in kwargs: + environment = Environments.get(kwargs["environment_id"]) + access_args["application"] = environment.application + access_args["portfolio"] = environment.application.portfolio + if override is not None and override(g.current_user, **access_args, **kwargs): return True diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 9a1db545..8cacb658 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -92,18 +92,17 @@ def update(application_id): ) -@applications_bp.route( - "/applications//update_env_roles", methods=["POST"] -) -@user_can(Permissions.ASSIGN_ENVIRONMENT_MEMBER, message="update application") -def update_env_roles(application_id): - application = Applications.get(application_id) +@applications_bp.route("/environments//roles", methods=["POST"]) +@user_can(Permissions.ASSIGN_ENVIRONMENT_MEMBER, message="update environment roles") +def update_env_roles(environment_id): + environment = Environments.get(environment_id) + application = environment.application env_roles_form = EnvironmentRolesForm(http_request.form) if env_roles_form.validate(): env_data = env_roles_form.data Environments.update_env_roles_by_environment( - environment_id=env_data["env_id"], team_roles=env_data["team_roles"] + environment_id=environment_id, team_roles=env_data["team_roles"] ) return redirect(url_for("applications.settings", application_id=application.id)) else: diff --git a/atst/utils/context_processors.py b/atst/utils/context_processors.py index 4b82320b..d0567d4f 100644 --- a/atst/utils/context_processors.py +++ b/atst/utils/context_processors.py @@ -5,7 +5,7 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db from atst.domain.authz import Authorization -from atst.models import Application, Portfolio, TaskOrder +from atst.models import Application, Environment, Portfolio, TaskOrder from atst.models.permissions import Permissions from atst.domain.portfolios.scopes import ScopedPortfolio @@ -25,6 +25,14 @@ def get_portfolio_from_context(view_args): .filter(Application.id == view_args["application_id"]) ) + elif "environment_id" in view_args: + query = ( + db.session.query(Portfolio) + .join(Application, Application.portfolio_id == Portfolio.id) + .join(Environment, Environment.application_id == Application.id) + .filter(Environment.id == view_args["environment_id"]) + ) + elif "task_order_id" in view_args: query = ( db.session.query(Portfolio) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 9bc2953a..354022cf 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -197,7 +197,7 @@ def test_user_with_permission_can_update_team_env_roles(client, user_session): user_session(application.portfolio.owner) response = client.post( - url_for("applications.update_env_roles", application_id=application.id), + url_for("applications.update_env_roles", environment_id=environment.id), data=form_data, follow_redirects=True, ) @@ -235,7 +235,7 @@ def test_user_without_permission_cannot_update_team_env_roles(client, user_sessi user_session(app_role_without_perms.user) response = client.post( - url_for("applications.update_env_roles", application_id=application.id), + url_for("applications.update_env_roles", environment_id=environment.id), data=form_data, follow_redirects=True, ) From 72cc12f195b3f342d716eb411cbfef2283aeaeb6 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 16:38:13 -0400 Subject: [PATCH 14/16] Update todo formatting --- atst/routes/applications/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8cacb658..169af8c9 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -107,8 +107,8 @@ def update_env_roles(environment_id): return redirect(url_for("applications.settings", application_id=application.id)) else: # TODO: Create a better pattern to handle when a form doesn't validate - # if a user is submitting the data via the web page then they - # should never have any form validation errors + # if a user is submitting the data via the web page then they + # should never have any form validation errors return render_template( "portfolios/applications/settings.html", application=application, From 7f4a18a49bdcfdb2c58217e289a7f00047cfee1d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 24 Apr 2019 17:09:32 -0400 Subject: [PATCH 15/16] Move test checking route access into test_access and rename test checking if route is working --- tests/routes/applications/test_settings.py | 42 +--------------------- tests/test_access.py | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 354022cf..2b9ec47e 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -166,7 +166,7 @@ def test_user_without_permission_cannot_update_application(client, user_session) assert application.description == "Cool stuff happening here!" -def test_user_with_permission_can_update_team_env_roles(client, user_session): +def test_update_team_env_roles(client, user_session): environment = EnvironmentFactory.create() application = environment.application env_role_1 = EnvironmentRoleFactory.create( @@ -209,46 +209,6 @@ def test_user_with_permission_can_update_team_env_roles(client, user_session): assert EnvironmentRoles.get(app_role.user.id, environment.id) -def test_user_without_permission_cannot_update_team_env_roles(client, user_session): - environment = EnvironmentFactory.create() - application = environment.application - app_role_without_perms = ApplicationRoleFactory.create(application=application) - env_role_1 = EnvironmentRoleFactory.create( - environment=environment, role=CSPRole.BASIC_ACCESS.value - ) - env_role_2 = EnvironmentRoleFactory.create( - environment=environment, role=CSPRole.BASIC_ACCESS.value - ) - app_role = ApplicationRoleFactory.create(application=application) - form_data = { - "env_id": environment.id, - "team_roles-0-user_id": env_role_1.user.id, - "team_roles-0-name": env_role_1.user.full_name, - "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, - "team_roles-1-user_id": env_role_2.user.id, - "team_roles-1-name": env_role_2.user.full_name, - "team_roles-1-role": "", - "team_roles-2-user_id": app_role.user.id, - "team_roles-2-name": app_role.user.full_name, - "team_roles-2-role": CSPRole.TECHNICAL_READ.value, - } - - user_session(app_role_without_perms.user) - response = client.post( - url_for("applications.update_env_roles", environment_id=environment.id), - data=form_data, - follow_redirects=True, - ) - - assert response.status_code == 404 - assert env_role_1.role == CSPRole.BASIC_ACCESS.value - assert ( - EnvironmentRoles.get(env_role_2.user.id, environment.id).role - == CSPRole.BASIC_ACCESS.value - ) - assert not EnvironmentRoles.get(app_role.user.id, environment.id) - - def test_user_can_only_access_apps_in_their_portfolio(client, user_session): portfolio = PortfolioFactory.create() other_portfolio = PortfolioFactory.create( diff --git a/tests/test_access.py b/tests/test_access.py index 660a7c84..9839902c 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -8,12 +8,14 @@ import atst from atst.app import make_app, make_config from atst.domain.auth import UNPROTECTED_ROUTES as _NO_LOGIN_REQUIRED from atst.domain.permission_sets import PermissionSets +from atst.models.environment_role import CSPRole from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( AttachmentFactory, ApplicationFactory, ApplicationRoleFactory, + EnvironmentFactory, InvitationFactory, PortfolioFactory, PortfolioRoleFactory, @@ -168,6 +170,41 @@ def test_applications_create_access(post_url_assert_status): post_url_assert_status(rando, url, 404) +# applications.update_env_roles +def test_applications_update_team_env_roles(post_url_assert_status): + ccpo = UserFactory.create_ccpo() + owner = user_with() + app_admin = user_with() + rando = user_with() + app_member = UserFactory.create() + + portfolio = PortfolioFactory.create( + owner=owner, applications=[{"name": "mos eisley"}] + ) + application = portfolio.applications[0] + environment = EnvironmentFactory.create(application=application) + + ApplicationRoleFactory.create( + user=app_admin, + application=application, + permission_sets=PermissionSets.get_many( + [ + PermissionSets.VIEW_APPLICATION, + PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, + PermissionSets.EDIT_APPLICATION_TEAM, + PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, + ] + ), + ) + ApplicationRoleFactory.create(user=app_member) + + url = url_for("applications.update_env_roles", environment_id=environment.id) + post_url_assert_status(ccpo, url, 302) + post_url_assert_status(owner, url, 302) + post_url_assert_status(app_admin, url, 302) + post_url_assert_status(rando, url, 404) + + # portfolios.create_member def test_portfolios_create_member_access(post_url_assert_status): ccpo = user_with(PermissionSets.EDIT_PORTFOLIO_ADMIN) From 3c8115090937de4b6d3be82fbc084e22022737f7 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 25 Apr 2019 10:43:22 -0400 Subject: [PATCH 16/16] Nested form inherits from FlaskForm to avoid redundancy --- atst/forms/app_settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index d63cf4b1..44bcbc8e 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -1,10 +1,11 @@ +from flask_wtf import FlaskForm from wtforms.fields import StringField, HiddenField, RadioField, FieldList, FormField from .forms import BaseForm from .data import ENV_ROLES -class EnvMemberRoleForm(BaseForm): +class EnvMemberRoleForm(FlaskForm): name = StringField() user_id = HiddenField() role = RadioField(choices=ENV_ROLES, coerce=BaseForm.remove_empty_string)