Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove custom Permission and Role model #52

Closed
4 tasks done
jokiefer opened this issue Mar 15, 2021 · 0 comments · Fixed by #58
Closed
4 tasks done

remove custom Permission and Role model #52

jokiefer opened this issue Mar 15, 2021 · 0 comments · Fixed by #58
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Milestone

Comments

@jokiefer
Copy link
Member

jokiefer commented Mar 15, 2021

Proposed Changes

remove the custom Permission and Role model from MrMap.

This results in the following changes:

structure\models.py

  • remove
class Permission(models.Model):
    name = models.CharField(max_length=500, choices=PermissionEnum.as_choices(), unique=True)

    def __str__(self):
        return str(self.name)

    def get_permission_set(self) -> set:
        p_list = set()
        perms = self.__dict__
        if perms.get("id", None) is not None:
            del perms["id"]
        if perms.get("_state", None) is not None:
            del perms["_state"]
        for perm_key, perm_val in perms.items():
            if perm_val:
                p_list.add(perm_key)
        return p_list


class Role(models.Model):
    name = models.CharField(max_length=100)
    description = models.TextField()
    permissions = models.ManyToManyField(Permission)

    def __str__(self):
        return self.name

    def has_permission(self, perm: PermissionEnum):
        """ Checks whether a permission can be found inside this role

        Args:
            perm (PermissionEnum): The permission to be checked
        Returns:
             bool
        """
        return self.permissions.filter(
            name=perm.value
        ).exists()
  • remove field
class MrMapGroup(Group):
    ...
    # todo: remove role ForeignKey
    role = models.ForeignKey(Role, on_delete=models.CASCADE, null=True)
    ...
  • remove custom permissions depending functions
class MrMapUser(AbstractUser):
    @cached_property
    def all_permissions(self) -> set:
        """Returns a set containing all permission identifiers as strings in a list.

        Returns:
             A set of permission strings
        """
        return self.get_all_permissions()

    def get_all_permissions(self, group: MrMapGroup = None) -> set:
        """Returns a set containing all permission identifiers as strings in a list.

        The list is generated by fetching all permissions from all groups the user is part of.
        Alternatively the list is generated by fetching all permissions from a special group.

        Args:
            group: The group object
        Returns:
             A set of permission strings
        """
        if group is not None:
            groups = MrMapGroup.objects.filter(id=group.id)
        else:
            groups = self.get_groups
        all_perm = set(groups.values_list("role__permissions__name", flat=True))
        return all_perm

    def has_perm(self, perm, obj=None) -> bool:
        # Active superusers have all permissions.
        if self.is_active and self.is_superuser:
            return True

        has_perm = self.get_groups.filter(
            role__permissions__name=perm
        )
        has_perm = has_perm.exists()
        return has_perm

    # do not overwrite has_perms cause of using django permission system in Rest API
    def has_permissions(self, perm_list, obj=None) -> bool:
        has_perm = self.get_groups.filter(
            role__permissions__name__in=perm_list
        )
        has_perm = has_perm.exists()
        return has_perm

MrMap/management/commands/setup_settings.py

  • migrate all custom Permissions settings to the built-in Permision system.

example - current:

DEFAULT_GROUPS = [
    {
        "name": _("Group Administrator"),
        "description": _("Permission group. Holds users which are allowed to manage groups."),
        "parent_group": None,
        "permissions": [
            PermissionEnum.CAN_CREATE_GROUP,
            PermissionEnum.CAN_DELETE_GROUP,
            PermissionEnum.CAN_EDIT_GROUP,
            PermissionEnum.CAN_ADD_USER_TO_GROUP,
            PermissionEnum.CAN_REMOVE_USER_FROM_GROUP,
            PermissionEnum.CAN_TOGGLE_PUBLISH_REQUESTS,
            PermissionEnum.CAN_REQUEST_TO_BECOME_PUBLISHER,
        ]
    },
...
]

should be changed in:

DEFAULT_GROUPS = [
    {
        "name": _("Group Administrator"),
        "description": _("Permission group. Holds users which are allowed to manage groups."),
        "parent_group": None,
        "permissions": [
            'add_mrmapgroup',
            'change_mrmapgroup',
            'delete_mrmapgroup',
            'view_mrmapgroup',
            'change_groupinvitationrequest',
            'change_publishrequest',
        ]
    },
...
]

We should still handle it with the common way by using enums.

cause we refactor all views as class based views, with matching the default CreateView, UpdateView, DeleteView and so on, we don't need to create fancy new custom permissions, if it is not really necessary. Adding or removing a user to a group is a simple update of a given MrMapGroup instance.

Justification

  • The role concept is also handled by the is_permission_group flag or MrMapGroup
  • Simplify the codebase by using existing common django built in code
@jokiefer jokiefer added type: housekeeping Changes to the application which do not directly impact the end user status: accepted This issue has been accepted for implementation labels Mar 15, 2021
@jokiefer jokiefer added this to the v1.0.0 milestone Mar 15, 2021
jokiefer added a commit that referenced this issue Mar 16, 2021
jokiefer added a commit that referenced this issue Mar 16, 2021
…on_and_Role_model

closes #52: remove custom Permission and Role model and implement dja…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant