feat: Store reset password ID in the submit URL

config/identity/handler/interaction/handler.json

@@ -19,7 +19,6 @@
         { "@id": "urn:solid-server:auth:password:LoginInteractionHandler" },
         { "@id": "urn:solid-server:auth:password:SessionInteractionHandler" },
         { "@id": "urn:solid-server:auth:password:ForgotPasswordInteractionHandler" },
-        { "@id": "urn:solid-server:auth:password:ResetPasswordViewInteractionHandler" },
         { "@id": "urn:solid-server:auth:password:ResetPasswordInteractionHandler" }
       ]
     },

config/identity/handler/interaction/handlers/reset-password.json

@@ -2,29 +2,17 @@
   "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld",
   "comment": "Exports 2 handlers: one for viewing the page and one for doing the reset.",
   "@graph": [
-    {
-      "comment": "Renders the reset password page",
-      "@id": "urn:solid-server:auth:password:ResetPasswordViewInteractionHandler",
-      "@type": "RouterHandler",
-      "allowedMethods": [ "GET" ],
-      "allowedPathNames": [ "^/idp/resetpassword/?$" ],
-      "handler": {
-        "@type": "ResetPasswordViewHandler",
-        "renderHandler": { "@id": "urn:solid-server:auth:password:ResetPasswordRenderHandler" }
-      }
-    },
     {
       "comment": "Handles the reset password page submission",
       "@id": "urn:solid-server:auth:password:ResetPasswordInteractionHandler",
-      "@type": "RouterHandler",
+      "@type": "IdpRouteController",
-      "allowedMethods": [ "POST" ],
+      "pathName": "^/idp/resetpassword/[^/]+$",
-      "allowedPathNames": [ "^/idp/resetpassword/?$" ],
+      "postHandler": {
-      "handler": {
         "@type": "ResetPasswordHandler",
         "args_accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" },
-        "args_renderHandler": { "@id": "urn:solid-server:auth:password:ResetPasswordRenderHandler" },
         "args_messageRenderHandler": { "@id": "urn:solid-server:auth:password:MessageRenderHandler" }
-      }
+      },
+      "renderHandler": { "@id": "urn:solid-server:auth:password:ResetPasswordRenderHandler" }
     },
 
     {

src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts

@@ -79,7 +79,7 @@ export class ForgotPasswordHandler extends InteractionHttpHandler {
    */
   private async sendResetMail(recordId: string, email: string): Promise<void> {
     this.logger.info(`Sending password reset to ${email}`);
-    const resetLink = urljoin(this.baseUrl, this.idpPath, `resetpassword?rid=${recordId}`);
+    const resetLink = urljoin(this.baseUrl, this.idpPath, `resetpassword/${recordId}`);
     const renderedEmail = await this.templateEngine.render({ resetLink });
     await this.emailSender.handleSafe({
       recipient: email,

src/identity/interaction/email-password/handler/ResetPasswordHandler.ts

@@ -3,15 +3,12 @@ import { getLoggerFor } from '../../../../logging/LogUtil';
 import type { HttpHandlerInput } from '../../../../server/HttpHandler';
 import { HttpHandler } from '../../../../server/HttpHandler';
 import type { TemplateHandler } from '../../../../server/util/TemplateHandler';
-import { createErrorMessage } from '../../../../util/errors/ErrorUtil';
 import { getFormDataRequestBody } from '../../util/FormDataUtil';
-import { assertPassword } from '../EmailPasswordUtil';
+import { assertPassword, throwIdpInteractionError } from '../EmailPasswordUtil';
 import type { AccountStore } from '../storage/AccountStore';
-import type { ResetPasswordRenderHandler } from './ResetPasswordRenderHandler';
 
 export interface ResetPasswordHandlerArgs {
   accountStore: AccountStore;
-  renderHandler: ResetPasswordRenderHandler;
   messageRenderHandler: TemplateHandler<{ message: string }>;
 }
 
@@ -23,26 +20,24 @@ export class ResetPasswordHandler extends HttpHandler {
   protected readonly logger = getLoggerFor(this);
 
   private readonly accountStore: AccountStore;
-  private readonly renderHandler: ResetPasswordRenderHandler;
   private readonly messageRenderHandler: TemplateHandler<{ message: string }>;
 
   public constructor(args: ResetPasswordHandlerArgs) {
     super();
     this.accountStore = args.accountStore;
-    this.renderHandler = args.renderHandler;
     this.messageRenderHandler = args.messageRenderHandler;
   }
 
   public async handle(input: HttpHandlerInput): Promise<void> {
-    let prefilledRecordId = '';
     try {
+      // Extract record ID from request URL
+      const recordId = /\/([^/]+)$/u.exec(input.request.url!)?.[1];
       // Validate input data
-      const { password, confirmPassword, recordId } = await getFormDataRequestBody(input.request);
+      const { password, confirmPassword } = await getFormDataRequestBody(input.request);
       assert(
         typeof recordId === 'string' && recordId.length > 0,
         'Invalid request. Open the link from your email again',
       );
-      prefilledRecordId = recordId;
       assertPassword(password, confirmPassword);
 
       await this.resetPassword(recordId, password);
@@ -52,14 +47,8 @@ export class ResetPasswordHandler extends HttpHandler {
           message: 'Your password was successfully reset.',
         },
       });
-    } catch (err: unknown) {
+    } catch (error: unknown) {
-      await this.renderHandler.handleSafe({
+      throwIdpInteractionError(error);
-        response: input.response,
-        contents: {
-          errorMessage: createErrorMessage(err),
-          recordId: prefilledRecordId,
-        },
-      });
     }
   }
 

src/identity/interaction/email-password/handler/ResetPasswordRenderHandler.ts

@@ -1,12 +0,0 @@
-import { TemplateHandler } from '../../../../server/util/TemplateHandler';
-
-export interface ResetPasswordRenderHandlerProps {
-  errorMessage: string;
-  recordId: string;
-}
-
-/**
- * A special {@link RenderHandler} for the Reset Password form
- * that includes the required props for rendering the reset password form.
- */
-export abstract class ResetPasswordRenderHandler extends TemplateHandler<ResetPasswordRenderHandlerProps> {}

src/identity/interaction/email-password/handler/ResetPasswordViewHandler.ts

@@ -1,36 +0,0 @@
-import assert from 'assert';
-import { parse } from 'url';
-import type { HttpHandlerInput } from '../../../../server/HttpHandler';
-import { HttpHandler } from '../../../../server/HttpHandler';
-import { throwIdpInteractionError } from '../EmailPasswordUtil';
-import type { ResetPasswordRenderHandler } from './ResetPasswordRenderHandler';
-
-/**
- * Handles the creation of the Reset Password form
- * after the user clicks on it from the link provided in the email.
- */
-export class ResetPasswordViewHandler extends HttpHandler {
-  private readonly renderHandler: ResetPasswordRenderHandler;
-
-  public constructor(renderHandler: ResetPasswordRenderHandler) {
-    super();
-    this.renderHandler = renderHandler;
-  }
-
-  public async handle({ request, response }: HttpHandlerInput): Promise<void> {
-    try {
-      assert(request.url, 'The request must have a URL');
-      const recordId = parse(request.url, true).query.rid;
-      assert(
-        typeof recordId === 'string' && recordId.length > 0,
-        'A forgot password record ID must be provided. Use the link you have received by email.',
-      );
-      await this.renderHandler.handleSafe({
-        response,
-        contents: { errorMessage: '', recordId },
-      });
-    } catch (error: unknown) {
-      throwIdpInteractionError(error, {});
-    }
-  }
-}

src/index.ts

@@ -24,8 +24,6 @@ export * from './identity/interaction/email-password/handler/ForgotPasswordHandl
 export * from './identity/interaction/email-password/handler/LoginHandler';
 export * from './identity/interaction/email-password/handler/RegistrationHandler';
 export * from './identity/interaction/email-password/handler/ResetPasswordHandler';
-export * from './identity/interaction/email-password/handler/ResetPasswordRenderHandler';
-export * from './identity/interaction/email-password/handler/ResetPasswordViewHandler';
 
 // Identity/Interaction/Email-Password/Storage
 export * from './identity/interaction/email-password/storage/AccountStore';

templates/identity/email-password/reset-password.html.ejs

@@ -13,7 +13,7 @@
   </header>
   <main>
     <h1>Reset password</h1>
-    <form action="/idp/resetpassword" method="post">
+    <form method="post">
       <%if (errorMessage) { %>
         <p class="error"><%= errorMessage %></p>
       <% } %>
@@ -26,11 +26,9 @@
           </li>
           <li>
             <label for="confirmPassword">Confirm new password</label>
-            <input id="confirmPassword" type="password" name="password" placeholder="">
+            <input id="confirmPassword" type="password" name="confirmPassword" placeholder="">
           </li>
         </ol>
-
-        <input type="hidden" name="recordId" value="<%= recordId %>" />
       </fieldset>
 
       <p class="actions"><button type="submit" name="submit">Reset password</button></p>

test/integration/Identity.test.ts

@@ -200,7 +200,7 @@ describe('A Solid server with IDP', (): void => {
       const match = /(http:.*)$/u.exec(mail.text);
       expect(match).toBeDefined();
       nextUrl = match![1];
-      expect(nextUrl).toContain('resetpassword?rid=');
+      expect(nextUrl).toMatch(/\/resetpassword\/[^/]+$/u);
     });
 
     it('resets the password through the given link.', async(): Promise<void> => {
@@ -209,14 +209,12 @@ describe('A Solid server with IDP', (): void => {
       expect(res.status).toBe(200);
       const text = await res.text();
       const relative = load(text)('form').attr('action');
-      expect(typeof relative).toBe('string');
+      // Reset password form has no action causing the current URL to be used
+      expect(relative).toBeUndefined();
 
-      const recordId = load(text)('input[name="recordId"]').attr('value');
+      // POST the new password to the same URL
-      expect(typeof recordId).toBe('string');
+      const formData = stringify({ password: password2, confirmPassword: password2 });
-
+      res = await fetch(nextUrl, {
-      // POST the new password
-      const formData = stringify({ password: password2, confirmPassword: password2, recordId });
-      res = await fetch(new URL(relative!, baseUrl).href, {
         method: 'POST',
         headers: { 'content-type': APPLICATION_X_WWW_FORM_URLENCODED },
         body: formData,

test/unit/identity/interaction/email-password/handler/ForgotPasswordHandler.test.ts

@@ -15,7 +15,7 @@ describe('A ForgotPasswordHandler', (): void => {
   const response: HttpResponse = {} as any;
   const email = 'test@test.email';
   const recordId = '123456';
-  const html = `<a href="/base/idp/resetpassword?rid=${recordId}">Reset Password</a>`;
+  const html = `<a href="/base/idp/resetpassword/${recordId}">Reset Password</a>`;
   const renderParams = { response, contents: { errorMessage: '', prefilled: { email }}};
   const provider: Provider = {} as any;
   let messageRenderHandler: IdpRenderHandler;
@@ -76,7 +76,7 @@ describe('A ForgotPasswordHandler', (): void => {
     expect(emailSender.handleSafe).toHaveBeenLastCalledWith({
       recipient: email,
       subject: 'Reset your password',
-      text: `To reset your password, go to this link: http://test.com/base/idp/resetpassword?rid=${recordId}`,
+      text: `To reset your password, go to this link: http://test.com/base/idp/resetpassword/${recordId}`,
       html,
     });
     expect(messageRenderHandler.handleSafe).toHaveBeenCalledTimes(1);

test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts

@@ -1,9 +1,6 @@
 import {
   ResetPasswordHandler,
 } from '../../../../../../src/identity/interaction/email-password/handler/ResetPasswordHandler';
-import type {
-  ResetPasswordRenderHandler,
-} from '../../../../../../src/identity/interaction/email-password/handler/ResetPasswordRenderHandler';
 import type { AccountStore } from '../../../../../../src/identity/interaction/email-password/storage/AccountStore';
 import type { HttpRequest } from '../../../../../../src/server/HttpRequest';
 import type { HttpResponse } from '../../../../../../src/server/HttpResponse';
@@ -14,9 +11,9 @@ describe('A ResetPasswordHandler', (): void => {
   let request: HttpRequest;
   const response: HttpResponse = {} as any;
   const recordId = '123456';
+  const url = `/resetURL/${recordId}`;
   const email = 'alice@test.email';
   let accountStore: AccountStore;
-  let renderHandler: ResetPasswordRenderHandler;
   let messageRenderHandler: TemplateHandler<{ message: string }>;
   let handler: ResetPasswordHandler;
 
@@ -27,52 +24,39 @@ describe('A ResetPasswordHandler', (): void => {
       changePassword: jest.fn(),
     } as any;
 
-    renderHandler = {
-      handleSafe: jest.fn(),
-    } as any;
-
     messageRenderHandler = {
       handleSafe: jest.fn(),
     } as any;
 
     handler = new ResetPasswordHandler({
       accountStore,
-      renderHandler,
       messageRenderHandler,
     });
   });
 
-  it('renders errors for non-string recordIds.', async(): Promise<void> => {
+  it('errors for non-string recordIds.', async(): Promise<void> => {
     const errorMessage = 'Invalid request. Open the link from your email again';
     request = createPostFormRequest({});
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
+    await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage);
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1);
+    request = createPostFormRequest({}, '');
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId: '' }});
+    await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage);
-    request = createPostFormRequest({ recordId: [ 'a', 'b' ]});
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(2);
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId: '' }});
   });
 
-  it('renders errors for invalid passwords.', async(): Promise<void> => {
+  it('errors for invalid passwords.', async(): Promise<void> => {
     const errorMessage = 'Password and confirmation do not match';
-    request = createPostFormRequest({ recordId, password: 'password!', confirmPassword: 'otherPassword!' });
+    request = createPostFormRequest({ password: 'password!', confirmPassword: 'otherPassword!' }, url);
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
+    await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage);
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }});
   });
 
-  it('renders errors for invalid emails.', async(): Promise<void> => {
+  it('errors for invalid emails.', async(): Promise<void> => {
     const errorMessage = 'This reset password link is no longer valid.';
-    request = createPostFormRequest({ recordId, password: 'password!', confirmPassword: 'password!' });
+    request = createPostFormRequest({ password: 'password!', confirmPassword: 'password!' }, url);
     (accountStore.getForgotPasswordRecord as jest.Mock).mockResolvedValueOnce(undefined);
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
+    await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage);
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }});
   });
 
   it('renders a message on success.', async(): Promise<void> => {
-    request = createPostFormRequest({ recordId, password: 'password!', confirmPassword: 'password!' });
+    request = createPostFormRequest({ password: 'password!', confirmPassword: 'password!' }, url);
     await expect(handler.handle({ request, response })).resolves.toBeUndefined();
     expect(accountStore.getForgotPasswordRecord).toHaveBeenCalledTimes(1);
     expect(accountStore.getForgotPasswordRecord).toHaveBeenLastCalledWith(recordId);
@@ -87,10 +71,8 @@ describe('A ResetPasswordHandler', (): void => {
 
   it('has a default error for non-native errors.', async(): Promise<void> => {
     const errorMessage = 'Unknown error: not native';
-    request = createPostFormRequest({ recordId, password: 'password!', confirmPassword: 'password!' });
+    request = createPostFormRequest({ password: 'password!', confirmPassword: 'password!' }, url);
     (accountStore.getForgotPasswordRecord as jest.Mock).mockRejectedValueOnce('not native');
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
+    await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage);
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }});
   });
 });

test/unit/identity/interaction/email-password/handler/ResetPasswordViewHandler.test.ts

@@ -1,48 +0,0 @@
-import type {
-  ResetPasswordRenderHandler,
-} from '../../../../../../src/identity/interaction/email-password/handler/ResetPasswordRenderHandler';
-import {
-  ResetPasswordViewHandler,
-} from '../../../../../../src/identity/interaction/email-password/handler/ResetPasswordViewHandler';
-import type { HttpRequest } from '../../../../../../src/server/HttpRequest';
-import type { HttpResponse } from '../../../../../../src/server/HttpResponse';
-
-describe('A ResetPasswordViewHandler', (): void => {
-  let request: HttpRequest;
-  const response: HttpResponse = {} as any;
-  let renderHandler: ResetPasswordRenderHandler;
-  let handler: ResetPasswordViewHandler;
-
-  beforeEach(async(): Promise<void> => {
-    request = {} as any;
-
-    renderHandler = {
-      handleSafe: jest.fn(),
-    } as any;
-
-    handler = new ResetPasswordViewHandler(renderHandler);
-  });
-
-  it('requires a URL.', async(): Promise<void> => {
-    await expect(handler.handle({ request, response })).rejects.toThrow('The request must have a URL');
-  });
-
-  it('requires a record ID.', async(): Promise<void> => {
-    request.url = '/foo';
-    await expect(handler.handle({ request, response })).rejects
-      .toThrow('A forgot password record ID must be provided. Use the link you have received by email.');
-    request.url = '/foo?wrong=recordId';
-    await expect(handler.handle({ request, response })).rejects
-      .toThrow('A forgot password record ID must be provided. Use the link you have received by email.');
-  });
-
-  it('renders the response.', async(): Promise<void> => {
-    request.url = '/foo?rid=recordId';
-    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
-    expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({
-      response,
-      contents: { errorMessage: '', recordId: 'recordId' },
-    });
-  });
-});

test/unit/identity/interaction/email-password/handler/Util.ts

@@ -6,9 +6,11 @@ import { guardedStreamFrom } from '../../../../../../src/util/StreamUtil';
  * Creates a mock HttpRequest which is a stream of an object encoded as application/x-www-form-urlencoded
  * and a matching content-type header.
  * @param data - Object to encode.
+ * @param url - URL value of the request.
  */
-export function createPostFormRequest(data: NodeJS.Dict<any>): HttpRequest {
+export function createPostFormRequest(data: NodeJS.Dict<any>, url?: string): HttpRequest {
   const request = guardedStreamFrom(stringify(data)) as HttpRequest;
   request.headers = { 'content-type': 'application/x-www-form-urlencoded' };
+  request.url = url;
   return request;
 }