From 31bcb662b5ce1ca0d57a8e9bd1cf7e134d3c5439 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 7 Mar 2019 06:22:23 -0500 Subject: [PATCH 01/20] add join table between portfolio_roles and roles --- ...6_add_permission_sets_to_portfolio_role.py | 33 +++++++++++++++++++ atst/models/portfolio_role.py | 12 ++++++- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/938a31795096_add_permission_sets_to_portfolio_role.py diff --git a/alembic/versions/938a31795096_add_permission_sets_to_portfolio_role.py b/alembic/versions/938a31795096_add_permission_sets_to_portfolio_role.py new file mode 100644 index 00000000..f31042d1 --- /dev/null +++ b/alembic/versions/938a31795096_add_permission_sets_to_portfolio_role.py @@ -0,0 +1,33 @@ +"""add permission sets to portfolio_role + +Revision ID: 938a31795096 +Revises: db161adbafdf +Create Date: 2019-03-07 06:13:05.400911 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '938a31795096' +down_revision = 'db161adbafdf' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('portfolio_roles_roles', + sa.Column('portfolio_role_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('role_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.ForeignKeyConstraint(['portfolio_role_id'], ['portfolio_roles.id'], ), + sa.ForeignKeyConstraint(['role_id'], ['roles.id'], ) + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('portfolio_roles_roles') + # ### end Alembic commands ### diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index b41ae322..57e39c5b 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -1,5 +1,5 @@ from enum import Enum -from sqlalchemy import Index, ForeignKey, Column, Enum as SQLAEnum +from sqlalchemy import Index, ForeignKey, Column, Enum as SQLAEnum, Table from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import relationship @@ -30,6 +30,14 @@ class Status(Enum): PENDING = "pending" +portfolio_roles_roles = Table( + "portfolio_roles_roles", + Base.metadata, + Column("portfolio_role_id", UUID(as_uuid=True), ForeignKey("portfolio_roles.id")), + Column("role_id", UUID(as_uuid=True), ForeignKey("roles.id")), +) + + class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): __tablename__ = "portfolio_roles" @@ -48,6 +56,8 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING) + permission_sets = relationship("Role", secondary=portfolio_roles_roles) + def __repr__(self): return "".format( self.role.name, self.portfolio.name, self.user_id, self.id From 798eb7e1b0b382cbe874eca511d35d524535701f Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 7 Mar 2019 09:35:30 -0500 Subject: [PATCH 02/20] first cut of portfolio permission sets --- atst/domain/roles.py | 84 ++++++++++++++++++++++++++++++++++++++ atst/models/permissions.py | 41 ++++++++++++++++++- script/seed_roles.py | 4 +- 3 files changed, 126 insertions(+), 3 deletions(-) diff --git a/atst/domain/roles.py b/atst/domain/roles.py index 12d9a6c6..49a78135 100644 --- a/atst/domain/roles.py +++ b/atst/domain/roles.py @@ -161,6 +161,90 @@ PORTFOLIO_ROLES = [ }, ] +PORTFOLIO_PERMISSION_SETS = [ + { + "name": "view_portfolio_application_management", + "description": "View applications and related resources", + "display_name": "Application Management", + "permissions": [ + Permissions.VIEW_APPLICATION, + Permissions.VIEW_APPLICATION_MEMBER, + Permissions.VIEW_ENVIRONMENT, + ], + }, + { + "name": "edit_portfolio_application_management", + "description": "Edit applications and related resources", + "display_name": "Application Management", + "permissions": [ + Permissions.EDIT_APPLICATION, + Permissions.CREATE_APPLICATION, + Permissions.EDIT_APPLICATION_MEMBER, + Permissions.CREATE_APPLICATION_MEMBER, + Permissions.EDIT_ENVIRONMENT, + Permissions.CREATE_ENVIRONMENT, + ], + }, + { + "name": "view_portfolio_funding", + "description": "View a portfolio's task orders", + "display_name": "Funding", + "permissions": [ + Permissions.VIEW_PORTFOLIO_FUNDING, + Permissions.VIEW_TASK_ORDER_DETAILS, + ], + }, + { + "name": "edit_portfolio_funding", + "description": "Edit a portfolio's task orders and add new ones", + "display_name": "Funding", + "permissions": [ + Permissions.CREATE_TASK_ORDER, + Permissions.EDIT_TASK_ORDER_DETAILS, + ], + }, + { + "name": "view_portfolio_reports", + "description": "View a portfolio's reports", + "display_name": "Reporting", + "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], + }, + { + "name": "edit_portfolio_reports", + "description": "Edit a portfolio's reports (no-op)", + "display_name": "Reporting", + "permissions": [], + }, + { + "name": "view_portfolio_admin", + "description": "View a portfolio's admin options", + "display_name": "Portfolio Administration", + "permissions": [ + Permissions.VIEW_PORTFOLIO_ADMIN, + Permissions.VIEW_PORTFOLIO_NAME, + Permissions.VIEW_PORTFOLIO_USERS, + Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, + Permissions.VIEW_PORTFOLIO_POC, + ], + }, + { + "name": "edit_portfolio_admin", + "description": "Edit a portfolio's admin options", + "display_name": "Portfolio Administration", + "permissions": [ + Permissions.EDIT_PORTFOLIO_NAME, + Permissions.EDIT_PORTFOLIO_USERS, + Permissions.CREATE_PORTFOLIO_USERS, + ], + }, + { + "name": "portfolio_poc", + "description": "Permissions belonging to the Portfolio POC", + "display_name": "Portfolio Point of Contact", + "permissions": [Permissions.EDIT_PORTFOLIO_POC, Permissions.ARCHIVE_PORTFOLIO], + }, +] + class Roles(object): @classmethod diff --git a/atst/models/permissions.py b/atst/models/permissions.py index 77d8ffa7..95c504f1 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -27,7 +27,6 @@ class Permissions(object): VIEW_ATAT_PERMISSIONS = "view_atat_permissions" TRANSFER_OWNERSHIP_OF_PORTFOLIO = "transfer_ownership_of_portfolio" VIEW_PORTFOLIO_MEMBERS = "view_portfolio_members" - VIEW_PORTFOLIO = "view_portfolio" ADD_APPLICATION_IN_PORTFOLIO = "add_application_in_portfolio" DELETE_APPLICATION_IN_PORTFOLIO = "delete_application_in_portfolio" @@ -47,3 +46,43 @@ class Permissions(object): VIEW_TASK_ORDER = "view_task_order" UPDATE_TASK_ORDER = "update_task_order" ADD_TASK_ORDER_OFFICER = "add_task_order_officers" + + # new portfolio permissions + # base portfolio perms + VIEW_PORTFOLIO = "view_portfolio" + + # application management + VIEW_APPLICATION = "view_application" + EDIT_APPLICATION = "edit_application" + CREATE_APPLICATION = "create_application" + VIEW_APPLICATION_MEMBER = "view_application_member" + EDIT_APPLICATION_MEMBER = "edit_application_member" + CREATE_APPLICATION_MEMBER = "create_application_member" + VIEW_ENVIRONMENT = "view_environment" + EDIT_ENVIRONMENT = "edit_environment" + CREATE_ENVIRONMENT = "create_environment" + + # funding + VIEW_PORTFOLIO_FUNDING = "view_portfolio_funding" # TO summary page + CREATE_TASK_ORDER = "create_task_order" # create a new TO + VIEW_TASK_ORDER_DETAILS = "view_task_order_details" # individual TO page + EDIT_TASK_ORDER_DETAILS = ( + "edit_task_order_details" + ) # edit TO that has not been finalized + + # reporting + VIEW_PORTFOLIO_REPORTS = "view_portfolio_reports" + + # portfolio admin + VIEW_PORTFOLIO_ADMIN = "view_portfolio_admin" + VIEW_PORTFOLIO_NAME = "view_portfolio_name" + EDIT_PORTFOLIO_NAME = "edit_portfolio_name" + VIEW_PORTFOLIO_USERS = "view_portfolio_users" + EDIT_PORTFOLIO_USERS = "edit_portfolio_users" + CREATE_PORTFOLIO_USERS = "create_portfolio_users" + VIEW_PORTFOLIO_ACTIVITY_LOG = "view_portfolio_activity_log" + VIEW_PORTFOLIO_POC = "view_portfolio_poc" + + # portfolio POC + EDIT_PORTFOLIO_POC = "edit_portfolio_poc" + ARCHIVE_PORTFOLIO = "archive_portfolio" diff --git a/script/seed_roles.py b/script/seed_roles.py index 00b96920..5e1cb4f1 100755 --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -10,11 +10,11 @@ from sqlalchemy.orm.exc import NoResultFound from atst.app import make_config, make_app from atst.database import db from atst.models import Role, Permissions -from atst.domain.roles import ATAT_ROLES, PORTFOLIO_ROLES +from atst.domain.roles import ATAT_ROLES, PORTFOLIO_ROLES, PORTFOLIO_PERMISSION_SETS def seed_roles(): - for role_info in ATAT_ROLES + PORTFOLIO_ROLES: + for role_info in ATAT_ROLES + PORTFOLIO_ROLES + PORTFOLIO_PERMISSION_SETS: role = Role(**role_info) try: existing_role = db.session.query(Role).filter_by(name=role.name).one() From 8af3986a2d77cd5d94c4654045c3716dbeb54bee Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 7 Mar 2019 20:11:39 -0500 Subject: [PATCH 03/20] set portfolio member permission sets --- atst/domain/portfolio_roles.py | 21 ++++++++++- atst/domain/portfolios/portfolios.py | 11 ++++-- atst/forms/new_member.py | 39 +++++++++++++++++++++ templates/portfolios/members/new.html | 25 ++++++++++++- tests/domain/test_portfolio_roles.py | 19 ++++++++++ tests/routes/portfolios/test_invitations.py | 9 ++++- tests/routes/portfolios/test_members.py | 6 ++++ translations.yaml | 6 ++++ 8 files changed, 130 insertions(+), 6 deletions(-) diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index e9e88574..8d885455 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -80,7 +80,7 @@ class PortfolioRoles(object): raise NotFoundError("portfolio role") @classmethod - def add(cls, user, portfolio_id, role_name): + def add(cls, user, portfolio_id, role_name, permission_sets=None): role = Roles.get(role_name) new_portfolio_role = None @@ -103,12 +103,31 @@ class PortfolioRoles(object): status=PortfolioRoleStatus.PENDING, ) + if permission_sets: + new_portfolio_role.permission_sets = PortfolioRoles._permission_sets_for_names( + permission_sets + ) + user.portfolio_roles.append(new_portfolio_role) db.session.add(user) db.session.commit() return new_portfolio_role + _DEFAULT_PORTFOLIO_PERMS_SETS = { + "view_portfolio_application_management", + "view_portfolio_funding", + "view_portfolio_reports", + "view_portfolio_admin", + } + + @classmethod + def _permission_sets_for_names(cls, set_names): + perms_set_names = PortfolioRoles._DEFAULT_PORTFOLIO_PERMS_SETS.union( + set(set_names) + ) + return [Roles.get(perms_set_name) for perms_set_name in perms_set_names] + @classmethod def update_role(cls, portfolio_role, role_name): new_role = Roles.get(role_name) diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 2e5282e2..342e3b74 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -105,11 +105,16 @@ class Portfolios(object): atat_role_name="default", provisional=True, ) - return Portfolios.add_member(portfolio, new_user, data["portfolio_role"]) + permission_sets = data.get("permission_sets", []) + return Portfolios.add_member( + portfolio, new_user, data["portfolio_role"], permission_sets=permission_sets + ) @classmethod - def add_member(cls, portfolio, member, role_name): - portfolio_role = PortfolioRoles.add(member, portfolio.id, role_name) + def add_member(cls, portfolio, member, role_name, permission_sets=None): + portfolio_role = PortfolioRoles.add( + member, portfolio.id, role_name, permission_sets + ) return portfolio_role @classmethod diff --git a/atst/forms/new_member.py b/atst/forms/new_member.py index cdea0c56..88d6ec21 100644 --- a/atst/forms/new_member.py +++ b/atst/forms/new_member.py @@ -32,3 +32,42 @@ class NewMemberForm(BaseForm): default="", description=translate("forms.new_member.portfolio_role_description"), ) + + perms_app_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_application_management", "View Only"), + ("edit_portfolio_application_management", "Edit Access"), + ], + ) + perms_funding = SelectField( + None, + choices=[ + ("view_portfolio_funding", "View Only"), + ("edit_portfolio_funding", "Edit Access"), + ], + ) + perms_reporting = SelectField( + None, + choices=[ + ("view_portfolio_reports", "View Only"), + ("edit_portfolio_reports", "Edit Access"), + ], + ) + perms_portfolio_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_admin", "View Only"), + ("edit_portfolio_admin", "Edit Access"), + ], + ) + + @property + def data(self): + _data = super().data + _data["permission_sets"] = [] + for field in _data: + if "perms" in field: + _data["permission_sets"].append(_data[field]) + + return _data diff --git a/templates/portfolios/members/new.html b/templates/portfolios/members/new.html index 3603bd62..58f8cf67 100644 --- a/templates/portfolios/members/new.html +++ b/templates/portfolios/members/new.html @@ -23,10 +23,33 @@ {{ TextInput(form.email,placeholder='jane@mail.mil', validation='email') }} {{ TextInput(form.dod_id,placeholder='10-digit number on the back of the CAC', validation='dodId') }} {{ Selector(form.portfolio_role) }} + + + + + + + + + + + + + + + +
{{ "portfolios.members.permissions.app_mgmt" | translate }}{{ "portfolios.members.permissions.funding" | translate }}{{ "portfolios.members.permissions.reporting" | translate }}{{ "portfolios.members.permissions.portfolio_mgmt" | translate }}
+ {{ form.perms_app_mgmt() }} + + {{ form.perms_funding() }} + + {{ form.perms_reporting() }} + + {{ form.perms_portfolio_mgmt() }} +
-
diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 6d28b82c..3f2d9af6 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -63,3 +63,22 @@ def test_portfolio_role_permissions(): PortfolioRoles.portfolio_role_permissions(portfolio_two, new_user) == default_perms ) + + +def test_add_portfolio_role_with_permission_sets(): + portfolio = PortfolioFactory.create() + new_user = UserFactory.create() + permission_sets = ["edit_portfolio_application_management"] + port_role = PortfolioRoles.add( + new_user, portfolio.id, "developer", permission_sets=permission_sets + ) + assert len(port_role.permission_sets) == 5 + expected_names = [ + "edit_portfolio_application_management", + "view_portfolio_application_management", + "view_portfolio_funding", + "view_portfolio_reports", + "view_portfolio_admin", + ] + actual_names = [prms.name for prms in port_role.permission_sets] + assert expected_names == expected_names diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index 4e382127..a4d89144 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -47,7 +47,14 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): user_session(portfolio.owner) client.post( url_for("portfolios.create_member", portfolio_id=portfolio.id), - data={"portfolio_role": "developer", **user_info}, + data={ + "portfolio_role": "developer", + "perms_app_mgmt": "view_portfolio_application_management", + "perms_funding": "view_portfolio_funding", + "perms_reporting": "view_portfolio_reports", + "perms_portfolio_mgmt": "view_portfolio_admin", + **user_info, + }, ) user = Users.get_by_dod_id(user_info["dod_id"]) diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 0deb6a06..6932c174 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -85,6 +85,10 @@ def test_create_member(client, user_session): "last_name": "Zuckerman", "email": "some_pig@zuckermans.com", "portfolio_role": "developer", + "perms_app_mgmt": "view_portfolio_application_management", + "perms_funding": "view_portfolio_funding", + "perms_reporting": "view_portfolio_reports", + "perms_portfolio_mgmt": "view_portfolio_admin", }, follow_redirects=True, ) @@ -94,6 +98,8 @@ def test_create_member(client, user_session): assert user.has_portfolios assert user.invitations assert len(queue.get_queue()) == queue_length + 1 + portfolio_role = user.portfolio_roles[0] + assert len(portfolio_role.permission_sets) == 4 def test_view_member_shows_role(client, user_session): diff --git a/translations.yaml b/translations.yaml index 535e8f82..28c6c951 100644 --- a/translations.yaml +++ b/translations.yaml @@ -558,6 +558,12 @@ portfolios: subheading: Team Management admin: activity_log_title: Activity Log + members: + permissions: + app_mgmt: App Mgmt + funding: Funding + reporting: Reporting + portfolio_mgmt: Portfolio Mgmt testing: example_string: Hello World example_with_variables: 'Hello, {name}!' From 6805041b138e50164dd12e6cdf41020672348309 Mon Sep 17 00:00:00 2001 From: dandds Date: Sun, 10 Mar 2019 15:21:52 -0400 Subject: [PATCH 04/20] portfolio role knows its entire permissions list --- atst/models/portfolio_role.py | 6 ++++++ tests/models/test_portfolio_role.py | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index 57e39c5b..3be7c8f2 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -58,6 +58,12 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): permission_sets = relationship("Role", secondary=portfolio_roles_roles) + @property + def permissions(self): + return [ + perm for permset in self.permission_sets for perm in permset.permissions + ] + def __repr__(self): return "".format( self.role.name, self.portfolio.name, self.user_id, self.id diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 06db96e3..25fec901 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -3,6 +3,7 @@ import datetime from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.domain.applications import Applications +from atst.domain.roles import Roles from atst.models.portfolio_role import Status from atst.models.role import Role from atst.models.invitation import Status as InvitationStatus @@ -298,3 +299,11 @@ def test_can_list_all_environments(): ) assert len(portfolio.all_environments) == 9 + + +def test_can_list_all_permissions(): + role_one = Roles.get("view_portfolio_funding") + role_two = Roles.get("view_portfolio_reports") + port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) + expected_perms = role_one.permissions + role_two.permissions + assert expected_perms == expected_perms From 44a4d989782c8d18aab06d4981f236fbaae6926d Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 11 Mar 2019 17:25:35 -0400 Subject: [PATCH 05/20] switch portfolio authorization to rely on new permission sets --- atst/domain/applications.py | 4 +- atst/domain/audit_log.py | 3 +- atst/domain/authz.py | 11 ++- atst/domain/environments.py | 4 +- atst/domain/invitations.py | 2 +- atst/domain/portfolios/portfolios.py | 48 +++++----- atst/domain/portfolios/scopes.py | 6 +- atst/domain/roles.py | 87 +++++++++++-------- atst/domain/task_orders.py | 16 ++-- atst/routes/portfolios/index.py | 2 +- atst/routes/portfolios/members.py | 4 +- templates/portfolios/applications/index.html | 6 +- .../portfolios/applications/members.html | 2 +- templates/portfolios/header.html | 4 +- templates/portfolios/members/index.html | 2 +- templates/portfolios/reports/index.html | 2 +- tests/domain/test_audit_log.py | 3 + tests/domain/test_authz.py | 20 ++++- tests/domain/test_portfolios.py | 18 ++-- tests/domain/test_task_orders.py | 21 +++-- tests/factories.py | 28 +++++- tests/routes/portfolios/test_members.py | 23 ++--- 22 files changed, 204 insertions(+), 112 deletions(-) diff --git a/atst/domain/applications.py b/atst/domain/applications.py index bdd80a96..243ed733 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -27,7 +27,7 @@ class Applications(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.VIEW_APPLICATION_IN_PORTFOLIO, + Permissions.VIEW_APPLICATION, "view application in portfolio", ) @@ -56,7 +56,7 @@ class Applications(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.VIEW_APPLICATION_IN_PORTFOLIO, + Permissions.VIEW_APPLICATION, "view application in portfolio", ) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 78392cd5..c33be753 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -36,6 +36,7 @@ class AuditLog(object): @classmethod def get_all_events(cls, user, pagination_opts=None): + # TODO: general audit log permissions Authorization.check_atat_permission( user, Permissions.VIEW_AUDIT_LOG, "view audit log" ) @@ -46,7 +47,7 @@ class AuditLog(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.VIEW_PORTFOLIO_AUDIT_LOG, + Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, "view portfolio audit log", ) return AuditEventQuery.get_ws_events(portfolio.id, pagination_opts) diff --git a/atst/domain/authz.py b/atst/domain/authz.py index 7075a48b..6fb27924 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -1,4 +1,3 @@ -from atst.domain.portfolio_roles import PortfolioRoles from atst.models.permissions import Permissions from atst.domain.exceptions import UnauthorizedError @@ -6,9 +5,13 @@ from atst.domain.exceptions import UnauthorizedError class Authorization(object): @classmethod def has_portfolio_permission(cls, user, portfolio, permission): - return permission in PortfolioRoles.portfolio_role_permissions( - portfolio, user - ) or Authorization.is_ccpo(user) + port_role = next( + (pr for pr in user.portfolio_roles if pr.portfolio == portfolio), None + ) + if port_role: + return permission in port_role.permissions + else: + return False @classmethod def has_atat_permission(cls, user, permission): diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 2164318b..d667ebb0 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -64,7 +64,7 @@ class Environments(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.ADD_AND_ASSIGN_CSP_ROLES, + Permissions.EDIT_APPLICATION_MEMBER, "assign environment roles", ) updated = False @@ -104,7 +104,7 @@ class Environments(object): Authorization.check_portfolio_permission( user, environment.portfolio, - Permissions.REMOVE_CSP_ROLES, + Permissions.EDIT_APPLICATION_MEMBER, "revoke environment access", ) EnvironmentRoles.delete(environment.id, target_user.id) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 06dab623..3fcee7eb 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -119,7 +119,7 @@ class Invitations(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + Permissions.CREATE_PORTFOLIO_USERS, "resend a portfolio invitation", ) diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 342e3b74..def155d3 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -1,4 +1,4 @@ -from atst.domain.roles import Roles +from atst.domain.roles import Roles, PORTFOLIO_PERMISSION_SETS from atst.domain.authz import Authorization from atst.models.permissions import Permissions from atst.domain.users import Users @@ -20,8 +20,13 @@ class Portfolios(object): portfolio = PortfoliosQuery.create( name=name, defense_component=defense_component ) + perms_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] Portfolios._create_portfolio_role( - user, portfolio, "owner", status=PortfolioRoleStatus.ACTIVE + user, + portfolio, + "owner", + status=PortfolioRoleStatus.ACTIVE, + permission_sets=perms_sets, ) PortfoliosQuery.add_and_commit(portfolio) return portfolio @@ -39,7 +44,7 @@ class Portfolios(object): def get_for_update_applications(cls, user, portfolio_id): portfolio = PortfoliosQuery.get(portfolio_id) Authorization.check_portfolio_permission( - user, portfolio, Permissions.ADD_APPLICATION_IN_PORTFOLIO, "add application" + user, portfolio, Permissions.CREATE_APPLICATION, "add application" ) return portfolio @@ -50,7 +55,7 @@ class Portfolios(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.EDIT_PORTFOLIO_INFORMATION, + Permissions.EDIT_PORTFOLIO_NAME, "update portfolio information", ) @@ -62,7 +67,7 @@ class Portfolios(object): Authorization.check_portfolio_permission( user, portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + Permissions.EDIT_PORTFOLIO_USERS, "update a portfolio member", ) @@ -72,10 +77,7 @@ class Portfolios(object): def get_with_members(cls, user, portfolio_id): portfolio = PortfoliosQuery.get(portfolio_id) Authorization.check_portfolio_permission( - user, - portfolio, - Permissions.VIEW_PORTFOLIO_MEMBERS, - "view portfolio members", + user, portfolio, Permissions.VIEW_PORTFOLIO_USERS, "view portfolio members" ) return portfolio @@ -91,10 +93,7 @@ class Portfolios(object): @classmethod def create_member(cls, user, portfolio, data): Authorization.check_portfolio_permission( - user, - portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - "create portfolio member", + user, portfolio, Permissions.EDIT_PORTFOLIO_USERS, "create portfolio member" ) new_user = Users.get_or_create_by_dod_id( @@ -120,21 +119,27 @@ class Portfolios(object): @classmethod def update_member(cls, user, portfolio, member, role_name): Authorization.check_portfolio_permission( - user, - portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - "edit portfolio member", + user, portfolio, Permissions.EDIT_PORTFOLIO_USERS, "edit portfolio member" ) return PortfolioRoles.update_role(member, role_name) @classmethod def _create_portfolio_role( - cls, user, portfolio, role_name, status=PortfolioRoleStatus.PENDING + cls, + user, + portfolio, + role_name, + status=PortfolioRoleStatus.PENDING, + permission_sets=None, ): role = Roles.get(role_name) + + if permission_sets is None: + permission_sets = [] + portfolio_role = PortfoliosQuery.create_portfolio_role( - user, role, portfolio, status=status + user, role, portfolio, status=status, permission_sets=permission_sets ) PortfoliosQuery.add_and_commit(portfolio_role) return portfolio_role @@ -157,10 +162,7 @@ class Portfolios(object): def revoke_access(cls, user, portfolio_id, portfolio_role_id): portfolio = PortfoliosQuery.get(portfolio_id) Authorization.check_portfolio_permission( - user, - portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - "revoke portfolio access", + user, portfolio, Permissions.EDIT_PORTFOLIO_USERS, "revoke portfolio access" ) portfolio_role = PortfolioRoles.get_by_id(portfolio_role_id) diff --git a/atst/domain/portfolios/scopes.py b/atst/domain/portfolios/scopes.py index a3fc686e..9d058813 100644 --- a/atst/domain/portfolios/scopes.py +++ b/atst/domain/portfolios/scopes.py @@ -31,7 +31,7 @@ class ScopedPortfolio(ScopedResource): @property def applications(self): can_view_all_applications = Authorization.has_portfolio_permission( - self.user, self.resource, Permissions.VIEW_APPLICATION_IN_PORTFOLIO + self.user, self.resource, Permissions.VIEW_APPLICATION ) if can_view_all_applications: @@ -54,9 +54,7 @@ class ScopedApplication(ScopedResource): @property def environments(self): can_view_all_environments = Authorization.has_portfolio_permission( - self.user, - self.resource.portfolio, - Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, + self.user, self.resource.portfolio, Permissions.VIEW_ENVIRONMENT ) if can_view_all_environments: diff --git a/atst/domain/roles.py b/atst/domain/roles.py index 49a78135..18bc1a2f 100644 --- a/atst/domain/roles.py +++ b/atst/domain/roles.py @@ -161,7 +161,13 @@ PORTFOLIO_ROLES = [ }, ] -PORTFOLIO_PERMISSION_SETS = [ +_VIEW_PORTFOLIO_PERMISSION_SETS = [ + { + "name": "view_portfolio", + "description": "View basic portfolio info", + "display_name": "View Portfolio", + "permissions": [Permissions.VIEW_PORTFOLIO], + }, { "name": "view_portfolio_application_management", "description": "View applications and related resources", @@ -172,6 +178,36 @@ PORTFOLIO_PERMISSION_SETS = [ Permissions.VIEW_ENVIRONMENT, ], }, + { + "name": "view_portfolio_funding", + "description": "View a portfolio's task orders", + "display_name": "Funding", + "permissions": [ + Permissions.VIEW_PORTFOLIO_FUNDING, + Permissions.VIEW_TASK_ORDER_DETAILS, + ], + }, + { + "name": "view_portfolio_reports", + "description": "View a portfolio's reports", + "display_name": "Reporting", + "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], + }, + { + "name": "view_portfolio_admin", + "description": "View a portfolio's admin options", + "display_name": "Portfolio Administration", + "permissions": [ + Permissions.VIEW_PORTFOLIO_ADMIN, + Permissions.VIEW_PORTFOLIO_NAME, + Permissions.VIEW_PORTFOLIO_USERS, + Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, + Permissions.VIEW_PORTFOLIO_POC, + ], + }, +] + +_EDIT_PORTFOLIO_PERMISSION_SETS = [ { "name": "edit_portfolio_application_management", "description": "Edit applications and related resources", @@ -185,15 +221,6 @@ PORTFOLIO_PERMISSION_SETS = [ Permissions.CREATE_ENVIRONMENT, ], }, - { - "name": "view_portfolio_funding", - "description": "View a portfolio's task orders", - "display_name": "Funding", - "permissions": [ - Permissions.VIEW_PORTFOLIO_FUNDING, - Permissions.VIEW_TASK_ORDER_DETAILS, - ], - }, { "name": "edit_portfolio_funding", "description": "Edit a portfolio's task orders and add new ones", @@ -203,30 +230,12 @@ PORTFOLIO_PERMISSION_SETS = [ Permissions.EDIT_TASK_ORDER_DETAILS, ], }, - { - "name": "view_portfolio_reports", - "description": "View a portfolio's reports", - "display_name": "Reporting", - "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], - }, { "name": "edit_portfolio_reports", "description": "Edit a portfolio's reports (no-op)", "display_name": "Reporting", "permissions": [], }, - { - "name": "view_portfolio_admin", - "description": "View a portfolio's admin options", - "display_name": "Portfolio Administration", - "permissions": [ - Permissions.VIEW_PORTFOLIO_ADMIN, - Permissions.VIEW_PORTFOLIO_NAME, - Permissions.VIEW_PORTFOLIO_USERS, - Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, - Permissions.VIEW_PORTFOLIO_POC, - ], - }, { "name": "edit_portfolio_admin", "description": "Edit a portfolio's admin options", @@ -237,14 +246,24 @@ PORTFOLIO_PERMISSION_SETS = [ Permissions.CREATE_PORTFOLIO_USERS, ], }, - { - "name": "portfolio_poc", - "description": "Permissions belonging to the Portfolio POC", - "display_name": "Portfolio Point of Contact", - "permissions": [Permissions.EDIT_PORTFOLIO_POC, Permissions.ARCHIVE_PORTFOLIO], - }, ] +PORTFOLIO_PERMISSION_SETS = ( + _VIEW_PORTFOLIO_PERMISSION_SETS + + _EDIT_PORTFOLIO_PERMISSION_SETS + + [ + { + "name": "portfolio_poc", + "description": "Permissions belonging to the Portfolio POC", + "display_name": "Portfolio Point of Contact", + "permissions": [ + Permissions.EDIT_PORTFOLIO_POC, + Permissions.ARCHIVE_PORTFOLIO, + ], + } + ] +) + class Roles(object): @classmethod diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 63239679..3f3a5b51 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -57,7 +57,7 @@ class TaskOrders(object): try: task_order = db.session.query(TaskOrder).filter_by(id=task_order_id).one() Authorization.check_task_order_permission( - user, task_order, Permissions.VIEW_TASK_ORDER, "view task order" + user, task_order, Permissions.VIEW_TASK_ORDER_DETAILS, "view task order" ) return task_order @@ -67,7 +67,7 @@ class TaskOrders(object): @classmethod def create(cls, creator, portfolio): Authorization.check_portfolio_permission( - creator, portfolio, Permissions.UPDATE_TASK_ORDER, "add task order" + creator, portfolio, Permissions.CREATE_TASK_ORDER, "add task order" ) task_order = TaskOrder(portfolio=portfolio, creator=creator) @@ -79,7 +79,7 @@ class TaskOrders(object): @classmethod def update(cls, user, task_order, **kwargs): Authorization.check_task_order_permission( - user, task_order, Permissions.UPDATE_TASK_ORDER, "update task order" + user, task_order, Permissions.EDIT_TASK_ORDER_DETAILS, "update task order" ) for key, value in kwargs.items(): @@ -150,7 +150,7 @@ class TaskOrders(object): Authorization.check_portfolio_permission( user, task_order.portfolio, - Permissions.ADD_TASK_ORDER_OFFICER, + Permissions.EDIT_TASK_ORDER_DETAILS, "add task order officer", ) @@ -170,7 +170,13 @@ class TaskOrders(object): portfolio_user = existing_member.user else: member = Portfolios.create_member( - user, portfolio, {**officer_data, "portfolio_role": "officer"} + user, + portfolio, + { + **officer_data, + "portfolio_role": "officer", + "permission_sets": ["edit_portfolio_funding"], + }, ) portfolio_user = member.user diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 274c26b4..8bf9aff6 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -64,7 +64,7 @@ def portfolio_reports(portfolio_id): Authorization.check_portfolio_permission( g.current_user, portfolio, - Permissions.VIEW_USAGE_DOLLARS, + Permissions.VIEW_PORTFOLIO_REPORTS, "view portfolio reports", ) diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index b61510e6..1e744fd3 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -110,7 +110,7 @@ def view_member(portfolio_id, member_id): Authorization.check_portfolio_permission( g.current_user, portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + Permissions.EDIT_PORTFOLIO_USERS, "edit this portfolio user", ) member = PortfolioRoles.get(portfolio_id, member_id) @@ -144,7 +144,7 @@ def update_member(portfolio_id, member_id): Authorization.check_portfolio_permission( g.current_user, portfolio, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + Permissions.EDIT_PORTFOLIO_USERS, "edit this portfolio user", ) member = PortfolioRoles.get(portfolio_id, member_id) diff --git a/templates/portfolios/applications/index.html b/templates/portfolios/applications/index.html index 64e1f412..9e885e5f 100644 --- a/templates/portfolios/applications/index.html +++ b/templates/portfolios/applications/index.html @@ -3,7 +3,7 @@ {% extends "portfolios/base.html" %} -{% set can_create_applications = user_can(permissions.ADD_APPLICATION_IN_PORTFOLIO) %} +{% set can_create_applications = user_can(permissions.CREATE_APPLICATION) %} {% block portfolio_content %} @@ -41,13 +41,13 @@

{{ application.name }}

{{ application.description }}
- {% if user_can(permissions.RENAME_APPLICATION_IN_PORTFOLIO) %} + {% if user_can(permissions.EDIT_APPLICATION) %} {{ "portfolios.applications.app_settings_text" | translate }}
{% endif %} - {% if user_can(permissions.VIEW_PORTFOLIO_MEMBERS) %} + {% if user_can(permissions.VIEW_PORTFOLIO_USERS) %} {{ "portfolios.applications.team_text" | translate }} {{ application.num_users }} diff --git a/templates/portfolios/applications/members.html b/templates/portfolios/applications/members.html index fbb02cf4..cbe648bb 100644 --- a/templates/portfolios/applications/members.html +++ b/templates/portfolios/applications/members.html @@ -11,7 +11,7 @@ {% if not portfolio.members %} - {% set user_can_invite = user_can(permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE) %} + {% set user_can_invite = user_can(permissions.CREATE_PORTFOLIO_USERS) %} {{ EmptyState( 'There are currently no members in this Portfolio.', diff --git a/templates/portfolios/header.html b/templates/portfolios/header.html index c86701da..57c81412 100644 --- a/templates/portfolios/header.html +++ b/templates/portfolios/header.html @@ -56,7 +56,7 @@
- {% if user_can(permissions.VIEW_USAGE_DOLLARS) %} + {% if user_can(permissions.VIEW_PORTFOLIO_REPORTS) %} {{ Link( icon='chart-pie', text='navigation.portfolio_navigation.breadcrumbs.reports' | translate, @@ -70,7 +70,7 @@ url=url_for("portfolios.portfolio_funding", portfolio_id=portfolio.id), active=request.url_rule.endpoint == "portfolios.portfolio_funding", ) }} - {% if user_can(permissions.EDIT_PORTFOLIO_INFORMATION) %} + {% if user_can(permissions.EDIT_PORTFOLIO_NAME) %} {{ Link( icon='cog', text='navigation.portfolio_navigation.breadcrumbs.admin' | translate, diff --git a/templates/portfolios/members/index.html b/templates/portfolios/members/index.html index 976cffc0..4e406029 100644 --- a/templates/portfolios/members/index.html +++ b/templates/portfolios/members/index.html @@ -9,7 +9,7 @@ {% if not portfolio.members %} - {% set user_can_invite = user_can(permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE) %} + {% set user_can_invite = user_can(permissions.CREATE_PORTFOLIO_USERS) %} {{ EmptyState( 'There are currently no members in this Portfolio.', diff --git a/templates/portfolios/reports/index.html b/templates/portfolios/reports/index.html index 5c024ba6..fd008644 100644 --- a/templates/portfolios/reports/index.html +++ b/templates/portfolios/reports/index.html @@ -134,7 +134,7 @@ {% if not portfolio.applications %} - {% set can_create_applications = user_can(permissions.ADD_APPLICATION_IN_PORTFOLIO) %} + {% set can_create_applications = user_can(permissions.CREATE_APPLICATION) %} {% set message = 'This portfolio has no cloud environments set up, so there is no spending data to report. Create an application with some cloud environments to get started.' if can_create_applications else 'This portfolio has no cloud environments set up, so there is no spending data to report. Contact the portfolio owner to set up some cloud environments.' diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 7d4277a6..9673da97 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -27,6 +27,7 @@ def test_non_admin_cannot_view_audit_log(developer): AuditLog.get_all_events(developer) +@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_audit_log(ccpo): events = AuditLog.get_all_events(ccpo) assert len(events) > 0 @@ -41,6 +42,7 @@ def test_paginate_audit_log(ccpo): assert len(events) == 25 +@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_ws_audit_log(ccpo): portfolio = PortfolioFactory.create() events = AuditLog.get_portfolio_events(ccpo, portfolio) @@ -66,6 +68,7 @@ def test_ws_owner_can_view_ws_audit_log(): assert len(events) > 0 +@pytest.mark.skip(reason="all portfolio users can view audit log") def test_other_users_cannot_view_ws_audit_log(): with pytest.raises(UnauthorizedError): portfolio = PortfolioFactory.create() diff --git a/tests/domain/test_authz.py b/tests/domain/test_authz.py index a92fe2f1..3467dcbe 100644 --- a/tests/domain/test_authz.py +++ b/tests/domain/test_authz.py @@ -1,8 +1,10 @@ import pytest -from tests.factories import TaskOrderFactory, UserFactory +from tests.factories import TaskOrderFactory, UserFactory, PortfolioRoleFactory from atst.domain.authz import Authorization +from atst.domain.roles import Roles from atst.domain.exceptions import UnauthorizedError +from atst.models.permissions import Permissions @pytest.fixture @@ -40,3 +42,19 @@ def test_check_is_ko_or_cor(task_order, invalid_user): with pytest.raises(UnauthorizedError): Authorization.check_is_ko_or_cor(invalid_user, task_order) + + +def test_has_portfolio_permission(): + role_one = Roles.get("view_portfolio_funding") + role_two = Roles.get("view_portfolio_reports") + port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) + different_user = UserFactory.create() + assert Authorization.has_portfolio_permission( + port_role.user, port_role.portfolio, Permissions.VIEW_PORTFOLIO_REPORTS + ) + assert not Authorization.has_portfolio_permission( + port_role.user, port_role.portfolio, Permissions.CREATE_TASK_ORDER + ) + assert not Authorization.has_portfolio_permission( + different_user, port_role.portfolio, Permissions.VIEW_PORTFOLIO_REPORTS + ) diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 9972a682..a98dad34 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -6,6 +6,7 @@ from atst.domain.portfolios import Portfolios, PortfolioError from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments +from atst.domain.roles import Roles, PORTFOLIO_PERMISSION_SETS from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import UserFactory, PortfolioRoleFactory, PortfolioFactory @@ -144,6 +145,7 @@ def test_owner_can_view_portfolio_members(portfolio, portfolio_owner): assert portfolio +@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_portfolio_members(portfolio, portfolio_owner): ccpo = UserFactory.from_atat_role("ccpo") assert Portfolios.get_with_members(ccpo, portfolio.id) @@ -156,6 +158,7 @@ def test_random_user_cannot_view_portfolio_members(portfolio): portfolio = Portfolios.get_with_members(developer, portfolio.id) +@pytest.mark.skip(reason="should be reworked pending application member changes") def test_scoped_portfolio_only_returns_a_users_applications_and_environments( portfolio, portfolio_owner ): @@ -199,8 +202,9 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin( ) admin = UserFactory.from_atat_role("default") - Portfolios._create_portfolio_role( - admin, portfolio, "admin", status=PortfolioRoleStatus.ACTIVE + perm_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + PortfolioRoleFactory.create( + user=admin, portfolio=portfolio, permission_sets=perm_sets ) scoped_portfolio = Portfolios.get(admin, portfolio.id) @@ -260,14 +264,16 @@ def test_get_for_update_information(portfolio, portfolio_owner): assert portfolio == owner_ws admin = UserFactory.create() - Portfolios._create_portfolio_role( - admin, portfolio, "admin", status=PortfolioRoleStatus.ACTIVE + perm_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + PortfolioRoleFactory.create( + user=admin, portfolio=portfolio, permission_sets=perm_sets ) admin_ws = Portfolios.get_for_update_information(admin, portfolio.id) assert portfolio == admin_ws - ccpo = UserFactory.from_atat_role("ccpo") - assert Portfolios.get_for_update_information(ccpo, portfolio.id) + # TODO: implement ccpo roles + # ccpo = UserFactory.from_atat_role("ccpo") + # assert Portfolios.get_for_update_information(ccpo, portfolio.id) developer = UserFactory.from_atat_role("developer") with pytest.raises(UnauthorizedError): diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index c7d2d371..b11fcda8 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -93,6 +93,9 @@ def test_add_officer_who_is_already_portfolio_member(): assert member.user == owner and member.role_name == "owner" +from atst.domain.roles import Roles, _VIEW_PORTFOLIO_PERMISSION_SETS + + def test_task_order_access(): creator = UserFactory.create() member = UserFactory.create() @@ -111,19 +114,25 @@ def test_task_order_access(): portfolio = PortfolioFactory.create(owner=creator) task_order = TaskOrderFactory.create(creator=creator, portfolio=portfolio) - PortfolioRoleFactory.create(user=member, portfolio=task_order.portfolio) + PortfolioRoleFactory.create( + user=member, + portfolio=task_order.portfolio, + permission_sets=[ + Roles.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS + ], + ) TaskOrders.add_officer( creator, task_order, "contracting_officer", officer.to_dictionary() ) - check_access([creator, officer], [member, rando], "get", [task_order.id]) - check_access([creator], [officer, member, rando], "create", [portfolio]) + check_access([creator, officer, member], [rando], "get", [task_order.id]) + check_access([creator, officer], [member, rando], "create", [portfolio]) check_access([creator, officer], [member, rando], "update", [task_order]) check_access( - [creator], - [officer, member, rando], + [creator, officer], + [member, rando], "add_officer", - [task_order, "contracting_officer", rando.to_dictionary()], + [task_order, "contracting_officer", UserFactory.dictionary()], ) diff --git a/tests/factories.py b/tests/factories.py index d2c0618f..05d101b1 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -14,7 +14,13 @@ from atst.models.task_order import TaskOrder from atst.models.user import User from atst.models.role import Role from atst.models.portfolio import Portfolio -from atst.domain.roles import Roles, PORTFOLIO_ROLES +from atst.domain.roles import ( + Roles, + PORTFOLIO_ROLES, + PORTFOLIO_PERMISSION_SETS, + _VIEW_PORTFOLIO_PERMISSION_SETS, + _EDIT_PORTFOLIO_PERMISSION_SETS, +) from atst.models.portfolio_role import PortfolioRole, Status as PortfolioRoleStatus from atst.models.environment_role import EnvironmentRole from atst.models.invitation import Invitation, Status as InvitationStatus @@ -68,6 +74,14 @@ def random_portfolio_role(): return Roles.get(choice["name"]) +def base_portfolio_permission_sets(): + return [Roles.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS] + + +def get_all_portfolio_permission_sets(): + return [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + + class Base(factory.alchemy.SQLAlchemyModelFactory): @classmethod def dictionary(cls, **attrs): @@ -124,16 +138,27 @@ class PortfolioFactory(Base): role=Roles.get("owner"), user=owner, status=PortfolioRoleStatus.ACTIVE, + permission_sets=get_all_portfolio_permission_sets(), ) for member in members: user = member.get("user", UserFactory.create()) role_name = member["role_name"] + + perms_set = None + if member.get("permissions_sets"): + perms_set = [ + Roles.get(perm_set) for perm_set in member.get("permission_sets") + ] + else: + perms_set = [] + PortfolioRoleFactory.create( portfolio=portfolio, role=Roles.get(role_name), user=user, status=PortfolioRoleStatus.ACTIVE, + permission_sets=perms_set, ) portfolio.applications = applications @@ -189,6 +214,7 @@ class PortfolioRoleFactory(Base): role = factory.LazyFunction(random_portfolio_role) user = factory.SubFactory(UserFactory) status = PortfolioRoleStatus.PENDING + permission_sets = factory.LazyFunction(base_portfolio_permission_sets) class EnvironmentRoleFactory(Base): diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 6932c174..c04af69c 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -22,19 +22,23 @@ def create_portfolio_and_invite_user( ws_status=PortfolioRoleStatus.PENDING, invite_status=InvitationStatus.PENDING, ): - portfolio = PortfolioFactory.create() + owner = UserFactory.create() + portfolio = PortfolioFactory.create(owner=owner) if ws_role != "owner": user = UserFactory.create() member = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=ws_status ) InvitationFactory.create( - user=portfolio.owner, + inviter=portfolio.owner, + user=user, portfolio_role=member, email=member.user.email, status=invite_status, ) - return portfolio + return (portfolio, member) + else: + return (portfolio, portfolio.members[0]) def test_user_with_permission_has_add_member_link(client, user_session): @@ -248,11 +252,10 @@ def test_does_not_show_any_buttons_if_owner(client, user_session): def test_only_shows_revoke_access_button_if_active(client, user_session): - portfolio = create_portfolio_and_invite_user( + portfolio, member = create_portfolio_and_invite_user( ws_status=PortfolioRoleStatus.ACTIVE, invite_status=InvitationStatus.ACCEPTED ) user_session(portfolio.owner) - member = portfolio.members[1] response = client.get( url_for( "portfolios.view_member", @@ -266,11 +269,11 @@ def test_only_shows_revoke_access_button_if_active(client, user_session): def test_only_shows_revoke_invite_button_if_pending(client, user_session): - portfolio = create_portfolio_and_invite_user( + portfolio, member = create_portfolio_and_invite_user( ws_status=PortfolioRoleStatus.PENDING, invite_status=InvitationStatus.PENDING ) user_session(portfolio.owner) - member = portfolio.members[1] + # member = next((memb for memb in portfolio.members if memb != portfolio.owner), None) response = client.get( url_for( "portfolios.view_member", @@ -284,12 +287,11 @@ def test_only_shows_revoke_invite_button_if_pending(client, user_session): def test_only_shows_resend_button_if_expired(client, user_session): - portfolio = create_portfolio_and_invite_user( + portfolio, member = create_portfolio_and_invite_user( ws_status=PortfolioRoleStatus.PENDING, invite_status=InvitationStatus.REJECTED_EXPIRED, ) user_session(portfolio.owner) - member = portfolio.members[1] response = client.get( url_for( "portfolios.view_member", @@ -303,11 +305,10 @@ def test_only_shows_resend_button_if_expired(client, user_session): def test_only_shows_resend_button_if_revoked(client, user_session): - portfolio = create_portfolio_and_invite_user( + portfolio, member = create_portfolio_and_invite_user( ws_status=PortfolioRoleStatus.PENDING, invite_status=InvitationStatus.REVOKED ) user_session(portfolio.owner) - member = portfolio.members[1] response = client.get( url_for( "portfolios.view_member", From 0c2ab6fb7a9f00ba358c7d354cbed7939cf5de1f Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 05:56:59 -0400 Subject: [PATCH 06/20] remove portfolio_role direct relationship to role --- ...ab219ada_remove_portfolio_roles_role_id.py | 30 +++++++++ atst/domain/portfolio_roles.py | 63 +------------------ atst/domain/portfolios/portfolios.py | 16 +---- atst/domain/portfolios/query.py | 4 +- atst/models/portfolio.py | 4 +- atst/models/portfolio_role.py | 18 +----- atst/routes/__init__.py | 4 +- atst/routes/portfolios/members.py | 4 +- templates/portfolios/members/index.html | 2 +- tests/domain/test_audit_log.py | 5 +- tests/domain/test_portfolio_roles.py | 56 +---------------- tests/domain/test_portfolios.py | 3 +- tests/domain/test_task_orders.py | 6 +- tests/factories.py | 8 --- tests/models/test_portfolio_role.py | 26 ++------ tests/routes/portfolios/test_applications.py | 9 +-- tests/routes/portfolios/test_members.py | 25 +++++--- tests/routes/portfolios/test_task_orders.py | 40 +++++++++--- tests/routes/task_orders/test_invite.py | 3 - tests/routes/test_home.py | 4 +- 20 files changed, 111 insertions(+), 219 deletions(-) create mode 100644 alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py diff --git a/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py b/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py new file mode 100644 index 00000000..8a261f29 --- /dev/null +++ b/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py @@ -0,0 +1,30 @@ +"""remove portfolio_roles role_id + +Revision ID: 0e71ab219ada +Revises: 938a31795096 +Create Date: 2019-03-12 05:58:17.029899 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '0e71ab219ada' +down_revision = '938a31795096' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('workspace_roles_role_id_fkey', 'portfolio_roles', type_='foreignkey') + op.drop_column('portfolio_roles', 'role_id') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('portfolio_roles', sa.Column('role_id', postgresql.UUID(), autoincrement=False, nullable=False)) + op.create_foreign_key('workspace_roles_role_id_fkey', 'portfolio_roles', 'roles', ['role_id'], ['id']) + # ### end Alembic commands ### diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index 8d885455..db01f0e3 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -9,7 +9,6 @@ from atst.models.portfolio_role import ( from atst.models.user import User from .roles import Roles -from .users import Users from .exceptions import NotFoundError @@ -53,17 +52,6 @@ class PortfolioRoles(object): except NoResultFound: return None - @classmethod - def portfolio_role_permissions(cls, portfolio, user): - portfolio_role = PortfolioRoles._get_active_portfolio_role( - portfolio.id, user.id - ) - atat_permissions = set(user.atat_role.permissions) - portfolio_permissions = ( - [] if portfolio_role is None else portfolio_role.role.permissions - ) - return set(portfolio_permissions).union(atat_permissions) - @classmethod def _get_portfolio_role(cls, user, portfolio_id): try: @@ -80,9 +68,7 @@ class PortfolioRoles(object): raise NotFoundError("portfolio role") @classmethod - def add(cls, user, portfolio_id, role_name, permission_sets=None): - role = Roles.get(role_name) - + def add(cls, user, portfolio_id, permission_sets=None): new_portfolio_role = None try: existing_portfolio_role = ( @@ -94,13 +80,9 @@ class PortfolioRoles(object): .one() ) new_portfolio_role = existing_portfolio_role - new_portfolio_role.role = role except NoResultFound: new_portfolio_role = PortfolioRole( - user=user, - role_id=role.id, - portfolio_id=portfolio_id, - status=PortfolioRoleStatus.PENDING, + user=user, portfolio_id=portfolio_id, status=PortfolioRoleStatus.PENDING ) if permission_sets: @@ -137,47 +119,6 @@ class PortfolioRoles(object): db.session.commit() return portfolio_role - @classmethod - def add_many(cls, portfolio_id, portfolio_role_dicts): - portfolio_roles = [] - - for user_dict in portfolio_role_dicts: - try: - user = Users.get(user_dict["id"]) - except NoResultFound: - default_role = Roles.get("developer") - user = User(id=user_dict["id"], atat_role=default_role) - - try: - role = Roles.get(user_dict["portfolio_role"]) - except NoResultFound: - raise NotFoundError("role") - - try: - existing_portfolio_role = ( - db.session.query(PortfolioRole) - .filter( - PortfolioRole.user == user, - PortfolioRole.portfolio_id == portfolio_id, - ) - .one() - ) - new_portfolio_role = existing_portfolio_role - new_portfolio_role.role = role - except NoResultFound: - new_portfolio_role = PortfolioRole( - user=user, role_id=role.id, portfolio_id=portfolio_id - ) - - user.portfolio_roles.append(new_portfolio_role) - portfolio_roles.append(new_portfolio_role) - - db.session.add(user) - - db.session.commit() - - return portfolio_roles - @classmethod def enable(cls, portfolio_role): portfolio_role.status = PortfolioRoleStatus.ACTIVE diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index def155d3..0741856c 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -24,7 +24,6 @@ class Portfolios(object): Portfolios._create_portfolio_role( user, portfolio, - "owner", status=PortfolioRoleStatus.ACTIVE, permission_sets=perms_sets, ) @@ -111,9 +110,7 @@ class Portfolios(object): @classmethod def add_member(cls, portfolio, member, role_name, permission_sets=None): - portfolio_role = PortfolioRoles.add( - member, portfolio.id, role_name, permission_sets - ) + portfolio_role = PortfolioRoles.add(member, portfolio.id, permission_sets) return portfolio_role @classmethod @@ -126,20 +123,13 @@ class Portfolios(object): @classmethod def _create_portfolio_role( - cls, - user, - portfolio, - role_name, - status=PortfolioRoleStatus.PENDING, - permission_sets=None, + cls, user, portfolio, status=PortfolioRoleStatus.PENDING, permission_sets=None ): - role = Roles.get(role_name) - if permission_sets is None: permission_sets = [] portfolio_role = PortfoliosQuery.create_portfolio_role( - user, role, portfolio, status=status, permission_sets=permission_sets + user, portfolio, status=status, permission_sets=permission_sets ) PortfoliosQuery.add_and_commit(portfolio_role) return portfolio_role diff --git a/atst/domain/portfolios/query.py b/atst/domain/portfolios/query.py index df82efbb..eb78c56d 100644 --- a/atst/domain/portfolios/query.py +++ b/atst/domain/portfolios/query.py @@ -18,5 +18,5 @@ class PortfoliosQuery(Query): ) @classmethod - def create_portfolio_role(cls, user, role, portfolio, **kwargs): - return PortfolioRole(user=user, role=role, portfolio=portfolio, **kwargs) + def create_portfolio_role(cls, user, portfolio, **kwargs): + return PortfolioRole(user=user, portfolio=portfolio, **kwargs) diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 2d9a3aa0..0b133ae1 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -23,7 +23,9 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def owner(self): def _is_portfolio_owner(portfolio_role): - return portfolio_role.role.name == "owner" + return "portfolio_poc" in [ + perms_set.name for perms_set in portfolio_role.permission_sets + ] owner = first_or_none(_is_portfolio_owner, self.roles) return owner.user if owner else None diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index 3be7c8f2..8d1afb97 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -10,7 +10,6 @@ from atst.database import db from atst.models.environment_role import EnvironmentRole from atst.models.application import Application from atst.models.environment import Environment -from atst.models.role import Role MEMBER_STATUSES = { @@ -47,9 +46,6 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): ) portfolio = relationship("Portfolio", back_populates="roles") - role_id = Column(UUID(as_uuid=True), ForeignKey("roles.id"), nullable=False) - role = relationship("Role") - user_id = Column( UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False ) @@ -65,19 +61,15 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): ] def __repr__(self): - return "".format( - self.role.name, self.portfolio.name, self.user_id, self.id + return "".format( + self.portfolio.name, self.user_id, self.id, self.permissions ) @property def history(self): previous_state = self.get_changes() change_set = {} - if "role_id" in previous_state: - from_role_id = previous_state["role_id"][0] - from_role = db.session.query(Role).filter(Role.id == from_role_id).one() - to_role = self.role_name - change_set["role"] = [from_role.name, to_role] + # TODO: need to update to include permission_sets if "status" in previous_state: from_status = previous_state["status"][0].value to_status = self.status.value @@ -121,10 +113,6 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def has_dod_id_error(self): return self.latest_invitation and self.latest_invitation.is_rejected_wrong_user - @property - def role_name(self): - return self.role.name - @property def user_name(self): return self.user.full_name diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 4183d8ac..f7b64c79 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -64,7 +64,9 @@ def home(): elif num_portfolios == 1: portfolio_role = user.portfolio_roles[0] portfolio_id = portfolio_role.portfolio.id - is_portfolio_owner = portfolio_role.role.name == "owner" + is_portfolio_owner = "portfolio_poc" in [ + ps.name for ps in portfolio_role.permission_sets + ] if is_portfolio_owner: return redirect( diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index 1e744fd3..3fb86e65 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -28,7 +28,7 @@ def serialize_portfolio_role(portfolio_role): "name": portfolio_role.user_name, "status": portfolio_role.display_status, "id": portfolio_role.user_id, - "role": portfolio_role.role_displayname, + "role": "admin", "num_env": portfolio_role.num_environment_roles, "edit_link": url_for( "portfolios.view_member", @@ -115,7 +115,7 @@ def view_member(portfolio_id, member_id): ) member = PortfolioRoles.get(portfolio_id, member_id) applications = Applications.get_all(g.current_user, member, portfolio) - form = EditMemberForm(portfolio_role=member.role_name) + form = EditMemberForm(portfolio_role="admin") editable = g.current_user == member.user can_revoke_access = Portfolios.can_revoke_access_for(portfolio, member) diff --git a/templates/portfolios/members/index.html b/templates/portfolios/members/index.html index 4e406029..ffc91cf9 100644 --- a/templates/portfolios/members/index.html +++ b/templates/portfolios/members/index.html @@ -9,7 +9,7 @@ {% if not portfolio.members %} - {% set user_can_invite = user_can(permissions.CREATE_PORTFOLIO_USERS) %} + {% set user_can_invite = user_can(Permissions.CREATE_PORTFOLIO_USERS) %} {{ EmptyState( 'There are currently no members in this Portfolio.', diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 9673da97..6686379a 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -53,10 +53,7 @@ def test_ws_admin_can_view_ws_audit_log(): portfolio = PortfolioFactory.create() admin = UserFactory.create() PortfolioRoleFactory.create( - portfolio=portfolio, - user=admin, - role=Roles.get("admin"), - status=PortfolioRoleStatus.ACTIVE, + portfolio=portfolio, user=admin, status=PortfolioRoleStatus.ACTIVE ) events = AuditLog.get_portfolio_events(admin, portfolio) assert len(events) > 0 diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 3f2d9af6..623e6835 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -11,66 +11,12 @@ from tests.factories import ( ) -def test_can_create_new_portfolio_role(): - portfolio = PortfolioFactory.create() - new_user = UserFactory.create() - - portfolio_role_dicts = [{"id": new_user.id, "portfolio_role": "owner"}] - portfolio_roles = PortfolioRoles.add_many(portfolio.id, portfolio_role_dicts) - - assert portfolio_roles[0].user_id == new_user.id - assert portfolio_roles[0].user.atat_role.name == new_user.atat_role.name - assert portfolio_roles[0].role.name == new_user.portfolio_roles[0].role.name - - -def test_can_update_existing_portfolio_role(): - portfolio = PortfolioFactory.create() - new_user = UserFactory.create() - - PortfolioRoles.add_many( - portfolio.id, [{"id": new_user.id, "portfolio_role": "owner"}] - ) - portfolio_roles = PortfolioRoles.add_many( - portfolio.id, [{"id": new_user.id, "portfolio_role": "developer"}] - ) - - assert portfolio_roles[0].user.atat_role.name == new_user.atat_role.name - assert portfolio_roles[0].role.name == new_user.portfolio_roles[0].role.name - - -def test_portfolio_role_permissions(): - portfolio_one = PortfolioFactory.create() - portfolio_two = PortfolioFactory.create() - new_user = UserFactory.create() - PortfolioRoleFactory.create( - portfolio=portfolio_one, - user=new_user, - role=Roles.get("developer"), - status=PortfolioRoleStatus.ACTIVE, - ) - PortfolioRoleFactory.create( - portfolio=portfolio_two, - user=new_user, - role=Roles.get("developer"), - status=PortfolioRoleStatus.PENDING, - ) - - default_perms = set(new_user.atat_role.permissions) - assert len( - PortfolioRoles.portfolio_role_permissions(portfolio_one, new_user) - ) > len(default_perms) - assert ( - PortfolioRoles.portfolio_role_permissions(portfolio_two, new_user) - == default_perms - ) - - def test_add_portfolio_role_with_permission_sets(): portfolio = PortfolioFactory.create() new_user = UserFactory.create() permission_sets = ["edit_portfolio_application_management"] port_role = PortfolioRoles.add( - new_user, portfolio.id, "developer", permission_sets=permission_sets + new_user, portfolio.id, permission_sets=permission_sets ) assert len(port_role.permission_sets) == 5 expected_names = [ diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index a98dad34..1d117875 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -53,7 +53,7 @@ def test_get_for_update_applications_allows_owner(portfolio, portfolio_owner): def test_get_for_update_applications_blocks_developer(portfolio): developer = UserFactory.create() - PortfolioRoles.add(developer, portfolio.id, "developer") + PortfolioRoles.add(developer, portfolio.id) with pytest.raises(UnauthorizedError): Portfolios.get_for_update_applications(developer, portfolio.id) @@ -120,7 +120,6 @@ def test_update_portfolio_role_role(portfolio, portfolio_owner): portfolio_owner, portfolio, member, role_name ) assert updated_member.portfolio == portfolio - assert updated_member.role_name == role_name def test_need_permission_to_update_portfolio_role_role(portfolio, portfolio_owner): diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index b11fcda8..ef99f014 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -2,6 +2,7 @@ import pytest from atst.domain.task_orders import TaskOrders, TaskOrderError, DD254s from atst.domain.exceptions import UnauthorizedError +from atst.domain.roles import Roles, _VIEW_PORTFOLIO_PERMISSION_SETS from atst.models.attachment import Attachment from tests.factories import ( @@ -90,10 +91,7 @@ def test_add_officer_who_is_already_portfolio_member(): assert task_order.contracting_officer == owner member = task_order.portfolio.members[0] - assert member.user == owner and member.role_name == "owner" - - -from atst.domain.roles import Roles, _VIEW_PORTFOLIO_PERMISSION_SETS + assert member.user == owner def test_task_order_access(): diff --git a/tests/factories.py b/tests/factories.py index 05d101b1..f203c5f1 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -69,11 +69,6 @@ def _random_date(year_min, year_max, operation): ) -def random_portfolio_role(): - choice = random.choice(PORTFOLIO_ROLES) - return Roles.get(choice["name"]) - - def base_portfolio_permission_sets(): return [Roles.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS] @@ -135,7 +130,6 @@ class PortfolioFactory(Base): PortfolioRoleFactory.create( portfolio=portfolio, - role=Roles.get("owner"), user=owner, status=PortfolioRoleStatus.ACTIVE, permission_sets=get_all_portfolio_permission_sets(), @@ -155,7 +149,6 @@ class PortfolioFactory(Base): PortfolioRoleFactory.create( portfolio=portfolio, - role=Roles.get(role_name), user=user, status=PortfolioRoleStatus.ACTIVE, permission_sets=perms_set, @@ -211,7 +204,6 @@ class PortfolioRoleFactory(Base): model = PortfolioRole portfolio = factory.SubFactory(PortfolioFactory) - role = factory.LazyFunction(random_portfolio_role) user = factory.SubFactory(UserFactory) status = PortfolioRoleStatus.PENDING permission_sets = factory.LazyFunction(base_portfolio_permission_sets) diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 25fec901..218ce966 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -1,3 +1,4 @@ +import pytest import datetime from atst.domain.environments import Environments @@ -26,7 +27,7 @@ def test_has_no_ws_role_history(session): user = UserFactory.create() portfolio = PortfolioFactory.create(owner=owner) - portfolio_role = PortfolioRoles.add(user, portfolio.id, "developer") + portfolio_role = PortfolioRoles.add(user, portfolio.id) create_event = ( session.query(AuditEvent) .filter( @@ -38,6 +39,7 @@ def test_has_no_ws_role_history(session): assert not create_event.changed_state +@pytest.mark.skip(reason="need to update audit log permission set handling") def test_has_ws_role_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -47,9 +49,7 @@ def test_has_ws_role_history(session): # in order to get the history, we don't want the PortfolioRoleFactory # to commit after create() PortfolioRoleFactory._meta.sqlalchemy_session_persistence = "flush" - portfolio_role = PortfolioRoleFactory.create( - portfolio=portfolio, user=user, role=role - ) + portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=user) PortfolioRoles.update_role(portfolio_role, "admin") changed_events = ( session.query(AuditEvent) @@ -138,7 +138,7 @@ def test_event_details(): user = UserFactory.create() portfolio = PortfolioFactory.create(owner=owner) - portfolio_role = PortfolioRoles.add(user, portfolio.id, "developer") + portfolio_role = PortfolioRoles.add(user, portfolio.id) assert portfolio_role.event_details["updated_user_name"] == user.displayname assert portfolio_role.event_details["updated_user_id"] == str(user.id) @@ -185,22 +185,6 @@ def test_has_environment_roles(): assert portfolio_role.has_environment_roles -def test_role_displayname(): - owner = UserFactory.create() - developer_data = { - "dod_id": "1234567890", - "first_name": "Test", - "last_name": "User", - "email": "test.user@mail.com", - "portfolio_role": "developer", - } - - portfolio = PortfolioFactory.create(owner=owner) - portfolio_role = Portfolios.create_member(owner, portfolio, developer_data) - - assert portfolio_role.role_displayname == "Developer" - - def test_status_when_member_is_active(): portfolio_role = PortfolioRoleFactory.create(status=Status.ACTIVE) assert portfolio_role.display_status == "Active" diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index 573773ca..bac4fe39 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -29,7 +29,7 @@ def test_user_without_permission_has_no_budget_report_link(client, user_session) user = UserFactory.create() portfolio = PortfolioFactory.create() Portfolios._create_portfolio_role( - user, portfolio, "developer", status=PortfolioRoleStatus.ACTIVE + user, portfolio, status=PortfolioRoleStatus.ACTIVE ) user_session(user) response = client.get("/portfolios/{}/applications".format(portfolio.id)) @@ -45,10 +45,7 @@ def test_user_with_permission_has_activity_log_link(client, user_session): ccpo = UserFactory.from_atat_role("ccpo") admin = UserFactory.create() PortfolioRoleFactory.create( - portfolio=portfolio, - user=admin, - role=Roles.get("admin"), - status=PortfolioRoleStatus.ACTIVE, + portfolio=portfolio, user=admin, status=PortfolioRoleStatus.ACTIVE ) user_session(portfolio.owner) @@ -103,7 +100,7 @@ def test_user_with_permission_has_add_application_link(client, user_session): def test_user_without_permission_has_no_add_application_link(client, user_session): user = UserFactory.create() portfolio = PortfolioFactory.create() - Portfolios._create_portfolio_role(user, portfolio, "developer") + Portfolios._create_portfolio_role(user, portfolio) user_session(user) response = client.get("/portfolios/{}/applications".format(portfolio.id)) assert ( diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index c04af69c..012b53e7 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -45,6 +45,7 @@ def test_user_with_permission_has_add_member_link(client, user_session): portfolio = PortfolioFactory.create() user_session(portfolio.owner) response = client.get("/portfolios/{}/members".format(portfolio.id)) + assert response.status_code == 200 assert ( 'href="/portfolios/{}/members/new"'.format(portfolio.id).encode() in response.data @@ -54,7 +55,7 @@ def test_user_with_permission_has_add_member_link(client, user_session): def test_user_without_permission_has_no_add_member_link(client, user_session): user = UserFactory.create() portfolio = PortfolioFactory.create() - Portfolios._create_portfolio_role(user, portfolio, "developer") + Portfolios._create_portfolio_role(user, portfolio) user_session(user) response = client.get("/portfolios/{}/members".format(portfolio.id)) assert ( @@ -66,8 +67,8 @@ def test_user_without_permission_has_no_add_member_link(client, user_session): def test_permissions_for_view_member(client, user_session): user = UserFactory.create() portfolio = PortfolioFactory.create() - Portfolios._create_portfolio_role(user, portfolio, "developer") - member = PortfolioRoles.add(user, portfolio.id, "developer") + Portfolios._create_portfolio_role(user, portfolio) + member = PortfolioRoles.add(user, portfolio.id) user_session(user) response = client.get( url_for("portfolios.view_member", portfolio_id=portfolio.id, member_id=user.id) @@ -106,11 +107,12 @@ def test_create_member(client, user_session): assert len(portfolio_role.permission_sets) == 4 +@pytest.mark.skip(reason="permission set display not implemented") def test_view_member_shows_role(client, user_session): user = UserFactory.create() portfolio = PortfolioFactory.create() - Portfolios._create_portfolio_role(user, portfolio, "developer") - member = PortfolioRoles.add(user, portfolio.id, "developer") + Portfolios._create_portfolio_role(user, portfolio) + member = PortfolioRoles.add(user, portfolio.id) user_session(portfolio.owner) response = client.get( url_for("portfolios.view_member", portfolio_id=portfolio.id, member_id=user.id) @@ -119,10 +121,11 @@ def test_view_member_shows_role(client, user_session): assert "initial-choice='developer'".encode() in response.data +@pytest.mark.skip(reason="need to re-implement for permission set changes") def test_update_member_portfolio_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() - member = PortfolioRoles.add(user, portfolio.id, "developer") + member = PortfolioRoles.add(user, portfolio.id) user_session(portfolio.owner) response = client.post( url_for( @@ -136,10 +139,11 @@ def test_update_member_portfolio_role(client, user_session): assert member.role_name == "security_auditor" +@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_portfolio_role_with_no_data(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() - member = PortfolioRoles.add(user, portfolio.id, "developer") + member = PortfolioRoles.add(user, portfolio.id) user_session(portfolio.owner) response = client.post( url_for( @@ -152,10 +156,11 @@ def test_update_member_portfolio_role_with_no_data(client, user_session): assert member.role_name == "developer" +@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_environment_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() - member = PortfolioRoles.add(user, portfolio.id, "developer") + member = PortfolioRoles.add(user, portfolio.id) application = Applications.create( portfolio.owner, portfolio, @@ -173,7 +178,6 @@ def test_update_member_environment_role(client, user_session): "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id ), data={ - "portfolio_role": "developer", "env_" + str(env1_id): "security_auditor", "env_" + str(env2_id): "devops", }, @@ -189,7 +193,7 @@ def test_update_member_environment_role(client, user_session): def test_update_member_environment_role_with_no_data(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() - member = PortfolioRoles.add(user, portfolio.id, "developer") + member = PortfolioRoles.add(user, portfolio.id) application = Applications.create( portfolio.owner, portfolio, @@ -263,6 +267,7 @@ def test_only_shows_revoke_access_button_if_active(client, user_session): member_id=member.user.id, ) ) + assert response.status_code == 200 assert "Remove Portfolio Access" in response.data.decode() assert "Revoke Invitation" not in response.data.decode() assert "Resend Invitation" not in response.data.decode() diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index ad93242a..0b74dc57 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -230,10 +230,13 @@ class TestTaskOrderInvitations: def test_ko_can_view_task_order(client, user_session, portfolio, user): PortfolioRoleFactory.create( - role=Roles.get("owner"), portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=user) user_session(user) @@ -294,16 +297,22 @@ def test_ko_can_view_ko_review_page(client, user_session): cor = UserFactory.create() PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=ko, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=cor, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create( portfolio=portfolio, @@ -365,10 +374,13 @@ def test_mo_redirected_to_build_page(client, user_session, portfolio): def test_cor_redirected_to_build_page(client, user_session, portfolio): cor = UserFactory.create() PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=cor, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create( portfolio=portfolio, contracting_officer_representative=cor @@ -384,10 +396,13 @@ def test_submit_completed_ko_review_page_as_cor( client, user_session, pdf_upload, portfolio, user ): PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create( @@ -429,10 +444,13 @@ def test_submit_completed_ko_review_page_as_ko( ko = UserFactory.create() PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=ko, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=ko) @@ -470,10 +488,13 @@ def test_submit_completed_ko_review_page_as_ko( def test_so_review_page(app, client, user_session, portfolio): so = UserFactory.create() PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=so, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) @@ -508,10 +529,13 @@ def test_so_review_page(app, client, user_session, portfolio): def test_submit_so_review(app, client, user_session, portfolio): so = UserFactory.create() PortfolioRoleFactory.create( - role=Roles.get("officer"), portfolio=portfolio, user=so, status=PortfolioStatus.ACTIVE, + permission_sets=[ + Roles.get("view_portfolio"), + Roles.get("view_portfolio_funding"), + ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) dd_254_data = DD254Factory.dictionary() diff --git a/tests/routes/task_orders/test_invite.py b/tests/routes/task_orders/test_invite.py index cd07db23..452c437d 100644 --- a/tests/routes/task_orders/test_invite.py +++ b/tests/routes/task_orders/test_invite.py @@ -28,9 +28,6 @@ def test_invite_officers_to_task_order(client, user_session, queue): # owner and three officers are portfolio members assert len(portfolio.members) == 4 - roles = [member.role.name for member in portfolio.members] - # officers exist in roles - assert roles.count("officer") == 3 # email invitations are enqueued assert len(queue.get_queue()) == 3 # task order has relationship to user for each officer role diff --git a/tests/routes/test_home.py b/tests/routes/test_home.py index a2eeba20..768f04d4 100644 --- a/tests/routes/test_home.py +++ b/tests/routes/test_home.py @@ -33,7 +33,7 @@ def test_non_owner_user_with_one_portfolio_redirected_to_portfolio_applications( user = UserFactory.create() portfolio = PortfolioFactory.create() Portfolios._create_portfolio_role( - user, portfolio, "developer", status=PortfolioRoleStatus.ACTIVE + user, portfolio, status=PortfolioRoleStatus.ACTIVE ) user_session(user) @@ -51,7 +51,7 @@ def test_non_owner_user_with_mulitple_portfolios_redirected_to_portfolios( portfolio = PortfolioFactory.create() portfolios.append(portfolio) role = Portfolios._create_portfolio_role( - user, portfolio, "developer", status=PortfolioRoleStatus.ACTIVE + user, portfolio, status=PortfolioRoleStatus.ACTIVE ) user_session(user) From 0eb2d4fa3cd5b85b7902fefb066a19adb6e66898 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 08:33:59 -0400 Subject: [PATCH 07/20] remove frontend role selection for new member --- atst/domain/portfolios/portfolios.py | 4 ++-- atst/forms/new_member.py | 10 ---------- templates/portfolios/members/new.html | 1 - tests/domain/test_portfolios.py | 2 +- tests/routes/portfolios/test_invitations.py | 6 +++--- 5 files changed, 6 insertions(+), 17 deletions(-) diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 0741856c..748b386f 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -105,11 +105,11 @@ class Portfolios(object): ) permission_sets = data.get("permission_sets", []) return Portfolios.add_member( - portfolio, new_user, data["portfolio_role"], permission_sets=permission_sets + portfolio, new_user, permission_sets=permission_sets ) @classmethod - def add_member(cls, portfolio, member, role_name, permission_sets=None): + def add_member(cls, portfolio, member, permission_sets=None): portfolio_role = PortfolioRoles.add(member, portfolio.id, permission_sets) return portfolio_role diff --git a/atst/forms/new_member.py b/atst/forms/new_member.py index 88d6ec21..8c683dc7 100644 --- a/atst/forms/new_member.py +++ b/atst/forms/new_member.py @@ -7,8 +7,6 @@ from atst.forms.validators import IsNumber from atst.forms.fields import SelectField from atst.utils.localization import translate -from .data import PORTFOLIO_ROLES - class NewMemberForm(BaseForm): @@ -25,14 +23,6 @@ class NewMemberForm(BaseForm): translate("forms.new_member.dod_id_label"), validators=[Required(), Length(min=10), IsNumber()], ) - portfolio_role = SelectField( - translate("forms.new_member.portfolio_role_label"), - choices=PORTFOLIO_ROLES, - validators=[Required()], - default="", - description=translate("forms.new_member.portfolio_role_description"), - ) - perms_app_mgmt = SelectField( None, choices=[ diff --git a/templates/portfolios/members/new.html b/templates/portfolios/members/new.html index 58f8cf67..190a3716 100644 --- a/templates/portfolios/members/new.html +++ b/templates/portfolios/members/new.html @@ -22,7 +22,6 @@ {{ TextInput(form.last_name) }} {{ TextInput(form.email,placeholder='jane@mail.mil', validation='email') }} {{ TextInput(form.dod_id,placeholder='10-digit number on the back of the CAC', validation='dodId') }} - {{ Selector(form.portfolio_role) }} diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 1d117875..34b926ea 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -243,7 +243,7 @@ def test_for_user_returns_active_portfolios_for_user(portfolio, portfolio_owner) def test_for_user_does_not_return_inactive_portfolios(portfolio, portfolio_owner): bob = UserFactory.from_atat_role("default") - Portfolios.add_member(portfolio, bob, "developer") + Portfolios.add_member(portfolio, bob) PortfolioFactory.create() bobs_portfolios = Portfolios.for_user(bob) diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index a4d89144..e6ea7a95 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -45,10 +45,9 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): user_info = UserFactory.dictionary() user_session(portfolio.owner) - client.post( + response = client.post( url_for("portfolios.create_member", portfolio_id=portfolio.id), data={ - "portfolio_role": "developer", "perms_app_mgmt": "view_portfolio_application_management", "perms_funding": "view_portfolio_funding", "perms_reporting": "view_portfolio_reports", @@ -57,6 +56,7 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): }, ) + assert response.status_code == 302 user = Users.get_by_dod_id(user_info["dod_id"]) token = user.invitations[0].token @@ -101,7 +101,7 @@ def test_user_who_has_not_accepted_portfolio_invite_cannot_view(client, user_ses user_session(portfolio.owner) response = client.post( url_for("portfolios.create_member", portfolio_id=portfolio.id), - data={"portfolio_role": "developer", **user.to_dictionary()}, + data=user.to_dictionary(), ) # user tries to view portfolio before accepting invitation From 720c227c3268bbd39bc83e3ff8d62ef75e7b279d Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 10:11:30 -0400 Subject: [PATCH 08/20] remove frontend role selection for edit member --- atst/domain/portfolio_roles.py | 7 +-- atst/domain/portfolios/portfolios.py | 5 +- atst/forms/edit_member.py | 18 ------- atst/forms/new_member.py | 63 ---------------------- atst/forms/portfolio_member.py | 71 +++++++++++++++++++++++++ atst/routes/portfolios/members.py | 26 +++------ templates/portfolios/members/edit.html | 25 ++++++++- tests/domain/test_portfolios.py | 4 +- tests/routes/portfolios/test_members.py | 26 +++++---- 9 files changed, 127 insertions(+), 118 deletions(-) delete mode 100644 atst/forms/edit_member.py create mode 100644 atst/forms/portfolio_member.py diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index db01f0e3..e7e58ed6 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -111,12 +111,13 @@ class PortfolioRoles(object): return [Roles.get(perms_set_name) for perms_set_name in perms_set_names] @classmethod - def update_role(cls, portfolio_role, role_name): - new_role = Roles.get(role_name) - portfolio_role.role = new_role + def update(cls, portfolio_role, set_names): + new_permission_sets = PortfolioRoles._permission_sets_for_names(set_names) + portfolio_role.permission_sets = new_permission_sets db.session.add(portfolio_role) db.session.commit() + return portfolio_role @classmethod diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 748b386f..7bbea7be 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -114,12 +114,13 @@ class Portfolios(object): return portfolio_role @classmethod - def update_member(cls, user, portfolio, member, role_name): + def update_member(cls, user, portfolio, member, permission_sets): Authorization.check_portfolio_permission( user, portfolio, Permissions.EDIT_PORTFOLIO_USERS, "edit portfolio member" ) - return PortfolioRoles.update_role(member, role_name) + # need to update perms sets here + return PortfolioRoles.update(member, permission_sets) @classmethod def _create_portfolio_role( diff --git a/atst/forms/edit_member.py b/atst/forms/edit_member.py deleted file mode 100644 index 766ed65c..00000000 --- a/atst/forms/edit_member.py +++ /dev/null @@ -1,18 +0,0 @@ -from wtforms.validators import Required - -from .forms import BaseForm -from atst.forms.fields import SelectField -from atst.utils.localization import translate - -from .data import PORTFOLIO_ROLES - - -class EditMemberForm(BaseForm): - # This form also accepts a field for each environment in each application - # that the user is a member of - - portfolio_role = SelectField( - translate("forms.edit_member.portfolio_role_label"), - choices=PORTFOLIO_ROLES, - validators=[Required()], - ) diff --git a/atst/forms/new_member.py b/atst/forms/new_member.py index 8c683dc7..e69de29b 100644 --- a/atst/forms/new_member.py +++ b/atst/forms/new_member.py @@ -1,63 +0,0 @@ -from wtforms.fields import StringField -from wtforms.fields.html5 import EmailField -from wtforms.validators import Required, Email, Length - -from .forms import BaseForm -from atst.forms.validators import IsNumber -from atst.forms.fields import SelectField -from atst.utils.localization import translate - - -class NewMemberForm(BaseForm): - - first_name = StringField( - label=translate("forms.new_member.first_name_label"), validators=[Required()] - ) - last_name = StringField( - label=translate("forms.new_member.last_name_label"), validators=[Required()] - ) - email = EmailField( - translate("forms.new_member.email_label"), validators=[Required(), Email()] - ) - dod_id = StringField( - translate("forms.new_member.dod_id_label"), - validators=[Required(), Length(min=10), IsNumber()], - ) - perms_app_mgmt = SelectField( - None, - choices=[ - ("view_portfolio_application_management", "View Only"), - ("edit_portfolio_application_management", "Edit Access"), - ], - ) - perms_funding = SelectField( - None, - choices=[ - ("view_portfolio_funding", "View Only"), - ("edit_portfolio_funding", "Edit Access"), - ], - ) - perms_reporting = SelectField( - None, - choices=[ - ("view_portfolio_reports", "View Only"), - ("edit_portfolio_reports", "Edit Access"), - ], - ) - perms_portfolio_mgmt = SelectField( - None, - choices=[ - ("view_portfolio_admin", "View Only"), - ("edit_portfolio_admin", "Edit Access"), - ], - ) - - @property - def data(self): - _data = super().data - _data["permission_sets"] = [] - for field in _data: - if "perms" in field: - _data["permission_sets"].append(_data[field]) - - return _data diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py new file mode 100644 index 00000000..f75c2869 --- /dev/null +++ b/atst/forms/portfolio_member.py @@ -0,0 +1,71 @@ +from wtforms.fields import StringField +from wtforms.fields.html5 import EmailField +from wtforms.validators import Required, Email, Length + +from .forms import BaseForm +from atst.forms.validators import IsNumber +from atst.forms.fields import SelectField +from atst.utils.localization import translate + + +class PermissionsForm(BaseForm): + perms_app_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_application_management", "View Only"), + ("edit_portfolio_application_management", "Edit Access"), + ], + ) + perms_funding = SelectField( + None, + choices=[ + ("view_portfolio_funding", "View Only"), + ("edit_portfolio_funding", "Edit Access"), + ], + ) + perms_reporting = SelectField( + None, + choices=[ + ("view_portfolio_reports", "View Only"), + ("edit_portfolio_reports", "Edit Access"), + ], + ) + perms_portfolio_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_admin", "View Only"), + ("edit_portfolio_admin", "Edit Access"), + ], + ) + + @property + def data(self): + _data = super().data + _data["permission_sets"] = [] + for field in _data: + if "perms" in field: + _data["permission_sets"].append(_data[field]) + + return _data + + +class EditForm(PermissionsForm): + # This form also accepts a field for each environment in each application + # that the user is a member of + pass + + +class NewForm(PermissionsForm): + first_name = StringField( + label=translate("forms.new_member.first_name_label"), validators=[Required()] + ) + last_name = StringField( + label=translate("forms.new_member.last_name_label"), validators=[Required()] + ) + email = EmailField( + translate("forms.new_member.email_label"), validators=[Required(), Email()] + ) + dod_id = StringField( + translate("forms.new_member.dod_id_label"), + validators=[Required(), Length(min=10), IsNumber()], + ) diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index 3fb86e65..30ba503b 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -10,8 +10,7 @@ from atst.domain.portfolio_roles import PortfolioRoles, MEMBER_STATUS_CHOICES from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.services.invitation import Invitation as InvitationService -from atst.forms.new_member import NewMemberForm -from atst.forms.edit_member import EditMemberForm +import atst.forms.portfolio_member as member_forms from atst.forms.data import ( ENVIRONMENT_ROLES, ENV_ROLE_MODAL_DESCRIPTION, @@ -70,7 +69,7 @@ def application_members(portfolio_id, application_id): @portfolios_bp.route("/portfolios//members/new") def new_member(portfolio_id): portfolio = Portfolios.get(g.current_user, portfolio_id) - form = NewMemberForm() + form = member_forms.NewForm() return render_template( "portfolios/members/new.html", portfolio=portfolio, form=form ) @@ -79,7 +78,7 @@ def new_member(portfolio_id): @portfolios_bp.route("/portfolios//members/new", methods=["POST"]) def create_member(portfolio_id): portfolio = Portfolios.get(g.current_user, portfolio_id) - form = NewMemberForm(http_request.form) + form = member_forms.NewForm(http_request.form) if form.validate(): try: @@ -115,7 +114,7 @@ def view_member(portfolio_id, member_id): ) member = PortfolioRoles.get(portfolio_id, member_id) applications = Applications.get_all(g.current_user, member, portfolio) - form = EditMemberForm(portfolio_role="admin") + form = member_forms.EditForm(portfolio_role="admin") editable = g.current_user == member.user can_revoke_access = Portfolios.can_revoke_access_for(portfolio, member) @@ -157,20 +156,11 @@ def update_member(portfolio_id, member_id): env_role = form_dict[entry] or None ids_and_roles.append({"id": env_id, "role": env_role}) - form = EditMemberForm(http_request.form) + form = member_forms.EditForm(http_request.form) if form.validate(): - new_role_name = None - if form.data["portfolio_role"] != member.role.name: - member = Portfolios.update_member( - g.current_user, portfolio, member, form.data["portfolio_role"] - ) - new_role_name = member.role_displayname - flash( - "portfolio_role_updated", - member_name=member.user_name, - updated_role=new_role_name, - ) - + member = Portfolios.update_member( + g.current_user, portfolio, member, form.data["permission_sets"] + ) updated_roles = Environments.update_environment_roles( g.current_user, portfolio, member, ids_and_roles ) diff --git a/templates/portfolios/members/edit.html b/templates/portfolios/members/edit.html index 2e23890c..6c31ae1c 100644 --- a/templates/portfolios/members/edit.html +++ b/templates/portfolios/members/edit.html @@ -21,7 +21,30 @@

