User tests: Successful: Unsuccessful:
With this change we make sure the reset token is invalidated (set to another value) when the account mail is changed.
With this patch this is not possible any more as the token has been changed at the time the mail has been changed
Without this patch the old token still works.
none.
This issue has been reported to the JSST by Ramesh From J2store (@rameshelamathi) but the JSST decided to fix this in the public tracker. Thanks!
Status | New | ⇒ | Pending |
Milestone |
Added: |
Category | ⇒ | Administration com_users Front End |
Labels |
Added:
?
|
You might as well just blank out the token. Unless you're sending the new token to the user it's unusable anyway.
Well this token / activation field also applys to the "Activation"
When we have an empty field this is marked as "Activated". I'm happy to apply any code just wanted to make sure the UI thing is not changed at all.
If the UI is solely relying on "token field is empty" IMO that's an issue too.
The problem you're running into now is you're changing the activation token and not giving the new token to the user. So if they try to activate the account with the token they already have, they won't be able to actually activate the account.
The problem you're running into now is you're changing the activation token and not giving the new token to the user. So if they try to activate the account with the token they already have, they won't be able to actually activate the account.
Wich is exactly what is expected ;) But I'm happy to empty the token too.
I get the intent. But the user has no idea that changing their email is invalidating that token and they need a new one to finish the activation step. That's the issue I'm raising, without providing the new token it is now impossible for a user to activate their own account in this scenario.
Milestone |
Removed: |
Category | Administration com_users Front End | ⇒ | Administration com_users Language & Strings Front End |
Labels |
Added:
?
|
Looks fine to me.
thanks
@brianteeman can you please give me feedback about the new strings I added here to be send as mail to the users when the mail has been changed:
COM_USERS_EMAIL_REGISTERED_WITH_ADMIN_ACTIVATION_CHANGED_MAIL_BODY="Hello %s,\n\nThank you for registering at %s. Your account is created and must be verified before you can use it.\nBefore the latest confirmation the account mail has been changed so we need you to confirm the new mail please select the following link or copy-paste it in your browser:\n %s \n\nAfter verification an administrator will be notified to activate your account. You'll receive a confirmation when it's done.\nOnce that account has been activated you may login to %s using the following username and the password you entered during registration:\n\nUsername: %s"
COM_USERS_EMAIL_REGISTERED_WITH_ACTIVATION_CHANGED_MAIL_BODY="Hello %s,\n\nThank you for registering at %s. Your account is created and must be activated before you can use it.\nBefore the latest confirmation the account mail has been changed so we need you to confirm the new mail please select the following link or copy-paste it in your browser:\n%s \n\nAfter activation you may login to %s using the following username and the password you entered during registration:\n\nUsername: %s"
If I understand this correctly every time you say "mail" you should say "email address"
Thanks just pushed the fix
Thanks @brianteeman This is ready now for testing
Milestone |
Added: |
Milestone |
Removed: |
Milestone |
Added: |
Title |
|
Title |
|
Title |
|
Title |
|
Category | Administration com_users Front End Language & Strings | ⇒ | Administration com_users Front End |
Category | Administration com_users Front End | ⇒ | Administration com_users Language & Strings Front End |
Category | Administration com_users Front End Language & Strings | ⇒ | Administration com_users Front End |
Category | Administration com_users Front End | ⇒ | Administration com_users Language & Strings Front End |
Because I had some problems to understand the new procedure yesterday ("in a terribly tired mood"): Are the testing instructions still correct after additional commits? I had some problems to test this pr. When I tried to change the email I got a message like "Change your password, too" but when I tried that, I couldn't save the profile and saw the same message.
Old or newly created password: Always the same message.
It looks like, that after any attempt to save the profile with a new email a new activation email is sent but the page doesn't inform the user accordingly. I've found these emails this morning.
I tested again today. > I get the activation email and cllick the link after steps above and get:
I don't know if relevant: My settings:
Title |
|
Title |
|
Labels |
Removed:
J3 Issue
|
Milestone |
Removed: |
Milestone |
Added: |
This is not working for me. Patched J3.9.5 with Michael's baby. Steps taken for testing as described above. PW request has been send and is received by new user. Logging in on frontened, changing email in the profile and trying to save gives
Warning
Profile could not be saved: Could not bind profile data: Passwords do not match. Please re-enter password.
If in the user manager keep using the new email and try to safe (after the patch) gives
Error
Save failed with the following error: Passwords do not match. Please re-enter password.
Trying to do the same with keeping the old email in the user manager in the backend gives
Error
Save failed with the following error: You can't reuse your current password, please enter a new password.
I have tested this item
see my comment
Why drone pr is failing?
It seams the JavaScript tests are having problems. Drone has been rebooted: https://ci.joomla.org/joomla/joomla-cms/18687
Milestone for this should now be changed to 3.9.9
I have tested this item
I've tested this issue successfully with 3.9.8
I have tested this item
I've tested this issue successfully with 3.9.8
Any update here @bembelimen ?
Any update here @bembelimen ?
Any update here @bembelimen ?
Category | Administration com_users Front End Language & Strings | ⇒ | Administration com_users Language & Strings |
Done thanks @wilsonge @bembelimen ready for testing again :)
@joomla/security can you please do some testing here
One issue found. Perhaps it's just a Route class problem and not related to this pr!! (No, see next comment):
Super User creates account (Registered). back-end
Registered User gets welcome email and logs in
Registered User requests password reset and got email.
Super User changes email address. back-end
Registered User gets immediately the following email text to the new email address with incomplete URL (domain missing).
What's relevant here is that I have not activated SSL in Joomla but on host side.
user.php line 206 and 207:
$linkMode = (int) $app->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;
$data['activate'] = Route::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode);
When I use hard coded:
$linkMode = Route::TLS_FORCE;
The $data['activate']
URL is correct (with domain part).
Thanks @HLeithner
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-10-17 09:42:42 |
Closed_By | ⇒ | zero-24 |
Status | Closed | ⇒ | New |
Closed_Date | 2020-10-17 09:42:42 | ⇒ | |
Closed_By | zero-24 | ⇒ |
Status | New | ⇒ | Pending |
Hmm I'm not sure whats up with AppVeyor but this PR is up-to-date with staging now.
Please fix phpcs.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-07 17:19:57 |
Closed_By | ⇒ | zero-24 |
You might as well just blank out the token. Unless you're sending the new token to the user it's unusable anyway.