diff --git a/config/identity/handler/interaction/handler.json b/config/identity/handler/interaction/handler.json index 4fc29ef5e..94e2b9553 100644 --- a/config/identity/handler/interaction/handler.json +++ b/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" } ] }, diff --git a/config/identity/handler/interaction/handlers/reset-password.json b/config/identity/handler/interaction/handlers/reset-password.json index 042bf59f5..4fb0059e0 100644 --- a/config/identity/handler/interaction/handlers/reset-password.json +++ b/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", - "allowedMethods": [ "POST" ], - "allowedPathNames": [ "^/idp/resetpassword/?$" ], - "handler": { + "@type": "IdpRouteController", + "pathName": "^/idp/resetpassword/[^/]+$", + "postHandler": { "@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" } }, { diff --git a/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts b/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts index 8c49385af..db20b87fb 100644 --- a/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts +++ b/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 { 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, diff --git a/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts b/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts index 02af6e89c..9fdecc8d5 100644 --- a/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts +++ b/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 { - 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) { - await this.renderHandler.handleSafe({ - response: input.response, - contents: { - errorMessage: createErrorMessage(err), - recordId: prefilledRecordId, - }, - }); + } catch (error: unknown) { + throwIdpInteractionError(error); } } diff --git a/src/identity/interaction/email-password/handler/ResetPasswordRenderHandler.ts b/src/identity/interaction/email-password/handler/ResetPasswordRenderHandler.ts deleted file mode 100644 index 0f949acdb..000000000 --- a/src/identity/interaction/email-password/handler/ResetPasswordRenderHandler.ts +++ /dev/null @@ -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 {} diff --git a/src/identity/interaction/email-password/handler/ResetPasswordViewHandler.ts b/src/identity/interaction/email-password/handler/ResetPasswordViewHandler.ts deleted file mode 100644 index bf9c1fb3a..000000000 --- a/src/identity/interaction/email-password/handler/ResetPasswordViewHandler.ts +++ /dev/null @@ -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 { - 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, {}); - } - } -} diff --git a/src/index.ts b/src/index.ts index 844cc3c10..30c510649 100644 --- a/src/index.ts +++ b/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'; diff --git a/templates/identity/email-password/reset-password.html.ejs b/templates/identity/email-password/reset-password.html.ejs index 3ebcada86..788d02294 100644 --- a/templates/identity/email-password/reset-password.html.ejs +++ b/templates/identity/email-password/reset-password.html.ejs @@ -13,7 +13,7 @@

Reset password

-
+ <%if (errorMessage) { %>

<%= errorMessage %>

<% } %> @@ -26,11 +26,9 @@
  • - +
  • - -

    diff --git a/test/integration/Identity.test.ts b/test/integration/Identity.test.ts index 0631e317b..867ab82ac 100644 --- a/test/integration/Identity.test.ts +++ b/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 => { @@ -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'); - expect(typeof recordId).toBe('string'); - - // POST the new password - const formData = stringify({ password: password2, confirmPassword: password2, recordId }); - res = await fetch(new URL(relative!, baseUrl).href, { + // POST the new password to the same URL + const formData = stringify({ password: password2, confirmPassword: password2 }); + res = await fetch(nextUrl, { method: 'POST', headers: { 'content-type': APPLICATION_X_WWW_FORM_URLENCODED }, body: formData, diff --git a/test/unit/identity/interaction/email-password/handler/ForgotPasswordHandler.test.ts b/test/unit/identity/interaction/email-password/handler/ForgotPasswordHandler.test.ts index a3369c52e..f62549014 100644 --- a/test/unit/identity/interaction/email-password/handler/ForgotPasswordHandler.test.ts +++ b/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 = `Reset Password`; + const html = `Reset Password`; 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); diff --git a/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts b/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts index 9e6b2cddf..d9ff3cab0 100644 --- a/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts +++ b/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 => { + it('errors for non-string recordIds.', async(): Promise => { const errorMessage = 'Invalid request. Open the link from your email again'; request = createPostFormRequest({}); - await expect(handler.handle({ request, response })).resolves.toBeUndefined(); - expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1); - expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId: '' }}); - 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: '' }}); + await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage); + request = createPostFormRequest({}, ''); + await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage); }); - it('renders errors for invalid passwords.', async(): Promise => { + it('errors for invalid passwords.', async(): Promise => { const errorMessage = 'Password and confirmation do not match'; - request = createPostFormRequest({ recordId, password: 'password!', confirmPassword: 'otherPassword!' }); - await expect(handler.handle({ request, response })).resolves.toBeUndefined(); - expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1); - expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }}); + request = createPostFormRequest({ password: 'password!', confirmPassword: 'otherPassword!' }, url); + await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage); }); - it('renders errors for invalid emails.', async(): Promise => { + it('errors for invalid emails.', async(): Promise => { 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(); - expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1); - expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }}); + await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage); }); it('renders a message on success.', async(): Promise => { - 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 => { 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(); - expect(renderHandler.handleSafe).toHaveBeenCalledTimes(1); - expect(renderHandler.handleSafe).toHaveBeenLastCalledWith({ response, contents: { errorMessage, recordId }}); + await expect(handler.handle({ request, response })).rejects.toThrow(errorMessage); }); }); diff --git a/test/unit/identity/interaction/email-password/handler/ResetPasswordViewHandler.test.ts b/test/unit/identity/interaction/email-password/handler/ResetPasswordViewHandler.test.ts deleted file mode 100644 index 482076df2..000000000 --- a/test/unit/identity/interaction/email-password/handler/ResetPasswordViewHandler.test.ts +++ /dev/null @@ -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 => { - request = {} as any; - - renderHandler = { - handleSafe: jest.fn(), - } as any; - - handler = new ResetPasswordViewHandler(renderHandler); - }); - - it('requires a URL.', async(): Promise => { - await expect(handler.handle({ request, response })).rejects.toThrow('The request must have a URL'); - }); - - it('requires a record ID.', async(): Promise => { - 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 => { - 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' }, - }); - }); -}); diff --git a/test/unit/identity/interaction/email-password/handler/Util.ts b/test/unit/identity/interaction/email-password/handler/Util.ts index 0348f17b7..73bfdc8e6 100644 --- a/test/unit/identity/interaction/email-password/handler/Util.ts +++ b/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): HttpRequest { +export function createPostFormRequest(data: NodeJS.Dict, url?: string): HttpRequest { const request = guardedStreamFrom(stringify(data)) as HttpRequest; request.headers = { 'content-type': 'application/x-www-form-urlencoded' }; + request.url = url; return request; }