Skip to content

Commit f669e5c

Browse files
committed
fix issue 1770 and 1774: added tests and functionality from PR 1770, fixed loadUser function
1 parent c38fe6e commit f669e5c

File tree

3 files changed

+42
-14
lines changed

3 files changed

+42
-14
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
</div>
1515

1616
<div class="alert alert-success">
17-
<p>A Reset Password link has been sent to your email.</p>
17+
<p>A Reset Password link has been sent from the associated email account.</p>
1818
</div>
1919
</div>
2020
</body>

lib/requests/password-reset-email-request.js

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class PasswordResetEmailRequest extends AuthRequest {
9494
.then(() => request.validate())
9595
.then(() => request.loadUser())
9696
.then(userAccount => request.sendResetLink(userAccount))
97-
.then(() => request.renderSuccess())
97+
.then(() => request.resetLinkMessage())
9898
.catch(error => request.error(error))
9999
}
100100

@@ -123,15 +123,10 @@ class PasswordResetEmailRequest extends AuthRequest {
123123
return this.accountManager.accountExists(username)
124124
.then(exists => {
125125
if (!exists) {
126-
try {
127-
const userAccount = this.accountManager.userAccountFrom({ username })
128-
this.accountManager.verifyEmailDependencies(userAccount)
129-
} catch (err) {
130-
console.log(err.message)
131-
if (err.message === 'Account recovery email has not been provided') {
132-
return this.renderSuccess()
133-
}
134-
}
126+
// For security reason avoid leaking error information
127+
// See: https://github.com/nodeSolidServer/node-solid-server/issues/1770
128+
this.accountManager.verifyEmailDependencies()
129+
return this.resetLinkMessage()
135130
}
136131

137132
const userData = { username }
@@ -199,7 +194,7 @@ class PasswordResetEmailRequest extends AuthRequest {
199194
/**
200195
* Displays the 'your reset link has been sent' success message view
201196
*/
202-
renderSuccess () {
197+
resetLinkMessage () {
203198
this.response.render('auth/reset-link-sent')
204199
}
205200
}

test/unit/password-reset-email-request-test.js

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,39 @@ describe('PasswordResetEmailRequest', () => {
154154
expect(request.error).to.not.have.been.called()
155155
})
156156
})
157+
158+
it('should hande a reset request with no username without privacy leakage', () => {
159+
const host = SolidHost.from({ serverUri: 'https://example.com' })
160+
const store = { suffixAcl: '.acl' }
161+
const accountManager = AccountManager.from({ host, multiuser: true, store })
162+
accountManager.loadAccountRecoveryEmail = sinon.stub().resolves('alice@example.com')
163+
accountManager.sendPasswordResetEmail = sinon.stub().resolves()
164+
accountManager.accountExists = sinon.stub().resolves(false)
165+
166+
const returnToUrl = 'https://example.com/resource'
167+
const username = 'alice'
168+
const response = HttpMocks.createResponse()
169+
response.render = sinon.stub()
170+
171+
const options = { accountManager, username, returnToUrl, response }
172+
const request = new PasswordResetEmailRequest(options)
173+
174+
sinon.spy(request, 'error')
175+
sinon.spy(request, 'validate')
176+
sinon.spy(request, 'loadUser')
177+
178+
return PasswordResetEmailRequest.handlePost(request)
179+
.then(() => {
180+
expect(request.validate).to.have.been.called()
181+
expect(request.loadUser).to.have.been.called()
182+
expect(request.loadUser).to.throw()
183+
}).catch(() => {
184+
expect(request.error).to.have.been.called()
185+
expect(response.render).to.have.been.calledWith('auth/reset-link-sent')
186+
expect(accountManager.loadAccountRecoveryEmail).to.not.have.been.called()
187+
expect(accountManager.sendPasswordResetEmail).to.not.have.been.called()
188+
})
189+
})
157190
})
158191

159192
describe('loadUser()', () => {
@@ -183,7 +216,7 @@ describe('PasswordResetEmailRequest', () => {
183216
const options = { accountManager, username }
184217
const request = new PasswordResetEmailRequest(options)
185218

186-
sinon.spy(request, 'renderSuccess')
219+
sinon.spy(request, 'resetLinkMessage')
187220
sinon.spy(accountManager, 'userAccountFrom')
188221
sinon.spy(accountManager, 'verifyEmailDependencies')
189222

@@ -195,7 +228,7 @@ describe('PasswordResetEmailRequest', () => {
195228
done()
196229
})
197230
.catch(() => {
198-
expect(request.renderSuccess).to.have.been.called()
231+
expect(request.resetLinkMessage).to.have.been.called()
199232
done()
200233
})
201234
})

0 commit comments

Comments
 (0)