Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issue where attributes and predicates match #640

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion demo/src/Faber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export class Faber extends BaseAgent {

public async acceptConnection(invitation_url: string) {
const connectionRecord = await this.receiveConnectionRequest(invitation_url)

this.connectionRecordAliceId = await this.waitForConnection(connectionRecord)
}

Expand Down
13 changes: 7 additions & 6 deletions packages/core/src/modules/proofs/services/ProofService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { EventEmitter } from '../../../agent/EventEmitter'
import { InjectionSymbols } from '../../../constants'
import { Attachment, AttachmentData } from '../../../decorators/attachment/Attachment'
import { AriesFrameworkError } from '../../../error'
import { checkProofRequestForDuplicates } from '../../../utils'
import { JsonEncoder } from '../../../utils/JsonEncoder'
import { JsonTransformer } from '../../../utils/JsonTransformer'
import { uuid } from '../../../utils/uuid'
Expand Down Expand Up @@ -257,8 +258,8 @@ export class ProofService {
comment?: string
}
): Promise<ProofProtocolMsgReturnType<RequestPresentationMessage>> {
// Assert attribute and predicate group names do not match
this.assertAttributePredicateGroupNamesDoNotMatch(proofRequest)
// Assert attribute and predicate (group) names do not match
checkProofRequestForDuplicates(proofRequest)

// Assert
proofRecord.assertState(ProofState.ProposalReceived)
Expand Down Expand Up @@ -305,8 +306,8 @@ export class ProofService {
): Promise<ProofProtocolMsgReturnType<RequestPresentationMessage>> {
this.logger.debug(`Creating proof request`)

// Assert attribute and predicate group names do not match
this.assertAttributePredicateGroupNamesDoNotMatch(proofRequest)
// Assert attribute and predicate (group) names do not match
checkProofRequestForDuplicates(proofRequest)

// Assert
connectionRecord?.assertReady()
Expand Down Expand Up @@ -369,8 +370,8 @@ export class ProofService {
}
await validateOrReject(proofRequest)

// Assert attribute and predicate group names do not match
this.assertAttributePredicateGroupNamesDoNotMatch(proofRequest)
// Assert attribute and predicate (group) names do not match
checkProofRequestForDuplicates(proofRequest)

this.logger.debug('received proof request', proofRequest)

Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/utils/assertNoDuplicates.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { AriesFrameworkError } from '../error/AriesFrameworkError'

export function assertNoDuplicatesInArray(arr: string[]) {
const arrayLength = arr.length
const uniqueArrayLength = new Set(arr).size

if (arrayLength === uniqueArrayLength) return

const duplicates = arr.filter((item, index) => arr.indexOf(item) != index)
throw new AriesFrameworkError(`The proof request contains duplicate items: ${duplicates.toString()}`)
}
1 change: 1 addition & 0 deletions packages/core/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export * from './MultiBaseEncoder'
export * from './buffer'
export * from './MultiHashEncoder'
export * from './JWE'
export * from './indyProofRequest'
22 changes: 22 additions & 0 deletions packages/core/src/utils/indyProofRequest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { ProofRequest } from '../modules/proofs/models/ProofRequest'

import { assertNoDuplicatesInArray } from './assertNoDuplicates'

export function attributesToArray(proofRequest: ProofRequest) {
// Attributes can contain either a `name` string value or an `names` string array. We reduce it to a single array
// containing all attribute names from the requested attributes.
return Array.from(proofRequest.requestedAttributes.values()).reduce<string[]>(
(names, a) => [...names, ...(a.name ? [a.name] : a.names ? a.names : [])],
[]
)
}

export function predicatesToArray(proofRequest: ProofRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think predicateNamesToArray, and same for attributes, Is a better name.

return Array.from(proofRequest.requestedPredicates.values()).map((a) => a.name)
}

export function checkProofRequestForDuplicates(proofRequest: ProofRequest) {
const attributes = attributesToArray(proofRequest)
const predicates = predicatesToArray(proofRequest)
assertNoDuplicatesInArray(attributes.concat(predicates))
}
26 changes: 26 additions & 0 deletions packages/core/tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,29 @@ export async function setupProofsTest(faberName: string, aliceName: string, auto
aliceReplay,
}
}

export async function setupSecondCredential(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still being used somewhere?

faberAgent: Agent,
aliceAgent: Agent,
faberConnection: ConnectionRecord
): Promise<string> {
const { definition } = await prepareForIssuance(faberAgent, ['name', 'age'])

const credentialPreview = CredentialPreview.fromRecord({
name: 'John',
age: '99',
})

await issueCredential({
issuerAgent: faberAgent,
issuerConnectionId: faberConnection.id,
holderAgent: aliceAgent,
credentialTemplate: {
credentialDefinitionId: definition.id,
comment: 'some comment about credential',
preview: credentialPreview,
},
})

return definition.id
}
168 changes: 164 additions & 4 deletions packages/core/tests/proofs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,19 @@ import {
ProofState,
ProposePresentationMessage,
RequestPresentationMessage,
ProofRequest,
AriesFrameworkError,
} from '../src'
import { checkProofRequestForDuplicates } from '../src/utils'

import { setupProofsTest, waitForProofRecord } from './helpers'
import { setupProofsTest, setupSecondCredential, waitForProofRecord } from './helpers'
import testLogger from './logger'

describe('Present Proof', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only tests the createRequest method. I think the e2e test were useful to check where indy throws error, but it might be easier to create unit tests for the specific service methods like you did with the credential service.

let faberAgent: Agent
let aliceAgent: Agent
let credDefId: CredDefId
let secCredDefId: string
let faberConnection: ConnectionRecord
let aliceConnection: ConnectionRecord
let presentationPreview: PresentationPreview
Expand All @@ -29,6 +33,8 @@ describe('Present Proof', () => {
testLogger.test('Initializing the agents')
;({ faberAgent, aliceAgent, credDefId, faberConnection, aliceConnection, presentationPreview } =
await setupProofsTest('Faber agent', 'Alice agent'))
testLogger.test('Issuing second credential')
secCredDefId = await setupSecondCredential(faberAgent, aliceAgent, faberConnection)
})

afterAll(async () => {
Expand Down Expand Up @@ -365,8 +371,162 @@ describe('Present Proof', () => {
requestedAttributes: attributes,
requestedPredicates: predicates,
})
).rejects.toThrowError(
`The proof request contains an attribute group name that matches a predicate group name: age`
)
).rejects.toThrowError(`The proof request contains duplicate items: age`)
})

