? Success

User tests: Successful: Unsuccessful:

avatar lcdservices
lcdservices
24 Aug 2015

#7737 - enforce frontend construction for user activation URL
#7737


when the register method constructs the user activation URL, it doesn't explicitly enforce a frontend URL. this means that 3rd party extensions wanting to trigger registration (and thus activation emails) from a backend tool cannot make use of the core register method.

since the activation URL is only valid from the frontend, we should always enforce the proper construction.

there are two locations to be modified:
https://github.com/joomla/joomla-cms/blob/staging/components/com_users/models/registration.php#L410
https://github.com/joomla/joomla-cms/blob/staging/components/com_users/models/registration.php#L447

In both cases, we can just include the base url inside JRoute::_() to achieve the enforcement:

$data['activate'] = JRoute::_($base . '/index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false);

avatar lcdservices lcdservices - open - 24 Aug 2015
avatar lcdservices lcdservices - change - 24 Aug 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Aug 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 25 Aug 2015
Title
Update registration.php
user activation link doesn't enforce frontend base URL
avatar zero-24
zero-24 - comment - 25 Aug 2015

@lcdservices I have just tested this but it don't work for me.

How to test

  • enable frontend registratration
  • choose "self" registration
  • go to the frontend and register an account
  • click the activation link. --> Works
  • got to the backend and set "admin registration"
  • go to the frontend and register an account
  • click the activation link. --> Works
  • apply this patch

  • choose "self" registration

  • go to the frontend and register an account
  • click the activation link. --> don't work here
  • got to the backend and set "admin registration"
  • go to the frontend and register an account
  • click the activation link. --> don't work here.

expected result

http://www.example.org/dev/e3/index.php?option=com_users&task=registration.activate&token=ab7bc2b688beb2477c103ce4ee5b318b

actual result

http://www.example.org/index.php?option=com_users&task=registration.activate&token=ab7bc2b688beb2477c103ce4ee5b318b

avatar zero-24 zero-24 - change - 25 Aug 2015
Status Pending Information Required
avatar zero-24 zero-24 - test_item - 25 Aug 2015 - Tested unsuccessfully
avatar zero-24 zero-24 - change - 25 Aug 2015
Category Front End
avatar infograf768
infograf768 - comment - 25 Aug 2015

I wonder if we need $base here.
Maybe we would just need `JRoute::_(JUri::root() . 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false);

avatar joomdonation
joomdonation - comment - 26 Aug 2015

This code is wrong. And I think the solution is quite simple. We just need to remove administrator/ from $data['activate'] when this method is called from administrator area with this code

if (JFactory::getApplication()->isAdmin())
{
    $adminPos         = strrpos($data['activate'], 'administrator/');
    $data['activate'] = substr_replace($data['activate'], '', $adminPos, 14);
}

I made a small PR to your branch to make it works like that lcdservices#1. Please try to test it and if it works, please merge it so that other testers can test it

avatar lcdservices
lcdservices - comment - 26 Aug 2015

I think the suggestion from @infograf768 is much cleaner and more direct than doing a string replacement after the URL has been constructed. @zero-24 -- can you test the revised PR? I don't have a dev site set up where Joomla is installed in a subdirectory (which is the use case that created the issues you noticed).

avatar joomdonation
joomdonation - comment - 26 Aug 2015

@lcdservices While the suggestion from @infograf768 works, it always return none-sef URL. Also, I haven't seen anywhere in our core code which pass full URL into JRoute::_ method, so I don't think that the code is good enough.

avatar zero-24
zero-24 - comment - 26 Aug 2015

hmm generaly it works but it looks like we need to fix is also here:
https://github.com/lcdservices/joomla-cms/blob/patch-1/components/com_users/models/registration.php#L85

We also get with this patch applyed:

error_registratration

But i have no clue why?

avatar zero-24
zero-24 - comment - 26 Aug 2015

hmm i guess the seccond on was an error on my side. I can not reproduce this again?!

Ok. So only point on need to be fixed see:
https://github.com/lcdservices/joomla-cms/blob/patch-1/components/com_users/models/registration.php#L85

and the comment by @joomdonation

avatar joomdonation
joomdonation - comment - 26 Aug 2015

@zero-24 I think we don't have to worry at the code at activate method (which you mentioned). The reason is because usually, third party developer only uses register method to create a Joomla user in Joomla core (I use it in my extension too), but for the activate method, I think it is just handled by Joomla core (when users click on the activation link).

avatar lcdservices
lcdservices - comment - 26 Aug 2015

ok...
@zero-24 I fixed it in the additional location you identified
@joomdonation - I really don't like the string replace method -- it's too hackish. but I agree it seems the only way to do this while preserving SEF urls. So I restructured it to handle it in this way.

avatar zero-24 zero-24 - change - 27 Aug 2015
Status Information Required Pending
avatar joomdonation
joomdonation - comment - 18 Sep 2015

@lcdservices Could you merge @zero-24 PR so that travis happy and we can get this PR merged?

I use this function in my extension, too, and would love to have this merged in next release of Joomla so that I don't have to change code / fix bugs for client

avatar lcdservices
lcdservices - comment - 18 Sep 2015

done. sorry, didn't notice he had done it as a PR against my patch.

avatar zero-24 zero-24 - test_item - 18 Sep 2015 - Tested successfully
avatar zero-24 zero-24 - alter_testresult - 18 Sep 2015 - joomdonation: Tested successfully
avatar zero-24 zero-24 - change - 18 Sep 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 18 Sep 2015

RTC based on testing :smile:


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

avatar joomla-cms-bot joomla-cms-bot - change - 18 Sep 2015
Labels Added: ?
avatar infograf768
infograf768 - comment - 19 Sep 2015

Any reason not to use a simpler:
$data['activate'] = str_replace('administrator/', '', $data['activate'])
instead of looking for its position?

avatar joomdonation
joomdonation - comment - 19 Sep 2015

Might have bug in case you you have a site called administrator

for example, http://localhost/administrator/administrator

Of course it will only happens in that very special case but we should avoid that.

avatar zero-24 zero-24 - change - 19 Sep 2015
Milestone Added:
avatar rdeutz rdeutz - change - 1 Oct 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-10-01 18:13:27
Closed_By rdeutz
avatar rdeutz rdeutz - close - 1 Oct 2015
avatar joomla-cms-bot joomla-cms-bot - close - 1 Oct 2015
avatar rdeutz rdeutz - reference | 881dbc2 - 1 Oct 15
avatar rdeutz rdeutz - merge - 1 Oct 2015
avatar rdeutz rdeutz - close - 1 Oct 2015
avatar joomla-cms-bot joomla-cms-bot - change - 1 Oct 2015
Labels Removed: ?
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone

Add a Comment

Login with GitHub to post a comment