Fix user settings and calculations in impersonation mode

apps/api/src/app/activities/activities.controller.ts

@@ -1,3 +1,4 @@
+import { UserService } from '@ghostfolio/api/app/user/user.service';
 import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator';
 import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard';
 import { isActivityInFuture } from '@ghostfolio/api/helper/activity.helper';
@@ -54,7 +55,8 @@ export class ActivitiesController {
     private readonly dataProviderService: DataProviderService,
     private readonly dataGatheringService: DataGatheringService,
     private readonly impersonationService: ImpersonationService,
-    @Inject(REQUEST) private readonly request: RequestWithUser
+    @Inject(REQUEST) private readonly request: RequestWithUser,
+    private readonly userService: UserService
   ) {}
 
   @Delete()
@@ -169,8 +171,9 @@ export class ActivitiesController {
 
     const impersonationUserId =
       await this.impersonationService.validateImpersonationId(impersonationId);
+    const userId = impersonationUserId || this.request.user.id;
 
-    const userCurrency = this.request.user.settings.settings.baseCurrency;
+    const { settings } = await this.userService.user({ id: userId });
 
     const { activities, count } = await this.activitiesService.getActivities({
       endDate,
@@ -180,10 +183,10 @@ export class ActivitiesController {
       sortDirection,
       startDate,
       take,
-      userCurrency,
+      userId,
       includeDrafts: true,
       types: activityTypes,
-      userId: impersonationUserId || this.request.user.id,
+      userCurrency: settings.settings.baseCurrency,
       withExcludedAccountsAndActivities: true
     });
 
@@ -200,12 +203,14 @@ export class ActivitiesController {
   ): Promise<ActivityResponse> {
     const impersonationUserId =
       await this.impersonationService.validateImpersonationId(impersonationId);
-    const userCurrency = this.request.user.settings.settings.baseCurrency;
+    const userId = impersonationUserId || this.request.user.id;
+
+    const { settings } = await this.userService.user({ id: userId });
 
     const { activities } = await this.activitiesService.getActivities({
-      userCurrency,
+      userId,
       includeDrafts: true,
-      userId: impersonationUserId || this.request.user.id,
+      userCurrency: settings.settings.baseCurrency,
       withExcludedAccountsAndActivities: true
     });
 

apps/api/src/app/activities/activities.module.ts

@@ -2,6 +2,7 @@ import { AccountBalanceService } from '@ghostfolio/api/app/account-balance/accou
 import { AccountService } from '@ghostfolio/api/app/account/account.service';
 import { CacheModule } from '@ghostfolio/api/app/cache/cache.module';
 import { RedisCacheModule } from '@ghostfolio/api/app/redis-cache/redis-cache.module';
+import { UserModule } from '@ghostfolio/api/app/user/user.module';
 import { RedactValuesInResponseModule } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.module';
 import { TransformDataSourceInRequestModule } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.module';
 import { TransformDataSourceInResponseModule } from '@ghostfolio/api/interceptors/transform-data-source-in-response/transform-data-source-in-response.module';
@@ -39,7 +40,8 @@ import { ActivitiesService } from './activities.service';
     SymbolProfileModule,
     TagModule,
     TransformDataSourceInRequestModule,
-    TransformDataSourceInResponseModule
+    TransformDataSourceInResponseModule,
+    UserModule
   ],
   providers: [AccountBalanceService, AccountService, ActivitiesService]
 })

apps/api/src/app/endpoints/public/public.controller.ts

