Merge pull request #1775 from nodeSolidServer/fix/issue#1774

fix issue 1774 & 1770

Author: zg009

default-views/auth/reset-link-sent.hbs

@@ -14,7 +14,7 @@
   </div>
 
   <div class="alert alert-success">
-    <p>A Reset Password link has been sent to your email.</p>
+    <p>A Reset Password link has been sent from the associated email account.</p>
   </div>
 </div>
 </body>

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')
     }
   }

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')
   }
 }

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('[email protected]')
+      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()
         })
     })