{{ member.user.full_name }}

- {{ Selector(form.portfolio_role) }} +
+ + + + + + + + + + + + + + +
{{ "portfolios.members.permissions.app_mgmt" | translate }}{{ "portfolios.members.permissions.funding" | translate }}{{ "portfolios.members.permissions.reporting" | translate }}{{ "portfolios.members.permissions.portfolio_mgmt" | translate }}
+ {{ form.perms_app_mgmt() }} + + {{ form.perms_funding() }} + + {{ form.perms_reporting() }} + + {{ form.perms_portfolio_mgmt() }} +
diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 34b926ea..54b0f740 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -114,10 +114,10 @@ def test_update_portfolio_role_role(portfolio, portfolio_owner): } PortfolioRoleFactory._meta.sqlalchemy_session_persistence = "flush" member = PortfolioRoleFactory.create(portfolio=portfolio) - role_name = "admin" + permission_sets = ["edit_portfolio_funding"] updated_member = Portfolios.update_member( - portfolio_owner, portfolio, member, role_name + portfolio_owner, portfolio, member, permission_sets=permission_sets ) assert updated_member.portfolio == portfolio diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 012b53e7..af63da9a 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -12,10 +12,18 @@ from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.roles import Roles from atst.queue import queue from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.invitation import Status as InvitationStatus +_DEFAULT_PERMS_FORM_DATA = { + "perms_app_mgmt": "view_portfolio_application_management", + "perms_funding": "view_portfolio_funding", + "perms_reporting": "view_portfolio_reports", + "perms_portfolio_mgmt": "view_portfolio_admin", +} + def create_portfolio_and_invite_user( ws_role="developer", @@ -90,10 +98,7 @@ def test_create_member(client, user_session): "last_name": "Zuckerman", "email": "some_pig@zuckermans.com", "portfolio_role": "developer", - "perms_app_mgmt": "view_portfolio_application_management", - "perms_funding": "view_portfolio_funding", - "perms_reporting": "view_portfolio_reports", - "perms_portfolio_mgmt": "view_portfolio_admin", + **_DEFAULT_PERMS_FORM_DATA, }, follow_redirects=True, ) @@ -121,7 +126,6 @@ def test_view_member_shows_role(client, user_session): assert "initial-choice='developer'".encode() in response.data -@pytest.mark.skip(reason="need to re-implement for permission set changes") def test_update_member_portfolio_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() @@ -131,20 +135,20 @@ def test_update_member_portfolio_role(client, user_session): url_for( "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id ), - data={"portfolio_role": "security_auditor"}, + data={**_DEFAULT_PERMS_FORM_DATA, "perms_funding": "edit_portfolio_funding"}, follow_redirects=True, ) assert response.status_code == 200 - assert b"role updated successfully" in response.data - assert member.role_name == "security_auditor" + edit_funding = Roles.get("edit_portfolio_funding") + assert edit_funding in member.permission_sets -@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_portfolio_role_with_no_data(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() member = PortfolioRoles.add(user, portfolio.id) user_session(portfolio.owner) + original_perms_len = len(member.permission_sets) response = client.post( url_for( "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id @@ -153,10 +157,9 @@ def test_update_member_portfolio_role_with_no_data(client, user_session): follow_redirects=True, ) assert response.status_code == 200 - assert member.role_name == "developer" + assert len(member.permission_sets) == original_perms_len -@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_environment_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() @@ -180,6 +183,7 @@ def test_update_member_environment_role(client, user_session): data={ "env_" + str(env1_id): "security_auditor", "env_" + str(env2_id): "devops", + **_DEFAULT_PERMS_FORM_DATA, }, follow_redirects=True, ) From 1a122c533594fe7e0acafb08145d80882917e1f6 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 10:25:50 -0400 Subject: [PATCH 09/20] rename role -> permission_set everywhere --- ...4_rename_roles_table_to_permission_sets.py | 28 +++++++++++++++ atst/domain/{roles.py => permission_sets.py} | 12 +++---- atst/domain/portfolio_roles.py | 6 ++-- atst/domain/portfolios/portfolios.py | 6 ++-- atst/domain/users.py | 6 ++-- atst/forms/data.py | 2 +- atst/models/__init__.py | 2 +- atst/models/{role.py => permission_set.py} | 6 ++-- atst/models/portfolio_role.py | 6 ++-- atst/models/user.py | 4 +-- script/remove_sample_data.py | 2 +- script/seed_roles.py | 36 ++++++++++++------- tests/domain/test_audit_log.py | 2 +- tests/domain/test_authz.py | 6 ++-- tests/domain/test_portfolio_roles.py | 2 +- tests/domain/test_portfolios.py | 6 ++-- tests/domain/test_roles.py | 8 ++--- tests/domain/test_task_orders.py | 4 +-- tests/factories.py | 19 +++++----- tests/models/test_portfolio_role.py | 7 ++-- tests/routes/portfolios/test_applications.py | 2 +- tests/routes/portfolios/test_members.py | 4 +-- tests/routes/portfolios/test_task_orders.py | 34 +++++++++--------- tests/test_auth.py | 4 +-- 24 files changed, 130 insertions(+), 84 deletions(-) create mode 100644 alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py rename atst/domain/{roles.py => permission_sets.py} (97%) rename atst/models/{role.py => permission_set.py} (84%) diff --git a/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py b/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py new file mode 100644 index 00000000..64f629fd --- /dev/null +++ b/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py @@ -0,0 +1,28 @@ +"""rename roles table to permission_sets + +Revision ID: a19138e386c4 +Revises: 0e71ab219ada +Create Date: 2019-03-13 10:18:35.770296 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'a19138e386c4' +down_revision = '0e71ab219ada' +branch_labels = None +depends_on = None + + +def upgrade(): + op.rename_table("roles", "permission_sets") + op.rename_table("portfolio_roles_roles", "portfolio_roles_permission_sets") + op.alter_column("portfolio_roles_permission_sets", "role_id", new_column_name="permission_set_id") + + +def downgrade(): + op.rename_table("permission_sets", "roles") + op.rename_table("portfolio_roles_permission_sets", "portfolio_roles_roles") + op.alter_column("portfolio_roles_permission_sets", "permission_set_id", new_column_name="role_id") diff --git a/atst/domain/roles.py b/atst/domain/permission_sets.py similarity index 97% rename from atst/domain/roles.py rename to atst/domain/permission_sets.py index 18bc1a2f..4c70b4e2 100644 --- a/atst/domain/roles.py +++ b/atst/domain/permission_sets.py @@ -1,7 +1,7 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.models import Role, Permissions +from atst.models import PermissionSet, Permissions from .exceptions import NotFoundError @@ -265,16 +265,16 @@ PORTFOLIO_PERMISSION_SETS = ( ) -class Roles(object): +class PermissionSets(object): @classmethod - def get(cls, role_name): + def get(cls, perms_set_name): try: - role = db.session.query(Role).filter_by(name=role_name).one() + role = db.session.query(PermissionSet).filter_by(name=perms_set_name).one() except NoResultFound: - raise NotFoundError("role") + raise NotFoundError("permission_set") return role @classmethod def get_all(cls): - return db.session.query(Role).all() + return db.session.query(PermissionSet).all() diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index e7e58ed6..19e5125b 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -8,7 +8,7 @@ from atst.models.portfolio_role import ( ) from atst.models.user import User -from .roles import Roles +from .permission_sets import PermissionSets from .exceptions import NotFoundError @@ -108,7 +108,9 @@ class PortfolioRoles(object): perms_set_names = PortfolioRoles._DEFAULT_PORTFOLIO_PERMS_SETS.union( set(set_names) ) - return [Roles.get(perms_set_name) for perms_set_name in perms_set_names] + return [ + PermissionSets.get(perms_set_name) for perms_set_name in perms_set_names + ] @classmethod def update(cls, portfolio_role, set_names): diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 7bbea7be..f6170fa9 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -1,4 +1,4 @@ -from atst.domain.roles import Roles, PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS from atst.domain.authz import Authorization from atst.models.permissions import Permissions from atst.domain.users import Users @@ -20,7 +20,9 @@ class Portfolios(object): portfolio = PortfoliosQuery.create( name=name, defense_component=defense_component ) - perms_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + perms_sets = [ + PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS + ] Portfolios._create_portfolio_role( user, portfolio, diff --git a/atst/domain/users.py b/atst/domain/users.py index 5742017a..c91a17b4 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -4,7 +4,7 @@ from sqlalchemy.exc import IntegrityError from atst.database import db from atst.models import User -from .roles import Roles +from .permission_sets import PermissionSets from .exceptions import NotFoundError, AlreadyExistsError, UnauthorizedError @@ -29,7 +29,7 @@ class Users(object): @classmethod def create(cls, dod_id, atat_role_name=None, **kwargs): - atat_role = Roles.get(atat_role_name) + atat_role = PermissionSets.get(atat_role_name) try: user = User(dod_id=dod_id, atat_role=atat_role, **kwargs) @@ -56,7 +56,7 @@ class Users(object): def update_role(cls, user_id, atat_role_name): user = Users.get(user_id) - atat_role = Roles.get(atat_role_name) + atat_role = PermissionSets.get(atat_role_name) user.atat_role = atat_role db.session.add(user) diff --git a/atst/forms/data.py b/atst/forms/data.py index 23b1d0d4..3a515f6d 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,4 +1,4 @@ -from atst.domain.roles import PORTFOLIO_ROLES as PORTFOLIO_ROLE_DEFINITIONS +from atst.domain.permission_sets import PORTFOLIO_ROLES as PORTFOLIO_ROLE_DEFINITIONS from atst.utils.localization import translate, translate_duration diff --git a/atst/models/__init__.py b/atst/models/__init__.py index ee3a7958..bf8dc338 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -3,7 +3,7 @@ from sqlalchemy.ext.declarative import declarative_base Base = declarative_base() from .permissions import Permissions -from .role import Role +from .permission_set import PermissionSet from .user import User from .portfolio_role import PortfolioRole from .portfolio import Portfolio diff --git a/atst/models/role.py b/atst/models/permission_set.py similarity index 84% rename from atst/models/role.py rename to atst/models/permission_set.py index 9bf81d7e..cb5ddb92 100644 --- a/atst/models/role.py +++ b/atst/models/permission_set.py @@ -5,8 +5,8 @@ from sqlalchemy.orm.attributes import flag_modified from atst.models import Base, types, mixins -class Role(Base, mixins.TimestampsMixin): - __tablename__ = "roles" +class PermissionSet(Base, mixins.TimestampsMixin): + __tablename__ = "permission_sets" id = types.Id() name = Column(String, index=True, unique=True, nullable=False) @@ -27,6 +27,6 @@ class Role(Base, mixins.TimestampsMixin): flag_modified(self, "permissions") def __repr__(self): - return "".format( + return "".format( self.name, self.description, self.permissions, self.id ) diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index 8d1afb97..be9a6983 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -30,10 +30,10 @@ class Status(Enum): portfolio_roles_roles = Table( - "portfolio_roles_roles", + "portfolio_roles_permission_sets", Base.metadata, Column("portfolio_role_id", UUID(as_uuid=True), ForeignKey("portfolio_roles.id")), - Column("role_id", UUID(as_uuid=True), ForeignKey("roles.id")), + Column("permission_set_id", UUID(as_uuid=True), ForeignKey("permission_sets.id")), ) @@ -52,7 +52,7 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING) - permission_sets = relationship("Role", secondary=portfolio_roles_roles) + permission_sets = relationship("PermissionSet", secondary=portfolio_roles_roles) @property def permissions(self): diff --git a/atst/models/user.py b/atst/models/user.py index f3f40044..709672cc 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -11,9 +11,9 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): id = types.Id() username = Column(String) - atat_role_id = Column(UUID(as_uuid=True), ForeignKey("roles.id")) + atat_role_id = Column(UUID(as_uuid=True), ForeignKey("permission_sets.id")) - atat_role = relationship("Role") + atat_role = relationship("PermissionSet") portfolio_roles = relationship("PortfolioRole", backref="user") email = Column(String, unique=True) diff --git a/script/remove_sample_data.py b/script/remove_sample_data.py index f7a5f806..6528db8d 100644 --- a/script/remove_sample_data.py +++ b/script/remove_sample_data.py @@ -16,7 +16,7 @@ from atst.models.audit_event import AuditEvent from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole from atst.models.application import Application -from atst.models.role import Role +from atst.models.permission_set import PermissionSet from atst.models.user import User from atst.models.portfolio_role import PortfolioRole from atst.models.portfolio import Portfolio diff --git a/script/seed_roles.py b/script/seed_roles.py index 5e1cb4f1..fc93c523 100755 --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -9,23 +9,35 @@ sys.path.append(parent_dir) from sqlalchemy.orm.exc import NoResultFound from atst.app import make_config, make_app from atst.database import db -from atst.models import Role, Permissions -from atst.domain.roles import ATAT_ROLES, PORTFOLIO_ROLES, PORTFOLIO_PERMISSION_SETS +from atst.models import PermissionSet, Permissions +from atst.domain.permission_sets import ( + ATAT_ROLES, + PORTFOLIO_ROLES, + PORTFOLIO_PERMISSION_SETS, +) def seed_roles(): - for role_info in ATAT_ROLES + PORTFOLIO_ROLES + PORTFOLIO_PERMISSION_SETS: - role = Role(**role_info) + for permission_set_info in ATAT_ROLES + PORTFOLIO_ROLES + PORTFOLIO_PERMISSION_SETS: + permission_set = PermissionSet(**permission_set_info) try: - existing_role = db.session.query(Role).filter_by(name=role.name).one() - existing_role.description = role.description - existing_role.permissions = role.permissions - existing_role.display_name = role.display_name - db.session.add(existing_role) - print("Updated existing role {}".format(existing_role.name)) + existing_permission_set = ( + db.session.query(PermissionSet) + .filter_by(name=permission_set.name) + .one() + ) + existing_permission_set.description = permission_set.description + existing_permission_set.permissions = permission_set.permissions + existing_permission_set.display_name = permission_set.display_name + db.session.add(existing_permission_set) + print( + "Updated existing permission_set {}".format( + existing_permission_set.name + ) + ) except NoResultFound: - db.session.add(role) - print("Added new role {}".format(role.name)) + db.session.add(permission_set) + print("Added new permission_set {}".format(permission_set.name)) db.session.commit() diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 6686379a..dd4baaa5 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -2,7 +2,7 @@ import pytest from atst.domain.audit_log import AuditLog from atst.domain.exceptions import UnauthorizedError -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( UserFactory, diff --git a/tests/domain/test_authz.py b/tests/domain/test_authz.py index 3467dcbe..c240e523 100644 --- a/tests/domain/test_authz.py +++ b/tests/domain/test_authz.py @@ -2,7 +2,7 @@ import pytest from tests.factories import TaskOrderFactory, UserFactory, PortfolioRoleFactory from atst.domain.authz import Authorization -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import UnauthorizedError from atst.models.permissions import Permissions @@ -45,8 +45,8 @@ def test_check_is_ko_or_cor(task_order, invalid_user): def test_has_portfolio_permission(): - role_one = Roles.get("view_portfolio_funding") - role_two = Roles.get("view_portfolio_reports") + role_one = PermissionSets.get("view_portfolio_funding") + role_two = PermissionSets.get("view_portfolio_reports") port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) different_user = UserFactory.create() assert Authorization.has_portfolio_permission( diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 623e6835..e7108c2e 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -1,7 +1,7 @@ from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.users import Users from atst.models.portfolio_role import Status as PortfolioRoleStatus -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from tests.factories import ( PortfolioFactory, diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 54b0f740..fd2a58b7 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -6,7 +6,7 @@ from atst.domain.portfolios import Portfolios, PortfolioError from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments -from atst.domain.roles import Roles, PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import UserFactory, PortfolioRoleFactory, PortfolioFactory @@ -201,7 +201,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin( ) admin = UserFactory.from_atat_role("default") - perm_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + perm_sets = [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] PortfolioRoleFactory.create( user=admin, portfolio=portfolio, permission_sets=perm_sets ) @@ -263,7 +263,7 @@ def test_get_for_update_information(portfolio, portfolio_owner): assert portfolio == owner_ws admin = UserFactory.create() - perm_sets = [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + perm_sets = [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] PortfolioRoleFactory.create( user=admin, portfolio=portfolio, permission_sets=perm_sets ) diff --git a/tests/domain/test_roles.py b/tests/domain/test_roles.py index b0da57f1..997627db 100644 --- a/tests/domain/test_roles.py +++ b/tests/domain/test_roles.py @@ -1,18 +1,18 @@ import pytest -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import NotFoundError def test_get_all_roles(): - roles = Roles.get_all() + roles = PermissionSets.get_all() assert roles def test_get_existing_role(): - role = Roles.get("developer") + role = PermissionSets.get("developer") assert role.name == "developer" def test_get_nonexistent_role(): with pytest.raises(NotFoundError): - Roles.get("nonexistent") + PermissionSets.get("nonexistent") diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index ef99f014..fe62f17a 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -2,7 +2,7 @@ import pytest from atst.domain.task_orders import TaskOrders, TaskOrderError, DD254s from atst.domain.exceptions import UnauthorizedError -from atst.domain.roles import Roles, _VIEW_PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import PermissionSets, _VIEW_PORTFOLIO_PERMISSION_SETS from atst.models.attachment import Attachment from tests.factories import ( @@ -116,7 +116,7 @@ def test_task_order_access(): user=member, portfolio=task_order.portfolio, permission_sets=[ - Roles.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS + PermissionSets.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS ], ) TaskOrders.add_officer( diff --git a/tests/factories.py b/tests/factories.py index f203c5f1..97cfd3f2 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -12,10 +12,10 @@ from atst.models.environment import Environment from atst.models.application import Application from atst.models.task_order import TaskOrder from atst.models.user import User -from atst.models.role import Role +from atst.models.permission_set import PermissionSet from atst.models.portfolio import Portfolio -from atst.domain.roles import ( - Roles, +from atst.domain.permission_sets import ( + PermissionSets, PORTFOLIO_ROLES, PORTFOLIO_PERMISSION_SETS, _VIEW_PORTFOLIO_PERMISSION_SETS, @@ -70,11 +70,13 @@ def _random_date(year_min, year_max, operation): def base_portfolio_permission_sets(): - return [Roles.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS] + return [ + PermissionSets.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS + ] def get_all_portfolio_permission_sets(): - return [Roles.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + return [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] class Base(factory.alchemy.SQLAlchemyModelFactory): @@ -91,7 +93,7 @@ class UserFactory(Base): email = factory.Faker("email") first_name = factory.Faker("first_name") last_name = factory.Faker("last_name") - atat_role = factory.LazyFunction(lambda: Roles.get("default")) + atat_role = factory.LazyFunction(lambda: PermissionSets.get("default")) dod_id = factory.LazyFunction(random_dod_id) phone_number = factory.LazyFunction(random_phone_number) service_branch = factory.LazyFunction(random_service_branch) @@ -104,7 +106,7 @@ class UserFactory(Base): @classmethod def from_atat_role(cls, atat_role_name, **kwargs): - role = Roles.get(atat_role_name) + role = PermissionSets.get(atat_role_name) return cls.create(atat_role=role, **kwargs) @@ -142,7 +144,8 @@ class PortfolioFactory(Base): perms_set = None if member.get("permissions_sets"): perms_set = [ - Roles.get(perm_set) for perm_set in member.get("permission_sets") + PermissionSets.get(perm_set) + for perm_set in member.get("permission_sets") ] else: perms_set = [] diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 218ce966..0a484624 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -4,9 +4,8 @@ import datetime from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.domain.applications import Applications -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.models.portfolio_role import Status -from atst.models.role import Role from atst.models.invitation import Status as InvitationStatus from atst.models.audit_event import AuditEvent from atst.models.portfolio_role import Status as PortfolioRoleStatus @@ -286,8 +285,8 @@ def test_can_list_all_environments(): def test_can_list_all_permissions(): - role_one = Roles.get("view_portfolio_funding") - role_two = Roles.get("view_portfolio_reports") + role_one = PermissionSets.get("view_portfolio_funding") + role_two = PermissionSets.get("view_portfolio_reports") port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) expected_perms = role_one.permissions + role_two.permissions assert expected_perms == expected_perms diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index bac4fe39..20e927f6 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -12,7 +12,7 @@ from tests.factories import ( from atst.domain.applications import Applications from atst.domain.portfolios import Portfolios -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.models.portfolio_role import Status as PortfolioRoleStatus diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index af63da9a..06957f18 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -12,7 +12,7 @@ from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.queue import queue from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.invitation import Status as InvitationStatus @@ -139,7 +139,7 @@ def test_update_member_portfolio_role(client, user_session): follow_redirects=True, ) assert response.status_code == 200 - edit_funding = Roles.get("edit_portfolio_funding") + edit_funding = PermissionSets.get("edit_portfolio_funding") assert edit_funding in member.permission_sets diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 0b74dc57..b694e2a3 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -2,7 +2,7 @@ from flask import url_for import pytest from datetime import timedelta, date -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.domain.task_orders import TaskOrders from atst.models.portfolio_role import Status as PortfolioStatus from atst.models.invitation import Status as InvitationStatus @@ -234,8 +234,8 @@ def test_ko_can_view_task_order(client, user_session, portfolio, user): user=user, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=user) @@ -301,8 +301,8 @@ def test_ko_can_view_ko_review_page(client, user_session): user=ko, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) PortfolioRoleFactory.create( @@ -310,8 +310,8 @@ def test_ko_can_view_ko_review_page(client, user_session): user=cor, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) task_order = TaskOrderFactory.create( @@ -378,8 +378,8 @@ def test_cor_redirected_to_build_page(client, user_session, portfolio): user=cor, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) task_order = TaskOrderFactory.create( @@ -400,8 +400,8 @@ def test_submit_completed_ko_review_page_as_cor( user=user, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) @@ -448,8 +448,8 @@ def test_submit_completed_ko_review_page_as_ko( user=ko, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) @@ -492,8 +492,8 @@ def test_so_review_page(app, client, user_session, portfolio): user=so, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) @@ -533,8 +533,8 @@ def test_submit_so_review(app, client, user_session, portfolio): user=so, status=PortfolioStatus.ACTIVE, permission_sets=[ - Roles.get("view_portfolio"), - Roles.get("view_portfolio_funding"), + PermissionSets.get("view_portfolio"), + PermissionSets.get("view_portfolio_funding"), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) diff --git a/tests/test_auth.py b/tests/test_auth.py index 30f8c47b..af467fdb 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -4,7 +4,7 @@ import pytest from flask import session, url_for from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS from atst.domain.users import Users -from atst.domain.roles import Roles +from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import NotFoundError from atst.domain.auth import UNPROTECTED_ROUTES from .factories import UserFactory @@ -48,7 +48,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True ) - role = Roles.get("ccpo") + role = PermissionSets.get("ccpo") monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.get_user", lambda *args: UserFactory.create(atat_role=role), From d3c3209fc013c70f3d961a4cb17fe8a30706e231 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 11:24:03 -0400 Subject: [PATCH 10/20] cleanup unused portfolio roles --- atst/domain/permission_sets.py | 106 ------------------------ atst/forms/data.py | 7 -- atst/routes/portfolios/members.py | 7 +- script/seed_roles.py | 8 +- templates/portfolios/members/index.html | 12 --- tests/domain/test_audit_log.py | 2 +- tests/domain/test_environments.py | 8 +- tests/domain/test_portfolios.py | 12 +-- tests/domain/test_roles.py | 4 +- tests/domain/test_users.py | 30 +++---- tests/factories.py | 1 - tests/models/test_environments.py | 2 +- 12 files changed, 32 insertions(+), 167 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index 4c70b4e2..b162a15c 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -54,112 +54,6 @@ ATAT_ROLES = [ "permissions": [Permissions.REQUEST_JEDI_PORTFOLIO], }, ] -PORTFOLIO_ROLES = [ - { - "name": "owner", - "display_name": "Portfolio Owner", - "description": "Adds, edits, deactivates access to all applications, environments, and members. Views budget reports. Initiates and edits JEDI Cloud requests.", - "permissions": [ - Permissions.REQUEST_JEDI_PORTFOLIO, - Permissions.VIEW_ORIGINAL_JEDI_REQEUST, - Permissions.VIEW_USAGE_REPORT, - Permissions.VIEW_USAGE_DOLLARS, - Permissions.ADD_AND_ASSIGN_CSP_ROLES, - Permissions.REMOVE_CSP_ROLES, - Permissions.REQUEST_NEW_CSP_ROLE, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - Permissions.VIEW_ASSIGNED_ATAT_ROLE_CONFIGURATIONS, - Permissions.VIEW_ASSIGNED_CSP_ROLE_CONFIGURATIONS, - Permissions.DEACTIVATE_PORTFOLIO, - Permissions.VIEW_ATAT_PERMISSIONS, - Permissions.VIEW_PORTFOLIO, - Permissions.VIEW_PORTFOLIO_MEMBERS, - Permissions.EDIT_PORTFOLIO_INFORMATION, - Permissions.ADD_APPLICATION_IN_PORTFOLIO, - Permissions.DELETE_APPLICATION_IN_PORTFOLIO, - Permissions.DEACTIVATE_APPLICATION_IN_PORTFOLIO, - Permissions.VIEW_APPLICATION_IN_PORTFOLIO, - Permissions.RENAME_APPLICATION_IN_PORTFOLIO, - Permissions.ADD_ENVIRONMENT_IN_APPLICATION, - Permissions.DELETE_ENVIRONMENT_IN_APPLICATION, - Permissions.DEACTIVATE_ENVIRONMENT_IN_APPLICATION, - Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, - Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, - Permissions.VIEW_PORTFOLIO_AUDIT_LOG, - Permissions.VIEW_TASK_ORDER, - Permissions.UPDATE_TASK_ORDER, - Permissions.ADD_TASK_ORDER_OFFICER, - ], - }, - { - "name": "admin", - "display_name": "Administrator", - "description": "Adds and edits applications, environments, members, but cannot deactivate. Cannot view budget reports or JEDI Cloud requests.", - "permissions": [ - Permissions.VIEW_USAGE_REPORT, - Permissions.ADD_AND_ASSIGN_CSP_ROLES, - Permissions.REMOVE_CSP_ROLES, - Permissions.REQUEST_NEW_CSP_ROLE, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - Permissions.VIEW_ASSIGNED_ATAT_ROLE_CONFIGURATIONS, - Permissions.VIEW_ASSIGNED_CSP_ROLE_CONFIGURATIONS, - Permissions.VIEW_PORTFOLIO, - Permissions.VIEW_PORTFOLIO_MEMBERS, - Permissions.EDIT_PORTFOLIO_INFORMATION, - Permissions.ADD_APPLICATION_IN_PORTFOLIO, - Permissions.DELETE_APPLICATION_IN_PORTFOLIO, - Permissions.DEACTIVATE_APPLICATION_IN_PORTFOLIO, - Permissions.VIEW_APPLICATION_IN_PORTFOLIO, - Permissions.RENAME_APPLICATION_IN_PORTFOLIO, - Permissions.ADD_ENVIRONMENT_IN_APPLICATION, - Permissions.DELETE_ENVIRONMENT_IN_APPLICATION, - Permissions.DEACTIVATE_ENVIRONMENT_IN_APPLICATION, - Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, - Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, - Permissions.VIEW_PORTFOLIO_AUDIT_LOG, - Permissions.VIEW_TASK_ORDER, - Permissions.UPDATE_TASK_ORDER, - Permissions.ADD_TASK_ORDER_OFFICER, - ], - }, - { - "name": "developer", - "display_name": "Developer", - "description": "Views only the applications and environments they are granted access to. Can also view members associated with each environment.", - "permissions": [Permissions.VIEW_USAGE_REPORT, Permissions.VIEW_PORTFOLIO], - }, - { - "name": "billing_auditor", - "display_name": "Billing Auditor", - "description": "Views only the applications and environments they are granted access to. Can also view budgets and reports associated with the portfolio.", - "permissions": [ - Permissions.VIEW_USAGE_REPORT, - Permissions.VIEW_USAGE_DOLLARS, - Permissions.VIEW_PORTFOLIO, - ], - }, - { - "name": "security_auditor", - "description": "Views only the applications and environments they are granted access to. Can also view activity logs.", - "display_name": "Security Auditor", - "permissions": [ - Permissions.VIEW_ASSIGNED_ATAT_ROLE_CONFIGURATIONS, - Permissions.VIEW_ASSIGNED_CSP_ROLE_CONFIGURATIONS, - Permissions.VIEW_ATAT_PERMISSIONS, - Permissions.VIEW_PORTFOLIO, - ], - }, - { - "name": "officer", - "description": "Officer involved with setting up a Task Order", - "display_name": "Task Order Officer", - "permissions": [ - Permissions.VIEW_PORTFOLIO, - Permissions.VIEW_USAGE_REPORT, - Permissions.VIEW_USAGE_DOLLARS, - ], - }, -] _VIEW_PORTFOLIO_PERMISSION_SETS = [ { diff --git a/atst/forms/data.py b/atst/forms/data.py index 3a515f6d..4223de08 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,4 +1,3 @@ -from atst.domain.permission_sets import PORTFOLIO_ROLES as PORTFOLIO_ROLE_DEFINITIONS from atst.utils.localization import translate, translate_duration @@ -107,12 +106,6 @@ COMPLETION_DATE_RANGES = [ ("Above 12 months", "Above 12 months"), ] -PORTFOLIO_ROLES = [ - (role["name"], {"name": role["display_name"], "description": role["description"]}) - for role in PORTFOLIO_ROLE_DEFINITIONS - if role["name"] is not "officer" -] - ENVIRONMENT_ROLES = [ ( "developer", diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index 30ba503b..55383529 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -11,11 +11,7 @@ from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.services.invitation import Invitation as InvitationService import atst.forms.portfolio_member as member_forms -from atst.forms.data import ( - ENVIRONMENT_ROLES, - ENV_ROLE_MODAL_DESCRIPTION, - PORTFOLIO_ROLE_DEFINITIONS, -) +from atst.forms.data import ENVIRONMENT_ROLES, ENV_ROLE_MODAL_DESCRIPTION from atst.domain.authz import Authorization from atst.models.permissions import Permissions @@ -45,7 +41,6 @@ def portfolio_members(portfolio_id): return render_template( "portfolios/members/index.html", portfolio=portfolio, - role_choices=PORTFOLIO_ROLE_DEFINITIONS, status_choices=MEMBER_STATUS_CHOICES, members=members_list, ) diff --git a/script/seed_roles.py b/script/seed_roles.py index fc93c523..0a2a2703 100755 --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -10,15 +10,11 @@ from sqlalchemy.orm.exc import NoResultFound from atst.app import make_config, make_app from atst.database import db from atst.models import PermissionSet, Permissions -from atst.domain.permission_sets import ( - ATAT_ROLES, - PORTFOLIO_ROLES, - PORTFOLIO_PERMISSION_SETS, -) +from atst.domain.permission_sets import ATAT_ROLES, PORTFOLIO_PERMISSION_SETS def seed_roles(): - for permission_set_info in ATAT_ROLES + PORTFOLIO_ROLES + PORTFOLIO_PERMISSION_SETS: + for permission_set_info in ATAT_ROLES + PORTFOLIO_PERMISSION_SETS: permission_set = PermissionSet(**permission_set_info) try: existing_permission_set = ( diff --git a/templates/portfolios/members/index.html b/templates/portfolios/members/index.html index ffc91cf9..c55835fc 100644 --- a/templates/portfolios/members/index.html +++ b/templates/portfolios/members/index.html @@ -29,7 +29,6 @@ id="search-template" class='member-list' v-bind:members='{{ members | tojson}}' - v-bind:role_choices='{{ role_choices | tojson}}' v-bind:status_choices='{{ status_choices | tojson}}'>
@@ -50,17 +49,6 @@ {% endfor %}
- -
- - -
diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index dd4baaa5..94128e89 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -19,7 +19,7 @@ def ccpo(): @pytest.fixture(scope="function") def developer(): - return UserFactory.from_atat_role("default") + return UserFactory.create() def test_non_admin_cannot_view_audit_log(developer): diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 8936d1be..78cc8b0d 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -14,7 +14,7 @@ def test_create_environments(): def test_create_environment_role_creates_cloud_id(session): owner = UserFactory.create() - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() portfolio = PortfolioFactory.create( owner=owner, @@ -38,7 +38,7 @@ def test_create_environment_role_creates_cloud_id(session): def test_update_environment_roles(): owner = UserFactory.create() - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() portfolio = PortfolioFactory.create( owner=owner, @@ -81,7 +81,7 @@ def test_update_environment_roles(): def test_remove_environment_role(): owner = UserFactory.create() - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() portfolio = PortfolioFactory.create( owner=owner, members=[{"user": developer, "role_name": "developer"}], @@ -132,7 +132,7 @@ def test_remove_environment_role(): def test_no_update_to_environment_roles(): owner = UserFactory.create() - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() portfolio = PortfolioFactory.create( owner=owner, diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index fd2a58b7..fe2b7672 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -151,7 +151,7 @@ def test_ccpo_can_view_portfolio_members(portfolio, portfolio_owner): def test_random_user_cannot_view_portfolio_members(portfolio): - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() with pytest.raises(UnauthorizedError): portfolio = Portfolios.get_with_members(developer, portfolio.id) @@ -175,7 +175,7 @@ def test_scoped_portfolio_only_returns_a_users_applications_and_environments( "My application 2", ["dev", "staging", "prod"], ) - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() dev_environment = Environments.add_member( new_application.environments[0], developer, "developer" ) @@ -200,7 +200,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin( ["dev", "staging", "prod"], ) - admin = UserFactory.from_atat_role("default") + admin = UserFactory.create() perm_sets = [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] PortfolioRoleFactory.create( user=admin, portfolio=portfolio, permission_sets=perm_sets @@ -230,7 +230,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_owner( def test_for_user_returns_active_portfolios_for_user(portfolio, portfolio_owner): - bob = UserFactory.from_atat_role("default") + bob = UserFactory.create() PortfolioRoleFactory.create( user=bob, portfolio=portfolio, status=PortfolioRoleStatus.ACTIVE ) @@ -242,7 +242,7 @@ def test_for_user_returns_active_portfolios_for_user(portfolio, portfolio_owner) def test_for_user_does_not_return_inactive_portfolios(portfolio, portfolio_owner): - bob = UserFactory.from_atat_role("default") + bob = UserFactory.create() Portfolios.add_member(portfolio, bob) PortfolioFactory.create() bobs_portfolios = Portfolios.for_user(bob) @@ -274,7 +274,7 @@ def test_get_for_update_information(portfolio, portfolio_owner): # ccpo = UserFactory.from_atat_role("ccpo") # assert Portfolios.get_for_update_information(ccpo, portfolio.id) - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() with pytest.raises(UnauthorizedError): Portfolios.get_for_update_information(developer, portfolio.id) diff --git a/tests/domain/test_roles.py b/tests/domain/test_roles.py index 997627db..58601d9f 100644 --- a/tests/domain/test_roles.py +++ b/tests/domain/test_roles.py @@ -9,8 +9,8 @@ def test_get_all_roles(): def test_get_existing_role(): - role = PermissionSets.get("developer") - assert role.name == "developer" + role = PermissionSets.get("portfolio_poc") + assert role.name == "portfolio_poc" def test_get_nonexistent_role(): diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index de6c7fc8..69a83c69 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -8,14 +8,14 @@ DOD_ID = "my_dod_id" def test_create_user(): - user = Users.create(DOD_ID, "developer") - assert user.atat_role.name == "developer" + user = Users.create(DOD_ID, "default") + assert user.atat_role.name == "default" def test_create_user_with_existing_email(): - Users.create(DOD_ID, "developer", email="thisusersemail@usersRus.com") + Users.create(DOD_ID, "default", email="thisusersemail@usersRus.com") with pytest.raises(AlreadyExistsError): - Users.create(DOD_ID, "admin", email="thisusersemail@usersRus.com") + Users.create(DOD_ID, "ccpo", email="thisusersemail@usersRus.com") def test_create_user_with_nonexistent_role(): @@ -24,61 +24,61 @@ def test_create_user_with_nonexistent_role(): def test_get_or_create_nonexistent_user(): - user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="developer") + user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") assert user.dod_id == DOD_ID def test_get_or_create_existing_user(): - Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="developer") - user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="developer") + Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") + user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") assert user def test_get_user(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") user = Users.get(new_user.id) assert user.id == new_user.id def test_get_nonexistent_user(): - Users.create(DOD_ID, "developer") + Users.create(DOD_ID, "default") with pytest.raises(NotFoundError): Users.get(uuid4()) def test_get_user_by_dod_id(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") user = Users.get_by_dod_id(DOD_ID) assert user == new_user def test_update_role(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") updated_user = Users.update_role(new_user.id, "ccpo") assert updated_user.atat_role.name == "ccpo" def test_update_role_with_nonexistent_user(): - Users.create(DOD_ID, "developer") + Users.create(DOD_ID, "default") with pytest.raises(NotFoundError): Users.update_role(uuid4(), "ccpo") def test_update_existing_user_with_nonexistent_role(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") with pytest.raises(NotFoundError): Users.update_role(new_user.id, "nonexistent") def test_update_user(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") updated_user = Users.update(new_user, {"first_name": "Jabba"}) assert updated_user.first_name == "Jabba" def test_update_user_with_dod_id(): - new_user = Users.create(DOD_ID, "developer") + new_user = Users.create(DOD_ID, "default") with pytest.raises(UnauthorizedError) as excinfo: Users.update(new_user, {"dod_id": "1234567890"}) diff --git a/tests/factories.py b/tests/factories.py index 97cfd3f2..23ed2a86 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -16,7 +16,6 @@ from atst.models.permission_set import PermissionSet from atst.models.portfolio import Portfolio from atst.domain.permission_sets import ( PermissionSets, - PORTFOLIO_ROLES, PORTFOLIO_PERMISSION_SETS, _VIEW_PORTFOLIO_PERMISSION_SETS, _EDIT_PORTFOLIO_PERMISSION_SETS, diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 1e415efa..0dad7874 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -5,7 +5,7 @@ from tests.factories import PortfolioFactory, UserFactory def test_add_user_to_environment(): owner = UserFactory.create() - developer = UserFactory.from_atat_role("developer") + developer = UserFactory.create() portfolio = PortfolioFactory.create(owner=owner) application = Applications.create( From 78aa2dfcc6f85ffc726b969bcc58d2436fb48130 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 06:46:40 -0400 Subject: [PATCH 11/20] permission set names should be constants on the domain class --- atst/domain/permission_sets.py | 59 ++++++++++++--------- atst/domain/portfolio_roles.py | 8 +-- atst/domain/task_orders.py | 4 +- atst/forms/portfolio_member.py | 17 +++--- atst/models/portfolio.py | 3 +- tests/domain/test_authz.py | 4 +- tests/domain/test_portfolio_roles.py | 12 ++--- tests/domain/test_portfolios.py | 2 +- tests/models/test_portfolio_role.py | 4 +- tests/routes/portfolios/test_invitations.py | 9 ++-- tests/routes/portfolios/test_members.py | 15 +++--- tests/routes/portfolios/test_task_orders.py | 32 +++++------ 12 files changed, 93 insertions(+), 76 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index b162a15c..3de4492e 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -5,6 +5,32 @@ from atst.models import PermissionSet, Permissions from .exceptions import NotFoundError +class PermissionSets(object): + VIEW_PORTFOLIO = "view_portfolio" + VIEW_PORTFOLIO_APPLICATION_MANAGEMENT = "view_portfolio_application_management" + VIEW_PORTFOLIO_FUNDING = "view_portfolio_funding" + VIEW_PORTFOLIO_REPORTS = "view_portfolio_reports" + VIEW_PORTFOLIO_ADMIN = "view_portfolio_admin" + EDIT_PORTFOLIO_APPLICATION_MANAGEMENT = "edit_portfolio_application_management" + EDIT_PORTFOLIO_FUNDING = "edit_portfolio_funding" + EDIT_PORTFOLIO_REPORTS = "edit_portfolio_reports" + EDIT_PORTFOLIO_ADMIN = "edit_portfolio_admin" + PORTFOLIO_POC = "portfolio_poc" + + @classmethod + def get(cls, perms_set_name): + try: + role = db.session.query(PermissionSet).filter_by(name=perms_set_name).one() + except NoResultFound: + raise NotFoundError("permission_set") + + return role + + @classmethod + def get_all(cls): + return db.session.query(PermissionSet).all() + + ATAT_ROLES = [ { "name": "ccpo", @@ -57,13 +83,13 @@ ATAT_ROLES = [ _VIEW_PORTFOLIO_PERMISSION_SETS = [ { - "name": "view_portfolio", + "name": PermissionSets.VIEW_PORTFOLIO, "description": "View basic portfolio info", "display_name": "View Portfolio", "permissions": [Permissions.VIEW_PORTFOLIO], }, { - "name": "view_portfolio_application_management", + "name": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, "description": "View applications and related resources", "display_name": "Application Management", "permissions": [ @@ -73,7 +99,7 @@ _VIEW_PORTFOLIO_PERMISSION_SETS = [ ], }, { - "name": "view_portfolio_funding", + "name": PermissionSets.VIEW_PORTFOLIO_FUNDING, "description": "View a portfolio's task orders", "display_name": "Funding", "permissions": [ @@ -82,13 +108,13 @@ _VIEW_PORTFOLIO_PERMISSION_SETS = [ ], }, { - "name": "view_portfolio_reports", + "name": PermissionSets.VIEW_PORTFOLIO_REPORTS, "description": "View a portfolio's reports", "display_name": "Reporting", "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], }, { - "name": "view_portfolio_admin", + "name": PermissionSets.VIEW_PORTFOLIO_ADMIN, "description": "View a portfolio's admin options", "display_name": "Portfolio Administration", "permissions": [ @@ -103,7 +129,7 @@ _VIEW_PORTFOLIO_PERMISSION_SETS = [ _EDIT_PORTFOLIO_PERMISSION_SETS = [ { - "name": "edit_portfolio_application_management", + "name": PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, "description": "Edit applications and related resources", "display_name": "Application Management", "permissions": [ @@ -116,7 +142,7 @@ _EDIT_PORTFOLIO_PERMISSION_SETS = [ ], }, { - "name": "edit_portfolio_funding", + "name": PermissionSets.EDIT_PORTFOLIO_FUNDING, "description": "Edit a portfolio's task orders and add new ones", "display_name": "Funding", "permissions": [ @@ -125,13 +151,13 @@ _EDIT_PORTFOLIO_PERMISSION_SETS = [ ], }, { - "name": "edit_portfolio_reports", + "name": PermissionSets.EDIT_PORTFOLIO_REPORTS, "description": "Edit a portfolio's reports (no-op)", "display_name": "Reporting", "permissions": [], }, { - "name": "edit_portfolio_admin", + "name": PermissionSets.EDIT_PORTFOLIO_ADMIN, "description": "Edit a portfolio's admin options", "display_name": "Portfolio Administration", "permissions": [ @@ -157,18 +183,3 @@ PORTFOLIO_PERMISSION_SETS = ( } ] ) - - -class PermissionSets(object): - @classmethod - def get(cls, perms_set_name): - try: - role = db.session.query(PermissionSet).filter_by(name=perms_set_name).one() - except NoResultFound: - raise NotFoundError("permission_set") - - return role - - @classmethod - def get_all(cls): - return db.session.query(PermissionSet).all() diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index 19e5125b..204ef80f 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -97,10 +97,10 @@ class PortfolioRoles(object): return new_portfolio_role _DEFAULT_PORTFOLIO_PERMS_SETS = { - "view_portfolio_application_management", - "view_portfolio_funding", - "view_portfolio_reports", - "view_portfolio_admin", + PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.VIEW_PORTFOLIO_FUNDING, + PermissionSets.VIEW_PORTFOLIO_REPORTS, + PermissionSets.VIEW_PORTFOLIO_ADMIN, } @classmethod diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 3f3a5b51..3055f31d 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -7,6 +7,7 @@ from atst.models.permissions import Permissions from atst.models.dd_254 import DD254 from atst.domain.portfolios import Portfolios from atst.domain.authz import Authorization +from atst.domain.permission_sets import PermissionSets from .exceptions import NotFoundError @@ -174,8 +175,7 @@ class TaskOrders(object): portfolio, { **officer_data, - "portfolio_role": "officer", - "permission_sets": ["edit_portfolio_funding"], + "permission_sets": [PermissionSets.EDIT_PORTFOLIO_FUNDING], }, ) portfolio_user = member.user diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py index f75c2869..36e0d3cd 100644 --- a/atst/forms/portfolio_member.py +++ b/atst/forms/portfolio_member.py @@ -2,6 +2,7 @@ from wtforms.fields import StringField from wtforms.fields.html5 import EmailField from wtforms.validators import Required, Email, Length +from atst.domain.permission_sets import PermissionSets from .forms import BaseForm from atst.forms.validators import IsNumber from atst.forms.fields import SelectField @@ -12,29 +13,29 @@ class PermissionsForm(BaseForm): perms_app_mgmt = SelectField( None, choices=[ - ("view_portfolio_application_management", "View Only"), - ("edit_portfolio_application_management", "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, "View Only"), + (PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, "Edit Access"), ], ) perms_funding = SelectField( None, choices=[ - ("view_portfolio_funding", "View Only"), - ("edit_portfolio_funding", "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_FUNDING, "View Only"), + (PermissionSets.EDIT_PORTFOLIO_FUNDING, "Edit Access"), ], ) perms_reporting = SelectField( None, choices=[ - ("view_portfolio_reports", "View Only"), - ("edit_portfolio_reports", "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_REPORTS, "View Only"), + (PermissionSets.EDIT_PORTFOLIO_REPORTS, "Edit Access"), ], ) perms_portfolio_mgmt = SelectField( None, choices=[ - ("view_portfolio_admin", "View Only"), - ("edit_portfolio_admin", "Edit Access"), + (PermissionSets.VIEW_PORTFOLIO_ADMIN, "View Only"), + (PermissionSets.EDIT_PORTFOLIO_ADMIN, "Edit Access"), ], ) diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 0b133ae1..599de3e6 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -4,6 +4,7 @@ from itertools import chain from atst.models import Base, mixins, types from atst.models.portfolio_role import PortfolioRole, Status as PortfolioRoleStatus +from atst.domain.permission_sets import PermissionSets from atst.utils import first_or_none from atst.database import db @@ -23,7 +24,7 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def owner(self): def _is_portfolio_owner(portfolio_role): - return "portfolio_poc" in [ + return PermissionSets.PORTFOLIO_POC in [ perms_set.name for perms_set in portfolio_role.permission_sets ] diff --git a/tests/domain/test_authz.py b/tests/domain/test_authz.py index c240e523..fdf72fdd 100644 --- a/tests/domain/test_authz.py +++ b/tests/domain/test_authz.py @@ -45,8 +45,8 @@ def test_check_is_ko_or_cor(task_order, invalid_user): def test_has_portfolio_permission(): - role_one = PermissionSets.get("view_portfolio_funding") - role_two = PermissionSets.get("view_portfolio_reports") + role_one = PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING) + role_two = PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_REPORTS) port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) different_user = UserFactory.create() assert Authorization.has_portfolio_permission( diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index e7108c2e..8ffbed81 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -14,17 +14,17 @@ from tests.factories import ( def test_add_portfolio_role_with_permission_sets(): portfolio = PortfolioFactory.create() new_user = UserFactory.create() - permission_sets = ["edit_portfolio_application_management"] + permission_sets = [PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT] port_role = PortfolioRoles.add( new_user, portfolio.id, permission_sets=permission_sets ) assert len(port_role.permission_sets) == 5 expected_names = [ - "edit_portfolio_application_management", - "view_portfolio_application_management", - "view_portfolio_funding", - "view_portfolio_reports", - "view_portfolio_admin", + PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.VIEW_PORTFOLIO_FUNDING, + PermissionSets.VIEW_PORTFOLIO_REPORTS, + PermissionSets.VIEW_PORTFOLIO_ADMIN, ] actual_names = [prms.name for prms in port_role.permission_sets] assert expected_names == expected_names diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index fe2b7672..9140d82d 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -114,7 +114,7 @@ def test_update_portfolio_role_role(portfolio, portfolio_owner): } PortfolioRoleFactory._meta.sqlalchemy_session_persistence = "flush" member = PortfolioRoleFactory.create(portfolio=portfolio) - permission_sets = ["edit_portfolio_funding"] + permission_sets = [PermissionSets.EDIT_PORTFOLIO_FUNDING] updated_member = Portfolios.update_member( portfolio_owner, portfolio, member, permission_sets=permission_sets diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 0a484624..7c67fe5d 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -285,8 +285,8 @@ def test_can_list_all_environments(): def test_can_list_all_permissions(): - role_one = PermissionSets.get("view_portfolio_funding") - role_two = PermissionSets.get("view_portfolio_reports") + role_one = PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING) + role_two = PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_REPORTS) port_role = PortfolioRoleFactory.create(permission_sets=[role_one, role_two]) expected_perms = role_one.permissions + role_two.permissions assert expected_perms == expected_perms diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index e6ea7a95..7719a380 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -12,6 +12,7 @@ from atst.domain.portfolios import Portfolios from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.invitation import Status as InvitationStatus from atst.domain.users import Users +from atst.domain.permission_sets import PermissionSets def test_existing_member_accepts_valid_invite(client, user_session): @@ -48,10 +49,10 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): response = client.post( url_for("portfolios.create_member", portfolio_id=portfolio.id), data={ - "perms_app_mgmt": "view_portfolio_application_management", - "perms_funding": "view_portfolio_funding", - "perms_reporting": "view_portfolio_reports", - "perms_portfolio_mgmt": "view_portfolio_admin", + "perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, + "perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING, + "perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS, + "perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN, **user_info, }, ) diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 06957f18..910042b5 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -18,10 +18,10 @@ from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.invitation import Status as InvitationStatus _DEFAULT_PERMS_FORM_DATA = { - "perms_app_mgmt": "view_portfolio_application_management", - "perms_funding": "view_portfolio_funding", - "perms_reporting": "view_portfolio_reports", - "perms_portfolio_mgmt": "view_portfolio_admin", + "perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, + "perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING, + "perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS, + "perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN, } @@ -135,11 +135,14 @@ def test_update_member_portfolio_role(client, user_session): url_for( "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id ), - data={**_DEFAULT_PERMS_FORM_DATA, "perms_funding": "edit_portfolio_funding"}, + data={ + **_DEFAULT_PERMS_FORM_DATA, + "perms_funding": PermissionSets.EDIT_PORTFOLIO_FUNDING, + }, follow_redirects=True, ) assert response.status_code == 200 - edit_funding = PermissionSets.get("edit_portfolio_funding") + edit_funding = PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_FUNDING) assert edit_funding in member.permission_sets diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index b694e2a3..4af42ce1 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -234,8 +234,8 @@ def test_ko_can_view_task_order(client, user_session, portfolio, user): user=user, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=user) @@ -301,8 +301,8 @@ def test_ko_can_view_ko_review_page(client, user_session): user=ko, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) PortfolioRoleFactory.create( @@ -310,8 +310,8 @@ def test_ko_can_view_ko_review_page(client, user_session): user=cor, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) task_order = TaskOrderFactory.create( @@ -378,8 +378,8 @@ def test_cor_redirected_to_build_page(client, user_session, portfolio): user=cor, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) task_order = TaskOrderFactory.create( @@ -400,8 +400,8 @@ def test_submit_completed_ko_review_page_as_cor( user=user, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) @@ -448,8 +448,8 @@ def test_submit_completed_ko_review_page_as_ko( user=ko, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) @@ -492,8 +492,8 @@ def test_so_review_page(app, client, user_session, portfolio): user=so, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) @@ -533,8 +533,8 @@ def test_submit_so_review(app, client, user_session, portfolio): user=so, status=PortfolioStatus.ACTIVE, permission_sets=[ - PermissionSets.get("view_portfolio"), - PermissionSets.get("view_portfolio_funding"), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), ], ) task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) From ee37a5543a562a5f3b49defe0954d2cee7dd7794 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 08:50:38 -0400 Subject: [PATCH 12/20] organize portfolio permission sets by domain --- atst/domain/permission_sets.py | 101 +++++++++++++++++-------------- atst/domain/portfolio_roles.py | 4 +- tests/domain/test_task_orders.py | 6 +- tests/factories.py | 11 ++-- 4 files changed, 66 insertions(+), 56 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index 3de4492e..9f6eef96 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -81,13 +81,16 @@ ATAT_ROLES = [ }, ] -_VIEW_PORTFOLIO_PERMISSION_SETS = [ +_PORTFOLIO_BASIC_PERMISSION_SETS = [ { "name": PermissionSets.VIEW_PORTFOLIO, "description": "View basic portfolio info", "display_name": "View Portfolio", "permissions": [Permissions.VIEW_PORTFOLIO], - }, + } +] + +_PORTFOLIO_APP_MGMT_PERMISSION_SETS = [ { "name": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, "description": "View applications and related resources", @@ -98,36 +101,6 @@ _VIEW_PORTFOLIO_PERMISSION_SETS = [ Permissions.VIEW_ENVIRONMENT, ], }, - { - "name": PermissionSets.VIEW_PORTFOLIO_FUNDING, - "description": "View a portfolio's task orders", - "display_name": "Funding", - "permissions": [ - Permissions.VIEW_PORTFOLIO_FUNDING, - Permissions.VIEW_TASK_ORDER_DETAILS, - ], - }, - { - "name": PermissionSets.VIEW_PORTFOLIO_REPORTS, - "description": "View a portfolio's reports", - "display_name": "Reporting", - "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], - }, - { - "name": PermissionSets.VIEW_PORTFOLIO_ADMIN, - "description": "View a portfolio's admin options", - "display_name": "Portfolio Administration", - "permissions": [ - Permissions.VIEW_PORTFOLIO_ADMIN, - Permissions.VIEW_PORTFOLIO_NAME, - Permissions.VIEW_PORTFOLIO_USERS, - Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, - Permissions.VIEW_PORTFOLIO_POC, - ], - }, -] - -_EDIT_PORTFOLIO_PERMISSION_SETS = [ { "name": PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, "description": "Edit applications and related resources", @@ -141,6 +114,18 @@ _EDIT_PORTFOLIO_PERMISSION_SETS = [ Permissions.CREATE_ENVIRONMENT, ], }, +] + +_PORTFOLIO_FUNDING_PERMISSION_SETS = [ + { + "name": PermissionSets.VIEW_PORTFOLIO_FUNDING, + "description": "View a portfolio's task orders", + "display_name": "Funding", + "permissions": [ + Permissions.VIEW_PORTFOLIO_FUNDING, + Permissions.VIEW_TASK_ORDER_DETAILS, + ], + }, { "name": PermissionSets.EDIT_PORTFOLIO_FUNDING, "description": "Edit a portfolio's task orders and add new ones", @@ -150,12 +135,36 @@ _EDIT_PORTFOLIO_PERMISSION_SETS = [ Permissions.EDIT_TASK_ORDER_DETAILS, ], }, +] + +_PORTFOLIO_REPORTS_PERMISSION_SETS = [ + { + "name": PermissionSets.VIEW_PORTFOLIO_REPORTS, + "description": "View a portfolio's reports", + "display_name": "Reporting", + "permissions": [Permissions.VIEW_PORTFOLIO_REPORTS], + }, { "name": PermissionSets.EDIT_PORTFOLIO_REPORTS, "description": "Edit a portfolio's reports (no-op)", "display_name": "Reporting", "permissions": [], }, +] + +_PORTFOLIO_ADMIN_PERMISSION_SETS = [ + { + "name": PermissionSets.VIEW_PORTFOLIO_ADMIN, + "description": "View a portfolio's admin options", + "display_name": "Portfolio Administration", + "permissions": [ + Permissions.VIEW_PORTFOLIO_ADMIN, + Permissions.VIEW_PORTFOLIO_NAME, + Permissions.VIEW_PORTFOLIO_USERS, + Permissions.VIEW_PORTFOLIO_ACTIVITY_LOG, + Permissions.VIEW_PORTFOLIO_POC, + ], + }, { "name": PermissionSets.EDIT_PORTFOLIO_ADMIN, "description": "Edit a portfolio's admin options", @@ -168,18 +177,20 @@ _EDIT_PORTFOLIO_PERMISSION_SETS = [ }, ] +_PORTFOLIO_POC_PERMISSION_SETS = [ + { + "name": "portfolio_poc", + "description": "Permissions belonging to the Portfolio POC", + "display_name": "Portfolio Point of Contact", + "permissions": [Permissions.EDIT_PORTFOLIO_POC, Permissions.ARCHIVE_PORTFOLIO], + } +] + PORTFOLIO_PERMISSION_SETS = ( - _VIEW_PORTFOLIO_PERMISSION_SETS - + _EDIT_PORTFOLIO_PERMISSION_SETS - + [ - { - "name": "portfolio_poc", - "description": "Permissions belonging to the Portfolio POC", - "display_name": "Portfolio Point of Contact", - "permissions": [ - Permissions.EDIT_PORTFOLIO_POC, - Permissions.ARCHIVE_PORTFOLIO, - ], - } - ] + _PORTFOLIO_BASIC_PERMISSION_SETS + + _PORTFOLIO_APP_MGMT_PERMISSION_SETS + + _PORTFOLIO_FUNDING_PERMISSION_SETS + + _PORTFOLIO_REPORTS_PERMISSION_SETS + + _PORTFOLIO_ADMIN_PERMISSION_SETS + + _PORTFOLIO_POC_PERMISSION_SETS ) diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index 204ef80f..b5e8189f 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -96,7 +96,7 @@ class PortfolioRoles(object): return new_portfolio_role - _DEFAULT_PORTFOLIO_PERMS_SETS = { + DEFAULT_PORTFOLIO_PERMISSION_SETS = { PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, PermissionSets.VIEW_PORTFOLIO_FUNDING, PermissionSets.VIEW_PORTFOLIO_REPORTS, @@ -105,7 +105,7 @@ class PortfolioRoles(object): @classmethod def _permission_sets_for_names(cls, set_names): - perms_set_names = PortfolioRoles._DEFAULT_PORTFOLIO_PERMS_SETS.union( + perms_set_names = PortfolioRoles.DEFAULT_PORTFOLIO_PERMISSION_SETS.union( set(set_names) ) return [ diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index fe62f17a..e1d82f10 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -2,7 +2,8 @@ import pytest from atst.domain.task_orders import TaskOrders, TaskOrderError, DD254s from atst.domain.exceptions import UnauthorizedError -from atst.domain.permission_sets import PermissionSets, _VIEW_PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import PermissionSets +from atst.domain.portfolio_roles import PortfolioRoles from atst.models.attachment import Attachment from tests.factories import ( @@ -116,7 +117,8 @@ def test_task_order_access(): user=member, portfolio=task_order.portfolio, permission_sets=[ - PermissionSets.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS + PermissionSets.get(prms) + for prms in PortfolioRoles.DEFAULT_PORTFOLIO_PERMISSION_SETS ], ) TaskOrders.add_officer( diff --git a/tests/factories.py b/tests/factories.py index 23ed2a86..4a786739 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -14,17 +14,13 @@ from atst.models.task_order import TaskOrder from atst.models.user import User from atst.models.permission_set import PermissionSet from atst.models.portfolio import Portfolio -from atst.domain.permission_sets import ( - PermissionSets, - PORTFOLIO_PERMISSION_SETS, - _VIEW_PORTFOLIO_PERMISSION_SETS, - _EDIT_PORTFOLIO_PERMISSION_SETS, -) +from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS from atst.models.portfolio_role import PortfolioRole, Status as PortfolioRoleStatus from atst.models.environment_role import EnvironmentRole from atst.models.invitation import Invitation, Status as InvitationStatus from atst.models.dd_254 import DD254 from atst.domain.invitations import Invitations +from atst.domain.portfolio_roles import PortfolioRoles def random_choice(choices): @@ -70,7 +66,8 @@ def _random_date(year_min, year_max, operation): def base_portfolio_permission_sets(): return [ - PermissionSets.get(prms["name"]) for prms in _VIEW_PORTFOLIO_PERMISSION_SETS + PermissionSets.get(prms) + for prms in PortfolioRoles.DEFAULT_PORTFOLIO_PERMISSION_SETS ] From ad56ddc60e509b2bf0b4a317c7137c542caf5780 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 13:47:57 -0400 Subject: [PATCH 13/20] implement PermissionSets.get_many for getting multiple permission sets by name --- atst/domain/permission_sets.py | 8 +++++++ atst/domain/portfolio_roles.py | 15 +++++++++--- atst/domain/portfolios/portfolios.py | 6 ++--- tests/domain/test_permission_sets.py | 32 +++++++++++++++++++++++++ tests/domain/test_portfolio_roles.py | 3 ++- tests/domain/test_portfolios.py | 11 ++++++--- tests/domain/test_roles.py | 18 -------------- tests/factories.py | 2 +- tests/routes/portfolios/test_members.py | 2 +- 9 files changed, 66 insertions(+), 31 deletions(-) create mode 100644 tests/domain/test_permission_sets.py delete mode 100644 tests/domain/test_roles.py diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index 9f6eef96..b045523a 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -30,6 +30,14 @@ class PermissionSets(object): def get_all(cls): return db.session.query(PermissionSet).all() + @classmethod + def get_many(cls, perms_set_names): + return ( + db.session.query(PermissionSet) + .filter(PermissionSet.name.in_(perms_set_names)) + .all() + ) + ATAT_ROLES = [ { diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index b5e8189f..153d4707 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -101,16 +101,25 @@ class PortfolioRoles(object): PermissionSets.VIEW_PORTFOLIO_FUNDING, PermissionSets.VIEW_PORTFOLIO_REPORTS, PermissionSets.VIEW_PORTFOLIO_ADMIN, + PermissionSets.VIEW_PORTFOLIO, } + PORTFOLIO_PERMISSION_SETS = DEFAULT_PORTFOLIO_PERMISSION_SETS.union( + { + PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.EDIT_PORTFOLIO_FUNDING, + PermissionSets.EDIT_PORTFOLIO_REPORTS, + PermissionSets.EDIT_PORTFOLIO_ADMIN, + PermissionSets.PORTFOLIO_POC, + } + ) + @classmethod def _permission_sets_for_names(cls, set_names): perms_set_names = PortfolioRoles.DEFAULT_PORTFOLIO_PERMISSION_SETS.union( set(set_names) ) - return [ - PermissionSets.get(perms_set_name) for perms_set_name in perms_set_names - ] + return PermissionSets.get_many(perms_set_names) @classmethod def update(cls, portfolio_role, set_names): diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index f6170fa9..9ce6c2e9 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -1,4 +1,4 @@ -from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import PermissionSets from atst.domain.authz import Authorization from atst.models.permissions import Permissions from atst.domain.users import Users @@ -20,9 +20,7 @@ class Portfolios(object): portfolio = PortfoliosQuery.create( name=name, defense_component=defense_component ) - perms_sets = [ - PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS - ] + perms_sets = PermissionSets.get_many(PortfolioRoles.PORTFOLIO_PERMISSION_SETS) Portfolios._create_portfolio_role( user, portfolio, diff --git a/tests/domain/test_permission_sets.py b/tests/domain/test_permission_sets.py new file mode 100644 index 00000000..df19c24d --- /dev/null +++ b/tests/domain/test_permission_sets.py @@ -0,0 +1,32 @@ +import pytest +from atst.domain.permission_sets import PermissionSets +from atst.domain.exceptions import NotFoundError +from atst.utils import first_or_none + + +def test_get_all(): + roles = PermissionSets.get_all() + assert roles + + +def test_get_existing_permission_set(): + role = PermissionSets.get("portfolio_poc") + assert role.name == "portfolio_poc" + + +def test_get_nonexistent_permission_set(): + with pytest.raises(NotFoundError): + PermissionSets.get("nonexistent") + + +def test_get_many(): + perms_sets = PermissionSets.get_many( + [PermissionSets.VIEW_PORTFOLIO_FUNDING, PermissionSets.EDIT_PORTFOLIO_FUNDING] + ) + assert len(perms_sets) == 2 + assert first_or_none( + lambda p: p.name == PermissionSets.VIEW_PORTFOLIO_FUNDING, perms_sets + ) + assert first_or_none( + lambda p: p.name == PermissionSets.EDIT_PORTFOLIO_FUNDING, perms_sets + ) diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 8ffbed81..5bdc9bc0 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -18,13 +18,14 @@ def test_add_portfolio_role_with_permission_sets(): port_role = PortfolioRoles.add( new_user, portfolio.id, permission_sets=permission_sets ) - assert len(port_role.permission_sets) == 5 + assert len(port_role.permission_sets) == 6 expected_names = [ PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, PermissionSets.VIEW_PORTFOLIO_FUNDING, PermissionSets.VIEW_PORTFOLIO_REPORTS, PermissionSets.VIEW_PORTFOLIO_ADMIN, + PermissionSets.VIEW_PORTFOLIO, ] actual_names = [prms.name for prms in port_role.permission_sets] assert expected_names == expected_names diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 9140d82d..e2e743ae 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -9,7 +9,12 @@ from atst.domain.environments import Environments from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS from atst.models.portfolio_role import Status as PortfolioRoleStatus -from tests.factories import UserFactory, PortfolioRoleFactory, PortfolioFactory +from tests.factories import ( + UserFactory, + PortfolioRoleFactory, + PortfolioFactory, + get_all_portfolio_permission_sets, +) @pytest.fixture(scope="function") @@ -201,7 +206,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin( ) admin = UserFactory.create() - perm_sets = [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + perm_sets = get_all_portfolio_permission_sets() PortfolioRoleFactory.create( user=admin, portfolio=portfolio, permission_sets=perm_sets ) @@ -263,7 +268,7 @@ def test_get_for_update_information(portfolio, portfolio_owner): assert portfolio == owner_ws admin = UserFactory.create() - perm_sets = [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + perm_sets = get_all_portfolio_permission_sets() PortfolioRoleFactory.create( user=admin, portfolio=portfolio, permission_sets=perm_sets ) diff --git a/tests/domain/test_roles.py b/tests/domain/test_roles.py deleted file mode 100644 index 58601d9f..00000000 --- a/tests/domain/test_roles.py +++ /dev/null @@ -1,18 +0,0 @@ -import pytest -from atst.domain.permission_sets import PermissionSets -from atst.domain.exceptions import NotFoundError - - -def test_get_all_roles(): - roles = PermissionSets.get_all() - assert roles - - -def test_get_existing_role(): - role = PermissionSets.get("portfolio_poc") - assert role.name == "portfolio_poc" - - -def test_get_nonexistent_role(): - with pytest.raises(NotFoundError): - PermissionSets.get("nonexistent") diff --git a/tests/factories.py b/tests/factories.py index 4a786739..bdb27c69 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -72,7 +72,7 @@ def base_portfolio_permission_sets(): def get_all_portfolio_permission_sets(): - return [PermissionSets.get(prms["name"]) for prms in PORTFOLIO_PERMISSION_SETS] + return PermissionSets.get_many(PortfolioRoles.PORTFOLIO_PERMISSION_SETS) class Base(factory.alchemy.SQLAlchemyModelFactory): diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 910042b5..5d149f6f 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -109,7 +109,7 @@ def test_create_member(client, user_session): assert user.invitations assert len(queue.get_queue()) == queue_length + 1 portfolio_role = user.portfolio_roles[0] - assert len(portfolio_role.permission_sets) == 4 + assert len(portfolio_role.permission_sets) == 5 @pytest.mark.skip(reason="permission set display not implemented") From 79cb7ac2eb8123437241d1481f987755f4a8045a Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 14:24:41 -0400 Subject: [PATCH 14/20] use first_or_none utility for finding portfolio role --- atst/domain/authz.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/atst/domain/authz.py b/atst/domain/authz.py index 6fb27924..eae7cf3f 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -1,3 +1,4 @@ +from atst.utils import first_or_none from atst.models.permissions import Permissions from atst.domain.exceptions import UnauthorizedError @@ -5,8 +6,8 @@ from atst.domain.exceptions import UnauthorizedError class Authorization(object): @classmethod def has_portfolio_permission(cls, user, portfolio, permission): - port_role = next( - (pr for pr in user.portfolio_roles if pr.portfolio == portfolio), None + port_role = first_or_none( + lambda pr: pr.portfolio == portfolio, user.portfolio_roles ) if port_role: return permission in port_role.permissions From b0e48134b0cd34a12548b80652550a017f20657c Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 14:53:07 -0400 Subject: [PATCH 15/20] trim old permissions list --- atst/domain/permission_sets.py | 39 ++------------------------- atst/models/permissions.py | 47 --------------------------------- tests/domain/test_portfolios.py | 1 + 3 files changed, 3 insertions(+), 84 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index b045523a..e78a7cab 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -44,48 +44,13 @@ ATAT_ROLES = [ "name": "ccpo", "display_name": "CCPO", "description": "", - "permissions": [ - Permissions.VIEW_ORIGINAL_JEDI_REQEUST, - Permissions.REVIEW_AND_APPROVE_JEDI_PORTFOLIO_REQUEST, - Permissions.MODIFY_ATAT_ROLE_PERMISSIONS, - Permissions.CREATE_CSP_ROLE, - Permissions.DELETE_CSP_ROLE, - Permissions.DEACTIVE_CSP_ROLE, - Permissions.MODIFY_CSP_ROLE_PERMISSIONS, - Permissions.VIEW_USAGE_REPORT, - Permissions.VIEW_USAGE_DOLLARS, - Permissions.ADD_AND_ASSIGN_CSP_ROLES, - Permissions.REMOVE_CSP_ROLES, - Permissions.REQUEST_NEW_CSP_ROLE, - Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - Permissions.VIEW_ASSIGNED_ATAT_ROLE_CONFIGURATIONS, - Permissions.VIEW_ASSIGNED_CSP_ROLE_CONFIGURATIONS, - Permissions.DEACTIVATE_PORTFOLIO, - Permissions.VIEW_ATAT_PERMISSIONS, - Permissions.TRANSFER_OWNERSHIP_OF_PORTFOLIO, - Permissions.VIEW_PORTFOLIO, - Permissions.VIEW_PORTFOLIO_MEMBERS, - Permissions.ADD_APPLICATION_IN_PORTFOLIO, - Permissions.DELETE_APPLICATION_IN_PORTFOLIO, - Permissions.DEACTIVATE_APPLICATION_IN_PORTFOLIO, - Permissions.VIEW_APPLICATION_IN_PORTFOLIO, - Permissions.RENAME_APPLICATION_IN_PORTFOLIO, - Permissions.ADD_ENVIRONMENT_IN_APPLICATION, - Permissions.DELETE_ENVIRONMENT_IN_APPLICATION, - Permissions.DEACTIVATE_ENVIRONMENT_IN_APPLICATION, - Permissions.VIEW_ENVIRONMENT_IN_APPLICATION, - Permissions.RENAME_ENVIRONMENT_IN_APPLICATION, - Permissions.ADD_TAG_TO_PORTFOLIO, - Permissions.REMOVE_TAG_FROM_PORTFOLIO, - Permissions.VIEW_AUDIT_LOG, - Permissions.VIEW_PORTFOLIO_AUDIT_LOG, - ], + "permissions": [Permissions.VIEW_AUDIT_LOG], }, { "name": "default", "display_name": "Default", "description": "", - "permissions": [Permissions.REQUEST_JEDI_PORTFOLIO], + "permissions": [], }, ] diff --git a/atst/models/permissions.py b/atst/models/permissions.py index 95c504f1..6f1b52c7 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -1,53 +1,6 @@ class Permissions(object): VIEW_AUDIT_LOG = "view_audit_log" - VIEW_PORTFOLIO_AUDIT_LOG = "view_portfolio_audit_log" - REQUEST_JEDI_PORTFOLIO = "request_jedi_portfolio" - VIEW_ORIGINAL_JEDI_REQEUST = "view_original_jedi_request" - REVIEW_AND_APPROVE_JEDI_PORTFOLIO_REQUEST = ( - "review_and_approve_jedi_portfolio_request" - ) - MODIFY_ATAT_ROLE_PERMISSIONS = "modify_atat_role_permissions" - CREATE_CSP_ROLE = "create_csp_role" - DELETE_CSP_ROLE = "delete_csp_role" - DEACTIVE_CSP_ROLE = "deactivate_csp_role" - MODIFY_CSP_ROLE_PERMISSIONS = "modify_csp_role_permissions" - VIEW_USAGE_REPORT = "view_usage_report" - VIEW_USAGE_DOLLARS = "view_usage_dollars" - ADD_AND_ASSIGN_CSP_ROLES = "add_and_assign_csp_roles" - REMOVE_CSP_ROLES = "remove_csp_roles" - REQUEST_NEW_CSP_ROLE = "request_new_csp_role" - ASSIGN_AND_UNASSIGN_ATAT_ROLE = "assign_and_unassign_atat_role" - - VIEW_ASSIGNED_ATAT_ROLE_CONFIGURATIONS = "view_assigned_atat_role_configurations" - VIEW_ASSIGNED_CSP_ROLE_CONFIGURATIONS = "view_assigned_csp_role_configurations" - - EDIT_PORTFOLIO_INFORMATION = "edit_portfolio_information" - DEACTIVATE_PORTFOLIO = "deactivate_portfolio" - VIEW_ATAT_PERMISSIONS = "view_atat_permissions" - TRANSFER_OWNERSHIP_OF_PORTFOLIO = "transfer_ownership_of_portfolio" - VIEW_PORTFOLIO_MEMBERS = "view_portfolio_members" - - ADD_APPLICATION_IN_PORTFOLIO = "add_application_in_portfolio" - DELETE_APPLICATION_IN_PORTFOLIO = "delete_application_in_portfolio" - DEACTIVATE_APPLICATION_IN_PORTFOLIO = "deactivate_application_in_portfolio" - VIEW_APPLICATION_IN_PORTFOLIO = "view_application_in_portfolio" - RENAME_APPLICATION_IN_PORTFOLIO = "rename_application_in_portfolio" - - ADD_ENVIRONMENT_IN_APPLICATION = "add_environment_in_application" - DELETE_ENVIRONMENT_IN_APPLICATION = "delete_environment_in_application" - DEACTIVATE_ENVIRONMENT_IN_APPLICATION = "deactivate_environment_in_application" - VIEW_ENVIRONMENT_IN_APPLICATION = "view_environment_in_application" - RENAME_ENVIRONMENT_IN_APPLICATION = "rename_environment_in_application" - - ADD_TAG_TO_PORTFOLIO = "add_tag_to_portfolio" - REMOVE_TAG_FROM_PORTFOLIO = "remove_tag_from_portfolio" - - VIEW_TASK_ORDER = "view_task_order" - UPDATE_TASK_ORDER = "update_task_order" - ADD_TASK_ORDER_OFFICER = "add_task_order_officers" - - # new portfolio permissions # base portfolio perms VIEW_PORTFOLIO = "view_portfolio" diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index e2e743ae..b806e9a5 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -255,6 +255,7 @@ def test_for_user_does_not_return_inactive_portfolios(portfolio, portfolio_owner assert len(bobs_portfolios) == 0 +@pytest.mark.skip(reason="CCPO status not fully implemented") def test_for_user_returns_all_portfolios_for_ccpo(portfolio, portfolio_owner): sam = UserFactory.from_atat_role("ccpo") PortfolioFactory.create() From 762d2289a33238fb2fa9f70767eac3f5e6f17732 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 14:57:27 -0400 Subject: [PATCH 16/20] rename portfolio_role -> permission_sets join table, remove older roles property --- atst/models/portfolio_role.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index be9a6983..eb589157 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -29,7 +29,7 @@ class Status(Enum): PENDING = "pending" -portfolio_roles_roles = Table( +portfolio_roles_permission_sets = Table( "portfolio_roles_permission_sets", Base.metadata, Column("portfolio_role_id", UUID(as_uuid=True), ForeignKey("portfolio_roles.id")), @@ -52,7 +52,9 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING) - permission_sets = relationship("PermissionSet", secondary=portfolio_roles_roles) + permission_sets = relationship( + "PermissionSet", secondary=portfolio_roles_permission_sets + ) @property def permissions(self): @@ -117,10 +119,6 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def user_name(self): return self.user.full_name - @property - def role_displayname(self): - return self.role.display_name - @property def is_active(self): return self.status == Status.ACTIVE From f99156c0cba51dd7f230ee38078a5f363b3b86d7 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 15:09:09 -0400 Subject: [PATCH 17/20] fix permissions typo in template --- templates/portfolios/members/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/portfolios/members/index.html b/templates/portfolios/members/index.html index c55835fc..ea02c028 100644 --- a/templates/portfolios/members/index.html +++ b/templates/portfolios/members/index.html @@ -9,7 +9,7 @@ {% if not portfolio.members %} - {% set user_can_invite = user_can(Permissions.CREATE_PORTFOLIO_USERS) %} + {% set user_can_invite = user_can(permissions.CREATE_PORTFOLIO_USERS) %} {{ EmptyState( 'There are currently no members in this Portfolio.', From 0e48191d75ea70bf2939633674488f74091389f0 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 14 Mar 2019 15:18:13 -0400 Subject: [PATCH 18/20] fix tests fromm rebase --- tests/routes/portfolios/test_task_orders.py | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 4af42ce1..9f3c5fe8 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -572,10 +572,7 @@ def test_resend_invite_when_invalid_invite_officer( ) PortfolioRoleFactory.create( - role=Roles.get("owner"), - portfolio=portfolio, - user=user, - status=PortfolioStatus.ACTIVE, + portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE ) user_session(user) @@ -604,10 +601,7 @@ def test_resend_invite_when_officer_type_missing( ) PortfolioRoleFactory.create( - role=Roles.get("owner"), - portfolio=portfolio, - user=user, - status=PortfolioStatus.ACTIVE, + portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE ) user_session(user) @@ -634,10 +628,7 @@ def test_resend_invite_when_ko(app, client, user_session, portfolio, user): ) portfolio_role = PortfolioRoleFactory.create( - role=Roles.get("owner"), - portfolio=portfolio, - user=user, - status=PortfolioStatus.ACTIVE, + portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE ) original_invitation = Invitations.create( @@ -678,10 +669,7 @@ def test_resend_invite_when_not_pending(app, client, user_session, portfolio, us ) portfolio_role = PortfolioRoleFactory.create( - role=Roles.get("owner"), - portfolio=portfolio, - user=user, - status=PortfolioStatus.ACTIVE, + portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE ) original_invitation = InvitationFactory.create( From 0a2e5607b9ceaedec762202376973ef441587303 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 15 Mar 2019 07:14:09 -0400 Subject: [PATCH 19/20] fix migration downgrades --- alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py | 2 +- .../a19138e386c4_rename_roles_table_to_permission_sets.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py b/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py index 8a261f29..4c0ddcb6 100644 --- a/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py +++ b/alembic/versions/0e71ab219ada_remove_portfolio_roles_role_id.py @@ -25,6 +25,6 @@ def upgrade(): def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('portfolio_roles', sa.Column('role_id', postgresql.UUID(), autoincrement=False, nullable=False)) + op.add_column('portfolio_roles', sa.Column('role_id', postgresql.UUID(), autoincrement=False, nullable=True)) op.create_foreign_key('workspace_roles_role_id_fkey', 'portfolio_roles', 'roles', ['role_id'], ['id']) # ### end Alembic commands ### diff --git a/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py b/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py index 64f629fd..5da2cb2d 100644 --- a/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py +++ b/alembic/versions/a19138e386c4_rename_roles_table_to_permission_sets.py @@ -24,5 +24,5 @@ def upgrade(): def downgrade(): op.rename_table("permission_sets", "roles") - op.rename_table("portfolio_roles_permission_sets", "portfolio_roles_roles") op.alter_column("portfolio_roles_permission_sets", "permission_set_id", new_column_name="role_id") + op.rename_table("portfolio_roles_permission_sets", "portfolio_roles_roles") From 23584da57b086dac268527615d64c5552704e9ef Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 15 Mar 2019 14:41:24 -0400 Subject: [PATCH 20/20] changes to increase test branch coverage --- atst/models/permission_set.py | 13 ----------- tests/domain/test_portfolios.py | 20 ++++++++++++++++ tests/models/test_portfolio_role.py | 23 ++++++++++++++++--- .../routes/task_orders/test_new_task_order.py | 12 ++++++++++ 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/atst/models/permission_set.py b/atst/models/permission_set.py index cb5ddb92..1818fbb7 100644 --- a/atst/models/permission_set.py +++ b/atst/models/permission_set.py @@ -1,6 +1,5 @@ from sqlalchemy import String, Column from sqlalchemy.dialects.postgresql import ARRAY -from sqlalchemy.orm.attributes import flag_modified from atst.models import Base, types, mixins @@ -14,18 +13,6 @@ class PermissionSet(Base, mixins.TimestampsMixin): description = Column(String, nullable=False) permissions = Column(ARRAY(String), index=True, server_default="{}", nullable=False) - def add_permission(self, permission): - perms_set = set(self.permissions) - perms_set.add(permission) - self.permissions = list(perms_set) - flag_modified(self, "permissions") - - def remove_permission(self, permission): - perms_set = set(self.permissions) - perms_set.discard(permission) - self.permissions = list(perms_set) - flag_modified(self, "permissions") - def __repr__(self): return "".format( self.name, self.description, self.permissions, self.id diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index b806e9a5..dd752416 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -162,6 +162,26 @@ def test_random_user_cannot_view_portfolio_members(portfolio): portfolio = Portfolios.get_with_members(developer, portfolio.id) +def test_scoped_portfolio_for_admin_missing_view_apps_perms(portfolio_owner, portfolio): + Applications.create( + portfolio_owner, + portfolio, + "My Application 2", + "My application 2", + ["dev", "staging", "prod"], + ) + restricted_admin = UserFactory.create() + PortfolioRoleFactory.create( + portfolio=portfolio, + user=restricted_admin, + permission_sets=[PermissionSets.get(PermissionSets.VIEW_PORTFOLIO)], + ) + scoped_portfolio = Portfolios.get(restricted_admin, portfolio.id) + assert scoped_portfolio.id == portfolio.id + assert len(portfolio.applications) == 1 + assert len(scoped_portfolio.applications) == 0 + + @pytest.mark.skip(reason="should be reworked pending application member changes") def test_scoped_portfolio_only_returns_a_users_applications_and_environments( portfolio, portfolio_owner diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 7c67fe5d..198408c8 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -21,7 +21,7 @@ from tests.factories import ( from atst.domain.portfolio_roles import PortfolioRoles -def test_has_no_ws_role_history(session): +def test_has_no_portfolio_role_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -39,7 +39,7 @@ def test_has_no_ws_role_history(session): @pytest.mark.skip(reason="need to update audit log permission set handling") -def test_has_ws_role_history(session): +def test_has_portfolio_role_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -62,7 +62,7 @@ def test_has_ws_role_history(session): assert changed_events[0].changed_state["role"][1] == "admin" -def test_has_ws_status_history(session): +def test_has_portfolio_status_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -189,6 +189,11 @@ def test_status_when_member_is_active(): assert portfolio_role.display_status == "Active" +def test_status_when_member_is_disabled(): + portfolio_role = PortfolioRoleFactory.create(status=Status.DISABLED) + assert portfolio_role.display_status == "Disabled" + + def test_status_when_invitation_has_been_rejected_for_expirations(): portfolio = PortfolioFactory.create() user = UserFactory.create() @@ -213,6 +218,18 @@ def test_status_when_invitation_has_been_rejected_for_wrong_user(): assert portfolio_role.display_status == "Error on invite" +def test_status_when_invitation_has_been_revoked(): + portfolio = PortfolioFactory.create() + user = UserFactory.create() + portfolio_role = PortfolioRoleFactory.create( + portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING + ) + invitation = InvitationFactory.create( + portfolio_role=portfolio_role, status=InvitationStatus.REVOKED + ) + assert portfolio_role.display_status == "Invite revoked" + + def test_status_when_invitation_is_expired(): portfolio = PortfolioFactory.create() user = UserFactory.create() diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index fe4ec09e..f27e7ee1 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -340,3 +340,15 @@ def test_review_task_order_form(client, user_session, task_order): ) assert response.status_code == 200 + + +def test_update_task_order_clears_unnecessary_other_responses(): + user = UserFactory.create() + to_data = TaskOrderFactory.dictionary() + to_data["complexity"] = ["storage"] + to_data["complexity_other"] = "something else" + to_data["dev_team"] = ["civilians"] + to_data["dev_team_other"] = "something else" + workflow = UpdateTaskOrderWorkflow(user, to_data) + assert workflow.task_order_form_data["complexity_other"] is None + assert workflow.task_order_form_data["dev_team_other"] is None