openwallet-foundation-labs / sd-jwt-kotlin

A Kotlin implementation of the Selective Disclosure JWT (SD-JWT) spec.
Apache License 2.0
23 stars 10 forks source link

extend credential creation using claims structure as json structure #3

Closed DhiaSlm closed 1 year ago

DhiaSlm commented 1 year ago

Currently we are only able to create an sd-jwt credential knowing its serializable java object at compile time. However, in a real life application where an Issuer would like to dynamically issue a credential which structure is only known at run time, the available method becomes not useful. A solution would be to create a credential out of a generic structure e.g json String or JSONObject.

DhiaSlm commented 1 year ago

@fabian-hk I have prepared a pull request but I still need the rights to push it.

fabian-hk commented 1 year ago

Can you please fork the repository and open a pull request?

DhiaSlm commented 1 year ago

@fabian-hk I have updated the PR and tried to include all your comments. It appeared to be a bit more complicated than expected, especially the validation part. I think it's ready for review now (I hope)

However, I still have some concern regarding structure validation of JSONArrays, more specifically in the case of an array of JSONObject. One question is, can we find a case where a claim is in the form of JSONArrays of JSONObject, and that the array elements have different structure? Following is an example:

"complexClaim" : [ {"key1" : "value1" , "key2":"value2"}, {"key3": "value3", "key4":"value4"}, {"signature":"valueOfSig"}, "nonJson" ]

complexStructure = JSONObject(
            mapOf(
                Pair("name", "Max Muster"),
                Pair(
                    "complexClaim", JSONArray(
                        setOf(
                            JSONObject(mapOf(Pair("key1", "value1"), Pair("key2", "value2"))),
                            JSONObject(mapOf(Pair("key3", "value3"), Pair("key4", "value4"))),
                            JSONObject(mapOf(Pair("signature", "valueOfSig"))),
                            "nonJson"
                        )
                    )
                )
            )
        )

In this case how is the disclosure structure can be build accordingly? For example if i want to disclose 1 element of the list e.g {"signature":"valueOfSig"}

Maybe you can check this out and suggest a test case if applicable?

Let me know if it is better to discuss this further (in a call)

fabian-hk commented 1 year ago

@DhiaSlm I reviewed your PR again and created some comments. I am not sure if this case with different JSON Array items is applicable. Do you know if this can be handled in the Python reference implementation? I think so far this implementation cannot handle this.

DhiaSlm commented 1 year ago

@fabian-hk First of all thank you for the review and the comments. Unfortunately I don't have an idea about other libs other than kotlin libs (in addition to this one, I played around a bit with waltid lib, which has similar behavior as it is based on nimbus jwt lib ), and I never worked with Python so far. Besides, even though theoritically possible, I cant think of a real life credential use case where a json array of claims has different structures as elements. Usually, an array of elements should include elements of the same structure, for example a claim that is listing a holder uni degree listing which subjects in the degree or even listing certificates along the academic career. In that case, I think it makes sense to enforce the same structure of elements in the list, otherwise it can be unnecessarily complex model.

fabian-hk commented 1 year ago

@DhiaSlm for me it is okay to enforce the same type in an array right now. The current implementation with the data classes enforces it automatically. If somebody needs a different behavior in the future we can still change the logic.