From 6822680bc831c60fc9d9b322a681e7c26acd4b73 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 23 Apr 2019 16:37:29 -0400 Subject: [PATCH] Ensure that a member is an application member before adding the user to an environment --- atst/domain/environment_roles.py | 11 +++---- atst/domain/environments.py | 35 ++++++++++++---------- tests/domain/test_environments.py | 10 +++++++ tests/models/test_portfolio_role.py | 2 ++ tests/routes/applications/test_settings.py | 3 ++ 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index 99728467..a34aa233 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -7,11 +7,12 @@ from atst.models import EnvironmentRole class EnvironmentRoles(object): @classmethod def create(cls, user, environment, role): - env_role = EnvironmentRole(user=user, environment=environment, role=role) - if not user.cloud_id: - user.cloud_id = app.csp.cloud.create_user(user) - app.csp.cloud.create_role(env_role) - return env_role + if user.is_app_member(environment.application): + env_role = EnvironmentRole(user=user, environment=environment, role=role) + if not user.cloud_id: + user.cloud_id = app.csp.cloud.create_user(user) + app.csp.cloud.create_role(env_role) + return env_role @classmethod def get(cls, user_id, environment_id): diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 44eaad9a..51a29321 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -5,6 +5,7 @@ from atst.database import db from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole from atst.models.application import Application +from atst.domain.application_roles import ApplicationRoles from atst.domain.environment_roles import EnvironmentRoles from atst.domain.users import Users @@ -32,6 +33,7 @@ class Environments(object): @classmethod def add_member(cls, environment, user, role): + ApplicationRoles.create(user=user, application=environment.application) environment_user = EnvironmentRoles.create( user=user, environment=environment, role=role ) @@ -66,23 +68,24 @@ class Environments(object): def update_env_role(cls, environment, user, new_role): updated = False - if new_role is None: - 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: - 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 - ) - updated = True - db.session.add(env_role) + if user.is_app_member(environment.application): + if new_role is None: + 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: + 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 + ) + updated = True + db.session.add(env_role) - if updated: - db.session.commit() + if updated: + db.session.commit() return updated diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index af8bb2a4..b6d15ca1 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -11,6 +11,7 @@ from tests.factories import ( PortfolioFactory, EnvironmentFactory, EnvironmentRoleFactory, + ApplicationRoleFactory, ) @@ -24,6 +25,9 @@ def test_create_environments(): def test_update_env_role(): env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) new_role = CSPRole.TECHNICAL_READ.value + ApplicationRoleFactory.create( + user=env_role.user, application=env_role.environment.application + ) assert Environments.update_env_role(env_role.environment, env_role.user, new_role) assert env_role.role == new_role @@ -31,6 +35,9 @@ def test_update_env_role(): def test_update_env_role_no_access(): env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) + ApplicationRoleFactory.create( + user=env_role.user, application=env_role.environment.application + ) assert Environments.update_env_role(env_role.environment, env_role.user, None) assert not EnvironmentRoles.get(env_role.user.id, env_role.environment.id) @@ -48,6 +55,7 @@ def test_update_env_role_no_change(): def test_update_env_role_creates_cloud_id_for_new_member(session): user = UserFactory.create() env = EnvironmentFactory.create() + ApplicationRoleFactory.create(user=user, application=env.application) assert not user.cloud_id assert Environments.update_env_role(env, user, CSPRole.TECHNICAL_READ.value) assert EnvironmentRoles.get(user.id, env.id) @@ -65,6 +73,8 @@ def test_update_env_roles_by_environment(): env_role_3 = EnvironmentRoleFactory.create( environment=environment, role=CSPRole.TECHNICAL_READ.value ) + for user in [env_role_1.user, env_role_2.user, env_role_3.user]: + ApplicationRoleFactory.create(user=user, application=environment.application) team_roles = [ { diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 4b5e6601..a0af25e5 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -17,6 +17,7 @@ from tests.factories import ( EnvironmentFactory, EnvironmentRoleFactory, ApplicationFactory, + ApplicationRoleFactory, PortfolioFactory, ) from atst.domain.portfolio_roles import PortfolioRoles @@ -116,6 +117,7 @@ def test_has_env_role_history(session): portfolio = PortfolioFactory.create(owner=owner) portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=user) application = ApplicationFactory.create(portfolio=portfolio) + ApplicationRoleFactory.create(user=user, application=application) environment = EnvironmentFactory.create( application=application, name="new environment!" ) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 2b9ec47e..88e97c84 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -178,6 +178,9 @@ def test_update_team_env_roles(client, user_session): env_role_3 = EnvironmentRoleFactory.create( environment=environment, role=CSPRole.BASIC_ACCESS.value ) + for user in [env_role_1.user, env_role_2.user, env_role_3.user]: + ApplicationRoleFactory.create(user=user, application=application) + app_role = ApplicationRoleFactory.create(application=application) form_data = { "env_id": environment.id,