Handle direct_post return from wallet to verifier #2702
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
woutslakhorst
force-pushed
the
feature/2694/verifier_handle_response
branch
4 times, most recently
from
1a31b91 to
a7a7bf1
Compare
…2626) wip added bunch of tests added missing API tests fix failing test add tests for failing directpost responses refactor ami client and reuse response parsing/code checking happy flow tests for holder service error tests for holder service comment touchup on some comments
woutslakhorst
force-pushed
the
feature/2694/verifier_handle_response
branch
from
a7a7bf1 to
c3f1594
Compare
remove url decode from middleware, now handled by codegen bind add callback, fix request logger fix logger test add e2e test added handling of error direct_post
woutslakhorst
force-pushed
the
feature/2694/verifier_handle_response
branch
from
c3f1594 to
5963fd9
Compare
reinkrul
reviewed
Jan 18, 2024
| signInstruction.Mappings[0].Path = "$.verifiableCredential" | ||
| } | ||
|
|
||
| // todo the check below actually depends on the format of the credential and not the format of the VP |
There was a problem hiding this comment.
It did create an error when running the e2e-test. But "fixing" it now is a bit pro-active since VC formats are going to be changed anyway.
reinkrul
requested changes
Jan 18, 2024
vcr/pe/presentation_submission.go
Outdated
Comment on lines
154
to
155
| // commented for now because it's needed for VPs in JWT format as well. | ||
| //if format == vc.JSONLDPresentationProofFormat { |
There was a problem hiding this comment.
just remove comment and commented out code?
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create callback URL: %w", err) | ||
| } | ||
| callbackURL = callbackURL.JoinPath("callback") |
There was a problem hiding this comment.
not specified on the OpenAPI spec?
reinkrul
reviewed
Jan 22, 2024
| }, | ||
| }, nil | ||
| } | ||
| log.Logger().Errorf("verifier responded with incorrect callbackURI: \"%s\" for wallet: %s", redirectURI, walletDID.String()) |
There was a problem hiding this comment.
Suggested change
| log.Logger().Errorf("verifier responded with incorrect callbackURI: \"%s\" for wallet: %s", redirectURI, walletDID.String()) | |
| log.Logger().Errorf("Verifier responded with incorrect callbackURI: \"%s\" for wallet: %s", redirectURI, walletDID) |
reinkrul
approved these changes
Jan 22, 2024
gerardsn
reviewed
Jan 22, 2024
| summary: Used by wallets to post the authorization response or error to. | ||
| description: | | ||
| Specified by https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#name-response-mode-direct_postjw | ||
| The response is either an error response with error, error_description and state filled or a submission with vp_token and presentation_submission filled. |
There was a problem hiding this comment.
State param is RECOMMENDED in an authorization request, but MUST always be in the response if present in the request. Other implementation are likely not to rely on the nonce
RFC6749
state
REQUIRED if the "state" parameter was present in the client
authorization request. The exact value received from the
client.
rolandgroen
added a commit
that referenced
this pull request
Jan 25, 2024
* master: (38 commits) VDR: Fix duplicate results when listing DIDs (#2743) VDR: API to list owned DIDs (#2742) VDR: Move did:nuts specific to did:nuts creator (#2735) Discovery: API to list configured Discovery Services (#2739) PKI: Adjust validation logging; OK = trace, NOK = info (#2736) Discovery: update client copy of Discovery Services (#2718) VDR: Group all DID Document management funcs in a new interface (#2737) Handle direct_post return from wallet to verifier (#2702) Bump github.com/nats-io/nats.go from 1.31.0 to 1.32.0 (#2724) Discovery: fix API of Client.Search() (#2730) Bump github.com/amacneil/dbmate/v2 from 2.10.0 to 2.11.0 (#2729) Discovery: DID registration by clients (#2709) Config: Add HTTP-client timeout (#2725) Policy: prevent nil dereference when no backend is configured (#2723) Bump github.com/nats-io/nats-server/v2 from 2.10.7 to 2.10.9 (#2721) SQL: specify lengths for VARCHAR columns to avoid different behavior with different RDBMS's (#2717) Bump golang from 1.21.5-alpine to 1.21.6-alpine (#2719) Handle authorization request from verifier by holder/wallet (#2680) Bump golang.org/x/crypto from 0.17.0 to 0.18.0 (#2716) Bump github.com/lestrrat-go/jwx/v2 from 2.0.18 to 2.0.19 (#2715) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #2694
the /token API is also implemented but not yet used in the e2e test. The next PR should combine the two.
some method signatures are getting quite long. We might need to refactor these after completion of the entire flow.
the constructed callback url for the holder will be implemented by the next PR.
nonceandchallengeare still a bit of a mess, but types will change in the future anyway...