Don't treat permission name as a machine readable field, use slug instead #115

Merged
Oleg-Komarov merged 2 commits from permissions-use-slug into main 2024-05-06 17:40:07 +02:00
4 changed files with 42 additions and 42 deletions

View File

@ -202,8 +202,13 @@ class TagAdmin(admin.ModelAdmin):
return () return ()
class VersionPermissionAdmin(admin.ModelAdmin):
list_display = ('name', 'slug')
admin.site.register(models.Extension, ExtensionAdmin) admin.site.register(models.Extension, ExtensionAdmin)
admin.site.register(models.Version, VersionAdmin) admin.site.register(models.Version, VersionAdmin)
admin.site.register(models.Maintainer, MaintainerAdmin) admin.site.register(models.Maintainer, MaintainerAdmin)
admin.site.register(models.License, LicenseAdmin) admin.site.register(models.License, LicenseAdmin)
admin.site.register(models.Tag, TagAdmin) admin.site.register(models.Tag, TagAdmin)
admin.site.register(models.VersionPermission, VersionPermissionAdmin)

View File

@ -0,0 +1,23 @@
# Generated by Django 4.2.11 on 2024-05-06 12:10
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('extensions', '0027_unique_preview_files'),
]
operations = [
migrations.AlterField(
model_name='license',
name='slug',
field=models.SlugField(help_text='Should be taken from https://spdx.org/licenses/', unique=True),
),
migrations.AlterField(
model_name='versionpermission',
name='slug',
field=models.SlugField(help_text='Permissions add-ons are expected to need.', unique=True),
),
]

View File

@ -19,8 +19,6 @@ from constants.base import (
EXTENSION_TYPE_SLUGS, EXTENSION_TYPE_SLUGS,
FILE_STATUS_CHOICES, FILE_STATUS_CHOICES,
) )
from constants.licenses import ALL_LICENSES
from constants.version_permissions import ALL_VERSION_PERMISSIONS
import common.help_texts import common.help_texts
import extensions.fields import extensions.fields
@ -90,22 +88,13 @@ class License(CreatedModifiedMixin, models.Model):
blank=False, blank=False,
null=False, null=False,
help_text='Should be taken from https://spdx.org/licenses/', help_text='Should be taken from https://spdx.org/licenses/',
unique=True,
) )
url = models.URLField(blank=False, null=False) url = models.URLField(blank=False, null=False)
def __str__(self) -> str: def __str__(self) -> str:
return f'{self.name}' return f'{self.name}'
@classmethod
def generate(cls):
"""Generate License records from constants."""
licenses = [cls(id=li.id, name=li.name, slug=li.slug, url=li.url) for li in ALL_LICENSES]
cls.objects.bulk_create(licenses)
@classmethod
def get_by_name(cls, name: str):
return cls.objects.filter(name__startswith=name).first()
@classmethod @classmethod
def get_by_slug(cls, slug: str): def get_by_slug(cls, slug: str):
return cls.objects.filter(slug__startswith=slug).first() return cls.objects.filter(slug__startswith=slug).first()
@ -386,28 +375,16 @@ class VersionPermission(CreatedModifiedMixin, models.Model):
blank=False, blank=False,
null=False, null=False,
help_text='Permissions add-ons are expected to need.', help_text='Permissions add-ons are expected to need.',
unique=True,
) )
help = models.CharField(max_length=128, null=False, blank=False, unique=True) help = models.CharField(max_length=128, null=False, blank=False, unique=True)
def __str__(self) -> str: def __str__(self) -> str:
return f'{self.name}' return f'{self.name}'
@classmethod
def generate(cls):
"""Generate Permission records from constants."""
permissions = [
cls(id=li.id, name=li.name, slug=li.slug, help=li.help)
for li in ALL_VERSION_PERMISSIONS
]
cls.objects.bulk_create(permissions)
@classmethod
def get_by_name(cls, name: str):
return cls.objects.filter(name__startswith=name).first()
@classmethod @classmethod
def get_by_slug(cls, slug: str): def get_by_slug(cls, slug: str):
return cls.objects.filter(slug__startswith=slug).first() return cls.objects.get(slug=slug)
Oleg-Komarov marked this conversation as resolved Outdated

I'm replacing this with an exact match. Was there any usecase to do a prefix match?

I'm replacing this with an exact match. Was there any usecase to do a prefix match?

I don't recall, if it wasn't explained on the corresponding commit than just assume an exact match should be fine. I think I copied this over from the license and other code snippets

I don't recall, if it wasn't explained on the corresponding commit than just assume an exact match should be fine. I think I copied this over from the license and other code snippets
class Tag(CreatedModifiedMixin, models.Model): class Tag(CreatedModifiedMixin, models.Model):
@ -537,12 +514,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model
return return
for permission_name in _permissions: for permission_name in _permissions:
permission = VersionPermission.get_by_name(permission_name) permission = VersionPermission.get_by_slug(permission_name)
# Just ignore versions that are incompatible.
if not permission:
Oleg-Komarov marked this conversation as resolved Outdated

@dfelinto here we explicitly allow to submit a manifest with incorrect permissions names. is it intentional? shouldn't we return a validation error instead if somebody mistypes a permission slug?

@dfelinto here we explicitly allow to submit a manifest with incorrect permissions names. is it intentional? shouldn't we return a validation error instead if somebody mistypes a permission slug?

Right, it should be an error indeed. Maybe already is (caught early on by the validator)?

Right, it should be an error indeed. Maybe already is (caught early on by the validator)?
continue
self.permissions.add(permission) self.permissions.add(permission)
def set_initial_licenses(self, _licenses): def set_initial_licenses(self, _licenses):

View File

@ -54,8 +54,7 @@ class FileMIMETypeValidator:
class ExtensionIDManifestValidator: class ExtensionIDManifestValidator:
""" """Make sure the extension id is valid:
Make sure the extension id is valid:
* Extension id consists of Unicode letters, numbers or underscores. * Extension id consists of Unicode letters, numbers or underscores.
* Neither hyphens nor spaces are supported. * Neither hyphens nor spaces are supported.
* Each extension id most be unique across all extensions. * Each extension id most be unique across all extensions.
@ -307,8 +306,9 @@ class PermissionsValidator:
is_error = True is_error = True
else: else:
for permission in value: for permission in value:
if VersionPermission.get_by_slug(permission): try:
continue VersionPermission.get_by_slug(permission)
except VersionPermission.DoesNotExist:
is_error = True is_error = True
logger.info(f'Permission unavailable: {permission}') logger.info(f'Permission unavailable: {permission}')