Implement login via eth wallet #52

valiafetisov · 2023-04-19T10:44:15Z

Goal

User is able to login via eth wallet

Context

Instead of using login + password implemented initially (based on the initial requirements), we should aim to support blockchain-based authentication as the primary one, based on the new input in powerhouse-inc/powerhouse#358. Therefore, login + password authentication have to be removed and instead a new authentication methods have to be introduced. On the technical level: we would ask the user to sign an off-chain message (e.g. sessionId) using their wallet (e.g. metamask), then confirm that signed message actually signed by the address that they claim to own. The result of the procedure will be (as far as I understand the procedure):

Address of the user in the eth:0x... format (instead of the login) stored in the database
Message signed by this address, stored in the database (to be able to audit this transaction in the future)
The rest can stay the same for now (sessions, jwt tokens, etc) as it has to be discussed first

My current, uneducated understanding:

1st endpoint creates "challenge" (ie a message to be signed) stores it in the database
2nd endpoint accepts the address and the challenge result
- verifies that message is signed by the claimed address
- returns valid token

Tasks

Find the official spec that describes the authentication process using ethereum public/private keys
- Link it here
- Make short implementation summary if needed
Find and compare established libraries that implement the spec
Outline implementation proposal
Implement API methods
- Provide clear instructions on how to use it without UI inside a readme
Update UI to support it

The text was updated successfully, but these errors were encountered:

KirillDogadin-std · 2023-04-19T15:42:18Z

Find the official spec that describes the authentication process using ethereum public/private keys

hmm.. what do you mean by "official" - what is the entity?

also i've found a bunch of articles from metamask/ethereum itself on siging messages, but not on how its used in an auth process. Are those way too different from the link that's being asked for or do they suffice?

valiafetisov · 2023-04-19T15:53:36Z

what do you mean by "official" - what is the entity?

Whatever entity came up with the spec. If there is a web spec for it, then it's should be from W3C, if it's from ethereum, then one from them, etc.

Are those way too different from the link that's being asked for or do they suffice?

This implies that I know the state of the crypto-based authentication, but I am not. In the best case there is a spec on how this kind of authentication suppose to work with libraries that implement it for multiply wallets. In the worst case there is nothing and we have to figure out everything ourself from scratch starting from message signing. By the fact that I've seen websites that implement requested feature I assume we wouldn't be the first to do that and that probably there are libraries/knowledge to build upon without reinventing the wheel

KirillDogadin-std · 2023-04-20T07:31:04Z

well, here's a batch of links

official spec on the auth seems to be that, i did not deep dive in there with full understanding yet, but after scan-read-through it seems to be the thing. click (same link as before)
not the official official, but still is a nice explainer on how things should work: click
a package that handles the auth with web3: click

Feel free to react to the above already. For now i will read & understand in detail what i've linked above (e.g. w3c spec and the npm package) and propose the more detailed impl flow in the next post.

valiafetisov · 2023-04-20T07:57:32Z

Few quick points:

Looking at the web3auth it seems a bit strange, because it requires clientId from their cloud platform or can it be used standalone?
- BTW, https://github.com/Web3Auth/web3auth-web has no licence specified, which means we can't use it
The tutorial uses the web3 library, but if you're about to write some code, I would recommend to stick to the ethers.js which we're familiar with already
Found tutorial is quite bad since it doesn't use a challenge which means the same message from another place can be used to sign in to our website. It at least have to sign website's url, not the wallet own address

KirillDogadin-std · 2023-04-20T08:13:31Z

Looking at the web3auth it seems a bit strange, because it requires clientId from their cloud platform or can it be used standalone?

does not look like it can work without the client id

KirillDogadin-std · 2023-04-20T08:41:03Z

Impl proposal is as follows:

package used for verification

Backend
- signUp/In mutation called
  - check if wallet is already a user, depending on the result also perform user creation later
  - create challenge as uuid (which later will be used as sessionId)
  - send the challenge as response
  - store the challenge in the database table Challenges
  - wait for challenge completion mutation call
  - on challenge completion mutation:
    - verify the signature of the completed challenge.
    - create user with for the wallet if did not exist previously
    - create the session with the id equal to the challenge
    - delete the challenge from the db.
Frontend.
- create a component + container that interacts with metamask
  - requests challenge
  - asks to sign it
  - when signed - submits the result
  - receives the jwt and stores it in the browser.

valiafetisov · 2023-04-20T09:37:22Z

