[BUG] Adding a plugin to an apphooked page as admin raises permission error

aacimov commented 4 months ago

Description

I have an AppHooked application with namespaced URL configuration (each AppHook represents different Product category and Category is my namespace).

Since I want to be able to add plugins to the AppHooked page placeholders there is no default List view shown on the AppHooked page but instead I made a CMS Plugin named Products list which routes customers to product's detail page with the detail URL with namespaced URL respectively.

On my first AppHooked page (Serial production) everything works as expected, added a Product list CMS plugin and it works. On my other AppHooked page (Hand made production) when I try adding the same plugin it raises the error saying You don't have permission to add plugin but I am logged in as an admin. No other plugin can be added to the same placeholder as well. It also reflects to the first AppHooked page where the plugin is added. I also cannot edit the current plugin (Product list).

Steps to reproduce

Create two pages (e.g. Page 1 and Page 2)
Add the same AppHook with different namespaced configs (AppHook Products with configs Config 1 and Config 2) to Page 1 and Page 2 respectively
Add a custom made CMS Plugin showing the list of products (Product list) to Page 1 - works well
Try adding the same custom made CMS Plugin (Product list) to Page 2
Permission error raised in the plugin modal window
Since this point any other plugin (e.g. TextPlugin) added to any of the stated AppHooked pages does not work raising the permission error while same issue raising when trying to edit a successfully added Product list plugin (step 3.)

Expected behaviour

Plugins should not raise the permission error since logged in as an admin user. Plugins should be added with no errors.

Additional information (CMS/Python/Django versions)

Django 4.2.10
Django CMS 4.1.0
Django Treebeard 4.7.1
Python 3.10
MySQL 8.0.34 (utf8mb4_general_ci collation)

Do you want to help fix this issue?

[x] Yes, I want to help fix this issue and I will join #workgroup-pr-review on Slack to confirm with the community that a PR is welcome.
[ ] No, I only want to report the issue.

aacimov commented 4 months ago

Any other soul having the issue? I need to decide whether to use Django CMS 4 for more complex solutions or not. It really is a deal-breaker for me at the moment since I have incoming projects with the same principle of hooking up pages.

I couldn't find anything related in the docs, or I missed it.

Thanks a lot!

fsbraun commented 4 months ago

I have not encountered this issue when using multiple instances of namespaced apphooks.

