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

Implement PresentationDefinition field filter schema #102

Merged
merged 15 commits into from
Oct 27, 2023
2 changes: 2 additions & 0 deletions config/detekt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ style:
excludeGuardClauses: true
SpacingBetweenPackageAndImports:
active: true
ThrowsCount:
active: false
UnnecessaryAbstractClass:
active: true
UnnecessaryInheritance:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public class SubmissionRequirement(
/**
* Enumeration representing presentation rule options.
*/
// TODO this does not serialize correctly but sub reqs not supported right now
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite follow what this comment means. Would you mind elaborating? And perhaps also opening an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jackson does not know how to serialize/deserialize enum class Rules and enum class Optionality in a SubmissionRequirement, we probably need to write custom serializers/deserializers. However, the PEX impl throws an exception if SubmissionRequirement is present, so it doesn't seem worth writing the custom modules.

Want to have a convo with @decentralgabe around whether we should drop SubmissionRequirement from our data model entirely.

Can open an issue to track.

Copy link
Member

Choose a reason for hiding this comment

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

I'd just drop it for now - can add it back when/if needed

public enum class Rules {
All, Pick
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,59 +1,127 @@
package web5.sdk.credentials

import com.fasterxml.jackson.databind.JsonNode
import com.networknt.schema.JsonSchema
import com.nfeld.jsonpathkt.JsonPath
import com.nfeld.jsonpathkt.extension.read
import com.nimbusds.jose.Payload
import com.nimbusds.jwt.JWTParser
import com.nimbusds.jwt.SignedJWT

/**
* A utility object for performing operations related to presentation exchanges.
* The `PresentationExchange` object provides functions for working with Verifiable Credentials
* and Presentation Definitions during a presentation exchange process.
*/
public object PresentationExchange {
/**
* Selects credentials from the given list that satisfy the provided presentation definition.
* Selects credentials that satisfy a given presentation definition.
*
* @param credentials A list of verifiable credentials.
* @param presentationDefinition The Presentation Definition to be satisfied.
* @return A list of verifiable credentials that meet the presentation definition criteria.
* @param credentials The list of Verifiable Credentials to select from.
* @param presentationDefinition The Presentation Definition to match against.
* @return A list of Verifiable Credentials that satisfy the Presentation Definition.
* @throws UnsupportedOperationException If the method is untested and not recommended for use.
*/
public fun selectCredentials(
credentials: List<VerifiableCredential>,
presentationDefinition: PresentationDefinitionV2
): List<VerifiableCredential> {
throw UnsupportedOperationException("pex is untested")
// return credentials.filter { satisfiesPresentationDefinition(it, presentationDefinition) }
// Uncomment the following line to filter credentials based on the Presentation Definition
// return credentials.filter { satisfiesPresentationDefinition(it, presentationDefinition) }
}

/**
* Checks if the given [presentationDefinition] is satisfied based on the provided input descriptors and constraints.
* Validates if a Verifiable Credential JWT satisfies a Presentation Definition.
*
* @param presentationDefinition The Presentation Definition to be evaluated.
* @return `true` if the Presentation Definition is satisfied, `false` otherwise.
* @throws UnsupportedOperationException if certain features like Submission Requirements or Field Filters are not implemented.
* @param vcJwt The Verifiable Credential JWT as a string.
* @param presentationDefinition The Presentation Definition to validate against.
* @throws UnsupportedOperationException If the Presentation Definition's Submission Requirements
* feature is not implemented.
*/
public fun satisfiesPresentationDefinition(
credential: VerifiableCredential,
vcJwt: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious what was the motivation behind this change?

I worry that people may forget to verify the jwt, before doing anything with it. How do you propose we can handle that concern?

Copy link
Contributor

@nitro-neal nitro-neal Oct 27, 2023

Choose a reason for hiding this comment

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

I think its because ssome of the filter could be for the actual payload of the vc
$.iss is the correct way to filter by issuer not vc.issuer (this will be removed as per spec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

agree w/ the concern, but it's better to decouple the logic IMO

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like you're advocating for leaving as is. I remain cautious about the approach. Agree with the intent of decoupling verification logic from validation.

My proposal is to stick to the following convention:

  • When a function is dealing with data that's assumed to be trusted, use the VerifiableCredential type.
  • When a function is dealing with data that needs to be verified, use the secured representation (i.e. vcJwt or DI).

For this function, the data is assumed to be trusted already. Given that, it would fall to the first bucket.
On the other hand, the VerifiableCredential.verify function would follow the second pattern.

Alternatively, we could define types to represent:

  • A secured verifiable credential: JwVerifiableCredential and DataIntegrityVerifiableCredential.
  • An unsecured verifiable credential UnsecuredVerifiableCredential.

This would make it very difficult for users to screw up.

Either of these two would be more beneficial IMHO. Those are my suggestions, I'll leave it up to @phoebe-lew to decide whether they're useful or not.

presentationDefinition: PresentationDefinitionV2
): Boolean {
) {
val vc = JWTParser.parse(vcJwt) as SignedJWT

if (!presentationDefinition.submissionRequirements.isNullOrEmpty()) {
throw UnsupportedOperationException(
"Presentation Definition's Submission Requirements feature is not implemented"
)
}

return presentationDefinition.inputDescriptors
presentationDefinition.inputDescriptors
.filter { !it.constraints.fields.isNullOrEmpty() }
.all { inputDescriptorWithFields ->
val requiredFields = inputDescriptorWithFields.constraints.fields!!.filter { it.optional != true }

var satisfied = true
for (field in requiredFields) {
// we ignore field filters
if (field.filterSchema != null) {
throw UnsupportedOperationException("Field Filter is not implemented")
}
.forEach { inputDescriptorWithFields ->
Copy link
Contributor

@nitro-neal nitro-neal Oct 27, 2023

Choose a reason for hiding this comment

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

Does this cover the scenario:
if there are no filter and only a path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should via ln79 but will add a test :)

validateInputDescriptorsWithFields(inputDescriptorWithFields, vc.payload)
}
}

/**
* Validates the input descriptors with associated fields in a Verifiable Credential.
*
* @param inputDescriptorWithFields The Input Descriptor with associated fields.
* @param vcPayload The payload of the Verifiable Credential.
*/
private fun validateInputDescriptorsWithFields(
inputDescriptorWithFields: InputDescriptorV2,
vcPayload: Payload
) {
val requiredFields = inputDescriptorWithFields.constraints.fields!!.filter { it.optional != true }

requiredFields.forEach { field ->
val vcPayloadJson = JsonPath.parse(vcPayload.toString())
?: throw PresentationExchangeError("Failed to parse VC $vcPayload as JsonNode")

Check warning on line 74 in credentials/src/main/kotlin/web5/sdk/credentials/PresentationExchange.kt

View check run for this annotation

Codecov / codecov/patch

credentials/src/main/kotlin/web5/sdk/credentials/PresentationExchange.kt#L74

Added line #L74 was not covered by tests

val matchedFields = field.path.mapNotNull { path -> vcPayloadJson.read<JsonNode>(path) }
if (matchedFields.isEmpty()) {
throw PresentationExchangeError("Could not find matching field for path: ${field.path.joinToString()}")

Check warning on line 78 in credentials/src/main/kotlin/web5/sdk/credentials/PresentationExchange.kt

View check run for this annotation

Codecov / codecov/patch

credentials/src/main/kotlin/web5/sdk/credentials/PresentationExchange.kt#L78

Added line #L78 was not covered by tests
}

if (field.path.any { path -> credential.getFieldByJsonPath(path) == null }) {
satisfied = false
break
when {
field.filterSchema != null -> {
matchedFields.any { fieldValue ->
when {
// When the field is an array, JSON schema is applied to each array item.
fieldValue.isArray -> {
if (fieldValue.none { valueSatisfiesFieldFilterSchema(it, field.filterSchema!!) })
throw PresentationExchangeError("Validating $fieldValue against ${field.filterSchema} failed")
true
}

// Otherwise, JSON schema is applied to the entire value.
else -> {
valueSatisfiesFieldFilterSchema(fieldValue, field.filterSchema!!)
}
}
}
}
return satisfied

else -> return
}
}
}

/**
* Checks if a field's value satisfies the given JSON schema.
*
* @param fieldValue The JSON field value to validate.
* @param schema The JSON schema to validate against.
* @return `true` if the value satisfies the schema, `false` otherwise.
*/
private fun valueSatisfiesFieldFilterSchema(fieldValue: JsonNode, schema: JsonSchema): Boolean {
val validationMessages = schema.validate(fieldValue)
return when {
validationMessages.isEmpty() -> true
// TODO try and surface the validation messages in error
else -> false
}
}
}

/**
* Custom error class for exceptions related to the Presentation Exchange.
*
* @param message The error message.
*/
public class PresentationExchangeError(message: String) : Error(message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't find where error is defined, is it an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error is java.lang.Error, I'm actually not sure when to use error vs exception

Copy link
Contributor

Choose a reason for hiding this comment

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

From https://docs.oracle.com/javase%2F7%2Fdocs%2Fapi%2F%2F/java/lang/Error.html

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.

That sounds really scary 😱

In other PRs, we've tried to stick subclassing RuntimeException, like what's below https://github.com/TBD54566975/web5-kt/blob/02ea4b31a28cf68a4a3f9a9abc745e3273ceee79/dids/src/main/kotlin/web5/sdk/dids/DidIonManager.kt#L720

Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import org.erdtman.jcs.JsonCanonicalizer
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import java.io.File

class PresentationDefinitionTest {
val jsonMapper: ObjectMapper = ObjectMapper()
.registerKotlinModule()
.findAndRegisterModules()
.setSerializationInclusion(JsonInclude.Include.NON_NULL)

@Test
Expand Down Expand Up @@ -54,19 +54,19 @@ class PresentationDefinitionTest {

@Test
fun `serialization is idempotent`(){
val pdString = PRESENTATION_DEFINITION.trimIndent()
val pdString = File("src/test/resources/pd_sanctions.json").readText().trimIndent()
val parsedPd = jsonMapper.readValue(pdString, PresentationDefinitionV2::class.java)
val parsedString = jsonMapper.writeValueAsString(parsedPd)

assertEquals(
JsonCanonicalizer(PRESENTATION_DEFINITION).encodedString,
JsonCanonicalizer(pdString).encodedString,
JsonCanonicalizer(parsedString).encodedString,
)
}

@Test
fun `can deserialize`() {
val pdString = PRESENTATION_DEFINITION.trimIndent()
val pdString = File("src/test/resources/pd_sanctions.json").readText().trimIndent()

assertDoesNotThrow { jsonMapper.readValue(pdString, PresentationDefinitionV2::class.java) }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package web5.sdk.credentials

import assertk.assertFailure
import assertk.assertions.messageContains
import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.kotlin.registerKotlinModule
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.assertDoesNotThrow
import web5.sdk.crypto.InMemoryKeyManager
import web5.sdk.dids.DidKey
import java.io.File
import kotlin.test.Test

class PresentationExchangeTest {
private val keyManager = InMemoryKeyManager()
private val issuerDid = DidKey.create(keyManager)
private val holderDid = DidKey.create(keyManager)
private val jsonMapper: ObjectMapper = ObjectMapper()
.registerKotlinModule()
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious what this does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one gets jackson's objectmapper to work nicely with kotlin

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting! First time i've seen this. Thanks!

.setSerializationInclusion(JsonInclude.Include.NON_NULL)
@Suppress("MaximumLineLength")
val sanctionsVcJwt =
"eyJhbGciOiJFZERTQSJ9.eyJpc3MiOiJkaWQ6a2V5Ono2TWtrdU5tSmF0ZUNUZXI1V0JycUhCVUM0YUM3TjlOV1NyTURKNmVkQXY1V0NmMiIsInN1YiI6ImRpZDprZXk6ejZNa2t1Tm1KYXRlQ1RlcjVXQnJxSEJVQzRhQzdOOU5XU3JNREo2ZWRBdjVXQ2YyIiwidmMiOnsiQGNvbnRleHQiOlsiaHR0cHM6Ly93d3cudzMub3JnLzIwMTgvY3JlZGVudGlhbHMvdjEiXSwiaWQiOiIxNjk4NDIyNDAxMzUyIiwidHlwZSI6WyJWZXJpZmlhYmxlQ3JlZGVudGlhbCIsIlNhbmN0aW9uc0NyZWRlbnRpYWwiXSwiaXNzdWVyIjoiZGlkOmtleTp6Nk1ra3VObUphdGVDVGVyNVdCcnFIQlVDNGFDN045TldTck1ESjZlZEF2NVdDZjIiLCJpc3N1YW5jZURhdGUiOiIyMDIzLTEwLTI3VDE2OjAwOjAxWiIsImNyZWRlbnRpYWxTdWJqZWN0Ijp7ImlkIjoiZGlkOmtleTp6Nk1ra3VObUphdGVDVGVyNVdCcnFIQlVDNGFDN045TldTck1ESjZlZEF2NVdDZjIiLCJiZWVwIjoiYm9vcCJ9fX0.Xhd9nDdkGarYFr6FP7wqsgj5CK3oGTfKU2LHNMvFIsvatgYlSucShDPI8uoeJ_G31uYPke-LJlRy-WVIhkudDg"


private fun readPd(path: String): String {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private fun readPd(path: String): String {
private fun readFile(path: String): String {

return File(path).readText().trimIndent()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return File(path).readText().trimIndent()
return File(path).readText()

}

@Nested
inner class SatisfiesPresentationDefinition {
@Test
fun `does not throw when VC satisfies tbdex PD`() {
val pd = jsonMapper.readValue(
readPd("src/test/resources/pd_sanctions.json"),
PresentationDefinitionV2::class.java
)

assertDoesNotThrow { PresentationExchange.satisfiesPresentationDefinition(sanctionsVcJwt, pd) }
}

@Test
fun `does not throw when VC satisfies PD with field filter schema on array`() {
val pd = jsonMapper.readValue(
readPd("src/test/resources/pd_filter_array.json"),
PresentationDefinitionV2::class.java
)
val vc = VerifiableCredential.create(
type = "StreetCred",
issuer = issuerDid.uri,
subject = holderDid.uri,
data = StreetCredibility(localRespect = "high", legit = true)
)
val vcJwt = vc.sign(issuerDid)

assertDoesNotThrow { PresentationExchange.satisfiesPresentationDefinition(vcJwt, pd) }
}

@Test
fun `does not throw when VC satisfies PD with field filter schema on value`() {
val pd = jsonMapper.readValue(
readPd("src/test/resources/pd_filter_value.json"),
PresentationDefinitionV2::class.java
)
val vc = VerifiableCredential.create(
type = "StreetCred",
issuer = issuerDid.uri,
subject = holderDid.uri,
data = StreetCredibility(localRespect = "high", legit = true)
)
val vcJwt = vc.sign(issuerDid)

assertDoesNotThrow { PresentationExchange.satisfiesPresentationDefinition(vcJwt, pd) }
}

@Test
fun `does not throw when VC satisfies PD with field constraint`() {
val pd = jsonMapper.readValue(
readPd("src/test/resources/pd_path_no_filter.json"),
PresentationDefinitionV2::class.java
)
val vc = VerifiableCredential.create(
type = "StreetCred",
issuer = issuerDid.uri,
subject = holderDid.uri,
data = StreetCredibility(localRespect = "high", legit = true)
)
val vcJwt = vc.sign(issuerDid)

assertDoesNotThrow { PresentationExchange.satisfiesPresentationDefinition(vcJwt, pd) }
}

@Test
fun `throws when VC does not satisfy requirements`() {
val pd = jsonMapper.readValue(
readPd("src/test/resources/pd_sanctions.json"),
PresentationDefinitionV2::class.java
)
val vc = VerifiableCredential.create(
type = "StreetCred",
issuer = issuerDid.uri,
subject = holderDid.uri,
data = StreetCredibility(localRespect = "high", legit = true)
)
val vcJwt = vc.sign(issuerDid)

assertFailure {
PresentationExchange.satisfiesPresentationDefinition(vcJwt, pd)
}.messageContains("Validating [\"VerifiableCredential\",\"StreetCred\"]")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class StatusListCredentialTest {

@Test
fun `should parse valid VerifiableCredential from specification example`() {
val specExampleRevocableVcText = File("src/test/testdata/revocableVc.json").readText()
val specExampleRevocableVcText = File("src/test/resources/revocable_vc.json").readText()

val specExampleRevocableVc = VerifiableCredential.fromJson(
specExampleRevocableVcText
Expand Down
34 changes: 0 additions & 34 deletions credentials/src/test/kotlin/web5/sdk/credentials/TestData.kt

This file was deleted.

25 changes: 25 additions & 0 deletions credentials/src/test/resources/pd_filter_array.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"id": "ec11a434-fe24-479b-aae0-511428b37e4f",
"input_descriptors": [
{
"id": "7b928839-f0b1-4237-893d-b27124b57952",
"constraints": {
"fields": [
{
"path": [
"$.vc.type[*]",
"$.type[*]"
],
"filter": {
"type": "string",
"contains": {
"type": "string",
"const": "StreetCred"
}
}
}
]
}
}
]
}
Loading
Loading