diff --git a/CHANGELOG.md b/CHANGELOG.md index 3035d0eb2..d2af41573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Added a tool to import activities into the portfolio to the server of the Model Context Protocol (MCP) (experimental) + ### Changed +- Extended the access to share the portfolio to support the _Restricted view and manage_ permission (experimental) - Extended the tool to get the accounts of the portfolio in the server of the Model Context Protocol (MCP) to support the filtering by account (experimental) - Upgraded `replace-in-file` from version `8.4.0` to `9.0.0` - Upgraded `stripe` from version `22.3.2` to `22.5.0` diff --git a/README.md b/README.md index cb69cdc64..7c6f5ee7a 100644 --- a/README.md +++ b/README.md @@ -358,7 +358,7 @@ Grant access of type _Public_ in the _Access_ tab of _My Ghostfolio_. ## Model Context Protocol (experimental) -The _Model Context Protocol_ (MCP) server lets an AI client read your portfolio. +The _Model Context Protocol_ (MCP) server lets an AI client read your portfolio and import activities into it. ### Prerequisites @@ -366,7 +366,7 @@ The _Model Context Protocol_ (MCP) server lets an AI client read your portfolio. - Set `ROOT_URL` to the public URL of your instance if a client calls the endpoint from a browser page. The host name of `ROOT_URL` is the only accepted origin. - Grant an access of the type _MCP_ in _My Ghostfolio_ under _Access_ and copy its identifier -An _MCP_ access has (restricted) read scopes. It can neither change data nor read the monetary values. +An _MCP_ access has (restricted) read scopes and never reads the monetary values. Grant the _Restricted view and manage_ permission to let the client also import activities. ### Connect a client diff --git a/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts b/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts new file mode 100644 index 000000000..b07441e73 --- /dev/null +++ b/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts @@ -0,0 +1,285 @@ +import { ImportValidationError } from '@ghostfolio/api/app/import/errors/import-validation.error'; +import { ImportService } from '@ghostfolio/api/app/import/import.service'; +import { UserService } from '@ghostfolio/api/app/user/user.service'; +import { REQUIRES_SCOPE_KEY } from '@ghostfolio/api/decorators/requires-scope.decorator'; +import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; +import { MCP_MAX_ACTIVITIES } from '@ghostfolio/common/config'; +import { Activity } from '@ghostfolio/common/interfaces'; +import { permissions } from '@ghostfolio/common/permissions'; +import { scopes } from '@ghostfolio/common/scopes'; +import type { + ImpersonationContext, + UserWithSettings +} from '@ghostfolio/common/types'; + +import { HttpException, Logger } from '@nestjs/common'; +import { RpcException } from '@nestjs/microservices'; +import { DataSource, Type as ActivityType } from '@prisma/client'; +import { getReasonPhrase, StatusCodes } from 'http-status-codes'; + +import { + GhostfolioMcpController, + IMPORT_ACTIVITIES_PARAMETERS +} from './mcp.controller'; + +// The controller reads the columns of the tables from the AiService, which +// imports two packages which ship as an ECMAScript module only, which Jest +// cannot transform. The mocks only make the imports resolvable, because no +// test calls them. +jest.mock('@openrouter/ai-sdk-provider', () => { + return { createOpenRouter: jest.fn() }; +}); + +jest.mock('ai', () => { + return { generateText: jest.fn() }; +}); + +function createActivity(overrides: Record = {}) { + return { + currency: 'USD', + date: '2024-01-01', + fee: 0, + quantity: 1, + symbol: 'AAPL', + type: ActivityType.BUY, + unitPrice: 100, + ...overrides + }; +} + +describe('GhostfolioMcpController', () => { + const impersonation = { userId: 'user-id' } as ImpersonationContext; + + let configuration: Record; + let configurationService: ConfigurationService; + let controller: GhostfolioMcpController; + let importService: ImportService; + let userService: UserService; + + function setupUser(userPermissions: string[]) { + jest.spyOn(userService, 'user').mockResolvedValue({ + permissions: userPermissions + } as UserWithSettings); + } + + beforeEach(() => { + configuration = { + DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER: [], + ENABLE_FEATURE_SUBSCRIPTION: false + }; + + configurationService = { + get: jest.fn().mockImplementation((key: string) => { + return configuration[key]; + }) + } as unknown as ConfigurationService; + + importService = { + import: jest.fn().mockResolvedValue([]) + } as unknown as ImportService; + + userService = { user: jest.fn() } as unknown as UserService; + + controller = new GhostfolioMcpController( + undefined, + undefined, + configurationService, + importService, + userService + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('Import activities', () => { + it('Requires the scope to create an activity', () => { + expect( + Reflect.getMetadata( + REQUIRES_SCOPE_KEY, + GhostfolioMcpController.prototype.importActivities + ) + ).toEqual([scopes.activityCreate]); + }); + + it('Refuses a user without the permission to create an activity', async () => { + setupUser([]); + + await expect( + controller.importActivities(impersonation, { + activities: [createActivity()] + }) + ).rejects.toThrow(HttpException); + + expect(importService.import).not.toHaveBeenCalled(); + }); + + it('Gives the number of the imported and of the skipped activities', async () => { + setupUser([permissions.createActivity]); + + jest + .spyOn(importService, 'import') + .mockResolvedValue([{ id: 'activity-id' } as Activity]); + + expect( + await controller.importActivities(impersonation, { + activities: [createActivity(), createActivity({ quantity: 2 })] + }) + ).toEqual({ + content: [ + { + text: 'Imported activities: 1\nSkipped duplicate activities: 1', + type: 'text' + } + ] + }); + }); + + it('Resolves the mask of the data source of the Ghostfolio data provider', async () => { + setupUser([permissions.createActivity]); + + configuration.DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER = [DataSource.YAHOO]; + configuration.ENABLE_FEATURE_SUBSCRIPTION = true; + + await controller.importActivities(impersonation, { + activities: [createActivity({ dataSource: DataSource.GHOSTFOLIO })] + }); + + expect(importService.import).toHaveBeenCalledWith( + expect.objectContaining({ + activitiesDto: [ + expect.objectContaining({ dataSource: DataSource.YAHOO }) + ] + }) + ); + }); + + it('Keeps the data source if the subscription is not enabled', async () => { + setupUser([permissions.createActivity]); + + configuration.DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER = [DataSource.YAHOO]; + configuration.ENABLE_FEATURE_SUBSCRIPTION = false; + + await controller.importActivities(impersonation, { + activities: [createActivity({ dataSource: DataSource.GHOSTFOLIO })] + }); + + expect(importService.import).toHaveBeenCalledWith( + expect.objectContaining({ + activitiesDto: [ + expect.objectContaining({ dataSource: DataSource.GHOSTFOLIO }) + ] + }) + ); + }); + + it('Passes on the message of a validation only', async () => { + setupUser([permissions.createActivity]); + + jest + .spyOn(importService, 'import') + .mockRejectedValue( + new ImportValidationError('activities.0.symbol ("X") is not valid') + ); + + await expect( + controller.importActivities(impersonation, { + activities: [createActivity()] + }) + ).rejects.toThrow( + new RpcException('activities.0.symbol ("X") is not valid') + ); + }); + + it('Hides the message of an unexpected error and writes it to the log', async () => { + setupUser([permissions.createActivity]); + + const error = new Error( + 'Unique constraint failed on the fields: (dataSource)' + ); + + const logError = jest + .spyOn(Logger.prototype, 'error') + .mockImplementation(); + + jest.spyOn(importService, 'import').mockRejectedValue(error); + + await expect( + controller.importActivities(impersonation, { + activities: [createActivity()] + }) + ).rejects.toThrow( + new RpcException(getReasonPhrase(StatusCodes.INTERNAL_SERVER_ERROR)) + ); + + expect(logError).toHaveBeenCalledWith(error); + }); + + it('Does not write the message of a validation to the log', async () => { + setupUser([permissions.createActivity]); + + const logError = jest + .spyOn(Logger.prototype, 'error') + .mockImplementation(); + + jest + .spyOn(importService, 'import') + .mockRejectedValue( + new ImportValidationError('activities.0.accountId ("X") is not valid') + ); + + await expect( + controller.importActivities(impersonation, { + activities: [createActivity({ accountId: 'X' })] + }) + ).rejects.toThrow(RpcException); + + expect(logError).not.toHaveBeenCalled(); + }); + }); + + describe('Parameters of the tool to import activities', () => { + function parse(activities: unknown[]) { + return IMPORT_ACTIVITIES_PARAMETERS.safeParse({ activities }).success; + } + + it('Refuses a currency in lower case', () => { + expect(parse([createActivity({ currency: 'usd' })])).toBe(false); + }); + + it('Accepts a currency in upper case', () => { + expect(parse([createActivity({ currency: 'USD' })])).toBe(true); + }); + + it('Refuses a date at or before the epoch', () => { + expect(parse([createActivity({ date: '0000-01-01' })])).toBe(false); + }); + + it('Refuses an empty symbol', () => { + expect(parse([createActivity({ symbol: '' })])).toBe(false); + }); + + it('Refuses an empty identifier of an account', () => { + expect(parse([createActivity({ accountId: '' })])).toBe(false); + }); + + it('Removes a tag, because the tool takes no tag', () => { + expect( + IMPORT_ACTIVITIES_PARAMETERS.parse({ + activities: [createActivity({ tags: ['tag-id'] })] + }).activities[0] + ).not.toHaveProperty('tags'); + }); + + it(`Refuses more than ${MCP_MAX_ACTIVITIES} activities`, () => { + expect( + parse( + Array.from({ length: MCP_MAX_ACTIVITIES + 1 }, () => { + return createActivity(); + }) + ) + ).toBe(false); + }); + }); +}); diff --git a/apps/api/src/app/endpoints/mcp/mcp.controller.ts b/apps/api/src/app/endpoints/mcp/mcp.controller.ts index f8261f5c0..baf68659b 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.controller.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.controller.ts @@ -1,9 +1,14 @@ import { AiService } from '@ghostfolio/api/app/endpoints/ai/ai.service'; +import { ImportValidationError } from '@ghostfolio/api/app/import/errors/import-validation.error'; +import { ImportService } from '@ghostfolio/api/app/import/import.service'; +import { UserService } from '@ghostfolio/api/app/user/user.service'; import { Impersonation } from '@ghostfolio/api/decorators/impersonation.decorator'; import { RequiresScopeOfAccess } from '@ghostfolio/api/decorators/requires-scope-of-access.decorator'; import { DATE_RANGE_PATTERN } from '@ghostfolio/api/dtos/date-range-filter.dto'; import { McpToolExceptionFilter } from '@ghostfolio/api/filters/mcp-tool-exception.filter'; +import { getUnmaskedGhostfolioDataSource } from '@ghostfolio/api/helper/data-source.helper'; import { ApiService } from '@ghostfolio/api/services/api/api.service'; +import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; import { getIntervalFromDateRange } from '@ghostfolio/common/calculation-helper'; import { DATE_RANGES, @@ -11,13 +16,20 @@ import { MCP_MAX_ACCOUNTS, MCP_MAX_ACTIVITIES } from '@ghostfolio/common/config'; +import { + isValidCurrencyCode, + isValidDateAfter1970 +} from '@ghostfolio/common/helper'; +import { Activity } from '@ghostfolio/common/interfaces'; +import { hasPermission, permissions } from '@ghostfolio/common/permissions'; import { scopes } from '@ghostfolio/common/scopes'; import type { ImpersonationContext } from '@ghostfolio/common/types'; -import { UseFilters } from '@nestjs/common'; -import { Payload } from '@nestjs/microservices'; +import { HttpException, Logger, UseFilters } from '@nestjs/common'; +import { Payload, RpcException } from '@nestjs/microservices'; import { AssetClass, DataSource, Type as ActivityType } from '@prisma/client'; import { McpController, Tool } from '@rekog/mcp-nest'; +import { getReasonPhrase, StatusCodes } from 'http-status-codes'; import { z } from 'zod'; const GET_ACCOUNTS_PARAMETERS = z.object({ @@ -89,12 +101,55 @@ const GET_ACTIVITIES_PARAMETERS = z.object({ .describe(`The number of activities to get, at most ${MCP_MAX_ACTIVITIES}`) }); +export const IMPORT_ACTIVITIES_PARAMETERS = z.object({ + activities: z + .array( + z.object({ + accountId: z + .string() + .min(1) + .optional() + .describe('The identifier of the account of the activity'), + comment: z.string().optional().describe('The comment of the activity'), + currency: z + .string() + .refine(isValidCurrencyCode) + .describe( + 'The currency of the fee and of the unit price, as an ISO 4217 code in upper case' + ), + dataSource: z + .enum(DataSource) + .optional() + .describe('The data source of the asset profile'), + date: z + .string() + .refine(isValidDateAfter1970) + .describe( + 'The date of the activity, as an ISO 8601 date or date and time' + ), + fee: z.number().min(0).describe('The fee of the activity'), + quantity: z.number().min(0).describe('The quantity of the activity'), + symbol: z.string().min(1).describe('The symbol of the asset profile'), + type: z.enum(ActivityType).describe('The type of the activity'), + unitPrice: z.number().min(0).describe('The unit price of the activity') + }) + ) + .min(1) + .max(MCP_MAX_ACTIVITIES) + .describe(`The activities to import, at most ${MCP_MAX_ACTIVITIES}`) +}); + @McpController() @UseFilters(McpToolExceptionFilter) export class GhostfolioMcpController { + private readonly logger = new Logger(GhostfolioMcpController.name); + public constructor( private readonly aiService: AiService, - private readonly apiService: ApiService + private readonly apiService: ApiService, + private readonly configurationService: ConfigurationService, + private readonly importService: ImportService, + private readonly userService: UserService ) {} @RequiresScopeOfAccess(scopes.accountRead) @@ -210,4 +265,88 @@ export class GhostfolioMcpController { return { content: [{ text: prompt, type: 'text' as const }] }; } + + /** + * The transport gives the tool to every client, because it filters the list + * of the tools by the scopes of request.user, which a request of an access + * never has. The guard refuses the call itself, hence the description names + * the permission which the access needs. + */ + @RequiresScopeOfAccess(scopes.activityCreate) + @Tool({ + annotations: { + destructiveHint: false, + openWorldHint: false, + readOnlyHint: false, + title: 'Import activities' + }, + description: `Imports activities into the portfolio and gives the number of the imported activities and the number of the skipped activities. An activity is skipped if an equal activity is in the portfolio already, hence send each activity one time only: two equal activities of the same call are both imported. The access needs the permission "Restricted view and manage". At most ${MCP_MAX_ACTIVITIES} activities are imported per call, while the instance can have a lower limit, which an error names. An error does not remove the activities of the same call which are imported already, hence get the activities after an error before you import them again.`, + name: 'import-activities', + parameters: IMPORT_ACTIVITIES_PARAMETERS + }) + public async importActivities( + @Impersonation() { userId }: ImpersonationContext, + @Payload() { activities }: z.infer + ) { + const user = await this.userService.user({ id: userId }); + + if (!hasPermission(user?.permissions, permissions.createActivity)) { + throw new HttpException( + getReasonPhrase(StatusCodes.FORBIDDEN), + StatusCodes.FORBIDDEN + ); + } + + const ghostfolioDataSources = this.configurationService.get( + 'ENABLE_FEATURE_SUBSCRIPTION' + ) + ? this.configurationService.get('DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER') + : []; + + const activitiesDto = activities.map((activity) => { + return { + ...activity, + dataSource: getUnmaskedGhostfolioDataSource({ + ghostfolioDataSources, + dataSource: activity.dataSource + }) + }; + }); + + let importedActivities: Activity[]; + + try { + importedActivities = await this.importService.import({ + activitiesDto, + user, + accountsWithBalancesDto: [], + assetProfilesWithMarketDataDto: [], + platformsDto: [], + tagsDto: [] + }); + } catch (error) { + // The message of a validation names the activity which is not valid and + // is written for the caller, hence it is passed on + if (error instanceof ImportValidationError) { + throw new RpcException(error.message); + } + + // Every other message can carry internals of the application, hence it + // is written to the log and the reason phrase is passed on instead + this.logger.error(error); + + throw new RpcException( + getReasonPhrase(StatusCodes.INTERNAL_SERVER_ERROR) + ); + } + + const text = [ + `Imported activities: ${importedActivities.length}`, + `Skipped duplicate activities: ${ + activities.length - importedActivities.length + }` + ].join('\n'); + + return { content: [{ text, type: 'text' as const }] }; + } } diff --git a/apps/api/src/app/endpoints/mcp/mcp.module.ts b/apps/api/src/app/endpoints/mcp/mcp.module.ts index a29fae619..e2fbce79f 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.module.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.module.ts @@ -1,4 +1,6 @@ import { AiModule } from '@ghostfolio/api/app/endpoints/ai/ai.module'; +import { ImportModule } from '@ghostfolio/api/app/import/import.module'; +import { UserModule } from '@ghostfolio/api/app/user/user.module'; import { environment } from '@ghostfolio/api/environments/environment'; import { ApiModule } from '@ghostfolio/api/services/api/api.module'; import { ConfigurationModule } from '@ghostfolio/api/services/configuration/configuration.module'; @@ -16,7 +18,7 @@ import { GhostfolioMcpController } from './mcp.controller'; @Module({ controllers: [GhostfolioMcpController], - imports: [AiModule, ApiModule, ConfigurationModule], + imports: [AiModule, ApiModule, ConfigurationModule, ImportModule, UserModule], providers: [ { inject: [ConfigurationService], @@ -26,7 +28,7 @@ import { GhostfolioMcpController } from './mcp.controller'; return new McpStrategy({ instructions: - 'Ghostfolio is a wealth management application. The tools read the portfolio of the user who granted the access. They give no quantity and no monetary value (except the unit price of an activity).', + 'Ghostfolio is a wealth management application. The tools read the portfolio of the user who granted the access and import activities into it. They give no quantity and no monetary value (except the unit price of an activity).', name: 'ghostfolio', title: 'Ghostfolio', transports: [ diff --git a/apps/api/src/app/import/errors/import-validation.error.ts b/apps/api/src/app/import/errors/import-validation.error.ts new file mode 100644 index 000000000..a228c9933 --- /dev/null +++ b/apps/api/src/app/import/errors/import-validation.error.ts @@ -0,0 +1,7 @@ +export class ImportValidationError extends Error { + public constructor(message: string) { + super(message); + + this.name = 'ImportValidationError'; + } +} diff --git a/apps/api/src/app/import/import.controller.ts b/apps/api/src/app/import/import.controller.ts index cd378d07d..79fde2800 100644 --- a/apps/api/src/app/import/import.controller.ts +++ b/apps/api/src/app/import/import.controller.ts @@ -2,8 +2,6 @@ import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorat 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 { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; -import { SubscriptionType } from '@ghostfolio/common/enums'; import { ImportResponse } from '@ghostfolio/common/interfaces'; import { hasPermission, permissions } from '@ghostfolio/common/permissions'; import type { RequestWithUser } from '@ghostfolio/common/types'; @@ -34,7 +32,6 @@ export class ImportController { private readonly logger = new Logger(ImportController.name); public constructor( - private readonly configurationService: ConfigurationService, private readonly importService: ImportService, @Inject(REQUEST) private readonly request: RequestWithUser ) {} @@ -59,21 +56,9 @@ export class ImportController { ); } - let maxActivitiesToImport = this.configurationService.get( - 'MAX_ACTIVITIES_TO_IMPORT' - ); - - if ( - this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - this.request.user.subscription?.type === SubscriptionType.Premium - ) { - maxActivitiesToImport = Number.MAX_SAFE_INTEGER; - } - try { const activities = await this.importService.import({ isDryRun, - maxActivitiesToImport, accountsWithBalancesDto: importData.accounts ?? [], activitiesDto: importData.activities, assetProfilesWithMarketDataDto: importData.assetProfiles ?? [], @@ -104,16 +89,9 @@ export class ImportController { @Param('dataSource') dataSource: DataSource, @Param('symbol') symbol: string ): Promise { - let maxActivitiesToImport = this.configurationService.get( - 'MAX_ACTIVITIES_TO_IMPORT' - ); - - if ( - this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - this.request.user.subscription?.type === SubscriptionType.Premium - ) { - maxActivitiesToImport = Number.MAX_SAFE_INTEGER; - } + const maxActivitiesToImport = this.importService.getMaxActivitiesToImport({ + user: this.request.user + }); const activities = await this.importService.getDividends({ dataSource, diff --git a/apps/api/src/app/import/import.module.ts b/apps/api/src/app/import/import.module.ts index 8aebcfa08..1a04e7505 100644 --- a/apps/api/src/app/import/import.module.ts +++ b/apps/api/src/app/import/import.module.ts @@ -23,6 +23,7 @@ import { ImportService } from './import.service'; @Module({ controllers: [ImportController], + exports: [ImportService], imports: [ AccountModule, ActivitiesModule, diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index ca6966a9f..5f202f4ac 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -24,6 +24,7 @@ import { CreateAssetProfileDto, CreateOrderDto } from '@ghostfolio/common/dtos'; +import { SubscriptionType } from '@ghostfolio/common/enums'; import { getAssetProfileIdentifier, isValidCustomAssetProfileSymbol, @@ -49,6 +50,7 @@ import { isSameSecond, parseISO } from 'date-fns'; import { omit, uniqBy } from 'lodash'; import { randomUUID } from 'node:crypto'; +import { ImportValidationError } from './errors/import-validation.error'; import { ImportDataDto } from './import-data.dto'; import { AssetProfileToCreate } from './interfaces/asset-profile-to-create.interface'; @@ -186,12 +188,22 @@ export class ImportService { } } + public getMaxActivitiesToImport({ user }: { user: UserWithSettings }) { + if ( + this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && + user.subscription?.type === SubscriptionType.Premium + ) { + return Number.MAX_SAFE_INTEGER; + } + + return this.configurationService.get('MAX_ACTIVITIES_TO_IMPORT'); + } + public async import({ accountsWithBalancesDto, activitiesDto, assetProfilesWithMarketDataDto, isDryRun = false, - maxActivitiesToImport, platformsDto, tagsDto, user @@ -200,7 +212,6 @@ export class ImportService { activitiesDto: ImportDataDto['activities']; assetProfilesWithMarketDataDto: ImportDataDto['assetProfiles']; isDryRun?: boolean; - maxActivitiesToImport: number; platformsDto: ImportDataDto['platforms']; tagsDto: ImportDataDto['tags']; user: UserWithSettings; @@ -210,6 +221,7 @@ export class ImportService { const ghostfolioDataSources = this.configurationService.get( 'DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER' ); + const maxActivitiesToImport = this.getMaxActivitiesToImport({ user }); const platformIdMapping: { [oldPlatformId: string]: string } = {}; const tagIdMapping: { [oldTagId: string]: string } = {}; const userCurrency = user.settings.settings.baseCurrency; @@ -224,7 +236,7 @@ export class ImportService { dataSource === DataSource.MANUAL && !isValidCustomAssetProfileSymbol(symbol) ) { - throw new Error( + throw new ImportValidationError( `assetProfiles.${index}.symbol ("${symbol}") must be a UUID or start with the prefix "${ghostfolioPrefix}_" for the data source ("${DataSource.MANUAL}")` ); } else if ( @@ -236,7 +248,7 @@ export class ImportService { ghostfolioDataSources }); - throw new Error( + throw new ImportValidationError( `assetProfiles.${index}.symbol ("${symbol}") is not valid for the data source ("${maskedDataSource}")` ); } @@ -261,7 +273,7 @@ export class ImportService { activity.dataSource === DataSource.MANUAL && !isValidCustomAssetProfileSymbol(activity.symbol) ) { - throw new Error( + throw new ImportValidationError( `activities.${index}.symbol ("${activity.symbol}") must be a UUID or start with the prefix "${ghostfolioPrefix}_" for the data source ("${DataSource.MANUAL}")` ); } else if ( @@ -273,7 +285,7 @@ export class ImportService { dataSource: activity.dataSource }); - throw new Error( + throw new ImportValidationError( `activities.${index}.symbol ("${activity.symbol}") is not valid for the data source ("${maskedDataSource}")` ); } @@ -358,7 +370,7 @@ export class ImportService { } } else { if (!canCreatePlatform) { - throw new Error( + throw new ImportValidationError( `Insufficient permissions to create platform ("${platform.name}")` ); } @@ -388,7 +400,7 @@ export class ImportService { if (!existingTagOfUser) { if (!canCreateOwnTag) { - throw new Error( + throw new ImportValidationError( `Insufficient permissions to create custom tag ("${tag.name}")` ); } @@ -765,6 +777,21 @@ export class ImportService { }); } + // Validate the accounts before any activity is created, since an account + // which does not belong to the user is dropped without a notice otherwise + for (const [index, { accountId }] of activitiesDto.entries()) { + if ( + accountId && + !accounts.some(({ id }) => { + return id === accountId; + }) + ) { + throw new ImportValidationError( + `activities.${index}.accountId ("${accountId}") is not valid` + ); + } + } + const tags = (await this.tagService.getTagsForUser(user.id)).map( ({ id, name }) => { return { id, name }; diff --git a/apps/api/src/helper/data-source.helper.ts b/apps/api/src/helper/data-source.helper.ts index f3ed75229..fff8eaac1 100644 --- a/apps/api/src/helper/data-source.helper.ts +++ b/apps/api/src/helper/data-source.helper.ts @@ -65,3 +65,15 @@ export function getMaskedGhostfolioDataSource({ ? DataSource.GHOSTFOLIO : dataSource; } + +export function getUnmaskedGhostfolioDataSource({ + dataSource, + ghostfolioDataSources +}: { + dataSource?: DataSource; + ghostfolioDataSources: string[]; +}) { + return dataSource === DataSource.GHOSTFOLIO && ghostfolioDataSources?.[0] + ? (ghostfolioDataSources[0] as DataSource) + : dataSource; +} diff --git a/apps/api/src/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor.ts b/apps/api/src/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor.ts index 4ab0794ec..7b537cd52 100644 --- a/apps/api/src/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor.ts +++ b/apps/api/src/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor.ts @@ -1,4 +1,7 @@ -import { decodeDataSource } from '@ghostfolio/api/helper/data-source.helper'; +import { + decodeDataSource, + getUnmaskedGhostfolioDataSource +} from '@ghostfolio/api/helper/data-source.helper'; import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service'; import { @@ -27,23 +30,19 @@ export class TransformDataSourceInRequestInterceptor< if (this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION')) { if (request.body?.activities) { - const dataSourceGhostfolioDataProvider = this.configurationService.get( + const ghostfolioDataSources = this.configurationService.get( 'DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER' - )?.[0]; + ); request.body.activities = request.body.activities.map((activity) => { if (DataSource[activity.dataSource]) { - if ( - activity.dataSource === 'GHOSTFOLIO' && - dataSourceGhostfolioDataProvider - ) { - return { - ...activity, - dataSource: dataSourceGhostfolioDataProvider - }; - } else { - return activity; - } + return { + ...activity, + dataSource: getUnmaskedGhostfolioDataSource({ + ghostfolioDataSources, + dataSource: activity.dataSource + }) + }; } else { return { ...activity, 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 0d0924f80..9366574ac 100644 --- a/apps/api/src/services/data-provider/data-provider.service.ts +++ b/apps/api/src/services/data-provider/data-provider.service.ts @@ -1,3 +1,4 @@ +import { ImportValidationError } from '@ghostfolio/api/app/import/errors/import-validation.error'; import { ImportDataDto } from '@ghostfolio/api/app/import/import-data.dto'; import { RedisCacheService } from '@ghostfolio/api/app/redis-cache/redis-cache.service'; import { getMaskedGhostfolioDataSource } from '@ghostfolio/api/helper/data-source.helper'; @@ -226,7 +227,9 @@ export class DataProviderService implements OnModuleInit { subscription: UserWithSettings['subscription']; }) { if (activitiesDto?.length > maxActivitiesToImport) { - throw new Error(`Too many activities (${maxActivitiesToImport} at most)`); + throw new ImportValidationError( + `Too many activities (${maxActivitiesToImport} at most)` + ); } const assetProfiles: { @@ -250,7 +253,7 @@ export class DataProviderService implements OnModuleInit { }); if (!dataSources.includes(dataSource)) { - throw new Error( + throw new ImportValidationError( `${activityPath}.dataSource ("${dataSource}") is not valid` ); } @@ -259,7 +262,7 @@ export class DataProviderService implements OnModuleInit { dataSource !== DataSource.MANUAL && isValidCustomAssetProfileSymbol(symbol) ) { - throw new Error( + throw new ImportValidationError( `${activityPath}.symbol ("${symbol}") is not valid for the data source ("${maskedDataSource}")` ); } @@ -271,7 +274,7 @@ export class DataProviderService implements OnModuleInit { const dataProvider = this.getDataProvider(DataSource[dataSource]); if (dataProvider.getDataProviderInfo().isPremium) { - throw new Error( + throw new ImportValidationError( `${activityPath}.dataSource ("${maskedDataSource}") requires Ghostfolio Premium` ); } @@ -328,7 +331,7 @@ export class DataProviderService implements OnModuleInit { } catch {} if (!assetProfile?.name) { - throw new Error( + throw new ImportValidationError( `${activityPath}.symbol ("${symbol}") cannot be resolved by the data source ("${maskedDataSource}")` ); } diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts index 82924b81f..b0be5c0c5 100644 --- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts +++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts @@ -7,6 +7,7 @@ import { hasPermission, permissions } from '@ghostfolio/common/permissions'; import { Scope, getAccessLevel, + getScopesOfAccess, getScopesOfAccessLevel, hasScope, scopes @@ -190,12 +191,20 @@ export class GfCreateOrUpdateAccessDialogComponent implements OnInit { } else { granteeUserIdControl?.clearValidators(); granteeUserIdControl?.setValue(null); - - // An access which is not granted to a user never exposes the - // monetary values and never changes data - this.accessForm.get('accessLevel')?.setValue('READ_RESTRICTED'); } + // Narrow the permission to the scopes which the type permits, because + // an access which is not granted to a user never exposes the monetary + // values and a public access never changes data + this.accessForm.get('accessLevel')?.setValue( + getAccessLevel( + getScopesOfAccess({ + scopes: getScopesOfAccessLevel(this.accessLevel), + type: accessType + }) + ) + ); + if (!canApplyFiltersToAccess({ type: accessType })) { this.accessForm.get('filters')?.setValue(null); } diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html index b2957f9c2..89893e570 100644 --- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html +++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html @@ -82,20 +82,17 @@ + @if (!isPublicAccess) { + + + + } @if (accessForm.get('type')?.value === 'PRIVATE') { - diff --git a/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index 0a17d2058..58ac417c4 100644 --- a/libs/common/src/lib/helper.spec.ts +++ b/libs/common/src/lib/helper.spec.ts @@ -13,7 +13,9 @@ import { isCurrency, isCurrencySymbol, isSplitRatio, + isValidCurrencyCode, isValidCustomAssetProfileSymbol, + isValidDateAfter1970, isValidGranteeOfAccess, resolveUserSettings } from '@ghostfolio/common/helper'; @@ -371,6 +373,28 @@ describe('Helper', () => { }); }); + describe('Is valid currency code', () => { + it('Currency code in lower case', () => { + expect(isValidCurrencyCode('usd')).toEqual(false); + }); + + it('Currency code in upper case', () => { + expect(isValidCurrencyCode('USD')).toEqual(true); + }); + + it('Derived currency', () => { + expect(isValidCurrencyCode('GBp')).toEqual(true); + }); + + it('Empty currency code', () => { + expect(isValidCurrencyCode('')).toEqual(false); + }); + + it('Unknown currency code', () => { + expect(isValidCurrencyCode('XYZ')).toEqual(false); + }); + }); + describe('Is valid custom asset profile symbol', () => { it('Empty symbol', () => { expect(isValidCustomAssetProfileSymbol('')).toEqual(false); @@ -399,6 +423,56 @@ describe('Helper', () => { }); }); + describe('Is valid date after 1970', () => { + it('Date', () => { + expect(isValidDateAfter1970('2024-01-01')).toEqual(true); + }); + + it('Date and time', () => { + expect(isValidDateAfter1970('2024-01-01T12:00:00.000Z')).toEqual(true); + }); + + it('Date before 1970', () => { + expect(isValidDateAfter1970('0000-01-01')).toEqual(false); + }); + + it('Date object', () => { + expect( + isValidDateAfter1970(new Date('2024-01-01T12:00:00.000Z')) + ).toEqual(true); + }); + + it('Date object of 1970', () => { + expect(isValidDateAfter1970(new Date(0))).toEqual(false); + }); + + it('Date of 1970', () => { + expect(isValidDateAfter1970('1970-01-01T00:00:00.000Z')).toEqual(false); + }); + + // A date without a time is read in UTC, hence the result is the same in + // every time zone of the server + it('Date of 1970 without a time', () => { + expect(isValidDateAfter1970('1970-01-01')).toEqual(false); + }); + + it('Date after 1970 without a time', () => { + expect(isValidDateAfter1970('1970-01-02')).toEqual(true); + }); + + it('Date with an expanded year', () => { + expect(isValidDateAfter1970('+010000-01-01')).toEqual(true); + }); + + it('Empty date', () => { + expect(isValidDateAfter1970('')).toEqual(false); + }); + + it('Free-text date', () => { + expect(isValidDateAfter1970('yesterday')).toEqual(false); + }); + }); + describe('Is valid grantee of access', () => { const granteeUserId = 'ffb08949-2f8a-4b6e-88fd-0f1e6b6b5f5d'; diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index b6b097be9..f0f9e979c 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -15,7 +15,9 @@ import { getDate, getMonth, getYear, + isAfter, isMatch, + isValid, parse, parseISO, subDays @@ -668,10 +670,27 @@ export function isUserSettingOfAuthenticatedUser(aKey: string) { ); } +export function isValidCurrencyCode(aCurrency: string) { + if (!aCurrency) { + return false; + } + + return ( + isDerivedCurrency(aCurrency) || + (aCurrency === aCurrency.toUpperCase() && isISO4217CurrencyCode(aCurrency)) + ); +} + export function isValidCustomAssetProfileSymbol(aSymbol: string) { return hasGhostfolioPrefix(aSymbol) || isUUID(aSymbol); } +export function isValidDateAfter1970(aDate: Date | string) { + const date = isString(aDate) ? parseISO(aDate, { in: utc }) : aDate; + + return isValid(date) && isAfter(date, new Date(0)); +} + /** * A private access is granted to a user, while a public access and an access * of a client of the model context protocol are credentials on their own and diff --git a/libs/common/src/lib/scopes.spec.ts b/libs/common/src/lib/scopes.spec.ts index 1f8bb07b2..877c5aa71 100644 --- a/libs/common/src/lib/scopes.spec.ts +++ b/libs/common/src/lib/scopes.spec.ts @@ -207,8 +207,8 @@ describe('Scopes', () => { ).not.toContain(scopes.portfolioReadValues); }); - // The dialog offers the write scopes for a private access only, hence this - // function is the sole barrier for a public access + // The dialog offers no permission which changes data for a public access, + // hence this function is the sole barrier for it it('Gives no write scope', () => { expect( getScopesOfAccess({ @@ -248,7 +248,9 @@ describe('Scopes', () => { ).not.toContain(scopes.portfolioReadValues); }); - it('Gives no write scope', () => { + // The controller exposes the tool to import activities as the only tool + // which writes, hence a further write scope stays ineffective + it('Gives the write scope to create an activity only', () => { expect( getScopesOfAccess({ scopes: [...SCOPES_OF_READ_ACCESS, ...SCOPES_OF_WRITE_ACCESS], @@ -256,7 +258,23 @@ describe('Scopes', () => { }).filter((scope) => { return SCOPES_OF_WRITE_ACCESS.includes(scope as Scope); }) - ).toEqual([]); + ).toEqual([scopes.activityCreate]); + }); + + // The dialog narrows the access level to the scopes which the type + // permits, hence the restricted access level with the write scopes has to + // survive the intersection + it('Keeps the access level to change the data without the monetary values', () => { + expect( + getAccessLevel( + getScopesOfAccess({ + scopes: getScopesOfAccessLevel( + 'CREATE_READ_RESTRICTED_UPDATE_DELETE' + ), + type: 'MCP' + }) + ) + ).toEqual('CREATE_READ_RESTRICTED_UPDATE_DELETE'); }); }); diff --git a/libs/common/src/lib/scopes.ts b/libs/common/src/lib/scopes.ts index 27cba0e72..18270c788 100644 --- a/libs/common/src/lib/scopes.ts +++ b/libs/common/src/lib/scopes.ts @@ -67,12 +67,7 @@ export const SCOPES_OF_READ_RESTRICTED_ACCESS: readonly Scope[] = * ineffective even if it is stored. */ const SCOPES_OF_TYPE: Record = { - MCP: [ - ...SCOPES_OF_READ_RESTRICTED_ACCESS - // Write scope is not permitted yet, because the controller exposes read - // tools only - // ...SCOPES_OF_WRITE_ACCESS - ], + MCP: [...SCOPES_OF_READ_RESTRICTED_ACCESS, scopes.activityCreate], PRIVATE: Object.values(scopes), PUBLIC: SCOPES_OF_PUBLIC_ACCESS }; diff --git a/libs/common/src/lib/validator-constraints/is-after-1970.ts b/libs/common/src/lib/validator-constraints/is-after-1970.ts index 9dc0b04c0..d55dc67c5 100644 --- a/libs/common/src/lib/validator-constraints/is-after-1970.ts +++ b/libs/common/src/lib/validator-constraints/is-after-1970.ts @@ -1,8 +1,10 @@ +import { isValidDateAfter1970 } from '@ghostfolio/common/helper'; + import { ValidatorConstraint, ValidatorConstraintInterface } from 'class-validator'; -import { format, isAfter } from 'date-fns'; +import { format } from 'date-fns'; @ValidatorConstraint({ name: 'isAfter1970' }) export class IsAfter1970Constraint implements ValidatorConstraintInterface { @@ -10,7 +12,7 @@ export class IsAfter1970Constraint implements ValidatorConstraintInterface { return `date must be after ${format(new Date(0), 'yyyy')}`; } - public validate(aDate: Date) { - return isAfter(aDate, new Date(0)); + public validate(aDate: Date | string) { + return isValidDateAfter1970(aDate); } } diff --git a/libs/common/src/lib/validators/is-currency-code.ts b/libs/common/src/lib/validators/is-currency-code.ts index 52d99816b..8124b426c 100644 --- a/libs/common/src/lib/validators/is-currency-code.ts +++ b/libs/common/src/lib/validators/is-currency-code.ts @@ -1,4 +1,4 @@ -import { isDerivedCurrency } from '@ghostfolio/common/helper'; +import { isValidCurrencyCode } from '@ghostfolio/common/helper'; import { registerDecorator, @@ -6,7 +6,6 @@ import { ValidatorConstraint, ValidatorConstraintInterface } from 'class-validator'; -import { isISO4217CurrencyCode } from 'class-validator'; export function IsCurrencyCode(validationOptions?: ValidationOptions) { return function (object: object, propertyName: string) { @@ -27,14 +26,6 @@ export class IsExtendedCurrencyConstraint implements ValidatorConstraintInterfac } public validate(currency: string) { - // Return true if currency is a derived currency or a standard ISO 4217 code - return ( - isDerivedCurrency(currency) || - (this.isUpperCase(currency) && isISO4217CurrencyCode(currency)) - ); - } - - private isUpperCase(aString: string) { - return aString === aString?.toUpperCase(); + return isValidCurrencyCode(currency); } }