Skip to content

Commit

Permalink
Merge pull request #5327 from flexion/devex-move-logger-intermediate-…
Browse files Browse the repository at this point in the history
…branch-to-test-1725486587

Devex Make explicit unified logger
  • Loading branch information
zachrog committed Sep 5, 2024
2 parents db38cc0 + d3384e0 commit 3d438e1
Show file tree
Hide file tree
Showing 19 changed files with 248 additions and 313 deletions.
4 changes: 2 additions & 2 deletions web-api/src/app-public.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { createApplicationContext } from './applicationContext';
import { expressLogger } from './logger';
import { get } from './persistence/dynamodbClientService';
import { getCurrentInvoke } from '@vendia/serverless-express';
import { json, urlencoded } from 'body-parser';
import { lambdaWrapper } from './lambdaWrapper';
import { logger } from './logger';
import { set } from 'lodash';
import cors from 'cors';
import express from 'express';
Expand Down Expand Up @@ -67,7 +67,7 @@ app.use((req, res, next) => {

next();
});
app.use(logger());
app.use(expressLogger);

import { advancedQueryLimiter } from './middleware/advancedQueryLimiter';
import { casePublicSearchLambda } from './lambdas/public-api/casePublicSearchLambda';
Expand Down
4 changes: 2 additions & 2 deletions web-api/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { downloadPolicyUrlLambda } from './lambdas/documents/downloadPolicyUrlLa
import { editPaperFilingLambda } from './lambdas/documents/editPaperFilingLambda';
import { editPractitionerDocumentLambda } from './lambdas/practitioners/editPractitionerDocumentLambda';
import { exportPendingReportLambda } from '@web-api/lambdas/pendingItems/exportPendingReportLambda';
import { expressLogger } from './logger';
import { fetchPendingItemsLambda } from './lambdas/pendingItems/fetchPendingItemsLambda';
import { fileAndServeCourtIssuedDocumentLambda } from './lambdas/cases/fileAndServeCourtIssuedDocumentLambda';
import { fileCorrespondenceDocumentLambda } from './lambdas/correspondence/fileCorrespondenceDocumentLambda';
Expand Down Expand Up @@ -127,7 +128,6 @@ import { getUsersPendingEmailLambda } from './lambdas/users/getUsersPendingEmail
import { getWorkItemLambda } from './lambdas/workitems/getWorkItemLambda';
import { ipLimiter } from './middleware/ipLimiter';
import { lambdaWrapper } from './lambdaWrapper';
import { logger } from './logger';
import { loginLambda } from '@web-api/lambdas/auth/loginLambda';
import { opinionAdvancedSearchLambda } from './lambdas/documents/opinionAdvancedSearchLambda';
import { orderAdvancedSearchLambda } from './lambdas/documents/orderAdvancedSearchLambda';
Expand Down Expand Up @@ -276,7 +276,7 @@ app.use((req, res, next) => {

next();
});
app.use(logger());
app.use(expressLogger);

