Allow to change password through RPC endpoints through the reset password workflow

Created on 24 August 2017, about 7 years ago
Updated 7 November 2023, about 1 year ago

This is a follow up to #2847708: RPC endpoint to reset user password and just adding it for discussion.

I was hoping to submit a patch for this but I found that this will require a bit of rework as how _skipProtectedUserFieldConstraint is worked in the workflow will likely either need duplication or some proper planning on how to rework.

I wonder if the validation of a password reset token should be added to the actual ProtectedUserFieldConstraint instead of outside.

In the meantime, I worked on an alternative to this on https://www.drupal.org/sandbox/hanoii/2904436

It should work for both 8.3.x and 8.4.x.

Feature request
Status

Needs work

Version

11.0 🔥

Component
User module 

Last updated about 19 hours ago

Created by

🇦🇷Argentina hanoii 🇦🇷UTC-3

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to Needs review about 1 year ago
  • last update about 1 year ago
    Patch Failed to Apply
  • 🇨🇳China skyredwang Shanghai

    Here is the first draft, no tests. I am reusing the /user/password endpoint, if the HTTP method is POST, then this implementation assume it is for resetting password, if the HTTP method is PATCH, then it is for changing password.

  • last update about 1 year ago
    30,438 pass
  • 🇮🇳India _utsavsharma

    As previous patch failed , patch for 11.x.

  • last update about 1 year ago
    Patch Failed to Apply
  • Status changed to Needs work about 1 year ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
    1. +++ b/core/modules/user/src/Controller/UserAuthenticationController.php
      @@ -260,15 +262,52 @@ public function resetPassword(Request $request) {
      +        if (!$account->isAuthenticated()) {
      +          throw new BadRequestHttpException('Authentication is required to change your password.');
      +        }
      

      Once the patch request is a separate route you can make this a route requirement.

    2. +++ b/core/modules/user/src/Controller/UserAuthenticationController.php
      @@ -260,15 +262,52 @@ public function resetPassword(Request $request) {
      +        // Set existing password if set in the form state.
      

      No form state here.

    3. +++ b/core/modules/user/user.routing.yml
      @@ -132,8 +132,8 @@ user.pass:
       user.pass.http:
         path: '/user/password'
         defaults:
      -    _controller: \Drupal\user\Controller\UserAuthenticationController::resetPassword
      -  methods: [POST]
      +    _controller: \Drupal\user\Controller\UserAuthenticationController::changeOrResetPassword
      +  methods: [POST, PATCH]
         requirements:
           _access: 'TRUE'
           _format: 'json'
      

      Let's add a different route for just patch. And let it have it's own controller. I don;t think we shoudl be renaming resetPassword here. And I think having a separate changePassword method is okay.

      We can refactor the common parts of both methods into private functions on the controller.

  • 🇨🇳China skyredwang Shanghai

    The reason I didn't want to use a seperate controller is because the existing resetPassword uses /user/password.

    /user/password is generic, if I use a new path like /user/change_password for changing password, then we have two end points for the consumer. One generic, one specific, that's confusing to me. What do you think?

    Attached improved the code comment per suggestion 2

  • 🇨🇳China skyredwang Shanghai

    Corrected the patch path

  • last update about 1 year ago
    Patch Failed to Apply
  • last update about 1 year ago
    Patch Failed to Apply
  • 🇺🇸United States bradjones1 Digital Nomad Life

    Re-queued the patch since it's against 11.x but DrupalCI was running it against 10.1.

    Also just want to say I love that this is using HTTP verb semantics to determine the operation to be performed!

  • Status changed to Needs review about 1 year ago
  • last update about 1 year ago
    Patch Failed to Apply
  • 🇨🇳China skyredwang Shanghai

    I spoke with @alexpott on Slack, and understood that I can keep the same endpoint but defining a new route based on the new HTTP method.

    I pushed the change to the issue fork 11.x

  • @skyredwang opened merge request.
  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    Hiding patches as work is in a MR with correct branch.

    But appears to have some failures.

  • Status changed to Needs review about 1 year ago
  • 🇨🇳China skyredwang Shanghai

    The lastest version is functioning. It supports two use cases:

    - Change password knowing the existing password
    - Change password without knowing the existing password, but have the timestamp and token from forget password URL

    Remaining work to do:

    - Tests
    - Some of the code are copied from /core/modules/user/src/AccountForm.php and /opt/drupal/web/core/modules/user/src/Controller/UserController.php , better to re-organize them, but need suggestions, as I am not familiar with core.

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    Passed tests, just needs it's own coverage as mentioned.

Production build 0.71.5 2024