Skip to content

Commit

Permalink
profile: Add reset password feature.
Browse files Browse the repository at this point in the history
  • Loading branch information
freezy committed May 15, 2019
1 parent 731f036 commit 0afd8fe
Show file tree
Hide file tree
Showing 11 changed files with 308 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Looks like you were changing your email address at VPDB. Please let us know that

{{{ confirmationUrl }}}

Once you have done that, you'll get all email to this new address.
Once you have done that, youll get all email to this new address.

Have a nice day!

Expand Down
11 changes: 11 additions & 0 deletions src/app/common/email-templates/password-reset-request.handlebars
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Hi {{{ user.name }}},

Someone, probably you, requested to reset your password at VPDB. You can do that by clicking on the link below.

{{{ confirmationUrl }}}

If it wasn’t you, you can ignore this email.

Have a nice day!

-The VPDB Server.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Hi {{{ user.name }}},

Just writing you to let you know that your password at VPDB has been successfully reset.

Cheers!

-The VPDB Server.
18 changes: 16 additions & 2 deletions src/app/common/mailer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ class Mailer {
});
}

public async resetPasswordRequest(requestState: RequestState, user: UserDocument, email: string, token: string): Promise<SentMessageInfo> {
return this.sendEmail(requestState, user, 'Password reset at VPDB', 'password-reset-request', {
user,
confirmationUrl: settings.webUri('/reset-password/' + token),
}, undefined, email);
}

public async resetPasswordSuccess(requestState: RequestState, user: UserDocument): Promise<SentMessageInfo> {
return this.sendEmail(requestState, user, 'Your password has been reset', 'password-reset-success', {
user,
});
}

