Feature/waas integration auth #41
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
…authenticate with the WaaS API and receive our JWT (used to create a WaaSWallet). SequenceWaaS assembly now references the SequenceAuthentication assembly. WaaSLogin requires the ILogin interface and the OpenIdAuthenticator class (which implements OpenID Connect to obtain the id_token, access_token, and occasionally refresh_token from the social sign in provider). WaaSLogin will also require an EthWallet (from the SequenceEthereum assembly); SequenceAuthentication does not depend on SequenceEthereum (nor should it); SequenceWaaS already depends on SequenceEthereum -> this is why WaaSLogin is part of the SequenceWaaS assembly as opposed to the SequenceAuthentication assembly. SequenceExamples assembly now depends on the SequenceWaaS assembly. This was an inevitability since SequenceExamples would've needed to create a WaaS wallet at some point (even if it relies upon the IWallet interface from SequenceEthereum throughout). Note that SequenceExamples depends on all other Sequence assemblies in the project, as you might expect.
…t_secret - each client will use their own client_id so we can identify the partner
…row exceptions in built app
….1 standard as opposed to .NET 2.0)
…CPP to build instead. Remove MacOS specific setup code for deep linking that throws errors on IL2CPP (and doesn't seem to be required). Added additional section to docs, Supported Platforms, to make a note of this.
…credentials. Moved WaaS login related classes to their own files in WaaS/DataTypes/Authentication
…Session and WaaSSessionData classes into their own file and outside the RegisterSessionResponse class definition so that they can be more readily re-used
…ntation (lacking client id still)
…aky. Fixing the flakyness is proving to be challenging and time consuming and likely not worth the effort
…ustom URL scheme on MacOS.
… Expand UI tests using mocks to test that we can navigate to the LoginSuccessPage from the LoginPage via the social sign in buttons.
…t when we log in to WaaS
BellringerQuinn
commented
Dec 18, 2023
| @@ -254,14 +254,15 @@ public async Task TestWalletSignMessage() | |||
|
|
|||
| private static IEnumerable<object[]> iWalletTestCases() | |||
| { | |||
| var adapter = WaaSToWalletAdapter.CreateAsync(new WaaSWallet( | |||
| "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJwYXJ0bmVyX2lkIjoyLCJ3YWxsZXQiOiIweDY2MDI1MDczNGYzMTY0NDY4MWFlMzJkMDViZDdlOGUyOWZlYTI5ZTEifQ.FC8WmaC_hW4svdrs4rxyKcvoekfVYFkFFvGwUOXzcHA")).Result; | |||
| // TOdo fix test | |||
There was a problem hiding this comment.
Auth no longer works this way
taylanpince
reviewed
Dec 20, 2023
| private string ExtractIssFromJwt(string jwt) | ||
| { | ||
| IdTokenJwtPayload payload = JwtHelper.GetIdTokenJwtPayload(jwt); | ||
| return payload.iss.Replace("https://", "").Replace("http://", ""); |
There was a problem hiding this comment.
Just curious why this is needed?
There was a problem hiding this comment.
Including the https:// or http:// in the iss will give error:
Error fetching credentials from AWS: Invalid login token. Issuer doesn't match providerName
If I recall correctly, the backend is matching with a string and doesn't expect those to be there
taylanpince
reviewed
Dec 20, 2023
| private static readonly string DiscordClientId = ""; // Todo replace | ||
| private static readonly string FacebookClientId = ""; // Todo replace | ||
| private static readonly string AppleClientId = ""; // Todo replace | ||
| private static readonly string RedirectUrl = "https://3d41-142-115-54-118.ngrok-free.app/"; |
There was a problem hiding this comment.
I assume these are configurable in the GUI?
There was a problem hiding this comment.
It's my understanding that everyone using our oauth system should have the same client ids, so they are hardcoded for now. Easy to expose them as I've done with the redirect url if need be.
taylanpince
reviewed
Dec 20, 2023
| private static readonly string DiscordClientId = ""; // Todo replace | ||
| private static readonly string FacebookClientId = ""; // Todo replace | ||
| private static readonly string AppleClientId = ""; // Todo replace | ||
| private static readonly string RedirectUrl = "https://3d41-142-115-54-118.ngrok-free.app/"; |
There was a problem hiding this comment.
I assume these are configurable in the GUI?
There was a problem hiding this comment.
Redirect URL, yes, in a later PR. For now, they are hardcoded
taylanpince
approved these changes
Dec 20, 2023
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.
Authenticate into WaaS using social sign in
Validated on: