From f669e5c956943fb5ca91ff6c8be004940d5a7bb5 Mon Sep 17 00:00:00 2001 From: Zach Date: Sat, 16 Mar 2024 18:48:04 -0500 Subject: [PATCH] fix issue 1770 and 1774: added tests and functionality from PR 1770, fixed loadUser function --- default-views/auth/reset-link-sent.hbs | 2 +- lib/requests/password-reset-email-request.js | 17 +++------ .../unit/password-reset-email-request-test.js | 37 ++++++++++++++++++- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/default-views/auth/reset-link-sent.hbs b/default-views/auth/reset-link-sent.hbs index 1059f963a..aeef6d832 100644 --- a/default-views/auth/reset-link-sent.hbs +++ b/default-views/auth/reset-link-sent.hbs @@ -14,7 +14,7 @@
-

A Reset Password link has been sent to your email.

+

A Reset Password link has been sent from the associated email account.

diff --git a/lib/requests/password-reset-email-request.js b/lib/requests/password-reset-email-request.js index 91cf1fcdd..4ae5b0ac9 100644 --- a/lib/requests/password-reset-email-request.js +++ b/lib/requests/password-reset-email-request.js @@ -94,7 +94,7 @@ class PasswordResetEmailRequest extends AuthRequest { .then(() => request.validate()) .then(() => request.loadUser()) .then(userAccount => request.sendResetLink(userAccount)) - .then(() => request.renderSuccess()) + .then(() => request.resetLinkMessage()) .catch(error => request.error(error)) } @@ -123,15 +123,10 @@ class PasswordResetEmailRequest extends AuthRequest { return this.accountManager.accountExists(username) .then(exists => { if (!exists) { - try { - const userAccount = this.accountManager.userAccountFrom({ username }) - this.accountManager.verifyEmailDependencies(userAccount) - } catch (err) { - console.log(err.message) - if (err.message === 'Account recovery email has not been provided') { - return this.renderSuccess() - } - } + // For security reason avoid leaking error information + // See: https://github.com/nodeSolidServer/node-solid-server/issues/1770 + this.accountManager.verifyEmailDependencies() + return this.resetLinkMessage() } const userData = { username } @@ -199,7 +194,7 @@ class PasswordResetEmailRequest extends AuthRequest { /** * Displays the 'your reset link has been sent' success message view */ - renderSuccess () { + resetLinkMessage () { this.response.render('auth/reset-link-sent') } } diff --git a/test/unit/password-reset-email-request-test.js b/test/unit/password-reset-email-request-test.js index 81e85bcbf..0f27878fd 100644 --- a/test/unit/password-reset-email-request-test.js +++ b/test/unit/password-reset-email-request-test.js @@ -154,6 +154,39 @@ describe('PasswordResetEmailRequest', () => { expect(request.error).to.not.have.been.called() }) }) + + it('should hande a reset request with no username without privacy leakage', () => { + const host = SolidHost.from({ serverUri: 'https://example.com' }) + const store = { suffixAcl: '.acl' } + const accountManager = AccountManager.from({ host, multiuser: true, store }) + accountManager.loadAccountRecoveryEmail = sinon.stub().resolves('alice@example.com') + accountManager.sendPasswordResetEmail = sinon.stub().resolves() + accountManager.accountExists = sinon.stub().resolves(false) + + const returnToUrl = 'https://example.com/resource' + const username = 'alice' + const response = HttpMocks.createResponse() + response.render = sinon.stub() + + const options = { accountManager, username, returnToUrl, response } + const request = new PasswordResetEmailRequest(options) + + sinon.spy(request, 'error') + sinon.spy(request, 'validate') + sinon.spy(request, 'loadUser') + + return PasswordResetEmailRequest.handlePost(request) + .then(() => { + expect(request.validate).to.have.been.called() + expect(request.loadUser).to.have.been.called() + expect(request.loadUser).to.throw() + }).catch(() => { + expect(request.error).to.have.been.called() + expect(response.render).to.have.been.calledWith('auth/reset-link-sent') + expect(accountManager.loadAccountRecoveryEmail).to.not.have.been.called() + expect(accountManager.sendPasswordResetEmail).to.not.have.been.called() + }) + }) }) describe('loadUser()', () => { @@ -183,7 +216,7 @@ describe('PasswordResetEmailRequest', () => { const options = { accountManager, username } const request = new PasswordResetEmailRequest(options) - sinon.spy(request, 'renderSuccess') + sinon.spy(request, 'resetLinkMessage') sinon.spy(accountManager, 'userAccountFrom') sinon.spy(accountManager, 'verifyEmailDependencies') @@ -195,7 +228,7 @@ describe('PasswordResetEmailRequest', () => { done() }) .catch(() => { - expect(request.renderSuccess).to.have.been.called() + expect(request.resetLinkMessage).to.have.been.called() done() }) })