[#4208] - Unify the handeling of the login and logout redirect Ref #4142
- Closed
- 19 Jan 2015
- Medium
- Build: staging
- Easy Test
- # 4208
- Diff
- zero-24:patch-10
- Failure The Travis CI build failed Details
User tests: Successful: Unsuccessful:
Issue
JTracker: http://issues.joomla.org/tracker/joomla-cms/4142
Gh: #4142
Login redirect don't work for external URLs and currently we use a simple text field to store the value.
I would vote for changing the behavior on the com_users login form to the behavior that we have on mod_login that only internal urls are allow and use a dropdown to select it.
This is a workable solution
How to test
- apply patch
- create a new menu itm with the com_users login view. (not mod_login ;))
- choose a login redirect page with the dropdown
- choose a logout redirect page with the dropdown
- save
- login by using this menu itm
- make sure the redirect is correct
- logout by using the menu itm
- make sure the redirect is correct
This is B/C save
The old way is that we use a string containing the redirect url that stored in the params the new way is to store the ID.
This part of code will handel it:
$itemid = $params->get($type);
if (is_numeric($itemid))
{
[...]
}
if ($itemid != '')
{
// For B/C reasons
// The value in the param is not a number and not null
// so the param store a old value like a URL and it will used.
$url = $itemid;
}https://github.com/zero-24/joomla-cms/blob/patch-10/components/com_users/helpers/login.php#L62-68
If the params stores a old value like a url it will be used. If it store the id it also works.
Possible B/C break
if someone override one of these files (the views):
https://github.com/zero-24/joomla-cms/blob/patch-10/components/com_users/views/login/tmpl/default_logout.php
https://github.com/zero-24/joomla-cms/blob/patch-10/components/com_users/views/login/tmpl/default_login.php
They need to change the line
<input type="hidden" name="return" value="<?php echo base64_encode($this->params->get('logout_redirect_url', $this->form->getValue('return'))); ?>" />to
<input type="hidden" name="return" value="<?php echo $this->redirect; ?>" />Question
Is this a B/C Issue so this need to wait for 4.0 or can we add this to 3.3/3.4?
If it is a B/C Issue can we handel this any how?
Other mirror changes
- Some mirror CS issues specifically on the default.xml file.
- Move the TFA function (getTwoFactorMethods()) to UsersHelperLogin
jissues-bot
- | Labels |
Added:
?
|
||
| Rel_Number | ⇒ | 4142 | |
| Relation Type | ⇒ | Related to |
| Easy | No | ⇒ | Yes |
B/C issue imho.
It indeed is. I already discussed some possible ways around it with @zero-24.
Most likely it means creating a new parameter for it while keeping the old one hidden. Then the code can check for the presence of the new one and if not uses the old existing one.
@Bakual @infograf768 can you have again a look into this in case of B/C?
- I have hidden the old value (field) and add a new for the ID
- I have add some checks to the code. That should prefer the "old" value if it exists
- Now we set the new url also to the "old" field if we have a new one.
@hering can you have a test on this?
Here is a easy component that can help you
http://docs.joomla.org/Component_Patchtester_for_Testers
The easyest way to do is the following:
1. install com_patchster on a dev site (see link above)
2. apply the patch using com_patchtester (see the video)
3. test the options (dropdown) on the view
4. login and logout using the FE view.
5. report your results back here.
Note: If you still get redirect to a bad/old site please make sure that you choose a new login and logout page with the dropdown. As we use the old value if there is no now present.
Thanks
Personally I don't like this change. I agree that using a menuitem field would be better but I don't like the helper. This looks overcomplicated.
Personally I don't like this change. I agree that using a menuitem field would be better but I don't like the helper. This looks overcomplicated.
ok. it is just a copy of the helper from mod_login
https://github.com/joomla/joomla-cms/blob/staging/modules/mod_login/helper.php
I only add some code to be B/C.
I don't like the helper. This looks overcomplicated.
Please explain how we can have it easier/less complicated?
| Category | ⇒ | Components Front End |
Can you elucidate your comment e.g. with a better solution? I'm still open for any comment that make the code better 
nicksavov
- | Labels |
Added:
?
|
||
@test
Environment: Joomla! 3.3.6 with test data loaded
Step 1: I've followed the instructions and selected two already existing menus from the drop-downs, that pointed to articles. Login and logout work fine: the selected articles did appear.
Step 2: I've created a menu item with External URL, tested the menu item: working, selected in the drop-down of the login: error - 404 component not found.
User note: I find this patch useful, it makes the component more user friendly.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4208.
@test
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4208.
I am setting this to Needs Review based on the comments above by the PLT members so that they can make a final decision. Thanks for contributing and testing this issue
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4208.
| Status | Pending | ⇒ | Needs Review |
| Labels |
Removed:
?
|
||
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-01-19 08:53:38 |
B/C issue imho.