Fix front end XTD menus and contacts buttons #12321

dgrammatiko · 2016-10-06T12:14:25Z

Pull Request for Issue #12309

Summary of Changes

Testing Instructions

Install latests 3.7.x branch https://github.com/joomla/joomla-cms/archive/3.7.x.zip
Enable all editor xtd plugins
Go to frontend, edit an article or something and try to add a menu link and a contact link

Without patch you get an error

Apply this PR and retry, you should be able to select a menu and a contact respectively

EDIT: PLEASE TRY inserting an article as well!!!

Documentation Changes Required

None

@infograf768 check this one

infograf768 · 2016-10-06T12:50:15Z

👍
I will now close mine.

infograf768 · 2016-10-06T12:50:59Z

I have tested this item ✅ successfully on f552092

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12321.}

izharaazmi · 2016-10-06T12:55:05Z

components/com_contact/controller.php

+	{
+		$this->input = JFactory::getApplication()->input;
+
+		// Article frontpage Editor contact proxying:


Why do we need ===?

IMHO is a good practice to perform strict comparations where possible

zero-24 · 2016-10-06T13:29:37Z

components/com_menus/controller.php

+/**
+ * Modules manager master display controller.
+ *
+ * @since  3.5


Since tag please ;)

infograf768 · 2016-10-06T13:31:53Z

and also change modules to menus in the comments ;)

andrepereiradasilva · 2016-10-06T13:15:44Z

administrator/components/com_contact/views/contacts/tmpl/modal.php

+if ($app->isSite())
+{
+	JSession::checkToken('get') or die(JText::_('JINVALID_TOKEN'));
+}


shouldn't this token check be at the begining of the file, like https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/views/articles/tmpl/modal.php#L14 ?

You're right!

dgrammatiko · 2016-10-06T15:48:46Z

hmmm is also articles xtd button broken?

Fixed

infograf768 · 2016-10-07T06:42:03Z

I have tested this item ✅ successfully on

All looks correct now.

Found an issue concerning modals in general in 3.7.0. Creating Issue.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12321.}

infograf768 · 2016-10-08T09:23:48Z

IMPORTANT!

To use the xtd menu one has to be at least administrator.
A Manager (and below) trying to use it gets in admin WITH the menus in the modal...
For it to work the Manager has to have Access administration Interface for menus

same for xtd modules

And in frontend an sql error:

This issue is also related to the discussions:
#12338
and #10653

The reason why an Author (and above) can use the xtd contacts is that it has Create and Edit Own permissions.

As for xtd modules, it has Create permissions and CAN use it in frontend BUT a Manager canNOT in back-end if no Access to Administration Interface...

I guess we got deep here into quite a few permissions issue concerning these xtd.

Hackwar · 2016-10-13T17:52:24Z

components/com_contact/router.php

 	 */
 	public function __construct($app = null, $menu = null)
 	{
+		if ($app->input->get('view') === 'contacts' && $app->input->get('layout') === 'modal')


Why are you doing this? This would disable the router for this whole page.

I knew that this was not the right solution, that's why I ping you here!

Hackwar · 2016-10-13T17:52:46Z

components/com_content/router.php

 	 */
 	public function __construct($app = null, $menu = null)
 	{
+		if ($app->input->get('view') === 'articles' && $app->input->get('layout') === 'modal')


Same as above. This would break a lot of things.

infograf768 · 2016-10-14T07:22:55Z

What about using instead

public function __construct($app = null, $menu = null)
    {
        $params = JComponentHelper::getParams('com_content');
        $this->noIDs = (bool) $params->get('sef_ids');
        $categories = new JComponentRouterViewconfiguration('categories');
        $categories->setKey('id');
        $this->registerView($categories);
        $category = new JComponentRouterViewconfiguration('category');
        $category->setKey('id')->setParent($categories, 'catid')->setNestable()->addLayout('blog');
        $this->registerView($category);
        $article = new JComponentRouterViewconfiguration('article');
        $article->setKey('id')->setParent($category, 'catid');
        $articles = new JComponentRouterViewconfiguration('articles'); // added
        $this->registerView($article);
        $this->registerView($articles);  //added
        $this->registerView(new JComponentRouterViewconfiguration('archive'));
        $this->registerView(new JComponentRouterViewconfiguration('featured'));
        $this->registerView(new JComponentRouterViewconfiguration('form'));

Would that also break a lot of things?

infograf768 · 2016-10-14T07:28:50Z

Same for contacts.
@Hackwar ?

infograf768 · 2016-10-14T09:40:06Z

@DGT41
Concerning the back-end "you are not authorised for modules", I have tested modifying
/administrator/components/com_modules/modules.php
to

<?php
/**
 * @package     Joomla.Administrator
 * @subpackage  com_modules
 *
 * @copyright   Copyright (C) 2005 - 2016 Open Source Matters, Inc. All rights reserved.
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

defined('_JEXEC') or die;
JHtml::_('behavior.tabstate');

$user = JFactory::getUser();
$input = JFactory::getApplication()->input;

if (($input->get('layout') !== 'modal' && $input->get('view') !== 'modules')
    && !$user->authorise('core.manage', 'com_modules'))
{
    throw new JAccessExceptionNotallowed(JText::_('JERROR_ALERTNOAUTHOR'), 403);
}

$controller = JControllerLegacy::getInstance('Modules');
$controller->execute($input->get('task'));
$controller->redirect();

and it now works here.

A similar patch works for menu too but I get the same sql error as in frontend (see above):

<?php
/**
 * @package     Joomla.Administrator
 * @subpackage  com_menus
 *
 * @copyright   Copyright (C) 2005 - 2016 Open Source Matters, Inc. All rights reserved.
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

defined('_JEXEC') or die;

$input = JFactory::getApplication()->input;
$user = JFactory::getUser();

if (($input->get('layout') !== 'modal' && $input->get('view') !== 'items')
    && !$user->authorise('core.manage', 'com_menus'))
{
    throw new JAccessExceptionNotallowed(JText::_('JERROR_ALERTNOAUTHOR'), 403);
}

$controller = JControllerLegacy::getInstance('Menus');
$controller->execute($input->get('task'));
$controller->redirect();

What do you think?

dgrammatiko · 2016-10-14T10:38:28Z

@infograf768 can you create a pr with those changes?

infograf768 · 2016-10-14T14:27:33Z

@DGT41
We still have to solve this weird sql error for menu

dgrammatiko · 2016-10-14T14:28:35Z

@infograf768 shouldn't we keep the if statement for performance? Now it's gonna execute the code in all views...
Will take a look at that later on tonight, doing something else right now

@Hackwar can you confirm that @infograf768 solution is the appropriate here?

infograf768 · 2016-10-14T14:44:11Z

FYI, when no assoc, similar sql error:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') AND a.access IN (1,1,2,3) ORDER BY a.lft asc LIMIT 20' at line 9 SQL=SELECT `a`.`id`,`a`.`menutype`,`a`.`title`,`a`.`alias`,`a`.`note`,`a`.`path`,`a`.`link`,`a`.`type`,`a`.`parent_id`,`a`.`level`,`a`.`published` AS `a.published`,`a`.`component_id`,`a`.`checked_out`,`a`.`checked_out_time`,`a`.`browserNav`,`a`.`access`,`a`.`img`,`a`.`template_style_id`,`a`.`params`,`a`.`lft`,`a`.`rgt`,`a`.`home`,`a`.`language`,`a`.`client_id`,CASE WHEN a.type = 'component' THEN a.published+2(e.enabled-1) WHEN a.type = 'url'AND a.published != -2 THEN a.published+2 WHEN a.type = 'url'AND a.published = -2 THEN a.published-1 WHEN a.type = 'alias'AND a.published != -2 THEN a.published+4 WHEN a.type = 'alias'AND a.published = -2 THEN a.published-1 WHEN a.type = 'separator'AND a.published != -2 THEN a.published+6 WHEN a.type = 'separator'AND a.published = -2 THEN a.published-1 WHEN a.type = 'heading'AND a.published != -2 THEN a.published+8 WHEN a.type = 'heading'AND a.published = -2 THEN a.published-1 END AS published ,l.title AS language_title, l.image AS language_image,u.name AS editor,c.element AS componentname,ag.title AS access_level,`mt`.`title` AS `menutype_title`,e.name AS name FROM `#menu` AS a LEFT JOIN `#languages` AS l ON l.lang_code = a.language LEFT JOIN `#__users` AS u ON u.id = a.checked_out LEFT JOIN `#__extensions` AS c ON c.extension_id = a.component_id LEFT JOIN #__viewlevels AS ag ON ag.id = a.access LEFT JOIN `#__menu_types` AS `mt` ON `mt`.`menutype` = `a`.`menutype` LEFT JOIN #__extensions AS e ON e.extension_id = a.component_id WHERE a.id > 1 AND a.client_id = 0 AND (a.published IN (0, 1)) AND a.menutype IN() AND a.access IN (1,1,2,3) ORDER BY a.lft asc LIMIT 20 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') AND a.access IN (1,1,2,3)' at line 9 SQL=SELECT COUNT() FROM `#menu` AS a LEFT JOIN `#languages` AS l ON l.lang_code = a.language LEFT JOIN `#__users` AS u ON u.id = a.checked_out LEFT JOIN `#__extensions` AS c ON c.extension_id = a.component_id LEFT JOIN #__viewlevels AS ag ON ag.id = a.access LEFT JOIN `#__menu_types` AS `mt` ON `mt`.`menutype` = `a`.`menutype` LEFT JOIN #__extensions AS e ON e.extension_id = a.component_id WHERE a.id > 1 AND a.client_id = 0 AND (a.published IN (0, 1)) AND a.menutype IN() AND a.access IN (1,1,2,3)

dgrammatiko · 2016-10-14T15:07:52Z

@infograf768 changing line 353 from
$query->where('a.menutype IN(' . implode(',', $types) . ')');
to

            if (is_array($types) && !empty($types))
            {
                $query->where('a.menutype IN(' . implode(',', $types) . ')');
            }

solves the sql problem for me...

infograf768 · 2016-10-14T15:16:19Z

@DGT41
It does too! Great!
Manager can now see the items in the modal and author can too in frontend.

We just have to wait for @Hackwar to reply.

dgrammatiko · 2016-10-14T15:18:00Z

@infograf768 nope try the filters...

infograf768 · 2016-10-14T15:20:54Z

When using the filters I indeed do get
"The most recent request was denied because it contained an invalid security token. Please refresh the page and try again."

infograf768 · 2016-10-16T07:10:28Z

Evidently, I do not agree with

if ($input->get('view') === 'contacts' && $input->get('layout') === 'modal')
{
    if (!JFactory::getUser()->authorise('core.edit', 'com_contact'))
    {
        JFactory::getApplication()->enqueueMessage(JText::_('JERROR_ALERTNOAUTHOR'), 'warning');
        return;
    }
}

as this limits the access to editors, does not let them edit.own and prevents authors to use the xtd contact.
See #12353

As I commented there, we can change in the xtd as well as inROOT/components/com_contact/contact.php —and its equivalent for content.php— this code to let authors access.

Hackwar · 2016-10-16T07:19:26Z

I also replied to your comment. Yes, this might limit this feature to just editors. But the solution to that problem is not to remove the check altogether, but to properly extend it.

Hackwar · 2016-10-16T07:28:25Z

I just checked this again with an editor user and just my changes from #12420 and it works perfectly fine without the changes to the router. The changes to the router are neither necessary, nor are they right. Please don't merge those changes.

infograf768 · 2016-10-16T10:26:09Z

I just checked this again with an editor user and just my changes from #12420 and it works perfectly fine without the changes to the router.

I do not confirm this. We NEED the changes in the router of we get a Notice: Undefined index: contacts or Notice: Undefined index: articles as @DGT41 got above.

mbabker · 2016-10-16T15:02:17Z

as this limits the access to editors, does not let them edit.own and prevents authors to use the xtd contact.
See #12353

The entire logic we use for these ACL checks where we're proxying a frontend request to the backend is fatally flawed as the permissions checks are at the component level of ACL. These ACL checks have to get smarter for correct processing (i.e. it needs to be possible to check at the category level).

Hackwar · 2016-10-16T15:21:52Z

there was indeed a notice that needed to be fixed. This is done in #12434 That still means that the changes to the router are not correct.

infograf768 · 2016-10-16T15:44:16Z

@mbabker
I posted a new PR for staging with more access checks
#12435
I know it is not enough and should be more complex (categories and such), but together with #12353 (comment) it now lets authors use the xtds which they could not before.
We have to take into account the 3.6.3 release which will come soon.

infograf768 · 2016-10-16T16:20:32Z

@DGT41
I confirm that #12434 solves the issue and we do not need the changes anymore in the router.
Please add the lang file loading
#12321 (comment)

and change

if (!JFactory::getUser()->authorise('core.edit', 'com_contact'))

to

if (!JFactory::getUser()->authorise('core.create', 'com_contact'))

and we at last should be Ok... 😺

infograf768 · 2016-10-17T06:19:51Z

Also, we need an access check for menu.

if ($app->input->get('view') === 'items' && $app->input->get('layout') === 'modal')
{
    if (!JFactory::getUser()->authorise('core.create', 'com_menus'))
    {
        $app->enqueueMessage(JText::_('JERROR_ALERTNOAUTHOR'), 'warning');
        return;
    }
}

infograf768 · 2016-10-19T10:30:42Z

Works for editors up. We need the staging part for the xtd editors and ROOT/components/com_content/content.php which have been lately updated.

infograf768 · 2016-11-04T07:11:09Z

I have tested this item ✅ successfully on d09c3d8

One more tester

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12321.}

RickR2H · 2016-11-04T10:25:02Z

I have tested this item ✅ successfully on d09c3d8

After the patch the links worked! Thanks for the PR @DGT41

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12321.}

brianteeman · 2016-11-04T11:09:10Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12321.}

* menus * cs * xtd-contacts * session check, correct version * articles * cs identation fix * Update modules.php solving issue for Manager enable to use xtd module in back-end * Update router.php Solving router issue and let xtd article work fine * Update router.php Solving contact router issue * Filters and menu sql fixes * unification * implode empty array * fixes * revert router, add lang, add/change ACL * Update router.php * Update router.php

joomla-cms-bot added the PR-3.7.x label Oct 6, 2016

dgrammatiko changed the title ~~Fix front end XTD menus button~~ Fix front end XTD menus and contacts buttons Oct 6, 2016

infograf768 mentioned this pull request Oct 6, 2016

Release blocker 3.7.x. New xtd editors broken in frontend. WIP!! #12320

Closed

izharaazmi reviewed Oct 6, 2016

View reviewed changes

zero-24 added this to the Joomla 3.7.0 milestone Oct 6, 2016

zero-24 added the Release Blocker label Oct 6, 2016

zero-24 reviewed Oct 6, 2016

View reviewed changes

andrepereiradasilva reviewed Oct 6, 2016

View reviewed changes

infograf768 mentioned this pull request Oct 7, 2016

370 Regression: Modals do not close #12335

Closed

infograf768 mentioned this pull request Oct 8, 2016

"page break" button in frontend can't be used by an "Author" #12338

Closed

Hackwar reviewed Oct 13, 2016

View reviewed changes

menus

c01d9ed

infograf768 mentioned this pull request Oct 16, 2016

3.6.3: Allow author to use xtd-editors #12353

Merged

Hackwar mentioned this pull request Oct 16, 2016

Routing: Fixing notice for non-existing views #12434

Merged

dgrammatiko and others added 3 commits October 18, 2016 17:13

revert router, add lang, add/change ACL

a99742b

Update router.php

73cc68c

Update router.php

d09c3d8

rdeutz changed the base branch from 3.7.x to staging October 25, 2016 17:54

Bakual added PR-staging and removed PR-3.7.x labels Oct 26, 2016

infograf768 mentioned this pull request Nov 4, 2016

New xtd-editors menu and contact display buttons only if correct permissions #12746

Closed

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 4, 2016

brianteeman removed the Release Blocker label Nov 4, 2016

rdeutz merged commit bcdc414 into joomla:staging Nov 4, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 4, 2016

dgrammatiko mentioned this pull request Nov 4, 2016

Fix error notice on front-end search #12750

Merged

dgrammatiko deleted the 3.7.z branch March 18, 2017 07:36

infograf768 mentioned this pull request Jul 8, 2019

Load global admin language file in Editors-XTD modals #25472

Closed

Uh oh!

Uh oh!

Fix front end XTD menus and contacts buttons #12321

Fix front end XTD menus and contacts buttons #12321

Uh oh!

Conversation

dgrammatiko commented Oct 6, 2016 • edited by joomla-cms-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

Documentation Changes Required

Uh oh!

infograf768 commented Oct 6, 2016

Uh oh!

infograf768 commented Oct 6, 2016

Uh oh!

izharaazmi Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

andrepereiradasilva Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

zero-24 Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Oct 6, 2016

Uh oh!

andrepereiradasilva Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgrammatiko Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

dgrammatiko commented Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infograf768 commented Oct 7, 2016

Uh oh!

infograf768 commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hackwar Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

dgrammatiko Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

Hackwar Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgrammatiko commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

dgrammatiko commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

dgrammatiko commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

dgrammatiko commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 14, 2016

Uh oh!

infograf768 commented Oct 16, 2016

Uh oh!

Hackwar commented Oct 16, 2016

Uh oh!

Hackwar commented Oct 16, 2016

Uh oh!

dgrammatiko commented Oct 6, 2016 •

edited by joomla-cms-bot

Loading

andrepereiradasilva Oct 6, 2016 •

edited

Loading

dgrammatiko commented Oct 6, 2016 •

edited

Loading

infograf768 commented Oct 8, 2016 •

edited

Loading

infograf768 commented Oct 14, 2016 •

edited

Loading