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

present-proof v2 puts records to the storage it can't read back any more #1687

Closed
matgnt opened this issue Mar 24, 2022 · 1 comment · Fixed by #1690
Closed

present-proof v2 puts records to the storage it can't read back any more #1687

matgnt opened this issue Mar 24, 2022 · 1 comment · Fixed by #1690
Assignees

Comments

@matgnt
Copy link
Contributor

matgnt commented Mar 24, 2022

I ran into this issue and I think it might be possible at the moment that someone sends data to running 3rd party acapy instances to break their usage of reading their own received presentation exchanges from acapy - and it wouldn't be straight forward to recover from it (deleting such records via the API).

Here are the steps to reproduce the situation:
I'm trying to send a W3C presentation. I'm using
/present-proof-2.0/send-proposal for this and I'm probably using some wrong format I guess, but this is not the problem here.
Here is the (probably wrong) data that I send to the endpoint:


{
    "auto_present": true,
    "comment": "test",
    "connection_id": "7a5d96b0-98fa-4208-9cec-8779ebb55c21",
    "presentation_proposal": {
      "dif": {

        "input_descriptors": [
            {
                "id": "XXX",
                "name": "CO2 Confirmation",
                "schema": [
                    {
                        "uri": "https://www.w3.org/2018/credentials#VerifiableCredential"
                    }
                ],
                "constraints": {
                    "fields": [
                        {
                            "path": [
                                "$.type"
                            ],
                            "filter": {
                                "type": "string",
                                "pattern": "Co2ConfirmationCredential"
                            }
                        }
                    ]
                }
            }
        ]
      }
    },
    "trace": false
  }

Above data is just to make the problem reproducible.

I'm using sub-wallets with askar profile: "--multitenancy-config", "{\"wallet_type\":\"askar-profile\"}"

Now, what happens on the other side (receiver of the proposal, the acapy behind the given connection_id) is (a lot of auto-flags ON... but probably not relevant here) acapy stores a record that it can't read back any more.
When trying to read via /present-proof-2.0/records

marshmallow.exceptions.ValidationError: {'presentation_definition': ['Missing data for required field.'], 'input_descriptors': ['Unknown field.']}

It breaks in a way that the endpoint doesn't deliver any other item from this endpoint any more.
Debugging the pres_ex_id and trying to delete from the according endpoint DELETE /present-proof-2.0/records/{pres_ex_id} also doesn't work because it tries to read the item from storage before it removes it.

So my question - if I should try to fix this - what would be the expected behavior in such cases?

  1. we should never get such wrong data items into the storage in the first place and it's ok to not being able to read this AND other data back from storage in such rare cases (current situation)
  2. DELETE should still work and command line should print out the problematic pres_ex_id
  3. Listing records should still work for non-problematic records
  4. Listing records should still work and add a "dummy" record that just contains the pres_ex_id for the problematic record (being able to use the DELETE endpoint with it...)
  5. any other options?
@swcurran
Copy link
Contributor

Good questions -- and that is an ugly bug. Needs to be fixed pretty quickly. @shaangill025 and @andrewwhitehead please review and weigh in on this.

My $0.02CDN (but I would question if I'm the right person here...):

The first one is the most important. To me, the data validation needs to be beefed up in this case. My question would be whether that is all we need to do?

Item 3 should still work, but I suspect that would create an ugly performance issue -- e.g. having to read record by record from the database somehow vs. getting the set. If it is possible, could it be that if the current query be tried and on failure, go to some clean up routine to iterate through the records to list the good ones.

After that, I'm over my head. I hope option 1 is sufficient.

@shaangill025 shaangill025 self-assigned this Mar 24, 2022
matgnt added a commit to boschresearch/aries-cloudagent-python that referenced this issue Mar 30, 2022
Related to openwallet-foundation#1687 this allows deleting a presentation exchange
record without reading it from storage.

Signed-off-by: Matthias Binzer <matthias.binzer@de.bosch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants