Skip to content

Commit

Permalink
Prevent creation of RepositoryAttribute duplicates by accepting a Rea…
Browse files Browse the repository at this point in the history
…dAttributeRequestItem (#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>
  • Loading branch information
Milena-Czierlinski and mergify[bot] authored Feb 19, 2025
1 parent 4514483 commit 7dadb23
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { AcceptRequestItemParametersJSON } from "../../incoming/decide/AcceptReq

export interface AcceptReadAttributeRequestItemParametersWithExistingAttributeJSON extends AcceptRequestItemParametersJSON {
existingAttributeId: string;
tags?: string[];
}

export interface AcceptReadAttributeRequestItemParametersWithNewAttributeJSON extends AcceptRequestItemParametersJSON {
Expand All @@ -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;
Expand Down Expand Up @@ -54,6 +59,16 @@ export class AcceptReadAttributeRequestItemParameters extends Serializable {
);
}

if (value.newAttribute && value.tags) {
throw new ValidationError(
AcceptReadAttributeRequestItemParameters.name,
nameof<AcceptReadAttributeRequestItemParameters>((x) => x.newAttribute),
`You cannot specify both ${nameof<AcceptReadAttributeRequestItemParameters>(
(x) => x.newAttribute
)} and ${nameof<AcceptReadAttributeRequestItemParameters>((x) => x.tags)}.`
);
}

if (!value.existingAttributeId && !value.newAttribute) {
throw new ValidationError(
AcceptReadAttributeRequestItemParameters.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
AttributeSuccessionAcceptResponseItem,
IdentityAttribute,
IdentityAttributeQuery,
IQLQuery,
ReadAttributeAcceptResponseItem,
ReadAttributeRequestItem,
RejectResponseItem,
Expand All @@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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("");
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions packages/consumption/test/core/TestUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,4 +476,9 @@ export class TestUtil {
const file = await from.files.sendFile(params);
return file;
}

public static async cleanupAttributes(consumptionController: ConsumptionController): Promise<void> {
const attributes = await consumptionController.attributes.getLocalAttributes({});
await Promise.all(attributes.map((attribute) => consumptionController.attributes.deleteAttributeUnsafe(attribute.id)));
}
}
Loading

0 comments on commit 7dadb23

Please sign in to comment.