Merge pull request #2213 from alphagov/check-parent-folder-same-service

Check parent folder belongs to the same service

app/dao/template_folder_dao.py

@@ -3,8 +3,11 @@ from app.dao.dao_utils import transactional
 from app.models import TemplateFolder
 
 
-def dao_get_template_folder_by_id(template_folder_id):
+def dao_get_template_folder_by_id_and_service_id(template_folder_id, service_id):
-    return TemplateFolder.query.filter(TemplateFolder.id == template_folder_id).one()
+    return TemplateFolder.query.filter(
+        TemplateFolder.id == template_folder_id,
+        TemplateFolder.service_id == service_id
+    ).one()
 
 
 @transactional

app/template_folder/rest.py

@@ -1,14 +1,15 @@
 from flask import Blueprint, jsonify, request
 from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm.exc import NoResultFound
 
 from app.dao.template_folder_dao import (
     dao_create_template_folder,
-    dao_get_template_folder_by_id,
+    dao_get_template_folder_by_id_and_service_id,
     dao_update_template_folder,
     dao_delete_template_folder
 )
 from app.dao.services_dao import dao_fetch_service_by_id
-from app.errors import register_errors
+from app.errors import InvalidRequest, register_errors
 from app.models import TemplateFolder
 from app.template_folder.template_folder_schema import (
     post_create_template_folder_schema,
@@ -46,6 +47,12 @@ def create_template_folder(service_id):
 
     validate(data, post_create_template_folder_schema)
 
+    if data.get('parent_id') is not None:
+        try:
+            dao_get_template_folder_by_id_and_service_id(data['parent_id'], service_id)
+        except NoResultFound:
+            raise InvalidRequest("parent_id not found", status_code=400)
+
     template_folder = TemplateFolder(
         service_id=service_id,
         name=data['name'].strip(),
@@ -63,7 +70,7 @@ def rename_template_folder(service_id, template_folder_id):
 
     validate(data, post_rename_template_folder_schema)
 
-    template_folder = dao_get_template_folder_by_id(template_folder_id)
+    template_folder = dao_get_template_folder_by_id_and_service_id(template_folder_id, service_id)
     template_folder.name = data['name']
 
     dao_update_template_folder(template_folder)
@@ -73,7 +80,7 @@ def rename_template_folder(service_id, template_folder_id):
 
 @template_folder_blueprint.route('/<uuid:template_folder_id>', methods=['DELETE'])
 def delete_template_folder(service_id, template_folder_id):
-    template_folder = dao_get_template_folder_by_id(template_folder_id)
+    template_folder = dao_get_template_folder_by_id_and_service_id(template_folder_id, service_id)
 
     # don't allow deleting if there's anything in the folder (even if it's just more empty subfolders)
     if template_folder.subfolders or template_folder.templates:

tests/app/template_folder/test_template_folder_rest.py

@@ -74,9 +74,6 @@ def test_create_template_folder_fails_if_missing_fields(admin_request, sample_se
 
 
 def test_create_template_folder_fails_if_unknown_parent_id(admin_request, sample_service):
-    # create existing folder
-    create_template_folder(sample_service)
-
     resp = admin_request.post(
         'template_folder.create_template_folder',
         service_id=sample_service.id,
@@ -88,6 +85,21 @@ def test_create_template_folder_fails_if_unknown_parent_id(admin_request, sample
     assert resp['message'] == 'parent_id not found'
 
 
+def test_create_template_folder_fails_if_parent_id_from_different_service(admin_request, sample_service):
+    s1 = create_service(service_name='a')
+    parent_folder_id = create_template_folder(s1).id
+
+    resp = admin_request.post(
+        'template_folder.create_template_folder',
+        service_id=sample_service.id,
+        _data={'name': 'bar', 'parent_id': str(parent_folder_id)},
+        _expected_status=400
+    )
+
+    assert resp['result'] == 'error'
+    assert resp['message'] == 'parent_id not found'
+
+
 def test_rename_template_folder(admin_request, sample_service):
     existing_folder = create_template_folder(sample_service)