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/models/account-manager.js b/lib/models/account-manager.js index c41cb1ade..f1d4f8152 100644 --- a/lib/models/account-manager.js +++ b/lib/models/account-manager.js @@ -537,7 +537,7 @@ class AccountManager { throw new Error('Email service is not set up') } - if (!userAccount.email) { + if (userAccount && !userAccount.email) { throw new Error('Account recovery email has not been provided') } } diff --git a/lib/requests/password-reset-email-request.js b/lib/requests/password-reset-email-request.js index a4e40c33a..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,7 +123,10 @@ class PasswordResetEmailRequest extends AuthRequest { return this.accountManager.accountExists(username) .then(exists => { if (!exists) { - throw new Error('Account not found for that username') + // 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 } @@ -191,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 2861a0fe2..0f27878fd 100644 --- a/test/unit/password-reset-email-request-test.js +++ b/test/unit/password-reset-email-request-test.js @@ -14,6 +14,7 @@ const HttpMocks = require('node-mocks-http') const PasswordResetEmailRequest = require('../../lib/requests/password-reset-email-request') const AccountManager = require('../../lib/models/account-manager') const SolidHost = require('../../lib/models/solid-host') +const EmailService = require('../../lib/services/email-service') describe('PasswordResetEmailRequest', () => { describe('constructor()', () => { @@ -153,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()', () => { @@ -175,16 +209,26 @@ describe('PasswordResetEmailRequest', () => { it('should throw an error if the user does not exist', done => { const host = SolidHost.from({ serverUri: 'https://example.com' }) const store = { suffixAcl: '.acl' } - const accountManager = AccountManager.from({ host, multiuser: true, store }) + const emailService = sinon.stub().returns(EmailService) + const accountManager = AccountManager.from({ host, multiuser: true, store, emailService }) accountManager.accountExists = sinon.stub().resolves(false) const username = 'alice' - const options = { accountManager, username } const request = new PasswordResetEmailRequest(options) + sinon.spy(request, 'resetLinkMessage') + sinon.spy(accountManager, 'userAccountFrom') + sinon.spy(accountManager, 'verifyEmailDependencies') + request.loadUser() - .catch(error => { - expect(error.message).to.equal('Account not found for that username') + .then(() => { + expect(accountManager.userAccountFrom).to.have.been.called() + expect(accountManager.verifyEmailDependencies).to.have.been.called() + expect(accountManager.verifyEmailDependencies).to.throw() + done() + }) + .catch(() => { + expect(request.resetLinkMessage).to.have.been.called() done() }) })