Improve MCP access with view permission

apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts

@@ -2,6 +2,7 @@ import { REQUIRES_SCOPE_KEY } from '@ghostfolio/api/decorators/requires-scope.de
 import { McpToolExceptionFilter } from '@ghostfolio/api/filters/mcp-tool-exception.filter';
 import { AccessGuard } from '@ghostfolio/api/guards/access.guard';
 import { getMcpUserOfBearerToken } from '@ghostfolio/api/helper/bearer-token.helper';
+import { SubscriptionType } from '@ghostfolio/common/enums';
 import {
   getScopesOfAccess,
   getScopesOfAccessLevel,
@@ -130,6 +131,44 @@ async function getWithValuesOfReadTools(accessLevel: AccessLevel) {
   });
 }
 
+/**
+ * Gives whether the controller asks the service for the asset performance in
+ * base currency of an access with the permission "View"
+ */
+async function getWithAssetPerformanceInBaseCurrency(
+  subscriptionType?: SubscriptionType
+) {
+  const mcpService = { getPerformance: jest.fn() };
+
+  const controller = new GhostfolioMcpController(
+    mcpService as unknown as McpService
+  );
+
+  await controller.getPerformance(
+    {
+      isActive: true,
+      scopes: getScopesOfAccess({
+        scopes: getScopesOfAccessLevel('READ'),
+        type: 'MCP'
+      }),
+      userId: 'user-id',
+      userSettings: { baseCurrency: 'CHF' },
+      userSubscription: subscriptionType
+        ? ({
+            type: subscriptionType
+          } as ImpersonationContext['userSubscription'])
+        : undefined
+    },
+    {} as Parameters<GhostfolioMcpController['getPerformance']>[1]
+  );
+
+  return (
+    mcpService.getPerformance.mock.calls[0][0] as {
+      withAssetPerformanceInBaseCurrency: boolean;
+    }
+  ).withAssetPerformanceInBaseCurrency;
+}
+
 describe('GhostfolioMcpController', () => {
   // A tool without the decorator of the scope would be open to every access,
   // hence a new tool has to declare its scope
@@ -270,6 +309,22 @@ describe('GhostfolioMcpController', () => {
     ]);
   });
 