I wonder what Placeholder you are adding the custom plugins to. While django CMS 3 allowed access to page placeholders on apphook root pages, this was never a documented feature and is not immediately possible for v4 without making them explicitly available and set them for the toolbar (see https://github.com/django-cms/django-cms/issues/7712).

I am suspecting you are somehow accessing the wrong placeholder and the permission denied is coming from the fact that the placeholder belongs to a published (and therefore write-protected) page. But that's just a stab in the dark.

Can you check which placeholders you are trying to add plugins to. Are they different for the instances (they should)? Do they belong to a publsihed object?

aacimov commented 4 months ago

Hmmm. I am actually using my default "Content" template with "Content" placeholder. AppHook role is actually just to be able to hook the URL root to the CMS page (to be able to get e.g. /serial-production/1/test-product/ or /handmade-production/2/another-test-product/

The point is controlling the list through my CMS plugin added to "Content" placeholder (e.g. items shown on 1 page) and/or adding a Disclaimer at the bottom of the list.

Indeed v3 allowed this and it was an awesome feature since clients often ask for display control through plugins.

I am not sure I understand why this is not possible in v4 since the Toolbar is not involved at all. It is not the detail page with PlaceholderMixin "deep down the url structure".

And the weird thing is that it works as expected on my Page 1. When same Apphook with different namespace is added to Page 2 the error appears.

Thanks in advance.

fsbraun commented 4 months ago

The reason this changed in v4 are the edit and preview endpoints. The toolbar is the thing allowing you to edit a page in the first place. So it is involved. ;-) The edit endpoints for your apphook will get the apphook view which then must advertise which model instance is to be edited or previewed. This happened implicitly in v3 through the URL of the page (which also was a URL for a valid page).

So you must be doing something in your view to show the placeholder. I assume this code causes the issue. The issue in linked contains a snipped which will advertise the page content object for the apphook's root URL. Maybe it solves your problem? Maybe you can share your apphook root view's code?

aacimov commented 4 months ago

@fsbraun my bad for not understanding. It may sound a little hostile in my previous reply but it really wasn't (when I read it again it really could have been misinterpreted).

My urls.py has only the detail URL:

urlpatterns = [
    re_path(r'^(?P<product_id>[\d]+)/(?P<product_slug>[\w-]+)/', ProductDetail.as_view(), name='product_detail'),
]

AppHook root is not defined since I am using a custom Plugin with list queryset which is added to my "Content" placeholder.

My cms_app_config.py

class CMSConfigApp(CMSApp):

    def get_configs(self):
        return self.app_config.objects.all()

    def get_config(self, namespace):
        try:
            return self.app_config.objects.get(namespace=namespace)
        except ObjectDoesNotExist:
            return None

    def get_config_add_url(self):
        try:
            return reverse('admin:%s_%s_add' % (self.app_config._meta.app_label,
                                                self.app_config._meta.model_name))
        except:  # pragma: no cover
            return reverse('admin:%s_%s_add' % (self.app_config._meta.app_label,
                                                self.app_config._meta.module_name))

My cms_apps.py

@apphook_pool.register
class ProductsApp(CMSConfigApp):
    app_config = Category
    app_name = 'products'
    name = _('Products')

    def get_urls(self, page=None, language=None, **kwargs):
        return ['myappname.app.products.urls']

My cms_plugins.py

@plugin_pool.register_plugin
class ProductsListPlugin(CMSPluginBase):
    model = ProductListCMSPlugin
    module = _("Products")
    name = _('Products list')
    render_template = "products/plugins/products_list_plugin.html"
    cache = False

    def render(self, context, instance, placeholder):
        context = super(ProductsListPlugin, self).render(context, instance, placeholder)
        category = instance.category
        paginated_items = instance.paginated_items

        try:
            products = Product.objects.filter(category=category, published=True).order_by('order')
        except category.DoesNotExist or category is None:
            products = Product.objects.filter(published=True).order_by('order')

        context.update({
            'products': products,
            'paginated_items': paginated_items
        })

        return context

DB screenshot for these AppHooked pages.

I hope it clarifies my setup.

fsbraun commented 4 months ago

Everything's all right - no worries!! OK, I now understand that you do not have a root URL defined for your apphook. I will need some time to take a look and try to reproduce.

aacimov commented 4 months ago

I thought it is better to write-out my code. It is a bit unusual but it gives a lot of flexibility, especially as some of my clients want to be able to change "almost" everything. Maybe someone will also find it helpful :D

Thanks a lot for your help.

aacimov commented 3 months ago

Hey @fsbraun I am still stuck with this, any progress? I saw on Slack you guys were busy but just pinging.

Tnx a lot.

fsbraun commented 3 months ago

Do you have a traceback to narrow it down?

aacimov commented 3 months ago

Not really. It just raises You do not have permission to add a plugin in a popup.

In my console i get: [13/Mar/2024 20:42:28] "GET /admin/cms/placeholder/add-plugin/?placeholder_id=143&plugin_type=ProductsListPlugin&cms_path=/admin/cms/placeholder/object/5/edit/84/&plugin_language=hr&plugin_position=1 HTTP/1.1" 403 28

I hope it helps.

fsbraun commented 3 months ago

What happens if you set CMS_PERMISSIONS = False in your settings?

aacimov commented 3 months ago

Same thing as before. Permission error. No changes whatsoever.

fsbraun commented 3 months ago

Let's try tracking this down a bit more... In a shell, can you do:

>>> from django.contrib.auth.models import User
>>> user = User.objects.get(username="<your superuser here>")
>>> from cms.models.placeholdermodel import Placeholder
>>> placeholder = Placeholder.objects.get(pk=143)
>>> placeholder.has_add_plugin_permission(user, "TextPlugin")
>>> placeholder.has_change_permission(user)

aacimov commented 3 months ago

Let's try tracking this down a bit more... In a shell, can you do:
from cms.models.placeholdermodels import Placeholder

This is actually from cms.models.placeholdermodel (without s, just in case someone stumbles upon)

placeholder = Placeholder.objects.get(pk=143) placeholder.has_add_plugin_permission(, "TextPlugin")

Here I get AttributeError: 'str' object has no attribute 'has_perm', I changed it to respect my plugin as well, both get same error.

placeholder.has_change_permission()

Didn't get to here.

fsbraun commented 3 months ago

Sorry, I was not precise enough. You need to give the user as a User object, not a string. I updated the comment above.

aacimov commented 3 months ago

Cool. True for both but not working :D

fsbraun commented 3 months ago

Is p.check_source(user) also True??!?

aacimov commented 3 months ago

No, it's False.

fsbraun commented 3 months ago

Sorry, for the obvious question: The language you try to change is a draft? (Background: the check_source method checks if the object containing placeholders is editable, i.e. a draft).

aacimov commented 3 months ago

Not sure what you are referring to, but yes, it is a new draft of the page. Language is only 1 (that damn Croatian again ;) , db collation is general tho, haha).

