diff --git a/src/subdomains/core/custody/controllers/custody-account.controller.ts b/src/subdomains/core/custody/controllers/custody-account.controller.ts index a8af59d97a..73723cddef 100644 --- a/src/subdomains/core/custody/controllers/custody-account.controller.ts +++ b/src/subdomains/core/custody/controllers/custody-account.controller.ts @@ -88,7 +88,8 @@ export class CustodyAccountController { dto.description, ); - return CustodyAccountDtoMapper.toDto(custodyAccount, CustodyAccessLevel.WRITE); + // The caller just created it, so it is theirs by definition. + return CustodyAccountDtoMapper.toDto(custodyAccount, CustodyAccessLevel.WRITE, true); } @Put(':id') @@ -107,7 +108,13 @@ export class CustodyAccountController { dto.description, ); - return CustodyAccountDtoMapper.toDto(custodyAccount, CustodyAccessLevel.WRITE); + // Reached through the write guard, which a grantee passes too — ownership is a separate + // question from the level and has to be answered from the account itself. + return CustodyAccountDtoMapper.toDto( + custodyAccount, + CustodyAccessLevel.WRITE, + custodyAccount.isOwnedBy(jwt.account), + ); } @Get(':id/balance') diff --git a/src/subdomains/core/custody/dto/output/custody-account.dto.ts b/src/subdomains/core/custody/dto/output/custody-account.dto.ts index 5f35f0dc31..87aebd0e15 100644 --- a/src/subdomains/core/custody/dto/output/custody-account.dto.ts +++ b/src/subdomains/core/custody/dto/output/custody-account.dto.ts @@ -22,6 +22,9 @@ export class CustodyAccountDto { @ApiProperty({ enum: CustodyAccessLevel, description: 'Access level for current user' }) accessLevel: CustodyAccessLevel; + @ApiProperty({ description: 'Whether the current user owns this account rather than being granted access to it' }) + isOwner: boolean; + @ApiPropertyOptional({ type: CustodyUserDto }) owner?: CustodyUserDto; } diff --git a/src/subdomains/core/custody/entities/__tests__/custody-account.entity.spec.ts b/src/subdomains/core/custody/entities/__tests__/custody-account.entity.spec.ts new file mode 100644 index 0000000000..b8c1fccdc1 --- /dev/null +++ b/src/subdomains/core/custody/entities/__tests__/custody-account.entity.spec.ts @@ -0,0 +1,78 @@ +import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { CustodyAccessLevel, CustodyAccountStatus } from '../../enums/custody'; +import { CustodyAccountAccess } from '../custody-account-access.entity'; +import { CustodyAccount } from '../custody-account.entity'; + +describe('CustodyAccount', () => { + const ownerId = 100; + const granteeId = 200; + const strangerId = 300; + const custodyAccountId = 1; + + function userData(overrides: Partial = {}): UserData { + return Object.assign(new UserData(), { id: ownerId, users: [], custodyAccounts: [], ...overrides }); + } + + function custodyAccount(overrides: Partial = {}): CustodyAccount { + return Object.assign(new CustodyAccount(), { + id: custodyAccountId, + title: 'Own Safe', + description: 'Owner account', + owner: userData(), + requiredSignatures: 1, + status: CustodyAccountStatus.ACTIVE, + accessGrants: [], + ...overrides, + }); + } + + function accessGrant(params: { + id?: number; + account: CustodyAccount; + userData: UserData; + accessLevel: CustodyAccessLevel; + active: boolean; + }): CustodyAccountAccess { + return Object.assign(new CustodyAccountAccess(), { + id: params.id ?? 10, + account: params.account, + userData: params.userData, + accessLevel: params.accessLevel, + active: params.active, + }); + } + + describe('#isOwnedBy(...)', () => { + it("returns true for the owner's user-data id", () => { + const account = custodyAccount(); + + expect(account.isOwnedBy(ownerId)).toBe(true); + }); + + it('returns false for a grantee with an active write grant on the account', () => { + const account = custodyAccount(); + const grant = accessGrant({ + account, + userData: userData({ id: granteeId }), + accessLevel: CustodyAccessLevel.WRITE, + active: true, + }); + account.accessGrants = [grant]; + + expect(account.isOwnedBy(granteeId)).toBe(false); + }); + + it('returns false for a user-data id with no ownership or access grant', () => { + const account = custodyAccount(); + const grant = accessGrant({ + account, + userData: userData({ id: granteeId }), + accessLevel: CustodyAccessLevel.WRITE, + active: true, + }); + account.accessGrants = [grant]; + + expect(account.isOwnedBy(strangerId)).toBe(false); + }); + }); +}); diff --git a/src/subdomains/core/custody/entities/custody-account.entity.ts b/src/subdomains/core/custody/entities/custody-account.entity.ts index 5d20057f62..3a8125d867 100644 --- a/src/subdomains/core/custody/entities/custody-account.entity.ts +++ b/src/subdomains/core/custody/entities/custody-account.entity.ts @@ -24,4 +24,13 @@ export class CustodyAccount extends IEntity { @OneToMany(() => CustodyAccountAccess, (access) => access.account) accessGrants: CustodyAccountAccess[]; + + /** + * Whether this account belongs to the given user_data, as opposed to merely being reachable + * through a grant. Distinct from the access level: a grantee can hold WRITE without owning + * anything, and an owner can narrow themselves to READ while still owning it. + */ + isOwnedBy(accountId: number): boolean { + return this.owner.id === accountId; + } } diff --git a/src/subdomains/core/custody/mappers/__tests__/custody-asset-balance-dto.mapper.spec.ts b/src/subdomains/core/custody/mappers/__tests__/custody-asset-balance-dto.mapper.spec.ts index c254de057b..1711bf9ae4 100644 --- a/src/subdomains/core/custody/mappers/__tests__/custody-asset-balance-dto.mapper.spec.ts +++ b/src/subdomains/core/custody/mappers/__tests__/custody-asset-balance-dto.mapper.spec.ts @@ -100,5 +100,279 @@ describe('CustodyAssetBalanceDtoMapper', () => { expect(result[0].interest).toBe(interest); expect(result[0].interestValue).toEqual({ chf: interest, eur: interest, usd: interest }); }); + + it('returns balances ordered by descending CHF value regardless of input order', () => { + const aaaAsset: Asset = createCustomAsset({ + id: 20, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 21, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const cccAsset: Asset = createCustomAsset({ + id: 22, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: 50 }), + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: 100 }), + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 500 }), + ]; + const interestByAssetName = new Map(); + + const result = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, interestByAssetName); + + expect(result.map((balance) => [balance.asset.name, balance.value.chf])).toEqual([ + ['BBB', 500], + ['AAA', 100], + ['CCC', 50], + ]); + }); + + it('falls back to alphabetical asset name when CHF values are equal', () => { + const aaaAsset: Asset = createCustomAsset({ + id: 30, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 31, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const cccAsset: Asset = createCustomAsset({ + id: 32, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: 100 }), + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 100 }), + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: 100 }), + ]; + const interestByAssetName = new Map(); + + const result = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, interestByAssetName); + + expect(result.map((balance) => balance.asset.name)).toEqual(['AAA', 'BBB', 'CCC']); + }); + + it('returns the same order regardless of input order', () => { + const aaaAsset: Asset = createCustomAsset({ + id: 40, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 41, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const cccAsset: Asset = createCustomAsset({ + id: 42, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const dddAsset: Asset = createCustomAsset({ + id: 43, + name: 'DDD', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 100 }), + Object.assign(new CustodyBalance(), { asset: dddAsset, balance: 50 }), + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: 200 }), + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: 100 }), + ]; + const firstInterestByAssetName = new Map(); + const secondInterestByAssetName = new Map(); + + const firstResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, firstInterestByAssetName); + const secondResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances( + [...balances].reverse(), + secondInterestByAssetName, + ); + + expect(firstResult.map((balance) => balance.asset.name)).toEqual( + secondResult.map((balance) => balance.asset.name), + ); + }); + + it('sorts negative CHF values after all positive values', () => { + const aaaAsset: Asset = createCustomAsset({ + id: 50, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 51, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const cccAsset: Asset = createCustomAsset({ + id: 52, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: -50 }), + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: 100 }), + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 500 }), + ]; + const interestByAssetName = new Map(); + + const result = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, interestByAssetName); + + expect(result.map((balance) => [balance.asset.name, balance.value.chf])).toEqual([ + ['BBB', 500], + ['CCC', 100], + ['AAA', -50], + ]); + }); + + it('places a non-finite CHF value last regardless of input order', () => { + const aaaAsset: Asset = createCustomAsset({ + id: 60, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 61, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const dddAsset: Asset = createCustomAsset({ + id: 62, + name: 'DDD', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const cccAsset: Asset = createCustomAsset({ + id: 63, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + // NaN plus two positive values is not enough: the old raw-value comparator and the ranked + // comparator happen to return the same order for both forward and reversed input, so the + // order-stability assertion cannot catch a regression. This four-value set adds a negative + // value: the old comparator returns BBB, CCC, DDD, AAA forward and BBB, AAA, CCC, DDD + // reversed, while the ranked comparator returns BBB, DDD, AAA, CCC both ways. Collapsing + // this back to three entries silently removes the only thing this test checks, so it must + // not be "simplified" later. + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: -5 }), + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 10 }), + Object.assign(new CustodyBalance(), { asset: dddAsset, balance: 0 }), + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: NaN }), + ]; + const firstInterestByAssetName = new Map(); + const secondInterestByAssetName = new Map(); + + const firstResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, firstInterestByAssetName); + const secondResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances( + [...balances].reverse(), + secondInterestByAssetName, + ); + + expect(firstResult.map((balance) => balance.asset.name)).toEqual( + secondResult.map((balance) => balance.asset.name), + ); + expect(firstResult[firstResult.length - 1].asset.name).toBe('CCC'); + expect(secondResult[secondResult.length - 1].asset.name).toBe('CCC'); + expect(firstResult[firstResult.length - 1].value.chf).toBeNaN(); + expect(secondResult[secondResult.length - 1].value.chf).toBeNaN(); + }); + + it('places a positive Infinity CHF value last, the same as a non-finite NaN value', () => { + const cccAsset: Asset = createCustomAsset({ + id: 70, + name: 'CCC', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const aaaAsset: Asset = createCustomAsset({ + id: 71, + name: 'AAA', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + const bbbAsset: Asset = createCustomAsset({ + id: 72, + name: 'BBB', + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + // Infinity is distinct from NaN because it is ordinarily comparable: the old raw-value + // comparator sorts it first instead of scrambling the order, so the position assertions + // catch the regression here. The order-stability assertion remains for consistency even + // though it does not distinguish the old and ranked comparators for this dataset. + const balances: CustodyBalance[] = [ + Object.assign(new CustodyBalance(), { asset: cccAsset, balance: Infinity }), + Object.assign(new CustodyBalance(), { asset: aaaAsset, balance: 200 }), + Object.assign(new CustodyBalance(), { asset: bbbAsset, balance: 100 }), + ]; + const firstInterestByAssetName = new Map(); + const secondInterestByAssetName = new Map(); + + const firstResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances(balances, firstInterestByAssetName); + const secondResult = CustodyAssetBalanceDtoMapper.mapCustodyBalances( + [...balances].reverse(), + secondInterestByAssetName, + ); + + expect(firstResult.map((balance) => balance.asset.name)).toEqual( + secondResult.map((balance) => balance.asset.name), + ); + expect(firstResult[firstResult.length - 1].asset.name).toBe('CCC'); + expect(secondResult[secondResult.length - 1].asset.name).toBe('CCC'); + expect(firstResult[firstResult.length - 1].value.chf).toBe(Infinity); + expect(secondResult[secondResult.length - 1].value.chf).toBe(Infinity); + }); }); }); diff --git a/src/subdomains/core/custody/mappers/custody-account-dto.mapper.ts b/src/subdomains/core/custody/mappers/custody-account-dto.mapper.ts index 225203f1b3..ed7de3a829 100644 --- a/src/subdomains/core/custody/mappers/custody-account-dto.mapper.ts +++ b/src/subdomains/core/custody/mappers/custody-account-dto.mapper.ts @@ -5,13 +5,14 @@ import { CustodyAccount } from '../entities/custody-account.entity'; import { CustodyAccessLevel } from '../enums/custody'; export class CustodyAccountDtoMapper { - static toDto(custodyAccount: CustodyAccount, accessLevel: CustodyAccessLevel): CustodyAccountDto { + static toDto(custodyAccount: CustodyAccount, accessLevel: CustodyAccessLevel, isOwner: boolean): CustodyAccountDto { return { id: custodyAccount.id, title: custodyAccount.title, description: custodyAccount.description, isLegacy: false, accessLevel, + isOwner, owner: custodyAccount.owner ? { id: custodyAccount.owner.id } : undefined, }; } @@ -23,6 +24,7 @@ export class CustodyAccountDtoMapper { description: undefined, isLegacy: true, accessLevel: CustodyAccessLevel.WRITE, + isOwner: true, owner: { id: userData.id }, }; } diff --git a/src/subdomains/core/custody/mappers/custody-asset-balance-dto.mapper.ts b/src/subdomains/core/custody/mappers/custody-asset-balance-dto.mapper.ts index 599d574a77..4f7a1a93aa 100644 --- a/src/subdomains/core/custody/mappers/custody-asset-balance-dto.mapper.ts +++ b/src/subdomains/core/custody/mappers/custody-asset-balance-dto.mapper.ts @@ -20,13 +20,31 @@ export class CustodyAssetBalanceDtoMapper { ): CustodyAssetBalanceDto[] { const groups = Util.groupByAccessor(custodyBalances, (b) => b.asset.name); - return Array.from(groups.values()).map((g) => { + const balances = Array.from(groups.values()).map((g) => { const asset = g[0].asset; const balance = Util.sumObjValue(g, 'balance'); const interestInfo = interestByAssetName.get(asset.name); return this.map(asset, balance, interestInfo); }); + + // The balances arrive in whatever order the database returned them — nothing orders that + // query — so the same holdings could come back in a different order on every request, with + // the list visibly reshuffling for no reason. Largest position first is both stable and the + // order someone reading a portfolio expects; equal values fall back to the name so the + // result is fully determined. + // + // A non-finite value is ranked last rather than compared. NaN is the reason: it makes every + // difference falsy, which would send the pair to the name comparison and cost the ordering + // its transitivity — the very unpredictability this sort exists to remove. Infinities are + // ranked the same way, not because they break the comparison but because they are the same + // class of corrupted data: no legitimate calculation produces one here, only an already + // broken balance can, and calculateAccruedInterest treats every non-finite figure alike for + // exactly that reason. It is not thrown, for the reason recorded there too: one damaged + // position must not take the customer's whole balance response down with it. + const rank = (value: number): number => (Number.isFinite(value) ? value : -Infinity); + + return balances.sort((a, b) => rank(b.value.chf) - rank(a.value.chf) || a.asset.name.localeCompare(b.asset.name)); } private static map(asset: Asset, balance: number, interestInfo?: CustodyInterestInfo): CustodyAssetBalanceDto { diff --git a/src/subdomains/core/custody/services/__tests__/custody-account.service.spec.ts b/src/subdomains/core/custody/services/__tests__/custody-account.service.spec.ts index 28f0cf8c90..7db1104b78 100644 --- a/src/subdomains/core/custody/services/__tests__/custody-account.service.spec.ts +++ b/src/subdomains/core/custody/services/__tests__/custody-account.service.spec.ts @@ -355,6 +355,7 @@ describe('CustodyAccountService', () => { id: ownAccountId, accessLevel: CustodyAccessLevel.WRITE, isLegacy: false, + isOwner: true, }), ); }); @@ -378,6 +379,7 @@ describe('CustodyAccountService', () => { id: ownAccountId, accessLevel: CustodyAccessLevel.READ, isLegacy: false, + isOwner: true, }), ); }); @@ -423,6 +425,7 @@ describe('CustodyAccountService', () => { id: foreignAccountId, accessLevel: CustodyAccessLevel.READ, isLegacy: false, + isOwner: false, }), ); @@ -431,6 +434,46 @@ describe('CustodyAccountService', () => { expect(ownOccurrences).toHaveLength(1); }); + it('lists a shared foreign account with write access as not owned', async () => { + const foreignAccount = foreignCustodyAccount(); + const sharedWriteGrant = accessGrant({ + account: foreignAccount, + userData: ownerUserData(), + accessLevel: CustodyAccessLevel.WRITE, + active: true, + }); + userDataService.getUserData.mockResolvedValue(ownerUserData()); + mockFindActiveGrants([sharedWriteGrant]); + + const result = await service.getCustodyAccountsForUser(ownerId); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: foreignAccountId, + accessLevel: CustodyAccessLevel.WRITE, + isLegacy: false, + isOwner: false, + }), + ); + }); + + it('lists a legacy account as owned for a custody user without owned accounts', async () => { + const custodyUserId = 55; + userDataService.getUserData.mockResolvedValue(ownerUserData({ users: [custodyRoleUser(custodyUserId)] })); + mockFindActiveGrants([]); + + const result = await service.getCustodyAccountsForUser(ownerId); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + isLegacy: true, + isOwner: true, + }), + ); + }); + it("filters out inactive grants, another user's grants, and grants on non-active accounts", async () => { const account = ownCustodyAccount(); const legitimateGrant = accessGrant({ diff --git a/src/subdomains/core/custody/services/custody-account.service.ts b/src/subdomains/core/custody/services/custody-account.service.ts index 52eda89931..bf4c540b90 100644 --- a/src/subdomains/core/custody/services/custody-account.service.ts +++ b/src/subdomains/core/custody/services/custody-account.service.ts @@ -68,12 +68,12 @@ export class CustodyAccountService { }, relations: { account: { owner: true } }, }); - const sharedAccounts = activeGrants.filter((a) => a.account.owner.id !== accountId); + const sharedAccounts = activeGrants.filter((a) => !a.account.isOwnedBy(accountId)); // A grant on an own account narrows the owner's level — see checkAccess. Without this the // list would offer WRITE where the authorisation only grants inspection. const ownLevelByAccount = new Map( - activeGrants.filter((a) => a.account.owner.id === accountId).map((a) => [a.account.id, a.accessLevel]), + activeGrants.filter((a) => a.account.isOwnedBy(accountId)).map((a) => [a.account.id, a.accessLevel]), ); const custodyAccounts: CustodyAccountDto[] = [ @@ -81,9 +81,9 @@ export class CustodyAccountService { // No grant on an own account means the owner keeps full disposal — that is the rule, // not a fallback for a missing value. const level = ownLevelByAccount.get(ca.id) ?? CustodyAccessLevel.WRITE; - return CustodyAccountDtoMapper.toDto(ca, level); + return CustodyAccountDtoMapper.toDto(ca, level, true); }), - ...sharedAccounts.map((a) => CustodyAccountDtoMapper.toDto(a.account, a.accessLevel)), + ...sharedAccounts.map((a) => CustodyAccountDtoMapper.toDto(a.account, a.accessLevel, false)), ]; // Legacy Safe = absence of any owned account row; independent of shared grants. Hidden @@ -157,7 +157,7 @@ export class CustodyAccountService { // owner's own grant is the only way to express that, so it must not be overridden here. // Managing grants stays with the owner regardless (requireOwner), so this cannot lock // anyone out of their own account. - if (custodyAccount.owner.id === accountId && !access) { + if (custodyAccount.isOwnedBy(accountId) && !access) { return { custodyAccount, isLegacy: false }; } @@ -208,7 +208,7 @@ export class CustodyAccountService { const ownerId = custodyAccount.owner.id; // Owner already holds every Safe row; multi-account ambiguity only matters for grantees. - if (ownerId === callerAccountId) { + if (custodyAccount.isOwnedBy(callerAccountId)) { return ownerId; } @@ -384,7 +384,7 @@ export class CustodyAccountService { relations: { owner: true }, }); - if (!custodyAccount || custodyAccount.owner.id !== accountId) { + if (!custodyAccount || !custodyAccount.isOwnedBy(accountId)) { throw new ForbiddenException('Only the account owner can manage access grants'); } @@ -446,7 +446,7 @@ export class CustodyAccountService { account: CustodyAccount, newLevel: CustodyAccessLevel, ): void { - const isOwnGrant = access.userData.id === account.owner.id; + const isOwnGrant = account.isOwnedBy(access.userData.id); const isElevation = access.accessLevel === CustodyAccessLevel.READ && newLevel === CustodyAccessLevel.WRITE; if (!isOwnGrant && isElevation && account.status !== CustodyAccountStatus.ACTIVE) { @@ -461,7 +461,7 @@ export class CustodyAccountService { * account without an owner row and make the level unrecordable, so it stays refused. */ private rejectOwnerGrantRevocation(access: CustodyAccountAccess, account: CustodyAccount): void { - if (access.userData.id === account.owner.id) { + if (account.isOwnedBy(access.userData.id)) { throw new BadRequestException("Cannot revoke the account owner's access grant"); } }