So the spec is EIP-4361 (linking it, since it wasn't mentioned above). Following the task list of the issue:

Can you please summarise the spec? Or at least link scheme or quote important parts here
Are there other specs?
Are there other libraries that implement the spec?
Can you please outline the 10% difference that wasn't followed by the suggested library? (As the linked library 90% follows the spec, according to its readme)
How hard is it to implement the spec ourselves?

signUp/In

I think there is now no user-facing difference between signIn and signUp, so should be a single mutation for that. And additionally there is a new mutation for receiving the challenge. Can you please in more details describe how they would work together, when records will be created/updated? Because create the session with the id equal to the challenge doesn't sounds completely right to me. Also, as pointed out above, instead of the challenge we can potentially use domain name of the service to make it protected against stealing

create a component + container that interacts with metamask

Ideally, there is already a component that accepts multiply wallet providers including hardware wallets. If this functionality can be separated into a different PR, you can do that, the responsibility of the first PR is to ensure they will be compatible.

KirillDogadin-std · 2023-04-20T12:47:37Z

This post is solely regarding the

10 % difference

the package version what has this in patchnotes is 0.2.0

and then there's an issue bytesbay/web3-token#12 that asks to follow the spec with the response "done" on the same date.

No pr attached, no ground provided to the 90%.

and it seems the mainainer pushes (or used to) directly to main without documenting the steps and reasoning

i found this commit on the main branch on the same date as above

bytesbay/web3-token@db73a52#diff-1cf10e2d963e76f47a9159a602cb2b8706c6c0f542fa653cbff205cb622720ea

Unless i am blind, there's also no commit message that details the changes and their reasoning. which then means, in order to figure out the 10% of the difference, i have to get proficient in the package's code and then compare my knowledge with the spec. 😨

I am deprioritising this part of investigation now

KirillDogadin-std · 2023-04-24T06:58:13Z

Can you please summarise the spec?

Here is the shorter version of the spec with the most relevant info:

message must be prefixed with \x19Ethereum Signed Message:\n<length of message>
message must have:
- address requesting the signing
- domain requesting the signing,
- version of the message
- chain identifier
- uri for scoping (referring to the resource that is the subject of the signing)
- nonce acecptable to the relying party
- issued-at timestamp
signature presented to the server, that checks the signature's validity and content
verifying signed message:
- must check the format of the message, and check against the values of the message
- must verify signature
wallet signing prompt must display the domain, address, statement, resources

Are there other specs?

I would say that there certainly are, e.g. ones listed above. But the current once seems most relevant to us.
Also it's at the github repo of ethereum which more or less means that it's the one to orient upon.

How hard is it to implement the spec ourselves?

Hard, but not that we have much choice here, do we?

Given that the spec description is fairly clear and there's already a pending investigation for what is the 10%
in the linked npm package, we probably should seriously considering building it ourselves and using the existing
one as the inspiration.

Since there can now be no trust to the existing package without documentation on how exactly it is different from the spec,
we might as well just go through the code, build similar structure + rewrite the blocks of code that could be refactored/improved
along the way of going through the existing implementation we might find out what is the 10% difference and if it is
reasonable and does not introduce security flaws, we could stick to the package. If the 10% are not clear or not safe,
then we develop the package and mb even make it the part of the sidebase??

I think there is now no user-facing difference between signIn and signUp, so should be a single mutation for that.

True, sorry if this was not clear from initial wording :D

Can you please in more details describe how they would work together, when records will be created/updated?

after thinking about it for more, it seems that we just should first sort out the approach with the existing
package. Since in case we follow the spec, the previous proposal of mine will be significantly changed.
Lmk if you still want elaboration on this part.

Ideally, there is already a component that accepts multiply wallet providers including hardware wallets. If this functionality can be separated into a different PR, you can do that, the responsibility of the first PR is to ensure they will be compatible.

👍

valiafetisov · 2023-04-24T09:22:53Z

Are there other specs?

I would say that there certainly are, e.g. ones listed above. But the current once seems most relevant to us.

Which other specs are listed above? I can only see EIP-4361 that I've linked.

My question intends to check and compare if there are better, more commonly used specs that exists/plays well with web standards, like https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API (I know it's something else, the key word is "like")

we probably should seriously considering building it ourselves and using the existing
one as the inspiration.

Before rewriting the package, I would properly search if there are existing packages that implements required functionality. Quickly googling reveals packages that wasn't linked above:

"sign in with ethereum" with 38k weekly downloads https://www.npmjs.com/package/siwe
eip4361-specific functions https://www.npmjs.com/package/eip4361-tools

Why can't we use them?

nonce acecptable to the relying party

I think you missed very important point here: used to prevent replay attacks, at least 8 alphanumeric characters – it is what I called challenge above. A one-time id which prevents another user from submitting the same signed message and receiving another valid token. It is critical point to prevent replay attacks (especially is we plan to store signed messages in our database for accountability). Technically that means that session id should be stored in the database before the user actually receives valid token. It seems that web3-token author also didn't get the purpose of nonce. Please check if other packages linked above do not autogenerate nonce (or at least do it properly)

KirillDogadin-std · 2023-04-24T11:27:13Z

Which other specs are listed above? I can only see EIP-4361 that I've linked.

this #52 (comment) contains the bunch of references to specs. I'm not sure if you're considering them as such , at least i do

I could look further obviously, but i doubt it's worth it since there's the official one. Also quick googling (< 10 minutes) did not provide me with something that looks like worth mentioning

Before rewriting the package, I would properly search if there are existing packages that implements required functionality. Quickly googling reveals packages that wasn't linked above:

"sign in with ethereum" with 38k weekly downloads https://www.npmjs.com/package/siwe
eip4361-specific functions https://www.npmjs.com/package/eip4361-tools

there's no reason to not use one of the ones you've linked. Moreover, it seems like siwe is much preferred here since it provides an api on the higher level and good examples.

Specifically relating to the backend example as here we could:

Instead of just blindly returning nonce in the corresponding endpoint:
1. have a separate db table for tracking challanges or an in-memory object (map)
2. on request for challenge: generate the session-id and store it with the wallet address in (1.1) entity for later lookup
On signature verification request:
1. lookup what is the wallet, what is the message, what is the session id
2. verify signature.

nonce

siwe does this which seems reasonable. Since it uses proper generation with alphanumeric chars under the hood.

in any case, we could just use session id as nonce and additionally check that it matches the one that we expect the wallet to have. Otherwise we afai understand would have to do double work to esnure that auto-generated nonce is actually matching the one that the message has signed.

i would suggest that i try out the PoC draft with the siwe and see how it works out.

valiafetisov · 2023-04-24T12:29:09Z

in any case, we could just use session id as nonce

Agree, challenge = session id = nonce. I would say uuid is sufficient

additionally check that it matches the one that we expect the wallet to have

Checking challenge against the wallet is not sufficient and is not needed, we need to ensure that challenge was used only once. Once again, attack vector is:

User A receives challenge the message to sign, signs it and sends it back to us
User B gets message from User A and sends it back to us again

have a separate db table for tracking challanges or an in-memory object (map)

I would instead propose to maybe reuse session table by adding something like status column. When challenge is requested we send the complete message to be signed, and already create session record with status challenge. When we receive singed message we validate, signature, domain, sessionId(aka nonce), updating status to session. The table is indeed can be different, but then we would end up with sessionId ≠ nonce

i would suggest that i try out the PoC draft with the siwe and see how it works out.

Alright, but I don't want to conclude the investigation, since almost none of the tasks listed in this issue was not followed properly yet (nor ticked)

KirillDogadin-std · 2023-04-24T13:22:02Z

I would instead propose to maybe reuse session table

i see a set changes that are consequences of this proposal that make data less... reliable/consitent (idk what is the proper workding here, so see the example):

  id                  String    @id @default(uuid())
  createdAt           DateTime  @default(now())
  createdBy           String
  referenceExpiryDate DateTime?
  name                String?
  revokedAt           DateTime?
  referenceTokenId    String
  isUserCreated       Boolean   @default(false)
  creator User @relation(fields: [createdBy], references: [id], onDelete: Cascade)
  @@unique([createdBy, id])

so, say we add the column with the status, then:

createdAt claims that the session is created at X, while in fact it would be activated/confirmed at X+k | k > 0. Which then potentially introduces problems while using this field / understanding what it means.
referenceTokenId: would have to become optional in this case and populated later.
createdBy would either have to become optional or populated right away.

optional: we would have to write code that manages this field being set and risk forgetting to set this in future operations / refactoring / ...
populated right away: we have to drag where status == in queries which list the sessions / fetch a session of a user.

new state column itself

introduces a possibility where the user would be able to spawn multiple pending sessions without ever confirming them. So we have to write operations to prevent this.

having a memory hashmap splits the state management logic. and solves all (1) - (4). The code that is to be written for this though is not the greater amount. The downside is obviously that one has to clean it up once in a while:

hashmap has a separate created at field
no need optional reference token id in the prisma db, since now session table only contains confirmed / active sessions.
created by is always present in session table and is only added when the session is confirmed. no new logic to consider while writing session crud
hashmap of {wallet: challenge} structure only stores single pending session at a time and when a user wants to create another one, previous pending (if exists) is overwritten

valiafetisov · 2023-04-24T13:38:22Z

i see a set changes that are consequences of this proposal that make data less... reliable/consitent

No problem for me if you would use another table to store challenges. But please avoid in-memory hashmaps since they are not transparent, don't work well with pod restarts, scaling, etc. I don't see a reason to store something in memory in the first place since the operation doesn't require speed optimisation.

KirillDogadin-std · 2023-04-24T13:46:36Z

Summary of the discussion to make it in accordance with issue checklist

Find the official spec that describes the authentication process using ethereum public/private keys
Link it here

EIP-4361

Make short implementation summary if needed

message must be prefixed with \x19Ethereum Signed Message:\n<length of message>
message must have:
- address requesting the signing
- domain requesting the signing,
- version of the message
- chain identifier
- uri for scoping (referring to the resource that is the subject of the signing)
- nonce acecptable to the relying party
- issued-at timestamp
signature presented to the server, that checks the signature's validity and content
verifying signed message:
- must check the format of the message, and check against the values of the message
- must verify signature
- must check nonce to not be used ever before
wallet signing prompt must display the domain, address, statement, resources

Find and compare established libraries that implement the spec

"sign in with ethereum" with 38k weekly downloads https://www.npmjs.com/package/siwe
- popular
- relatively easy to use + has examples
eip4361-specific functions https://www.npmjs.com/package/eip4361-tools
- not that popular
- more low level,
- more explicit
https://www.npmjs.com/package/web3-token
- documentation is not the best
- claims to not follow the eip-4361 fully , not clear what parts are not fully followd, hard to trace what parts are not followed and what are the risks of using this one.
- medium popularity

Outline implementation proposal

initial poc as per #52 (comment) and this but with table instead of hashmap

more details to follow

valiafetisov · 2023-06-07T08:38:05Z

KirillDogadin-std mentioned this issue May 2, 2023

Wallet auth #53

Closed

valiafetisov mentioned this issue Jun 20, 2023

Wallet auth #55

Merged

BracketJohn mentioned this issue Jun 28, 2023

Provider: Implement crypto-wallet-based authentication sidebase/nuxt-auth#467

Closed

3 tasks

valiafetisov closed this as completed in #55 Jun 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement login via eth wallet #52

Implement login via eth wallet #52

valiafetisov commented Apr 19, 2023 •

edited

Loading

KirillDogadin-std commented Apr 19, 2023

valiafetisov commented Apr 19, 2023

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

valiafetisov commented Apr 20, 2023 •

edited

Loading

KirillDogadin-std commented Apr 20, 2023

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

valiafetisov commented Apr 20, 2023

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Apr 24, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Apr 24, 2023

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Apr 24, 2023

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Jun 7, 2023 •

edited

Loading

Implement login via eth wallet #52

Implement login via eth wallet #52

Comments

valiafetisov commented Apr 19, 2023 • edited Loading

Goal

Context

Tasks

KirillDogadin-std commented Apr 19, 2023

valiafetisov commented Apr 19, 2023

KirillDogadin-std commented Apr 20, 2023 • edited Loading

valiafetisov commented Apr 20, 2023 • edited Loading

KirillDogadin-std commented Apr 20, 2023

KirillDogadin-std commented Apr 20, 2023 • edited Loading

valiafetisov commented Apr 20, 2023

KirillDogadin-std commented Apr 20, 2023 • edited Loading

KirillDogadin-std commented Apr 24, 2023 • edited Loading

valiafetisov commented Apr 24, 2023 • edited Loading

KirillDogadin-std commented Apr 24, 2023 • edited Loading

valiafetisov commented Apr 24, 2023

KirillDogadin-std commented Apr 24, 2023 • edited Loading

valiafetisov commented Apr 24, 2023

KirillDogadin-std commented Apr 24, 2023 • edited Loading

Summary of the discussion to make it in accordance with issue checklist

valiafetisov commented Jun 7, 2023 • edited Loading

valiafetisov commented Apr 19, 2023 •

edited

Loading

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

valiafetisov commented Apr 20, 2023 •

edited

Loading

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

KirillDogadin-std commented Apr 20, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Apr 24, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

KirillDogadin-std commented Apr 24, 2023 •

edited

Loading

valiafetisov commented Jun 7, 2023 •

edited

Loading