diff --git a/reviewboard/webapi/models.py b/reviewboard/webapi/models.py index e0264a4e8af72b93ec61380a41963db5a5f9e0c6..5c236b0b74d90485a28e9b1c02c6a705b39f8b65 100644 --- a/reviewboard/webapi/models.py +++ b/reviewboard/webapi/models.py @@ -1,8 +1,9 @@ from __future__ import unicode_literals from django.contrib.auth.models import User +from django.core.exceptions import ValidationError from django.db import models -from django.utils import timezone +from django.utils import six, timezone from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from djblets.db.fields import JSONField @@ -49,6 +50,111 @@ class WebAPIToken(models.Model): def __str__(self): return 'Web API token for %s' % self.user + @classmethod + def validate_policy(cls, policy): + """Validates an API policy. + + This will check to ensure that the policy is in a suitable format + and contains the information required in a format that can be parsed. + + If a failure is found, a ValidationError will be raised describing + the error and where it was found. + """ + from reviewboard.webapi.resources import resources + + if not isinstance(policy, dict): + raise ValidationError(_('The policy must be a JSON object.')) + + if policy == {}: + # Empty policies are equivalent to allowing full access. If this + # is empty, we can stop here. + return + + if 'resources' not in policy: + raise ValidationError( + _("The policy is missing a 'resources' section.")) + + resources_section = policy['resources'] + + if not isinstance(resources_section, dict): + raise ValidationError( + _("The policy's 'resources' section must be a JSON object.")) + + if not resources_section: + raise ValidationError( + _("The policy's 'resources' section must not be empty.")) + + if '*' in resources_section: + cls._validate_policy_section(resources_section, '*', + 'resources.*') + + resource_policies = [ + (section_name, section) + for section_name, section in six.iteritems(resources_section) + if section_name != '*' + ] + + if resource_policies: + valid_policy_ids = cls._get_valid_policy_ids(resources.root) + + for policy_id, section in resource_policies: + if policy_id not in valid_policy_ids: + raise ValidationError( + _("'%s' is not a valid resource policy ID.") + % policy_id) + + for subsection_name, subsection in six.iteritems(section): + if not isinstance(subsection_name, six.text_type): + raise ValidationError( + _("%s must be a string in 'resources.%s'") + % (subsection_name, policy_id)) + + cls._validate_policy_section( + section, subsection_name, + 'resources.%s.%s' % (policy_id, subsection_name)) + + @classmethod + def _validate_policy_section(cls, parent_section, section_name, + full_section_name): + section = parent_section[section_name] + + if not isinstance(section, dict): + raise ValidationError( + _("The '%s' section must be a JSON object.") + % full_section_name) + + if 'allow' not in section and 'block' not in section: + raise ValidationError( + _("The '%s' section must have 'allow' and/or 'block' " + "rules.") + % full_section_name) + + if 'allow' in section and not isinstance(section['allow'], list): + raise ValidationError( + _("The '%s' section's 'allow' rule must be a list.") + % full_section_name) + + if 'block' in section and not isinstance(section['block'], list): + raise ValidationError( + _("The '%s' section's 'block' rule must be a list.") + % full_section_name) + + @classmethod + def _get_valid_policy_ids(cls, resource, result=None): + if result is None: + result = set() + + if hasattr(resource, 'policy_id'): + result.add(resource.policy_id) + + for child_resource in resource.list_child_resources: + cls._get_valid_policy_ids(child_resource, result) + + for child_resource in resource.item_child_resources: + cls._get_valid_policy_ids(child_resource, result) + + return result + class Meta: verbose_name = _('Web API token') verbose_name_plural = _('Web API tokens') diff --git a/reviewboard/webapi/resources/api_token.py b/reviewboard/webapi/resources/api_token.py index ef297123c52ddfbd36502ac931063b9518dfd33a..bf12d0b3825b23a9195304c5eb90476dd42f0a5e 100644 --- a/reviewboard/webapi/resources/api_token.py +++ b/reviewboard/webapi/resources/api_token.py @@ -2,8 +2,9 @@ from __future__ import unicode_literals import json -from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.utils import six +from django.utils.translation import ugettext as _ from djblets.util.decorators import augment_method_from from djblets.webapi.decorators import (webapi_login_required, webapi_request_fields, @@ -198,10 +199,10 @@ class APITokenResource(WebAPIResource): if 'policy' in kwargs: try: token.policy = self._validate_policy(kwargs['policy']) - except Exception as e: + except ValidationError as e: return INVALID_FORM_DATA, { 'fields': { - 'policy': six.text_type(e), + 'policy': e.message, }, } @@ -235,8 +236,17 @@ class APITokenResource(WebAPIResource): """ pass - def _validate_policy(self, policy): - return json.loads(policy) + def _validate_policy(self, policy_str): + try: + policy = json.loads(policy_str) + except Exception as e: + raise ValidationError( + _('The policy is not valid JSON: %s') + % six.text_type(e)) + + self.model.validate_policy(policy) + + return policy api_token_resource = APITokenResource() diff --git a/reviewboard/webapi/tests/test_api_policy.py b/reviewboard/webapi/tests/test_api_policy.py index fde11682eebc44eb6f6ce3ea3758e908311265be..b63e533b9c7cc6cd83109f196f20d50851c8f7a1 100644 --- a/reviewboard/webapi/tests/test_api_policy.py +++ b/reviewboard/webapi/tests/test_api_policy.py @@ -1,9 +1,11 @@ from __future__ import unicode_literals +from django.core.exceptions import ValidationError from django.utils import six from reviewboard.testing import TestCase from reviewboard.webapi.base import WebAPIResource +from reviewboard.webapi.models import WebAPIToken class PolicyTestResource(WebAPIResource): @@ -303,3 +305,332 @@ class APIPolicyTests(TestCase): if allowed: self.fail('Expected %s to be blocked, but was allowed' % method) + + +class APIPolicyValidationTests(TestCase): + """Tests API policy validation.""" + def test_empty(self): + """Testing WebAPIToken.validate_policy with empty policy""" + WebAPIToken.validate_policy({}) + + def test_not_object(self): + """Testing WebAPIToken.validate_policy without JSON object""" + self.assertRaisesMessage( + ValidationError, + 'The policy must be a JSON object.', + WebAPIToken.validate_policy, + []) + + # + # Top-level 'resources' object + # + + def test_no_resources_section(self): + """Testing WebAPIToken.validate_policy with non-empty policy and + no resources section + """ + self.assertRaisesMessage( + ValidationError, + "The policy is missing a 'resources' section.", + WebAPIToken.validate_policy, + { + 'foo': {} + }) + + def test_resources_empty(self): + """Testing WebAPIToken.validate_policy with empty resources section""" + self.assertRaisesMessage( + ValidationError, + "The policy's 'resources' section must not be empty.", + WebAPIToken.validate_policy, + { + 'resources': {} + }) + + def test_resources_invalid_format(self): + """Testing WebAPIToken.validate_policy with resources not an object""" + self.assertRaisesMessage( + ValidationError, + "The policy's 'resources' section must be a JSON object.", + WebAPIToken.validate_policy, + { + 'resources': [] + }) + + # + # '*' section + # + + def test_global_valid(self): + """Testing WebAPIToken.validate_policy with valid '*' section""" + WebAPIToken.validate_policy({ + 'resources': { + '*': { + 'allow': ['*'], + 'block': ['POST'], + } + } + }) + + def test_empty_global(self): + """Testing WebAPIToken.validate_policy with empty '*' section""" + self.assertRaisesMessage( + ValidationError, + "The 'resources.*' section must have 'allow' and/or 'block' " + "rules.", + WebAPIToken.validate_policy, + { + 'resources': { + '*': {} + } + }) + + def test_global_not_object(self): + """Testing WebAPIToken.validate_policy with '*' section not a + JSON object + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.*' section must be a JSON object.", + WebAPIToken.validate_policy, + { + 'resources': { + '*': [] + } + }) + + def test_global_allow_not_list(self): + """Testing WebAPIToken.validate_policy with *.allow not a list""" + self.assertRaisesMessage( + ValidationError, + "The 'resources.*' section's 'allow' rule must be a list.", + WebAPIToken.validate_policy, + { + 'resources': { + '*': { + 'allow': {} + } + } + }) + + def test_global_block_not_list(self): + """Testing WebAPIToken.validate_policy with *.block not a list""" + self.assertRaisesMessage( + ValidationError, + "The 'resources.*' section's 'block' rule must be a list.", + WebAPIToken.validate_policy, + { + 'resources': { + '*': { + 'block': {} + } + } + }) + + # + # resource-specific '*' section + # + + def test_resource_global_valid(self): + """Testing WebAPIToken.validate_policy with .* valid""" + WebAPIToken.validate_policy({ + 'resources': { + 'repository': { + '*': { + 'allow': ['*'], + 'block': ['POST'], + }, + } + } + }) + + def test_resource_global_empty(self): + """Testing WebAPIToken.validate_policy with .* empty""" + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.*' section must have 'allow' and/or " + "'block' rules.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '*': {} + } + } + }) + + def test_resource_global_invalid_policy_id(self): + """Testing WebAPIToken.validate_policy with .* with + invalid policy ID + """ + self.assertRaisesMessage( + ValidationError, + "'foobar' is not a valid resource policy ID.", + WebAPIToken.validate_policy, + { + 'resources': { + 'foobar': { + '*': { + 'allow': ['*'], + } + } + } + }) + + def test_resource_global_not_object(self): + """Testing WebAPIToken.validate_policy with .* not an + object + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.*' section must be a JSON object.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '*': [] + } + } + }) + + def test_resource_global_allow_not_list(self): + """Testing WebAPIToken.validate_policy with .*.allow not + a list + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.*' section's 'allow' rule must be a " + "list.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '*': { + 'allow': {} + } + } + } + }) + + def test_resource_global_block_not_list(self): + """Testing WebAPIToken.validate_policy with .*.block not + a list + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.*' section's 'block' rule must be a " + "list.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '*': { + 'block': {} + } + } + } + }) + + # + # resource-specific ID section + # + + def test_resource_id_valid(self): + """Testing WebAPIToken.validate_policy with . valid""" + WebAPIToken.validate_policy({ + 'resources': { + 'repository': { + '42': { + 'allow': ['*'], + 'block': ['POST'], + }, + } + } + }) + + def test_resource_id_empty(self): + """Testing WebAPIToken.validate_policy with . empty""" + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.42' section must have 'allow' and/or " + "'block' rules.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '42': {} + } + } + }) + + def test_resource_id_invalid_id_type(self): + """Testing WebAPIToken.validate_policy with . with + invalid ID type + """ + self.assertRaisesMessage( + ValidationError, + "42 must be a string in 'resources.repository'", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + 42: { + 'allow': ['*'], + } + } + } + }) + + def test_resource_id_not_object(self): + """Testing WebAPIToken.validate_policy with . not an + object + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.42' section must be a JSON object.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '42': [] + } + } + }) + + def test_resource_id_allow_not_list(self): + """Testing WebAPIToken.validate_policy with ..allow not + a list + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.42' section's 'allow' rule must " + "be a list.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '42': { + 'allow': {} + } + } + } + }) + + def test_resource_id_block_not_list(self): + """Testing WebAPIToken.validate_policy with ..block not + a list + """ + self.assertRaisesMessage( + ValidationError, + "The 'resources.repository.42' section's 'block' rule must " + "be a list.", + WebAPIToken.validate_policy, + { + 'resources': { + 'repository': { + '42': { + 'block': {} + } + } + } + })