-
Notifications
You must be signed in to change notification settings - Fork 197
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
fix: process problem report message #1859
fix: process problem report message #1859
Conversation
Signed-off-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com>
|
||
const connection = messageContext.assertReadyConnection() | ||
|
||
agentContext.config.logger.debug(`Processing problem report with message id ${credentialProblemReportMessage.id}`) | ||
|
||
const credentialRecord = await this.getByProperties(agentContext, { | ||
threadId: credentialProblemReportMessage.threadId, | ||
connectionId: connection.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.
This is dangerous as it allows anyone to send a response to the credential exchange by knowing the thread id. For this reason we don't support problem reports currently for connectionsless exchanges.
If we want to support this, we need to handle it the same as is done in the processRequest message. Which consists of:
- query the record without connection id
- then if the record has a connection id:
- check if it matches the connection id from the exchange
- if the record does not have a connection id:
- check that the problem report is authorized based on the out of band exchange (the parentThreadId also need to match)
- if the incoming message has a connection associated, update the connectionId in the exchange record
This is already implemented like this for processRequest, and I think you can copy most of the logic
Signed-off-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com>
Signed-off-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com>
Hmm it seems there's a small issue with the logic (not from your code. I'll push some changes to this PR tomorrow to fix that as well) |
Signed-off-by: Timo Glastra <timo@animo.id>
Pushed some changes @sairanjit |
Signed-off-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com>
@TimoGlastra I removed this check const connection = messageContext.assertReadyConnection() As it was failing for connection less flows |
Summary
Fixes #1858