darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-01 07:35:34 -05:00
Code Issues Packages Projects Releases Wiki Activity

Always use both folder and service ID when getting template folder

Browse Source 
Currently there aren't any permission checks based on folder IDs in
the admin app or the API, so it's possible for a user to modify the
folder ID to perform operations on folders outside their service.

Our usual way to avoid this is to always use service_id filter when
fetching objects from the database.
This commit is contained in:
Alexey Bezhan
2018-11-07 13:48:18 +00:00
parent 1dbb24065d
commit 36f41c23e1
3 changed files with 10 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
app/dao/template_folder_dao.py
View File

@@ -3,8 +3,11 @@ from app.dao.dao_utils import transactional
from app.models import TemplateFolder 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 @transactional

11
app/template_folder/rest.py
View File

@@ -4,7 +4,7 @@ from sqlalchemy.orm.exc import NoResultFound
from app.dao.template_folder_dao import ( from app.dao.template_folder_dao import (
dao_create_template_folder, dao_create_template_folder,
dao_get_template_folder_by_id, dao_get_template_folder_by_id_and_service_id,
dao_update_template_folder, dao_update_template_folder,
dao_delete_template_folder dao_delete_template_folder
) )
@@ -49,13 +49,10 @@ def create_template_folder(service_id):
if data.get('parent_id') is not None: if data.get('parent_id') is not None:
try: try:
parent_folder = dao_get_template_folder_by_id(data['parent_id']) dao_get_template_folder_by_id_and_service_id(data['parent_id'], service_id)
except NoResultFound: except NoResultFound:
raise InvalidRequest("parent_id not found", status_code=400) raise InvalidRequest("parent_id not found", status_code=400)
if parent_folder.service_id != service_id:
raise InvalidRequest("parent_id belongs to a different service", status_code=400)
template_folder = TemplateFolder( template_folder = TemplateFolder(
service_id=service_id, service_id=service_id,
name=data['name'].strip(), name=data['name'].strip(),
@@ -73,7 +70,7 @@ def rename_template_folder(service_id, template_folder_id):
validate(data, post_rename_template_folder_schema) 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'] template_folder.name = data['name']
dao_update_template_folder(template_folder) dao_update_template_folder(template_folder)
@@ -83,7 +80,7 @@ def rename_template_folder(service_id, template_folder_id):
@template_folder_blueprint.route('/<uuid:template_folder_id>', methods=['DELETE']) @template_folder_blueprint.route('/<uuid:template_folder_id>', methods=['DELETE'])
def delete_template_folder(service_id, template_folder_id): 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) # 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: if template_folder.subfolders or template_folder.templates:

2
tests/app/template_folder/test_template_folder_rest.py
View File

@@ -97,7 +97,7 @@ def test_create_template_folder_fails_if_parent_id_from_different_service(admin_
) )
assert resp['result'] == 'error' assert resp['result'] == 'error'
assert resp['message'] == 'parent_id belongs to a different service' assert resp['message'] == 'parent_id not found'
def test_rename_template_folder(admin_request, sample_service): def test_rename_template_folder(admin_request, sample_service):