Bug: Domain only validation doesn't work for custom schemes #205
Conversation
…or-custom-schemes
…g-domain-only-validation-doesnt-work-for-custom-schemes
…chemes' of github.com:ProjectLibertyLabs/siwf into 200-bug-domain-only-validation-doesnt-work-for-custom-schemes
| } | ||
|
|
||
| // Setup now so that it is consistent for the entire test run | ||
| const now = Date.now(); | ||
|
|
||
| const loginMessageGood = (domain: string) => | ||
| const loginMessageUrl = (url: string) => |
There was a problem hiding this comment.
Allows to build test cases with different urls
| signature: { | ||
| algo: 'SR25519', | ||
| encoding: 'base16', | ||
| encodedValue: u8aToHex(ExampleUserKey.keyPair().sign(loginMessageGood(domain))), | ||
| encodedValue: u8aToHex(ExampleUserKey.keyPair().sign(loginMessageGood())), |
There was a problem hiding this comment.
domain will always match the URI in the test cases
| @@ -25,6 +25,19 @@ interface SiwxMessage { | |||
| uri: string; | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Added: helpful for hover over in IDE
libraries/js/src/response.test.ts
Outdated
| @@ -19,7 +19,9 @@ describe('getLoginResult', () => { | |||
| text: () => Promise.resolve('MOCK'), | |||
| } as any); | |||
|
|
|||
| await expect(getLoginResult('fakeAuthCode', { loginMsgDomain: 'localhost' })).to.resolves.toMatchObject(example); | |||
| await expect( | |||
| getLoginResult('fakeAuthCode', { loginMsgUri: 'testnet.frequencyaccess.com' }) | |||
There was a problem hiding this comment.
localhost changed in ExampleLogin()
94c3b34 to
06f5b0d
Compare
There was a problem hiding this comment.
Hmm, I've lost quite a bit of context since SIWFv1, but I just want to point out that domain and uri are two separate and unrelated things in the "Sign in with Ethereum" (Sign in With X) spec.
domain is the domain of the app requesting a signin (ie, mewe.com)
uri is the URI of the signing app (ie, frequencyaccess.com)
We should make sure that our use of these fields is as the standard expects; at this point I'm a little unclear if it does.
(Realizing that this may work fine for SIWFv2, but may break when we re-integrate standalone wallets and/or the Talisman SIWS package)
| @@ -107,7 +108,10 @@ Chain ID: frequency:{{chainReference}} | |||
| Issued At: {{issued-at}} | |||
| ``` | |||
|
|
|||
| Inside the message, `{{domain}}` is the domain of the application requesting the sign-in. `{{domain}}` should match the domain contained in the `URI` field. | |||
| @@ -107,7 +108,10 @@ Chain ID: frequency:{{chainReference}} | |||
| Issued At: {{issued-at}} | |||
| ``` | |||
|
|
|||
| Inside the message, `{{domain}}` is the domain of the application requesting the sign-in. `{{domain}}` should match the domain contained in the `URI` field. | |||
There was a problem hiding this comment.
Domain clarification
Problem
Closes #200