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

Using assign_perm in django 1.7 migration #281

Closed
xuhcc opened this issue Nov 13, 2014 · 10 comments
Closed

Using assign_perm in django 1.7 migration #281

xuhcc opened this issue Nov 13, 2014 · 10 comments

Comments

@xuhcc
Copy link

xuhcc commented Nov 13, 2014

Consider the following migration code:

from django.db import models, migrations
from guardian.shortcuts import assign_perm

def assign_perms(apps, schema_editor):
    Group = apps.get_model("auth", "Group")
    for group in Group.objects.all():
        assign_perm('products.view_product', group)
        # assign_perm('products.view_product', group, product)

class Migration(migrations.Migration):

    operations = [
        migrations.RunPython(assign_perms),
    ]

When applying this migration, assign_perm call gives an error:

guardian.exceptions.NotUserNorGroup: User/AnonymousUser or Group instance is required (got Group object)
@tctimmeh
Copy link

+1

2 similar comments
@cyber778
Copy link

cyber778 commented Apr 2, 2015

+1

@jelenak
Copy link

jelenak commented Jul 10, 2015

+1

@keattang
Copy link
Contributor

I fixed this error by importing Group from django.contrib.auth , however it now doesn't seem to be making any changes in the database (see #333). That's on django 1.8 and guardian 1.3 though so if someone could try on 1.7 that would be great

@xuhcc
Copy link
Author

xuhcc commented Jul 15, 2015

I've ended up using two helper functions for migration scripts:
https://gist.github.com/xuhcc/67871719116bdc0fee6c
(django-guardian==1.2.5)

@keattang
Copy link
Contributor

Still no love with 1.8/1.3 unfortunately. Using your helper functions I get the error
ValueError: Cannot query "project": Must be "ContentType" instance at

perm = Permission.objects.get(
            content_type=get_content_type(obj),
            codename=codename)

obj is an instance of a model named project that I get like this:

projs = get_objects_for_user(user, 'api.view_project')
        for proj in projs:
                assign_perm(apps, 'api.edit_project', user, proj)

@brianmay
Copy link
Contributor

Unfortunately, using assign_perms inside migrations (or any other schema dependent code not in the migration itself) is a really bad idea. I am not surprised it doesn't work either.

The reason it doesn't work it most likely in the logic to find the permission model isn't using the correct model returned by apps.get_model() required for the migration. This might be fixable.

However, there is a fundamental problem here that is not solvable. That is the database migrations do not attempt to serialize the assign_perm() function or any of the functions assign_perm() calls. So a later version of django-guardian could change the database schema and update assign_perm() to use the new schema. Your old migrations will suddenly break because they are now running the new assign_perm() code with the old database schema.

In fact, it is possible some the the requested features, such as roles, could require this change in the schema.

This is probably not the answer you were looking for, however I think you have to use lower level database operations to set the permissions. By this I mean directly interaction with the Permission db models. Django migrations should not be accessing database schema dependent functions outside the migration. This includes custom functions on the db model class also.

Possibly this limitation could be better documented. Pull requests appreciated.

As such closing this bug.

@jekel
Copy link

jekel commented Jul 14, 2016

hello
i have same problem with 1.4.4 version pf guardian and django 1.9.7
any assign_perm inside migrations does not work... :(
is any idea how can it be totally fixed or monkey patched inside migrations?

@imaia
Copy link

imaia commented Aug 17, 2017

The reason it doesn't work it most likely in the logic to find the permission model isn't using the correct model returned by apps.get_model() required for the migration. This might be fixable.

Maybe auth groups could be wrapped to behave like a guardian group.

@stevelacey
Copy link

stevelacey commented Dec 19, 2017

I am posting this here in case it's useful for anyone else, but I wrote this relatively generic assign_perm method for an initial set up migration, makes a few assumptions but probably helpful:

def assign_perm(perm, user_or_group, obj=None, apps=None):
    permission_model = apps.get_model('auth', 'Permission')
    permission = permission_model.objects.get(codename=perm)

    # Assumes object permission models live in the same app as the model they govern
    # and are named like `ProjectUserObjectPermission`/`ProjectGroupObjectPermission`
    obj_permission_model = apps.get_model(obj._meta.app_label, '{}{}ObjectPermission'.format(
        obj._meta.object_name,
        user_or_group._meta.object_name,
    ))
    obj_permission_model.objects.create(
        # `object_pk` and `content_type_id` keys if you're not using direct foreign keys
        # https://django-guardian.readthedocs.io/en/stable/userguide/performance.html#direct-foreign-keys
        content_object=obj,
        permission=permission,
        **{
            user_or_group._meta.model_name: user_or_group
        }
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants