From d152034e1b9150ca80c53ece22ee1300543752cd Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 25 Mar 2019 15:54:10 -0400 Subject: [PATCH 1/6] Add in check to make sure that user has portfolio and app perms --- atst/domain/authz/decorator.py | 5 +++ tests/routes/portfolios/test_applications.py | 41 ++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index 17755749..00519308 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -5,6 +5,7 @@ from flask import g, current_app as app, request from . import user_can_access from atst.domain.portfolios import Portfolios from atst.domain.task_orders import TaskOrders +from atst.domain.applications import Applications from atst.domain.exceptions import UnauthorizedError @@ -16,6 +17,10 @@ def check_access(permission, message, exception, *args, **kwargs): g.current_user, kwargs["portfolio_id"] ) + if "application_id" in kwargs: + application = Applications.get(kwargs["application_id"]) + access_args["portfolio"] = application.portfolio + if "task_order_id" in kwargs: task_order = TaskOrders.get(kwargs["task_order_id"]) access_args["portfolio"] = task_order.portfolio diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index b6245aaa..e8db5d2f 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -157,6 +157,47 @@ def test_user_without_permission_cannot_update_application(client, user_session) assert application.description == "Cool stuff happening here!" +def test_user_can_only_access_apps_in_their_portfolio(client, user_session): + portfolio = PortfolioFactory.create() + other_portfolio = PortfolioFactory.create( + applications=[ + { + "name": "Awesome Application", + "description": "More cool stuff happening here!", + "environments": [{"name": "dev"}], + } + ] + ) + other_application = other_portfolio.applications[0] + user_session(portfolio.owner) + + # user can't view application edit form + response = client.get( + "/portfolios/{}/applications/{}/edit".format(portfolio.id, other_application.id) + ) + assert response.status_code == 404 + + # user can't post update application form + response = client.post( + url_for( + "portfolios.update_application", + portfolio_id=portfolio.id, + application_id=other_application.id, + ), + data={"name": "New Name", "description": "A new description."}, + follow_redirects=True, + ) + assert response.status_code == 404 + + # user can't view application members + response = client.get( + "/portfolios/{}/applications/{}/members".format( + portfolio.id, other_application.id + ) + ) + assert response.status_code == 404 + + def create_environment(user): portfolio = PortfolioFactory.create() portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=user) From 5d2b8556ed4610c5626c1bac8449328c603c2362 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 25 Mar 2019 15:54:40 -0400 Subject: [PATCH 2/6] Check that user has portfolio and invite perms to revoke or resend invites --- atst/domain/authz/decorator.py | 5 ++ tests/routes/portfolios/test_invitations.py | 52 +++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index 00519308..0c1f0952 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -6,6 +6,7 @@ from . import user_can_access from atst.domain.portfolios import Portfolios from atst.domain.task_orders import TaskOrders from atst.domain.applications import Applications +from atst.domain.invitations import Invitations from atst.domain.exceptions import UnauthorizedError @@ -25,6 +26,10 @@ def check_access(permission, message, exception, *args, **kwargs): task_order = TaskOrders.get(kwargs["task_order_id"]) access_args["portfolio"] = task_order.portfolio + if "token" in kwargs: + invite = Invitations._get(kwargs["token"]) + access_args["portfolio"] = invite.portfolio_role.portfolio + if exception is not None and exception(g.current_user, **access_args, **kwargs): return True diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index a4352808..cee8363f 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -169,6 +169,58 @@ def test_revoke_invitation(client, user_session): assert invite.is_revoked +def test_user_can_only_revoke_invites_in_their_portfolio(client, user_session): + portfolio = PortfolioFactory.create() + other_portfolio = PortfolioFactory.create() + user = UserFactory.create() + ws_role = PortfolioRoleFactory.create( + user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING + ) + invite = InvitationFactory.create( + user_id=user.id, + portfolio_role=ws_role, + status=InvitationStatus.REJECTED_EXPIRED, + expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), + ) + user_session(portfolio.owner) + response = client.post( + url_for( + "portfolios.revoke_invitation", + portfolio_id=portfolio.id, + token=invite.token, + ) + ) + + assert response.status_code == 404 + assert not invite.is_revoked + + +def test_user_can_only_resend_invites_in_their_portfolio(client, user_session, queue): + portfolio = PortfolioFactory.create() + other_portfolio = PortfolioFactory.create() + user = UserFactory.create() + ws_role = PortfolioRoleFactory.create( + user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING + ) + invite = InvitationFactory.create( + user_id=user.id, + portfolio_role=ws_role, + status=InvitationStatus.REJECTED_EXPIRED, + expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), + ) + user_session(portfolio.owner) + response = client.post( + url_for( + "portfolios.resend_invitation", + portfolio_id=portfolio.id, + token=invite.token, + ) + ) + + assert response.status_code == 404 + assert len(queue.get_queue()) == 0 + + def test_resend_invitation_sends_email(client, user_session, queue): user = UserFactory.create() portfolio = PortfolioFactory.create() From b5571000fe6142bd9d7055e5e9c6d78bf809e6f4 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 26 Mar 2019 10:33:20 -0400 Subject: [PATCH 3/6] Update tests - remove references to Workspace, use url_for, and check to make sure time_updated does not change on the application --- tests/routes/portfolios/test_applications.py | 9 +++++++-- tests/routes/portfolios/test_invitations.py | 8 ++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index e8db5d2f..cc6a0f73 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -173,11 +173,16 @@ def test_user_can_only_access_apps_in_their_portfolio(client, user_session): # user can't view application edit form response = client.get( - "/portfolios/{}/applications/{}/edit".format(portfolio.id, other_application.id) + url_for( + "portfolios.edit_application", + portfolio_id=portfolio.id, + application_id=other_application.id, + ) ) assert response.status_code == 404 # user can't post update application form + time_updated = other_application.time_updated response = client.post( url_for( "portfolios.update_application", @@ -185,9 +190,9 @@ def test_user_can_only_access_apps_in_their_portfolio(client, user_session): application_id=other_application.id, ), data={"name": "New Name", "description": "A new description."}, - follow_redirects=True, ) assert response.status_code == 404 + assert time_updated == other_application.time_updated # user can't view application members response = client.get( diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index cee8363f..4baa895c 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -173,12 +173,12 @@ def test_user_can_only_revoke_invites_in_their_portfolio(client, user_session): portfolio = PortfolioFactory.create() other_portfolio = PortfolioFactory.create() user = UserFactory.create() - ws_role = PortfolioRoleFactory.create( + portfolio_role = PortfolioRoleFactory.create( user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING ) invite = InvitationFactory.create( user_id=user.id, - portfolio_role=ws_role, + portfolio_role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -199,12 +199,12 @@ def test_user_can_only_resend_invites_in_their_portfolio(client, user_session, q portfolio = PortfolioFactory.create() other_portfolio = PortfolioFactory.create() user = UserFactory.create() - ws_role = PortfolioRoleFactory.create( + portfolio_role = PortfolioRoleFactory.create( user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING ) invite = InvitationFactory.create( user_id=user.id, - portfolio_role=ws_role, + portfolio_role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) From 0d30b81ec5d634e21560b766cd878c9458afbef8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 26 Mar 2019 10:36:23 -0400 Subject: [PATCH 4/6] Use if/else to avoid multiple queries --- atst/domain/authz/decorator.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index 0c1f0952..29d12feb 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -13,23 +13,23 @@ from atst.domain.exceptions import UnauthorizedError def check_access(permission, message, exception, *args, **kwargs): access_args = {"message": message} - if "portfolio_id" in kwargs: - access_args["portfolio"] = Portfolios.get( - g.current_user, kwargs["portfolio_id"] - ) - if "application_id" in kwargs: application = Applications.get(kwargs["application_id"]) access_args["portfolio"] = application.portfolio - if "task_order_id" in kwargs: + elif "task_order_id" in kwargs: task_order = TaskOrders.get(kwargs["task_order_id"]) access_args["portfolio"] = task_order.portfolio - if "token" in kwargs: + elif "token" in kwargs: invite = Invitations._get(kwargs["token"]) access_args["portfolio"] = invite.portfolio_role.portfolio + elif "portfolio_id" in kwargs: + access_args["portfolio"] = Portfolios.get( + g.current_user, kwargs["portfolio_id"] + ) + if exception is not None and exception(g.current_user, **access_args, **kwargs): return True From 81635ae979b69cd8d6b14a69497a23487423c312 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 26 Mar 2019 12:01:44 -0400 Subject: [PATCH 5/6] Add tests to check the TO nested routes --- tests/routes/portfolios/test_task_orders.py | 152 ++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 3948eaec..48aaefd0 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -128,6 +128,20 @@ class TestPortfolioFunding: assert context["funding_end_date"] is expiring_to.end_date assert context["funded"] == False + def test_user_can_only_access_to_in_their_portfolio( + self, app, user_session, portfolio + ): + other_task_order = TaskOrderFactory.create() + user_session(portfolio.owner) + response = app.test_client().get( + url_for( + "portfolios.view_task_order", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ) + ) + assert response.status_code == 404 + class TestTaskOrderInvitations: def setup(self): @@ -227,6 +241,54 @@ class TestTaskOrderInvitations: assert len(queue.get_queue()) == queue_length assert response.status_code == 400 + def test_user_can_only_invite_to_task_order_in_their_portfolio( + self, user_session, client, portfolio + ): + other_task_order = TaskOrderFactory.create() + user_session(portfolio.owner) + + # user can't see invites + response = client.get( + url_for( + "portfolios.task_order_invitations", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ) + ) + assert response.status_code == 404 + + # user can't send invites + time_updated = other_task_order.time_updated + response = client.post( + url_for( + "portfolios.edit_task_order_invitations", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ), + data={ + "contracting_officer-first_name": "Luke", + "contracting_officer-last_name": "Skywalker", + "contracting_officer-dod_id": "0123456789", + "contracting_officer-email": "luke@skywalker.mil", + "contracting_officer-phone_number": "0123456789", + "contracting_officer-invite": "y", + }, + ) + assert response.status_code == 404 + assert time_updated == other_task_order.time_updated + + # user can't resend invites + response = client.post( + url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + invite_type="ko_invite", + ) + ) + assert response.status_code == 404 + assert time_updated == other_task_order.time_updated + def test_ko_can_view_task_order(client, user_session, portfolio, user): PortfolioRoleFactory.create( @@ -464,6 +526,57 @@ def test_submit_completed_ko_review_page_as_ko( assert task_order.loas == loa_list +def test_ko_can_only_access_their_to(app, user_session, client, portfolio, pdf_upload): + ko = UserFactory.create() + + PortfolioRoleFactory.create( + portfolio=portfolio, + user=ko, + status=PortfolioStatus.ACTIVE, + permission_sets=[ + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), + ], + ) + + task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=ko) + dd_254 = DD254Factory.create() + TaskOrders.add_dd_254(task_order, dd_254.to_dictionary()) + other_task_order = TaskOrderFactory.create() + user_session(ko) + + # KO can't see TO + response = client.get( + url_for( + "portfolios.ko_review", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ) + ) + assert response.status_code == 404 + + # KO can't submit review for TO + form_data = { + "start_date": "02/10/2019", + "end_date": "03/10/2019", + "number": "1938745981", + "loas-0": "1231231231", + "custom_clauses": "hi im a custom clause", + "pdf": pdf_upload, + } + + response = client.post( + url_for( + "portfolios.submit_ko_review", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ), + data=form_data, + ) + assert response.status_code == 404 + assert not TaskOrders.is_signed_by_ko(other_task_order) + + def test_so_review_page(app, client, user_session, portfolio): so = UserFactory.create() PortfolioRoleFactory.create( @@ -541,6 +654,45 @@ def test_submit_so_review(app, client, user_session, portfolio): assert task_order.dd_254.certifying_official == dd_254_data["certifying_official"] +def test_so_can_only_access_their_to(app, client, user_session, portfolio): + so = UserFactory.create() + PortfolioRoleFactory.create( + portfolio=portfolio, + user=so, + status=PortfolioStatus.ACTIVE, + permission_sets=[ + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO), + PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_FUNDING), + ], + ) + task_order = TaskOrderFactory.create(portfolio=portfolio, security_officer=so) + dd_254_data = DD254Factory.dictionary() + other_task_order = TaskOrderFactory.create() + user_session(so) + + # SO can't view dd254 + response = client.get( + url_for( + "portfolios.so_review", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ) + ) + assert response.status_code == 404 + + # SO can't submit dd254 + response = client.post( + url_for( + "portfolios.submit_so_review", + portfolio_id=portfolio.id, + task_order_id=other_task_order.id, + ), + data=dd_254_data, + ) + assert response.status_code == 404 + assert not other_task_order.dd_254 + + def test_resend_invite_when_invalid_invite_officer( app, client, user_session, portfolio, user ): From cf1b30d6ca218ac7ab6e048ade5756c6342d3482 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 26 Mar 2019 13:28:44 -0400 Subject: [PATCH 6/6] Use url_for --- tests/routes/portfolios/test_applications.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index cc6a0f73..4f3bac58 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -196,8 +196,10 @@ def test_user_can_only_access_apps_in_their_portfolio(client, user_session): # user can't view application members response = client.get( - "/portfolios/{}/applications/{}/members".format( - portfolio.id, other_application.id + url_for( + "portfolios.application_members", + portfolio_id=portfolio.id, + application_id=other_application.id, ) ) assert response.status_code == 404