Skip to content

Commit 3d232d8

Browse files
RI-7200 add request metadata to session metadata and exclude it from … (#4771)
* RI-7200 add request metadata to session metadata and exclude it from logs * RI-7187 fix tests + add logs data -> plain transformer
1 parent 7a8873a commit 3d232d8

File tree

8 files changed

+215
-21
lines changed

8 files changed

+215
-21
lines changed

redisinsight/api/src/common/decorators/client-metadata/client-metadata.decorator.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export const clientMetadataParamFactory = (
4646
db: options?.ignoreDbIndex
4747
? undefined
4848
: req?.headers?.[API_HEADER_DATABASE_INDEX],
49+
}, {
50+
groups: ['security'],
4951
});
5052

5153
const errors = validator.validateSync(clientMetadata, {

redisinsight/api/src/common/decorators/session/session-metadata.decorator.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,17 @@ export const sessionMetadataFromRequest = (
2424
'correlationId',
2525
]);
2626
const correlationId = request.res?.locals?.session?.correlationId || uuidv4();
27+
const requestMetadata = request.res?.locals?.session?.requestMetadata;
2728

2829
const requestSession = {
2930
userId,
3031
data,
3132
sessionId,
3233
correlationId,
34+
requestMetadata,
3335
};
3436

35-
// todo: do not forget to deal with session vs sessionMetadata property
36-
const session = plainToInstance(SessionMetadata, requestSession);
37+
const session = plainToInstance(SessionMetadata, requestSession, { groups: ['security'] });
3738

3839
const errors = validator.validateSync(session, {
3940
whitelist: false, // we need this to allow additional fields if needed for flexibility

redisinsight/api/src/common/logger/app-logger.spec.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ const getSessionMetadata = () =>
2525
plainToInstance(SessionMetadata, {
2626
userId: '123',
2727
sessionId: 'test-session-id',
28-
});
28+
requestMetadata: {
29+
any: 'data',
30+
},
31+
}, { groups: ['security' ] });
2932

3033
const getClientMetadata = () =>
3134
plainToInstance(ClientMetadata, {
@@ -34,7 +37,7 @@ const getClientMetadata = () =>
3437
context: ClientContext.Browser,
3538
uniqueId: 'unique-id',
3639
db: 1,
37-
});
40+
}, { groups: ['security' ] });
3841

3942
describe('AppLogger', () => {
4043
let logger: AppLogger;
@@ -115,7 +118,10 @@ describe('AppLogger', () => {
115118
...clientMetadata,
116119
sessionMetadata: undefined,
117120
},
118-
sessionMetadata: clientMetadata.sessionMetadata,
121+
sessionMetadata: {
122+
...clientMetadata.sessionMetadata,
123+
requestMetadata: undefined,
124+
},
119125
data: [{ foo: 'bar' }],
120126
error: undefined,
121127
});
@@ -137,7 +143,10 @@ describe('AppLogger', () => {
137143
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
138144
message: 'Test message',
139145
context: 'Test context',
140-
sessionMetadata,
146+
sessionMetadata: {
147+
...sessionMetadata,
148+
requestMetadata: undefined,
149+
},
141150
data: [{ foo: 'bar' }],
142151
error: undefined,
143152
});
@@ -168,7 +177,10 @@ describe('AppLogger', () => {
168177
...clientMetadata,
169178
sessionMetadata: undefined,
170179
},
171-
sessionMetadata: clientMetadata.sessionMetadata,
180+
sessionMetadata: {
181+
...clientMetadata.sessionMetadata,
182+
requestMetadata: undefined,
183+
},
172184
data: [{ foo: 'bar' }],
173185
error,
174186
});

redisinsight/api/src/common/logger/app-logger.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { LoggerService, Injectable } from '@nestjs/common';
22
import { WinstonModule, WinstonModuleOptions } from 'nest-winston';
33
import { cloneDeep, isString } from 'lodash';
44
import { ClientMetadata, SessionMetadata } from 'src/common/models';
5+
import { instanceToPlain } from 'class-transformer';
6+
import { logDataToPlain } from 'src/utils/logsFormatter';
57

68
type LogMeta = object;
79

