From 9614682e30770d04e1411986f05b7ff8c0d1a6de Mon Sep 17 00:00:00 2001 From: Bryan Stitt Date: Thu, 20 Oct 2022 08:14:38 +0000 Subject: [PATCH] more robust login --- FAQ.md | 12 ++++++ web3_proxy/src/app.rs | 2 +- web3_proxy/src/frontend/users.rs | 67 ++++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/FAQ.md b/FAQ.md index cea184a1..80f9fdf0 100644 --- a/FAQ.md +++ b/FAQ.md @@ -7,3 +7,15 @@ We send your transactions to multiple private relays to get them mined without e We have plans to return after the first successful response, but that won't get your transaction confirmed any faster. Soon, you can opt out of this behavior and we will broadcast your transactions publicly. + +## !) How do I sign a login message with cURL? + +``` + curl -d '{ + "address": "0x22fbd6248cb2837900c3fe69f725bc02dd3a3b33", + "msg": "0x73746167696e672e6c6c616d616e6f6465732e636f6d2077616e747320796f7520746f207369676e20696e207769746820796f757220457468657265756d206163636f756e743a0a3078323246624436323438634232383337393030633366653639663732356263303244643341334233330a0af09fa699f09fa699f09fa699f09fa699f09fa6990a0a5552493a2068747470733a2f2f73746167696e672e6c6c616d616e6f6465732e636f6d2f0a56657273696f6e3a20310a436861696e2049443a20310a4e6f6e63653a203031474654345052584342444157355844544643575957354a360a4973737565642041743a20323032322d31302d32305430373a32383a34342e3937323233343730385a0a45787069726174696f6e2054696d653a20323032322d31302d32305430373a34383a34342e3937323233343730385a", + "sig": "08478ba4646423d67b36b26d60d31b8a54c7b133a5260045b484df687c1fe8f4196dc69792019852c282fb2a1b030be130ef5b78864fff216cdd0c71929351761b", + "version": "3", + "signer": "MEW" + }' -H "Content-Type: application/json" --verbose "http://127.0.0.1:8544/user/login?invite_code=XYZ" +``` diff --git a/web3_proxy/src/app.rs b/web3_proxy/src/app.rs index af00b240..b542ca9c 100644 --- a/web3_proxy/src/app.rs +++ b/web3_proxy/src/app.rs @@ -359,7 +359,7 @@ impl Web3ProxyApp { .await .context("spawning private_rpcs")?; - if private_rpcs.conns.len() == 0 { + if private_rpcs.conns.is_empty() { None } else { // save the handle to catch any errors diff --git a/web3_proxy/src/frontend/users.rs b/web3_proxy/src/frontend/users.rs index 7237fc75..9f195a11 100644 --- a/web3_proxy/src/frontend/users.rs +++ b/web3_proxy/src/frontend/users.rs @@ -22,8 +22,10 @@ use sea_orm::{ActiveModelTrait, ColumnTrait, EntityTrait, QueryFilter, Transacti use serde::{Deserialize, Serialize}; use siwe::{Message, VerificationOpts}; use std::ops::Add; +use std::str::FromStr; use std::sync::Arc; use time::{Duration, OffsetDateTime}; +use tracing::{info, warn}; use ulid::Ulid; /// `GET /user/login/:user_address` or `GET /user/login/:user_address/:message_eip` -- Start the "Sign In with Ethereum" (siwe) login flow. @@ -134,12 +136,8 @@ pub struct PostLoginQuery { /// Email/password and other login methods are planned. #[derive(Deserialize)] pub struct PostLogin { - pub address: Address, - pub msg: String, - pub sig: Bytes, - // TODO: do we care about these? we should probably check the version is something we expect - // version: String, - // signer: String, + sig: String, + msg: String, } /// Successful logins receive a bearer_token and all of the user's api keys. @@ -169,18 +167,38 @@ pub async fn user_login_post( // we don't do per-user referral codes because we shouldn't collect what we don't need. // we don't need to build a social graph between addresses like that. if query.invite_code.as_ref() != Some(invite_code) { - todo!("if address is already registered, allow login! else, error") + warn!("if address is already registered, allow login! else, error"); + + // TODO: this return doesn't seem right + return Err(anyhow::anyhow!("checking invite_code"))?; } } // we can't trust that they didn't tamper with the message in some way - let their_msg: siwe::Message = payload.msg.parse().context("parsing user's message")?; + // TODO: it seems like some clients do things unexpectedly. these don't always parse + // let their_msg: siwe::Message = payload.msg.parse().context("parsing user's message")?; - let their_sig: [u8; 65] = payload - .sig - .as_ref() - .try_into() - .context("parsing signature")?; + // TODO: do this safely + let their_sig_bytes = Bytes::from_str(&payload.sig).context("parsing sig")?; + if their_sig_bytes.len() != 65 { + return Err(anyhow::anyhow!("checking signature length"))?; + } + let mut their_sig: [u8; 65] = [0; 65]; + for x in 0..65 { + their_sig[x] = their_sig_bytes[x] + } + + let their_msg: String = if payload.msg.starts_with("0x") { + let their_msg_bytes = Bytes::from_str(&payload.msg).context("parsing payload message")?; + + String::from_utf8_lossy(their_msg_bytes.as_ref()).to_string() + } else { + payload.msg + }; + + let their_msg = their_msg + .parse::() + .context("parsing string message")?; // TODO: this is fragile let login_nonce_key = format!("login_nonce:{}", &their_msg.nonce); @@ -193,17 +211,32 @@ pub async fn user_login_post( let our_msg: siwe::Message = our_msg.parse().context("parsing siwe message")?; + // TODO: info for now + info!(?our_msg, ?their_msg); + + // TODO: check the domain and a nonce? + // let timestamp be automatic let verify_config = VerificationOpts { // domain: Some(our_msg.domain.clone()), // nonce: Some(our_msg.nonce.clone()), ..Default::default() }; - // check the domain and a nonce. let timestamp be automatic - our_msg + // let their_verification = their_msg + // .verify(&their_sig, &verify_config) + // .await + // .context("verifying signature in their message"); + + let our_verification = our_msg .verify(&their_sig, &verify_config) .await - .context("verifying signature")?; + .context("verifying signature in our message"); + + info!(?our_verification); + + // TODO: proper error code. 5 + // their_verification?; + our_verification?; let bearer_token = Ulid::new(); @@ -223,7 +256,7 @@ pub async fn user_login_post( // the only thing we need from them is an address // everything else is optional let u = user::ActiveModel { - address: sea_orm::Set(payload.address.to_fixed_bytes().into()), + address: sea_orm::Set(our_msg.address.into()), ..Default::default() };