diff --git a/pybossa/api/project.py b/pybossa/api/project.py index fd1d387a1..ff25ad0be 100644 --- a/pybossa/api/project.py +++ b/pybossa/api/project.py @@ -30,7 +30,7 @@ from .api_base import APIBase from pybossa.model.project import Project from pybossa.cache.categories import get_all as get_categories -from pybossa.util import is_reserved_name, description_from_long_description +from pybossa.util import is_reserved_name, description_from_long_description, validate_ownership_id from pybossa.core import auditlog_repo, result_repo, http_signer from pybossa.auditlogger import AuditLogger from pybossa.data_access import ensure_user_assignment_to_project, set_default_amp_store @@ -147,6 +147,7 @@ def _validate_instance(self, project): msg = "Project short_name is not valid, as it's used by the system." raise ValueError(msg) ensure_user_assignment_to_project(project) + validate_ownership_id(project.info.get('ownership_id')) def _log_changes(self, old_project, new_project): auditlogger.add_log_entry(old_project, new_project, current_user) diff --git a/pybossa/themes/default b/pybossa/themes/default index 1ea341c95..8fe070534 160000 --- a/pybossa/themes/default +++ b/pybossa/themes/default @@ -1 +1 @@ -Subproject commit 1ea341c95032a6506f8b139eaf5bf050a9109dfb +Subproject commit 8fe0705349ef4f44d18b9ce86410a90334371cc4 diff --git a/pybossa/util.py b/pybossa/util.py index f75e7513d..16d432ba0 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -368,10 +368,12 @@ def datetime_filter(source, fmt): return source.strftime(fmt) -def validate_ownership_id(ownership_id): - if ownership_id == None or len(ownership_id) == 0: - return True - return ownership_id.isnumeric() and len(ownership_id) <= 20 +def validate_ownership_id(o_id): + ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID') + if o_id == None or len(o_id) == 0: + return + if not (o_id.isnumeric() and len(o_id) <= 20): + raise ValueError(f"{ownership_id_title} must be numeric and less than 20 characters. Got: {o_id}") class Pagination(object): diff --git a/pybossa/view/projects.py b/pybossa/view/projects.py index 3f071f183..e60bfc54a 100644 --- a/pybossa/view/projects.py +++ b/pybossa/view/projects.py @@ -3513,35 +3513,6 @@ def del_coowner(short_name, user_name=None): return abort(404) -@blueprint.route('//ownership_id', methods=['GET', 'PUT']) -@login_required -def ownership_id(short_name): - """Manage project project ownership identifer.""" - project, owner, ps = project_by_shortname(short_name) - response = {'title': current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')} - - if request.method == 'GET': - ensure_authorized_to('read', project) - response['ownership_id'] = project.info.get('ownership_id', None) - - if request.method == 'PUT': - ensure_authorized_to('update', project) - data = request.get_json() - new_ownership_id = data.get("ownership_id") - if not validate_ownership_id(new_ownership_id): - flash(gettext('Ownership ID must be numeric and less than 20 characters!'), 'error') - handle_content_type(response) - old_ownership_id = project.info.get('ownership_id', None) - if new_ownership_id != old_ownership_id: - auditlogger.log_event(project, current_user, 'update', 'project.ownership_id', - old_ownership_id, new_ownership_id) - project.info['ownership_id'] = new_ownership_id - project_repo.save(project) - response['ownership_id'] = new_ownership_id - flash(gettext('Ownership ID updated successfully'), 'success') - - return handle_content_type(response) - @blueprint.route('//projectreport/export', methods=['GET', 'POST']) @login_required @admin_or_subadmin_required diff --git a/test/test_ownership_id.py b/test/test_ownership_id.py deleted file mode 100644 index 5d38efb34..000000000 --- a/test/test_ownership_id.py +++ /dev/null @@ -1,77 +0,0 @@ -import json -from unittest.mock import patch -from test.helper import web -from test import with_context -from test import db, Fixtures -from test.factories import ProjectFactory, UserFactory -from pybossa.core import user_repo -from pybossa.repositories import ProjectRepository - - -class TestOwnershipId(web.Helper): - - def setup(self): - super(TestOwnershipId, self).setUp() - self.project_repo = ProjectRepository(db) - - @with_context - def test_00_access_ownership_id(self): - """Test admin and owner can access coowners page""" - self.register() - self.signin() - self.new_project() - - res = self.app.get('/project/sampleapp/ownership_id', content_type='application/json', follow_redirects=True) - assert "ownership_id" in str(res.data), res.data - - self.signout() - self.signin() - - - @with_context - def test_01_edit_ownership_id(self): - """Test admin and owner can edit ownership id""" - self.register() - csrf = self.get_csrf('/account/signin') - self.signin() - self.new_project() - - # test add id - payload = {'ownership_id': '12345'} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "12345" in str(res.data), res.data - # test same id as saved - payload = {'ownership_id': '12345'} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "12345" in str(res.data), res.data - # test remove id - payload = {'ownership_id': ''} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "12345" not in str(res.data), res.data - - self.signout() - self.signin() - - - @with_context - def test_02_invalid_ownership_ids(self): - """Test ownership id validation""" - self.register() - csrf = self.get_csrf('/account/signin') - self.signin() - self.new_project() - - payload = {'ownership_id': 'abcd123'} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "Ownership ID must be numeric and less than 20 characters!" in str(res.data), res.data - - payload = {'ownership_id': '123!!!abc'} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "Ownership ID must be numeric and less than 20 characters!" in str(res.data), res.data - - payload = {'ownership_id': '1111111111111111111111'} - res = self.app.put('/project/sampleapp/ownership_id', headers={'X-CSRFToken': csrf}, content_type='application/json', data=json.dumps(payload)) - assert "Ownership ID must be numeric and less than 20 characters!" in str(res.data), res.data - - self.signout() - self.signin() diff --git a/test/test_util.py b/test/test_util.py index 96a741392..1d6d2aec7 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -1898,3 +1898,21 @@ def test_map_locations_none(self): mapped_locations = util.map_locations(input_locations) assert mapped_locations['locations'] == expected_locations + + @with_context + def test_validate_ownership_id(self): + # valid ownership_id + ownership_id = "1111" + util.validate_ownership_id(ownership_id) + + # empty ownership_id + ownership_id = "" + util.validate_ownership_id(ownership_id) + + # ownership_id too long (> 20 chars) + ownership_id = "123412341234123412341234" + assert_raises(ValueError, util.validate_ownership_id, ownership_id) + + # ownership_id not numeric + ownership_id = "1234abcd1234" + assert_raises(ValueError, util.validate_ownership_id, ownership_id)