pk/atst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Simplify environment role updates in app settings.

Browse Source 
Use ApplicationRole.id instead of User.id in forms. This eliminates the
need for the function that checks whether a user is in a given
application, because looking up the application role will raise an error
if the user is not.
This commit is contained in:
dandds 2019-05-24 12:05:44 -04:00
parent 129f5e3031
commit 43ea922218
9 changed files with 64 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
atst/domain/application_roles.py
View File

@ -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):

6
atst/domain/environments.py
View File

@ -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

2
atst/forms/app_settings.py
View File

@ -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)

33
atst/routes/applications/settings.py
View File

@ -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
@ -47,7 +46,7 @@ def data_for_app_env_roles_form(application):
]
role_members_list.append(
{
"user_id": str(member.user.id),
"application_role_id": str(member.id),
"user_name": member.user_name,
"role_name": env_role.role,
}
@ -60,7 +59,7 @@ def data_for_app_env_roles_form(application):
role_members_list = environments_dict[env_id][NO_ACCESS]
role_members_list.append(
{
"user_id": str(member.user.id),
"application_role_id": str(member.id),
"user_name": member.user_name,
"role_name": NO_ACCESS,
}
@ -80,14 +79,6 @@ def data_for_app_env_roles_form(application):
return {"envs": nested_data}
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
def render_settings_page(application, **kwargs):
environments_obj = get_environments_obj_for_app(application=application)
members_form = AppEnvRolesForm(data=data_for_app_env_roles_form(application))
@ -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

18
js/components/__tests__/edit_environment_role.test.js
View File

@ -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)
})

4
js/components/forms/edit_environment_role.js
View File

@ -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 = []

12
templates/fragments/applications/edit_environment_team_form.html
View File

@ -36,11 +36,11 @@
v-bind:class="{'unassigned': checkNoAccess(member.role_name)}">
<span v-html='member.user_name'>
</span>
<span v-on:click="toggleSection(member.user_id)" class="icon-link right">
<span v-on:click="toggleSection(member.application_role_id)" class="icon-link right">
{{ Icon('edit', classes="icon--medium") }}
</span>
<div
v-show="selectedSection === member.user_id"
v-show="selectedSection === member.application_role_id"
class='environment-role__user-field'>
<div class="usa-input">
<fieldset
@ -56,7 +56,7 @@
v-bind:name="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-role_name'"
v-bind:id="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-role_name-' + roleinputindex"
type="radio"
v-bind:user-id='member.user_id'
v-bind:user-id='member.application_role_id'
v-bind:value='roleCategory.role'>
<label
v-bind:for="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-role_name-' + roleinputindex">
@ -71,10 +71,10 @@
</div>
</div>
<input
v-bind:id="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-user_id'"
v-bind:name="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-user_id'"
v-bind:id="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-application_role_id'"
v-bind:name="'envs-{{ loop.index0 }}-team_roles-' + roleindex + '-members-' + memberindex + '-application_role_id'"
type="hidden"
v-bind:value='member.user_id'>
v-bind:value='member.application_role_id'>
</li>
</ul>
</div>

17
tests/domain/test_environments.py
View File

@ -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,
},

68
tests/routes/applications/test_settings.py
View File

@ -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):