feat: auto accept proofs #367

berendsliedrecht · 2021-07-06T08:32:17Z

This adds automatically accepting of proof requests. This functions the same as #336 to stay as consistent as possible, yet they work completely seperate to eachother.

It allows for an agent- and recordwide configuration where the record configuration has priority.

Three states are allowed to be set:

always: accept any proof request/proposal no matter what the content might be.
contentApproved: accept any proof request/proposal as long as none of the content changes and it has to be accepted manually at least once.
never (DEFAULT) Does not automate anything and will just behave the same as before these changes.

Note: I am not sure whether we want to mention this as a feature in the README, so if there is any problem with this I will remove it.

Signed-off-by: Berend Sliedrecht <berend@animo.id>

codecov-commenter · 2021-07-06T08:57:41Z

Codecov Report

Merging #367 (7f39d2a) into main (3331a3c) will increase coverage by 0.10%.
The diff coverage is 90.81%.

@@            Coverage Diff             @@
##             main     #367      +/-   ##
==========================================
+ Coverage   86.42%   86.52%   +0.10%     
==========================================
  Files         234      236       +2     
  Lines        4803     4891      +88     
  Branches      750      773      +23     
==========================================
+ Hits         4151     4232      +81     
- Misses        652      659       +7

Impacted Files	Coverage Δ
packages/core/src/types.ts	`100.00% <ø> (ø)`
...ules/proofs/handlers/ProposePresentationHandler.ts	`82.60% <77.77%> (-17.40%)`	⬇️
...ules/proofs/handlers/RequestPresentationHandler.ts	`88.00% <85.00%> (-12.00%)`	⬇️
...src/modules/proofs/handlers/PresentationHandler.ts	`89.47% <85.71%> (-10.53%)`	⬇️
packages/core/src/agent/AgentConfig.ts	`91.22% <100.00%> (+0.48%)`	⬆️
...ges/core/src/modules/proofs/ProofAutoAcceptType.ts	`100.00% <100.00%> (ø)`
...ore/src/modules/proofs/ProofResponseCoordinator.ts	`100.00% <100.00%> (ø)`
packages/core/src/modules/proofs/ProofsModule.ts	`89.53% <100.00%> (+0.78%)`	⬆️
packages/core/src/modules/proofs/index.ts	`100.00% <100.00%> (ø)`
.../core/src/modules/proofs/repository/ProofRecord.ts	`89.74% <100.00%> (+0.26%)`	⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3331a3c...7f39d2a. Read the comment docs.

berendsliedrecht · 2021-07-06T10:53:29Z

Tests on Github are failing while local tests work. Will resolve this asap.

Signed-off-by: Berend Sliedrecht <berend@animo.id>

TimoGlastra

LGTM. Nice work @blu3beri!

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

Signed-off-by: Berend Sliedrecht <berend@animo.id>

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

Signed-off-by: Berend Sliedrecht <berend@animo.id>

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

JamesKEbert

If I recall correctly, we're waiting for additional followup to move the relevant validation changes into the service regardless of the automation configuration? Or am I remembering incorrectly here?

berendsliedrecht · 2021-07-17T22:06:32Z

If I recall correctly, we're waiting for additional followup to move the relevant validation changes into the service regardless of the automation configuration? Or am I remembering incorrectly here?

My understanding was first to merge this. And in a seprate PR abstract the functionality, add documentation stating the types of autoAccept. I think for now this will suffice, but I would not mind to change that stuff and merge it later.

jakubkoci · 2021-07-18T10:35:39Z

packages/core/src/modules/proofs/handlers/RequestPresentationHandler.ts

