Fix: Reject the sign request when cancelling the authentication method + fix cookies issue #16091
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
… for each account WalletConnect cannot operate on shared cookies in between accounts. This commit will split the cookies and cache in different folders with the user public key as folder name.
TODO: Analyse recovery options
|
@virginiabalducci This PR should also fix the shared cookies issue! |
Jenkins Builds
|
caybro
approved these changes
Aug 13, 2024
|
@caybro @Khushboo-dev-cpp This one is failing again 🙈 |
saledjenic
reviewed
Aug 13, 2024
| @@ -114,6 +114,7 @@ SQUtils.QObject { | |||
| if (session === null) | |||
| return | |||
| root.displayToastMessage(qsTr("Failed to authenticate %1 from %2").arg(methodStr).arg(session.peer.metadata.url), true) | |||
| root.sessionRequestResult(request, false /*isSuccessful*/) | |||
There was a problem hiding this comment.
@alexjba this is nice, but can we do the same if the user cancels the "Sign" request? That happens before the authentication popup is run.
There was a problem hiding this comment.
We do. We reject any sign request if the sign modal is closed in any other way except when hitting Sign
Yeah, this one tends to fail from time to time, I'll look into that. But I was never able to reproduce it locally, it's just on CI :/ |
Ive noticed too! def needs to be looked at... |
saledjenic
approved these changes
Aug 13, 2024
virginiabalducci
approved these changes
Aug 13, 2024
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.
What does the PR do
closes #16088
+Fixing the shared cookies issues on WalletConnect
Affected areas
WalletConnect
Architecture compliance
My PR is consistent with this document: Architecture guidelines
Screenshot of functionality (including design for comparison)
Impact on end user
What is the impact of these changes on the end user (before/after behaviour)
The dApp flow is completed even when the authentication fails.
Each user profile will use its own cookies for WC
How to test
Risk
Described potential risks and worst case scenarios.
Tick one: