diff --git a/CHANGELOG.md b/CHANGELOG.md index 9766cd8b7..9d6899c6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Added the write scopes to the access + ### Changed - Improved the performance of the portfolio snapshot calculation by indexing the activities diff --git a/apps/api/src/app/account-balance/account-balance.controller.ts b/apps/api/src/app/account-balance/account-balance.controller.ts index baf002bd3..a37001bc3 100644 --- a/apps/api/src/app/account-balance/account-balance.controller.ts +++ b/apps/api/src/app/account-balance/account-balance.controller.ts @@ -1,9 +1,12 @@ import { AccountService } from '@ghostfolio/api/app/account/account.service'; import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; -import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; +import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; +import { RequiresScope } from '@ghostfolio/api/decorators/requires-scope.decorator'; +import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.interceptor'; import { CreateAccountBalanceDto } from '@ghostfolio/common/dtos'; import { permissions } from '@ghostfolio/common/permissions'; -import type { RequestWithUser } from '@ghostfolio/common/types'; +import { scopes } from '@ghostfolio/common/scopes'; +import type { ImpersonationContext } from '@ghostfolio/common/types'; import { Controller, @@ -11,12 +14,9 @@ import { Post, Delete, HttpException, - Inject, Param, - UseGuards + UseInterceptors } from '@nestjs/common'; -import { REQUEST } from '@nestjs/core'; -import { AuthGuard } from '@nestjs/passport'; import { AccountBalance } from '@prisma/client'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -26,20 +26,21 @@ import { AccountBalanceService } from './account-balance.service'; export class AccountBalanceController { public constructor( private readonly accountBalanceService: AccountBalanceService, - private readonly accountService: AccountService, - @Inject(REQUEST) private readonly request: RequestWithUser + private readonly accountService: AccountService ) {} @HasPermission(permissions.createAccountBalance) @Post() - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.accountUpdate) + @UseInterceptors(RedactValuesInResponseInterceptor) public async createAccountBalance( - @Body() data: CreateAccountBalanceDto + @Body() data: CreateAccountBalanceDto, + @Impersonation() { userId }: ImpersonationContext ): Promise { const account = await this.accountService.account({ id_userId: { - id: data.accountId, - userId: this.request.user.id + userId, + id: data.accountId } }); @@ -60,13 +61,15 @@ export class AccountBalanceController { @HasPermission(permissions.deleteAccountBalance) @Delete(':id') - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.accountUpdate) + @UseInterceptors(RedactValuesInResponseInterceptor) public async deleteAccountBalance( + @Impersonation() { userId }: ImpersonationContext, @Param('id') id: string ): Promise { const accountBalance = await this.accountBalanceService.accountBalance({ id, - userId: this.request.user.id + userId }); if (!accountBalance) { diff --git a/apps/api/src/app/account-balance/account-balance.module.ts b/apps/api/src/app/account-balance/account-balance.module.ts index f7b1efc51..edb0f4deb 100644 --- a/apps/api/src/app/account-balance/account-balance.module.ts +++ b/apps/api/src/app/account-balance/account-balance.module.ts @@ -1,5 +1,6 @@ import { AccountService } from '@ghostfolio/api/app/account/account.service'; import { ExchangeRateDataModule } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.module'; +import { ImpersonationModule } from '@ghostfolio/api/services/impersonation/impersonation.module'; import { PrismaModule } from '@ghostfolio/api/services/prisma/prisma.module'; import { TagModule } from '@ghostfolio/api/services/tag/tag.module'; @@ -11,7 +12,12 @@ import { AccountBalanceService } from './account-balance.service'; @Module({ controllers: [AccountBalanceController], exports: [AccountBalanceService], - imports: [ExchangeRateDataModule, PrismaModule, TagModule], + imports: [ + ExchangeRateDataModule, + ImpersonationModule, + PrismaModule, + TagModule + ], providers: [AccountBalanceService, AccountService] }) export class AccountBalanceModule {} diff --git a/apps/api/src/app/account/account.controller.ts b/apps/api/src/app/account/account.controller.ts index 8b6692503..8fc1714db 100644 --- a/apps/api/src/app/account/account.controller.ts +++ b/apps/api/src/app/account/account.controller.ts @@ -3,7 +3,6 @@ import { PortfolioService } from '@ghostfolio/api/app/portfolio/portfolio.servic import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; import { RequiresScope } from '@ghostfolio/api/decorators/requires-scope.decorator'; -import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.interceptor'; import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor'; import { ApiService } from '@ghostfolio/api/services/api/api.service'; @@ -19,10 +18,7 @@ import { } from '@ghostfolio/common/interfaces'; import { permissions } from '@ghostfolio/common/permissions'; import { scopes } from '@ghostfolio/common/scopes'; -import type { - ImpersonationContext, - RequestWithUser -} from '@ghostfolio/common/types'; +import type { ImpersonationContext } from '@ghostfolio/common/types'; import { Body, @@ -30,16 +26,12 @@ import { Delete, Get, HttpException, - Inject, Param, Post, Put, Query, - UseGuards, UseInterceptors } from '@nestjs/common'; -import { REQUEST } from '@nestjs/core'; -import { AuthGuard } from '@nestjs/passport'; import { Account as AccountModel } from '@prisma/client'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -51,19 +43,22 @@ export class AccountController { private readonly accountBalanceService: AccountBalanceService, private readonly accountService: AccountService, private readonly apiService: ApiService, - private readonly portfolioService: PortfolioService, - @Inject(REQUEST) private readonly request: RequestWithUser + private readonly portfolioService: PortfolioService ) {} @Delete(':id') @HasPermission(permissions.deleteAccount) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) - public async deleteAccount(@Param('id') id: string): Promise { + @RequiresScope(scopes.accountDelete) + @UseInterceptors(RedactValuesInResponseInterceptor) + public async deleteAccount( + @Impersonation() { userId }: ImpersonationContext, + @Param('id') id: string + ): Promise { const account = await this.accountService.accountWithActivities( { id_userId: { id, - userId: this.request.user.id + userId } }, { activities: true } @@ -79,7 +74,7 @@ export class AccountController { return this.accountService.deleteAccount({ id_userId: { id, - userId: this.request.user.id + userId } }); } @@ -140,9 +135,11 @@ export class AccountController { @HasPermission(permissions.createAccount) @Post() - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.accountCreate) + @UseInterceptors(RedactValuesInResponseInterceptor) public async createAccount( - @Body() data: CreateAccountDto + @Body() data: CreateAccountDto, + @Impersonation() { userId }: ImpersonationContext ): Promise { const { balance, tags: tagIds, ...accountData } = data; @@ -153,12 +150,12 @@ export class AccountController { return this.accountService.createAccount({ balance, tagIds, + userId, data: { ...accountData, platform: { connect: { id: platformId } }, - user: { connect: { id: this.request.user.id } } - }, - userId: this.request.user.id + user: { connect: { id: userId } } + } }); } else { delete accountData.platformId; @@ -166,24 +163,23 @@ export class AccountController { return this.accountService.createAccount({ balance, tagIds, + userId, data: { ...accountData, - user: { connect: { id: this.request.user.id } } - }, - userId: this.request.user.id + user: { connect: { id: userId } } + } }); } } @HasPermission(permissions.updateAccount) @Post('transfer-balance') - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.accountUpdate) public async transferAccountBalance( - @Body() { accountIdFrom, accountIdTo, balance }: TransferBalanceDto + @Body() { accountIdFrom, accountIdTo, balance }: TransferBalanceDto, + @Impersonation() { userId }: ImpersonationContext ) { - const accountsOfUser = await this.accountService.getAccounts( - this.request.user.id - ); + const accountsOfUser = await this.accountService.getAccounts(userId); const accountFrom = accountsOfUser.find(({ id }) => { return id === accountIdFrom; @@ -215,28 +211,33 @@ export class AccountController { } await this.accountService.updateAccountBalance({ + userId, accountId: accountFrom.id, amount: -balance, - currency: accountFrom.currency, - userId: this.request.user.id + currency: accountFrom.currency }); await this.accountService.updateAccountBalance({ + userId, accountId: accountTo.id, amount: balance, - currency: accountFrom.currency, - userId: this.request.user.id + currency: accountFrom.currency }); } @HasPermission(permissions.updateAccount) @Put(':id') - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) - public async update(@Param('id') id: string, @Body() data: UpdateAccountDto) { + @RequiresScope(scopes.accountUpdate) + @UseInterceptors(RedactValuesInResponseInterceptor) + public async update( + @Body() data: UpdateAccountDto, + @Impersonation() { userId }: ImpersonationContext, + @Param('id') id: string + ) { const originalAccount = await this.accountService.account({ id_userId: { id, - userId: this.request.user.id + userId } }); @@ -256,16 +257,16 @@ export class AccountController { return this.accountService.updateAccount({ balance, tagIds, + userId, data: { ...accountData, platform: { connect: { id: platformId } }, - user: { connect: { id: this.request.user.id } } + user: { connect: { id: userId } } }, - userId: this.request.user.id, where: { id_userId: { id, - userId: this.request.user.id + userId } } }); @@ -276,18 +277,18 @@ export class AccountController { return this.accountService.updateAccount({ balance, tagIds, + userId, data: { ...accountData, platform: originalAccount.platformId ? { disconnect: true } : undefined, - user: { connect: { id: this.request.user.id } } + user: { connect: { id: userId } } }, - userId: this.request.user.id, where: { id_userId: { id, - userId: this.request.user.id + userId } } }); diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index bb02fcac5..caf4103b9 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/apps/api/src/app/activities/activities.controller.ts @@ -1,7 +1,6 @@ import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; import { RequiresScope } from '@ghostfolio/api/decorators/requires-scope.decorator'; -import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { isActivityInFuture } from '@ghostfolio/api/helper/activity.helper'; import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.interceptor'; import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor'; @@ -12,16 +11,14 @@ import { DataGatheringService } from '@ghostfolio/api/services/queues/data-gathe import { getIntervalFromDateRange } from '@ghostfolio/common/calculation-helper'; import { DATA_GATHERING_QUEUE_PRIORITY_HIGH } from '@ghostfolio/common/config'; import { CreateOrderDto, UpdateOrderDto } from '@ghostfolio/common/dtos'; +import { SubscriptionType } from '@ghostfolio/common/enums'; import { ActivitiesResponse, ActivityResponse } from '@ghostfolio/common/interfaces'; import { permissions } from '@ghostfolio/common/permissions'; import { scopes } from '@ghostfolio/common/scopes'; -import type { - ImpersonationContext, - RequestWithUser -} from '@ghostfolio/common/types'; +import type { ImpersonationContext } from '@ghostfolio/common/types'; import { Body, @@ -29,16 +26,12 @@ import { Delete, Get, HttpException, - Inject, Param, Post, Put, Query, - UseGuards, UseInterceptors } from '@nestjs/common'; -import { REQUEST } from '@nestjs/core'; -import { AuthGuard } from '@nestjs/passport'; import { Order } from '@prisma/client'; import { parseISO } from 'date-fns'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -53,15 +46,15 @@ export class ActivitiesController { private readonly activitiesService: ActivitiesService, private readonly apiService: ApiService, private readonly dataProviderService: DataProviderService, - private readonly dataGatheringService: DataGatheringService, - @Inject(REQUEST) private readonly request: RequestWithUser + private readonly dataGatheringService: DataGatheringService ) {} @Delete() @HasPermission(permissions.deleteActivity) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.activityDelete) @UseInterceptors(TransformDataSourceInRequestInterceptor) public async deleteActivities( + @Impersonation() { userId }: ImpersonationContext, @Query() { accounts, @@ -94,18 +87,22 @@ export class ActivitiesController { endDate, filters, startDate, - types: activityTypes, - userId: this.request.user.id + userId, + types: activityTypes }); } @Delete(':id') @HasPermission(permissions.deleteActivity) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) - public async deleteActivity(@Param('id') id: string): Promise { + @RequiresScope(scopes.activityDelete) + @UseInterceptors(RedactValuesInResponseInterceptor) + public async deleteActivity( + @Impersonation() { userId }: ImpersonationContext, + @Param('id') id: string + ): Promise { const activity = await this.activitiesService.order({ id, - userId: this.request.user.id + userId }); if (!activity) { @@ -208,11 +205,28 @@ export class ActivitiesController { @HasPermission(permissions.createActivity) @Post() - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.activityCreate) + @UseInterceptors(RedactValuesInResponseInterceptor) @UseInterceptors(TransformDataSourceInRequestInterceptor) - public async createActivity(@Body() data: CreateOrderDto): Promise { + public async createActivity( + @Body() data: CreateOrderDto, + @Impersonation() + { + authenticatedUserSubscription, + userId, + userSubscription + }: ImpersonationContext + ): Promise { + // Evaluate the more restrictive subscription of the authenticated user + // and the owner of the activity + const subscription = + userSubscription?.type === SubscriptionType.Basic + ? userSubscription + : authenticatedUserSubscription; + try { await this.dataProviderService.validateActivities({ + subscription, activitiesDto: [ { currency: data.currency, @@ -221,8 +235,7 @@ export class ActivitiesController { type: data.type } ], - maxActivitiesToImport: 1, - user: this.request.user + maxActivitiesToImport: 1 }); } catch (error) { throw new HttpException( @@ -248,6 +261,7 @@ export class ActivitiesController { const activity = await this.activitiesService.createActivity({ ...data, + userId, date: parseISO(data.date), SymbolProfile: { connectOrCreate: { @@ -267,8 +281,7 @@ export class ActivitiesController { tags: data.tags?.map((id) => { return { id }; }), - user: { connect: { id: this.request.user.id } }, - userId: this.request.user.id + user: { connect: { id: userId } } }); if (dataSource && !isActivityInFuture({ date: activity.date })) { @@ -291,15 +304,17 @@ export class ActivitiesController { @HasPermission(permissions.updateActivity) @Put(':id') - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.activityUpdate) + @UseInterceptors(RedactValuesInResponseInterceptor) @UseInterceptors(TransformDataSourceInRequestInterceptor) public async updateActivity( - @Param('id') id: string, - @Body() data: UpdateOrderDto + @Body() data: UpdateOrderDto, + @Impersonation() { userId }: ImpersonationContext, + @Param('id') id: string ) { const originalActivity = await this.activitiesService.order({ id, - userId: this.request.user.id + userId }); if (!originalActivity) { @@ -326,13 +341,14 @@ export class ActivitiesController { delete data.dataSource; return this.activitiesService.updateActivity({ + userId, data: { ...data, date, account: accountId ? { connect: { - id_userId: { id: accountId, userId: this.request.user.id } + id_userId: { userId, id: accountId } } } : { disconnect: true }, @@ -352,10 +368,9 @@ export class ActivitiesController { tags: data.tags?.map((id) => { return { id }; }), - user: { connect: { id: this.request.user.id } } + user: { connect: { id: userId } } }, originalDate: originalActivity.date, - userId: this.request.user.id, where: { id } diff --git a/apps/api/src/app/endpoints/watchlist/watchlist.controller.ts b/apps/api/src/app/endpoints/watchlist/watchlist.controller.ts index 11f3ff84e..d83398139 100644 --- a/apps/api/src/app/endpoints/watchlist/watchlist.controller.ts +++ b/apps/api/src/app/endpoints/watchlist/watchlist.controller.ts @@ -1,17 +1,13 @@ import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; import { RequiresScope } from '@ghostfolio/api/decorators/requires-scope.decorator'; -import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor'; import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor'; import { CreateWatchlistItemDto } from '@ghostfolio/common/dtos'; import { WatchlistResponse } from '@ghostfolio/common/interfaces'; import { permissions } from '@ghostfolio/common/permissions'; import { scopes } from '@ghostfolio/common/scopes'; -import { - ImpersonationContext, - RequestWithUser -} from '@ghostfolio/common/types'; +import { ImpersonationContext } from '@ghostfolio/common/types'; import { Body, @@ -19,14 +15,10 @@ import { Delete, Get, HttpException, - Inject, Param, Post, - UseGuards, UseInterceptors } from '@nestjs/common'; -import { REQUEST } from '@nestjs/core'; -import { AuthGuard } from '@nestjs/passport'; import { DataSource } from '@prisma/client'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -34,34 +26,34 @@ import { WatchlistService } from './watchlist.service'; @Controller('watchlist') export class WatchlistController { - public constructor( - @Inject(REQUEST) private readonly request: RequestWithUser, - private readonly watchlistService: WatchlistService - ) {} + public constructor(private readonly watchlistService: WatchlistService) {} @Post() @HasPermission(permissions.createWatchlistItem) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.watchlistCreate) @UseInterceptors(TransformDataSourceInRequestInterceptor) - public async createWatchlistItem(@Body() data: CreateWatchlistItemDto) { + public async createWatchlistItem( + @Body() data: CreateWatchlistItemDto, + @Impersonation() { userId }: ImpersonationContext + ) { return this.watchlistService.createWatchlistItem({ + userId, dataSource: data.dataSource, - symbol: data.symbol, - userId: this.request.user.id + symbol: data.symbol }); } @Delete(':dataSource/:symbol') @HasPermission(permissions.deleteWatchlistItem) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @RequiresScope(scopes.watchlistDelete) @UseInterceptors(TransformDataSourceInRequestInterceptor) public async deleteWatchlistItem( + @Impersonation() { userId }: ImpersonationContext, @Param('dataSource') dataSource: DataSource, @Param('symbol') symbol: string ) { - const watchlistItems = await this.watchlistService.getWatchlistItems( - this.request.user.id - ); + const watchlistItems = + await this.watchlistService.getWatchlistItems(userId); const watchlistItem = watchlistItems.find((item) => { return item.dataSource === dataSource && item.symbol === symbol; @@ -77,7 +69,7 @@ export class WatchlistController { return this.watchlistService.deleteWatchlistItem({ dataSource, symbol, - userId: this.request.user.id + userId }); } diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index 6d5f932e0..26162b9e4 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -664,7 +664,7 @@ export class ImportService { activitiesDto, assetProfilesWithMarketDataDto, maxActivitiesToImport, - user + subscription: user.subscription }); const activitiesExtendedWithErrors = await this.extendActivitiesWithErrors({ diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index b2dd8b69a..5b277dee5 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -2,7 +2,6 @@ import { ActivitiesService } from '@ghostfolio/api/app/activities/activities.ser import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; import { RequiresScope } from '@ghostfolio/api/decorators/requires-scope.decorator'; -import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { hasNotDefinedValuesInObject, nullifyValuesInObject @@ -41,12 +40,10 @@ import { Param, Put, Query, - UseGuards, UseInterceptors, Version } from '@nestjs/common'; import { REQUEST } from '@nestjs/core'; -import { AuthGuard } from '@nestjs/passport'; import { AssetClass, AssetSubClass, DataSource } from '@prisma/client'; import { Big } from 'big.js'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -658,17 +655,18 @@ export class PortfolioController { @HasPermission(permissions.updateActivity) @Put('holding/:dataSource/:symbol/tags') + @RequiresScope(scopes.activityUpdate) @UseInterceptors(TransformDataSourceInRequestInterceptor) - @UseGuards(AuthGuard('jwt'), HasPermissionGuard) public async updateHoldingTags( @Body() data: UpdateHoldingTagsDto, + @Impersonation() { userId }: ImpersonationContext, @Param('dataSource') dataSource: DataSource, @Param('symbol') symbol: string ): Promise { const holding = await this.portfolioService.getHolding({ dataSource, symbol, - userId: this.request.user.id + userId }); if (!holding) { @@ -681,8 +679,8 @@ export class PortfolioController { await this.portfolioService.updateTags({ dataSource, symbol, - tags: data.tags, - userId: this.request.user.id + userId, + tags: data.tags }); } } diff --git a/apps/api/src/decorators/impersonation.decorator.ts b/apps/api/src/decorators/impersonation.decorator.ts index 3964388e5..0fe95f29f 100644 --- a/apps/api/src/decorators/impersonation.decorator.ts +++ b/apps/api/src/decorators/impersonation.decorator.ts @@ -1,28 +1,32 @@ -import { getScopesOfOwnAccess } from '@ghostfolio/common/scopes'; import type { ImpersonationContext, RequestWithUser } from '@ghostfolio/common/types'; -import { createParamDecorator, ExecutionContext } from '@nestjs/common'; +import { + createParamDecorator, + ExecutionContext, + InternalServerErrorException +} from '@nestjs/common'; /** - * Provides the impersonation context of the request, which requires the - * ImpersonationGuard to be applied to the route + * Provides the impersonation context of the request, which the + * ImpersonationGuard resolves. A missing context is a mistake in the setup of + * the route and fails loudly, because a fallback to the own access would let a + * handler change data without any scope being evaluated. */ export const Impersonation = createParamDecorator( (_data: unknown, context: ExecutionContext): ImpersonationContext => { - const { impersonation, user } = context + const { impersonation } = context .switchToHttp() .getRequest(); - return ( - impersonation ?? { - isActive: false, - scopes: getScopesOfOwnAccess(), - userId: user?.id, - userSettings: user?.settings?.settings ?? {} - } - ); + if (!impersonation) { + throw new InternalServerErrorException( + 'The impersonation context is missing. Apply the RequiresScope decorator or the ImpersonationGuard to the route.' + ); + } + + return impersonation; } ); diff --git a/apps/api/src/guards/impersonation-write.guard.spec.ts b/apps/api/src/guards/impersonation-write.guard.spec.ts new file mode 100644 index 000000000..e70e4b2ca --- /dev/null +++ b/apps/api/src/guards/impersonation-write.guard.spec.ts @@ -0,0 +1,112 @@ +import { ALLOW_DURING_IMPERSONATION_KEY } from '@ghostfolio/api/decorators/allow-during-impersonation.decorator'; +import { REQUIRES_SCOPE_KEY } from '@ghostfolio/api/decorators/requires-scope.decorator'; +import { HEADER_KEY_IMPERSONATION } from '@ghostfolio/common/config'; +import { Scope, scopes } from '@ghostfolio/common/scopes'; + +import { HttpException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { ExecutionContextHost } from '@nestjs/core/helpers/execution-context-host'; + +import { ImpersonationWriteGuard } from './impersonation-write.guard'; + +describe('Impersonation write guard', () => { + function createGuard({ + isAllowedDuringImpersonation, + requiredScopes + }: { + isAllowedDuringImpersonation?: boolean; + requiredScopes?: Scope[]; + } = {}) { + const reflector = { + getAllAndOverride: (key: string) => { + if (key === ALLOW_DURING_IMPERSONATION_KEY) { + return isAllowedDuringImpersonation; + } + + if (key === REQUIRES_SCOPE_KEY) { + return requiredScopes; + } + + return undefined; + } + } as unknown as Reflector; + + return new ImpersonationWriteGuard(reflector); + } + + function createExecutionContext({ + isImpersonating, + method + }: { + isImpersonating: boolean; + method: string; + }) { + return new ExecutionContextHost([ + { + method, + headers: isImpersonating + ? { + [HEADER_KEY_IMPERSONATION.toLowerCase()]: + 'ffb08949-2f8a-4b6e-88fd-0f1e6b6b5f5d' + } + : {} + } + ]); + } + + it('Allows a read request during an impersonation', () => { + expect( + createGuard().canActivate( + createExecutionContext({ isImpersonating: true, method: 'GET' }) + ) + ).toEqual(true); + }); + + it('Allows a write request without an impersonation', () => { + expect( + createGuard().canActivate( + createExecutionContext({ isImpersonating: false, method: 'POST' }) + ) + ).toEqual(true); + }); + + it('Blocks a write request of a route without scopes', () => { + const guard = createGuard(); + + expect(() => { + return guard.canActivate( + createExecutionContext({ isImpersonating: true, method: 'POST' }) + ); + }).toThrow(HttpException); + }); + + // A read scope must not open a route which changes data, because the + // ScopeGuard grants it to every read access + it('Blocks a write request of a route with read scopes only', () => { + const guard = createGuard({ requiredScopes: [scopes.portfolioRead] }); + + expect(() => { + return guard.canActivate( + createExecutionContext({ isImpersonating: true, method: 'POST' }) + ); + }).toThrow(HttpException); + }); + + it('Leaves a write request of a route with a write scope to the ScopeGuard', () => { + expect( + createGuard({ + requiredScopes: [scopes.activityCreate] + }).canActivate( + createExecutionContext({ isImpersonating: true, method: 'POST' }) + ) + ).toEqual(true); + }); + + it('Allows a write request of a route which is allowed during an impersonation', () => { + expect( + createGuard({ isAllowedDuringImpersonation: true }).canActivate( + createExecutionContext({ isImpersonating: true, method: 'POST' }) + ) + ).toEqual(true); + }); +}); diff --git a/apps/api/src/guards/impersonation-write.guard.ts b/apps/api/src/guards/impersonation-write.guard.ts index ec94c8e10..ed961c778 100644 --- a/apps/api/src/guards/impersonation-write.guard.ts +++ b/apps/api/src/guards/impersonation-write.guard.ts @@ -1,5 +1,7 @@ import { ALLOW_DURING_IMPERSONATION_KEY } from '@ghostfolio/api/decorators/allow-during-impersonation.decorator'; +import { REQUIRES_SCOPE_KEY } from '@ghostfolio/api/decorators/requires-scope.decorator'; import { HEADER_KEY_IMPERSONATION } from '@ghostfolio/common/config'; +import { SCOPES_OF_WRITE_ACCESS, Scope } from '@ghostfolio/common/scopes'; import { CanActivate, @@ -15,6 +17,12 @@ import { StatusCodes, getReasonPhrase } from 'http-status-codes'; * authenticated user cannot be changed from a view presenting data of the * impersonated user. The header is evaluated instead of the resolved context to * fail closed, also for an identifier which cannot be resolved. + * + * A route which declares a write scope is left to the ScopeGuard, which + * evaluates the resolved context. This guard is global, hence it runs before + * the guards of the route and cannot read the context itself. A route which + * declares read scopes only is still blocked here, so that a read access can + * never reach a handler which changes data. */ @Injectable() export class ImpersonationWriteGuard implements CanActivate { @@ -45,6 +53,19 @@ export class ImpersonationWriteGuard implements CanActivate { return true; } + const requiredScopes = this.reflector.getAllAndOverride( + REQUIRES_SCOPE_KEY, + [context.getHandler(), context.getClass()] + ); + + const requiresWriteScope = requiredScopes?.some((scope) => { + return SCOPES_OF_WRITE_ACCESS.includes(scope); + }); + + if (requiresWriteScope) { + return true; + } + throw new HttpException( getReasonPhrase(StatusCodes.FORBIDDEN), StatusCodes.FORBIDDEN diff --git a/apps/api/src/guards/impersonation.guard.spec.ts b/apps/api/src/guards/impersonation.guard.spec.ts new file mode 100644 index 000000000..50f0d4757 --- /dev/null +++ b/apps/api/src/guards/impersonation.guard.spec.ts @@ -0,0 +1,98 @@ +import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; +import { + HEADER_KEY_IMPERSONATION, + HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED +} from '@ghostfolio/common/config'; +import { getScopesOfOwnAccess, scopes } from '@ghostfolio/common/scopes'; +import type { ImpersonationContext } from '@ghostfolio/common/types'; + +import { HttpException } from '@nestjs/common'; +import { ExecutionContextHost } from '@nestjs/core/helpers/execution-context-host'; +import { StatusCodes } from 'http-status-codes'; + +import { ImpersonationGuard } from './impersonation.guard'; + +describe('Impersonation guard', () => { + const userId = 'ffb08949-2f8a-4b6e-88fd-0f1e6b6b5f5d'; + + function createGuard(impersonation: ImpersonationContext) { + const impersonationService = { + resolve: async () => { + return impersonation; + } + } as unknown as ImpersonationService; + + return new ImpersonationGuard(impersonationService); + } + + function createExecutionContext(impersonationId?: string) { + const request = { + headers: impersonationId + ? { [HEADER_KEY_IMPERSONATION.toLowerCase()]: impersonationId } + : {}, + user: { id: userId } + }; + + return { context: new ExecutionContextHost([request]), request }; + } + + it('Resolves the own access without an identifier', async () => { + const { context, request } = createExecutionContext(); + + const guard = createGuard({ + userId, + isActive: false, + scopes: getScopesOfOwnAccess(), + userSettings: {} + }); + + expect(await guard.canActivate(context)).toEqual(true); + expect(request['impersonation'].isActive).toEqual(false); + }); + + it('Resolves an identifier of a granted access', async () => { + const { context, request } = createExecutionContext('an-access-id'); + + const guard = createGuard({ + isActive: true, + scopes: [scopes.portfolioRead], + userId: 'e2d43f0d-1a41-4b6e-9d5b-6f9a2b7c8d1e', + userSettings: {} + }); + + expect(await guard.canActivate(context)).toEqual(true); + expect(request['impersonation'].scopes).toEqual([scopes.portfolioRead]); + }); + + // A revoked or stale identifier must not fall back to the own access, + // because the client keeps presenting the data as the impersonated data + it('Denies an identifier which cannot be resolved', async () => { + const { context } = createExecutionContext('a-revoked-access-id'); + + const guard = createGuard({ + userId, + isActive: false, + scopes: getScopesOfOwnAccess(), + userSettings: {} + }); + + await expect(guard.canActivate(context)).rejects.toThrow(HttpException); + }); + + // The client relies on this message to remove the stale identifier + it('Denies an identifier which cannot be resolved with a distinct message', async () => { + const { context } = createExecutionContext('a-revoked-access-id'); + + const guard = createGuard({ + userId, + isActive: false, + scopes: getScopesOfOwnAccess(), + userSettings: {} + }); + + await expect(guard.canActivate(context)).rejects.toMatchObject({ + response: { message: HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED }, + status: StatusCodes.FORBIDDEN + }); + }); +}); diff --git a/apps/api/src/guards/impersonation.guard.ts b/apps/api/src/guards/impersonation.guard.ts index 88500f1b3..1269cff85 100644 --- a/apps/api/src/guards/impersonation.guard.ts +++ b/apps/api/src/guards/impersonation.guard.ts @@ -1,9 +1,24 @@ import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; -import { HEADER_KEY_IMPERSONATION } from '@ghostfolio/common/config'; +import { + HEADER_KEY_IMPERSONATION, + HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED +} from '@ghostfolio/common/config'; import type { RequestWithUser } from '@ghostfolio/common/types'; -import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; +import { + CanActivate, + ExecutionContext, + HttpException, + Injectable +} from '@nestjs/common'; +import { StatusCodes, getReasonPhrase } from 'http-status-codes'; +/** + * Resolves the impersonation context of the request. An identifier which + * cannot be resolved is rejected instead of falling back to the own access, so + * that a revoked or stale identifier can never present the data of the + * authenticated user as the data of the impersonated user. + */ @Injectable() export class ImpersonationGuard implements CanActivate { public constructor( @@ -13,13 +28,27 @@ export class ImpersonationGuard implements CanActivate { public async canActivate(context: ExecutionContext) { const request = context.switchToHttp().getRequest(); + const impersonationId = request.headers?.[ + HEADER_KEY_IMPERSONATION.toLowerCase() + ] as string; + request.impersonation = await this.impersonationService.resolve({ - impersonationId: request.headers?.[ - HEADER_KEY_IMPERSONATION.toLowerCase() - ] as string, + impersonationId, user: request.user }); + if (impersonationId && !request.impersonation.isActive) { + // The message is distinct from any other forbidden response, so that the + // client can remove the stale identifier instead of failing every request + throw new HttpException( + { + error: getReasonPhrase(StatusCodes.FORBIDDEN), + message: HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED + }, + StatusCodes.FORBIDDEN + ); + } + return true; } } diff --git a/apps/api/src/guards/scope.guard.spec.ts b/apps/api/src/guards/scope.guard.spec.ts new file mode 100644 index 000000000..5b5ab4f3c --- /dev/null +++ b/apps/api/src/guards/scope.guard.spec.ts @@ -0,0 +1,61 @@ +import { Scope, scopes } from '@ghostfolio/common/scopes'; + +import { HttpException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { ExecutionContextHost } from '@nestjs/core/helpers/execution-context-host'; + +import { ScopeGuard } from './scope.guard'; + +describe('Scope guard', () => { + function createGuard(requiredScopes?: Scope[]) { + const reflector = { + getAllAndOverride: () => { + return requiredScopes; + } + } as unknown as Reflector; + + return new ScopeGuard(reflector); + } + + function createExecutionContext(scopesOfImpersonation?: string[]) { + return new ExecutionContextHost([ + { + impersonation: scopesOfImpersonation + ? { scopes: scopesOfImpersonation } + : undefined + } + ]); + } + + it('Allows a route without required scopes', () => { + expect(createGuard().canActivate(createExecutionContext())).toEqual(true); + }); + + it('Allows a context which covers every required scope', () => { + expect( + createGuard([scopes.accountRead, scopes.accountUpdate]).canActivate( + createExecutionContext([ + scopes.accountRead, + scopes.accountUpdate, + scopes.portfolioRead + ]) + ) + ).toEqual(true); + }); + + it('Denies a context which covers one of two required scopes', () => { + const guard = createGuard([scopes.accountRead, scopes.accountUpdate]); + + expect(() => { + return guard.canActivate(createExecutionContext([scopes.accountRead])); + }).toThrow(HttpException); + }); + + it('Denies a missing context', () => { + const guard = createGuard([scopes.accountRead]); + + expect(() => { + return guard.canActivate(createExecutionContext()); + }).toThrow(HttpException); + }); +}); diff --git a/apps/api/src/helper/object.helper.spec.ts b/apps/api/src/helper/object.helper.spec.ts index ba8760c70..5a442ca9e 100644 --- a/apps/api/src/helper/object.helper.spec.ts +++ b/apps/api/src/helper/object.helper.spec.ts @@ -3036,4 +3036,91 @@ describe('redactAttributes', () => { }); console.timeEnd('redactAttributes execution time'); }); + + // An activity is a response of its own, hence it has to be redacted like an + // entry of the activities of a portfolio + it('should redact an activity which is the response itself', () => { + expect( + redactPaths({ + object: { + account: { + comment: 'Private note', + id: '480269ce-e12a-4fd1-ac88-c4b0ff3f899c', + name: 'Interactive Brokers Account' + }, + assetProfile: { + name: 'Apple Inc', + symbol: 'AAPL', + symbolMapping: { YAHOO: 'AAPL' }, + watchedByCount: 7 + }, + comment: 'Bought on a dip', + currency: 'USD', + date: '2021-11-30T23:00:00.000Z', + fee: 19.9, + feeInAssetProfileCurrency: 19.9, + feeInBaseCurrency: 18.2, + id: '8c623328-6035-4b5f-b6d5-702cc1c9c56b', + quantity: 50, + type: 'BUY', + unitPrice: 220.79, + value: 11039.5, + valueInBaseCurrency: 10123.4 + }, + paths: DEFAULT_REDACTED_PATHS + }) + ).toStrictEqual({ + account: { + comment: null, + id: '480269ce-e12a-4fd1-ac88-c4b0ff3f899c', + name: 'Interactive Brokers Account' + }, + assetProfile: { + name: 'Apple Inc', + symbol: 'AAPL', + symbolMapping: null, + watchedByCount: null + }, + comment: null, + currency: 'USD', + date: '2021-11-30T23:00:00.000Z', + fee: null, + feeInAssetProfileCurrency: null, + feeInBaseCurrency: null, + id: '8c623328-6035-4b5f-b6d5-702cc1c9c56b', + quantity: null, + type: 'BUY', + // A price per unit stays visible, like the average price and the market + // price of a holding + unitPrice: 220.79, + value: null, + valueInBaseCurrency: null + }); + }); + + // The write endpoints return a row of the database, which has no relation + it('should redact an activity without the relations', () => { + expect( + redactPaths({ + object: { + comment: 'Bought on a dip', + currency: 'USD', + fee: 19.9, + id: '8c623328-6035-4b5f-b6d5-702cc1c9c56b', + quantity: 50, + type: 'BUY', + unitPrice: 220.79 + }, + paths: DEFAULT_REDACTED_PATHS + }) + ).toStrictEqual({ + comment: null, + currency: 'USD', + fee: null, + id: '8c623328-6035-4b5f-b6d5-702cc1c9c56b', + quantity: null, + type: 'BUY', + unitPrice: 220.79 + }); + }); }); diff --git a/apps/api/src/services/data-provider/data-provider.service.ts b/apps/api/src/services/data-provider/data-provider.service.ts index 5484688b4..e999bd595 100644 --- a/apps/api/src/services/data-provider/data-provider.service.ts +++ b/apps/api/src/services/data-provider/data-provider.service.ts @@ -213,7 +213,7 @@ export class DataProviderService implements OnModuleInit { activitiesDto, assetProfilesWithMarketDataDto, maxActivitiesToImport, - user + subscription }: { activitiesDto: Pick< Partial, @@ -221,7 +221,7 @@ export class DataProviderService implements OnModuleInit { >[]; assetProfilesWithMarketDataDto?: ImportDataDto['assetProfiles']; maxActivitiesToImport: number; - user: UserWithSettings; + subscription: UserWithSettings['subscription']; }) { if (activitiesDto?.length > maxActivitiesToImport) { throw new Error(`Too many activities (${maxActivitiesToImport} at most)`); @@ -255,7 +255,7 @@ export class DataProviderService implements OnModuleInit { if ( this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - user.subscription?.type === SubscriptionType.Basic + subscription?.type === SubscriptionType.Basic ) { const dataProvider = this.getDataProvider(DataSource[dataSource]); diff --git a/apps/api/src/services/impersonation/impersonation.module.ts b/apps/api/src/services/impersonation/impersonation.module.ts index e4f503790..ff15af11a 100644 --- a/apps/api/src/services/impersonation/impersonation.module.ts +++ b/apps/api/src/services/impersonation/impersonation.module.ts @@ -1,10 +1,12 @@ +import { SubscriptionModule } from '@ghostfolio/api/app/subscription/subscription.module'; +import { ConfigurationModule } from '@ghostfolio/api/services/configuration/configuration.module'; import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; import { PrismaModule } from '@ghostfolio/api/services/prisma/prisma.module'; import { Module } from '@nestjs/common'; @Module({ - imports: [PrismaModule], + imports: [ConfigurationModule, PrismaModule, SubscriptionModule], providers: [ImpersonationService], exports: [ImpersonationService] }) diff --git a/apps/api/src/services/impersonation/impersonation.service.spec.ts b/apps/api/src/services/impersonation/impersonation.service.spec.ts new file mode 100644 index 000000000..d78a66f9a --- /dev/null +++ b/apps/api/src/services/impersonation/impersonation.service.spec.ts @@ -0,0 +1,245 @@ +import { SubscriptionService } from '@ghostfolio/api/app/subscription/subscription.service'; +import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; +import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; +import { DEFAULT_CURRENCY } from '@ghostfolio/common/config'; +import { SubscriptionType } from '@ghostfolio/common/enums'; +import { permissions } from '@ghostfolio/common/permissions'; +import { + getScopesOfOwnAccess, + getScopesOfUnrestrictedImpersonation, + scopes +} from '@ghostfolio/common/scopes'; +import type { UserWithSettings } from '@ghostfolio/common/types'; + +import { Access } from '@prisma/client'; + +import { ImpersonationService } from './impersonation.service'; + +describe('Impersonation service', () => { + const accessId = 'a5d3f2c1-9b4e-4c8a-8f2d-1e6b7c9a0d3f'; + const authenticatedUserId = 'ffb08949-2f8a-4b6e-88fd-0f1e6b6b5f5d'; + const impersonatedUserId = 'e2d43f0d-1a41-4b6e-9d5b-6f9a2b7c8d1e'; + + const authenticatedUser = { + id: authenticatedUserId, + permissions: [], + settings: { settings: { baseCurrency: 'CHF' } }, + subscription: { type: SubscriptionType.Premium } + } as unknown as UserWithSettings; + + function createService({ + access, + impersonatedUser, + isSubscriptionEnabled = false + }: { + access?: Partial; + impersonatedUser?: unknown; + isSubscriptionEnabled?: boolean; + } = {}) { + const getSubscription = jest.fn().mockResolvedValue({ + type: SubscriptionType.Basic + }); + + const configurationService = { + get: (key: string) => { + return key === 'ENABLE_FEATURE_SUBSCRIPTION' + ? isSubscriptionEnabled + : undefined; + } + } as unknown as ConfigurationService; + + const prismaService = { + access: { + findFirst: async () => { + return access ?? null; + } + }, + user: { + findUnique: async () => { + return impersonatedUser ?? null; + } + } + } as unknown as PrismaService; + + const subscriptionService = { + getSubscription + } as unknown as SubscriptionService; + + return { + getSubscription, + service: new ImpersonationService( + configurationService, + prismaService, + subscriptionService + ) + }; + } + + describe('Without an impersonation', () => { + it('Resolves the own access of the authenticated user', async () => { + const { service } = createService(); + + expect(await service.resolve({ user: authenticatedUser })).toEqual({ + authenticatedUserSubscription: authenticatedUser.subscription, + isActive: false, + scopes: getScopesOfOwnAccess(), + userId: authenticatedUserId, + userSettings: { baseCurrency: 'CHF' }, + userSubscription: authenticatedUser.subscription + }); + }); + + it('Resolves a user without settings', async () => { + const { service } = createService(); + + const { userSettings } = await service.resolve({ + user: { id: authenticatedUserId } as UserWithSettings + }); + + expect(userSettings).toEqual({}); + }); + }); + + describe('With an impersonation', () => { + const grantedAccess = { + granteeUserId: authenticatedUserId, + id: accessId, + permissions: ['READ'], + scopes: [scopes.portfolioRead], + userId: impersonatedUserId + } as unknown as Access; + + const impersonatedUser = { + createdAt: new Date('2024-01-01'), + id: impersonatedUserId, + settings: { settings: { baseCurrency: 'USD' } }, + subscriptions: [] + }; + + it('Resolves the scopes of the granted access', async () => { + const { service } = createService({ + access: grantedAccess, + impersonatedUser + }); + + expect( + await service.resolve({ + impersonationId: accessId, + user: authenticatedUser + }) + ).toEqual({ + accessId, + authenticatedUserSubscription: authenticatedUser.subscription, + isActive: true, + scopes: [scopes.portfolioRead], + userId: impersonatedUserId, + userSettings: { baseCurrency: 'USD' }, + userSubscription: undefined + }); + }); + + // The subscription of the authenticated user is required to evaluate the + // more restrictive of the two subscriptions + it('Keeps the subscription of the authenticated user', async () => { + const { service } = createService({ + access: grantedAccess, + impersonatedUser + }); + + const { authenticatedUserSubscription } = await service.resolve({ + impersonationId: accessId, + user: authenticatedUser + }); + + expect(authenticatedUserSubscription).toEqual( + authenticatedUser.subscription + ); + }); + + it('Falls back to the default currency without settings', async () => { + const { service } = createService({ + access: grantedAccess, + impersonatedUser: { ...impersonatedUser, settings: null } + }); + + const { userSettings } = await service.resolve({ + impersonationId: accessId, + user: authenticatedUser + }); + + expect(userSettings).toEqual({ baseCurrency: DEFAULT_CURRENCY }); + }); + + it('Omits the subscription while the feature is disabled', async () => { + const { getSubscription, service } = createService({ + access: grantedAccess, + impersonatedUser + }); + + const { userSubscription } = await service.resolve({ + impersonationId: accessId, + user: authenticatedUser + }); + + expect(userSubscription).toBeUndefined(); + expect(getSubscription).not.toHaveBeenCalled(); + }); + + it('Resolves the subscription while the feature is enabled', async () => { + const { getSubscription, service } = createService({ + access: grantedAccess, + impersonatedUser, + isSubscriptionEnabled: true + }); + + const { userSubscription } = await service.resolve({ + impersonationId: accessId, + user: authenticatedUser + }); + + expect(userSubscription).toEqual({ type: SubscriptionType.Basic }); + expect(getSubscription).toHaveBeenCalledWith({ + createdAt: impersonatedUser.createdAt, + subscriptions: [] + }); + }); + + // An administrator impersonates by a user id instead of an access id + it('Resolves the unrestricted scopes of an administrator', async () => { + const { service } = createService({ + impersonatedUser: { id: impersonatedUserId } + }); + + const { isActive, scopes: scopesOfImpersonation } = await service.resolve( + { + impersonationId: impersonatedUserId, + user: { + ...authenticatedUser, + permissions: [permissions.impersonateAllUsers] + } as UserWithSettings + } + ); + + expect(isActive).toEqual(true); + expect(scopesOfImpersonation).toEqual( + getScopesOfUnrestrictedImpersonation() + ); + }); + }); + + // The guard rejects the request in this case, hence the context must not + // present the data of the authenticated user as impersonated data + describe('With an identifier which cannot be resolved', () => { + it('Resolves the own access instead', async () => { + const { service } = createService(); + + const { isActive, userId } = await service.resolve({ + impersonationId: 'a-revoked-access-id', + user: authenticatedUser + }); + + expect(isActive).toEqual(false); + expect(userId).toEqual(authenticatedUserId); + }); + }); +}); diff --git a/apps/api/src/services/impersonation/impersonation.service.ts b/apps/api/src/services/impersonation/impersonation.service.ts index 1432c14aa..e1df1d3ce 100644 --- a/apps/api/src/services/impersonation/impersonation.service.ts +++ b/apps/api/src/services/impersonation/impersonation.service.ts @@ -1,3 +1,5 @@ +import { SubscriptionService } from '@ghostfolio/api/app/subscription/subscription.service'; +import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; import { DEFAULT_CURRENCY } from '@ghostfolio/common/config'; import { UserSettings } from '@ghostfolio/common/interfaces'; @@ -17,7 +19,11 @@ import { Access } from '@prisma/client'; @Injectable() export class ImpersonationService { - public constructor(private readonly prismaService: PrismaService) {} + public constructor( + private readonly configurationService: ConfigurationService, + private readonly prismaService: PrismaService, + private readonly subscriptionService: SubscriptionService + ) {} public async resolve({ impersonationId, @@ -31,19 +37,29 @@ export class ImpersonationService { if (!impersonatedUserId) { return { + authenticatedUserSubscription: user?.subscription, isActive: false, scopes: getScopesOfOwnAccess(), userId: user?.id, - userSettings: user?.settings?.settings ?? {} + userSettings: user?.settings?.settings ?? {}, + userSubscription: user?.subscription }; } - const settings = await this.prismaService.settings.findUnique({ - where: { userId: impersonatedUserId } + const isSubscriptionEnabled = this.configurationService.get( + 'ENABLE_FEATURE_SUBSCRIPTION' + ); + + const impersonatedUser = await this.prismaService.user.findUnique({ + include: { settings: true, subscriptions: isSubscriptionEnabled }, + where: { id: impersonatedUserId } }); + const settings = impersonatedUser?.settings?.settings as UserSettings; + return { accessId: impersonationId, + authenticatedUserSubscription: user?.subscription, isActive: true, // An access which has not been granted explicitly originates from the // permission to impersonate all users @@ -52,10 +68,16 @@ export class ImpersonationService { : getScopesOfUnrestrictedImpersonation(), userId: impersonatedUserId, userSettings: { - ...((settings?.settings ?? {}) as UserSettings), - baseCurrency: - (settings?.settings as UserSettings)?.baseCurrency ?? DEFAULT_CURRENCY - } + ...(settings ?? {}), + baseCurrency: settings?.baseCurrency ?? DEFAULT_CURRENCY + }, + userSubscription: + isSubscriptionEnabled && impersonatedUser + ? await this.subscriptionService.getSubscription({ + createdAt: impersonatedUser.createdAt, + subscriptions: impersonatedUser.subscriptions ?? [] + }) + : undefined }; } diff --git a/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts b/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts index 28e713a41..22585efc6 100644 --- a/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts +++ b/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts @@ -1,3 +1,4 @@ +import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { UserService } from '@ghostfolio/client/services/user/user.service'; import { DEFAULT_PAGE_SIZE, @@ -203,6 +204,9 @@ export class GfHoldingDetailDialogComponent implements OnInit { private readonly dataService = inject(DataService); private readonly destroyRef = inject(DestroyRef); private readonly formBuilder = inject(FormBuilder); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); private readonly router = inject(Router); private readonly userService = inject(UserService); @@ -586,10 +590,12 @@ export class GfHoldingDetailDialogComponent implements OnInit { if (state?.user) { this.user = state.user; - this.hasPermissionToCreateOwnTag = hasPermission( - this.user?.permissions, - permissions.createOwnTag - ); + // A tag created during an impersonation belongs to the authenticated + // user, hence it cannot be assigned to the data of the impersonated + // user + this.hasPermissionToCreateOwnTag = + !this.impersonationStorageService.getId() && + hasPermission(this.user?.permissions, permissions.createOwnTag); this.tagsAvailable = this.user?.tags diff --git a/apps/client/src/app/core/http-response.interceptor.ts b/apps/client/src/app/core/http-response.interceptor.ts index 7385e090c..42c5b3ffc 100644 --- a/apps/client/src/app/core/http-response.interceptor.ts +++ b/apps/client/src/app/core/http-response.interceptor.ts @@ -1,5 +1,7 @@ +import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { UserService } from '@ghostfolio/client/services/user/user.service'; import { WebAuthnService } from '@ghostfolio/client/services/web-authn.service'; +import { HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED } from '@ghostfolio/common/config'; import { InfoItem } from '@ghostfolio/common/interfaces'; import { internalRoutes, publicRoutes } from '@ghostfolio/common/routes/routes'; import { DataService } from '@ghostfolio/ui/services'; @@ -30,6 +32,9 @@ export class HttpResponseInterceptor implements HttpInterceptor { private snackBarRef: MatSnackBarRef | undefined; private readonly dataService = inject(DataService); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); private readonly router = inject(Router); private readonly snackBar = inject(MatSnackBar); private readonly userService = inject(UserService); @@ -46,6 +51,19 @@ export class HttpResponseInterceptor implements HttpInterceptor { return next.handle(request).pipe( catchError((error: HttpErrorResponse) => { if (error.status === StatusCodes.FORBIDDEN) { + if ( + error.error?.message === + HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED + ) { + // A stale identifier fails every guarded request, hence it is + // removed to make the application usable again + this.impersonationStorageService.removeId(); + + window.location.reload(); + + return throwError(error); + } + if (!this.snackBarRef) { if (this.info.isReadOnlyMode) { this.snackBarRef = this.snackBar.open( diff --git a/apps/client/src/app/pages/accounts/create-or-update-account-dialog/create-or-update-account-dialog.component.ts b/apps/client/src/app/pages/accounts/create-or-update-account-dialog/create-or-update-account-dialog.component.ts index b46ef3aff..4339f570b 100644 --- a/apps/client/src/app/pages/accounts/create-or-update-account-dialog/create-or-update-account-dialog.component.ts +++ b/apps/client/src/app/pages/accounts/create-or-update-account-dialog/create-or-update-account-dialog.component.ts @@ -1,3 +1,4 @@ +import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { UserService } from '@ghostfolio/client/services/user/user.service'; import { TAG_ID_DRAFT } from '@ghostfolio/common/config'; import { CreateAccountDto, UpdateAccountDto } from '@ghostfolio/common/dtos'; @@ -75,6 +76,9 @@ export class GfCreateOrUpdateAccountDialogComponent { private readonly dialogRef = inject>(MatDialogRef); private readonly formBuilder = inject(FormBuilder); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); private readonly userService = inject(UserService); protected get selectedPlatform() { @@ -87,10 +91,11 @@ export class GfCreateOrUpdateAccountDialogComponent { const { currencies } = this.dataService.fetchInfo(); this.currencies = currencies; - this.hasPermissionToCreateOwnTag = hasPermission( - this.data.user?.permissions, - permissions.createOwnTag - ); + // A tag created during an impersonation belongs to the authenticated user, + // hence it cannot be assigned to the data of the impersonated user + this.hasPermissionToCreateOwnTag = + !this.impersonationStorageService.getId() && + hasPermission(this.data.user?.permissions, permissions.createOwnTag); this.tagsAvailable = this.data.user?.tags diff --git a/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts b/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts index 2484f6afe..8f1447088 100644 --- a/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts +++ b/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts @@ -1,3 +1,4 @@ +import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { UserService } from '@ghostfolio/client/services/user/user.service'; import { ASSET_CLASS_MAPPING, DEFAULT_LOCALE } from '@ghostfolio/common/config'; import { CreateOrderDto, UpdateOrderDto } from '@ghostfolio/common/dtos'; @@ -115,6 +116,9 @@ export class GfCreateOrUpdateActivityDialogComponent { private readonly dialogRef = inject>(MatDialogRef); private readonly formBuilder = inject(FormBuilder); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); private locale = inject(MAT_DATE_LOCALE); private readonly userService = inject(UserService); @@ -124,10 +128,13 @@ export class GfCreateOrUpdateActivityDialogComponent { public ngOnInit() { this.currencyOfAssetProfile = this.data.activity?.assetProfile?.currency; - this.hasPermissionToCreateOwnTag = hasPermission( - this.data.user?.permissions, - permissions.createOwnTag - ); + + // A tag created during an impersonation belongs to the authenticated user, + // hence it cannot be assigned to the data of the impersonated user + this.hasPermissionToCreateOwnTag = + !this.impersonationStorageService.getId() && + hasPermission(this.data.user?.permissions, permissions.createOwnTag); + this.locale = this.data.user.settings.locale ?? DEFAULT_LOCALE; this.mode = this.data.activity?.id ? 'update' : 'create'; diff --git a/libs/common/src/lib/config.ts b/libs/common/src/lib/config.ts index 84d678458..f3ef2c30e 100644 --- a/libs/common/src/lib/config.ts +++ b/libs/common/src/lib/config.ts @@ -108,6 +108,7 @@ export const DEFAULT_PROCESSOR_PORTFOLIO_SNAPSHOT_COMPUTATION_TIMEOUT = ms('30 seconds'); export const DEFAULT_REDACTED_PATHS = [ + 'account.comment', 'accounts[*].balance', 'accounts[*].balanceInBaseCurrency', 'accounts[*].comment', @@ -126,6 +127,8 @@ export const DEFAULT_REDACTED_PATHS = [ 'activities[*].quantity', 'activities[*].value', 'activities[*].valueInBaseCurrency', + 'assetProfile.symbolMapping', + 'assetProfile.watchedByCount', 'balance', 'balanceInBaseCurrency', 'balances[*].account.comment', @@ -133,6 +136,8 @@ export const DEFAULT_REDACTED_PATHS = [ 'balances[*].valueInBaseCurrency', 'comment', 'dividendInBaseCurrency', + 'fee', + 'feeInAssetProfileCurrency', 'feeInBaseCurrency', 'grossPerformance', 'grossPerformanceWithCurrencyEffect', @@ -249,6 +254,9 @@ export const HEADER_KEY_TIMEZONE = 'Timezone'; export const HEADER_KEY_TOKEN = 'Authorization'; export const HEADER_KEY_SKIP_INTERCEPTOR = 'X-Skip-Interceptor'; +export const HTTP_RESPONSE_MESSAGE_IMPERSONATION_UNRESOLVED = + 'The impersonation identifier cannot be resolved'; + export const MAX_TOP_HOLDINGS = 50; export const NUMERICAL_PRECISION_THRESHOLD_3_FIGURES = 100; diff --git a/libs/common/src/lib/scopes.spec.ts b/libs/common/src/lib/scopes.spec.ts index feac97028..cb182207e 100644 --- a/libs/common/src/lib/scopes.spec.ts +++ b/libs/common/src/lib/scopes.spec.ts @@ -1,4 +1,6 @@ import { + SCOPES_OF_READ_ACCESS, + SCOPES_OF_WRITE_ACCESS, getScopesOfAccess, getScopesOfOwnAccess, getScopesOfUnrestrictedImpersonation, @@ -7,6 +9,51 @@ import { } from '@ghostfolio/common/scopes'; describe('Scopes', () => { + describe('Scopes of read access', () => { + // A new scope which reads data has to be added here deliberately, because + // an access with the permission to read receives this list + it('Covers every read scope', () => { + expect(SCOPES_OF_READ_ACCESS).toEqual([ + scopes.accountRead, + scopes.activityRead, + scopes.portfolioRead, + scopes.portfolioReadValues, + scopes.watchlistRead + ]); + }); + }); + + describe('Scopes of write access', () => { + // A new scope which changes data has to be added here deliberately, + // because the ImpersonationWriteGuard blocks the writes it does not cover + it('Covers every write scope', () => { + expect(SCOPES_OF_WRITE_ACCESS).toEqual([ + scopes.accountCreate, + scopes.accountDelete, + scopes.accountUpdate, + scopes.activityCreate, + scopes.activityDelete, + scopes.activityUpdate, + scopes.watchlistCreate, + scopes.watchlistDelete + ]); + }); + }); + + describe('Scopes of read and write access', () => { + // A new scope has to belong to exactly one of the two lists. A scope which + // belongs to neither list is granted to nobody, and a write scope which is + // missing from SCOPES_OF_WRITE_ACCESS is granted to every read access. + it('Cover every scope exactly once', () => { + const scopesOfReadAndWriteAccess = [ + ...SCOPES_OF_READ_ACCESS, + ...SCOPES_OF_WRITE_ACCESS + ].sort(); + + expect(scopesOfReadAndWriteAccess).toEqual(Object.values(scopes).sort()); + }); + }); + describe('Get scopes of access', () => { it('Scopes take precedence over the permissions', () => { expect( @@ -39,6 +86,18 @@ describe('Scopes', () => { ).not.toContain(scopes.portfolioReadValues); }); + it('The permission to read gives no write scope', () => { + const scopesOfAccess = getScopesOfAccess({ + granteeUserId: 'ffb08949-2f8a-4b6e-88fd-0f1e6b6b5f5d', + permissions: ['READ'], + scopes: [] + }); + + for (const scope of SCOPES_OF_WRITE_ACCESS) { + expect(scopesOfAccess).not.toContain(scope); + } + }); + it('Without permissions and scopes', () => { expect( getScopesOfAccess({ @@ -88,10 +147,18 @@ describe('Scopes', () => { // granted to the owner of the data it('Covers every scope', () => { expect(getScopesOfOwnAccess()).toEqual([ + scopes.accountCreate, + scopes.accountDelete, scopes.accountRead, + scopes.accountUpdate, + scopes.activityCreate, + scopes.activityDelete, scopes.activityRead, + scopes.activityUpdate, scopes.portfolioRead, scopes.portfolioReadValues, + scopes.watchlistCreate, + scopes.watchlistDelete, scopes.watchlistRead ]); }); @@ -100,7 +167,7 @@ describe('Scopes', () => { describe('Get scopes of unrestricted impersonation', () => { // A new scope has to be added here deliberately to confirm that it is // granted to an administrator impersonating an arbitrary user - it('Covers every scope but the monetary values', () => { + it('Covers every read scope but the monetary values', () => { expect(getScopesOfUnrestrictedImpersonation()).toEqual([ scopes.accountRead, scopes.activityRead, @@ -108,6 +175,14 @@ describe('Scopes', () => { scopes.watchlistRead ]); }); + + it('Gives no write scope', () => { + const scopesOfImpersonation = getScopesOfUnrestrictedImpersonation(); + + for (const scope of SCOPES_OF_WRITE_ACCESS) { + expect(scopesOfImpersonation).not.toContain(scope); + } + }); }); describe('Has scope', () => { diff --git a/libs/common/src/lib/scopes.ts b/libs/common/src/lib/scopes.ts index fda81b4a9..dc12b4ca9 100644 --- a/libs/common/src/lib/scopes.ts +++ b/libs/common/src/lib/scopes.ts @@ -7,27 +7,57 @@ import { AccessPermission } from '@prisma/client'; * the authenticated user and never widen it. */ export const scopes = { + accountCreate: 'account:create', + accountDelete: 'account:delete', accountRead: 'account:read', + accountUpdate: 'account:update', + activityCreate: 'activity:create', + activityDelete: 'activity:delete', activityRead: 'activity:read', + activityUpdate: 'activity:update', portfolioRead: 'portfolio:read', portfolioReadValues: 'portfolio:read:values', + watchlistCreate: 'watchlist:create', + watchlistDelete: 'watchlist:delete', watchlistRead: 'watchlist:read' } as const; export type Scope = (typeof scopes)[keyof typeof scopes]; -const SCOPES_OF_PUBLIC_ACCESS: Scope[] = [ +/** + * Scopes which read data + */ +export const SCOPES_OF_READ_ACCESS: readonly Scope[] = [ + scopes.accountRead, scopes.activityRead, - scopes.portfolioRead + scopes.portfolioRead, + scopes.portfolioReadValues, + scopes.watchlistRead ]; -const SCOPES_OF_READ_ACCESS = Object.values(scopes); +/** + * Scopes which change data + */ +export const SCOPES_OF_WRITE_ACCESS: readonly Scope[] = [ + scopes.accountCreate, + scopes.accountDelete, + scopes.accountUpdate, + scopes.activityCreate, + scopes.activityDelete, + scopes.activityUpdate, + scopes.watchlistCreate, + scopes.watchlistDelete +]; -const SCOPES_OF_READ_RESTRICTED_ACCESS = SCOPES_OF_READ_ACCESS.filter( - (scope) => { +const SCOPES_OF_PUBLIC_ACCESS: readonly Scope[] = [ + scopes.activityRead, + scopes.portfolioRead +]; + +const SCOPES_OF_READ_RESTRICTED_ACCESS: readonly Scope[] = + SCOPES_OF_READ_ACCESS.filter((scope) => { return scope !== scopes.portfolioReadValues; - } -); + }); export function getScopesOfAccess({ granteeUserId, @@ -38,22 +68,24 @@ export function getScopesOfAccess({ permissions?: AccessPermission[]; scopes?: string[]; }): string[] { - if (!scopesOfAccess?.length) { + let scopesToEvaluate: readonly string[] = scopesOfAccess ?? []; + + if (!scopesToEvaluate.length) { // TODO: Remove the derivation from the permissions once they have been // dropped from the access - scopesOfAccess = permissions?.includes('READ') + scopesToEvaluate = permissions?.includes('READ') ? SCOPES_OF_READ_ACCESS : SCOPES_OF_READ_RESTRICTED_ACCESS; } if (granteeUserId) { - return [...scopesOfAccess]; + return [...scopesToEvaluate]; } // An access which has not been granted to a user is public, hence it is // narrowed to the scopes exposed by the public endpoints return SCOPES_OF_PUBLIC_ACCESS.filter((scope) => { - return scopesOfAccess.includes(scope); + return scopesToEvaluate.includes(scope); }); } diff --git a/libs/common/src/lib/types/impersonation-context.type.ts b/libs/common/src/lib/types/impersonation-context.type.ts index 8bdfb9bb3..29b7970b1 100644 --- a/libs/common/src/lib/types/impersonation-context.type.ts +++ b/libs/common/src/lib/types/impersonation-context.type.ts @@ -1,14 +1,18 @@ import { UserSettings } from '@ghostfolio/common/interfaces'; +import { UserWithSettings } from '@ghostfolio/common/types'; /** - * Describes whose data a request presents. The user id and the settings belong - * to the impersonated user while an impersonation is active and to the - * authenticated user otherwise, so a handler can use them unconditionally. + * Describes whose data a request presents. The user id, the settings and the + * subscription belong to the impersonated user while an impersonation is + * active and to the authenticated user otherwise, so a handler can use them + * unconditionally. */ export interface ImpersonationContext { accessId?: string; + authenticatedUserSubscription?: UserWithSettings['subscription']; isActive: boolean; scopes: string[]; userId: string; userSettings: UserSettings; + userSubscription?: UserWithSettings['subscription']; }