Just a guess - could be if the apphook is added to a page the method is not working as expected?

fsbraun commented 3 months ago

You haven't published it to make the apphook visible? The version menu says, "Version #xy (draft)"? Is there a published version "behind" the current draft? Do you have DJANGOCMS_VERISONING_LOCK_VERSIONS set to True in the settings?

aacimov commented 3 months ago

You haven't published it to make the apphook visible? The version menu says, "Version #xy (draft)"? Is there a published version "behind" the current draft? Do you have DJANGOCMS_VERISONING_LOCK_VERSIONS set to True in the settings?

I clicked on Publish but obviously it didn't fully publish the page. Now I saw the notice saying: Version cannot be published. It happened after my plugin said I don't have permission to add it.

Now I went back to the admin and publish it again (so that the blue dot gets green). After going back to the page -> New draft -> add plugin - same thing happens again (Permission error).

DJANGOCMS_VERSIONING_LOCK_VERSIONS is not used. I am using only DJANGOCMS_VERSIONING_ALLOW_DELETING_VERSIONS = True.

fsbraun commented 3 months ago

OK, I think we're getting closer. Can you add another check in the shell:

>>> placeholder.source.versions.first().state
>>> placeholder.source.page.pagecontent_set(manager="admin_manager").all()

Particularly, I am looking for PageContent objects with an id larger than 84 (or whatever the number is that appears in your path when trying to add the plugin - it may have changed since republishing).

Can you also provide the full path of the browser location when adding the plugin?

aacimov commented 3 months ago

OK, I think we're getting closer. Can you add another check in the shell:
>>> placeholder.source.versions.first().state
>>> placeholder.source.page.pagecontent_set(manager="admin_manager").all()
Particularly, I am looking for PageContent objects with an id larger than 84 (or whatever the number is that appears in your path when trying to add the plugin - it may have changed since republishing).

Can you also provide the full path of the browser location when adding the plugin?

http://localhost:8000/admin/cms/placeholder/object/5/edit/88/ is the path.

placeholder.source.versions.first().state is unpublished.

placeholder.source.page.pagecontent_set(manager="admin_manager").all() gives below:

fsbraun commented 3 months ago

Which is the failing path on the console? Contains 88 or 89? Can you add the plugin if you manually enter the path http://localhost:8000/admin/cms/placeholder/object/5/edit/89/? Working hypothesis: The apphook leads to redirection to the wrong edit endpoint.

aacimov commented 3 months ago

Which is the failing path on the console? Contains 88 or 89?

88 in the runserver console.

Would it be easier if I just pack my app and send it to you? Just the failing app so you can examine the configs. Maybe it could be helpful.

aacimov commented 3 months ago

Which is the failing path on the console? Contains 88 or 89? Can you add the plugin if you manually enter the path http://localhost:8000/admin/cms/placeholder/object/5/edit/89/? Working hypothesis: The apphook leads to redirection to the wrong edit endpoint.

Nope, permission error. But I just realized that when I click on the menu for adding the plugin it redirects back to 88 from 89.

fsbraun commented 3 months ago

Can you try http://localhost:8000/admin/cms/placeholder/object/5/preview/89/ and tell me if it also redirects to 88? What state does the 89 version show?

