Fix divio/django-cms#2166 - disabling further action on populate calls i...

[benjaoming]

disabling further action on populate calls in case the user is not authenticated

[benjaoming]

This references #2166

[coveralls]

Coverage decreased (-0.2%) to 86.939% when pulling 48ba796 on benjaoming:toolbar_populate_ignore_anon into 33a4ad4 on divio:develop.

[yakky]

LGTM: a safe assumption that toolbar sholt not populated for anonymous users
Shippable build fails, but Travis is ok

[benjaoming]

Sorry that I didn't get back to this with a comment I was thinking about:

When the toolbar is displayed by accident during a session in which the user was logged out, it actually shows with a login menu.

Either we need to:

1. Remove that form from the toolbar template (See: https://github.com/divio/django-cms/blob/develop/cms/templates/cms/toolbar/toolbar.html#L15 )
   or
2. Somehow rollback this patch and figure out a way to allow the toolbar to be displayed for unauthenticated users.

I suggest 1) because it's not the toolbar's responsibility to show login forms.

[yakky]

Ok, I'll revert this PR so we can discuss the frontend solution
At this stage, it's actually toolbar responsibility to show the login for as it's the quickest way for an editor to enter.

[benjaoming]

But when do you display a toolbar to a non-authenticated user? I don't see the use case for that :)

[yakky]

That's the issue to solve: toolbar should only appear when requested by the user (appending "?edit" or TOOLBAR_URL__EDIT_ON parameter)

[benjaoming]

How about replacing the toolbar login form with a redirect to LOGIN_URL?next=request.path?edit=1 + messages.info("You need to sign in to edit") ?

[benjaoming]

(the redirect can be put in the toolbar middleware)

[yakky]

Project may not have a public login view, but only a login admin

[mmarzantowicz]

@benjaoming, I'm not sure I fully understand your proposal but messages.info("You need to sign in to edit") suggests that you're showing this message to someone who should never see it. There are (or there should be) only two kind of visitors/users: authenticated and not authenticated. Users from second group should never see anything related to service maintenance (such as cms toolbar).

[benjaoming]

@yakky in that case, user can simply set LOGIN_URL="/admin/"

@mmarzantowicz The point was that if ?edit=1 is present in the URL params then there's a point of showing a message that login is needed... but I don't really care, I don't think any of this is the responsibility of the toolbar. Login pages and message displaying is handled elsewhere.

And users that don't know that they need to login to edit CMS pages are useless editors IMO :)

[yakky]

@benjaoming could you provide a minimal and clean setup to reproduce the original issue? Ideally instructions on how to configure a project started with djangocms-installer to trigger the issue.

[Darkless012]

To reproduce the issue:

• install django-cms using djangocms-installer

• edit basic urls.py and add following:

  from django.contrib.auth import views as auth_views

  url(r'^logout/$', auth_views.logout, {'template_name': 'registration/logout.html'},
  name='auth_logout'),

• login to django cms

• visit URL: localhost:8000/en/logout/ (or subsitute en for language stated in installer)

[Darkless012]

I came out with a possible solution and hack that works.

(possible) Solution:

I did try/catch block around whole content "render_tag" in CMSToolbar class.
And return empty string if there is any error. So it will finishes page rendering.

I know that this way it throws out all errors, but that errors could only raise in this template tag.
Also in templatetags reference it is stated that it should return at least something. (or in case of template filters unmodified value)

Hack:

But that is definitely NOT a solution!
I was able to exclude toolbar on logout site by creating block that includes template which constructs toolbar. YUCK!

base.html
{% block cmstoolbar %}{% include "cms_toolbar.html" %}{% endblock %}

cms_toolbar.html
{% load cms_tags %}
{% cms_toolbar %}

the logout template
{% block cmstoolbar %}{% endblock %}

[Darkless012]

Sorry for excessive comments, just including "custom" toolbar that doesn't suffer from this bug.

http://pastebin.com/YYZPQnYu