@@ -106,8 +108,8 @@ export class AppLogger implements LoggerService {
106108
message,
107109
context,
108110
error,
109-
...userMetadata,
110-
data: optionalParamsCopy?.length ? optionalParamsCopy : undefined,
111+
...instanceToPlain(userMetadata),
112+
data: optionalParamsCopy?.length ? logDataToPlain(optionalParamsCopy) : undefined,
111113
};
112114
}
113115

redisinsight/api/src/common/models/client-metadata.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Session, SessionMetadata } from 'src/common/models/session';
1+
import { SessionMetadata } from 'src/common/models/session';
22
import { Type } from 'class-transformer';
33
import {
44
IsEnum,
@@ -23,7 +23,7 @@ export enum ClientContext {
2323

2424
export class ClientMetadata {
2525
@IsNotEmpty()
26-
@Type(() => Session)
26+
@Type(() => SessionMetadata)
2727
sessionMetadata: SessionMetadata;
2828

2929
@IsNotEmpty()

redisinsight/api/src/common/models/session.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { IsNotEmpty, IsObject, IsOptional, IsString } from 'class-validator';
22
import { BadRequestException } from '@nestjs/common';
33
import ERROR_MESSAGES from 'src/constants/error-messages';
4+
import { Expose } from 'class-transformer';
45

56
export interface ISessionMetadata {
67
userId: string;
@@ -9,21 +10,31 @@ export interface ISessionMetadata {
910
}
1011

1112
export class SessionMetadata implements ISessionMetadata {
13+
@Expose()
1214
@IsNotEmpty()
1315
@IsString()
1416
userId: string;
1517

18+
@Expose()
1619
@IsObject()
1720
data?: Record<string, any> = {};
1821

22+
@Expose({ groups: ['security'] })
23+
@IsObject()
24+
@IsOptional()
25+
requestMetadata?: Record<string, any> = {};
26+
27+
@Expose()
1928
@IsNotEmpty()
2029
@IsString()
2130
sessionId: string;
2231

32+
@Expose()
2333
@IsOptional()
2434
@IsString()
2535
uniqueId?: string;
2636

37+
@Expose()
2738
@IsOptional()
2839
@IsString()
2940
correlationId?: string;

redisinsight/api/src/utils/logsFormatter.spec.ts

Lines changed: 126 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,30 @@ import { BadRequestException, NotFoundException } from '@nestjs/common';
22
import { CloudOauthMisconfigurationException } from 'src/modules/cloud/auth/exceptions';
33
import { AxiosError, AxiosHeaders } from 'axios';
44
import { mockSessionMetadata } from 'src/__mocks__';
5-
import { getOriginalErrorCause, sanitizeError, sanitizeErrors } from './logsFormatter';
5+
import {
6+
ClientContext,
7+
ClientMetadata,
8+
SessionMetadata,
9+
} from 'src/common/models';
10+
import {
11+
getOriginalErrorCause,
12+
logDataToPlain,
13+
sanitizeError,
14+
sanitizeErrors,
15+
} from './logsFormatter';
616

717
const simpleError = new Error('Original error');
818
simpleError['some'] = 'field';
9-
const errorWithCause = new NotFoundException('Not found', { cause: simpleError });
10-
const errorWithCauseDepth2 = new BadRequestException('Bad req', { cause: errorWithCause });
11-
const errorWithCauseDepth3 = new CloudOauthMisconfigurationException('Misconfigured', { cause: errorWithCauseDepth2 });
19+
const errorWithCause = new NotFoundException('Not found', {
20+
cause: simpleError,
21+
});
22+
const errorWithCauseDepth2 = new BadRequestException('Bad req', {
23+
cause: errorWithCause,
24+
});
25+
const errorWithCauseDepth3 = new CloudOauthMisconfigurationException(
26+
'Misconfigured',
27+
{ cause: errorWithCauseDepth2 },
28+
);
1229
const axiosError = new AxiosError(
1330
'Request failed with status code 404',
1431
'NOT_FOUND',
@@ -32,6 +49,30 @@ const axiosError = new AxiosError(
3249
},
3350
);
3451

52+
const mockExtendedClientMetadata = Object.assign(new ClientMetadata(), {
53+
databaseId: 'sdb-id',
54+
context: ClientContext.Browser,
55+
sessionMetadata: Object.assign(new SessionMetadata(), {
56+
...mockSessionMetadata,
57+
data: {
58+
some: 'data',
59+
},
60+
requestMetadata: {
61+
some: 'meta',
62+
},
63+
}),
64+
});
65+
66+
const mockExtendedSessionMetadata = Object.assign(new SessionMetadata(), {
67+
...mockSessionMetadata,
68+
data: {
69+
some: 'data 2',
70+
},
71+
requestMetadata: {
72+
some: 'meta 2',
73+
},
74+
});
75+
3576
const mockLogData: any = {
3677
sessionMetadata: mockSessionMetadata,
3778
error: errorWithCauseDepth3,
@@ -57,6 +98,34 @@ const mockLogData: any = {
5798
};
5899
mockLogData.data.push({ circular: mockLogData.data });
59100

101+
const mockUnsafeLog: any = {
102+
clientMetadata: mockExtendedClientMetadata,
103+
error: errorWithCauseDepth3,
104+
data: [
105+
errorWithCauseDepth2,
106+
{
107+
any: [
108+
'other',
109+
{
110+
possible: 'data',
111+
with: [
112+
'nested',
113+
'structure',
114+
errorWithCause,
115+
{
116+
error: simpleError,
117+
},
118+
],
119+
},
120+
mockExtendedSessionMetadata,
121+
],
122+
},
123+
],
124+
};
125+
mockUnsafeLog.data.push(mockExtendedSessionMetadata);
126+
mockUnsafeLog.data[1].any[1].circular = mockExtendedClientMetadata;
127+
mockUnsafeLog.data.push(mockUnsafeLog.data);
128+
60129
describe('logsFormatter', () => {
61130
describe('getOriginalErrorCause', () => {
62131
it('should return last cause in the chain', () => {
@@ -89,7 +158,9 @@ describe('logsFormatter', () => {
89158
});
90159

91160
it('should return sanitized object with a single original cause for nested errors', () => {
92-
expect(sanitizeError(errorWithCauseDepth3, { omitSensitiveData: true })).toEqual({
161+
expect(
162+
sanitizeError(errorWithCauseDepth3, { omitSensitiveData: true }),
163+
).toEqual({
93164
type: 'CloudOauthMisconfigurationException',
94165
message: errorWithCauseDepth3.message,
95166
cause: {
@@ -174,4 +245,54 @@ describe('logsFormatter', () => {
174245
});
175246
});
176247
});
248+
249+
describe('logDataToPlain', () => {
250+
it('should sanitize all errors and replace circular dependencies after safeTransform of the data', () => {
251+
const result: any = logDataToPlain(mockUnsafeLog);
252+
253+
// should return error instances untouched
254+
expect(result.error).toBeInstanceOf(CloudOauthMisconfigurationException);
255+
expect(result.data[0]).toBeInstanceOf(BadRequestException);
256+
expect(result.data[1].any[1].with[2]).toBeInstanceOf(NotFoundException);
257+
expect(result.data[1].any[1].with[3].error).toBeInstanceOf(Error);
258+
259+
// should sanitize sessionMetadata instances and convert them to plain objects
260+
expect(result).toEqual({
261+
clientMetadata: {
262+
...mockExtendedClientMetadata,
263+
sessionMetadata: {
264+
...mockExtendedClientMetadata.sessionMetadata,
265+
requestMetadata: undefined,
266+
},
267+
},
268+
error: errorWithCauseDepth3,
269+
data: [
270+
errorWithCauseDepth2,
271+
{
272+
any: [
273+
'other',
274+
{
275+
circular: '[Circular]',
276+
possible: 'data',
277+
with: [
278+
'nested',
279+
'structure',
280+
errorWithCause,
281+
{
282+
error: simpleError,
283+
},
284+
],
285+
},
286+
{
287+
...mockExtendedSessionMetadata,
288+
requestMetadata: undefined,
289+
},
290+
],
291+
},
292+
'[Circular]',
293+
'[Circular]',
294+
],
295+
});
296+
});
297+
});
177298
});

0 commit comments

Comments
 (0)