public async welcomeLocal(requestState: RequestState, user: UserDocument): Promise<SentMessageInfo> {
return this.sendEmail(requestState, user, 'Welcome to VPDB!', 'welcome-local', { user });
}
Expand Down Expand Up @@ -355,9 +368,10 @@ class Mailer {
* @param {string} template Name of the Handlebars template, without path or extension
* @param {object} templateData Data passed to the Handlebars renderer
* @param {string} [enabledFlag] If set, user profile must have this preference set to true
* @param {string} [emailAddress] If set, this email as the recipient's email instead of the user's default.
* @return Promise<SentMessageInfo>
*/
private async sendEmail(requestState: RequestState, user: UserDocument, subject: string, template: string, templateData: object, enabledFlag: string = null): Promise<SentMessageInfo> {
private async sendEmail(requestState: RequestState, user: UserDocument, subject: string, template: string, templateData: object, enabledFlag?: string, emailAddress?: string): Promise<SentMessageInfo> {

const what = template.replace(/-/g, ' ');
if (!this.emailEnabled(user, enabledFlag)) {
Expand All @@ -372,7 +386,7 @@ class Mailer {
// setup email
const email: Mail.Options = {
from: { name: config.vpdb.email.sender.name, address: config.vpdb.email.sender.email },
to: { name: user.name, address: user.email },
to: { name: user.name, address: emailAddress || user.email },
subject,
text,
};
Expand Down
6 changes: 6 additions & 0 deletions src/app/common/slackbot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,12 @@ export class SlackBot {
case 'change_password':
msg.msg = 'Changed password.';
break;
case 'reset_password':
msg.msg = 'Reset password.';
break;
case 'request_reset_password':
msg.msg = 'Requested password to be reset.';
break;
case 'create_local_account':
msg.msg = 'Added local credentials.';
break;
Expand Down
16 changes: 16 additions & 0 deletions src/app/profile/profile.api.acl.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ describe('The ACLs of the `Profile` API', function() {
it('should deny updates of user profile', function(done) {
request.patch('/api/v1/profile').send({}).end(hlp.status(401, done));
});

it('should allow password reset requests', function(done) {
request.post('/api/v1/profile/request-password-reset').send({}).end(hlp.status(422, done));
});

it('should allow password resets', function(done) {
request.post('/api/v1/profile/password-reset').send({}).end(hlp.status(422, done));
});
});

describe('for logged clients (role member)', function() {
Expand All @@ -66,6 +74,14 @@ describe('The ACLs of the `Profile` API', function() {
it('should allow access to user logs', function(done) {
request.get('/api/v1/profile/logs').as('member').end(hlp.status(200, done));
});

it('should allow password reset requests', function(done) {
request.post('/api/v1/profile/request-password-reset').as('member').send({}).end(hlp.status(422, done));
});

it('should allow password resets', function(done) {
request.post('/api/v1/profile/password-reset').as('member').send({}).end(hlp.status(422, done));
});
});

describe('for members with the `contributor` role', function() {
Expand Down
2 changes: 2 additions & 0 deletions src/app/profile/profile.api.router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export class ProfileApiRouter implements ApiRouter {
this.router.get('/v1/profile', api.auth(api.view.bind(api), 'user', 'view', [ Scope.ALL, Scope.COMMUNITY ]));
this.router.patch('/v1/profile', api.auth(api.update.bind(api), 'user', 'update', [ Scope.ALL ]));
this.router.get('/v1/profile/confirm/:tkn', api.confirm.bind(api));
this.router.post('/v1/profile/request-password-reset', api.requestResetPassword.bind(api));
this.router.post('/v1/profile/password-reset', api.resetPassword.bind(api));

const logApi = new LogUserApi();
this.router.get('/v1/profile/logs', api.auth(logApi.list.bind(api), 'user', 'view', [ Scope.ALL ]));
Expand Down
96 changes: 95 additions & 1 deletion src/app/profile/profile.api.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ describe('The VPDB `profile` API', () => {
admin: { roles: [ 'admin' ]},
contributor: { roles: [ 'contributor' ]},
member: { roles: [ 'member' ]},
chprofile: { roles: [ 'member' ]}
chprofile: { roles: [ 'member' ]},
chpass: { roles: [ 'member' ]},
});
});

Expand Down Expand Up @@ -458,6 +459,99 @@ describe('The VPDB `profile` API', () => {
});
});

describe('when a local user requests a password reset', () => {

it('should fail if the email is not provided', async () => {
await api
.post('/v1/profile/request-password-reset', { })
.then(res => res.expectValidationError('email', 'Email must be provided'));
});

it('should fail if the email is not a string', async () => {
await api
.post('/v1/profile/request-password-reset', { email: { email: 'test' } })
.then(res => res.expectValidationError('email', 'Email must be a string'));
});

it('should fail if the email does not exist', async () => {
await api
.post('/v1/profile/request-password-reset', { email: 'zrrgamapel'})
.then(res => res.expectValidationError('email','Can\'t find that email, sorry'));
});

it('should fail if the email is linked to an OAuth account', async () => {
const oauthUser = await api.createOAuthUser('github');
await api
.post('/v1/profile/request-password-reset', { email: oauthUser.user.email })
.then(res => res.expectError(400,'you\'ve previously logged in via GitHub'));
});

it('should succeed for a valid email address', async () => {
const res = await api
.post('/v1/profile/request-password-reset', { email: api.getUser('chpass').email, returnEmailToken: 1 })
.then(res => res.expectStatus(200));
expect(res.data.message).to.contain('Email sent');
});

});

describe('when a local user resets the password', () => {

it('should fail if token is not provided', async () => {
await api
.post('/v1/profile/password-reset', {})
.then(res => res.expectValidationError('token', 'token must be provided'));
});

it('should fail if token is not a string', async () => {
await api
.post('/v1/profile/password-reset', { token: { token: 'asdf' }})
.then(res => res.expectValidationError('token', 'token must be a string'));
});

it('should fail if password is not provided', async () => {
await api
.post('/v1/profile/password-reset', { token: '1234' })
.then(res => res.expectValidationError('password', 'password must be provided'));
});

it('should fail if password is not a string', async () => {
await api
.post('/v1/profile/password-reset', { token: '1234', password: { password: 'asdf' }})
.then(res => res.expectValidationError('password', 'password must be a string'));
});

it('should fail if token is invalid', async () => {
await api
.post('/v1/profile/password-reset', { token: '1234', password: 'asdf' })
.then(res => res.expectValidationError('token', 'invalid token'));
});

it('should fail if the new password is invalid', async () => {
const user = api.getUser('chpass');
const res = await api
.post('/v1/profile/request-password-reset', { email: user.email, returnEmailToken: 1 })
.then(res => res.expectStatus(200));
const token = res.data.token;
await api
.post('/v1/profile/password-reset', { token: token, password: '1' })
.then(res => res.expectValidationError('password', 'Password must be at least'));
});

it('should succeed if the token and the new password are valid', async () => {
const user = api.getUser('chpass');
let res = await api
.post('/v1/profile/request-password-reset', { email: user.email, returnEmailToken: 1 })
.then(res => res.expectStatus(200));
const token = res.data.token;
res = await api
.post('/v1/profile/password-reset', { token: token, password: 'newpassword' })
.then(res => res.expectStatus(200));
expect(res.data.message).to.contain('Password updated');
});

});

describe.skip('when a non-local user sets its username', () => {

it('should succeed when providing a valid password', async () => {
Expand Down
114 changes: 113 additions & 1 deletion src/app/profile/profile.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

import { assign, pick, uniq } from 'lodash';
import { assign, pick, uniq, isString } from 'lodash';
import randomString from 'randomstring';

import { acl } from '../common/acl';
Expand Down Expand Up @@ -234,6 +234,118 @@ export class ProfileApi extends Api {
});
}

/**
* Requests a password request for a given email address
*
* @see POST /v1/profile/request-password-reset
* @param ctx Koa context
*/
public async requestResetPassword(ctx: Context) {

// FIXME rate limit!

if (!ctx.request.body.email) {
throw new ApiError().validationError('email', 'Email must be provided');
}
if (!isString(ctx.request.body.email)) {
throw new ApiError().validationError('email', 'Email must be a string');
}
const email = ctx.request.body.email.trim();

// TODO make sure newUser.email is sane (comes from user directly)
const user = await state.models.User.findOne({
$or: [
{ emails: email },
{ validated_emails: email },
],
}).exec();

// check if email exists
if (!user) {
throw new ApiError().validationError('email', 'Can\'t find that email, sorry.');
}

// check if user is local
if (!user.is_local) {
const providers = user.getProviderNames();
const lastProvider = providers.pop();
const via = providers.length ? [ providers.join(', '), lastProvider ].join(' or ') : lastProvider;
throw new ApiError('You don\'t have a password set because you\'ve previously logged in via %s.', via).status(400);
}

// set reset token
const token = randomString.generate(16);
await state.models.User.findOneAndUpdate({ _id: user._id },
{ $set: {
password_reset: {
token,
expires_at: new Date(Date.now() + 86400000), // 1d valid
}
} }).exec();

// log
await LogUserUtil.success(ctx, user, 'request_reset_password');

// return body
if (process.env.NODE_ENV === 'test' && ctx.request.body.returnEmailToken) {
this.success(ctx, { message: 'Email sent.', token });
} else {
/* istanbul ignore next: Test case is above */
this.success(ctx, { message: 'Email sent.' });
}


// send password reset mail
this.noAwait(async () => {
await mailer.resetPasswordRequest(ctx.state, user, email, token);
});
}

/**
* Sets a new password with a previously requested token
*
* @see POST /v1/profile/password-reset
* @param ctx Koa context
*/
public async resetPassword(ctx: Context) {

// validations
if (!ctx.request.body.token) {
throw new ApiError().validationError('token', 'Token must be provided');
}
if (!isString(ctx.request.body.token)) {
throw new ApiError().validationError('token', 'Token must be a string');
}
if (!ctx.request.body.password) {
throw new ApiError().validationError('password', 'Password must be provided');
}
if (!isString(ctx.request.body.password)) {
throw new ApiError().validationError('password', 'Password must be a string');
}
const user = await state.models.User.findOne({ 'password_reset.token': ctx.request.body.token });
if (!user) {
throw new ApiError().validationError('token', 'Invalid token', ctx.request.body.token);
}
if (user.password_reset.expires_at.getTime() < Date.now()) {
throw new ApiError().validationError('token', 'Token expired. Please request a password reset again.');
}

// save new password
user.password = ctx.request.body.password;
await user.save();

// log
await LogUserUtil.success(ctx, user, 'reset_password');

// return body
this.success(ctx, { message: 'Password updated.' });

// send confirmation
this.noAwait(async () => {
await mailer.resetPasswordSuccess(ctx.state, user);
});
}

private async changeAttributes(ctx: Context, updatedUser: UserDocument) {
// check for dupe name
if (ctx.request.body.name) {
Expand Down
12 changes: 10 additions & 2 deletions src/app/users/user.document.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ export interface UserDocument extends MetricsDocument {
roles?: string[];
_plan?: string;
is_local?: boolean;
providers?: UserProviders;
password_hash?: string;
password_salt?: string;
password_reset?: {
token: string;
expires_at: Date;
};
thumb?: string;
location?: string;
preferences?: UserPreferences;
Expand All @@ -53,7 +56,7 @@ export interface UserDocument extends MetricsDocument {
subscribed_releases: string[];
api_key: string;
};

providers?: UserProviders;
provider?: string;
provider_id?: string;
gravatar_id?: string;
Expand Down Expand Up @@ -98,6 +101,11 @@ export interface UserDocument extends MetricsDocument {
* @return {boolean} True if at least one role matches, false otherwise.
*/
hasRole?(role: string | string[]): boolean;

/**
* Returns the names of the OAuth provider the user has logged in.
*/
getProviderNames?(): string[];
}

export interface UserPreferences extends Document {
Expand Down
Loading

0 comments on commit 0afd8fe

Please sign in to comment.