test('attribute names match, same cred def filter', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are great! I think it would be good to move them to a util test in the utils tests directory. Then add tests to this file that makes sure the method is called where it should be (createRequest, etc...)

const attributes = {
name: new ProofAttributeInfo({
name: 'age',
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
age: new ProofAttributeInfo({
name: 'age',
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
}

const proofRequestOptions = {
name: 'test-proof-request',
requestedAttributes: attributes,
}

const nonce = 'testtesttest12345'

const proofRequest = new ProofRequest({
name: 'proof-request',
version: '1.0',
nonce,
requestedAttributes: proofRequestOptions.requestedAttributes,
})

expect(() => checkProofRequestForDuplicates(proofRequest)).toThrowError(AriesFrameworkError)
})

test('attribute names match with predicates name, same cred def filter', async () => {
const attributes = {
name: new ProofAttributeInfo({
name: 'age',
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
}

const predicates = {
age: new ProofPredicateInfo({
name: 'age',
predicateType: PredicateType.GreaterThanOrEqualTo,
predicateValue: 50,
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
}

const nonce = 'testtesttest12345'

const proofRequest = new ProofRequest({
name: 'proof-request',
version: '1.0',
nonce,
requestedAttributes: attributes,
requestedPredicates: predicates,
})

expect(() => checkProofRequestForDuplicates(proofRequest)).toThrowError(AriesFrameworkError)
})

test('attribute names match, different cred def filter', async () => {
const attributes = {
name: new ProofAttributeInfo({
name: 'age',
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
}

const predicates = {
age: new ProofPredicateInfo({
name: 'age',
predicateType: PredicateType.GreaterThanOrEqualTo,
predicateValue: 50,
restrictions: [
new AttributeFilter({
credentialDefinitionId: secCredDefId,
}),
],
}),
}

const proofRequestOptions = {
name: 'test-proof-request',
requestedAttributes: attributes,
predicates: predicates,
}

const nonce = 'testtesttest12345'

const proofRequest = new ProofRequest({
name: 'proof-request',
version: '1.0',
nonce,
requestedAttributes: proofRequestOptions.requestedAttributes,
})

expect(() => checkProofRequestForDuplicates(proofRequest)).resolves
})

test('attribute name matches with predicate name, different cred def filter', async () => {
const attributes = {
name: new ProofAttributeInfo({
name: 'age',
restrictions: [
new AttributeFilter({
credentialDefinitionId: credDefId,
}),
],
}),
}

const predicates = {
age: new ProofPredicateInfo({
name: 'age',
predicateType: PredicateType.GreaterThanOrEqualTo,
predicateValue: 50,
restrictions: [
new AttributeFilter({
credentialDefinitionId: secCredDefId,
}),
],
}),
}

const nonce = 'testtesttest12345'

const proofRequest = new ProofRequest({
name: 'proof-request',
version: '1.0',
nonce,
requestedAttributes: attributes,
requestedPredicates: predicates,
})

expect(() => checkProofRequestForDuplicates(proofRequest)).resolves
})
})