From 7dadb2393c9b76bbf405b2304a23da5bc78ecb1e Mon Sep 17 00:00:00 2001 From: Milena Czierlinski <146972016+Milena-Czierlinski@users.noreply.github.com> Date: Wed, 19 Feb 2025 12:55:02 +0100 Subject: [PATCH] Prevent creation of RepositoryAttribute duplicates by accepting a ReadAttributeRequestItem (#423) * feat: return validation error trying to create duplicate * test: canAccept doesn't allow to create duplicate * feat: add tags property to AcceptWithExistingAttributeParams * feat: validate added tags in canAccept * test: added tags in canAccept * feat: extend IdentityAttribute validation to IQLQuery * feat: succeed RepositoryAttribute to add tags calling accept * fix: merge tags in accept * test: answer with existing Attribute and new tags * test: delete Attributes before each test * test: check that tags are not used for ThirdPartyRelationshipAttributeQuery --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- ...cceptReadAttributeRequestItemParameters.ts | 15 + .../ReadAttributeRequestItemProcessor.ts | 46 ++- packages/consumption/test/core/TestUtil.ts | 5 + .../ReadAttributeRequestItemProcessor.test.ts | 295 +++++++++++++++++- 4 files changed, 357 insertions(+), 4 deletions(-) diff --git a/packages/consumption/src/modules/requests/itemProcessors/readAttribute/AcceptReadAttributeRequestItemParameters.ts b/packages/consumption/src/modules/requests/itemProcessors/readAttribute/AcceptReadAttributeRequestItemParameters.ts index 091c84d7d..eadf51af2 100644 --- a/packages/consumption/src/modules/requests/itemProcessors/readAttribute/AcceptReadAttributeRequestItemParameters.ts +++ b/packages/consumption/src/modules/requests/itemProcessors/readAttribute/AcceptReadAttributeRequestItemParameters.ts @@ -7,6 +7,7 @@ import { AcceptRequestItemParametersJSON } from "../../incoming/decide/AcceptReq export interface AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON extends AcceptRequestItemParametersJSON { existingAttributeId: string; + tags?: string[]; } export interface AcceptReadAttributeRequestItemParametersWithNewAttributeJSON extends AcceptRequestItemParametersJSON { @@ -23,6 +24,10 @@ export class AcceptReadAttributeRequestItemParameters extends Serializable { @validate({ nullable: true }) public existingAttributeId?: CoreId; + @serialize({ type: String }) + @validate({ nullable: true, customValidator: IdentityAttribute.validateTags }) + public tags?: string[]; + @serialize({ unionTypes: [IdentityAttribute, RelationshipAttribute] }) @validate({ nullable: true }) public newAttribute?: IdentityAttribute | RelationshipAttribute; @@ -54,6 +59,16 @@ export class AcceptReadAttributeRequestItemParameters extends Serializable { ); } + if (value.newAttribute && value.tags) { + throw new ValidationError( + AcceptReadAttributeRequestItemParameters.name, + nameof((x) => x.newAttribute), + `You cannot specify both ${nameof( + (x) => x.newAttribute + )} and ${nameof((x) => x.tags)}.` + ); + } + if (!value.existingAttributeId && !value.newAttribute) { throw new ValidationError( AcceptReadAttributeRequestItemParameters.name, diff --git a/packages/consumption/src/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.ts b/packages/consumption/src/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.ts index e3d569080..100c63668 100644 --- a/packages/consumption/src/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.ts +++ b/packages/consumption/src/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.ts @@ -3,6 +3,7 @@ import { AttributeSuccessionAcceptResponseItem, IdentityAttribute, IdentityAttributeQuery, + IQLQuery, ReadAttributeAcceptResponseItem, ReadAttributeRequestItem, RejectResponseItem, @@ -17,7 +18,7 @@ import { CoreAddress, CoreId } from "@nmshd/core-types"; import { RelationshipStatus, TransportCoreErrors } from "@nmshd/transport"; import { nameof } from "ts-simple-nameof"; import { ConsumptionCoreErrors } from "../../../../consumption/ConsumptionCoreErrors"; -import { AttributeSuccessorParams, LocalAttributeDeletionStatus, LocalAttributeShareInfo, PeerSharedAttributeSucceededEvent } from "../../../attributes"; +import { AttributeSuccessorParams, IAttributeSuccessorParams, LocalAttributeDeletionStatus, LocalAttributeShareInfo, PeerSharedAttributeSucceededEvent } from "../../../attributes"; import { LocalAttribute } from "../../../attributes/local/LocalAttribute"; import { ValidationResult } from "../../../common/ValidationResult"; import { GenericRequestItemProcessor } from "../GenericRequestItemProcessor"; @@ -99,7 +100,11 @@ export class ReadAttributeRequestItemProcessor extends GenericRequestItemProcess attribute = foundLocalAttribute.content; - if (requestItem.query instanceof IdentityAttributeQuery && attribute instanceof IdentityAttribute && this.accountController.identity.isMe(attribute.owner)) { + if ( + (requestItem.query instanceof IdentityAttributeQuery || requestItem.query instanceof IQLQuery) && + attribute instanceof IdentityAttribute && + this.accountController.identity.isMe(attribute.owner) + ) { if (foundLocalAttribute.isShared()) { return ValidationResult.error( ConsumptionCoreErrors.requests.attributeQueryMismatch( @@ -132,6 +137,10 @@ export class ReadAttributeRequestItemProcessor extends GenericRequestItemProcess ) ); } + + if (parsedParams.tags && parsedParams.tags.length > 0) { + attribute.tags = attribute.tags ? [...attribute.tags, ...parsedParams.tags] : parsedParams.tags; + } } if (requestItem.query instanceof ThirdPartyRelationshipAttributeQuery && attribute instanceof RelationshipAttribute) { @@ -173,6 +182,12 @@ export class ReadAttributeRequestItemProcessor extends GenericRequestItemProcess if (nonPendingRelationshipsToPeer.length === 0) { return ValidationResult.error(ConsumptionCoreErrors.requests.cannotShareRelationshipAttributeOfPendingRelationship()); } + + if (parsedParams.tags && parsedParams.tags.length > 0) { + return ValidationResult.error( + ConsumptionCoreErrors.requests.invalidAcceptParameters("When responding to a ThirdPartyRelationshipAttributeQuery, no tags may be specified.") + ); + } } } else if (parsedParams.isWithNewAttribute()) { if (requestItem.query instanceof ThirdPartyRelationshipAttributeQuery) { @@ -183,6 +198,20 @@ export class ReadAttributeRequestItemProcessor extends GenericRequestItemProcess ); } + if (requestItem.query instanceof IdentityAttributeQuery) { + const existingRepositoryAttribute = await this.consumptionController.attributes.getRepositoryAttributeWithSameValue( + (parsedParams.newAttribute.value as any).toJSON() + ); + + if (existingRepositoryAttribute) { + return ValidationResult.error( + ConsumptionCoreErrors.requests.invalidAcceptParameters( + `The new Attribute cannot be created because it has the same content.value as the already existing RepositoryAttribute with id '${existingRepositoryAttribute.id.toString()}'.` + ) + ); + } + } + attribute = parsedParams.newAttribute; const ownerIsEmptyString = attribute.owner.equals(""); @@ -253,11 +282,22 @@ export class ReadAttributeRequestItemProcessor extends GenericRequestItemProcess let sharedLocalAttribute; if (parsedParams.isWithExistingAttribute()) { - const existingSourceAttribute = await this.consumptionController.attributes.getLocalAttribute(parsedParams.existingAttributeId); + let existingSourceAttribute = await this.consumptionController.attributes.getLocalAttribute(parsedParams.existingAttributeId); if (!existingSourceAttribute) { throw TransportCoreErrors.general.recordNotFound(LocalAttribute, parsedParams.existingAttributeId.toString()); } + if (parsedParams.tags && parsedParams.tags.length > 0 && existingSourceAttribute.content instanceof IdentityAttribute) { + const mergedTags = existingSourceAttribute.content.tags ? [...existingSourceAttribute.content.tags, ...parsedParams.tags] : parsedParams.tags; + existingSourceAttribute.content.tags = mergedTags; + + const successorParams: IAttributeSuccessorParams = { + content: existingSourceAttribute.content + }; + const attributesAfterSuccession = await this.consumptionController.attributes.succeedRepositoryAttribute(parsedParams.existingAttributeId, successorParams); + existingSourceAttribute = attributesAfterSuccession.successor; + } + const latestSharedVersion = await this.consumptionController.attributes.getSharedVersionsOfAttribute(parsedParams.existingAttributeId, [requestInfo.peer], true); const wasSharedBefore = latestSharedVersion.length > 0; diff --git a/packages/consumption/test/core/TestUtil.ts b/packages/consumption/test/core/TestUtil.ts index 6d5c0ae54..bb95da7c6 100644 --- a/packages/consumption/test/core/TestUtil.ts +++ b/packages/consumption/test/core/TestUtil.ts @@ -476,4 +476,9 @@ export class TestUtil { const file = await from.files.sendFile(params); return file; } + + public static async cleanupAttributes(consumptionController: ConsumptionController): Promise { + const attributes = await consumptionController.attributes.getLocalAttributes({}); + await Promise.all(attributes.map((attribute) => consumptionController.attributes.deleteAttributeUnsafe(attribute.id))); + } } diff --git a/packages/consumption/test/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.test.ts b/packages/consumption/test/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.test.ts index 059dfc2d7..5da484275 100644 --- a/packages/consumption/test/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.test.ts +++ b/packages/consumption/test/modules/requests/itemProcessors/readAttribute/ReadAttributeRequestItemProcessor.test.ts @@ -2,6 +2,7 @@ import { IDatabaseConnection } from "@js-soft/docdb-access-abstractions"; import { AttributeAlreadySharedAcceptResponseItem, AttributeSuccessionAcceptResponseItem, + GivenName, IdentityAttribute, IdentityAttributeQuery, IQLQuery, @@ -61,6 +62,8 @@ describe("ReadAttributeRequestItemProcessor", function () { aThirdParty = thirdPartyAccountController.identity.address; }); + beforeEach(async () => await TestUtil.cleanupAttributes(consumptionController)); + afterAll(async function () { await connection.close(); }); @@ -650,7 +653,52 @@ describe("ReadAttributeRequestItemProcessor", function () { }); }); - test("can be called with property tags used in the IdentityAttributeQuery", async function () { + test("returns an error trying to answer with a new IdentityAttribute that is a duplicate of an existing RepositoryAttribute", async function () { + const sender = CoreAddress.from("Sender"); + const recipient = accountController.identity.address; + + const repositoryAttribute = await consumptionController.attributes.createRepositoryAttribute({ + content: TestObjectFactory.createIdentityAttribute({ + owner: recipient, + value: GivenName.fromAny({ value: "aGivenName" }) + }) + }); + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: IdentityAttributeQuery.from({ valueType: "GivenName" }) + }); + const requestId = await ConsumptionIds.request.generate(); + const request = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithNewAttributeJSON = { + accept: true, + newAttribute: TestObjectFactory.createIdentityAttribute({ + owner: recipient, + value: GivenName.fromAny({ value: "aGivenName" }) + }).toJSON() + }; + + const result = await processor.canAccept(requestItem, acceptParams, request); + + expect(result).errorValidationResult({ + code: "error.consumption.requests.invalidAcceptParameters", + message: `The new Attribute cannot be created because it has the same content.value as the already existing RepositoryAttribute with id '${repositoryAttribute.id.toString()}'.` + }); + }); + + test("returns success responding with a new Attribute that has additional tags than those requested by the IdentityAttributeQuery", async function () { const sender = CoreAddress.from("Sender"); const recipient = accountController.identity.address; @@ -689,6 +737,47 @@ describe("ReadAttributeRequestItemProcessor", function () { expect(result).successfulValidationResult(); }); + + test("returns success responding with an existing Attribute and specifying additional tags that are requested by the IdentityAttributeQuery", async function () { + const sender = CoreAddress.from("Sender"); + const recipient = accountController.identity.address; + + const repositoryAttribute = await consumptionController.attributes.createRepositoryAttribute({ + content: TestObjectFactory.createIdentityAttribute({ + owner: recipient, + value: GivenName.fromAny({ value: "aGivenName" }), + tags: ["anExistingTag"] + }) + }); + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: IdentityAttributeQuery.from({ tags: ["aNewTag", "anotherNewTag"], valueType: "GivenName" }) + }); + const requestId = await ConsumptionIds.request.generate(); + const request = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON = { + accept: true, + existingAttributeId: repositoryAttribute.id.toString(), + tags: ["aNewTag"] + }; + + const result = await processor.canAccept(requestItem, acceptParams, request); + + expect(result).successfulValidationResult(); + }); }); describe("canAccept ReadAttributeRequestitem with IQLQuery", function () { @@ -1186,6 +1275,60 @@ describe("ReadAttributeRequestItemProcessor", function () { }); }); + test("returns an error trying to respond with tags", async function () { + const sender = CoreAddress.from("Sender"); + const recipient = accountController.identity.address; + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: ThirdPartyRelationshipAttributeQuery.from({ + owner: ThirdPartyRelationshipAttributeQueryOwner.Recipient, + key: "aKey", + thirdParty: [aThirdParty.toString()] + }) + }); + const requestId = await ConsumptionIds.request.generate(); + const request = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const localAttribute = await consumptionController.attributes.createSharedLocalAttribute({ + content: RelationshipAttribute.from({ + key: "aKey", + confidentiality: RelationshipAttributeConfidentiality.Public, + owner: recipient, + value: ProprietaryString.from({ + title: "aTitle", + value: "aStringValue" + }) + }), + peer: aThirdParty, + requestReference: await ConsumptionIds.request.generate() + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON = { + accept: true, + existingAttributeId: localAttribute.id.toString(), + tags: ["aTag"] + }; + + const result = await processor.canAccept(requestItem, acceptParams, request); + + expect(result).errorValidationResult({ + code: "error.consumption.requests.invalidAcceptParameters", + message: "When responding to a ThirdPartyRelationshipAttributeQuery, no tags may be specified." + }); + }); + test("returns an error when a RelationshipAttribute was queried using a ThirdPartyRelationshipAttributeQuery and the Recipient tries to respond with a new RelationshipAttribute", async function () { const sender = CoreAddress.from("Sender"); @@ -1382,6 +1525,49 @@ describe("ReadAttributeRequestItemProcessor", function () { expect(createdAttribute!.shareInfo!.peer.toString()).toStrictEqual(incomingRequest.peer.toString()); }); + test("accept with existing RepositoryAttribute and new tags", async function () { + const sender = CoreAddress.from("Sender"); + + const repositoryAttribute = await consumptionController.attributes.createRepositoryAttribute({ + content: TestObjectFactory.createIdentityAttribute({ + owner: accountController.identity.address, + tags: ["anExistingTag"] + }) + }); + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: IdentityAttributeQuery.from({ tags: ["aNewTag", "anotherNewTag"], valueType: "GivenName" }) + }); + const requestId = await ConsumptionIds.request.generate(); + const incomingRequest = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON = { + accept: true, + existingAttributeId: repositoryAttribute.id.toString(), + tags: ["aNewTag"] + }; + + const result = await processor.accept(requestItem, acceptParams, incomingRequest); + expect(result).toBeInstanceOf(ReadAttributeAcceptResponseItem); + + const ownSharedIdentityAttribute = await consumptionController.attributes.getLocalAttribute((result as ReadAttributeAcceptResponseItem).attributeId); + const sourceAttribute = await consumptionController.attributes.getLocalAttribute(ownSharedIdentityAttribute!.shareInfo!.sourceAttribute!); + expect(sourceAttribute!.succeeds).toBeDefined(); + expect((sourceAttribute!.content as IdentityAttribute).tags).toStrictEqual(["anExistingTag", "aNewTag"]); + }); + test("accept with new IdentityAttribute", async function () { const sender = CoreAddress.from("Sender"); const recipient = accountController.identity.address; @@ -1545,6 +1731,64 @@ describe("ReadAttributeRequestItemProcessor", function () { expect(updatedPredecessorOwnSharedIdentityAttribute!.succeededBy).toStrictEqual(successorOwnSharedIdentityAttribute!.id); }); + test("accept with existing IdentityAttribute whose predecessor was already shared and new tags", async function () { + const sender = CoreAddress.from("Sender"); + + const predecessorRepositoryAttribute = await consumptionController.attributes.createRepositoryAttribute({ + content: TestObjectFactory.createIdentityAttribute({ + owner: accountController.identity.address, + value: GivenName.fromAny({ value: "aGivenName" }), + tags: ["anExistingTag"] + }) + }); + + await consumptionController.attributes.createSharedLocalAttributeCopy({ + sourceAttributeId: predecessorRepositoryAttribute.id, + peer: sender, + requestReference: CoreId.from("initialRequest") + }); + + const { successor: successorRepositoryAttribute } = await consumptionController.attributes.succeedRepositoryAttribute(predecessorRepositoryAttribute.id, { + content: IdentityAttribute.from({ + owner: accountController.identity.address, + value: GivenName.fromAny({ value: "aNewGivenName" }), + tags: ["anExistingTag"] + }) + }); + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: IdentityAttributeQuery.from({ tags: ["aNewTag", "anotherNewTag"], valueType: "GivenName" }) + }); + const requestId = await ConsumptionIds.request.generate(); + const incomingRequest = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON = { + accept: true, + existingAttributeId: successorRepositoryAttribute.id.toString(), + tags: ["aNewTag"] + }; + + const result = await processor.accept(requestItem, acceptParams, incomingRequest); + expect(result).toBeInstanceOf(AttributeSuccessionAcceptResponseItem); + + const successorOwnSharedIdentityAttribute = await consumptionController.attributes.getLocalAttribute((result as AttributeSuccessionAcceptResponseItem).successorId); + const sourceAttribute = await consumptionController.attributes.getLocalAttribute(successorOwnSharedIdentityAttribute!.shareInfo!.sourceAttribute!); + expect(sourceAttribute!.succeeds).toStrictEqual(successorRepositoryAttribute.id); + expect((sourceAttribute!.content as IdentityAttribute).tags).toStrictEqual(["anExistingTag", "aNewTag"]); + }); + test("accept with existing IdentityAttribute whose predecessor was already shared but is DeletedByPeer", async function () { const sender = CoreAddress.from("Sender"); @@ -1720,6 +1964,55 @@ describe("ReadAttributeRequestItemProcessor", function () { expect((result as AttributeAlreadySharedAcceptResponseItem).attributeId).toStrictEqual(alreadySharedAttribute.id); }); + test("accept with existing IdentityAttribute that is already shared and the latest shared version and new tags", async function () { + const sender = CoreAddress.from("Sender"); + + const repositoryAttribute = await consumptionController.attributes.createRepositoryAttribute({ + content: TestObjectFactory.createIdentityAttribute({ + owner: accountController.identity.address, + tags: ["anExistingTag"] + }) + }); + + await consumptionController.attributes.createSharedLocalAttributeCopy({ + sourceAttributeId: repositoryAttribute.id, + peer: sender, + requestReference: await CoreIdHelper.notPrefixed.generate() + }); + + const requestItem = ReadAttributeRequestItem.from({ + mustBeAccepted: true, + query: IdentityAttributeQuery.from({ tags: ["aNewTag", "anotherNewTag"], valueType: "GivenName" }) + }); + const requestId = await ConsumptionIds.request.generate(); + const incomingRequest = LocalRequest.from({ + id: requestId, + createdAt: CoreDate.utc(), + isOwn: false, + peer: sender, + status: LocalRequestStatus.DecisionRequired, + content: Request.from({ + id: requestId, + items: [requestItem] + }), + statusLog: [] + }); + + const acceptParams: AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON = { + accept: true, + existingAttributeId: repositoryAttribute.id.toString(), + tags: ["aNewTag"] + }; + + const result = await processor.accept(requestItem, acceptParams, incomingRequest); + expect(result).toBeInstanceOf(AttributeSuccessionAcceptResponseItem); + + const successorOwnSharedIdentityAttribute = await consumptionController.attributes.getLocalAttribute((result as AttributeSuccessionAcceptResponseItem).successorId); + const sourceAttribute = await consumptionController.attributes.getLocalAttribute(successorOwnSharedIdentityAttribute!.shareInfo!.sourceAttribute!); + expect(sourceAttribute!.succeeds).toStrictEqual(repositoryAttribute.id); + expect((sourceAttribute!.content as IdentityAttribute).tags).toStrictEqual(["anExistingTag", "aNewTag"]); + }); + test("accept with existing IdentityAttribute that is already shared and the latest shared version but is DeletedByPeer", async function () { const sender = CoreAddress.from("Sender");