diff --git a/CHANGELOG.md b/CHANGELOG.md index b6563cdc1..c571fdae7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Fixed the account aggregations in impersonation mode to be based on the impersonated user +- Fixed the base currency of the activities in impersonation mode to be based on the impersonated user +- Fixed the base currency of the dividends in impersonation mode to be based on the impersonated user +- Fixed the base currency of the user account settings in impersonation mode to be disabled +- Fixed the benchmark selector of the performance chart on the analysis page in impersonation mode to be disabled +- Fixed the emergency fund of the _X-ray_ page in impersonation mode to be based on the impersonated user +- Fixed the savings rate of the _FIRE_ calculator in impersonation mode to be presented +- Fixed the user settings in impersonation mode to be based on the impersonated user +- Fixed the validation of the impersonation identifier of an unknown user + ## 3.47.0 - 2026-08-10 ### Changed diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index 72056737c..63357e6b4 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/apps/api/src/app/activities/activities.controller.ts @@ -7,16 +7,19 @@ import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interc import { ApiService } from '@ghostfolio/api/services/api/api.service'; import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service'; import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; +import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; import { DataGatheringService } from '@ghostfolio/api/services/queues/data-gathering/data-gathering.service'; import { getIntervalFromDateRange } from '@ghostfolio/common/calculation-helper'; import { DATA_GATHERING_QUEUE_PRIORITY_HIGH, + DEFAULT_CURRENCY, HEADER_KEY_IMPERSONATION } from '@ghostfolio/common/config'; import { CreateOrderDto, UpdateOrderDto } from '@ghostfolio/common/dtos'; import { ActivitiesResponse, - ActivityResponse + ActivityResponse, + UserSettings } from '@ghostfolio/common/interfaces'; import { permissions } from '@ghostfolio/common/permissions'; import type { RequestWithUser } from '@ghostfolio/common/types'; @@ -54,6 +57,7 @@ export class ActivitiesController { private readonly dataProviderService: DataProviderService, private readonly dataGatheringService: DataGatheringService, private readonly impersonationService: ImpersonationService, + private readonly prismaService: PrismaService, @Inject(REQUEST) private readonly request: RequestWithUser ) {} @@ -169,8 +173,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 userCurrency = await this.getUserCurrency(impersonationUserId); const { activities, count } = await this.activitiesService.getActivities({ endDate, @@ -181,9 +186,9 @@ export class ActivitiesController { startDate, take, userCurrency, + userId, includeDrafts: true, types: activityTypes, - userId: impersonationUserId || this.request.user.id, withExcludedAccountsAndActivities: true }); @@ -200,12 +205,14 @@ export class ActivitiesController { ): Promise { const impersonationUserId = await this.impersonationService.validateImpersonationId(impersonationId); - const userCurrency = this.request.user.settings.settings.baseCurrency; + const userId = impersonationUserId || this.request.user.id; + + const userCurrency = await this.getUserCurrency(impersonationUserId); const { activities } = await this.activitiesService.getActivities({ userCurrency, + userId, includeDrafts: true, - userId: impersonationUserId || this.request.user.id, withExcludedAccountsAndActivities: true }); @@ -377,4 +384,18 @@ export class ActivitiesController { } }); } + + private async getUserCurrency(impersonationUserId: string) { + if (!impersonationUserId) { + return this.request.user.settings.settings.baseCurrency; + } + + const settings = await this.prismaService.settings.findUnique({ + where: { userId: impersonationUserId } + }); + + return ( + (settings?.settings as UserSettings)?.baseCurrency ?? DEFAULT_CURRENCY + ); + } } diff --git a/apps/api/src/app/endpoints/public/public.controller.ts b/apps/api/src/app/endpoints/public/public.controller.ts index 67bed71ef..53daf3469 100644 --- a/apps/api/src/app/endpoints/public/public.controller.ts +++ b/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 }), diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 953976a4a..3eb9ca4d9 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/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 ( diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 704de93f2..88f675008 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/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 { 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) { diff --git a/apps/api/src/app/user/user.service.ts b/apps/api/src/app/user/user.service.ts index a055f029e..ada59f460 100644 --- a/apps/api/src/app/user/user.service.ts +++ b/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, @@ -58,7 +59,7 @@ import { PerformanceCalculationType } from '@ghostfolio/common/types/performance import { Injectable, Logger } from '@nestjs/common'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { InjectThrottlerStorage, ThrottlerStorage } from '@nestjs/throttler'; -import { Prisma, Role, Settings, User } from '@prisma/client'; +import { Prisma, Role, User } from '@prisma/client'; import { differenceInDays, subDays } from 'date-fns'; import { isNil, without } from 'lodash'; import { createHmac } from 'node:crypto'; @@ -127,7 +128,7 @@ export class UserService { accounts, activitiesCount, firstActivity, - impersonationUserSettings, + impersonationUser, tagsForUser ] = await Promise.all([ this.prismaService.access.findMany({ @@ -156,16 +157,17 @@ export class UserService { where: { userId: impersonationUserId || user.id } }), impersonationUserId - ? this.prismaService.settings.findUnique({ - where: { userId: impersonationUserId } - }) - : Promise.resolve(null), + ? this.user({ id: impersonationUserId }) + : Promise.resolve(null), this.tagService.getTagsForUser(impersonationUserId || user.id) ]); - const baseCurrency = - (impersonationUserSettings?.settings as UserSettings)?.baseCurrency ?? - (settings.settings as UserSettings)?.baseCurrency; + const resolvedUserSettings = resolveUserSettings({ + impersonationUserSettings: impersonationUserId + ? ((impersonationUser?.settings?.settings ?? {}) as UserSettings) + : undefined, + userSettings: settings.settings as UserSettings + }); let referralPartners: ReferralPartner[]; @@ -220,9 +222,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 } }; } diff --git a/apps/api/src/services/impersonation/impersonation.service.ts b/apps/api/src/services/impersonation/impersonation.service.ts index 71c543a43..798a20e5c 100644 --- a/apps/api/src/services/impersonation/impersonation.service.ts +++ b/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 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 328cccba1..eceb31df3 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,7 +18,10 @@ 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 0091ae5d7..e21f54aaf 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,6 +69,7 @@ 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/components/user-account-settings/user-account-settings.component.ts b/apps/client/src/app/components/user-account-settings/user-account-settings.component.ts index a4d8a1ba1..83f7ddd9a 100644 --- a/apps/client/src/app/components/user-account-settings/user-account-settings.component.ts +++ b/apps/client/src/app/components/user-account-settings/user-account-settings.component.ts @@ -1,3 +1,4 @@ +import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { KEY_STAY_SIGNED_IN, KEY_TOKEN, @@ -90,6 +91,7 @@ export class GfUserAccountSettingsComponent implements OnInit { protected readonly deleteOwnUserForm = inject(NonNullableFormBuilder).group({ accessToken: ['', Validators.required] }); + protected hasImpersonationId: boolean; protected hasPermissionToDeleteOwnUser: boolean; protected hasPermissionToRequestOwnUserDeletion: boolean; protected hasPermissionToUpdateViewMode: boolean; @@ -129,6 +131,9 @@ export class GfUserAccountSettingsComponent implements OnInit { private readonly dataService = inject(DataService); private readonly deviceDetectorService = inject(DeviceDetectorService); private readonly destroyRef = inject(DestroyRef); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); private readonly notificationService = inject(NotificationService); private readonly settingsStorageService = inject(SettingsStorageService); private readonly snackBar = inject(MatSnackBar); @@ -140,6 +145,17 @@ export class GfUserAccountSettingsComponent implements OnInit { this.currencies = currencies; + this.impersonationStorageService + .onChangeHasImpersonation() + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((impersonationId) => { + this.hasImpersonationId = !!impersonationId; + + this.updateBaseCurrencyFormState(); + + this.changeDetectorRef.markForCheck(); + }); + this.userService.stateChanged .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe((state) => { @@ -192,11 +208,7 @@ export class GfUserAccountSettingsComponent implements OnInit { baseCurrency: this.user.settings.baseCurrency ?? null }); - if (this.hasPermissionToUpdateUserSettings) { - this.baseCurrencyForm.enable({ emitEvent: false }); - } else { - this.baseCurrencyForm.disable({ emitEvent: false }); - } + this.updateBaseCurrencyFormState(); if (this.user.settings.locale) { this.locales.push(this.user.settings.locale); @@ -431,4 +443,14 @@ export class GfUserAccountSettingsComponent implements OnInit { this.changeDetectorRef.markForCheck(); } + + private updateBaseCurrencyFormState() { + // The base currency belongs to the impersonated user while a change would be + // applied to the authenticated user + if (!this.hasImpersonationId && this.hasPermissionToUpdateUserSettings) { + this.baseCurrencyForm.enable({ emitEvent: false }); + } else { + this.baseCurrencyForm.disable({ emitEvent: false }); + } + } } 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 82751b882..3d22f0c68 100644 --- a/apps/client/src/app/pages/portfolio/analysis/analysis-page.html +++ b/apps/client/src/app/pages/portfolio/analysis/analysis-page.html @@ -137,6 +137,7 @@ [benchmarkDataItems]="benchmarkDataItems" [benchmarks]="benchmarks" [colorScheme]="user?.settings?.colorScheme" + [hasPermissionToUpdateUserSettings]="!impersonationId" [isLoading]="isLoadingBenchmarkComparator || isLoadingInvestmentChart" [locale]="user?.settings?.locale" [performanceDataItems]="performanceDataItemsInPercentage" diff --git a/apps/client/src/app/pages/portfolio/fire/fire-page.html b/apps/client/src/app/pages/portfolio/fire/fire-page.html index 13693a15a..7315f10cb 100644 --- a/apps/client/src/app/pages/portfolio/fire/fire-page.html +++ b/apps/client/src/app/pages/portfolio/fire/fire-page.html @@ -21,7 +21,7 @@ [locale]="user?.settings?.locale" [projectedTotalAmount]="user?.settings?.projectedTotalAmount" [retirementDate]="user?.settings?.retirementDate" - [savingsRate]="hasImpersonationId ? 0 : user?.settings?.savingsRate" + [savingsRate]="user?.settings?.savingsRate" [style.opacity]=" user?.subscription?.type === 'Basic' ? '0.67' : 'initial' " diff --git a/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index 6cc090170..db3f9677d 100644 --- a/libs/common/src/lib/helper.spec.ts +++ b/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,118 @@ 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('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 + const { 'filters.accounts': filtersAccounts } = resolveUserSettings({ + impersonationUserSettings: { + 'filters.accounts': ['3b3c2b5d-5a4f-4b0a-9d4f-9b1f5e6a7c8d'] + }, + userSettings: { + 'filters.accounts': ['0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d'] + } + }); + + expect(filtersAccounts).toEqual(['0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d']); + }); + + it('Presentation settings unset for the authenticated user do not leak', () => { + // An unset presentation setting must not fall back to the impersonated + // user, otherwise their appearance and language apply to the + // authenticated user + const { colorScheme, language, locale } = resolveUserSettings({ + impersonationUserSettings, + userSettings: { baseCurrency: 'CHF' } + }); + + expect(colorScheme).toBeUndefined(); + expect(language).toBeUndefined(); + expect(locale).toBeUndefined(); + }); + + 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' + }); + }); + }); }); diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index d67abe03c..2e4125ea9 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -51,7 +51,8 @@ import { AssetProfileIdentifier, AssetProfileItem, Benchmark, - PortfolioPosition + PortfolioPosition, + UserSettings } from './interfaces'; import { BenchmarkTrend, ColorScheme } from './types'; @@ -59,6 +60,28 @@ 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. +// The filters are included because they are always written back to the +// authenticated user, so reading them from the impersonated user would +// overwrite the filters of the authenticated user. +const PRESENTATION_USER_SETTINGS_KEYS: (keyof UserSettings)[] = [ + 'colorScheme', + 'dateRange', + 'filters.accounts', + 'filters.assetClasses', + 'filters.dataSource', + 'filters.symbol', + 'filters.tags', + 'holdingsViewMode', + 'isExperimentalFeatures', + 'isRestrictedView', + 'language', + 'locale', + 'viewMode' +]; + export function applyAssetProfileOverrides>( assetProfile: T, assetProfileOverrides: AssetProfileOverrides | null @@ -670,3 +693,24 @@ export function resolveMarketCondition( return { emoji: undefined }; } } + +export function resolveUserSettings({ + impersonationUserSettings, + userSettings +}: { + impersonationUserSettings?: UserSettings; + userSettings: UserSettings; +}): UserSettings { + if (!impersonationUserSettings) { + return { ...userSettings }; + } + + return { + ...impersonationUserSettings, + ...Object.fromEntries( + PRESENTATION_USER_SETTINGS_KEYS.map((key) => { + return [key, userSettings?.[key]]; + }) + ) + }; +}