From a332d1432edeffda47904e529b3fd00983076eb9 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 May 2019 09:51:25 -0400 Subject: [PATCH] Use application_role.id to reference users in team page forms. Membership in a resource should be decoupled from the users table. --- atst/domain/application_roles.py | 12 +++++++++ atst/forms/team.py | 2 +- atst/routes/applications/team.py | 11 +++----- .../fragments/applications/edit_team.html | 2 +- tests/domain/test_application_roles.py | 12 +++++++++ tests/routes/applications/test_team.py | 27 +++++++------------ 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/atst/domain/application_roles.py b/atst/domain/application_roles.py index c0e8e79b..1d5366c8 100644 --- a/atst/domain/application_roles.py +++ b/atst/domain/application_roles.py @@ -47,6 +47,18 @@ class ApplicationRoles(object): return app_role + @classmethod + def get_by_id(cls, id_): + try: + return ( + db.session.query(ApplicationRole) + .filter(ApplicationRole.id == id_) + .filter(ApplicationRole.status != ApplicationRoleStatus.DISABLED) + .one() + ) + except NoResultFound: + raise NotFoundError("portfolio_role") + @classmethod def update_permission_sets(cls, application_role, new_perm_sets_names): application_role.permission_sets = ApplicationRoles._permission_sets_for_names( diff --git a/atst/forms/team.py b/atst/forms/team.py index 29e46874..66fb549c 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -61,7 +61,7 @@ class PermissionsForm(FlaskForm): class MemberForm(FlaskForm): - user_id = HiddenField(validators=[Required()]) + role_id = HiddenField(validators=[Required()]) user_name = StringField() environment_roles = FieldList(FormField(EnvironmentForm)) permission_sets = FormField(PermissionsForm) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 3aa988e4..0b0eefb6 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -27,8 +27,6 @@ def get_form_permission_value(member, edit_perm_set): def get_team_form(application): team_data = [] for member in application.members: - user_id = member.user.id - user_name = member.user.full_name permission_sets = { "perms_team_mgmt": get_form_permission_value( member, PermissionSets.EDIT_APPLICATION_TEAM @@ -53,8 +51,8 @@ def get_team_form(application): ] team_data.append( { - "user_id": str(user_id), - "user_name": user_name, + "role_id": member.id, + "user_name": member.user_name, "permission_sets": permission_sets, "environment_roles": environment_roles, } @@ -99,7 +97,7 @@ def update_team(application_id): if form.validate(): for member_form in form.members: - app_role = ApplicationRoles.get(member_form.user_id.data, application.id) + app_role = ApplicationRoles.get_by_id(member_form.role_id.data) new_perms = [ perm for perm in member_form.data["permission_sets"] @@ -108,12 +106,11 @@ def update_team(application_id): ApplicationRoles.update_permission_sets(app_role, new_perms) for environment_role_form in member_form.environment_roles: - user = Users.get(member_form.user_id.data) environment = Environments.get( environment_role_form.environment_id.data ) Environments.update_env_role( - environment, user, environment_role_form.data.get("role") + environment, app_role.user, environment_role_form.data.get("role") ) flash("updated_application_team_settings", application_name=application.name) diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index 844e22dc..8139e12b 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -92,7 +92,7 @@ {% endif %} {% endcall %} - {{ member_form.user_id() }} + {{ member_form.role_id() }} {% endfor %} diff --git a/tests/domain/test_application_roles.py b/tests/domain/test_application_roles.py index b26dd13e..d6f2db69 100644 --- a/tests/domain/test_application_roles.py +++ b/tests/domain/test_application_roles.py @@ -71,3 +71,15 @@ def test_update_permission_sets(): assert app_role.permission_sets == view_app assert ApplicationRoles.update_permission_sets(app_role, new_perms_names) assert set(app_role.permission_sets) == set(new_perms + view_app) + + +def test_get_by_id(): + user = UserFactory.create() + application = ApplicationFactory.create() + app_role = ApplicationRoleFactory.create(user=user, application=application) + + assert ApplicationRoles.get_by_id(app_role.id) == app_role + app_role.status = ApplicationRoleStatus.DISABLED + + with pytest.raises(NotFoundError): + ApplicationRoles.get_by_id(app_role.id) diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index 99c7da78..eae6706d 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -25,12 +25,11 @@ def test_update_team_permissions(client, user_session): app_role = ApplicationRoleFactory.create( application=application, permission_sets=[] ) - app_user = app_role.user user_session(owner) response = client.post( url_for("applications.update_team", application_id=application.id), data={ - "members-0-user_id": app_user.id, + "members-0-role_id": app_role.id, "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, @@ -54,36 +53,30 @@ def test_update_team_with_bad_permission_sets(client, user_session): app_role = ApplicationRoleFactory.create( application=application, permission_sets=[] ) - app_user = app_role.user - permission_sets = app_user.permission_sets + permission_sets = app_role.permission_sets user_session(owner) response = client.post( url_for("applications.update_team", application_id=application.id), data={ - "members-0-user_id": app_user.id, + "members-0-role_id": app_role.id, "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, "members-0-permission_sets-perms_env_mgmt": "some random string", }, ) assert response.status_code == 400 - assert app_user.permission_sets == permission_sets + assert app_role.permission_sets == permission_sets def test_update_team_with_non_app_user(client, user_session): application = ApplicationFactory.create() owner = application.portfolio.owner - app_role = ApplicationRoleFactory.create( - application=application, permission_sets=[] - ) - non_app_user = UserFactory.create() - app_user = app_role.user user_session(owner) response = client.post( url_for("applications.update_team", application_id=application.id), data={ - "members-0-user_id": non_app_user.id, + "members-0-role_id": str(uuid.uuid4()), "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, @@ -99,16 +92,15 @@ def test_update_team_environment_roles(client, user_session): app_role = ApplicationRoleFactory.create( application=application, permission_sets=[] ) - app_user = app_role.user environment = EnvironmentFactory.create(application=application) env_role = EnvironmentRoleFactory.create( - user=app_user, environment=environment, role=CSPRole.NETWORK_ADMIN.value + user=app_role.user, environment=environment, role=CSPRole.NETWORK_ADMIN.value ) user_session(owner) response = client.post( url_for("applications.update_team", application_id=application.id), data={ - "members-0-user_id": app_user.id, + "members-0-role_id": app_role.id, "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, @@ -127,16 +119,15 @@ def test_update_team_revoke_environment_access(client, user_session, db, session app_role = ApplicationRoleFactory.create( application=application, permission_sets=[] ) - app_user = app_role.user environment = EnvironmentFactory.create(application=application) env_role = EnvironmentRoleFactory.create( - user=app_user, environment=environment, role=CSPRole.BASIC_ACCESS.value + user=app_role.user, environment=environment, role=CSPRole.BASIC_ACCESS.value ) user_session(owner) response = client.post( url_for("applications.update_team", application_id=application.id), data={ - "members-0-user_id": app_user.id, + "members-0-role_id": app_role.id, "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS,