From a3dd81ee0177d51c99b773c636e434e15580db63 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:01:04 +0200 Subject: [PATCH] Refactor impersonation mode into guard --- .../src/app/endpoints/tags/tags.controller.ts | 2 ++ .../src/app/portfolio/portfolio.controller.ts | 13 ++++++++++--- .../app/portfolio/portfolio.service.spec.ts | 15 --------------- .../api/src/app/portfolio/portfolio.service.ts | 6 +++++- apps/api/src/app/user/user.service.ts | 5 ++--- .../benchmark-comparator.component.html | 5 +---- .../benchmark-comparator.component.ts | 1 - .../portfolio/analysis/analysis-page.html | 1 - libs/common/src/lib/config.ts | 3 +++ libs/common/src/lib/helper.spec.ts | 18 ++++++++++++++++-- libs/common/src/lib/helper.ts | 1 + 11 files changed, 40 insertions(+), 30 deletions(-) diff --git a/apps/api/src/app/endpoints/tags/tags.controller.ts b/apps/api/src/app/endpoints/tags/tags.controller.ts index cd043b593..a61e1188c 100644 --- a/apps/api/src/app/endpoints/tags/tags.controller.ts +++ b/apps/api/src/app/endpoints/tags/tags.controller.ts @@ -1,3 +1,4 @@ +import { AllowDuringImpersonation } from '@ghostfolio/api/decorators/allow-during-impersonation.decorator'; import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { TagService } from '@ghostfolio/api/services/tag/tag.service'; @@ -23,6 +24,7 @@ import { AuthGuard } from '@nestjs/passport'; import { Tag } from '@prisma/client'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; +@AllowDuringImpersonation() @Controller('tags') export class TagsController { public constructor( diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index e3e87dfd5..234996edd 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -647,13 +647,20 @@ export class PortfolioController { @Get('report') @UseGuards(AuthGuard('jwt'), HasPermissionGuard, ImpersonationGuard) public async getReport( - @Impersonation() { userId }: ImpersonationContext + @Impersonation() { accessId, userId }: ImpersonationContext ): Promise { const report = await this.portfolioService.getReport({ userId }); if ( - this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - this.request.user.subscription?.type === SubscriptionType.Basic + // The evaluations of the rules interpolate absolute values, hence they + // are withheld from a restricted view + hasReadRestrictedAccessPermission({ + accesses: this.request.user?.accessesGet, + impersonationId: accessId + }) || + isRestrictedView(this.request.user) || + (this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && + this.request.user.subscription?.type === SubscriptionType.Basic) ) { for (const category of report.xRay.categories) { category.rules = null; diff --git a/apps/api/src/app/portfolio/portfolio.service.spec.ts b/apps/api/src/app/portfolio/portfolio.service.spec.ts index b503b18e9..9b6bacd44 100644 --- a/apps/api/src/app/portfolio/portfolio.service.spec.ts +++ b/apps/api/src/app/portfolio/portfolio.service.spec.ts @@ -8,7 +8,6 @@ import { UserService } from '@ghostfolio/api/app/user/user.service'; import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service'; import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service'; -import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/symbol-profile.service'; import { UNKNOWN_KEY } from '@ghostfolio/common/config'; import { parseDate } from '@ghostfolio/common/helper'; @@ -30,7 +29,6 @@ describe('PortfolioService', () => { let configurationService: ConfigurationService; let dataProviderService: DataProviderService; let exchangeRateDataService: ExchangeRateDataService; - let impersonationService: ImpersonationService; let portfolioCalculatorFactory: PortfolioCalculatorFactory; let portfolioService: PortfolioService; let symbolProfileService: SymbolProfileService; @@ -77,8 +75,6 @@ describe('PortfolioService', () => { null ); - impersonationService = new ImpersonationService(null, null); - portfolioCalculatorFactory = new PortfolioCalculatorFactory( configurationService, null, @@ -110,7 +106,6 @@ describe('PortfolioService', () => { dataProviderService, exchangeRateDataService, null, - impersonationService, null, null, symbolProfileService, @@ -245,10 +240,6 @@ describe('PortfolioService', () => { .spyOn(dataProviderService, 'getDataSourceForExchangeRates') .mockReturnValue(DataSource.YAHOO); - jest - .spyOn(impersonationService, 'validateImpersonationId') - .mockResolvedValue(null); - jest .spyOn(symbolProfileService, 'getSymbolProfiles') .mockResolvedValue([]); @@ -331,7 +322,6 @@ describe('PortfolioService', () => { const { holdings } = await portfolioService.getDetails({ filters: [], - impersonationId: userDummyData.id, userId: userDummyData.id }); @@ -371,10 +361,6 @@ describe('PortfolioService', () => { .spyOn(activitiesService, 'getActivities') .mockResolvedValue({ activities: [], count: 0 }); - jest - .spyOn(impersonationService, 'validateImpersonationId') - .mockResolvedValue(null); - jest.spyOn(portfolioService, 'getPerformance').mockResolvedValue({ performance: { currentValueInBaseCurrency: 3000, @@ -408,7 +394,6 @@ describe('PortfolioService', () => { balanceInBaseCurrency: 1000, emergencyFundHoldingsValueInBaseCurrency: 0, filteredValueInBaseCurrency: new Big(3000), - impersonationId: undefined, userCurrency: 'CHF', userId: userDummyData.id }); diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 498219dca..2cbafc076 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -506,6 +506,7 @@ export class PortfolioService { public async getDetails({ dateRange = DEFAULT_DATE_RANGE, filters, + user: userFromCaller, userId, withExcludedAccounts = false, withMarkets = false, @@ -513,12 +514,14 @@ export class PortfolioService { }: { dateRange?: DateRange; filters?: Filter[]; + user?: UserWithSettings; userId: string; withExcludedAccounts?: boolean; withMarkets?: boolean; withSummary?: boolean; }): Promise { - const user = await this.userService.user({ id: userId }); + const user = + userFromCaller ?? (await this.userService.user({ id: userId })); const userCurrency = this.getUserCurrency(user); const emergencyFund = new Big( @@ -1138,6 +1141,7 @@ export class PortfolioService { const { accounts, holdings, markets, marketsAdvanced, summary } = await this.getDetails({ + user, userId, withMarkets: true, withSummary: true diff --git a/apps/api/src/app/user/user.service.ts b/apps/api/src/app/user/user.service.ts index ada59f460..d74b90ede 100644 --- a/apps/api/src/app/user/user.service.ts +++ b/apps/api/src/app/user/user.service.ts @@ -163,9 +163,8 @@ export class UserService { ]); const resolvedUserSettings = resolveUserSettings({ - impersonationUserSettings: impersonationUserId - ? ((impersonationUser?.settings?.settings ?? {}) as UserSettings) - : undefined, + impersonationUserSettings: impersonationUser?.settings + ?.settings as UserSettings, userSettings: settings.settings as UserSettings }); diff --git a/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.html b/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.html index eceb31df3..328cccba1 100644 --- a/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.html +++ b/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.html @@ -18,10 +18,7 @@ Compare with... diff --git a/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.ts b/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.ts index e21f54aaf..0091ae5d7 100644 --- a/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.ts +++ b/apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.ts @@ -69,7 +69,6 @@ export class GfBenchmarkComparatorComponent implements OnChanges, OnDestroy { public readonly benchmarkDataItems = input([]); public readonly benchmarks = input[]>(); public readonly colorScheme = input.required(); - public readonly hasPermissionToUpdateUserSettings = input(); public readonly isLoading = input(); public readonly locale = input(getLocale()); public readonly performanceDataItems = input.required(); diff --git a/apps/client/src/app/pages/portfolio/analysis/analysis-page.html b/apps/client/src/app/pages/portfolio/analysis/analysis-page.html index 3d22f0c68..82751b882 100644 --- a/apps/client/src/app/pages/portfolio/analysis/analysis-page.html +++ b/apps/client/src/app/pages/portfolio/analysis/analysis-page.html @@ -137,7 +137,6 @@ [benchmarkDataItems]="benchmarkDataItems" [benchmarks]="benchmarks" [colorScheme]="user?.settings?.colorScheme" - [hasPermissionToUpdateUserSettings]="!impersonationId" [isLoading]="isLoadingBenchmarkComparator || isLoadingInvestmentChart" [locale]="user?.settings?.locale" [performanceDataItems]="performanceDataItemsInPercentage" diff --git a/libs/common/src/lib/config.ts b/libs/common/src/lib/config.ts index 265e27690..84d678458 100644 --- a/libs/common/src/lib/config.ts +++ b/libs/common/src/lib/config.ts @@ -153,6 +153,9 @@ export const DEFAULT_REDACTED_PATHS = [ 'platforms[*].balance', 'platforms[*].valueInBaseCurrency', 'quantity', + 'settings.emergencyFund', + 'settings.projectedTotalAmount', + 'settings.savingsRate', 'totalBalanceInBaseCurrency', 'totalDividendInBaseCurrency', 'totalInterestInBaseCurrency', diff --git a/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index db3f9677d..d7e44d139 100644 --- a/libs/common/src/lib/helper.spec.ts +++ b/libs/common/src/lib/helper.spec.ts @@ -441,6 +441,20 @@ describe('Helper', () => { }); }); + it('Benchmark stays with the authenticated user', () => { + // The benchmark is a comparison of the person looking at the screen and + // is gated by their subscription, so it must not follow the impersonated + // user + const { benchmark } = resolveUserSettings({ + impersonationUserSettings: { + benchmark: '82fd8dcc-4a0e-4dd0-b6cb-7b8a4b03e6b1' + }, + userSettings: { benchmark: '1e5a0e6a-1b8b-4d0e-9f0a-4c2b3d5e6f7a' } + }); + + expect(benchmark).toEqual('1e5a0e6a-1b8b-4d0e-9f0a-4c2b3d5e6f7a'); + }); + it('Filters stay with the authenticated user', () => { // The filters are always written back to the authenticated user, so // reading them from the impersonated user would overwrite them @@ -475,8 +489,8 @@ describe('Helper', () => { // the authenticated user into the impersonated portfolio expect( resolveUserSettings({ - userSettings: { annualInterestRate: 3 }, - impersonationUserSettings: { annualInterestRate: 5 } + impersonationUserSettings: { annualInterestRate: 5 }, + userSettings: { annualInterestRate: 3 } }).annualInterestRate ).toEqual(5); }); diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index 1b2350841..9aab8d930 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -67,6 +67,7 @@ export const DATE_FORMAT_YEARLY = 'yyyy'; // authenticated user, so reading them from the impersonated user would // overwrite the filters of the authenticated user. const USER_SETTINGS_KEYS_OF_AUTHENTICATED_USER: (keyof UserSettings)[] = [ + 'benchmark', 'colorScheme', 'dateRange', 'filters.accounts',