Refactor impersonation mode into guard

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(

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<PortfolioReportResponse> {
     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;

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
       });

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<PortfolioDetails & { hasErrors: boolean }> {
-    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

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
     });
 

apps/client/src/app/components/benchmark-comparator/benchmark-comparator.component.html

@@ -18,10 +18,7 @@
       <mat-label i18n>Compare with...</mat-label>
       <mat-select
         name="benchmark"
-        [disabled]="
-          !hasPermissionToUpdateUserSettings() ||
-          user()?.subscription?.type === 'Basic'
-        "
+        [disabled]="user()?.subscription?.type === 'Basic'"
         [value]="benchmark()?.id"
         (selectionChange)="onChangeBenchmark($event.value)"
       >

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<LineChartItem[]>([]);
   public readonly benchmarks = input<Partial<SymbolProfile>[]>();
   public readonly colorScheme = input.required<ColorScheme>();
-  public readonly hasPermissionToUpdateUserSettings = input<boolean>();
   public readonly isLoading = input<boolean>();
   public readonly locale = input(getLocale());
   public readonly performanceDataItems = input.required<LineChartItem[]>();

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"

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',

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);
     });

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',