Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Login URL is wrong (doesn't check layout) #19485

Merged
merged 2 commits into from May 10, 2019

Conversation

OctavianC
Copy link
Contributor

@OctavianC OctavianC commented Jan 30, 2018

Summary of Changes

The Legacy router grabs the incorrect item ID if you've created a logout menu item before creating the login menu; that's because it doesn't verify the layout. This patch fixes that. No idea how to fix the Modern routing though.

Testing Instructions

  • Make sure SEF is enabled
  • Make sure you're using URL Routing set to Legacy in Users > Manage > Options > Integration
  • The order is important: create a new Users > Logout menu item and after that, create a new Users > Login menu item.
  • Run the following code:
require_once JPATH_SITE.'/components/com_users/helpers/route.php';
var_dump(JRoute::_('index.php?option=com_users&view=login'));
var_dump(UsersHelperRoute::getLoginRoute());
die;

Expected result

string(21) "/joomla/index.php/login" string(3) "110" 

Actual result

string(21) "/joomla/index.php/logout" string(3) "109" 

Documentation Changes Required

@csthomas
Copy link
Contributor

IMO there is a little weird configuration after #7703.
Logout view should have own view=logout not index.php?option=com_users&view=login&layout=logout&task=user.menulogout

@brianteeman
Copy link
Contributor

isnt this related to the change being discussed at #19497

@OctavianC
Copy link
Contributor Author

OctavianC commented Jan 31, 2018

This was on 3.8.3 so no relation with that issue. This seems to have been added way back in #7703 - it added a new layout for the logout menu item, but the routing didn't take it into account (it assumed if it's view=login then it's a login menu item and picks up the Itemid - however, we now have a view=login&layout=logout which is a logout menu item - a little bit of discrepancy here, but everything should be fine after this is merged) :)

@brianteeman
Copy link
Contributor

OK - thanks for clarifying that

@Quy
Copy link
Contributor

Quy commented Feb 25, 2018

I have tested this item ✅ successfully on f655360


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19485.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on f655360


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19485.

@Quy
Copy link
Contributor

Quy commented Feb 3, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19485.

@joomla-cms-bot joomla-cms-bot added PR-staging RTC This Pull Request is Ready To Commit labels Feb 3, 2019
@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@alikon
Copy link
Contributor

alikon commented May 2, 2019

@HLeithner please a final response

1 year old rtc pr

@HLeithner HLeithner merged commit 91ebd57 into joomla:staging May 10, 2019
@HLeithner
Copy link
Member

thx

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 10, 2019
@HLeithner HLeithner added this to the Joomla 3.9.7 milestone May 10, 2019
tecpromotion pushed a commit to tecpromotion/joomla-cms that referenced this pull request May 23, 2019
@OctavianC OctavianC deleted the fix-login-url branch November 23, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants