jrief / djangocms-cascade

Build Single Page Applications using the Django-CMS plugin system
MIT License
165 stars 85 forks source link

Only an user with administrator right can use djangocms-cascade #211

Closed pcolmant closed 7 years ago

pcolmant commented 7 years ago

cms/utils/plugins.py, method def has_plugin_permission(user, plugin_type, permission_type): is used by the cms to evaluate the user permissions concerning the placeholder with a call to the django method user.has_perm(codename).

The codename is build with the app_label + '.' + django.contrib.auth.get_permission_codename

The use of

        # using a dummy name prevents `makemigrations` to create a model migration
        app_label = 'cascade_dummy'

in cmsplugin_cascade/plugin_base.py results in 'permision denied'

For the generic content, the django.contrib.auth.get_permission_codename return also an non existing codename in the DB table auth_permission.

jrief commented 7 years ago

Thanks for informing me about this. Yes, since we were always using Cascade as admin users, we never encountered this. Do you have any idea how to solve it?

pcolmant commented 7 years ago

The cms check the user rights with the placeholder without giving the posibility to the object inside the placeholder (ie the plugin) to surcharge the check.

But Django lets surcharge user.has_perm(codename) with the use of an authentication backend. And hopefully, Django allow multiple authentication backend. And nice in this case

The permissions given to the user will be the superset of all permissions returned by all backends. That is, Django grants a permission to a user that any one backend grants.

I have my own authentication backend and I made a first test with a code similar to :

class CascadeCustomBackend(ModelBackend):

    def has_perm(self, user_obj, perm, obj=None):
        if perm.startswith("cascade_dummy"):
            perm = perm.replace("cascade_dummy", "cmsplugin_cascade")
        return super(CascadeCustomBackend, self).has_perm(user_obj, perm, obj)

This works except for the generic cascade plugins. They have no entry into the DB table django_content_type. But because you need to have at least a container to add a generic plugin, I believe that settings not "cascade_dummy" as app_label for them but for example "cascade_dummy_generic", something like this in cmsplugin_cascade/plugin_base :

def create_proxy_model(name, model_mixins, base_model, attrs=None, module=None):
    """
    Create a Django Proxy Model on the fly, to be used by any Cascade Plugin.
    """
    class Meta:
        proxy = True
        # using a dummy name prevents `makemigrations` to create a model migration
        if module == "cmsplugin_cascade.generic.cms_plugins":
            app_label = 'cascade_dummy_generic'
        else:
            app_label = 'cascade_dummy'

Shall lets the authentication correctly authorize the user.

import re
ACTION_RE = re.compile(r"(?<=\.)(.*?)(?=\_)")

class CascadeCustomBackend(ModelBackend):

    def has_perm(self, user_obj, perm, obj=None):
        if perm.startswith("cascade_dummy"):
            if perm.startswith('cascade_dummy_generic'):
                match = ACTION_RE.search(perm)
                if match:
                    perm = 'cmsplugin_cascade.%s_bootstrapcontainerpluginmodel' % match.group(1)
                else:
                    return False
            else:
                perm = perm.replace('cascade_dummy', 'cmsplugin_cascade')
        return super(CascadeCustomBackend, self).has_perm(user_obj, perm, obj)

Complex, not very satisfactory, but it's a way to workaround the issue.

Dou you want I make a PR ?

jrief commented 7 years ago

I don't like this cascade_dummy stuff either. I had to introduce it because otherwise makemigrations creates migration files even for proxy models. Fortunately not for apps not in INSTALLED_APPS, therefore I've chosen cascade_dummy.

The problem in your code is, that an if/else-block in the inline Meta class doesn't work. Here the Meta class must be created on the fly.

I just created a branch named fixes/211-administrator-only-perms, which attempts to handle the creation of of the dummy app_labels in a more unique way. This could be used as a starting point for your CascadeCustomBackend.

jrief commented 7 years ago

@pcolmant Sorry for the long delay, but I think that today I found a better solution.

Using app_label = 'cascade_dummy' was kind of hack, to prevent Django to create migrations files whenever one changed the configuration of a plugin. This was specially nasty, since changing the configuration in ones project, resulted in a new migration file in cmsplugin_cascade – hence this hack.

However, as you pointed out, then the permission system did not work anymore. I didn't pay a lot of attention to this, because I always worked as administrator. In version 0.12, I now changed the logic. Instead of using a non existing app_label for the proxy models, in a faking migration file, I pretend that these proxy models have already been migrated. Therefore all proxy models now are part of the content types and the permission system. You can now even control in a very fine grained way, which user has which permissions on which plugin.

A downside of this, is that I'll have to fix the content type and permission system while bootstrapping the application, since it can be that some proxy models have been added or removed. This is not implemented yet.

Please test this in your environment and inform me about your experience. Note that you have to use my branch of the CMS, because a required PR is still pending. Please see the requirements for details.

jrief commented 7 years ago

@pcolmant On branch 0.12, I just released a version which handles the plugin permissions for you. Would be great, if you could test it.

jrief commented 7 years ago

@pcolmant Have you been able to test version 0.12 already?

pcolmant commented 7 years ago

Hi Jacob, I will do it this week. I just came back from holidays.

jrief commented 7 years ago

@pcolmant ping If your tests on this version fix this bug, I will release version 0.12.0 asap.

pcolmant commented 7 years ago

Hi Jacob,

This version fix the bug. No more need of a custom AUTHENTICATION_BACKENDS.

I have deployed on a new virtualenv on several sites in production :

django-cms==3.4.2 https://github.com/jrief/djangocms-cascade/archive/releases/0.12.x.zip djangocms-text-ckeditor==3.4.0

Thanks a lot for your work !

jrief commented 7 years ago

Thanks for testing. I'll release a version 0.12.0 officially.