+  it('Gives the asset performance in base currency for a Premium subscription', async () => {
+    expect(
+      await getWithAssetPerformanceInBaseCurrency(SubscriptionType.Premium)
+    ).toBe(true);
+  });
+
+  it('Gives the asset performance in base currency if the subscription is disabled', async () => {
+    expect(await getWithAssetPerformanceInBaseCurrency()).toBe(true);
+  });
+
+  it('Gives no asset performance in base currency for a Basic subscription', async () => {
+    expect(
+      await getWithAssetPerformanceInBaseCurrency(SubscriptionType.Basic)
+    ).toBe(false);
+  });
+
   it('Lists no tool for an inactive access', () => {
     expect(
       getNamesOfListedTools({

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

@@ -3,6 +3,7 @@ import { RequiresScopeOfAccess } from '@ghostfolio/api/decorators/requires-scope
 import { McpToolExceptionFilter } from '@ghostfolio/api/filters/mcp-tool-exception.filter';
 import { PortfolioTableService } from '@ghostfolio/api/services/portfolio-table/portfolio-table.service';
 import { MCP_MAX_ACTIVITIES } from '@ghostfolio/common/config';
+import { SubscriptionType } from '@ghostfolio/common/enums';
 import { hasScope, scopes } from '@ghostfolio/common/scopes';
 import type { ImpersonationContext } from '@ghostfolio/common/types';
 
@@ -37,7 +38,7 @@ export class GhostfolioMcpController {
       ', '
     )}. If the access reads the monetary values, these columns are given in addition: ${PortfolioTableService.getAccountsTableValueColumnNames().join(
       ', '
-    )}. The allocation in percentage is relative to the accounts of the result, hence the parameters change it.`,
+    )}. The allocation in percentage is relative to the accounts of the result, hence the parameters change it. The parameters change the value in base currency as well, with the holding parameter it is the value of the holding in the account without the cash balance. The balance is always the full cash balance of the account.`,
     name: 'get-accounts',
     parameters: GET_ACCOUNTS_PARAMETERS
   })
@@ -98,12 +99,18 @@ export class GhostfolioMcpController {
   })
   public async getPerformance(
     @Impersonation()
-    { scopes: impersonationScopes, userId }: ImpersonationContext,
+    {
+      scopes: impersonationScopes,
+      userId,
+      userSubscription
+    }: ImpersonationContext,
     @Payload() parameters: z.infer<typeof GET_PERFORMANCE_PARAMETERS>
   ) {
     return this.mcpService.getPerformance({
       ...parameters,
       userId,
+      withAssetPerformanceInBaseCurrency:
+        userSubscription?.type !== SubscriptionType.Basic,
       withValues: hasScope(impersonationScopes, scopes.portfolioReadValues)
     });
   }

apps/api/src/app/endpoints/mcp/mcp.module.ts

@@ -39,7 +39,7 @@ import { McpService } from './mcp.service';
 
         return new McpStrategy({
           instructions:
-            'Ghostfolio is a wealth management application. The tools read the portfolio and the watchlist of the user who granted the access and import activities into the portfolio. They give no quantity and no monetary value (except the unit price of an activity). Only the tools which the permission of the access covers are listed. To import activities, the access needs the permission "Restricted view and manage".',
+            'Ghostfolio is a wealth management application. The tools read the portfolio and the watchlist of the user who granted the access and import activities into the portfolio. They give the quantities and the monetary values only if the access has the permission "View" or "View and manage", otherwise no quantity and no monetary value (except the unit price of an activity). Only the tools which the permission of the access covers are listed. To import activities, the access needs the permission "Restricted view and manage" or "View and manage".',
           name: 'ghostfolio',
           resolveUser: getMcpUserOfBearerToken,
           title: 'Ghostfolio',

apps/api/src/app/endpoints/mcp/mcp.service.spec.ts

@@ -378,6 +378,18 @@ describe('McpService', () => {
         expect.objectContaining({ withValues: true })
       );
     });
+
+    it('Gives the asset performance in base currency if the subscription permits it', async () => {
+      await mcpService.getPerformance({
+        userId,
+        range: '2024',
+        withAssetPerformanceInBaseCurrency: true
+      });
+
+      expect(portfolioTableService.getPerformanceTable).toHaveBeenCalledWith(
+        expect.objectContaining({ withAssetPerformanceInBaseCurrency: true })
+      );
+    });
   });
 
   describe('getPortfolio', () => {

apps/api/src/app/endpoints/mcp/mcp.service.ts

@@ -110,9 +110,11 @@ export class McpService {
     holding,
     range,
     userId,
+    withAssetPerformanceInBaseCurrency,
     withValues
   }: z.infer<typeof GET_PERFORMANCE_PARAMETERS> & {
     userId: string;
+    withAssetPerformanceInBaseCurrency?: boolean;
     withValues?: boolean;
   }) {
     const filters = this.apiService.buildFiltersFromQueryParams({
@@ -124,6 +126,7 @@ export class McpService {
     const table = await this.portfolioTableService.getPerformanceTable({
       filters,
       userId,
+      withAssetPerformanceInBaseCurrency,
       withValues,
       dateRange: range
     });

apps/api/src/services/portfolio-table/portfolio-table.service.spec.ts

@@ -323,7 +323,6 @@ describe('PortfolioTableService', () => {
     it('gives the columns with a quantity or with a monetary value', () => {
       expect(PortfolioTableService.getHoldingsTableValueColumnNames()).toEqual([
         'Quantity',
-        'Market Price',
         'Value in Base Currency'
       ]);
     });
@@ -631,7 +630,7 @@ describe('PortfolioTableService', () => {
       expect(row).toMatch(/\| 75\.000% \|$/);
     });
 
-    it('gives the quantity, the market price and the value with the values', async () => {
+    it('gives the quantity and the value with the values', async () => {
       const result = await getHoldingsTable([createHolding()], {
         withValues: true
       });
@@ -640,16 +639,23 @@ describe('PortfolioTableService', () => {
         return line.startsWith('| Name of AAPL');
       });
 
-      expect(row).toMatch(/\| 75\.000% \| 5 \| 400 \| 2000\.00 \|$/);
+      expect(row).toMatch(/\| 75\.000% \| 5 \| 2000\.00 \|$/);
     });
   });
 
   describe('getPerformanceTable', () => {
     function getPerformanceTable(
       parameters: Parameters<typeof createPortfolioTableService>[0] = {},
-      withValues?: boolean
+      {
+        withAssetPerformanceInBaseCurrency,
+        withValues
+      }: {
+        withAssetPerformanceInBaseCurrency?: boolean;
+        withValues?: boolean;
+      } = {}
     ) {
       return createPortfolioTableService(parameters).getPerformanceTable({
+        withAssetPerformanceInBaseCurrency,
         withValues,
         dateRange: 'ytd',
         userId: 'user-id'
@@ -694,7 +700,10 @@ describe('PortfolioTableService', () => {
     });
 
     it('gives the current value and the performance in base currency with the values', async () => {
-      const result = await getPerformanceTable({}, true);
+      const result = await getPerformanceTable(
+        {},
+        { withAssetPerformanceInBaseCurrency: true, withValues: true }
+      );
 
       const [row] = result.split('\n').filter((line) => {
         return line.startsWith('| 10.000%');
@@ -705,6 +714,18 @@ describe('PortfolioTableService', () => {
       );
     });
 
+    it('gives no asset performance and no currency performance in base currency without the asset performance in base currency', async () => {
+      const result = await getPerformanceTable({}, { withValues: true });
+
+      const [row] = result.split('\n').filter((line) => {
+        return line.startsWith('| 10.000%');
+      });
+
+      expect(result).not.toContain('Asset Performance in Base Currency');
+      expect(result).not.toContain('Currency Performance in Base Currency');
+      expect(row).toBe('| 10.000% | 5.000% | 15.000% | 2000.00 | 300.00 |');
+    });
+
     it('gives a currency performance in base currency of zero without a sign', async () => {
       const result = await getPerformanceTable(
         {
@@ -713,7 +734,7 @@ describe('PortfolioTableService', () => {
             netPerformanceWithCurrencyEffect: 200
           })
         },
-        true
+        { withAssetPerformanceInBaseCurrency: true, withValues: true }
       );
 
       const [row] = result.split('\n').filter((line) => {

apps/api/src/services/portfolio-table/portfolio-table.service.ts

@@ -31,6 +31,12 @@ import { format } from 'date-fns';
 import { DataSourceTableContext } from './interfaces/data-source-table-context.interface';
 import { HoldingsTableColumnDefinition } from './types/holdings-table-column-definition.type';
 
+const ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME =
+  'Asset Performance in Base Currency';
+
+const CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME =
+  'Currency Performance in Base Currency';
+
 const DATA_SOURCE_COLUMN_NAME = 'Data Source';
 
 function getDataSourceColumnDefinition<T>(
@@ -281,13 +287,6 @@ export class PortfolioTableService {
         },
         name: 'Quantity'
       },
-      {
-        align: 'right',
-        getValue: ({ marketPrice }) => {
-          return marketPrice.toString();
-        },
-        name: 'Market Price'
-      },
       {
         align: 'right',
         getValue: ({ valueInBaseCurrency }) => {
@@ -348,14 +347,14 @@ export class PortfolioTableService {
         getValue: ({ netPerformance }) => {
           return getAmount(netPerformance);
         },
-        name: 'Asset Performance in Base Currency'
+        name: ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME
       },
       {
         align: 'right',
         getValue: ({ netPerformance, netPerformanceWithCurrencyEffect }) => {
           return getAmount(netPerformanceWithCurrencyEffect - netPerformance);
         },
-        name: 'Currency Performance in Base Currency'
+        name: CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME
       },
       {
         align: 'right',
@@ -665,11 +664,13 @@ export class PortfolioTableService {
     dateRange,
     filters,
     userId,
+    withAssetPerformanceInBaseCurrency = false,
     withValues = false
   }: {
     dateRange: DateRange;
     filters?: Filter[];
     userId: string;
+    withAssetPerformanceInBaseCurrency?: boolean;
     withValues?: boolean;
   }) {
     const { chart, performance } = await this.portfolioService.getPerformance({
@@ -689,6 +690,14 @@ export class PortfolioTableService {
               PortfolioTableService.PERFORMANCE_TABLE_COLUMN_DEFINITIONS,
             valueColumnDefinitions:
               PortfolioTableService.PERFORMANCE_TABLE_VALUE_COLUMN_DEFINITIONS
+          }).filter(({ name }) => {
+            return (
+              withAssetPerformanceInBaseCurrency ||
+              ![
+                ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME,
+                CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME
+              ].includes(name)
+            );
           }),
           rows: [performance]
         })

libs/common/src/lib/scopes.ts

@@ -74,8 +74,8 @@ const SCOPES_OF_TYPE: Record<AccessType, readonly Scope[]> = {
 };
 
 /**
- * Access types which combine a write scope with the restricted read access,
+ * Access types which can combine a write scope with the restricted read access,
- * because their tools change data without exposing the monetary values
+ * because their tools which change data do not expose the monetary values
  */
 export function canGrantRestrictedWriteAccess({ type }: { type: AccessType }) {
   return type === 'MCP';