Merge pull request #17310 from mozilla/clear-profile-cache

task(auth,graphql,profile,admin): Make sure profile cache is cleared before sending profile update event.
This commit is contained in:
Dan Schomburg 2024-08-30 09:13:57 -07:00 коммит произвёл GitHub
Родитель 0f699a3c67 cbe87a79f7
Коммит 84118b1168
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
19 изменённых файлов: 237 добавлений и 22 удалений

Просмотреть файл

@ -7,6 +7,7 @@ import { AuthClientFactory, AuthClientService } from './auth-client.service';
import { FirestoreFactory, FirestoreService } from './firestore.service';
import { CloudTasksFactory, CloudTasksService } from './cloud-tasks.service';
import { MetricsFactory } from 'fxa-shared/nestjs/metrics.service';
import { ProfileClientService } from './profile-client.service';
@Module({
providers: [
@ -14,7 +15,13 @@ import { MetricsFactory } from 'fxa-shared/nestjs/metrics.service';
FirestoreFactory,
CloudTasksFactory,
MetricsFactory,
ProfileClientService,
],
exports: [
AuthClientService,
FirestoreService,
CloudTasksService,
ProfileClientService,
],
exports: [AuthClientService, FirestoreService, CloudTasksService],
})
export class BackendModule {}

Просмотреть файл

@ -0,0 +1,60 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
import { Provider } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import superagent from 'superagent';
import { AuthClientService } from './auth-client.service';
import { ProfileClientService } from './profile-client.service';
import { ConfigService } from '@nestjs/config';
describe('ProfileClientService', () => {
let service: ProfileClientService;
let authClient: any;
const profileUrl = 'https://test.com';
const secretToken = 'secret123';
beforeEach(async () => {
authClient = {};
const MockConfig: Provider = {
provide: ConfigService,
useValue: {
get: jest.fn().mockImplementation((key: string) => {
if (key === 'profileServer') {
return {
url: profileUrl,
secretBearerToken: secretToken,
};
}
return null;
}),
},
};
const module: TestingModule = await Test.createTestingModule({
providers: [
ProfileClientService,
MockConfig,
{ provide: AuthClientService, useValue: authClient },
],
}).compile();
service = module.get<ProfileClientService>(ProfileClientService);
});
it('should be defined', () => {
expect(service).toBeDefined();
});
it('deletes cache', async () => {
const mockDelete = jest.spyOn(superagent, 'delete') as jest.Mock;
const mockSet = jest.fn().mockResolvedValue({ text: '{}' });
mockDelete.mockReturnValueOnce({
set: mockSet,
});
const result = await service.deleteCache('1234abcd');
expect(result).toStrictEqual({});
expect(mockSet).toBeCalledWith('Authorization', 'Bearer ' + secretToken);
expect(mockDelete).toBeCalledWith(profileUrl + '/cache/1234abcd');
});
});

Просмотреть файл

@ -0,0 +1,34 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
import { Inject, Injectable } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import AuthClient from 'fxa-auth-client';
import superagent from 'superagent';
import { AppConfig } from '../config';
import { AuthClientService } from './auth-client.service';
@Injectable()
export class ProfileClientService {
private profileServerUrl: string;
private profileServerToken: string;
constructor(
configService: ConfigService<AppConfig>,
@Inject(AuthClientService) private authAPI: AuthClient
) {
const profileConfig = configService.get(
'profileServer'
) as AppConfig['profileServer'];
this.profileServerUrl = profileConfig.url;
this.profileServerToken = profileConfig.secretBearerToken;
}
async deleteCache(uid: string) {
const result = await superagent
.delete(this.profileServerUrl + `/cache/${uid}`)
.set('Authorization', 'Bearer ' + this.profileServerToken);
return JSON.parse(result.text);
}
}

Просмотреть файл

@ -19,5 +19,9 @@
"queueName": "delete-accounts-queue",
"taskUrl": "http://host.docker.internal:9000/v1/cloud-tasks/accounts/delete"
}
},
"profileServer": {
"url": "http://localhost:1111",
"secretBearerToken": "8675309jenny"
}
}

Просмотреть файл