aacimov commented 3 months ago

Can you try http://localhost:8000/admin/cms/placeholder/object/5/preview/89/ and tell me if it also redirects to 88? What state does the 89 version show?

http://localhost:8000/admin/cms/placeholder/object/5/preview/89/ leads to a published version of the page, it doesn't redirect. It says Version 7 (Published) in the toolbar. In admin there is a blue dot, so something is definitely wrong.

aacimov commented 3 months ago

Are we talking about the home page of your project? If so, how does the behaviour change if you make some other page the home page?

Nope, it's not a homepage. I am attaching the .zip of the app. No sensitive info inside. Maybe it will help a lot. You will need to adjust the paths obviously.

products.zip

fsbraun commented 3 months ago

I'm still stabbing in the dark a bit, but this patch might help: pip install git+https://github.com/fsbraun/django-cms@fix/apphook-no-root. Would you be able to check out if it helps?

aacimov commented 3 months ago

I'm still stabbing in the dark a bit, but this patch might help: pip install git+https://github.com/fsbraun/django-cms@fix/apphook-no-root. Would you be able to check out if it helps?

Unfortunately it does not :-/

fsbraun commented 3 months ago

@aacimov By now, I have built a test apphook with no root URL and tested locally. It works as expected and I cannot reporduce the issue. I am confused what causes the redirect of the edit endpoint to a non-editable version. Can you locate where this redirect happens (in which view)?

aacimov commented 3 months ago

I got ill in the meantime so I will look into it in the next few days. Could you share you requirements.txt? Maybe it's some of the packages causing the issue. I am planning to drop my database and start over just to be sure. I can share my requirements.txt as well.

aacimov commented 3 months ago

@fsbraun just checked with the brand new DB, same issue. I am attaching my requirements.txt just in case so you can check if they match yours.

requirements.txt

EDIT: I think I found the issue - when I remove versioning everything works as expected! Is there a required order? I've put djangocms_versioning after the cms in my settings.py

fsbraun commented 3 months ago

@aacimov Order should not matter. I've moved this, since it seems to depend on versioning. I think you're redirected to a published/non-editable version of the PageContent. Can you find out what triggers the redirect?

I frequently run my test systems on the latest development branches. For the sake of testing it: Can you install the latest develop branches:

pip install git+https://github.com/django-cms/django-cms@develop-4
pip install git+https://github.com/django-cms/djangocms-versioning@master

aacimov commented 3 months ago

@fsbraun sorry for the delay. Installed latest, still the same permisson error while adding plugins (my own and other plugins, I hope you understood from my posts, no other can be added as well).

aacimov commented 3 months ago

@fsbraun I did not manage to find where the redirect happens so far. Since it is very much likely connected to djangocms-versioning could you point me in a direction perhaps? It would be helpful.

Tnx

fsbraun commented 3 months ago

@aacimov Can we schedule a zoom meeting or google meet to look at this together some time next week? PM me on Slack or Discord.

fsbraun commented 3 months ago

Will be fixed with django CMS 4.1.1

aacimov commented 2 months ago

Just had an extra issue regarding this issue saying (when trying to delete the whole record):

"Deleting post 'Test post' would result in the deletion of the associated objects, but you do not have privileges to delete the specified objects:

placeholder"

I am an admin of course.

fsbraun commented 2 months ago

Try doing this with the english admin. Should be fixed in 4.1.1

aacimov commented 2 months ago

Try doing this with the english admin. Should be fixed in 4.1.1

Just tried, same error.

fsbraun commented 2 months ago

Would you be able to test this with the latest build of django CMS (going to be 4.1.1): git+https://github.com/django-cms/django-cms@release/build?

aacimov commented 2 months ago

Installed - still the same unfortunately.

fsbraun commented 2 months ago

Hm. I fear that's not versioning-related. What happens if you remove these lines: https://github.com/django-cms/django-cms/blob/4e7f24f2847846b3770e086068ff729961b2c450/cms/admin/placeholderadmin.py#L200-L202

fsbraun commented 2 months ago

The more I look into the code base the more I think this is a bug. Can you raise it in the django-cms repo?

django-cms / djangocms-versioning