From ed25078c391990753094cd7ce4383f3298c8c8c9 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 18 Apr 2019 14:13:31 -0400 Subject: [PATCH 1/4] move portfolio context processor to generic location --- atst/routes/portfolios/__init__.py | 38 +---------- atst/utils/context_processors.py | 74 +++++++++++++++++++++ templates/portfolios/applications/team.html | 2 +- tests/test_access.py | 5 ++ 4 files changed, 82 insertions(+), 37 deletions(-) create mode 100644 atst/utils/context_processors.py diff --git a/atst/routes/portfolios/__init__.py b/atst/routes/portfolios/__init__.py index 4bac8b58..e3566528 100644 --- a/atst/routes/portfolios/__init__.py +++ b/atst/routes/portfolios/__init__.py @@ -12,41 +12,7 @@ from atst.domain.exceptions import UnauthorizedError from atst.domain.portfolios import Portfolios from atst.domain.authz import Authorization from atst.models.permissions import Permissions +from atst.utils.context_processors import portfolio as portfolio_context_processor -@portfolios_bp.context_processor -def portfolio(): - portfolio = None - if "portfolio_id" in http_request.view_args: - portfolio = Portfolios.get( - g.current_user, http_request.view_args["portfolio_id"] - ) - - def user_can(permission): - if portfolio: - return Authorization.has_portfolio_permission( - g.current_user, portfolio, permission - ) - return False - - if not portfolio is None: - active_task_orders = [ - task_order for task_order in portfolio.task_orders if task_order.is_active - ] - funding_end_date = ( - sorted(active_task_orders, key=attrgetter("end_date"))[-1].end_date - if active_task_orders - else None - ) - funded = len(active_task_orders) > 1 - else: - funding_end_date = None - funded = None - - return { - "portfolio": portfolio, - "permissions": Permissions, - "user_can": user_can, - "funding_end_date": funding_end_date, - "funded": funded, - } +portfolios_bp.context_processor(portfolio_context_processor) diff --git a/atst/utils/context_processors.py b/atst/utils/context_processors.py new file mode 100644 index 00000000..4b82320b --- /dev/null +++ b/atst/utils/context_processors.py @@ -0,0 +1,74 @@ +from operator import attrgetter + +from flask import request as http_request, g +from sqlalchemy.orm.exc import NoResultFound + +from atst.database import db +from atst.domain.authz import Authorization +from atst.models import Application, Portfolio, TaskOrder +from atst.models.permissions import Permissions +from atst.domain.portfolios.scopes import ScopedPortfolio + + +def get_portfolio_from_context(view_args): + query = None + + if "portfolio_id" in view_args: + query = db.session.query(Portfolio).filter( + Portfolio.id == view_args["portfolio_id"] + ) + + elif "application_id" in view_args: + query = ( + db.session.query(Portfolio) + .join(Application, Application.portfolio_id == Portfolio.id) + .filter(Application.id == view_args["application_id"]) + ) + + elif "task_order_id" in view_args: + query = ( + db.session.query(Portfolio) + .join(TaskOrder, TaskOrder.portfolio_id == Portfolio.id) + .filter(TaskOrder.id == view_args["task_order_id"]) + ) + + if query: + try: + portfolio = query.one() + + return ScopedPortfolio(g.current_user, portfolio) + except NoResultFound: + raise NotFoundError("portfolio") + + +def portfolio(): + portfolio = get_portfolio_from_context(http_request.view_args) + + def user_can(permission): + if portfolio: + return Authorization.has_portfolio_permission( + g.current_user, portfolio, permission + ) + return False + + if not portfolio is None: + active_task_orders = [ + task_order for task_order in portfolio.task_orders if task_order.is_active + ] + funding_end_date = ( + sorted(active_task_orders, key=attrgetter("end_date"))[-1].end_date + if active_task_orders + else None + ) + funded = len(active_task_orders) > 1 + else: + funding_end_date = None + funded = None + + return { + "portfolio": portfolio, + "permissions": Permissions, + "user_can": user_can, + "funding_end_date": funding_end_date, + "funded": funded, + } diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index c0550f29..39a5014a 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -14,7 +14,7 @@ ("portfolios.applications.team_settings.blank_slate.title" | translate), action_label=("portfolios.applications.team_settings.blank_slate.action_label" | translate), action_href='#' if user_can_invite else None, - sub_message=None if user_can_invite else ("portfolios.team_settings.blank_slate.sub_message" | translate), + sub_message=None if user_can_invite else ("portfolios.applications.team_settings.blank_slate.sub_message" | translate), icon='avatar' ) }} diff --git a/tests/test_access.py b/tests/test_access.py index 23346910..41a3cd22 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -71,6 +71,11 @@ def test_all_protected_routes_have_access_control( monkeypatch.setattr("atst.domain.portfolios.Portfolios.for_user", lambda *a: []) monkeypatch.setattr("atst.domain.portfolios.Portfolios.get", lambda *a: None) monkeypatch.setattr("atst.domain.task_orders.TaskOrders.get", lambda *a: Mock()) + monkeypatch.setattr("atst.domain.applications.Applications.get", lambda *a: Mock()) + monkeypatch.setattr("atst.domain.invitations.Invitations._get", lambda *a: Mock()) + monkeypatch.setattr( + "atst.utils.context_processors.get_portfolio_from_context", lambda *a: None + ) # patch the internal function the access decorator uses so that # we can check that it was called From 849c5d4b588ec85039012905ad1a187d052aa52d Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 18 Apr 2019 14:54:45 -0400 Subject: [PATCH 2/4] Rearrange and rename application routes. - move application routes to their own Flask blueprint - squash application routes to be resource-specific - reorganize application routes --- atst/app.py | 2 + atst/domain/environment_roles.py | 21 +- atst/routes/__init__.py | 6 +- atst/routes/applications/__init__.py | 32 ++ atst/routes/applications/index.py | 11 + atst/routes/applications/new.py | 36 ++ atst/routes/applications/settings.py | 76 ++++ atst/routes/applications/team.py | 49 +++ atst/routes/portfolios/__init__.py | 1 - atst/routes/portfolios/applications.py | 185 ---------- atst/routes/portfolios/index.py | 4 +- templates/portfolios/applications/edit.html | 4 +- templates/portfolios/applications/index.html | 10 +- templates/portfolios/applications/new.html | 2 +- templates/portfolios/breadcrumbs.html | 2 +- templates/portfolios/reports/index.html | 2 +- tests/domain/test_environment_roles.py | 27 -- tests/routes/applications/test_index.py | 70 ++++ tests/routes/applications/test_init.py | 47 +++ tests/routes/applications/test_new.py | 21 ++ tests/routes/applications/test_settings.py | 185 ++++++++++ tests/routes/applications/test_team.py | 14 + tests/routes/portfolios/test_applications.py | 358 ------------------- tests/test_access.py | 64 ++-- 24 files changed, 583 insertions(+), 646 deletions(-) create mode 100644 atst/routes/applications/__init__.py create mode 100644 atst/routes/applications/index.py create mode 100644 atst/routes/applications/new.py create mode 100644 atst/routes/applications/settings.py create mode 100644 atst/routes/applications/team.py delete mode 100644 atst/routes/portfolios/applications.py delete mode 100644 tests/domain/test_environment_roles.py create mode 100644 tests/routes/applications/test_index.py create mode 100644 tests/routes/applications/test_init.py create mode 100644 tests/routes/applications/test_new.py create mode 100644 tests/routes/applications/test_settings.py create mode 100644 tests/routes/applications/test_team.py delete mode 100644 tests/routes/portfolios/test_applications.py diff --git a/atst/app.py b/atst/app.py index 13c801f8..06836f10 100644 --- a/atst/app.py +++ b/atst/app.py @@ -14,6 +14,7 @@ from atst.filters import register_filters from atst.routes import bp from atst.routes.portfolios import portfolios_bp as portfolio_routes from atst.routes.task_orders import task_orders_bp +from atst.routes.applications import applications_bp from atst.routes.dev import bp as dev_routes from atst.routes.users import bp as user_routes from atst.routes.errors import make_error_pages @@ -71,6 +72,7 @@ def make_app(config): app.register_blueprint(bp) app.register_blueprint(portfolio_routes) app.register_blueprint(task_orders_bp) + app.register_blueprint(applications_bp) app.register_blueprint(user_routes) if ENV != "prod": diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index ab5a082c..99728467 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -1,9 +1,7 @@ from flask import current_app as app -from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.domain.exceptions import NotFoundError -from atst.models import EnvironmentRole, Environment, Application +from atst.models import EnvironmentRole class EnvironmentRoles(object): @@ -15,23 +13,6 @@ class EnvironmentRoles(object): app.csp.cloud.create_role(env_role) return env_role - @classmethod - def get_for_portfolio(cls, user_id, environment_id, portfolio_id): - try: - return ( - db.session.query(EnvironmentRole) - .join(Environment, EnvironmentRole.environment_id == Environment.id) - .join(Application, Environment.application_id == Application.id) - .filter( - EnvironmentRole.user_id == user_id, - EnvironmentRole.environment_id == environment_id, - Application.portfolio_id == portfolio_id, - ) - .one() - ) - except NoResultFound: - raise NotFoundError("environment_role") - @classmethod def get(cls, user_id, environment_id): existing_env_role = ( diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index b4543d07..c327f1b2 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -76,14 +76,16 @@ def home(): ) else: return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id) + url_for( + "applications.portfolio_applications", portfolio_id=portfolio_id + ) ) else: portfolios = Portfolios.for_user(g.current_user) first_portfolio = sorted(portfolios, key=lambda portfolio: portfolio.name)[0] return redirect( url_for( - "portfolios.portfolio_applications", portfolio_id=first_portfolio.id + "applications.portfolio_applications", portfolio_id=first_portfolio.id ) ) diff --git a/atst/routes/applications/__init__.py b/atst/routes/applications/__init__.py new file mode 100644 index 00000000..60f7f495 --- /dev/null +++ b/atst/routes/applications/__init__.py @@ -0,0 +1,32 @@ +from flask import Blueprint, current_app as app, g, redirect, url_for + +applications_bp = Blueprint("applications", __name__) + +from . import index +from . import new +from . import settings +from . import team +from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.exceptions import UnauthorizedError +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.models.permissions import Permissions +from atst.utils.context_processors import portfolio as portfolio_context_processor + +applications_bp.context_processor(portfolio_context_processor) + + +def wrap_environment_role_lookup(user, environment_id=None, **kwargs): + env_role = EnvironmentRoles.get(user.id, environment_id) + if not env_role: + raise UnauthorizedError(user, "access environment {}".format(environment_id)) + + return True + + +@applications_bp.route("/environments//access") +@user_can(None, override=wrap_environment_role_lookup, message="access environment") +def access_environment(environment_id): + env_role = EnvironmentRoles.get(g.current_user.id, environment_id) + token = app.csp.cloud.get_access_token(env_role) + + return redirect(url_for("atst.csp_environment_access", token=token)) diff --git a/atst/routes/applications/index.py b/atst/routes/applications/index.py new file mode 100644 index 00000000..d61417f4 --- /dev/null +++ b/atst/routes/applications/index.py @@ -0,0 +1,11 @@ +from flask import render_template + +from . import applications_bp +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.models.permissions import Permissions + + +@applications_bp.route("/portfolios//applications") +@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications") +def portfolio_applications(portfolio_id): + return render_template("portfolios/applications/index.html") diff --git a/atst/routes/applications/new.py b/atst/routes/applications/new.py new file mode 100644 index 00000000..070aa940 --- /dev/null +++ b/atst/routes/applications/new.py @@ -0,0 +1,36 @@ +from flask import redirect, render_template, request as http_request, url_for + +from . import applications_bp +from atst.domain.applications import Applications +from atst.domain.portfolios import Portfolios +from atst.forms.application import NewApplicationForm +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.models.permissions import Permissions + + +@applications_bp.route("/portfolios//applications/new") +@user_can(Permissions.CREATE_APPLICATION, message="view create new application form") +def new(portfolio_id): + form = NewApplicationForm() + return render_template("portfolios/applications/new.html", form=form) + + +@applications_bp.route("/portfolios//applications", methods=["POST"]) +@user_can(Permissions.CREATE_APPLICATION, message="create new application") +def create(portfolio_id): + portfolio = Portfolios.get_for_update(portfolio_id) + form = NewApplicationForm(http_request.form) + + if form.validate(): + application_data = form.data + Applications.create( + portfolio, + application_data["name"], + application_data["description"], + application_data["environment_names"], + ) + return redirect( + url_for("applications.portfolio_applications", portfolio_id=portfolio_id) + ) + else: + return render_template("portfolios/applications/new.html", form=form) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py new file mode 100644 index 00000000..8ef290d6 --- /dev/null +++ b/atst/routes/applications/settings.py @@ -0,0 +1,76 @@ +from flask import redirect, render_template, request as http_request, url_for + +from . import applications_bp +from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.applications import Applications +from atst.forms.application import ApplicationForm +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.models.permissions import Permissions +from atst.utils.flash import formatted_flash as flash + + +def get_environments_obj_for_app(application): + environments_obj = {} + + for env in application.environments: + environments_obj[env.name] = [] + for user in env.users: + env_role = EnvironmentRoles.get(user.id, env.id) + environments_obj[env.name].append( + {"name": user.full_name, "role": env_role.displayname} + ) + + return environments_obj + + +@applications_bp.route("/applications//settings") +@user_can(Permissions.VIEW_APPLICATION, message="view application edit form") +def settings(application_id): + application = Applications.get(application_id) + form = ApplicationForm(name=application.name, description=application.description) + + return render_template( + "portfolios/applications/edit.html", + application=application, + form=form, + environments_obj=get_environments_obj_for_app(application=application), + ) + + +@applications_bp.route("/applications//edit", methods=["POST"]) +@user_can(Permissions.EDIT_APPLICATION, message="update application") +def update(application_id): + application = Applications.get(application_id) + form = ApplicationForm(http_request.form) + if form.validate(): + application_data = form.data + Applications.update(application, application_data) + + return redirect( + url_for( + "applications.portfolio_applications", + portfolio_id=application.portfolio_id, + ) + ) + else: + return render_template( + "portfolios/applications/edit.html", + application=application, + form=form, + environments_obj=get_environments_obj_for_app(application=application), + ) + + +@applications_bp.route("/applications//delete", methods=["POST"]) +@user_can(Permissions.DELETE_APPLICATION, message="delete application") +def delete(application_id): + application = Applications.get(application_id) + Applications.delete(application) + + flash("application_deleted", application_name=application.name) + + return redirect( + url_for( + "applications.portfolio_applications", portfolio_id=application.portfolio_id + ) + ) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py new file mode 100644 index 00000000..c9e57296 --- /dev/null +++ b/atst/routes/applications/team.py @@ -0,0 +1,49 @@ +from flask import render_template + + +from . import applications_bp +from atst.domain.environments import Environments +from atst.domain.applications import Applications +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.models.permissions import Permissions +from atst.domain.permission_sets import PermissionSets +from atst.utils.localization import translate + + +def permission_str(member, edit_perm_set): + if member.has_permission_set(edit_perm_set): + return translate("portfolios.members.permissions.edit_access") + else: + return translate("portfolios.members.permissions.view_only") + + +@applications_bp.route("/applications//team") +@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications") +def team(application_id): + application = Applications.get(resource_id=application_id) + + environment_users = {} + for member in application.members: + user_id = member.user.id + environment_users[user_id] = { + "permissions": { + "delete_access": permission_str( + member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS + ), + "environment_management": permission_str( + member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS + ), + "team_management": permission_str( + member, PermissionSets.EDIT_APPLICATION_TEAM + ), + }, + "environments": Environments.for_user( + user=member.user, application=application + ), + } + + return render_template( + "portfolios/applications/team.html", + application=application, + environment_users=environment_users, + ) diff --git a/atst/routes/portfolios/__init__.py b/atst/routes/portfolios/__init__.py index e3566528..e851bdab 100644 --- a/atst/routes/portfolios/__init__.py +++ b/atst/routes/portfolios/__init__.py @@ -4,7 +4,6 @@ from operator import attrgetter portfolios_bp = Blueprint("portfolios", __name__) from . import index -from . import applications from . import members from . import invitations from . import task_orders diff --git a/atst/routes/portfolios/applications.py b/atst/routes/portfolios/applications.py deleted file mode 100644 index 952ec643..00000000 --- a/atst/routes/portfolios/applications.py +++ /dev/null @@ -1,185 +0,0 @@ -from flask import ( - current_app as app, - g, - redirect, - render_template, - request as http_request, - url_for, -) - -from . import portfolios_bp -from atst.domain.environment_roles import EnvironmentRoles -from atst.domain.environments import Environments -from atst.domain.exceptions import UnauthorizedError -from atst.domain.applications import Applications -from atst.domain.portfolios import Portfolios -from atst.forms.application import NewApplicationForm, ApplicationForm -from atst.domain.authz.decorator import user_can_access_decorator as user_can -from atst.models.permissions import Permissions -from atst.utils.flash import formatted_flash as flash -from atst.domain.permission_sets import PermissionSets -from atst.utils.localization import translate - - -@portfolios_bp.route("/portfolios//applications") -@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications") -def portfolio_applications(portfolio_id): - return render_template("portfolios/applications/index.html") - - -@portfolios_bp.route("/portfolios//applications/new") -@user_can(Permissions.CREATE_APPLICATION, message="view create new application form") -def new_application(portfolio_id): - form = NewApplicationForm() - return render_template("portfolios/applications/new.html", form=form) - - -@portfolios_bp.route("/portfolios//applications/new", methods=["POST"]) -@user_can(Permissions.CREATE_APPLICATION, message="create new application") -def create_application(portfolio_id): - portfolio = Portfolios.get_for_update(portfolio_id) - form = NewApplicationForm(http_request.form) - - if form.validate(): - application_data = form.data - Applications.create( - portfolio, - application_data["name"], - application_data["description"], - application_data["environment_names"], - ) - return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id) - ) - else: - return render_template("portfolios/applications/new.html", form=form) - - -def get_environments_obj_for_app(application): - environments_obj = {} - - for env in application.environments: - environments_obj[env.name] = [] - for user in env.users: - env_role = EnvironmentRoles.get(user.id, env.id) - environments_obj[env.name].append( - {"name": user.full_name, "role": env_role.displayname} - ) - - return environments_obj - - -@portfolios_bp.route("/portfolios//applications//edit") -@user_can(Permissions.VIEW_APPLICATION, message="view application edit form") -def edit_application(portfolio_id, application_id): - application = Applications.get(application_id, portfolio_id=portfolio_id) - form = ApplicationForm(name=application.name, description=application.description) - - return render_template( - "portfolios/applications/edit.html", - application=application, - form=form, - environments_obj=get_environments_obj_for_app(application=application), - ) - - -@portfolios_bp.route( - "/portfolios//applications//edit", methods=["POST"] -) -@user_can(Permissions.EDIT_APPLICATION, message="update application") -def update_application(portfolio_id, application_id): - application = Applications.get(application_id, portfolio_id=portfolio_id) - form = ApplicationForm(http_request.form) - if form.validate(): - application_data = form.data - Applications.update(application, application_data) - - return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id) - ) - else: - return render_template( - "portfolios/applications/edit.html", - application=application, - form=form, - environments_obj=get_environments_obj_for_app(application=application), - ) - - -def wrap_environment_role_lookup( - user, portfolio_id=None, environment_id=None, **kwargs -): - env_role = EnvironmentRoles.get_for_portfolio( - user.id, environment_id, portfolio_id=portfolio_id - ) - if not env_role: - raise UnauthorizedError(user, "access environment {}".format(environment_id)) - - return True - - -@portfolios_bp.route("/portfolios//environments//access") -@user_can(None, override=wrap_environment_role_lookup, message="access environment") -def access_environment(portfolio_id, environment_id): - env_role = EnvironmentRoles.get_for_portfolio( - g.current_user.id, environment_id, portfolio_id=portfolio_id - ) - token = app.csp.cloud.get_access_token(env_role) - - return redirect(url_for("atst.csp_environment_access", token=token)) - - -@portfolios_bp.route( - "/portfolios//applications//delete", methods=["POST"] -) -@user_can(Permissions.DELETE_APPLICATION, message="delete application") -def delete_application(portfolio_id, application_id): - application = Applications.get(application_id, portfolio_id=portfolio_id) - Applications.delete(application) - - flash("application_deleted", application_name=application.name) - - return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id) - ) - - -def permission_str(member, edit_perm_set): - if member.has_permission_set(edit_perm_set): - return translate("portfolios.members.permissions.edit_access") - else: - return translate("portfolios.members.permissions.view_only") - - -@portfolios_bp.route("/portfolios//applications//team") -@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications") -def application_team(portfolio_id, application_id): - application = Applications.get( - resource_id=application_id, portfolio_id=portfolio_id - ) - - environment_users = {} - for member in application.members: - user_id = member.user.id - environment_users[user_id] = { - "permissions": { - "delete_access": permission_str( - member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS - ), - "environment_management": permission_str( - member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS - ), - "team_management": permission_str( - member, PermissionSets.EDIT_APPLICATION_TEAM - ), - }, - "environments": Environments.for_user( - user=member.user, application=application - ), - } - - return render_template( - "portfolios/applications/team.html", - application=application, - environment_users=environment_users, - ) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index dcdee315..d2f5853d 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -171,7 +171,7 @@ def edit_portfolio(portfolio_id): if form.validate(): Portfolios.update(portfolio, form.data) return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio.id) + url_for("applications.portfolio_applications", portfolio_id=portfolio.id) ) else: # rerender portfolio admin page @@ -182,7 +182,7 @@ def edit_portfolio(portfolio_id): @user_can(Permissions.VIEW_PORTFOLIO, message="view portfolio") def show_portfolio(portfolio_id): return redirect( - url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id) + url_for("applications.portfolio_applications", portfolio_id=portfolio_id) ) diff --git a/templates/portfolios/applications/edit.html b/templates/portfolios/applications/edit.html index d1e3a8fb..24c8b5fb 100644 --- a/templates/portfolios/applications/edit.html +++ b/templates/portfolios/applications/edit.html @@ -11,7 +11,7 @@
{{ 'portfolios.applications.settings_heading' | translate }}
-
+
@@ -112,7 +112,7 @@
- + {{ form.csrf_token }}