Permissions: change from list to key/value pairs in manifest #168
@ -492,7 +492,11 @@ class VersionManager(models.Manager):
|
|||||||
def update_or_create(self, *args, **kwargs):
|
def update_or_create(self, *args, **kwargs):
|
||||||
# Stash the ManyToMany to be created after the Version has a valid ID already
|
# Stash the ManyToMany to be created after the Version has a valid ID already
|
||||||
licenses = kwargs.pop('licenses', [])
|
licenses = kwargs.pop('licenses', [])
|
||||||
permissions = kwargs.pop('permissions', [])
|
permissions = kwargs.pop('permissions', {})
|
||||||
|
# FIXME: ideally this dict->list transformation should be a caller's responsibility
|
||||||
|
# but we plan to change the model soon, so getting a dict as an input here makes sense
|
||||||
|
if permissions:
|
||||||
|
permissions = list(permissions.keys())
|
||||||
platforms = kwargs.pop('platforms', [])
|
platforms = kwargs.pop('platforms', [])
|
||||||
tags = kwargs.pop('tags', [])
|
tags = kwargs.pop('tags', [])
|
||||||
|
|
||||||
|
BIN
extensions/tests/files/addon-with-permissions.zip
Normal file
BIN
extensions/tests/files/addon-with-permissions.zip
Normal file
Binary file not shown.
Binary file not shown.
@ -523,19 +523,35 @@ class ValidateManifestFields(TestCase):
|
|||||||
**self.optional_fields,
|
**self.optional_fields,
|
||||||
}
|
}
|
||||||
|
|
||||||
data['permissions'] = ['files']
|
data['permissions'] = {'files': 'test files'}
|
||||||
ManifestValidator(data)
|
ManifestValidator(data)
|
||||||
|
|
||||||
data.pop('permissions')
|
data.pop('permissions')
|
||||||
ManifestValidator(data)
|
ManifestValidator(data)
|
||||||
|
|
||||||
data['permissions'] = ['non-supported-permission']
|
# punctuation
|
||||||
|
data['permissions'] = {'files': 'lalala.'}
|
||||||
with self.assertRaises(ValidationError) as e:
|
with self.assertRaises(ValidationError) as e:
|
||||||
ManifestValidator(data)
|
ManifestValidator(data)
|
||||||
|
|
||||||
message_begin = "Manifest value error: <code>permissions</code> expects a list of"
|
# empty reason
|
||||||
|
data['permissions'] = {'files': ''}
|
||||||
|
with self.assertRaises(ValidationError) as e:
|
||||||
|
ManifestValidator(data)
|
||||||
|
|
||||||
|
# too long reason
|
||||||
|
data['permissions'] = {
|
||||||
|
'files': 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
|
||||||
|
}
|
||||||
|
with self.assertRaises(ValidationError) as e:
|
||||||
|
ManifestValidator(data)
|
||||||
|
|
||||||
|
data['permissions'] = {'non-supported-permission': 'lalala'}
|
||||||
|
with self.assertRaises(ValidationError) as e:
|
||||||
|
ManifestValidator(data)
|
||||||
|
message_begin = "Manifest value error: <code>permissions</code> expects key/value pairs of"
|
||||||
self.assertIn(message_begin, e.exception.messages[0])
|
self.assertIn(message_begin, e.exception.messages[0])
|
||||||
self.assertIn('[\'files\', \'network\']', e.exception.messages[0])
|
self.assertIn('files, network', e.exception.messages[0])
|
||||||
|
|
||||||
# Check the permissions that will always be around.
|
# Check the permissions that will always be around.
|
||||||
message_end = e.exception.messages[0][len(message_begin) :]
|
message_end = e.exception.messages[0][len(message_begin) :]
|
||||||
@ -546,13 +562,13 @@ class ValidateManifestFields(TestCase):
|
|||||||
for permission in VersionPermission.objects.all():
|
for permission in VersionPermission.objects.all():
|
||||||
self.assertIn(permission.slug, message_end)
|
self.assertIn(permission.slug, message_end)
|
||||||
|
|
||||||
data['permissions'] = []
|
data['permissions'] = {}
|
||||||
with self.assertRaises(ValidationError) as e:
|
with self.assertRaises(ValidationError) as e:
|
||||||
ManifestValidator(data)
|
ManifestValidator(data)
|
||||||
self.assertEqual(1, len(e.exception.messages))
|
self.assertEqual(1, len(e.exception.messages))
|
||||||
|
|
||||||
# Make sure permissions are only defined for add-ons, but fail for themes.
|
# Make sure permissions are only defined for add-ons, but fail for themes.
|
||||||
data['permissions'] = ['files']
|
data['permissions'] = {'files': 'test files'}
|
||||||
data['type'] = EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.THEME]
|
data['type'] = EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.THEME]
|
||||||
data['tags'] = []
|
data['tags'] = []
|
||||||
with self.assertRaises(ValidationError) as e:
|
with self.assertRaises(ValidationError) as e:
|
||||||
@ -693,7 +709,7 @@ class VersionPermissionsTest(CreateFileTest):
|
|||||||
|
|
||||||
def test_version_permission_register(self):
|
def test_version_permission_register(self):
|
||||||
"""Make sure permissions are saved"""
|
"""Make sure permissions are saved"""
|
||||||
permissions = ['network']
|
permissions = {'network': 'talk to server'}
|
||||||
latest_version = self._test_version_permission(permissions)
|
latest_version = self._test_version_permission(permissions)
|
||||||
self.assertEqual(latest_version.permissions.count(), 1)
|
self.assertEqual(latest_version.permissions.count(), 1)
|
||||||
self.assertEqual(latest_version.permissions.first().slug, 'network')
|
self.assertEqual(latest_version.permissions.first().slug, 'network')
|
||||||
|
@ -63,6 +63,22 @@ EXPECTED_EXTENSION_DATA = {
|
|||||||
'version_str': '1.0.8',
|
'version_str': '1.0.8',
|
||||||
'slug': 'amaranth',
|
'slug': 'amaranth',
|
||||||
},
|
},
|
||||||
|
'addon-with-permissions.zip': {
|
||||||
|
'metadata': {
|
||||||
|
'tagline': 'Some add-on tag line',
|
||||||
|
'name': 'Some Add-on',
|
||||||
|
'id': 'some_addon',
|
||||||
|
'version': '0.1.0',
|
||||||
|
'blender_version_min': '4.2.0',
|
||||||
|
'type': 'add-on',
|
||||||
|
'permissions': {'files': 'reading files', 'network': 'talking to server'},
|
||||||
|
},
|
||||||
|
'file_hash': 'sha256:cce5f6268cb096cf8b070b17cab78ad2bc10f2ff85c625ef4a2b6db7c01a29ad',
|
||||||
|
'size_bytes': 748,
|
||||||
|
'tags': [],
|
||||||
|
'version_str': '0.1.0',
|
||||||
|
'slug': 'some-addon',
|
||||||
|
},
|
||||||
}
|
}
|
||||||
EXPECTED_VALIDATION_ERRORS = {
|
EXPECTED_VALIDATION_ERRORS = {
|
||||||
'empty.txt': {'source': ['Only .zip files are accepted.']},
|
'empty.txt': {'source': ['Only .zip files are accepted.']},
|
||||||
@ -147,6 +163,7 @@ class SubmitFileTest(TestCase):
|
|||||||
self.assertEqual(file.hash, file_hash)
|
self.assertEqual(file.hash, file_hash)
|
||||||
self.assertEqual(file.get_type_display(), 'Add-on')
|
self.assertEqual(file.get_type_display(), 'Add-on')
|
||||||
self.assertEqual(file.metadata['version'], version_str)
|
self.assertEqual(file.metadata['version'], version_str)
|
||||||
|
self.assertEqual(file.metadata.get('permissions'), other_metadata.get('permissions'))
|
||||||
|
|
||||||
def test_not_allowed_anonymous(self):
|
def test_not_allowed_anonymous(self):
|
||||||
with open(TEST_FILES_DIR / 'edit_breakdown-0.1.0.zip', 'rb') as fp:
|
with open(TEST_FILES_DIR / 'edit_breakdown-0.1.0.zip', 'rb') as fp:
|
||||||
|
@ -102,7 +102,10 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
|
|||||||
# avoid triggering additional db queries, reuse the prefetched queryset
|
# avoid triggering additional db queries, reuse the prefetched queryset
|
||||||
'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]),
|
'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]),
|
||||||
'license': [license_iter.slug for license_iter in matching_version.licenses.all()],
|
'license': [license_iter.slug for license_iter in matching_version.licenses.all()],
|
||||||
'permissions': [permission.slug for permission in matching_version.permissions.all()],
|
'permissions': {
|
||||||
|
permission.slug: permission.help
|
||||||
|
for permission in matching_version.permissions.all()
|
||||||
|
},
|
||||||
'platforms': [platform.slug for platform in matching_version.platforms.all()],
|
'platforms': [platform.slug for platform in matching_version.platforms.all()],
|
||||||
# TODO: handle copyright
|
# TODO: handle copyright
|
||||||
'tags': [str(tag) for tag in matching_version.tags.all()],
|
'tags': [str(tag) for tag in matching_version.tags.all()],
|
||||||
|
@ -321,7 +321,10 @@ class TypeValidator:
|
|||||||
|
|
||||||
|
|
||||||
class PermissionsValidator:
|
class PermissionsValidator:
|
||||||
example = ['files', 'network']
|
example = {
|
||||||
|
'files': "Import/export FBX from/to disk",
|
||||||
|
'network': "Need to sync motion-capture data to server",
|
||||||
|
}
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def validate(cls, *, name: str, value: str, manifest: dict) -> str:
|
def validate(cls, *, name: str, value: str, manifest: dict) -> str:
|
||||||
@ -342,22 +345,47 @@ class PermissionsValidator:
|
|||||||
if not is_bpy:
|
if not is_bpy:
|
||||||
return
|
return
|
||||||
|
|
||||||
if (type(value) != list) or (not value):
|
if not isinstance(value, dict) or not value:
|
||||||
is_error = True
|
is_error = True
|
||||||
else:
|
else:
|
||||||
for permission in value:
|
for permission, reason in value.items():
|
||||||
try:
|
try:
|
||||||
VersionPermission.get_by_slug(permission)
|
VersionPermission.get_by_slug(permission)
|
||||||
except VersionPermission.DoesNotExist:
|
except VersionPermission.DoesNotExist:
|
||||||
is_error = True
|
is_error = True
|
||||||
logger.info(f'Permission unavailable: {permission}')
|
logger.info(f'Permission unavailable: {permission}')
|
||||||
|
if not isinstance(reason, str):
|
||||||
|
return mark_safe(
|
||||||
|
'Manifest value error: <code>permissions</code> reasons must be strings.'
|
||||||
|
)
|
||||||
|
|
||||||
|
if not reason:
|
||||||
|
return mark_safe(
|
||||||
|
f'Manifest value error: <code>permissions</code> {permission} reason '
|
||||||
|
'is empty.'
|
||||||
|
)
|
||||||
|
|
||||||
|
if reason[-1] in {'.', '!', '?'}:
|
||||||
|
return mark_safe(
|
||||||
|
'Manifest value error: <code>permissions</code> {permission} reason '
|
||||||
|
'cannot end with any punctuation (.!?).'
|
||||||
|
)
|
||||||
|
|
||||||
|
max_length = 64
|
||||||
|
if len(reason) > max_length:
|
||||||
|
return mark_safe(
|
||||||
|
f'Manifest value error: <code>permissions</code> {permission} reason is '
|
||||||
|
'too long ({len(value)}), '
|
||||||
|
f'max-length: {max_length} characters'
|
||||||
|
)
|
||||||
|
|
||||||
if not is_error:
|
if not is_error:
|
||||||
return
|
return
|
||||||
|
|
||||||
error_message = (
|
error_message = (
|
||||||
f'Manifest value error: <code>permissions</code> expects a list of '
|
'Manifest value error: <code>permissions</code> expects key/value pairs of '
|
||||||
f'supported permissions. e.g.: {cls.example}. The supported permissions are: '
|
'supported permissions with the reasons why they are required. '
|
||||||
|
'The supported permissions are: '
|
||||||
)
|
)
|
||||||
|
|
||||||
for permission_ob in VersionPermission.objects.all():
|
for permission_ob in VersionPermission.objects.all():
|
||||||
|
Loading…
Reference in New Issue
Block a user