Extension page should be 404 for everyone unless publicly listed #167

Merged
Anna Sirota merged 5 commits from extension-detail-page-listed-only into main 2024-06-05 12:12:14 +02:00
7 changed files with 82 additions and 14 deletions
Showing only changes of commit 08ae4949a0 - Show all commits

View File

@ -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', [])

Binary file not shown.

View File

@ -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')

View File

@ -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:

View File

@ -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()],

View File

@ -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():