-
Notifications
You must be signed in to change notification settings - Fork 8
Feature DC-API #356
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
Feature DC-API #356
Conversation
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small remarks
var error = new InvalidRequestError("The authorization request does not match the HAIP"); | ||
return new AuthorizationRequestCancellation(responseUriOption, [error]); | ||
} | ||
// if (!IsHaipConform(authRequestJObject)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supposed to be uncommented?
@@ -1,5 +1,6 @@ | |||
using LanguageExt; | |||
using OneOf; | |||
using WalletFramework.Oid4Vc.Oid4Vp.DcApi.Models; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused
ClientId = clientId.Split(':')[1]; | ||
} | ||
else | ||
if (clientId is not null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt we make sure that the clientId is only null when we are handling a DcAPI request. Because for "normal" requests we would run into issues.
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Short description of what this resolves:
Changes proposed in this pull request:
Fixes: #