@ -602,6 +602,19 @@ const conf = convict({
},
},
},
profileServer: {
url: {
doc: 'URL of fxa-profile-server',
env: 'PROFILE_SERVER_URL',
default: 'http://localhost:1111/v1',
},
secretBearerToken: {
default: 'YOU MUST CHANGE ME',
doc: 'Secret for server-to-server bearer token auth for fxa-profile-server',
env: 'PROFILE_SERVER_AUTH_SECRET_BEARER_TOKEN',
format: 'String',
},
},
});
const configDir = __dirname;

Просмотреть файл

@ -46,6 +46,7 @@ import { FirestoreService } from '../../backend/firestore.service';
import { BasketService } from '../../newsletters/basket.service';
import { CloudTasksService } from '../../backend/cloud-tasks.service';
import { NotifierService } from '@fxa/shared/notifier';
import { ProfileClientService } from '../../backend/profile-client.service';
export const chance = new Chance();
@ -70,6 +71,7 @@ describe('#integration - AccountResolver', () => {
let resolver: AccountResolver;
let logger: any;
let notifier: any;
let profileClient: any;
let knex: Knex;
let db = {
account: Account,
@ -130,6 +132,12 @@ describe('#integration - AccountResolver', () => {
useValue: notifier,
};
profileClient = { deleteCache: jest.fn() };
const MockProfileClient: Provider = {
provide: ProfileClientService,
useValue: profileClient,
};
logger = { debug: jest.fn(), error: jest.fn(), info: jest.fn() };
const MockMozLogger: Provider = {
provide: MozLoggerService,
@ -249,6 +257,7 @@ describe('#integration - AccountResolver', () => {
MockAuthClient,
MockCloudTasks,
MockNotifier,
MockProfileClient,
],
}).compile();
@ -281,6 +290,7 @@ describe('#integration - AccountResolver', () => {
expect(updatedResult).toBeDefined();
expect(updatedResult.disabledAt).not.toBeNull();
expect(logger.info).toBeCalledTimes(3);
expect(profileClient.deleteCache).toBeCalledWith(USER_1.uid);
expect(notifier.send).toBeCalledWith({
event: 'profileDataChange',
data: {
@ -300,6 +310,7 @@ describe('#integration - AccountResolver', () => {
expect(updatedResult).toBeDefined();
expect(updatedResult.disabledAt).toBeNull();
expect(logger.info).toBeCalledTimes(2);
expect(profileClient.deleteCache).toBeCalledWith(USER_1.uid);
expect(notifier.send).toBeCalledWith({
event: 'profileDataChange',
data: {
@ -444,6 +455,7 @@ describe('#integration - AccountResolver', () => {
(await resolver.accountByEmail(USER_1.email, true, 'joe')) || ({} as any);
expect(result1).toBe(true);
expect(result2.locale).toBe('en-CA');
expect(profileClient.deleteCache).toBeCalledWith(USER_1.uid);
expect(notifier.send).toBeCalledWith({
event: 'profileDataChange',
data: {

Просмотреть файл

@ -53,6 +53,7 @@ import {
AccountDeleteTaskStatus,
} from '../model/account-delete-task.model';
import { NotifierService } from '@fxa/shared/notifier';
import { ProfileClientService } from '../../backend/profile-client.service';
const ACCOUNT_COLUMNS = [
'uid',
@ -116,7 +117,8 @@ export class AccountResolver {
private notifier: NotifierService,
@Inject(AuthClientService) private authAPI: AuthClient,
@Inject(FirestoreService) private firestore: Firestore,
@Inject(CloudTasksService) private cloudTask: CloudTasks
@Inject(CloudTasksService) private cloudTask: CloudTasks,
@Inject(ProfileClientService) private profileClient: ProfileClientService
) {}
@Features(AdminPanelFeature.AccountSearch)
@ -198,6 +200,7 @@ export class AccountResolver {
@Mutation((returns) => Boolean)
public async disableAccount(@Args('uid') uid: string) {
this.eventLogging.onEvent(EventNames.DisableLogin);
await this.profileClient.deleteCache(uid);
await this.notifier.send({
event: 'profileDataChange',
data: {
@ -226,6 +229,7 @@ export class AccountResolver {
.update({ locale: locale })
.where('uid', uidBuffer);
await this.profileClient.deleteCache(uid);
await this.notifier.send({
event: 'profileDataChange',
data: {
@ -246,6 +250,7 @@ export class AccountResolver {
.update({ disabledAt: null } as any)
.where('uid', uidBuffer);
await this.profileClient.deleteCache(uid);
await this.notifier.send({
event: 'profileDataChange',
data: {

Просмотреть файл

@ -35,7 +35,7 @@ import {
} from '../payments/iap/iap-formatter';
import { PayPalHelper } from '../payments/paypal/helper';
import { StripeHelper } from '../payments/stripe';
import { AuthLogger, AuthRequest } from '../types';
import { AuthLogger, AuthRequest, ProfileClient } from '../types';
import { deleteAccountIfUnverified } from './utils/account';
import emailUtils from './utils/email';
import requestHelper from './utils/request_helper';
@ -66,6 +66,7 @@ export class AccountHandler {
private accountEventsManager: AccountEventsManager;
private accountDeleteManager: AccountDeleteManager;
private accountTasks: AccountTasks;
private profileClient: ProfileClient;
constructor(
private log: AuthLogger,
@ -103,6 +104,7 @@ export class AccountHandler {
this.accountEventsManager = Container.get(AccountEventsManager);
this.accountDeleteManager = Container.get(AccountDeleteManager);
this.accountTasks = Container.get(AccountTasks);
this.profileClient = Container.get(ProfileClient);
}
private async generateRandomValues() {
@ -242,6 +244,8 @@ export class AccountHandler {
uid: account.uid,
userAgent: userAgentString,
});
await this.profileClient.deleteCache(account.uid);
await this.log.notifyAttachedServices('profileDataChange', request, {
uid: account.uid,
});
@ -1603,9 +1607,12 @@ export class AccountHandler {
uid: account.uid,
generation: account.verifierSetAt,
}),
this.log.notifyAttachedServices('profileDataChange', request, {
uid: account.uid,
}),
(async () => {
await this.profileClient.deleteCache(account.uid);
await this.log.notifyAttachedServices('profileDataChange', request, {
uid: account.uid,
});
})(),
this.oauth.removeTokensAndCodes(account.uid),
this.customs.reset(request, account.email),
]);

Просмотреть файл

@ -152,7 +152,8 @@ module.exports = function (
db,
config.domain,
devicesImpl,
config
config,
profile
);
const unblockCodes = require('./unblock-codes')(
log,
@ -161,7 +162,15 @@ module.exports = function (
config.signinUnblock,
customs
);
const totp = require('./totp')(log, db, mailer, customs, config.totp, glean);
const totp = require('./totp')(
log,
db,
mailer,
customs,
config.totp,
glean,
profile
);
const recoveryCodes = require('./recovery-codes')(
log,
db,

Просмотреть файл

@ -11,7 +11,7 @@ const SIGN_DOCS = require('../../docs/swagger/sign-api').default;
const DESCRIPTION = require('../../docs/swagger/shared/descriptions').default;
const crypto = require('crypto');
module.exports = (log, signer, db, domain, devices, config) => {
module.exports = (log, signer, db, domain, devices, config, profileClient) => {
const HOUR = 1000 * 60 * 60;
const rolloutRate = config.certificateSignDisableRolloutRate || 0;
@ -176,7 +176,9 @@ module.exports = (log, signer, db, domain, devices, config) => {
locale: request.app.acceptLanguage,
});
db.updateLocale(sessionToken.uid, request.app.acceptLanguage);
log.notifyAttachedServices(
await profileClient.deleteCache(sessionToken.uid);
await log.notifyAttachedServices(
'profileDataChange',
{},
{

Просмотреть файл

@ -16,7 +16,7 @@ const DESCRIPTION = require('../../docs/swagger/shared/descriptions').default;
const { Container } = require('typedi');
const { AccountEventsManager } = require('../account-events');
module.exports = (log, db, mailer, customs, config, glean) => {
module.exports = (log, db, mailer, customs, config, glean, profileClient) => {
const otpUtils = require('../../lib/routes/utils/otp')(log, config, db);
// Currently, QR codes are rendered with the highest possible
@ -155,6 +155,7 @@ module.exports = (log, db, mailer, customs, config, glean) => {
// See #5154.
await db.verifyTokensWithMethod(sessionToken.id, 'email-2fa');
await profileClient.deleteCache(uid);
await log.notifyAttachedServices(
'profileDataChange',
{},
@ -308,8 +309,9 @@ module.exports = (log, db, mailer, customs, config, glean) => {
glean.twoFactorAuth.codeComplete(request, { uid });
await profileClient.deleteCache(uid);
await log.notifyAttachedServices('profileDataChange', request, {
uid: sessionToken.uid,
uid
});
}

Просмотреть файл

@ -32,9 +32,10 @@ const {
const {
deleteAccountIfUnverified,
} = require('../../../lib/routes/utils/account');
const { AppConfig, AuthLogger } = require('../../../lib/types');
const { AppConfig, AuthLogger, ProfileClient } = require('../../../lib/types');
const defaultConfig = require('../../../config').default.getProperties();
const glean = mocks.mockGlean();
const profile = mocks.mockProfile();
const TEST_EMAIL = 'foo@gmail.com';
@ -142,6 +143,8 @@ const makeRoutes = function (options = {}, requireMocks = {}) {
accountManagerMock.quickDelete = mockAccountQuickDelete;
Container.set(AccountDeleteManager, accountManagerMock);
Container.set(ProfileClient, profile);
return accountRoutes(
log,
db,
@ -700,6 +703,9 @@ describe('deleteAccountIfUnverified', () => {
});
describe('/account/create', () => {
beforeEach(() => {
profile.deleteCache.resetHistory();
});
afterEach(() => {
glean.registration.accountCreated.reset();
glean.registration.confirmationEmailSent.reset();
@ -935,6 +941,9 @@ describe('/account/create', () => {
'it contained the correct metrics context metadata'
);
assert.equal(profile.deleteCache.callCount, 1);
assert.equal(profile.deleteCache.getCall(0).args[0], uid);
eventData = mockLog.notifier.send.getCall(1).args[0];
assert.equal(eventData.event, 'profileDataChange');
assert.equal(eventData.data.uid, uid);
@ -1150,6 +1159,10 @@ describe('/account/create', () => {
'foo',
'it was for the expected service'
);
assert.equal(profile.deleteCache.callCount, 1);
assert.equal(profile.deleteCache.getCall(0).args[0], uid);
eventData = mockLog.notifier.send.getCall(1).args[0];
assert.equal(eventData.event, 'profileDataChange');
assert.equal(eventData.data.uid, uid);

Просмотреть файл

@ -501,6 +501,10 @@ describe('DirectStripeRoutes', () => {
'Expected push.notifyProfileUpdated to be called once'
);
assert.isTrue(
directStripeRoutesInstance.profile.deleteCache.calledOnceWith(UID)
);
assert.isTrue(
directStripeRoutesInstance.log.notifyAttachedServices.calledOnceWith(
'profileDataChange',

Просмотреть файл

@ -20,6 +20,7 @@ let log,
request,
requestOptions,
mailer,
profile,
accountEventsManager;
const glean = mocks.mockGlean();
@ -109,7 +110,7 @@ describe('totp', () => {
it('should delete TOTP token in verified session', () => {
requestOptions.credentials.tokenVerified = true;
return setup(
{ db: { email: TEST_EMAIL } },
{ db: { email: TEST_EMAIL }, profile },
{},
'/totp/destroy',
requestOptions
@ -121,6 +122,17 @@ describe('totp', () => {
'called delete TOTP token'
);
assert.equal(
profile.deleteCache.callCount,
1,
'called profile client delete cache'
);
assert.equal(
profile.deleteCache.getCall(0).args[0],
'uid',
'called profile client delete cache'
);
assert.equal(
log.notifyAttachedServices.callCount,
1,
@ -225,6 +237,17 @@ describe('totp', () => {
assert.equal(db.totpToken.callCount, 1, 'called get TOTP token');
assert.equal(db.updateTotpToken.callCount, 1, 'update TOTP token');
assert.equal(
profile.deleteCache.callCount,
1,
'called profile client delete cache'
);
assert.equal(
profile.deleteCache.getCall(0).args[0],
'uid',
'called profile client delete cache'
);
assert.equal(
log.notifyAttachedServices.callCount,
1,
@ -505,6 +528,7 @@ function setup(results, errors, routePath, requestOptions) {
customs = mocks.mockCustoms(errors.customs);
mailer = mocks.mockMailer();
db = mocks.mockDB(results.db, errors.db);
profile = mocks.mockProfile();
db.createTotpToken = sinon.spy(() => {
return Promise.resolve({
qrCodeUrl: 'some base64 encoded png',
@ -527,24 +551,26 @@ function setup(results, errors, routePath, requestOptions) {
sharedSecret: secret,
});
});
routes = makeRoutes({ log, db, customs, mailer, glean });
routes = makeRoutes({ log, db, customs, mailer, glean, profile });
route = getRoute(routes, routePath);
request = mocks.mockRequest(requestOptions);
request.emitMetricsEvent = sinon.spy(() => Promise.resolve({}));
return runTest(route, request);
}
function makeRoutes(options = {}) {
const config = { step: 30, window: 1 };
Container.set(AccountEventsManager, accountEventsManager);
const { log, db, customs, mailer, glean } = options;
const { log, db, customs, mailer, glean, profile } = options;
return require('../../../lib/routes/totp')(
log,
db,
mailer,
customs,
config,
glean
glean,
profile
);
}

Просмотреть файл

@ -38,6 +38,7 @@ module.exports = {
SNS_TOPIC_ARN:
'arn:aws:sns:us-east-1:100010001000:fxa-account-change-dev',
SNS_TOPIC_ENDPOINT: 'http://localhost:4100/',
PROFILE_SERVER_AUTH_SECRET_BEARER_TOKEN: '8675309jenny',
},
filter_env: ['npm_'],
watch: ['src'],

Просмотреть файл

@ -14,6 +14,7 @@ import { AuthClientService } from './auth-client.service';
export class ProfileClientService {
private oauthClientId: string;
private profileServerUrl: string;
private profileServerToken: string;
constructor(
configService: ConfigService<AppConfig>,
@ -25,6 +26,7 @@ export class ProfileClientService {
) as AppConfig['profileServer'];
this.oauthClientId = oauthConfig.clientId;
this.profileServerUrl = profileConfig.url;
this.profileServerToken = profileConfig.secretBearerToken;
}
private async fetchToken(token: string, headers: Headers) {
@ -103,4 +105,11 @@ export class ProfileClientService {
.set('Authorization', 'Bearer ' + accessToken);
return JSON.parse(result.text);
}
async deleteCache(uid: string, headers: Headers) {
const result = await superagent
.delete(this.profileServerUrl + `/cache/${uid}`)
.set('Authorization', 'Bearer ' + this.profileServerToken);
return JSON.parse(result.text);
}
}

Просмотреть файл

@ -166,6 +166,12 @@ const conf = convict({
env: 'PROFILE_SERVER_URL',
default: 'http://localhost:1111/v1',
},
secretBearerToken: {
default: 'YOU MUST CHANGE ME',
doc: 'Secret for server-to-server bearer token auth for fxa-profile-server',
env: 'PROFILE_SERVER_AUTH_SECRET_BEARER_TOKEN',
format: 'String',
},
},
redis: {
accessTokens: {

Просмотреть файл

@ -475,7 +475,7 @@ describe('#integration - AccountResolver', () => {
describe('metricsOpt', () => {
it('opts out', async () => {
const result = await resolver.metricsOpt(USER_1.uid, {
const result = await resolver.metricsOpt(headers, USER_1.uid, {
clientMutationId: 'testid',
state: 'out',
});
@ -494,7 +494,7 @@ describe('#integration - AccountResolver', () => {
});
it('opts in', async () => {
const result = await resolver.metricsOpt(USER_1.uid, {
const result = await resolver.metricsOpt(headers, USER_1.uid, {
clientMutationId: 'testid',
state: 'in',
});
@ -514,7 +514,7 @@ describe('#integration - AccountResolver', () => {
it('fails with bad opt in state', async () => {
await expect(
resolver.metricsOpt(USER_1.uid, {
resolver.metricsOpt(headers, USER_1.uid, {
clientMutationId: 'testid',
state: 'In' as 'in',
})

Просмотреть файл

@ -449,9 +449,9 @@ export class AccountResolver {
@UseGuards(GqlAuthGuard, GqlCustomsGuard)
@CatchGatewayError
public async metricsOpt(
@GqlXHeaders() headers: Headers,
@GqlUserId() uid: string,
@Args('input', { type: () => MetricsOptInput })
input: MetricsOptInput
@Args('input', { type: () => MetricsOptInput }) input: MetricsOptInput
): Promise<BasicPayload> {
await Account.setMetricsOpt(uid, input.state);
@ -461,6 +461,7 @@ export class AccountResolver {
);
}
await this.profileAPI.deleteCache(uid, headers);
await this.notifier.send({
event: 'profileDataChange',
data: {