Displaying an Alert to create a Login Form when saving a Home menu item

[infograf768]

Pull Request for Issue #15730

Summary of Changes

If Home page(s) (in monolingual as well as multilingual sites) needs login to be displayed, it is necessary to also create and publish a Login Form menu item to get an Itemid.
As the former attempts to solve this via code have failed while keeping an Itemid and 3.8.4 router changes are likely to be reverted ( #19512 ), this will remind sites administrator to do so.
It is impossible to limit the display in function of the Access (at least I don't know how to) as users may have modified the default Accesses.

Testing Instructions

Patch and save a Home menu item.

After patch

[csthomas]

Maybe it would be good to display such warning only if there is no published menu item for index.php?option=com_users&view=login or/and if access is not public.

[ggppdk]

Maybe it would be good to display such warning only if there is no published menu item for index.php?option=com_users&view=login or/and if access is not public.

arguably, such a menu item for login can exist,
and then it can be unpublished deleted,
in that case this warning will not be seen

but always showing this warning, for the -rare- case that the home page is non-public access is a kind of annoying, so it is best to show it only if needed

[brianteeman]

I do not agree with this being added to the core. It is a very specific use case that will not apply to the majority of the web sites out there. If we are to start adding messages for every possibility then it will get crazy. The correct place for this information is in the documentation

[infograf768]

if access is not public

We can't base ourselves on the Accesses as these may have been totally modified.

but always showing this warning, for the -rare- case that the home page is non-public access is a kind of annoying

How many times does a site admin modifies home pages?

I do not agree with this being added to the core.

As usual, all that can help users not making mistakes, specially for non-English speaking people as Docs is barely translated, creates issues for you. No comments.

[ggppdk]

it is possible to make a simple check if the home page menu item has currently non-public access and only then display it !
thus we can avoid annoying the users

it is a very good trade-off , and that is the way things should work, something bad is detected , provide some immediate valueable feedback, instead of "expecting users to read documentation",
users will just see it as a bug

plus people may change access just by mistake, i have seen it happen , and i have done it once or twice myself

[brianteeman]

@ggppdk that would be much better than this PR

[rdeutz]

If we can detect a misconfiguration of the site clearly and if we can give the user proper advice how to fix it, then we should do it.

[brianteeman]

@rdeutz I agree with that. I just dont agree with this PR in its current form which always displays it

[infograf768]

@csthomas @ggppdk
Modified check to also include Access. Modified lang string to fit.
If some users have set their site far away from default accesses, it will indeed be their problem.

@brianteeman Appreciate you have at last figured out we do not need sending back people to the docs for everything.

[mbabker]

If we can absolutely guarantee that the public access level will always be "1" then IMO this is fine, otherwise we need to come up with a way to account for users who can and do customize viewing access levels and user group configurations.

[brianteeman]

i cant see how that guarantee can be given

[infograf768]

If we can absolutely guarantee that the public access level will always be "1" then IMO this is fine, otherwise we need to come up with a way to account for users who can and do customize viewing access levels and user group configurations.

This is why my original PR did not include access check as stated before.

[mbabker]

And likewise Brian has a valid point about not always showing the notice when saving a home item. I don't know what the right answer is for this but we can't just blindly always show the notice and we can't blindly make an assumption about a part of the system that can be configured by users.

[brianteeman]

Reading through all the related issues and I haven't yet found out what and why broke the previous behaviour whereby the login menu was now required. Surely that is the cause of the problem and it is there we should look for a solution.

[ggppdk]

Good point, but i think it is possible

we get the guest access level(s)
and then check that home page access is one of these access level(s)

// Check if current menu item is a home page, and it has an access level that is granted to guests
if ($data['home'] && $data['published'])
{
  $guest_accessible = in_array($data['access'], JAccess::getAuthorisedViewLevels(0));
  if (!$guest_accessible)
  {
    JFactory::getApplication()->enqueueMessage(JText::_('COM_MENUS_CREATE_LOGIN_FORM'),'warning');
  }
}

[Ooops-404]

As a user that likes to be spoofed, I would hate for Joomla to bother me with too many assumptions, like assuming I want visitors to login.

Maybe I missunderstood this.?

[csthomas]

If we can absolutely guarantee that the public access level will always be "1"

Maybe we can start from this

joomla-cms/libraries/src/Access/Access.php

Line 1070 in e9af548

$authorised = array(1);

[ggppdk]

@Ooops-404

it is about the 99%+of websites
for which websites (in the rare case that this happens)

• it will be a misconfiguration
• 99%+ of users will think it is a bug

so if the message is not relevant to some websites, then ok, they know what they are doing,
and they can ignore it

@csthomas i think the code that i posted works properly

so we do not need care about "Public" access level at all
in fact it is wrong to care "Public" access level
because message should be if guests can access home page menu item
and if not then they need to be able to access a login menu item

[ggppdk]

So if public access level is not relevant,
the message needs to change too, right

Something like this

Your home page menu item now has an access level that is not granted to non-logged users.
Please make sure that a login menu item has been created that is viewable by guests

[brianteeman]

I guess you folks really do like wasting your time.
This used to work without a problem. find out what changed that and staart from there. stop looking for bigger band aids

[csthomas]

@ggppdk I did not see your comment, when I was adding mine.

[ggppdk]

This used to work without a problem

i think yes, i think it used to be an issue, and then it was fixed at some point in the past
(not sure / don't remember)

and then it stopped working again in J3.8.x ?
is it still an issue after reverting router changes in J3.8.4 ?

About the code i proposed above, it can be useful in other cases,
it is proper way to check something is accessible by guest
we do not need to worry if "public" access level was configured differently in some websites

[ggppdk]

is it still an issue after reverting router changes in J3.8.4 ?

no the redirection loop , is still happening with J3.8.4 router changes reverted\

if there is no menu item (of any type, registration, profile, login, etc) for 'com_users' that is published and accessible by guest user then home page menu item is selected (which is not accessible) and you get a redirection loop

was this ever "working" without a menu item for of 'com_users' ?
i think not

• just indeed there is a solution that adding message of this PR

the login URL needs to be created without any menu Itemid if the home page menu is not accessible

[brianteeman]

Read the original issue - this started at J3.7

[infograf768]

I tested 3.6.5 and the difference is that no Itemid is added to the link when a login form is not present.
I may have a solution that would use an Itemid only when we have a login form.
In that case, no need for this Alert.

diff --git a/libraries/src/Application/SiteApplication.php b/libraries/src/Application/SiteApplication.php
index d679010..820437c 100644
--- a/libraries/src/Application/SiteApplication.php
+++ b/libraries/src/Application/SiteApplication.php
@@ -91,4 +91,14 @@
 
 				$url = \JRoute::_('index.php?option=com_users&view=login', false);
+				$menuId  = explode('Itemid=', $url);
+
+				// Get the home page menu item
+				$home_item = $menus->getDefault($this->getLanguage()->getTag());
+
+				// Do not use an Itemid if we are on the home page and need login
+				if (!empty($menuId[1]) && $home_item->id == $menuId[1])
+				{
+					$url = 'index.php?option=com_users&view=login';
+				}
 
 				$this->enqueueMessage(\JText::_('JGLOBAL_YOU_MUST_LOGIN_FIRST'));

What do you think?

[ggppdk]

Yes your code will work, and will prevent the redirect loop

just, it is better to also typecast $menuId[1] to integer
(although the typecast is not really needed because &Itemid= is at the end of the URL, but just to be safe)

if (!empty($menuId[1]) && $home_item->id == (int) $menuId[1])

And actually the fix is correct to be placed there
since you are preventing redirect loop for guest users
... and a few lines below there is similar check for home page menu item for logged users !
which prevents a redirect loop for logged users

[infograf768]

I will propose the PR a bit later.
As we will need $home_item = $menus->getDefault($this->getLanguage()->getTag()); twice, I will move it to the top of the method and add (int) to $menuId[1] although indeed it is not really necessary.

[infograf768]

Please test #19559

Closing this as we have a better patch.

[csthomas]

IMO this PR with @ggppdk changes #19535 (comment) would be enough for J3.x

[infograf768]

I do not think so as it includes the guest access in the array and, for sure, i would never give the guest access to a home page.

[ggppdk]

I do not think so as it includes the guest access in the array and, for sure, i would never give the guest access to a home page.

it is not about 'public' or about 'guest' access level
let me explain,
the code checks the access level has been granted to unlogged users

• (by default installation) it can include 'public' access level
• 'Guest' level
• then it can also include view access level 'AAAA'
• then it can also include view access level 'BBBB'

So ONLY if the home page menu item has any of the access levels

• 'public'
• 'guest level' (ok you would usually as you said not assign this level to home)
• 'view access level AAA'
• 'view access level BBB'

only then your unlogged visitor will be able to see the home page without being redirected to login screen,

• any other access level will cause a redirection to login screen

So in fact checking if home page has access level public, was wrong for 2 reasons

• public access level can have been changed / removed as mbabker said
• but also 2nd reason there are more levels to check, that is all levels granted to guest (unlogged user)

still this PR can be useful but with adding a different warning

• since we will not have a redirect loop, thanks to your other PR

the message of this PR can now become
"Home page has an access level that is not granted to unlogged visitors, you visitors will need to login before they can view the home page"

[infograf768]

"Home page has an access level that is not granted to unlogged visitors, you visitors will need to login before they can view the home page"

would in this case rather be I guess :

"This home page has an access level that is not granted to unlogged visitors, your visitors will need to login before they can view the page. A Login Form menu item should be created and published. It may be hidden if desired."

Before modifying this, I need to know if it would be accepted.
@mbabker @Bakual ?

[infograf768]

I found a way to keep JRoute in #19559

Please let me know what you think

[infograf768]

I closed #19559 as it did not work indeed.
Reopened this one and waiting for @mbabker and @Bakual to decide if I should modify as stated above.

[Hackwar]

Hello @infograf768, thank you for your contribution. We are really sorry that it took so long to respond to you. We've discussed this PR extensively in the CMS maintainer team and decided on closing it for now. We agree that a warning would be good if there is no login form and the homepage is non-public, but that warning should only show up when that actually is the case. Since access levels can be changed, simply checking for the ID 1 is not enough.
We also decided, that this would constitute a new feature (yes, that is debatable, but that was what we agreed upon in the end) and that we would rather see this added to Joomla 4.2 than to the 3.10 branch. It isn't a bug right now that breaks anything, but a UX improvement and that would rather be a new feature than a bug fix and thus should not be merged into 3.10 anymore.

We would be very happy if you could open a new PR against 4.2 with the proper checks and we will try our best to process that as soon as possible. It definitely shouldn't have to wait another 4 years...

[laoneo]

Closing and reopen #15730.