👍
I will now close mine.

I have tested this item ✅ successfully on f552092

and also change modules to menus in the comments ;)

hmmm is also articles xtd button broken?

Fixed

I have tested this item ✅ successfully on

All looks correct now.

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

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.

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?

Same for contacts.
@Hackwar ?

@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?

@infograf768 can you create a pr with those changes?

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

@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?

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) 

@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...

@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.

@infograf768 nope try the filters...

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."

Looks fine now. Will test again tomorrow morning.

@infograf768 there is another potential problem at lines 378-379:

            $groups = implode(',', $user->getAuthorisedViewLevels());
            $query->where('a.access IN (' . $groups . ')');

should be:

            $groups = $user->getAuthorisedViewLevels();

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

@dgt41
Which potential problem?
This code is all over core, in each model and even in some plugins.

        if (!$user->authorise('core.admin'))
        {
            $groups = implode(',', $user->getAuthorisedViewLevels());
            $query->where('a.access IN (' . $groups . ')');
        }

I mean that if the user is not in any group, he will not be able to do anything, not even login.
I tested and got
Warning
You can't access the private section of this site.

If really useful, this could be the subject of a new PR modifying this code everywhere in core at the same time.

@infograf768 in my comment above do the following:
change:

            $groups = implode(',', $user->getAuthorisedViewLevels());
            $query->where('a.access IN (' . $groups . ')');

to:

            $groups = implode(',', $user->getAuthorisedViewLevels());
$groups= array();
            $query->where('a.access IN (' . $groups . ')');

and get an sql error. The $user->getAuthorisedViewLevels() COULD return an empty array.
Basically what I am saying is that before executing an implode inside a query we should make sure that the array is not empty! This check needs to be added in a lot of places (do a search for implode(',', `

I've looked into this a bit further and on my end, the feature for com_content/articles works. Looking at the code that you are modifying here and what is being used in com_content, there are several differences there. The bug that you are seeing has nothing to do with the routing.

Basically what I am saying is that before executing an implode inside a query we should make sure that the array is not empty! This check needs to be added in a lot of places (do a search for implode(',', `

@dgt41 Indeed, there are multiple places in core where this can be corrected. No pb if you want to do it for what concerns this PR. A new PR is the best solution for other instances in core imho.

@Hackwar
Then as there is no more issue with the routers, I consider that part OK. As the PR works fine for all xtd now, we would just wait for @dgt41 to take care of the possible empty array.

@dgt41 Please have a look at my PR #12420 regarding the contact component. That PR uses identical code to com_content and its article feature. As I wrote, the router has nothing to do with these issues and the changes that you did should be reverted. I've not reviewed the other changes, but I'm wondering what you were doing with the modules button, since that works on my end without any changes.

In general, we should try to keep the code consistent. We already have working code in com_content and looking at that code, it looks fine to me. So I would expect the other buttons to work in the same manner.

@Hackwar done the changes from #12420. Still without the changes in the router.php front-end gets error, so...

I can not confirm that. I've got the changes from the contact component in my branch and without the routing changes and it works just fine. The changes that you did to the router are not correct, since there is no contacts view in the frontend.

@dgt41 @Hackwar
It seems you forgot that we are waiting for a merge from staging into 3.7
See #12353 which was already merged in staging where I do take off that check from /components/com_content/content.php and do the access checks (for authors and not only editors) in the plugins themselves.
I was waiting for that merge to do a similar PR for xtd contact and xtd menu. So, adding the check in contact here is not the solution for me.

@Hackwar

I can not confirm that. I've got the changes from the contact component in my branch and without the routing changes and it works just fine. The changes that you did to the router are not correct, since there is no contacts view in the frontend.

Please log in with an editor or a manager, and NOT as a superuser...

@infograf768 The change from #12353 PR actually introduced a security issue, as I wrote in the PR. This can NOT be released like that.

Regarding testing this with a non-Superuser account: Will do.

@infograf768 The change from #12353 PR actually introduced a security issue, as I wrote in the PR. This can NOT be released like that.

I have asked jsst and directly some maintainers about that. Did not get negative replies.
I replied to your comment. If there is a reason why an author would not be able to see what an editor can see, then we can modify that PR.

Now, concerning this PR, (as the routers were updated to include the articles and contacts views) all looks OK here, EXCEPT that we need to load the back-end contact lang ini file in frontend.

This can be solved by adding
JFactory::getLanguage()->load('com_contact', JPATH_ADMINISTRATOR);
in
ROOT/components/com_contact/contact.php

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.

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.

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.

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.

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).

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.

@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.

@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... 😺

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;
    }
}

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.

I have tested this item ✅ successfully on d09c3d8

One more tester

I have tested this item ✅ successfully on d09c3d8

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

RTC