Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

task(auth): Use nest di #17412

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libs/shared/mozlog/src/lib/mozlog.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* 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 { Injectable, Scope } from '@nestjs/common';
import { Inject, Injectable, Scope } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import mozlog, { Logger as MozLogger, LoggerFactory } from 'mozlog';

Expand All @@ -12,7 +12,7 @@ let logFactory: LoggerFactory;
export class MozLoggerService {
private mozlog: MozLogger;

constructor(configService: ConfigService) {
constructor(@Inject(ConfigService) configService: ConfigService) {
if (!logFactory) {
logFactory = mozlog(configService.get('log'));
}
Expand Down
4 changes: 2 additions & 2 deletions libs/shared/notifier/src/lib/notifier.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export class NotifierService {
private readonly config: NotifierSnsConfig;

constructor(
configService: ConfigService,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is @Inject(...) going to be necessary for everything consumed by Auth? We haven't had to do that over on the Subplat side so far as long as something is marked as @Injectable, is that still the case?

Copy link
Contributor Author

@dschom dschom Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I got an error without it which is a little strange, because as you point out it doesn't seem to have been necessary in other projects. I wonder if there's a better solution. I can try removing it, and see if I can get it working without adding injects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is apparently because esbuild does not properly support emitting metadata, so the decorators are only spotted and the information available in some parts:
evanw/esbuild#257

Switching to swc to run this should remedy it. Sigh.

private readonly log: MozLoggerService,
@Inject(ConfigService) configService: ConfigService,
@Inject(MozLoggerService) private readonly log: MozLoggerService,
@Inject(NotifierSnsService) private readonly sns: SNS,
@Inject(StatsDService) private readonly statsd: StatsD | undefined
) {
Expand Down
25 changes: 8 additions & 17 deletions packages/fxa-auth-server/bin/key_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
// Important! Must be required first to get proper hooks in place.
require('../lib/monitoring');

// Use nest as a DI framework. This lets us configure and reuse our
// shared libs in hapi as well as nestjs.
const { bridgeTypeDi } = require('../lib/bridge-nestjs');

const { config } = require('../config');

const { CapabilityManager } = require('@fxa/payments/capability');
Expand All @@ -31,7 +35,7 @@ const { Container } = require('typedi');
const { StripeHelper } = require('../lib/payments/stripe');
const { PlayBilling } = require('../lib/payments/iap/google-play');
const { CurrencyHelper } = require('../lib/payments/currencies');
const { AuthLogger, AuthFirestore, AppConfig } = require('../lib/types');
const { AuthLogger, AuthFirestore } = require('../lib/types');
const { setupFirestore } = require('../lib/firestore-db');
const { AppleIAP } = require('../lib/payments/iap/apple-app-store/apple-iap');
const { AccountEventsManager } = require('../lib/account-events');
Expand All @@ -44,22 +48,9 @@ const {
AccountTasksFactory,
} = require('@fxa/shared/cloud-tasks');
async function run(config) {
Container.set(AppConfig, config);

const statsd = config.statsd.enabled
? new StatsD({
...config.statsd,
errorHandler: (err) => {
// eslint-disable-next-line no-use-before-define
log.error('statsd.error', err);
},
})
: {
increment: () => {},
timing: () => {},
close: () => {},
};
Container.set(StatsD, statsd);
// Tranfers DI from nest to typedi
await bridgeTypeDi();
const statsd = Container.get(StatsD);

const log = require('../lib/log')({
...config.log,
Expand Down
18 changes: 18 additions & 0 deletions packages/fxa-auth-server/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,24 @@ const convictConf = convict({
env: 'SNS_TOPIC_ENDPOINT',
default: undefined,
},

notifier: {
sns: {
snsTopicArn: {
doc: 'Amazon SNS topic on which to send account event notifications. Set to "disabled" to turn off the notifier',
format: String,
env: 'SNS_TOPIC_ARN',
default: '',
},

snsTopicEndpoint: {
doc: 'Amazon SNS topic endpoint',
format: String,
env: 'SNS_TOPIC_ENDPOINT',
default: undefined,
},
},
},
emailNotifications: {
region: {
doc: 'The region where the queues live, most likely the same region we are sending email e.g. us-east-1, us-west-2',
Expand Down
49 changes: 49 additions & 0 deletions packages/fxa-auth-server/lib/bridge-nestjs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* 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 { Module } from '@nestjs/common';
import { NestFactory } from '@nestjs/core';
import { ConfigModule } from '@nestjs/config';
import { Container } from 'typedi';
import { MozLoggerService } from '@fxa/shared/mozlog';
import {
LegacyStatsDProvider,
StatsD,
StatsDService,
} from '@fxa/shared/metrics/statsd';
import { NotifierService, NotifierSnsFactory } from '@fxa/shared/notifier';

import config, { ConfigType } from '../config';
import { AppConfig } from './types';

@Module({
imports: [
ConfigModule.forRoot({
load: [(): ConfigType => config.getProperties()],
isGlobal: true,
}),
],
controllers: [],
providers: [
MozLoggerService,
NotifierSnsFactory,
NotifierService,
LegacyStatsDProvider,
],
})
export class AppModule {}

export async function getAppModuleInstance() {
return await NestFactory.createApplicationContext(AppModule);
}

export async function bridgeTypeDi() {
const appInstance = await getAppModuleInstance();

// Setup type di container for backwards compatibility
Container.set(AppConfig, config.getProperties());
Container.set(StatsD, appInstance.get(StatsDService));
Container.set(MozLoggerService, appInstance.resolve(MozLoggerService));
Container.set(NotifierService, appInstance.get(NotifierService));
}
75 changes: 9 additions & 66 deletions packages/fxa-auth-server/lib/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,80 +3,23 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

'use strict';
import { Container } from 'typedi';
import { NotifierService } from '../../../libs/shared/notifier/src';

/**
* This notifier is called by the logger via `notifyAttachedServices`
* to send notifications to Amazon SNS/SQS.
*/
const AWS = require('aws-sdk');
const { config } = require('../config');

const notifierSnsTopicArn = config.get('snsTopicArn');
const notifierSnsTopicEndpoint = config.get('snsTopicEndpoint');
let sns = {
publish: function (msg, cb) {
cb(null, { disabled: true });
},
};

if (notifierSnsTopicArn !== 'disabled') {
// Pull the region info out of the topic arn.
// For some reason we need to pass this in explicitly.
// Format is "arn:aws:sns:<region>:<other junk>"
const region = notifierSnsTopicArn.split(':')[3];
// This will pull in default credentials, region data etc
// from the metadata service available to the instance.
// It's magic, and it's awesome.
sns = new AWS.SNS({ endpoint: notifierSnsTopicEndpoint, region: region });
}

function formatMessageAttributes(msg) {
const attrs = {};
attrs.event_type = {
DataType: 'String',
StringValue: msg.event,
};
if (msg.email) {
attrs.email_domain = {
DataType: 'String',
StringValue: msg.email.split('@')[1],
};
}
return attrs;
}

module.exports = function notifierLog(log, statsd) {
return {
send: (event, callback) => {
const msg = event.data || {};
msg.event = event.event;

const startTime = Date.now();

sns.publish(
{
TopicArn: notifierSnsTopicArn,
Message: JSON.stringify(msg),
MessageAttributes: formatMessageAttributes(msg),
},
(err, data) => {
if (statsd) {
statsd.timing('notifier.publish', Date.now() - startTime);
}

if (err) {
log.error('Notifier.publish', { err: err });
} else {
log.trace('Notifier.publish', { success: true, data: data });
}

if (callback) {
callback(err, data);
}
}
);
if (!Container.has(NotifierService)) {
throw new Error(
'Missing Notifier Service! Was it registered at startup?'
);
}
const notifier = Container.get(NotifierService);
return notifier.send(event, callback);
},
// exported for testing purposes
__sns: sns,
};
};
2 changes: 1 addition & 1 deletion packages/fxa-auth-server/lib/routes/totp.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ module.exports = (log, db, mailer, customs, config, glean, profileClient) => {

await profileClient.deleteCache(uid);
await log.notifyAttachedServices('profileDataChange', request, {
uid
uid,
});
}

Expand Down
1 change: 1 addition & 0 deletions packages/fxa-auth-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"@googlemaps/google-maps-services-js": "^3.4.0",
"@hapi/hapi": "^20.2.1",
"@hapi/hoek": "^11.0.2",
"@nest/core": "^4.5.4",
"@types/convict": "5.2.2",
"@types/ejs": "^3.0.6",
"@types/mjml": "^4.7.4",
Expand Down
Loading