@@ -78,7 +78,7 @@ export class PublicController {
     ] = await Promise.all([
       this.portfolioService.getDetails({
         filters,
-        impersonationId: access.userId,
+        impersonationId: undefined,
         userId: user.id,
         withMarkets: true
       }),

apps/api/src/app/portfolio/portfolio.controller.ts

@@ -368,7 +368,8 @@ export class PortfolioController {
 
     let dividends = this.portfolioService.getDividends({
       activities,
-      groupBy
+      groupBy,
+      userCurrency
     });
 
     if (

apps/api/src/app/portfolio/portfolio.service.ts

@@ -45,7 +45,8 @@ import {
   getSum,
   isAccountExcluded,
   isDraftActivity,
-  parseDate
+  parseDate,
+  resolveUserSettings
 } from '@ghostfolio/common/helper';
 import {
   AccountsResponse,
@@ -195,10 +196,10 @@ export class PortfolioService {
         orderBy: { name: 'asc' }
       }),
       this.getDetails({
+        userId,
         withExcludedAccounts,
         filters: filtersWithoutSearchQueryFilter,
-        impersonationId: userId,
-        userId: this.request.user.id
+        impersonationId: undefined
       }),
       this.userService.user({ id: userId })
     ]);
@@ -355,10 +356,12 @@ export class PortfolioService {
 
   public getDividends({
     activities,
-    groupBy
+    groupBy,
+    userCurrency
   }: {
     activities: Activity[];
     groupBy?: GroupBy;
+    userCurrency: string;
   }): InvestmentItem[] {
     let dividends = activities.map(({ currency, date, value }) => {
       return {
@@ -366,7 +369,7 @@ export class PortfolioService {
         investment: this.exchangeRateDataService.toCurrency(
           value,
           currency,
-          this.getUserCurrency()
+          userCurrency
         )
       };
     });
@@ -1142,7 +1145,16 @@ export class PortfolioService {
     userId: string;
   }): Promise<PortfolioReportResponse> {
     userId = await this.getUserId(impersonationId, userId);
-    const userSettings = this.request.user.settings.settings as UserSettings;
+
+    const user = await this.userService.user({ id: userId });
+
+    // The rules are evaluated against the portfolio of the (potentially
+    // impersonated) user, while the translations follow the language of the
+    // authenticated user
+    const userSettings = resolveUserSettings({
+      impersonationUserSettings: user?.settings?.settings as UserSettings,
+      userSettings: this.request.user.settings.settings as UserSettings
+    });
 
     const { accounts, holdings, markets, marketsAdvanced, summary } =
       await this.getDetails({
@@ -2148,11 +2160,7 @@ export class PortfolioService {
   }
 
   private getUserCurrency(aUser?: UserWithSettings) {
-    return (
-      aUser?.settings?.settings.baseCurrency ??
-      this.request.user?.settings?.settings.baseCurrency ??
-      DEFAULT_CURRENCY
-    );
+    return aUser?.settings?.settings.baseCurrency ?? DEFAULT_CURRENCY;
   }
 
   private async getUserId(aImpersonationId: string, aUserId: string) {

apps/api/src/app/user/user.service.ts

@@ -41,6 +41,7 @@ import {
   THROTTLE_DAILY_TTL
 } from '@ghostfolio/common/config';
 import { SubscriptionType } from '@ghostfolio/common/enums';
+import { resolveUserSettings } from '@ghostfolio/common/helper';
 import {
   User as IUser,
   ReferralPartner,
@@ -163,9 +164,12 @@ export class UserService {
       this.tagService.getTagsForUser(impersonationUserId || user.id)
     ]);
 
-    const baseCurrency =
-      (impersonationUserSettings?.settings as UserSettings)?.baseCurrency ??
-      (settings.settings as UserSettings)?.baseCurrency;
+    const resolvedUserSettings = resolveUserSettings({
+      impersonationUserSettings: impersonationUserId
+        ? ((impersonationUserSettings?.settings ?? {}) as UserSettings)
+        : undefined,
+      userSettings: settings.settings as UserSettings
+    });
 
     let referralPartners: ReferralPartner[];
 
@@ -220,9 +224,9 @@ export class UserService {
       }),
       dateOfFirstActivity: firstActivity?.date ?? new Date(),
       settings: {
-        ...(settings.settings as UserSettings),
-        baseCurrency,
-        locale: (settings.settings as UserSettings)?.locale ?? locale
+        ...resolvedUserSettings,
+        baseCurrency: resolvedUserSettings.baseCurrency ?? DEFAULT_CURRENCY,
+        locale: resolvedUserSettings.locale ?? locale
       }
     };
   }

apps/api/src/services/impersonation/impersonation.service.ts

@@ -12,7 +12,11 @@ export class ImpersonationService {
     @Inject(REQUEST) private readonly request: RequestWithUser
   ) {}
 
-  public async validateImpersonationId(aId = '') {
+  public async validateImpersonationId(aId?: string) {
+    if (!aId) {
+      return null;
+    }
+
     if (this.request.user) {
       const accessObject = await this.prismaService.access.findFirst({
         where: {
@@ -29,7 +33,13 @@ export class ImpersonationService {
           permissions.impersonateAllUsers
         )
       ) {
-        return aId;
+        // The identifier is a user id in this case, hence verify its existence
+        const user = await this.prismaService.user.findUnique({
+          select: { id: true },
+          where: { id: aId }
+        });
+
+        return user?.id ?? null;
       }
     } else {
       // Public access

libs/common/src/lib/helper.spec.ts

@@ -12,8 +12,10 @@ import {
   isCurrency,
   isCurrencySymbol,
   isSplitRatio,
-  isValidCustomAssetProfileSymbol
+  isValidCustomAssetProfileSymbol,
+  resolveUserSettings
 } from '@ghostfolio/common/helper';
+import { UserSettings } from '@ghostfolio/common/interfaces';
 
 describe('Helper', () => {
   describe('Extract number from string', () => {
@@ -380,4 +382,89 @@ describe('Helper', () => {
       ).toEqual(true);
     });
   });
+
+  describe('Resolve user settings', () => {
+    const userSettings: UserSettings = {
+      baseCurrency: 'CHF',
+      colorScheme: 'DARK',
+      dateRange: '1y',
+      emergencyFund: 10000,
+      language: 'de',
+      locale: 'de-CH',
+      savingsRate: 500,
+      viewMode: 'DEFAULT'
+    };
+
+    const impersonationUserSettings: UserSettings = {
+      baseCurrency: 'USD',
+      colorScheme: 'LIGHT',
+      dateRange: 'ytd',
+      emergencyFund: 25000,
+      language: 'en',
+      locale: 'en-US',
+      savingsRate: 1000,
+      viewMode: 'ZEN'
+    };
+
+    it('Without impersonation', () => {
+      expect(
+        resolveUserSettings({
+          userSettings,
+          impersonationUserSettings: undefined
+        })
+      ).toEqual(userSettings);
+    });
+
+    it('Portfolio settings follow the impersonated user', () => {
+      const { baseCurrency, emergencyFund, savingsRate } = resolveUserSettings({
+        impersonationUserSettings,
+        userSettings
+      });
+
+      expect({ baseCurrency, emergencyFund, savingsRate }).toEqual({
+        baseCurrency: 'USD',
+        emergencyFund: 25000,
+        savingsRate: 1000
+      });
+    });
+
+    it('Presentation settings stay with the authenticated user', () => {
+      const { colorScheme, dateRange, language, locale, viewMode } =
+        resolveUserSettings({ impersonationUserSettings, userSettings });
+
+      expect({ colorScheme, dateRange, language, locale, viewMode }).toEqual({
+        colorScheme: 'DARK',
+        dateRange: '1y',
+        language: 'de',
+        locale: 'de-CH',
+        viewMode: 'DEFAULT'
+      });
+    });
+
+    it('Unknown settings default to the impersonated user', () => {
+      // A setting which is not classified as presentation must not leak from
+      // the authenticated user into the impersonated portfolio
+      expect(
+        resolveUserSettings({
+          userSettings: { annualInterestRate: 3 },
+          impersonationUserSettings: { annualInterestRate: 5 }
+        }).annualInterestRate
+      ).toEqual(5);
+    });
+
+    it('Impersonated user without settings', () => {
+      expect(
+        resolveUserSettings({
+          userSettings,
+          impersonationUserSettings: {}
+        })
+      ).toEqual({
+        colorScheme: 'DARK',
+        dateRange: '1y',
+        language: 'de',
+        locale: 'de-CH',
+        viewMode: 'DEFAULT'
+      });
+    });
+  });
 });

libs/common/src/lib/helper.ts

@@ -33,7 +33,7 @@ import {
   uk,
   zhCN
 } from 'date-fns/locale';
-import { get, isNil, isString } from 'lodash';
+import { get, isNil, isString, pick } from 'lodash';
 
 import {
   DEFAULT_CURRENCY,
@@ -51,7 +51,8 @@ import {
   AssetProfileIdentifier,
   AssetProfileItem,
   Benchmark,
-  PortfolioPosition
+  PortfolioPosition,
+  UserSettings
 } from './interfaces';
 import { BenchmarkTrend, ColorScheme } from './types';
 
@@ -59,6 +60,20 @@ export const DATE_FORMAT = 'yyyy-MM-dd';
 export const DATE_FORMAT_MONTHLY = 'MMMM yyyy';
 export const DATE_FORMAT_YEARLY = 'yyyy';
 
+// Settings which describe the person looking at the screen rather than the
+// portfolio being looked at. They stay with the authenticated user while
+// impersonating. Every other setting follows the impersonated user.
+const PRESENTATION_USER_SETTINGS_KEYS: (keyof UserSettings)[] = [
+  'colorScheme',
+  'dateRange',
+  'holdingsViewMode',
+  'isExperimentalFeatures',
+  'isRestrictedView',
+  'language',
+  'locale',
+  'viewMode'
+];
+
 export function applyAssetProfileOverrides<T extends Partial<SymbolProfile>>(
   assetProfile: T,
   assetProfileOverrides: AssetProfileOverrides | null
@@ -670,3 +685,20 @@ export function resolveMarketCondition(
     return { emoji: undefined };
   }
 }
+
+export function resolveUserSettings({
+  impersonationUserSettings,
+  userSettings
+}: {
+  impersonationUserSettings?: UserSettings;
+  userSettings: UserSettings;
+}): UserSettings {
+  if (!impersonationUserSettings) {
+    return { ...userSettings };
+  }
+
+  return {
+    ...impersonationUserSettings,
+    ...pick(userSettings ?? {}, PRESENTATION_USER_SETTINGS_KEYS)
+  };
+}