diff --git a/.secrets.baseline b/.secrets.baseline index 24361804..ffa18c6e 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-12-13T20:38:57Z", + "generated_at": "2019-12-18T15:29:41Z", "plugins_used": [ { "base64_limit": 4.5, @@ -170,7 +170,7 @@ "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", "is_secret": false, "is_verified": false, - "line_number": 659, + "line_number": 665, "type": "Hex High Entropy String" } ] diff --git a/atst/routes/applications/new.py b/atst/routes/applications/new.py index e9238775..9d673b04 100644 --- a/atst/routes/applications/new.py +++ b/atst/routes/applications/new.py @@ -1,9 +1,7 @@ -from flask import redirect, render_template, request as http_request, url_for, g +from flask import redirect, render_template, request as http_request, url_for from .blueprint import applications_bp from atst.domain.applications import Applications -from atst.domain.exceptions import AlreadyExistsError -from atst.domain.portfolios import Portfolios from atst.forms.application import NameAndDescriptionForm, EnvironmentsForm from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.permissions import Permissions @@ -13,6 +11,7 @@ from atst.routes.applications.settings import ( get_new_member_form, handle_create_member, handle_update_member, + handle_update_application, ) @@ -38,31 +37,6 @@ def render_new_application_form( return render_template(template, **render_args) -def update_application(form, application_id=None, portfolio_id=None): - if form.validate(): - application = None - try: - if application_id: - application = Applications.get(application_id) - application = Applications.update(application, form.data) - flash("application_updated", application_name=application.name) - else: - portfolio = Portfolios.get_for_update(portfolio_id) - application = Applications.create( - g.current_user, portfolio, **form.data - ) - flash("application_created", application_name=application.name) - - return application - - except AlreadyExistsError: - flash("application_name_error", name=form.data["name"]) - return False - - else: - return False - - @applications_bp.route("/portfolios//applications/new") @applications_bp.route("/applications//new/step_1") @user_can(Permissions.CREATE_APPLICATION, message="view create new application form") @@ -90,7 +64,7 @@ def create_or_update_new_application_step_1(portfolio_id=None, application_id=No form = get_new_application_form( {**http_request.form}, NameAndDescriptionForm, application_id ) - application = update_application(form, application_id, portfolio_id) + application = handle_update_application(form, application_id, portfolio_id) if application: return redirect( diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 66131c15..fc95c615 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -16,6 +16,7 @@ from atst.domain.csp.cloud import GeneralCSPException from atst.domain.common import Paginator from atst.domain.environment_roles import EnvironmentRoles from atst.domain.invitations import ApplicationInvitations +from atst.domain.portfolios import Portfolios from atst.forms.application_member import NewForm as NewMemberForm, UpdateMemberForm from atst.forms.application import NameAndDescriptionForm, EditEnvironmentForm from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS @@ -275,6 +276,29 @@ def handle_update_environment(form, application=None, environment=None): return False +def handle_update_application(form, application_id=None, portfolio_id=None): + if form.validate(): + application = None + + try: + if application_id: + application = Applications.get(application_id) + application = Applications.update(application, form.data) + flash("application_updated", application_name=application.name) + else: + portfolio = Portfolios.get_for_update(portfolio_id) + application = Applications.create( + g.current_user, portfolio, **form.data + ) + flash("application_created", application_name=application.name) + + return application + + except AlreadyExistsError: + flash("application_name_error", name=form.data["name"]) + return False + + @applications_bp.route("/applications//settings") @user_can(Permissions.VIEW_APPLICATION, message="view application edit form") def settings(application_id): @@ -334,10 +358,9 @@ def new_environment(application_id): def update(application_id): application = Applications.get(application_id) form = NameAndDescriptionForm(http_request.form) - if form.validate(): - application_data = form.data - Applications.update(application, application_data) + updated_application = handle_update_application(form, application_id) + if updated_application: return redirect( url_for( "applications.portfolio_applications", @@ -345,7 +368,10 @@ def update(application_id): ) ) else: - return render_settings_page(application=application, application_form=form) + return ( + render_settings_page(application=application, show_flash=True), + 400, + ) @applications_bp.route("/applications//delete", methods=["POST"]) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 597664d1..e9117166 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -274,6 +274,23 @@ def test_user_without_permission_cannot_update_application(client, user_session) assert application.description == "Cool stuff happening here!" +def test_update_application_enforces_unique_name(client, user_session, session): + portfolio = PortfolioFactory.create() + name = "Test Application" + application = ApplicationFactory.create(portfolio=portfolio, name=name) + dupe_application = ApplicationFactory.create(portfolio=portfolio) + user_session(portfolio.owner) + + session.begin_nested() + response = client.post( + url_for("applications.update", application_id=dupe_application.id), + data={"name": name, "description": dupe_application.description}, + ) + session.rollback() + + assert response.status_code == 400 + + def test_user_can_only_access_apps_in_their_portfolio(client, user_session): portfolio = PortfolioFactory.create() other_portfolio = PortfolioFactory.create( diff --git a/tests/test_access.py b/tests/test_access.py index ad4bd5be..ccae69ba 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -538,10 +538,16 @@ def test_applications_update_access(post_url_assert_status): ) app = portfolio.applications[0] + def _form_data(): + return { + "name": "Test Application %s" % (random.randrange(1, 1000)), + "description": "This is only a test", + } + url = url_for("applications.update", application_id=app.id) - post_url_assert_status(dev, url, 200) - post_url_assert_status(ccpo, url, 200) - post_url_assert_status(rando, url, 404) + post_url_assert_status(dev, url, 302, data=_form_data()) + post_url_assert_status(ccpo, url, 302, data=_form_data()) + post_url_assert_status(rando, url, 404, data=_form_data()) # applications.update_environments