diff --git a/atst/domain/application_roles.py b/atst/domain/application_roles.py
index 1d5366c8..177d49f0 100644
--- a/atst/domain/application_roles.py
+++ b/atst/domain/application_roles.py
@@ -57,7 +57,7 @@ class ApplicationRoles(object):
.one()
)
except NoResultFound:
- raise NotFoundError("portfolio_role")
+ raise NotFoundError("application_role")
@classmethod
def update_permission_sets(cls, application_role, new_perm_sets_names):
diff --git a/atst/domain/environments.py b/atst/domain/environments.py
index 515c7557..25f82522 100644
--- a/atst/domain/environments.py
+++ b/atst/domain/environments.py
@@ -6,7 +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 atst.domain.application_roles import ApplicationRoles
from .exceptions import NotFoundError
@@ -99,9 +99,9 @@ class Environments(object):
for member in team_roles:
new_role = member["role_name"]
- user = Users.get(member["user_id"])
+ app_role = ApplicationRoles.get_by_id(member["application_role_id"])
Environments.update_env_role(
- environment=environment, user=user, new_role=new_role
+ environment=environment, user=app_role.user, new_role=new_role
)
@classmethod
@@ -113,15 +113,6 @@ class Environments(object):
environment=environment, user=member, new_role=new_role
)
- @classmethod
- def get_members_by_role(cls, env, role):
- return (
- db.session.query(EnvironmentRole)
- .filter(EnvironmentRole.environment_id == env.id)
- .filter(EnvironmentRole.role == role)
- .all()
- )
-
@classmethod
def revoke_access(cls, environment, target_user):
EnvironmentRoles.delete(environment.id, target_user.id)
diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py
index 19801a10..805c1556 100644
--- a/atst/forms/app_settings.py
+++ b/atst/forms/app_settings.py
@@ -6,7 +6,7 @@ from .data import ENV_ROLES, ENV_ROLE_NO_ACCESS as NO_ACCESS
class MemberForm(FlaskForm):
- user_id = HiddenField()
+ application_role_id = HiddenField()
user_name = StringField()
role_name = RadioField(choices=ENV_ROLES, default=NO_ACCESS)
diff --git a/atst/models/application_role.py b/atst/models/application_role.py
index 7b343cd8..0a5ab672 100644
--- a/atst/models/application_role.py
+++ b/atst/models/application_role.py
@@ -1,12 +1,14 @@
from enum import Enum
from sqlalchemy import Index, ForeignKey, Column, Enum as SQLAEnum, Table
from sqlalchemy.dialects.postgresql import UUID
-from sqlalchemy.orm import relationship
+from sqlalchemy.orm import object_session, relationship
from sqlalchemy.event import listen
from atst.utils import first_or_none
from atst.models import Base, mixins
from atst.models.mixins.auditable import record_permission_sets_updates
+from atst.models.environment import Environment
+from atst.models.environment_role import EnvironmentRole
from .types import Id
@@ -91,6 +93,22 @@ class ApplicationRole(
"portfolio": self.application.portfolio.name,
}
+ @property
+ def environment_roles(self):
+ if getattr(self, "_environment_roles", None) is None:
+ roles = (
+ object_session(self)
+ .query(EnvironmentRole)
+ .join(Environment, Environment.application_id == self.application_id)
+ .filter(EnvironmentRole.environment_id == Environment.id)
+ .filter(EnvironmentRole.user_id == self.user_id)
+ .all()
+ )
+
+ setattr(self, "_environment_roles", roles)
+
+ return self._environment_roles
+
Index(
"application_role_user_application",
diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py
index a50e04ae..f7a0f96f 100644
--- a/atst/routes/applications/settings.py
+++ b/atst/routes/applications/settings.py
@@ -10,7 +10,6 @@ from atst.forms.application import ApplicationForm, EditEnvironmentForm
from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS
from atst.domain.authz.decorator import user_can_access_decorator as user_can
from atst.models.environment_role import CSPRole
-from atst.domain.exceptions import NotFoundError
from atst.models.permissions import Permissions
from atst.utils.flash import formatted_flash as flash
@@ -30,62 +29,54 @@ def get_environments_obj_for_app(application):
return environments_obj
-def serialize_members(member_list, role):
- serialized_list = []
-
- for member in member_list:
- serialized_list.append(
- {
- "user_id": str(member.user_id),
- "user_name": member.user.full_name,
- "role_name": role,
- }
- )
-
- return serialized_list
-
-
-def sort_env_users_by_role(env):
- users_list = []
- no_access_users = env.application.users - env.users
- no_access_list = [
- {"user_id": str(user.id), "user_name": user.full_name, "role_name": NO_ACCESS}
- for user in no_access_users
- ]
- users_list.append({"role": NO_ACCESS, "members": no_access_list})
-
- for role in CSPRole:
- users_list.append(
- {
- "role": role.value,
- "members": serialize_members(
- Environments.get_members_by_role(env, role.value), role.value
- ),
- }
- )
-
- return users_list
-
-
def data_for_app_env_roles_form(application):
- data = {"envs": []}
- for environment in application.environments:
- data["envs"].append(
- {
- "env_id": environment.id,
- "team_roles": sort_env_users_by_role(environment),
- }
- )
+ csp_roles = [role.value for role in CSPRole]
+ csp_roles.insert(0, NO_ACCESS)
+ # dictionary for sorting application members by environments
+ # and roles within those environments
+ environments_dict = {
+ e.id: {role_name: [] for role_name in csp_roles}
+ for e in application.environments
+ }
+ for member in application.members:
+ env_ids = set(environments_dict.keys())
+ for env_role in member.environment_roles:
+ role_members_list = environments_dict[env_role.environment_id][
+ env_role.role
+ ]
+ role_members_list.append(
+ {
+ "application_role_id": str(member.id),
+ "user_name": member.user_name,
+ "role_name": env_role.role,
+ }
+ )
+ env_ids.remove(env_role.environment_id)
- return data
+ # any leftover environment IDs are ones the app member
+ # does not have access to
+ for env_id in env_ids:
+ role_members_list = environments_dict[env_id][NO_ACCESS]
+ role_members_list.append(
+ {
+ "application_role_id": str(member.id),
+ "user_name": member.user_name,
+ "role_name": NO_ACCESS,
+ }
+ )
+ # transform the data into the shape the form needs
+ nested_data = [
+ {
+ "env_id": env_id,
+ "team_roles": [
+ {"role": role, "members": members} for role, members in roles.items()
+ ],
+ }
+ for env_id, roles in environments_dict.items()
+ ]
-def check_users_are_in_application(user_ids, application):
- existing_ids = [str(role.user_id) for role in application.roles]
- for user_id in user_ids:
- if not user_id in existing_ids:
- raise NotFoundError("application user", user_id)
- return True
+ return {"envs": nested_data}
def render_settings_page(application, **kwargs):
@@ -210,22 +201,10 @@ def update_env_roles(environment_id):
if form.validate():
env_data = []
- try:
- for env in form.envs.data:
- if env["env_id"] == str(environment.id):
- for role in env["team_roles"]:
- user_ids = [user["user_id"] for user in role["members"]]
- check_users_are_in_application(user_ids, application)
- env_data = env_data + role["members"]
- except NotFoundError as err:
- app.logger.warning(
- "User {} requested environment role change for unauthorized user {} in application {}.".format(
- g.current_user.id, err.resource_id, application.id
- ),
- extra={"tags": ["update", "failure"], "security_warning": True},
- )
-
- raise (err)
+ for env in form.envs.data:
+ if env["env_id"] == str(environment.id):
+ for role in env["team_roles"]:
+ env_data = env_data + role["members"]
Environments.update_env_roles_by_environment(
environment_id=environment_id, team_roles=env_data
diff --git a/js/components/__tests__/edit_environment_role.test.js b/js/components/__tests__/edit_environment_role.test.js
index 8242acb4..763db4a5 100644
--- a/js/components/__tests__/edit_environment_role.test.js
+++ b/js/components/__tests__/edit_environment_role.test.js
@@ -9,13 +9,13 @@ describe('EditEnvironmentRole', () => {
{
role: NO_ACCESS,
members: [
- { role_name: null, user_id: '123' },
- { role_name: null, user_id: '456' },
+ { role_name: null, application_role_id: '123' },
+ { role_name: null, application_role_id: '456' },
],
},
{
role: 'Basic Access',
- members: [{ role_name: 'Basic Access', user_id: '789' }],
+ members: [{ role_name: 'Basic Access', application_role_id: '789' }],
},
{
role: 'Network Admin',
@@ -24,13 +24,15 @@ describe('EditEnvironmentRole', () => {
{
role: 'Business Read-only',
members: [
- { role_name: 'Business Read-only', user_id: '012' },
- { role_name: 'Business Read-only', user_id: '345' },
+ { role_name: 'Business Read-only', application_role_id: '012' },
+ { role_name: 'Business Read-only', application_role_id: '345' },
],
},
{
role: 'Technical Read-only',
- members: [{ role_name: 'Technical Read-only', user_id: '678' }],
+ members: [
+ { role_name: 'Technical Read-only', application_role_id: '678' },
+ ],
},
]
@@ -53,7 +55,7 @@ describe('EditEnvironmentRole', () => {
expect(member_data).toEqual({
role_name: 'Technical Read-only',
- user_id: '678',
+ application_role_id: '678',
})
})
@@ -73,7 +75,7 @@ describe('EditEnvironmentRole', () => {
})
expect(techRole.members.length).toEqual(1)
- wrapper.vm.addUser({ user_id: '901' }, 'Technical Read-only')
+ wrapper.vm.addUser({ application_role_id: '901' }, 'Technical Read-only')
expect(techRole.members.length).toEqual(2)
})
diff --git a/js/components/forms/edit_environment_role.js b/js/components/forms/edit_environment_role.js
index e9081c34..c823758f 100644
--- a/js/components/forms/edit_environment_role.js
+++ b/js/components/forms/edit_environment_role.js
@@ -57,7 +57,7 @@ export const EditEnvironmentRole = {
getUserInfo: function(userId) {
for (let role of this.roleCategories) {
for (let member of role.members) {
- if (member.user_id === userId) {
+ if (member.application_role_id === userId) {
return member
}
}
@@ -67,7 +67,7 @@ export const EditEnvironmentRole = {
removeUser: function(userId) {
for (let role of this.roleCategories) {
role.members = role.members.filter(member => {
- return member.user_id !== userId
+ return member.application_role_id !== userId
})
if (!role.members) {
role.members = []
diff --git a/templates/fragments/applications/edit_environment_team_form.html b/templates/fragments/applications/edit_environment_team_form.html
index 393fa4f9..ca74234e 100644
--- a/templates/fragments/applications/edit_environment_team_form.html
+++ b/templates/fragments/applications/edit_environment_team_form.html
@@ -36,11 +36,11 @@
v-bind:class="{'unassigned': checkNoAccess(member.role_name)}">
-
+
{{ Icon('edit', classes="icon--medium") }}
+ v-bind:value='member.application_role_id'>
diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py
index 577c5b93..d50f6c24 100644
--- a/tests/domain/test_environments.py
+++ b/tests/domain/test_environments.py
@@ -67,28 +67,35 @@ def test_update_env_roles_by_environment():
env_role_1 = EnvironmentRoleFactory.create(
environment=environment, role=CSPRole.BASIC_ACCESS.value
)
+ app_role_1 = ApplicationRoleFactory.create(
+ user=env_role_1.user, application=environment.application
+ )
env_role_2 = EnvironmentRoleFactory.create(
environment=environment, role=CSPRole.NETWORK_ADMIN.value
)
+ app_role_2 = ApplicationRoleFactory.create(
+ user=env_role_2.user, application=environment.application
+ )
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)
+ app_role_3 = ApplicationRoleFactory.create(
+ user=env_role_3.user, application=environment.application
+ )
team_roles = [
{
- "user_id": env_role_1.user.id,
+ "application_role_id": app_role_1.id,
"user_name": env_role_1.user.full_name,
"role_name": CSPRole.BUSINESS_READ.value,
},
{
- "user_id": env_role_2.user.id,
+ "application_role_id": app_role_2.id,
"user_name": env_role_2.user.full_name,
"role_name": CSPRole.NETWORK_ADMIN.value,
},
{
- "user_id": env_role_3.user.id,
+ "application_role_id": app_role_3.id,
"user_name": env_role_3.user.full_name,
"role_name": None,
},
@@ -136,36 +143,6 @@ def test_update_env_roles_by_member():
assert not EnvironmentRoles.get(user.id, testing.id)
-def test_get_members_by_role(db):
- environment = EnvironmentFactory.create()
- env_role_1 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.BASIC_ACCESS.value
- )
- env_role_2 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.TECHNICAL_READ.value
- )
- env_role_3 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.TECHNICAL_READ.value
- )
- rando_env = EnvironmentFactory.create()
- rando_env_role = EnvironmentRoleFactory.create(
- environment=rando_env, role=CSPRole.BASIC_ACCESS.value
- )
-
- basic_access_members = Environments.get_members_by_role(
- environment, CSPRole.BASIC_ACCESS.value
- )
- technical_read_members = Environments.get_members_by_role(
- environment, CSPRole.TECHNICAL_READ.value
- )
- assert basic_access_members == [env_role_1]
- assert rando_env_role not in basic_access_members
- assert technical_read_members == [env_role_2, env_role_3]
- assert (
- Environments.get_members_by_role(environment, CSPRole.BUSINESS_READ.value) == []
- )
-
-
def test_get_scoped_environments(db):
developer = UserFactory.create()
portfolio = PortfolioFactory.create(
diff --git a/tests/models/test_application_role.py b/tests/models/test_application_role.py
index 5bfc102c..40216758 100644
--- a/tests/models/test_application_role.py
+++ b/tests/models/test_application_role.py
@@ -1,7 +1,7 @@
from atst.domain.permission_sets import PermissionSets
from atst.models.audit_event import AuditEvent
-from tests.factories import PortfolioFactory, UserFactory
+from tests.factories import *
def test_has_application_role_history(session):
@@ -38,3 +38,13 @@ def test_has_application_role_history(session):
old_state, new_state = changed_event.changed_state["permission_sets"]
assert old_state == [PermissionSets.VIEW_APPLICATION]
assert new_state == [PermissionSets.EDIT_APPLICATION_TEAM]
+
+
+def test_environment_roles():
+ application = ApplicationFactory.create()
+ environment = EnvironmentFactory.create(application=application)
+ user = UserFactory.create()
+ application_role = ApplicationRoleFactory.create(application=application, user=user)
+ environment_role = EnvironmentRoleFactory.create(environment=environment, user=user)
+
+ assert application_role.environment_roles == [environment_role]
diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py
index ff0b8d34..a3bbca91 100644
--- a/tests/routes/applications/test_settings.py
+++ b/tests/routes/applications/test_settings.py
@@ -10,7 +10,6 @@ from tests.factories import (
ApplicationFactory,
ApplicationRoleFactory,
)
-from atst.routes.applications.settings import check_users_are_in_application
from atst.domain.applications import Applications
from atst.domain.environment_roles import EnvironmentRoles
@@ -140,15 +139,19 @@ def test_data_for_app_env_roles_form(app, client, user_session):
{"env"},
)
env = application.environments[0]
- app_role = ApplicationRoleFactory.create(application=application)
+ app_role0 = ApplicationRoleFactory.create(application=application)
env_role1 = EnvironmentRoleFactory.create(
environment=env, role=CSPRole.BASIC_ACCESS.value
)
- ApplicationRoleFactory.create(application=application, user=env_role1.user)
+ app_role1 = ApplicationRoleFactory.create(
+ application=application, user=env_role1.user
+ )
env_role2 = EnvironmentRoleFactory.create(
environment=env, role=CSPRole.NETWORK_ADMIN.value
)
- ApplicationRoleFactory.create(application=application, user=env_role2.user)
+ app_role2 = ApplicationRoleFactory.create(
+ application=application, user=env_role2.user
+ )
user_session(portfolio.owner)
@@ -171,8 +174,8 @@ def test_data_for_app_env_roles_form(app, client, user_session):
"role": NO_ACCESS,
"members": [
{
- "user_id": str(app_role.user_id),
- "user_name": app_role.user.full_name,
+ "application_role_id": str(app_role0.id),
+ "user_name": app_role0.user.full_name,
"role_name": None,
}
],
@@ -181,7 +184,7 @@ def test_data_for_app_env_roles_form(app, client, user_session):
"role": CSPRole.BASIC_ACCESS.value,
"members": [
{
- "user_id": str(env_role1.user_id),
+ "application_role_id": str(app_role1.id),
"user_name": env_role1.user.full_name,
"role_name": CSPRole.BASIC_ACCESS.value,
}
@@ -191,7 +194,7 @@ def test_data_for_app_env_roles_form(app, client, user_session):
"role": CSPRole.NETWORK_ADMIN.value,
"members": [
{
- "user_id": str(env_role2.user_id),
+ "application_role_id": str(app_role2.id),
"user_name": env_role2.user.full_name,
"role_name": CSPRole.NETWORK_ADMIN.value,
}
@@ -260,57 +263,32 @@ def test_user_without_permission_cannot_update_application(client, user_session)
assert application.description == "Cool stuff happening here!"
-def test_check_users_are_in_application_raises_NotFoundError():
- application = ApplicationFactory.create()
- app_user_1 = UserFactory.create()
- app_user_2 = UserFactory.create()
- for user in [app_user_1, app_user_2]:
- ApplicationRoleFactory.create(user=user, application=application)
-
- non_app_user = UserFactory.create()
- user_ids = [app_user_1.id, app_user_2.id, non_app_user.id]
- with pytest.raises(NotFoundError):
- check_users_are_in_application(user_ids, application)
-
-
-def test_check_users_are_in_application():
- application = ApplicationFactory.create()
- app_user_1 = UserFactory.create()
- app_user_2 = UserFactory.create()
- app_user_3 = UserFactory.create()
-
- for user in [app_user_1, app_user_2, app_user_3]:
- ApplicationRoleFactory.create(user=user, application=application)
-
- user_ids = [str(app_user_1.id), str(app_user_2.id), str(app_user_3.id)]
- assert check_users_are_in_application(user_ids, application)
-
-
def test_update_team_env_roles(client, user_session):
environment = EnvironmentFactory.create()
application = environment.application
+ app_role_1 = ApplicationRoleFactory.create(application=application)
env_role_1 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.BASIC_ACCESS.value
+ environment=environment, role=CSPRole.BASIC_ACCESS.value, user=app_role_1.user
)
+ app_role_2 = ApplicationRoleFactory.create(application=application)
env_role_2 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.BASIC_ACCESS.value
+ environment=environment, role=CSPRole.BASIC_ACCESS.value, user=app_role_2.user
)
+ app_role_3 = ApplicationRoleFactory.create(application=application)
env_role_3 = EnvironmentRoleFactory.create(
- environment=environment, role=CSPRole.BASIC_ACCESS.value
+ environment=environment, role=CSPRole.BASIC_ACCESS.value, user=app_role_3.user
)
- 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)
+ app_role_4 = ApplicationRoleFactory.create(application=application)
form_data = {
"envs-0-env_id": environment.id,
- "envs-0-team_roles-0-members-0-user_id": app_role.user.id,
+ "envs-0-team_roles-0-members-0-application_role_id": app_role_4.id,
"envs-0-team_roles-0-members-0-role_name": CSPRole.TECHNICAL_READ.value,
- "envs-0-team_roles-1-members-0-user_id": env_role_1.user.id,
+ "envs-0-team_roles-1-members-0-application_role_id": app_role_1.id,
"envs-0-team_roles-1-members-0-role_name": CSPRole.NETWORK_ADMIN.value,
- "envs-0-team_roles-1-members-1-user_id": env_role_2.user.id,
+ "envs-0-team_roles-1-members-1-application_role_id": app_role_2.id,
"envs-0-team_roles-1-members-1-role_name": CSPRole.BASIC_ACCESS.value,
- "envs-0-team_roles-1-members-2-user_id": env_role_3.user.id,
+ "envs-0-team_roles-1-members-2-application_role_id": app_role_3.id,
"envs-0-team_roles-1-members-2-role_name": NO_ACCESS,
}
@@ -325,7 +303,7 @@ def test_update_team_env_roles(client, user_session):
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)
+ assert EnvironmentRoles.get(app_role_4.user.id, environment.id)
def test_user_can_only_access_apps_in_their_portfolio(client, user_session):