From 52b3ad6dc3a7d14ccad879159c94416aaebad600 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Sat, 21 Jan 2023 11:46:56 +0100 Subject: [PATCH] Feature/refactor value redaction interceptor (#1624) * Reuse redactAttributes() * Update changelog --- CHANGELOG.md | 1 + .../api/src/app/account/account.controller.ts | 67 +++------------ .../src/app/portfolio/portfolio.controller.ts | 36 +------- apps/api/src/helper/object.helper.ts | 20 +++-- .../redact-values-in-response.interceptor.ts | 84 +++++++------------ 5 files changed, 59 insertions(+), 149 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05bba514..e621c062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed the toggle _Original Shares_ vs. _Current Shares_ on the allocations page - Hid error messages related to no current investment in the client +- Refactored the value redaction interceptor for the impersonation mode ### Fixed diff --git a/apps/api/src/app/account/account.controller.ts b/apps/api/src/app/account/account.controller.ts index 9e170b7f..2a71042c 100644 --- a/apps/api/src/app/account/account.controller.ts +++ b/apps/api/src/app/account/account.controller.ts @@ -1,9 +1,6 @@ import { PortfolioService } from '@ghostfolio/api/app/portfolio/portfolio.service'; import { UserService } from '@ghostfolio/api/app/user/user.service'; -import { - nullifyValuesInObject, - nullifyValuesInObjects -} from '@ghostfolio/api/helper/object.helper'; +import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response.interceptor'; import { ImpersonationService } from '@ghostfolio/api/services/impersonation.service'; import { Accounts } from '@ghostfolio/common/interfaces'; import { hasPermission, permissions } from '@ghostfolio/common/permissions'; @@ -22,7 +19,8 @@ import { Param, Post, Put, - UseGuards + UseGuards, + UseInterceptors } from '@nestjs/common'; import { REQUEST } from '@nestjs/core'; import { AuthGuard } from '@nestjs/passport'; @@ -85,6 +83,7 @@ export class AccountController { @Get() @UseGuards(AuthGuard('jwt')) + @UseInterceptors(RedactValuesInResponseInterceptor) public async getAllAccounts( @Headers('impersonation-id') impersonationId ): Promise { @@ -94,39 +93,15 @@ export class AccountController { this.request.user.id ); - let accountsWithAggregations = - await this.portfolioService.getAccountsWithAggregations({ - userId: impersonationUserId || this.request.user.id, - withExcludedAccounts: true - }); - - if ( - impersonationUserId || - this.userService.isRestrictedView(this.request.user) - ) { - accountsWithAggregations = { - ...nullifyValuesInObject(accountsWithAggregations, [ - 'totalBalanceInBaseCurrency', - 'totalValueInBaseCurrency' - ]), - accounts: nullifyValuesInObjects(accountsWithAggregations.accounts, [ - 'balance', - 'balanceInBaseCurrency', - 'convertedBalance', - 'fee', - 'quantity', - 'unitPrice', - 'value', - 'valueInBaseCurrency' - ]) - }; - } - - return accountsWithAggregations; + return this.portfolioService.getAccountsWithAggregations({ + userId: impersonationUserId || this.request.user.id, + withExcludedAccounts: true + }); } @Get(':id') @UseGuards(AuthGuard('jwt')) + @UseInterceptors(RedactValuesInResponseInterceptor) public async getAccountById( @Headers('impersonation-id') impersonationId, @Param('id') id: string @@ -137,35 +112,13 @@ export class AccountController { this.request.user.id ); - let accountsWithAggregations = + const accountsWithAggregations = await this.portfolioService.getAccountsWithAggregations({ filters: [{ id, type: 'ACCOUNT' }], userId: impersonationUserId || this.request.user.id, withExcludedAccounts: true }); - if ( - impersonationUserId || - this.userService.isRestrictedView(this.request.user) - ) { - accountsWithAggregations = { - ...nullifyValuesInObject(accountsWithAggregations, [ - 'totalBalanceInBaseCurrency', - 'totalValueInBaseCurrency' - ]), - accounts: nullifyValuesInObjects(accountsWithAggregations.accounts, [ - 'balance', - 'balanceInBaseCurrency', - 'convertedBalance', - 'fee', - 'quantity', - 'unitPrice', - 'value', - 'valueInBaseCurrency' - ]) - }; - } - return accountsWithAggregations.accounts[0]; } diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 7879fa4a..e607a5f9 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -356,6 +356,7 @@ export class PortfolioController { @Get('positions') @UseGuards(AuthGuard('jwt')) + @UseInterceptors(RedactValuesInResponseInterceptor) @UseInterceptors(TransformDataSourceInResponseInterceptor) public async getPositions( @Headers('impersonation-id') impersonationId: string, @@ -370,27 +371,11 @@ export class PortfolioController { filterByTags }); - const result = await this.portfolioService.getPositions({ + return this.portfolioService.getPositions({ dateRange, filters, impersonationId }); - - if ( - impersonationId || - this.userService.isRestrictedView(this.request.user) - ) { - result.positions = result.positions.map((position) => { - return nullifyValuesInObject(position, [ - 'grossPerformance', - 'investment', - 'netPerformance', - 'quantity' - ]); - }); - } - - return result; } @Get('public/:accessId') @@ -460,6 +445,7 @@ export class PortfolioController { } @Get('position/:dataSource/:symbol') + @UseInterceptors(RedactValuesInResponseInterceptor) @UseInterceptors(TransformDataSourceInRequestInterceptor) @UseInterceptors(TransformDataSourceInResponseInterceptor) @UseGuards(AuthGuard('jwt')) @@ -468,27 +454,13 @@ export class PortfolioController { @Param('dataSource') dataSource, @Param('symbol') symbol ): Promise { - let position = await this.portfolioService.getPosition( + const position = await this.portfolioService.getPosition( dataSource, impersonationId, symbol ); if (position) { - if ( - impersonationId || - this.userService.isRestrictedView(this.request.user) - ) { - position = nullifyValuesInObject(position, [ - 'grossPerformance', - 'investment', - 'netPerformance', - 'orders', - 'quantity', - 'value' - ]); - } - return position; } diff --git a/apps/api/src/helper/object.helper.ts b/apps/api/src/helper/object.helper.ts index b24ee42e..7ee07b46 100644 --- a/apps/api/src/helper/object.helper.ts +++ b/apps/api/src/helper/object.helper.ts @@ -1,4 +1,4 @@ -import { cloneDeep, isObject } from 'lodash'; +import { cloneDeep, isArray, isObject } from 'lodash'; export function hasNotDefinedValuesInObject(aObject: Object): boolean { for (const key in aObject) { @@ -43,15 +43,23 @@ export function redactAttributes({ for (const option of options) { if (redactedObject.hasOwnProperty(option.attribute)) { - redactedObject[option.attribute] = - option.valueMap[redactedObject[option.attribute]] ?? - option.valueMap['*'] ?? - redactedObject[option.attribute]; + if (option.valueMap['*'] || option.valueMap['*'] === null) { + redactedObject[option.attribute] = option.valueMap['*']; + } else if (option.valueMap[redactedObject[option.attribute]]) { + redactedObject[option.attribute] = + option.valueMap[redactedObject[option.attribute]]; + } } else { // If the attribute is not present on the current object, // check if it exists on any nested objects for (const property in redactedObject) { - if (typeof redactedObject[property] === 'object') { + if (isArray(redactedObject[property])) { + redactedObject[property] = redactedObject[property].map( + (currentObject) => { + return redactAttributes({ options, object: currentObject }); + } + ); + } else if (isObject(redactedObject[property])) { // Recursively call the function on the nested object redactedObject[property] = redactAttributes({ options, diff --git a/apps/api/src/interceptors/redact-values-in-response.interceptor.ts b/apps/api/src/interceptors/redact-values-in-response.interceptor.ts index fa1b7f7f..724cdc45 100644 --- a/apps/api/src/interceptors/redact-values-in-response.interceptor.ts +++ b/apps/api/src/interceptors/redact-values-in-response.interceptor.ts @@ -1,5 +1,5 @@ -import { Activity } from '@ghostfolio/api/app/order/interfaces/activities.interface'; import { UserService } from '@ghostfolio/api/app/user/user.service'; +import { redactAttributes } from '@ghostfolio/api/helper/object.helper'; import { CallHandler, ExecutionContext, @@ -28,59 +28,35 @@ export class RedactValuesInResponseInterceptor hasImpersonationId || this.userService.isRestrictedView(request.user) ) { - if (data.accounts) { - for (const accountId of Object.keys(data.accounts)) { - if (data.accounts[accountId]?.balance !== undefined) { - data.accounts[accountId].balance = null; - } - } - } - - if (data.activities) { - data.activities = data.activities.map((activity: Activity) => { - if (activity.Account?.balance !== undefined) { - activity.Account.balance = null; - } - - if (activity.comment !== undefined) { - activity.comment = null; - } - - if (activity.fee !== undefined) { - activity.fee = null; - } - - if (activity.feeInBaseCurrency !== undefined) { - activity.feeInBaseCurrency = null; - } - - if (activity.quantity !== undefined) { - activity.quantity = null; - } - - if (activity.unitPrice !== undefined) { - activity.unitPrice = null; - } - - if (activity.value !== undefined) { - activity.value = null; - } - - if (activity.valueInBaseCurrency !== undefined) { - activity.valueInBaseCurrency = null; - } - - return activity; - }); - } - - if (data.filteredValueInBaseCurrency) { - data.filteredValueInBaseCurrency = null; - } - - if (data.totalValueInBaseCurrency) { - data.totalValueInBaseCurrency = null; - } + data = redactAttributes({ + object: data, + options: [ + 'balance', + 'balanceInBaseCurrency', + 'comment', + 'convertedBalance', + 'fee', + 'feeInBaseCurrency', + 'filteredValueInBaseCurrency', + 'grossPerformance', + 'investment', + 'netPerformance', + 'quantity', + 'symbolMapping', + 'totalBalanceInBaseCurrency', + 'totalValueInBaseCurrency', + 'unitPrice', + 'value', + 'valueInBaseCurrency' + ].map((attribute) => { + return { + attribute, + valueMap: { + '*': null + } + }; + }) + }); } return data;