Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(oob): support fetching shortened invitation urls #840

Merged
merged 50 commits into from
Jul 14, 2022
Merged

feat(oob): support fetching shortened invitation urls #840

merged 50 commits into from
Jul 14, 2022

Conversation

KolbyRKunz
Copy link
Contributor

@KolbyRKunz KolbyRKunz commented Jun 8, 2022 •
edited
Loading

Adds support for attempting to parse a URL as a shortened URL after failing to parse a URL normally, This follows the most recent changes to the out-of-band rfc with the known caveats involved.

TheTreek and others added 30 commits April 7, 2022 15:33
@TheTreek
Created messages for pickup-v2
bd89bc2 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@TheTreek
Formatting
b7be29c 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@KolbyRKunz
Added handlers for pickup-v2 messages
6d50301 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Formatting
924c30a 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@TheTreek
send delivery-request through websockets
6e9f655 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@TheTreek
Added todo to fix forcing of websockets
7f21fd3 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@TheTreek
Force return_route for pickup-v2, fix delivery handler, general cleanup
2e490cd 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@TheTreek
Added a limit on how many messages we can request from mediator
5573dcd 
Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@KolbyRKunz
Updated reciepnt modules to check for pickupV2
fc3e380 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@TheTreek
put try/catch around message delivery, send status-request on websock…
8af3669 
…et reconnect

Signed-off-by: Patrick Kenyon <treek.kenyon@gmail.com>
@KolbyRKunz
Created unit testing for MediatorRecipientService for pickupv2
c611880 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
merge conflicts
9195ec5 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Updated to PR requests
01452f6 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Made the message receiver a property of message recipient module
6cd31c2 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Moved requestStatus into a service to make it internal for now
d48dd60 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
changed initiateImplicitPickup to openWebSocketAndPickup
a51aa81 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
initial commit for testing url shortening
85d4c96 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
updated fromURL for shortened URL
5181d7d 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
added additional logic to handle shorten URL's that have been redirected
08d4106 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Improved url shortening
bb107e1 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Url shortening working, RFC discussion needed
a359176 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Merge branch 'main' into feature/shortUrl
fb8e808
@KolbyRKunz
cloned url shortening into oob
56ca498 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
comments about not being able to follow RFC
d5be405 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
updated query param for oob portion
c708a3c 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Merge branch 'main' into feature/shortUrl
f1c2ce1
@KolbyRKunz
tested against oob and added version transformers
6dcb8f7 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
formatting
6731c78 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
merge conflicts
3cc003e 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
formatting
142d278 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Json results can determine message type
6007927 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@TimoGlastra TimoGlastra mentioned this pull request Jun 17, 2022
Closed
@KolbyRKunz
tests for out of band shorten invitations written
06ed262 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
created tests for short URL
fb06a24 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
unit tests for shortenedUrls created and passing
2316bb5 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
merged main
6e63951 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
added awaits that were removed to restore failing tests
fe98b9a 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Merge branch 'main' into feature/shortUrl
2a1f496
@KolbyRKunz
Created tests for the transform function
b62bf77 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@TimoGlastra
Copy link
Contributor

I've added this to the Agenda for the WG call today, would be great if one of you @KolbyRKunz @JamesKEbert could attend!

@KolbyRKunz
added support for proper 302 redirects
6fb3672 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
created new method head to avoid breaking change and reverted awaits
9a051ec 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Copy link
Contributor Author

KolbyRKunz commented Jul 8, 2022 •
edited
Loading

After testing the trinsic shortener we found that if short url leads to a deep link fetch does not automatically follow the redirect and the workflow is at intended in the RFC, we added a statement that checks whether we followed the redirect or not and grabs the encrypted URL according to redirect status. This was a small change but also has us follow the RFC more closely.

Additionally we have noted that the trinsic shortener doe not follow the rfc in that if you supply the content-type as application/json we DO NOT get the invitation as json back from the fetch.

@TimoGlastra

TimoGlastra
TimoGlastra requested changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Left some nits on the naming / structure of the code, but the contents look good and look forward to merging this PR!

packages/core/src/modules/connections/messages/ConnectionInvitationMessage.ts Outdated Show resolved Hide resolved
packages/core/src/modules/connections/messages/ConnectionInvitationMessage.ts Outdated Show resolved Hide resolved
packages/core/src/modules/oob/__tests__/OutOfBandInvitation.test.ts Outdated Show resolved Hide resolved
packages/core/src/utils/__tests__/shortenedUrl.test.ts Outdated Show resolved Hide resolved
packages/core/src/utils/__tests__/shortenedUrl.test.ts Outdated Show resolved Hide resolved
packages/core/src/utils/parseInvitation.ts Outdated Show resolved Hide resolved
packages/core/src/utils/parseInvitation.ts Outdated Show resolved Hide resolved
packages/core/src/utils/parseInvitation.ts Outdated Show resolved Hide resolved
packages/core/src/utils/parseInvitation.ts Outdated Show resolved Hide resolved
packages/core/src/utils/parseInvitation.ts Show resolved Hide resolved
@KolbyRKunz
Url shortening requested chagnes
048073c 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
update for check types
54e6a80 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
added access to short url parsing in the out of band module, also upd…
692ebdf 
…ated receiveINvitationFromUrl to use the short url compatiable method

Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
spelling
3489e11 
Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com>
@KolbyRKunz
Copy link
Contributor Author

KolbyRKunz commented Jul 13, 2022 •
edited
Loading

@TimoGlastra The receiveInvitationFromUrl has been updated and the node version changed to greater than ^2.0. If there is not anything else I think this is ready to be merged.

@TimoGlastra TimoGlastra dismissed berendsliedrecht’s stale review July 14, 2022 08:00

comments have been resolved

@TimoGlastra TimoGlastra enabled auto-merge (squash) July 14, 2022 16:09
@TimoGlastra TimoGlastra merged commit 60ee0e5 into openwallet-foundation:main Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Iskander508 Iskander508 Iskander508 left review comments

@TimoGlastra TimoGlastra TimoGlastra approved these changes

@berendsliedrecht berendsliedrecht berendsliedrecht left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@KolbyRKunz @codecov-commenter @TimoGlastra @Iskander508 @berendsliedrecht @JamesKEbert @TheTreek