Skip to content

Commit

Permalink
Invalidate all tokens when email changes (#9855)
Browse files Browse the repository at this point in the history
  • Loading branch information
Betree authored Jul 16, 2024
1 parent fe6afc8 commit 998d590
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 36 deletions.
1 change: 1 addition & 0 deletions config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"confirmToken": 15
},
"setPasswordPerUserPerHour": 10,
"confirmEmailPerIpPerHour": 10,
"skipCleanOrdersLimitSlugs": "",
"enabledMasks": "",
"sendGuestConfirmPerMinutePerEmail": 1,
Expand Down
26 changes: 25 additions & 1 deletion server/graphql/common/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import emailLib from '../../lib/email';
import logger from '../../lib/logger';
import models, { Collective, Op, sequelize } from '../../models';
import User from '../../models/User';
import { ValidationFailed } from '../errors';
import { InvalidToken, ValidationFailed } from '../errors';

type CreateUserOptions = {
organizationData?: {
Expand Down Expand Up @@ -123,3 +123,27 @@ export const hasSeenLatestChangelogEntry = async (user: User): Promise<boolean>
}
return userChangelogViewDate >= latestChangelogUpdatePublishDate;
};

/**
* From a given token (generated in `updateUserEmail`) confirm the new email
* and updates the user record.
*/
export const confirmUserEmail = async emailConfirmationToken => {
if (!emailConfirmationToken) {
throw new ValidationFailed('Email confirmation token must be set');
}

const user = await models.User.findOne({ where: { emailConfirmationToken } });

if (!user) {
throw new InvalidToken('Invalid email confirmation token', 'INVALID_TOKEN', {
internalData: { emailConfirmationToken },
});
}

return user.update({
email: user.emailWaitingForValidation,
emailWaitingForValidation: null,
emailConfirmationToken: null,
});
};
22 changes: 22 additions & 0 deletions server/graphql/schemaV2.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -17904,6 +17904,16 @@ type Mutation {
currentPassword: String
): SetPasswordResponse!

"""
Confirm email for Individual. Scope: "account".
"""
confirmEmail(
"""
The token to confirm the email.
"""
token: String!
): IndividualConfirmEmailResponse!

"""
Submit a legal document
"""
Expand Down Expand Up @@ -20004,6 +20014,18 @@ type SetPasswordResponse {
token: String
}

type IndividualConfirmEmailResponse {
"""
The account that was confirmed
"""
individual: Individual!

"""
A new session token to use for the account. Only returned if user is signed in already.
"""
sessionToken: String
}

"""
The `Upload` scalar type represents a file upload.
"""
Expand Down
5 changes: 3 additions & 2 deletions server/graphql/v1/mutations.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import models from '../../models';
import { bulkCreateGiftCards, createGiftCardsForEmails } from '../../paymentProviders/opencollective/giftcard';
import { checkCanEmitGiftCards } from '../common/features';
import { editPublicMessage } from '../common/members';
import { createUser } from '../common/user';
import { confirmUserEmail, createUser } from '../common/user';
import { NotFound, RateLimitExceeded, Unauthorized } from '../errors';

