diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index 72056737c..12696472d 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/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 { 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 }); diff --git a/apps/api/src/app/activities/activities.module.ts b/apps/api/src/app/activities/activities.module.ts index 34091ba5e..fb0d3c4b0 100644 --- a/apps/api/src/app/activities/activities.module.ts +++ b/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] }) 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..6fb76cbdc 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, @@ -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 } }; } 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/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index 6cc090170..fbaccb1bd 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,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' + }); + }); + }); }); diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index d67abe03c..d4920bac3 100644 --- a/libs/common/src/lib/helper.ts +++ b/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>( 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) + }; +}