Skip to content

Commit

Permalink
fix issue 1770 and 1774: added tests and functionality from PR 1770, …
Browse files Browse the repository at this point in the history
…fixed loadUser function
  • Loading branch information
zg009 committed Mar 16, 2024
1 parent c38fe6e commit f669e5c
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 14 deletions.
2 changes: 1 addition & 1 deletion default-views/auth/reset-link-sent.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
17 changes: 6 additions & 11 deletions lib/requests/password-reset-email-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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')
}
}
Expand Down
37 changes: 35 additions & 2 deletions test/unit/password-reset-email-request-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('[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()', () => {
Expand Down Expand Up @@ -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')

Expand All @@ -195,7 +228,7 @@ describe('PasswordResetEmailRequest', () => {
done()
})
.catch(() => {
expect(request.renderSuccess).to.have.been.called()
expect(request.resetLinkMessage).to.have.been.called()
done()
})
})
Expand Down

0 comments on commit f669e5c

Please sign in to comment.