import {
Expand All @@ -26,7 +26,7 @@ import { editConnectedAccount } from './mutations/connectedAccounts';
import { createWebhook, deleteNotification, editWebhooks } from './mutations/notifications';
import * as paymentMethodsMutation from './mutations/paymentMethods';
import { editTier, editTiers } from './mutations/tiers';
import { confirmUserEmail, updateUserEmail } from './mutations/users';
import { updateUserEmail } from './mutations/users';
import { CollectiveInterfaceType } from './CollectiveInterface';
import {
CollectiveInputType,
Expand Down Expand Up @@ -180,6 +180,7 @@ const mutations = {
confirmUserEmail: {
type: UserType,
description: 'Confirm the new user email from confirmation token',
deprecationReason: '2024-07-15: Please use the mutation `confirmEmail` from GQLV2',
args: {
token: {
type: new GraphQLNonNull(GraphQLString),
Expand Down
26 changes: 1 addition & 25 deletions server/graphql/v1/mutations/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { activities } from '../../../constants';
import cache from '../../../lib/cache';
import emailLib from '../../../lib/email';
import models from '../../../models';
import { InvalidToken, RateLimitExceeded, Unauthorized, ValidationFailed } from '../../errors';
import { RateLimitExceeded, Unauthorized, ValidationFailed } from '../../errors';

const oneHourInSeconds = 60 * 60;

Expand Down Expand Up @@ -79,27 +79,3 @@ export const updateUserEmail = async (user, newEmail) => {

return user;
};

/**
* From a given token (generated in `updateUserEmail`) confirm the new email
* and updates the user record.
*/
export const confirmUserEmail = async emailConfirmationToken => {
if (!emailConfirmationToken) {
throw new ValidationFailed('Email confirmation token must be set');
}

const user = await models.User.findOne({ where: { emailConfirmationToken } });

if (!user) {
throw new InvalidToken('Invalid email confirmation token', 'INVALID_TOKEN', {
internalData: { emailConfirmationToken },
});
}

return user.update({
email: user.emailWaitingForValidation,
emailWaitingForValidation: null,
emailConfirmationToken: null,
});
};
66 changes: 62 additions & 4 deletions server/graphql/v2/mutation/IndividualMutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import assert from 'assert';
import bcrypt from 'bcrypt';
import config from 'config';
import express from 'express';
import { GraphQLBoolean, GraphQLNonNull, GraphQLString } from 'graphql';
import { GraphQLDateTime } from 'graphql-scalars';
import { GraphQLBoolean, GraphQLNonNull, GraphQLObjectType, GraphQLString } from 'graphql';
import { GraphQLDateTime, GraphQLNonEmptyString } from 'graphql-scalars';

import RateLimit, { ONE_HOUR_IN_SECONDS } from '../../../lib/rate-limit';
import TwoFactorAuthLib from '../../../lib/two-factor-authentication';
import { checkRemoteUserCanUseAccount } from '../../common/scope-check';
import { confirmUserEmail } from '../../common/user';
import { RateLimitExceeded, Unauthorized } from '../../errors';
import { GraphQLIndividual } from '../object/Individual';
import { GraphQLSetPasswordResponse } from '../object/SetPasswordResponse';
Expand Down Expand Up @@ -91,8 +92,8 @@ const individualMutations = {

let token;

// We don't want OAuth tokens to be exchanged against a session token
if (req.userToken?.type !== 'OAUTH') {
// We don't want OAuth/Personal tokens to be exchanged against a session token
if (!req.userToken && !req.personalToken) {
// Context: this is token generation when updating password
token = await user.generateSessionToken({
sessionId: req.jwtPayload?.sessionId,
Expand All @@ -104,6 +105,63 @@ const individualMutations = {
return { individual, token };
},
},
confirmEmail: {
description: 'Confirm email for Individual. Scope: "account".',
type: new GraphQLNonNull(
new GraphQLObjectType({
name: 'IndividualConfirmEmailResponse',
fields: {
individual: {
type: new GraphQLNonNull(GraphQLIndividual),
description: 'The account that was confirmed',
},
sessionToken: {
type: GraphQLString,
description: 'A new session token to use for the account. Only returned if user is signed in already.',
},
},
}),
),
args: {
token: {
type: new GraphQLNonNull(GraphQLNonEmptyString),
description: 'The token to confirm the email.',
},
},
resolve: async (_, { token: confirmEmailToken }, req) => {
// Forbid this route for OAuth and Personal Tokens. Remember to check the scope if you want to allow it.
// Also make sure to prevent exchanging OAuth/Personal tokens for session tokens.
if (req.userToken || req.personalToken) {
throw new Unauthorized('OAuth and Personal Tokens are not allowed for this route');
}

// Rate limit (by IP, since we support anonymous requests)
const rateLimitKey = `individual_confirm_email_ip_${req.ip}`;
const rateLimitMax = config.limits.confirmEmailPerIpPerHour;
const rateLimit = new RateLimit(rateLimitKey, rateLimitMax, ONE_HOUR_IN_SECONDS);
if (!(await rateLimit.registerCall())) {
throw new RateLimitExceeded();
}

// Confirm email
const user = await confirmUserEmail(confirmEmailToken);
const individual = await user.getCollective({ loaders: req.loaders });

// The sign-in token
let sessionToken;

// Re-generate the session token if the user is already signed in
if (req.remoteUser && req.remoteUser.id === user.id) {
sessionToken = await user.generateSessionToken({
sessionId: req.jwtPayload?.sessionId,
createActivity: false,
updateLastLoginAt: false,
});
}

return { individual, sessionToken };
},
},
};

export default individualMutations;
2 changes: 2 additions & 0 deletions server/middleware/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ const _authenticateUserByJwt = async (req, res, next) => {
reportMessageToSentry(`User has no collective linked`, { user });
next();
return;
} else if (req.jwtPayload.email && user.email !== req.jwtPayload.email) {
return next(new errors.Unauthorized('This token has expired'));
}

const { earlyAccess = {} } = user.collective.settings || {};
Expand Down
2 changes: 2 additions & 0 deletions server/models/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class User extends Model<InferAttributes<User>, InferCreationAttributes<User>> {
*/
jwt = function (payload = undefined, expiration = undefined) {
expiration = expiration || auth.TOKEN_EXPIRATION_LOGIN;
payload = payload || {};
payload.email = this.email; // Include email to easily invalidate all token types when email change
return auth.createJwt(this.id, payload, expiration);
};

Expand Down
105 changes: 101 additions & 4 deletions test/server/graphql/v2/mutation/IndividualMutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@ import { expect } from 'chai';
import config from 'config';
import crypto from 'crypto-js';
import gql from 'fake-tag';
import jwt from 'jsonwebtoken';
import { createSandbox } from 'sinon';
import speakeasy from 'speakeasy';
import request from 'supertest';

import { idEncode } from '../../../../../server/graphql/v2/identifiers';
import { fakeApplication, fakeUser, fakeUserToken, randEmail } from '../../../../test-helpers/fake-data';
import {
fakeApplication,
fakePersonalToken,
fakeUser,
fakeUserToken,
randEmail,
randStr,
} from '../../../../test-helpers/fake-data';
import { startTestServer, stopTestServer } from '../../../../test-helpers/server';
import { graphqlQueryV2 } from '../../../../utils';

Expand Down Expand Up @@ -192,10 +200,10 @@ describe('server/graphql/v2/mutation/IndividualMutations', () => {
.post('/graphql')
.send({ query: resetPasswordMutation, variables: { password: 'newpassword' } })
.set('Authorization', `Bearer ${token}`)
.expect(200);
.expect(401);

expect(res.body.errors).to.exist;
expect(res.body.errors[0].message).to.equal('This token has expired');
expect(res.body.error.code).to.equal(401);
expect(res.body.error.message).to.equal('This token has expired');
});
});

Expand Down Expand Up @@ -252,4 +260,93 @@ describe('server/graphql/v2/mutation/IndividualMutations', () => {
});
});
});

describe('confirmEmail', () => {
const confirmEmailMutation = gql`
mutation ConfirmEmail($token: NonEmptyString!) {
confirmEmail(token: $token) {
sessionToken
individual {
id
slug
}
}
}
`;

it('should error if the token is invalid', async () => {
const result = await graphqlQueryV2(confirmEmailMutation, { token: 'invalidtoken' });
expect(result.errors).to.exist;
expect(result.errors[0].message).to.equal('Invalid email confirmation token');
});

it('cannot be used with a OAuth or personal token', async () => {
const user = await fakeUser({ emailWaitingForValidation: randEmail(), emailConfirmationToken: randStr() });

const userToken = await fakeUserToken({ type: 'OAUTH', UserId: user.id });
const resultOauth = await graphqlQueryV2(
confirmEmailMutation,
{ token: user.emailConfirmationToken },
user,
null,
null,
userToken,
);
expect(resultOauth.errors).to.exist;
expect(resultOauth.errors[0].message).to.equal('OAuth and Personal Tokens are not allowed for this route');

const personalToken = await fakePersonalToken({ UserId: user.id });
const resultPersonalToken = await graphqlQueryV2(
confirmEmailMutation,
{ token: user.emailConfirmationToken },
user,
null,
null,
null,
personalToken,
);
expect(resultPersonalToken.errors).to.exist;
expect(resultPersonalToken.errors[0].message).to.equal(
'OAuth and Personal Tokens are not allowed for this route',
);
});

it('should confirm the new email', async () => {
const newEmail = randEmail();
const user = await fakeUser({ emailWaitingForValidation: newEmail, emailConfirmationToken: randStr() });
const result = await graphqlQueryV2(confirmEmailMutation, { token: user.emailConfirmationToken }); // Unauthenticated
expect(result.errors).to.not.exist;
expect(result.data.confirmEmail.sessionToken).to.not.exist; // Do not log in if not authenticated already
expect(result.data.confirmEmail.individual.slug).to.equal(user.collective.slug);

await user.reload();
expect(user.email).to.equal(newEmail);
expect(user.emailWaitingForValidation).to.be.null;
expect(user.emailConfirmationToken).to.be.null;
});

it('should confirm the new email and return a session token if logged in', async () => {
const newEmail = randEmail();
const user = await fakeUser({ emailWaitingForValidation: newEmail, emailConfirmationToken: randStr() });
const result = await graphqlQueryV2(confirmEmailMutation, { token: user.emailConfirmationToken }, user); // Authenticated
expect(result.errors).to.not.exist;
expect(result.data.confirmEmail.sessionToken).to.exist;
expect(result.data.confirmEmail.individual.slug).to.equal(user.collective.slug);

await user.reload();
expect(user.email).to.equal(newEmail);
expect(user.emailWaitingForValidation).to.be.null;
expect(user.emailConfirmationToken).to.be.null;

const decodedToken = jwt.decode(result.data.confirmEmail.sessionToken, { complete: true });
expect(decodedToken).to.containSubset({
header: { alg: 'HS256', typ: 'JWT', kid: 'HS256-2019-09-02' },
payload: {
scope: 'session',
email: newEmail,
sub: user.id.toString(),
},
});
});
});
});

0 comments on commit 998d590

Please sign in to comment.