/**
* Important note: order of routes DOES matter!
Expand Down
26 changes: 4 additions & 22 deletions web-api/src/applicationContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { UserCase } from '../../shared/src/business/entities/UserCase';
import { UserCaseNote } from '../../shared/src/business/entities/notes/UserCaseNote';
import { WorkItem } from '../../shared/src/business/entities/WorkItem';
import { WorkerMessage } from '@web-api/gateways/worker/workerRouter';
import { createLogger } from './createLogger';
import { defaultProvider } from '@aws-sdk/credential-provider-node';
import { environment } from '@web-api/environment';
import {
Expand All @@ -50,6 +49,7 @@ import { getDocumentGenerators } from './getDocumentGenerators';
import { getDynamoClient } from '@web-api/persistence/dynamo/getDynamoClient';
import { getEmailClient } from './persistence/messages/getEmailClient';
import { getEnvironment, getUniqueId } from '../../shared/src/sharedAppContext';
import { getLogger } from '@web-api/utilities/logger/getLogger';
import { getNotificationClient } from '@web-api/notifications/getNotificationClient';
import { getNotificationService } from '@web-api/notifications/getNotificationService';
import { getPersistenceGateway } from './getPersistenceGateway';
Expand All @@ -72,7 +72,6 @@ import { sendSetTrialSessionCalendarEvent } from './persistence/messages/sendSet
import { sendSlackNotification } from './dispatchers/slack/sendSlackNotification';
import { worker } from '@web-api/gateways/worker/worker';
import { workerLocal } from '@web-api/gateways/worker/workerLocal';

import axios from 'axios';
import pug from 'pug';
import sass from 'sass';
Expand All @@ -97,20 +96,8 @@ const entitiesByName = {
WorkItem,
};

export const createApplicationContext = (
appContextUser = {},
logger = createLogger(),
) => {
const user = new User(appContextUser);

if (process.env.NODE_ENV === 'production') {
const authenticated = user && Object.keys(user).length;
logger.defaultMeta = logger.defaultMeta || {};
logger.defaultMeta.user = authenticated
? user
: { role: 'unauthenticated' };
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const createApplicationContext = (appContextUser = {}) => {
return {
barNumberGenerator,
docketNumberGenerator,
Expand Down Expand Up @@ -282,12 +269,7 @@ export const createApplicationContext = (
}),
isAuthorized,
isCurrentColorActive,
logger: {
debug: (message, context?) => logger.debug(message, { context }),
error: (message, context?) => logger.error(message, { context }),
info: (message, context?) => logger.info(message, { context }),
warn: (message, context?) => logger.warn(message, { context }),
},
logger: getLogger(),
setTimeout: (callback, timeout) => setTimeout(callback, timeout),
};
};
Expand Down
14 changes: 10 additions & 4 deletions web-api/src/createLogger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { cloneDeep, isEqual, unset } from 'lodash';
import {
LoggerOptions,
config,
createLogger as createWinstonLogger,
format,
transports,
} from 'winston';
import { cloneDeep, isEqual, unset } from 'lodash';
import colorize from 'logform/colorize';
import combine from 'logform/combine';
import errors from 'logform/errors';
Expand Down Expand Up @@ -37,12 +38,17 @@ export const removeDuplicateLogInformation = format(logEntry => {
return copy;
});

export const createLogger = (opts = {}) => {
export const createLogger = (opts: LoggerOptions = {}) => {
const options = {
defaultMeta: {},
level: opts.logLevel || process.env.LOG_LEVEL || 'debug',
level: opts.level || process.env.LOG_LEVEL || 'debug',
levels: config.syslog.levels,
transports: [new transports.Console()],
transports: [
new transports.Console({
handleExceptions: true,
handleRejections: true,
}),
],
...opts,
};

Expand Down
4 changes: 3 additions & 1 deletion web-api/src/gateways/worker/workerLocal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import {
WorkerMessage,
workerRouter,
} from '@web-api/gateways/worker/workerRouter';
import { getLogger } from '@web-api/utilities/logger/getLogger';

export const workerLocal: WorkerHandler = async (
applicationContext: ServerApplicationContext,
{ message }: { message: WorkerMessage },
): Promise<void> => {
// Simulate what happens on a deployed environment when a message is sent to SQS.
const appContext = createApplicationContext(message.authorizedUser);
const appContext = createApplicationContext();
getLogger().addUser({ user: message.authorizedUser });
setTimeout(
async () => {
try {
Expand Down
4 changes: 3 additions & 1 deletion web-api/src/genericHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getUserFromAuthHeader,
handle,
} from './middleware/apiGatewayHelper';
import { getLogger } from '@web-api/utilities/logger/getLogger';

export const dataSecurityFilter = (
data,
Expand Down Expand Up @@ -85,7 +86,8 @@ export const genericHandler = (
return handle(awsEvent, async () => {
const user = getUserFromAuthHeader(awsEvent);
const clientConnectionId = getConnectionIdFromEvent(awsEvent);
const applicationContext = createApplicationContext(user, awsEvent.logger);
const applicationContext = createApplicationContext();
getLogger().addUser({ user });

delete awsEvent.logger;

Expand Down
4 changes: 3 additions & 1 deletion web-api/src/lambdas/cases/sealInLowerEnvironmentLambda.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { AuthUser } from '@shared/business/entities/authUser/AuthUser';
import { createApplicationContext } from '../../applicationContext';
import { getLogger } from '@web-api/utilities/logger/getLogger';
import { sealInLowerEnvironment } from '@web-api/business/useCaseHelper/sealInLowerEnvironment';

/**
Expand All @@ -16,7 +17,8 @@ export const sealInLowerEnvironmentLambda = async event => {
userId: 'N/A',
};

const applicationContext = createApplicationContext(user);
const applicationContext = createApplicationContext();
getLogger().addUser({ user });

const records = event.Records.map(record => ({
...JSON.parse(record.Sns.Message),
Expand Down
102 changes: 30 additions & 72 deletions web-api/src/lambdas/cognitoAuthorizer/cognito-authorizer.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { createLogger } from '../../createLogger';
import { handler } from './cognito-authorizer';
import { transports } from 'winston';
import axios from 'axios';
import fs from 'fs';
import jwk from 'jsonwebtoken';

const { createLogger: actualCreateLogger } = jest.requireActual(
'../../../src/createLogger',
);
jest.mock('../../../src/createLogger', () => {
return { createLogger: jest.fn() };
const mockLogger = {
addContext: jest.fn(),
clearContext: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
};
jest.mock('@web-api/utilities/logger/getLogger', () => {
return {
getLogger: () => mockLogger,
};
});
jest.mock('jsonwebtoken', () => {
return {
Expand Down Expand Up @@ -48,18 +50,9 @@ describe('cognito-authorizer', () => {
});
};

let event, context, transport;
let event, context;

beforeEach(() => {
transport = new transports.Stream({
stream: fs.createWriteStream('/dev/null'),
});

(createLogger as jest.Mock).mockImplementation(opts => {
opts.transports = [transport];
return actualCreateLogger(opts);
});

event = {
authorizationToken:
'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImFkbWlzc2lvbnNjbGVya0BleGFtcGxlLmNvbSIsIm5hbWUiOiJUZXN0IEFkbWlzc2lvbnMgQ2xlcmsiLCJyb2xlIjoiYWRtaXNzaW9uc2NsZXJrIiwic2VjdGlvbiI6ImFkbWlzc2lvbnMiLCJ1c2VySWQiOiI5ZDdkNjNiNy1kN2E1LTQ5MDUtYmE4OS1lZjcxYmYzMDA1N2YiLCJjdXN0b206cm9sZSI6ImFkbWlzc2lvbnNjbGVyayIsInN1YiI6IjlkN2Q2M2I3LWQ3YTUtNDkwNS1iYTg5LWVmNzFiZjMwMDU3ZiIsImlhdCI6MTYwOTQ0NTUyNn0.kow3pAUloDseD3isrxgtKBpcKsjMktbRBzY41c1NRqA',
Expand All @@ -79,20 +72,15 @@ describe('cognito-authorizer', () => {
});

jest.spyOn(axios, 'get');
jest.spyOn(transport, 'log');
});

it('returns unauthorized when token is missing', async () => {
event.authorizationToken = '';

await expect(() => handler(event, context)).rejects.toThrow('Unauthorized');

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('info'),
message: expect.stringContaining('No authorizationToken found'),
}),
expect.any(Function),
expect(mockLogger.info).toHaveBeenCalledWith(
'No authorizationToken found in the header',
);
});

Expand All @@ -103,15 +91,9 @@ describe('cognito-authorizer', () => {

await expect(() => handler(event, context)).rejects.toThrow('Unauthorized');

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('warning'),
message: expect.stringContaining(
'Could not fetch keys for token issuer',
),
stack: expect.stringContaining('Error: any error'),
}),
expect.any(Function),
expect(mockLogger.warn).toHaveBeenCalledWith(
'Could not fetch keys for token issuer, considering request unauthorized',
new Error('any error'),
);
});

Expand All @@ -122,15 +104,9 @@ describe('cognito-authorizer', () => {

await expect(() => handler(event, context)).rejects.toThrow('Unauthorized');

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('warning'),
message: expect.stringContaining(
'Could not fetch keys for token issuer',
),
stack: expect.any(String),
}),
expect.any(Function),
expect(mockLogger.warn).toHaveBeenCalledWith(
'Could not fetch keys for token issuer, considering request unauthorized',
expect.anything(),
);
});

Expand All @@ -143,17 +119,13 @@ describe('cognito-authorizer', () => {

await expect(() => handler(event, context)).rejects.toThrow('Unauthorized');

expect(transport.log).toHaveBeenCalledWith(
expect(mockLogger.warn).toHaveBeenCalledWith(
'The key used to sign the authorization token was not found in the user pool’s keys, considering request unauthorized',
expect.objectContaining({
issuer: expect.any(String),
keys: expect.any(Array),
level: expect.stringContaining('warning'),
message: expect.stringContaining(
'was not found in the user pool’s keys',
),
requestedKeyId: 'key-identifier',
}),
expect.any(Function),
);
});

Expand All @@ -175,13 +147,9 @@ describe('cognito-authorizer', () => {

await expect(() => handler(event, context)).rejects.toThrow('Unauthorized');

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('warning'),
message: expect.stringContaining('token is not valid'),
stack: expect.stringContaining('Error: verification failed'),
}),
expect.any(Function),
expect(mockLogger.warn).toHaveBeenCalledWith(
'The token is not valid, considering request unauthorized',
new Error('verification failed'),
);
});

Expand All @@ -207,14 +175,9 @@ describe('cognito-authorizer', () => {
}),
);

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('info'),
message: expect.stringContaining('Request authorized'),
metadata: expect.objectContaining({ policy }),
}),
expect.any(Function),
);
expect(mockLogger.info).toHaveBeenCalledWith('Request authorized', {
metadata: { policy },
});
});

it('returns IAM policy to allow invoking requested lambda when authorized using the payload custom:userId', async () => {
Expand All @@ -239,14 +202,9 @@ describe('cognito-authorizer', () => {
}),
);

expect(transport.log).toHaveBeenCalledWith(
expect.objectContaining({
level: expect.stringContaining('info'),
message: expect.stringContaining('Request authorized'),
metadata: expect.objectContaining({ policy }),
}),
expect.any(Function),
);
expect(mockLogger.info).toHaveBeenCalledWith('Request authorized', {
metadata: { policy },
});
});

it('caches keys for issuers', async () => {
Expand Down
Loading

0 comments on commit 3d438e1

Please sign in to comment.