diff --git a/pybossa/api/project.py b/pybossa/api/project.py index bb61c7d60..7d653f918 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, validate_ownership_id +from pybossa.util import is_reserved_name, description_from_long_description 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 @@ -149,7 +149,6 @@ 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/forms/dynamic_forms.py b/pybossa/forms/dynamic_forms.py index ea5719e8f..e731a871e 100644 --- a/pybossa/forms/dynamic_forms.py +++ b/pybossa/forms/dynamic_forms.py @@ -1,8 +1,9 @@ +from flask import current_app from flask_babel import lazy_gettext from flask_wtf import FlaskForm as Form from wtforms import SelectField, validators, TextField, BooleanField from pybossa.forms.fields.select_two import Select2Field -from .validator import AmpPvfValidator +from .validator import AmpPvfValidator, OwnershipIdValidator import wtforms @@ -38,6 +39,10 @@ def __init__(self, *args, **kwargs): lazy_gettext('Annotation Store PVF'), [AmpPvfValidator()]) + ProjectFormExtraInputs.ownership_id = TextField( + lazy_gettext(current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')), + [OwnershipIdValidator()]) + generate_form = ProjectFormExtraInputs(form_data, obj=obj) if data_access_levels and not form_data: generate_form.amp_store.data = bool(not obj or obj.amp_store) diff --git a/pybossa/forms/validator.py b/pybossa/forms/validator.py index fb2972bb8..6f0bd2ead 100644 --- a/pybossa/forms/validator.py +++ b/pybossa/forms/validator.py @@ -23,7 +23,7 @@ import re import requests -from pybossa.util import is_reserved_name, check_password_strength +from pybossa.util import is_reserved_name, check_password_strength, valid_ownership_id from pybossa.data_access import valid_user_type_based_data_access @@ -254,3 +254,17 @@ def __call__(self, form, field): amp_store = form.amp_store.data if amp_store and not(amp_pvf and self.pvf_format.match(amp_pvf)): raise ValidationError("Invalid PVF format. Must contain .") + + +class OwnershipIdValidator(object): + """Ensure Ownership ID is valid.""" + def __init__(self, message=None): + self.ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID') + if not message: + message = lazy_gettext(f"Invalid {self.ownership_id_title}.") + self.message = message + + def __call__(self, form, field): + o_id = form.ownership_id.data + if not valid_ownership_id(o_id): + raise ValidationError(f"{self.ownership_id_title} must be numeric and less than 20 characters.") diff --git a/pybossa/repositories/project_repository.py b/pybossa/repositories/project_repository.py index 47852c4d8..a0d66e026 100644 --- a/pybossa/repositories/project_repository.py +++ b/pybossa/repositories/project_repository.py @@ -32,7 +32,7 @@ from flask import current_app import re from functools import reduce - +from pybossa.util import valid_ownership_id class ProjectRepository(Repository): @@ -72,6 +72,7 @@ def save(self, project): self._verify_required_fields(project) self._verify_product_subproduct(project) self._verify_project_info_fields(project) + self._verify_ownership_id(project) try: self.db.session.add(project) self.db.session.commit() @@ -86,6 +87,7 @@ def update(self, project): self._verify_has_password(project) self._verify_data_classification(project) self._verify_annotation_config(project) + self._verify_ownership_id(project) try: self.db.session.merge(project) self.db.session.commit() @@ -215,6 +217,12 @@ def _verify_data_classification(self, project): if data_access: project.info['data_access'] = [data_access] + def _verify_ownership_id(self, project): + ownership_id = project.info.get('ownership_id') + if not valid_ownership_id(ownership_id): + ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID') + raise BadRequest(f"{ownership_id_title} must be numeric and less than 20 characters.") + def _verify_annotation_config(self, project): data_access = project.info.get('data_access', [])[0] valid_data_access = current_app.config.get('VALID_ACCESS_LEVELS', []) diff --git a/pybossa/themes/default b/pybossa/themes/default index 42195081a..f24287be5 160000 --- a/pybossa/themes/default +++ b/pybossa/themes/default @@ -1 +1 @@ -Subproject commit 42195081a6bb08ce711d5eea2c63ba73cb37d899 +Subproject commit f24287be54fadee345abcafd371836d563ca9b3d diff --git a/pybossa/util.py b/pybossa/util.py index 28df1e40f..943a27af0 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -373,12 +373,10 @@ def datetime_filter(source, fmt): return source.strftime(fmt) -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}") +def valid_ownership_id(o_id): + if not o_id or (isinstance(o_id, str) and o_id.isnumeric() and len(o_id) <= 20): + return True + return False class Pagination(object): diff --git a/pybossa/view/projects.py b/pybossa/view/projects.py index ee996a17c..7fa1a1952 100644 --- a/pybossa/view/projects.py +++ b/pybossa/view/projects.py @@ -108,7 +108,7 @@ from copy import deepcopy from pybossa.cache import delete_memoized from sqlalchemy.orm.attributes import flag_modified -from pybossa.util import admin_or_project_owner, validate_ownership_id +from pybossa.util import admin_or_project_owner, valid_ownership_id from pybossa.api.project import ProjectAPI cors_headers = ['Content-Type', 'Authorization'] diff --git a/test/test_forms.py b/test/test_forms.py index c87a4c682..780ec0644 100644 --- a/test/test_forms.py +++ b/test/test_forms.py @@ -168,6 +168,25 @@ def test_amp_pvf_validator_pass(self): u = validator.AmpPvfValidator() u.__call__(form, form.amp_pvf) + @with_context + @raises(ValidationError) + def test_ownership_id_validator_raises_error(self): + """Test OwnershipIdValidator raise exception with invalid Ownership ID value """ + form_data = dict() + form = dynamic_project_form(ProjectForm, form_data, [], {}, {}) + form.ownership_id.data = 'xxx yyy' + u = validator.OwnershipIdValidator() + u.__call__(form, form.ownership_id) + + @with_context + def test_ownership_id_validator_pass(self): + """Test OwnershipIdValidator pass with correct format Ownership ID value """ + form_data = dict() + form = dynamic_project_form(ProjectForm, form_data, [], {}, {}) + form.ownership_id.data = '123123123' + u = validator.OwnershipIdValidator() + u.__call__(form, form.ownership_id) + class TestRegisterForm(Test): diff --git a/test/test_util.py b/test/test_util.py index 0054120a0..b9053c2d5 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -1930,19 +1930,19 @@ def test_map_locations_none(self): assert mapped_locations['locations'] == expected_locations, (mapped_locations['locations'], expected_locations) @with_context - def test_validate_ownership_id(self): + def test_valid_ownership_id(self): # valid ownership_id ownership_id = "1111" - util.validate_ownership_id(ownership_id) + assert util.valid_ownership_id(ownership_id) == True # empty ownership_id ownership_id = "" - util.validate_ownership_id(ownership_id) + assert util.valid_ownership_id(ownership_id) == True # ownership_id too long (> 20 chars) ownership_id = "123412341234123412341234" - assert_raises(ValueError, util.validate_ownership_id, ownership_id) + assert util.valid_ownership_id(ownership_id) == False # ownership_id not numeric ownership_id = "1234abcd1234" - assert_raises(ValueError, util.validate_ownership_id, ownership_id) + assert util.valid_ownership_id(ownership_id) == False