+    proofRecord: ProofRecord,
+    messageContext: HandlerInboundMessage<RequestPresentationHandler>
+  ) {
+    const indyProofRequest = proofRecord.requestMessage?.indyProofRequest


This makes the handler kind of Indy specific.

Will fix this when we support multiple credential types.

jakubkoci · 2021-07-18T11:16:21Z

packages/core/tests/proofs-auto-accept.test.ts

+import { setupProofsTest, waitForProofRecord } from './helpers'
+import testLogger from './logger'
+
+describe('Auto accept present proof', () => {


Shouldn't we have tests also for accept never?

It seems we're testing just a happy path.

Since I assumed the default is Never, the proofs.test.ts will error if it does any automatic response. I could add the never` clause as a test, but it would feel a bit redundant.

jakubkoci · 2021-07-18T11:25:38Z

packages/core/src/modules/proofs/ProofResponseCoordinator.ts

+
+import { AgentConfig } from '../../agent/AgentConfig'
+
+import { AutoAcceptProof } from './ProofAutoAcceptType'


Maybe we should have two types, like AutoAcceptProof for the holder and AutoAcceptProofRequest request for the issuer. Similarly with credentials. But that's of course more of a future improvement idea.

Any particular reason for this? Maybe it allows for some use cases that I don't know of.

I see 2 reasons:
The first is expressiveness. AutoAcceptProof means auto acceptance proof from the verifier point of view but also auto acceptance of the proof request from the holder's perspective. Even now, I'm not sure if I wrote it correctly.

The second is a potential use case when someone uses an agent for more than one role. Let's say I want to accept proofs as a verifier but don't want to accept proof requests as a holder.

TimoGlastra · 2021-07-19T07:58:50Z

Merging. Additional improvements can be made in follow-up PRs

berendsliedrecht added 14 commits July 6, 2021 10:14

feat: propose proof always is auto accept

9dd797d

Signed-off-by: Berend Sliedrecht <berend@animo.id>

fixed for testing

8632876

Signed-off-by: Berend Sliedrecht <berend@animo.id>

fix for testing

8c0a1c7

Signed-off-by: Berend Sliedrecht <berend@animo.id>

feat: auto accept proof always working

73dfdfa

Signed-off-by: Berend Sliedrecht <berend@animo.id>

improved testing and added contentApproved

0826f19

Signed-off-by: Berend Sliedrecht <berend@animo.id>

revert changed from auto accept credential

f7d194e

Signed-off-by: Berend Sliedrecht <berend@animo.id>

removed autoAcceptCredential

456fce7

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Removed tags

1a9627d

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Copied from credentialresponsecoordinator

6c73742

Signed-off-by: Berend Sliedrecht <berend@animo.id>

minified tests

8d383ee

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Consistent logging

b46eff5

Signed-off-by: Berend Sliedrecht <berend@animo.id>

test: more in sync with other tests

5849035

Signed-off-by: Berend Sliedrecht <berend@animo.id>

added functionality in the README

2a1fad0

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Added better documentation

bc5aa3e

Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht requested a review from a team as a code owner July 6, 2021 08:32

Fix: functions were not async

1596a25

Signed-off-by: Berend Sliedrecht <berend@animo.id>

TimoGlastra approved these changes Jul 7, 2021

View reviewed changes

berendsliedrecht and others added 6 commits July 9, 2021 22:18

Merge branch 'main' into feature/auto-accept-proof

8e3c139

Merge remote-tracking branch 'origin/main' into feature/auto-accept-p…

659b17c

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

merged

44cebf0

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Merge remote-tracking branch 'origin/main' into feature/auto-accept-p…

938402f

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

Fix: feedback from auto-accept-credential

23f1d86

Signed-off-by: Berend Sliedrecht <berend@animo.id>

Merge remote-tracking branch 'origin/main' into feature/auto-accept-p…

7f39d2a

…roof Signed-off-by: Berend Sliedrecht <berend@animo.id>

JamesKEbert reviewed Jul 17, 2021

View reviewed changes

jakubkoci approved these changes Jul 18, 2021

View reviewed changes

TimoGlastra merged commit 735d578 into openwallet-foundation:main Jul 19, 2021

TimoGlastra mentioned this pull request Jul 19, 2021

Credentials and proofs auto-acceptance improvements #391

Closed

berendsliedrecht deleted the feature/auto-accept-proof branch August 31, 2022 07:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auto accept proofs #367

feat: auto accept proofs #367

berendsliedrecht commented Jul 6, 2021

codecov-commenter commented Jul 6, 2021 •

edited

Loading

berendsliedrecht commented Jul 6, 2021

TimoGlastra left a comment

JamesKEbert left a comment

berendsliedrecht commented Jul 17, 2021

jakubkoci Jul 18, 2021

berendsliedrecht Jul 18, 2021

jakubkoci Jul 18, 2021

jakubkoci Jul 18, 2021

berendsliedrecht Jul 18, 2021

jakubkoci Jul 18, 2021

berendsliedrecht Jul 18, 2021

jakubkoci Jul 18, 2021 •

edited

Loading

TimoGlastra commented Jul 19, 2021


		import { AgentConfig } from '../../agent/AgentConfig'

		import { AutoAcceptProof } from './ProofAutoAcceptType'

feat: auto accept proofs #367

feat: auto accept proofs #367

Conversation

berendsliedrecht commented Jul 6, 2021

codecov-commenter commented Jul 6, 2021 • edited Loading

Codecov Report

berendsliedrecht commented Jul 6, 2021

TimoGlastra left a comment

Choose a reason for hiding this comment

JamesKEbert left a comment

Choose a reason for hiding this comment

berendsliedrecht commented Jul 17, 2021

jakubkoci Jul 18, 2021

Choose a reason for hiding this comment

berendsliedrecht Jul 18, 2021

Choose a reason for hiding this comment

jakubkoci Jul 18, 2021

Choose a reason for hiding this comment

jakubkoci Jul 18, 2021

Choose a reason for hiding this comment

berendsliedrecht Jul 18, 2021

Choose a reason for hiding this comment

jakubkoci Jul 18, 2021

Choose a reason for hiding this comment

berendsliedrecht Jul 18, 2021

Choose a reason for hiding this comment

jakubkoci Jul 18, 2021 • edited Loading

Choose a reason for hiding this comment

TimoGlastra commented Jul 19, 2021

codecov-commenter commented Jul 6, 2021 •

edited

Loading

jakubkoci Jul 18, 2021 •

edited

Loading