From 512ebe3e95bf4e533562710a7f91c59616a9a197 Mon Sep 17 00:00:00 2001 From: Daniel Krol Date: Thu, 9 Jun 2022 17:04:49 -0400 Subject: [PATCH] Protocol changes * Regress from `lastSynced` to just `sequence` to start with something simpler * Simplified payload: separate metadata, assume canonical way to hmac it together * No more "wallet state" except as a simple wrapper on the front end * Version number in wallet payloads --- auth/auth.go | 16 +- main.go | 3 +- server/auth_test.go | 11 +- server/integration_test.go | 106 +++++----- server/register.go | 2 +- server/server.go | 18 +- server/server_test.go | 16 +- server/wallet.go | 177 +++++++++++++++++ server/wallet_state.go | 181 ------------------ .../{wallet_state_test.go => wallet_test.go} | 14 +- store/store.go | 137 ++++++------- store/store_test.go | 32 ++-- test_client/README.md | 149 +++++--------- test_client/gen-readme.py | 72 ++++--- test_client/test_client.py | 133 ++++++------- wallet/wallet.go | 40 +--- wallet/wallet_test.go | 19 -- 17 files changed, 505 insertions(+), 621 deletions(-) create mode 100644 server/wallet.go delete mode 100644 server/wallet_state.go rename server/{wallet_state_test.go => wallet_test.go} (64%) delete mode 100644 wallet/wallet_test.go diff --git a/auth/auth.go b/auth/auth.go index e238bbe..c57e668 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -11,13 +11,13 @@ import ( // TODO - Learn how to use https://github.com/golang/oauth2 instead // TODO - Look into jwt, etc. // For now I just want a process that's shaped like what I'm looking for. -// (email/password, encrypted wallets, hmac, lastSynced, etc) +// (email/password, encrypted wallets, hmac, sequence (eventually lastSynced), etc) type UserId int32 type Email string type DeviceId string type Password string -type AuthTokenString string +type TokenString string type AuthScope string const ScopeFull = AuthScope("*") @@ -34,11 +34,11 @@ type Auth struct{} // downloadKey and associated email. Currently these fields are safe to give // at that low security level, but keep this in mind as we change this struct. type AuthToken struct { - Token AuthTokenString `json:"token"` - DeviceId DeviceId `json:"deviceId"` - Scope AuthScope `json:"scope"` - UserId UserId `json:"userId"` - Expiration *time.Time `json:"expiration"` + Token TokenString `json:"token"` + DeviceId DeviceId `json:"deviceId"` + Scope AuthScope `json:"scope"` + UserId UserId `json:"userId"` + Expiration *time.Time `json:"expiration"` } const AuthTokenLength = 32 @@ -51,7 +51,7 @@ func (a *Auth) NewToken(userId UserId, deviceId DeviceId, scope AuthScope) (*Aut } return &AuthToken{ - Token: AuthTokenString(hex.EncodeToString(b)), + Token: TokenString(hex.EncodeToString(b)), DeviceId: deviceId, Scope: scope, UserId: userId, diff --git a/main.go b/main.go index beeff3f..dbdadf1 100644 --- a/main.go +++ b/main.go @@ -5,7 +5,6 @@ import ( "orblivion/lbry-id/auth" "orblivion/lbry-id/server" "orblivion/lbry-id/store" - "orblivion/lbry-id/wallet" ) func storeInit() (s store.Store) { @@ -23,6 +22,6 @@ func storeInit() (s store.Store) { func main() { store := storeInit() - srv := server.Init(&auth.Auth{}, &store, &wallet.WalletUtil{}) + srv := server.Init(&auth.Auth{}, &store) srv.Serve() } diff --git a/server/auth_test.go b/server/auth_test.go index 98b5e69..b811aaa 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -8,15 +8,14 @@ import ( "net/http" "net/http/httptest" "orblivion/lbry-id/auth" - "orblivion/lbry-id/wallet" "strings" "testing" ) func TestServerAuthHandlerSuccess(t *testing.T) { - testAuth := TestAuth{TestToken: auth.AuthTokenString("seekrit")} + testAuth := TestAuth{TestToken: auth.TokenString("seekrit")} testStore := TestStore{} - s := Server{&testAuth, &testStore, &wallet.WalletUtil{}} + s := Server{&testAuth, &testStore} requestBody := []byte(`{"deviceId": "dev-1", "email": "abc@example.com", "password": "123"}`) @@ -116,7 +115,7 @@ func TestServerAuthHandlerErrors(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Set this up to fail according to specification - testAuth := TestAuth{TestToken: auth.AuthTokenString("seekrit")} + testAuth := TestAuth{TestToken: auth.TokenString("seekrit")} testStore := TestStore{} if tc.authFailLogin { testStore.FailLogin = true @@ -125,9 +124,9 @@ func TestServerAuthHandlerErrors(t *testing.T) { } else if tc.storeFailSave { testStore.FailSave = true } else { - testAuth.TestToken = auth.AuthTokenString("seekrit") + testAuth.TestToken = auth.TokenString("seekrit") } - server := Server{&testAuth, &testStore, &wallet.WalletUtil{}} + server := Server{&testAuth, &testStore} // Make request req := httptest.NewRequest(tc.method, PathAuthToken, bytes.NewBuffer([]byte(tc.requestBody))) diff --git a/server/integration_test.go b/server/integration_test.go index a4dbc7b..e29854e 100644 --- a/server/integration_test.go +++ b/server/integration_test.go @@ -10,6 +10,7 @@ import ( "orblivion/lbry-id/auth" "orblivion/lbry-id/store" "orblivion/lbry-id/wallet" + "reflect" "testing" ) @@ -30,9 +31,9 @@ func checkStatusCode(t *testing.T, statusCode int, responseBody []byte, expected var errorResponse ErrorResponse err := json.Unmarshal(responseBody, &errorResponse) if err == nil { - t.Fatalf("http response: %+v", errorResponse) + t.Errorf("http response: %+v", errorResponse) } else { - t.Fatalf("%s", err) + t.Errorf("%s", err) } } } @@ -61,11 +62,7 @@ func TestIntegrationWalletUpdates(t *testing.T) { st, tmpFile := store.StoreTestInit(t) defer store.StoreTestCleanup(tmpFile) - s := Init( - &auth.Auth{}, - &st, - &wallet.WalletUtil{}, - ) + s := Init(&auth.Auth{}, &st) //////////////////// // Register email address - any device @@ -130,116 +127,107 @@ func TestIntegrationWalletUpdates(t *testing.T) { } //////////////////// - // Put first wallet state - device 1 + // Put first wallet - device 1 //////////////////// - var walletStateResponse WalletStateResponse + var walletResponse WalletResponse responseBody, statusCode = request( t, http.MethodPost, - s.postWalletState, - PathWalletState, - &walletStateResponse, + s.postWallet, + PathWallet, + &walletResponse, fmt.Sprintf(`{ + "version": 1, "token": "%s", - "walletStateJson": "{\"encryptedWallet\": \"blah\", \"lastSynced\":{\"dev-1\": 1}, \"deviceId\": \"dev-1\" }", + "encryptedWallet": "my-encrypted-wallet-1", + "sequence": 1, "hmac": "my-hmac-1" }`, authToken1.Token), ) checkStatusCode(t, statusCode, responseBody) - var walletState wallet.WalletStateMetadata - err := json.Unmarshal([]byte(walletStateResponse.WalletStateJson), &walletState) - - if err != nil { - t.Fatalf("Unexpected error: %+v", err) + expectedResponse := WalletResponse{ + Version: 1, + EncryptedWallet: wallet.EncryptedWallet("my-encrypted-wallet-1"), + Sequence: wallet.Sequence(1), + Hmac: wallet.WalletHmac("my-hmac-1"), } - sequence := walletState.Sequence() - if sequence != 1 { - t.Fatalf("Unexpected response Sequence(). want: %+v got: %+v", 1, sequence) + if !reflect.DeepEqual(walletResponse, expectedResponse) { + t.Fatalf("Unexpected response values. want: %+v got: %+v", expectedResponse, walletResponse) } //////////////////// - // Get wallet state - device 2 + // Get wallet - device 2 //////////////////// responseBody, statusCode = request( t, http.MethodGet, - s.getWalletState, - fmt.Sprintf("%s?token=%s", PathWalletState, authToken2.Token), - &walletStateResponse, + s.getWallet, + fmt.Sprintf("%s?token=%s", PathWallet, authToken2.Token), + &walletResponse, "", ) checkStatusCode(t, statusCode, responseBody) - err = json.Unmarshal([]byte(walletStateResponse.WalletStateJson), &walletState) - - if err != nil { - t.Fatalf("Unexpected error: %+v", err) - } - - sequence = walletState.Sequence() - if sequence != 1 { - t.Fatalf("Unexpected response Sequence(). want: %+v got: %+v", 1, sequence) + // Expect the same response getting from device 2 as when posting from device 1 + if !reflect.DeepEqual(walletResponse, expectedResponse) { + t.Fatalf("Unexpected response values. want: %+v got: %+v", expectedResponse, walletResponse) } //////////////////// - // Put second wallet state - device 2 + // Put second wallet - device 2 //////////////////// responseBody, statusCode = request( t, http.MethodPost, - s.postWalletState, - PathWalletState, - &walletStateResponse, + s.postWallet, + PathWallet, + &walletResponse, fmt.Sprintf(`{ + "version": 1, "token": "%s", - "walletStateJson": "{\"encryptedWallet\": \"blah2\", \"lastSynced\":{\"dev-1\": 1, \"dev-2\": 2}, \"deviceId\": \"dev-2\" }", + "encryptedWallet": "my-encrypted-wallet-2", + "sequence": 2, "hmac": "my-hmac-2" }`, authToken2.Token), ) checkStatusCode(t, statusCode, responseBody) - err = json.Unmarshal([]byte(walletStateResponse.WalletStateJson), &walletState) - - if err != nil { - t.Fatalf("Unexpected error: %+v", err) + expectedResponse = WalletResponse{ + Version: 1, + EncryptedWallet: wallet.EncryptedWallet("my-encrypted-wallet-2"), + Sequence: wallet.Sequence(2), + Hmac: wallet.WalletHmac("my-hmac-2"), } - sequence = walletState.Sequence() - if sequence != 2 { - t.Fatalf("Unexpected response Sequence(). want: %+v got: %+v", 2, sequence) + if !reflect.DeepEqual(walletResponse, expectedResponse) { + t.Fatalf("Unexpected response values. want: %+v got: %+v", expectedResponse, walletResponse) } //////////////////// - // Get wallet state - device 1 + // Get wallet - device 1 //////////////////// responseBody, statusCode = request( t, http.MethodGet, - s.getWalletState, - fmt.Sprintf("%s?token=%s", PathWalletState, authToken1.Token), - &walletStateResponse, + s.getWallet, + fmt.Sprintf("%s?token=%s", PathWallet, authToken1.Token), + &walletResponse, "", ) checkStatusCode(t, statusCode, responseBody) - err = json.Unmarshal([]byte(walletStateResponse.WalletStateJson), &walletState) - - if err != nil { - t.Fatalf("Unexpected error: %+v", err) - } - - sequence = walletState.Sequence() - if sequence != 2 { - t.Fatalf("Unexpected response Sequence(). want: %+v got: %+v", 2, sequence) + // Expect the same response getting from device 2 as when posting from device 1 + if !reflect.DeepEqual(walletResponse, expectedResponse) { + t.Fatalf("Unexpected response values. want: %+v got: %+v", expectedResponse, walletResponse) } } diff --git a/server/register.go b/server/register.go index d3e492e..b67f60c 100644 --- a/server/register.go +++ b/server/register.go @@ -47,7 +47,7 @@ func (s *Server) register(w http.ResponseWriter, req *http.Request) { return } - // TODO StatusCreated also for first walletState and/or for get auth token? + // TODO StatusCreated also for first wallet and/or for get auth token? w.WriteHeader(http.StatusCreated) fmt.Fprintf(w, string(response)) } diff --git a/server/server.go b/server/server.go index b22406d..d466233 100644 --- a/server/server.go +++ b/server/server.go @@ -7,30 +7,26 @@ import ( "net/http" "orblivion/lbry-id/auth" "orblivion/lbry-id/store" - "orblivion/lbry-id/wallet" ) // TODO proper doc comments! const PathAuthToken = "/auth/full" const PathRegister = "/signup" -const PathWalletState = "/wallet-state" +const PathWallet = "/wallet" type Server struct { - auth auth.AuthInterface - store store.StoreInterface - walletUtil wallet.WalletUtilInterface + auth auth.AuthInterface + store store.StoreInterface } func Init( auth auth.AuthInterface, store store.StoreInterface, - walletUtil wallet.WalletUtilInterface, ) *Server { return &Server{ - auth: auth, - store: store, - walletUtil: walletUtil, + auth: auth, + store: store, } } @@ -129,7 +125,7 @@ func getGetData(w http.ResponseWriter, req *http.Request) bool { // deviceId. Also this is apparently not idiomatic go error handling. func (s *Server) checkAuth( w http.ResponseWriter, - token auth.AuthTokenString, + token auth.TokenString, scope auth.AuthScope, ) *auth.AuthToken { authToken, err := s.store.GetToken(token) @@ -155,7 +151,7 @@ func (s *Server) checkAuth( func (s *Server) Serve() { http.HandleFunc(PathAuthToken, s.getAuthToken) - http.HandleFunc(PathWalletState, s.handleWalletState) + http.HandleFunc(PathWallet, s.handleWallet) http.HandleFunc(PathRegister, s.register) fmt.Println("Serving at :8090") diff --git a/server/server_test.go b/server/server_test.go index 8fb8a79..8ff0c74 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -11,7 +11,7 @@ import ( // Implementing interfaces for stubbed out packages type TestAuth struct { - TestToken auth.AuthTokenString + TestToken auth.TokenString FailGenToken bool } @@ -37,7 +37,7 @@ func (s *TestStore) SaveToken(token *auth.AuthToken) error { return nil } -func (s *TestStore) GetToken(auth.AuthTokenString) (*auth.AuthToken, error) { +func (s *TestStore) GetToken(auth.TokenString) (*auth.AuthToken, error) { return nil, nil } @@ -52,16 +52,16 @@ func (s *TestStore) CreateAccount(auth.Email, auth.Password) error { return nil } -func (s *TestStore) SetWalletState( +func (s *TestStore) SetWallet( UserId auth.UserId, - walletStateJson string, - sequence int, - hmac wallet.WalletStateHmac, -) (latestWalletStateJson string, latestHmac wallet.WalletStateHmac, updated bool, err error) { + encryptedWallet wallet.EncryptedWallet, + sequence wallet.Sequence, + hmac wallet.WalletHmac, +) (latestEncryptedWallet wallet.EncryptedWallet, latestSequence wallet.Sequence, latestHmac wallet.WalletHmac, sequenceCorrect bool, err error) { return } -func (s *TestStore) GetWalletState(UserId auth.UserId) (walletStateJson string, hmac wallet.WalletStateHmac, err error) { +func (s *TestStore) GetWallet(userId auth.UserId) (encryptedWallet wallet.EncryptedWallet, sequence wallet.Sequence, hmac wallet.WalletHmac, err error) { return } diff --git a/server/wallet.go b/server/wallet.go new file mode 100644 index 0000000..1f299fb --- /dev/null +++ b/server/wallet.go @@ -0,0 +1,177 @@ +package server + +import ( + "encoding/json" + "fmt" + "net/http" + "orblivion/lbry-id/auth" + "orblivion/lbry-id/store" + "orblivion/lbry-id/wallet" +) + +const CURRENT_VERSION = 1 + +type WalletRequest struct { + Version int `json:"version"` + Token auth.TokenString `json:"token"` + EncryptedWallet wallet.EncryptedWallet `json:"encryptedWallet"` + Sequence wallet.Sequence `json:"sequence"` + Hmac wallet.WalletHmac `json:"hmac"` +} + +func (r *WalletRequest) validate() bool { + return (r.Version == CURRENT_VERSION && + r.Token != auth.TokenString("") && + r.EncryptedWallet != wallet.EncryptedWallet("") && + r.Hmac != wallet.WalletHmac("") && + r.Sequence >= wallet.Sequence(1)) +} + +type WalletResponse struct { + Version int `json:"version"` + EncryptedWallet wallet.EncryptedWallet `json:"encryptedWallet"` + Sequence wallet.Sequence `json:"sequence"` + Hmac wallet.WalletHmac `json:"hmac"` + Error string `json:"error"` // in case of 409 Conflict responses. TODO - make field not show up if it's empty, to avoid confusion +} + +func (s *Server) handleWallet(w http.ResponseWriter, req *http.Request) { + if req.Method == http.MethodGet { + s.getWallet(w, req) + } else if req.Method == http.MethodPost { + s.postWallet(w, req) + } else { + errorJson(w, http.StatusMethodNotAllowed, "") + } +} + +// TODO - There's probably a struct-based solution here like with POST/PUT. +// We could put that struct up top as well. +func getWalletParams(req *http.Request) (token auth.TokenString, err error) { + tokenSlice, hasTokenSlice := req.URL.Query()["token"] + + if !hasTokenSlice { + err = fmt.Errorf("Missing token parameter") + } + + if err == nil { + token = auth.TokenString(tokenSlice[0]) + } + + return +} + +func (s *Server) getWallet(w http.ResponseWriter, req *http.Request) { + if !getGetData(w, req) { + return + } + + token, paramsErr := getWalletParams(req) + + if paramsErr != nil { + // In this specific case, the error is limited to values that are safe to + // give to the user. + errorJson(w, http.StatusBadRequest, paramsErr.Error()) + return + } + + authToken := s.checkAuth(w, token, auth.ScopeFull) + + if authToken == nil { + return + } + + latestEncryptedWallet, latestSequence, latestHmac, err := s.store.GetWallet(authToken.UserId) + + if err == store.ErrNoWallet { + errorJson(w, http.StatusNotFound, "No wallet") + return + } else if err != nil { + internalServiceErrorJson(w, err, "Error retrieving wallet") + return + } + + walletResponse := WalletResponse{ + Version: CURRENT_VERSION, + EncryptedWallet: latestEncryptedWallet, + Sequence: latestSequence, + Hmac: latestHmac, + } + + var response []byte + response, err = json.Marshal(walletResponse) + + if err != nil { + internalServiceErrorJson(w, err, "Error generating wallet response") + return + } + + fmt.Fprintf(w, string(response)) +} + +func (s *Server) postWallet(w http.ResponseWriter, req *http.Request) { + var walletRequest WalletRequest + if !getPostData(w, req, &walletRequest) { + return + } + + authToken := s.checkAuth(w, walletRequest.Token, auth.ScopeFull) + if authToken == nil { + return + } + + latestEncryptedWallet, latestSequence, latestHmac, sequenceCorrect, err := s.store.SetWallet( + authToken.UserId, + walletRequest.EncryptedWallet, + walletRequest.Sequence, + walletRequest.Hmac, + ) + + var response []byte + + if err == store.ErrNoWallet { + // We failed to update, and when we tried pulling the latest wallet, + // there was nothing there. This should only happen if the client sets + // sequence != 1 for the first wallet, which would be a bug. + // TODO - figure out better error messages and/or document this + errorJson(w, http.StatusConflict, "Bad sequence number (No existing wallet)") + return + } else if err != nil { + // Something other than sequence error + internalServiceErrorJson(w, err, "Error saving wallet") + return + } + + walletResponse := WalletResponse{ + Version: CURRENT_VERSION, + EncryptedWallet: latestEncryptedWallet, + Sequence: latestSequence, + Hmac: latestHmac, + } + + if !sequenceCorrect { + // TODO - should we even call this an error? + walletResponse.Error = "Bad sequence number" + } + response, err = json.Marshal(walletResponse) + + if err != nil { + internalServiceErrorJson(w, err, "Error generating walletResponse") + return + } + + // Response Code: + // 200: Update successful + // 409: Update unsuccessful, probably due to new wallet's + // sequence not being 1 + current wallet's sequence + // + // Response Body: + // Current wallet (if it exists). If update successful, we just return + // the same one passed in. If update not successful, return the latest one + // from the db for the client to merge. + if sequenceCorrect { + fmt.Fprintf(w, string(response)) + } else { + http.Error(w, string(response), http.StatusConflict) + } +} diff --git a/server/wallet_state.go b/server/wallet_state.go deleted file mode 100644 index 8f6aa9e..0000000 --- a/server/wallet_state.go +++ /dev/null @@ -1,181 +0,0 @@ -package server - -import ( - "encoding/json" - "fmt" - "net/http" - "orblivion/lbry-id/auth" - "orblivion/lbry-id/store" - "orblivion/lbry-id/wallet" -) - -type WalletStateRequest struct { - Token auth.AuthTokenString `json:"token"` - WalletStateJson string `json:"walletStateJson"` - Hmac wallet.WalletStateHmac `json:"hmac"` -} - -func (r *WalletStateRequest) validate() bool { - return (r.Token != auth.AuthTokenString("") && - r.WalletStateJson != "" && - r.Hmac != wallet.WalletStateHmac("")) -} - -type WalletStateResponse struct { - WalletStateJson string `json:"walletStateJson"` - Hmac wallet.WalletStateHmac `json:"hmac"` - Error string `json:"error"` // in case of 409 Conflict responses. TODO - make field not show up if it's empty, to avoid confusion -} - -func (s *Server) handleWalletState(w http.ResponseWriter, req *http.Request) { - if req.Method == http.MethodGet { - s.getWalletState(w, req) - } else if req.Method == http.MethodPost { - s.postWalletState(w, req) - } else { - errorJson(w, http.StatusMethodNotAllowed, "") - } -} - -// TODO - There's probably a struct-based solution here like with POST/PUT. -// We could put that struct up top as well. -func getWalletStateParams(req *http.Request) (token auth.AuthTokenString, err error) { - tokenSlice, hasTokenSlice := req.URL.Query()["token"] - - if !hasTokenSlice { - err = fmt.Errorf("Missing token parameter") - } - - if err == nil { - token = auth.AuthTokenString(tokenSlice[0]) - } - - return -} - -func (s *Server) getWalletState(w http.ResponseWriter, req *http.Request) { - if !getGetData(w, req) { - return - } - - token, paramsErr := getWalletStateParams(req) - - if paramsErr != nil { - // In this specific case, the error is limited to values that are safe to - // give to the user. - errorJson(w, http.StatusBadRequest, paramsErr.Error()) - return - } - - authToken := s.checkAuth(w, token, auth.ScopeFull) - - if authToken == nil { - return - } - - latestWalletStateJson, latestHmac, err := s.store.GetWalletState(authToken.UserId) - - var response []byte - - if err == store.ErrNoWalletState { - errorJson(w, http.StatusNotFound, "No wallet state") - return - } else if err != nil { - internalServiceErrorJson(w, err, "Error retrieving walletState") - return - } - - walletStateResponse := WalletStateResponse{ - WalletStateJson: latestWalletStateJson, - Hmac: latestHmac, - } - response, err = json.Marshal(walletStateResponse) - - if err != nil { - internalServiceErrorJson(w, err, "Error generating latestWalletState response") - return - } - - fmt.Fprintf(w, string(response)) -} - -func (s *Server) postWalletState(w http.ResponseWriter, req *http.Request) { - var walletStateRequest WalletStateRequest - if !getPostData(w, req, &walletStateRequest) { - return - } - - var walletStateMetadata wallet.WalletStateMetadata - if err := json.Unmarshal([]byte(walletStateRequest.WalletStateJson), &walletStateMetadata); err != nil { - errorJson(w, http.StatusBadRequest, "Malformed walletStateJson") - return - } - - if s.walletUtil.ValidateWalletStateMetadata(&walletStateMetadata) { - // TODO - } - - authToken := s.checkAuth(w, walletStateRequest.Token, auth.ScopeFull) - if authToken == nil { - return - } - - // TODO - We could do an extra check - pull from db, make sure the new - // walletStateMetadata doesn't regress lastSynced for any given device. - // This is primarily the responsibility of the clients, but we may want to - // trade a db call here for a double-check against bugs in the client. - // We do already do some validation checks here, but those doesn't require - // new database calls. - - latestWalletStateJson, latestHmac, updated, err := s.store.SetWalletState( - authToken.UserId, - walletStateRequest.WalletStateJson, - walletStateMetadata.Sequence(), - walletStateRequest.Hmac, - ) - - var response []byte - - if err == store.ErrNoWalletState { - // We failed to update, and when we tried pulling the latest wallet state, - // there was nothing there. This should only happen if the client sets - // sequence != 1 for the first walletState, which would be a bug. - // TODO - figure out better error messages and/or document this - errorJson(w, http.StatusConflict, "Bad sequence number (No existing wallet state)") - return - } else if err != nil { - // Something other than sequence error - internalServiceErrorJson(w, err, "Error saving walletState") - return - } - - walletStateResponse := WalletStateResponse{ - WalletStateJson: latestWalletStateJson, - Hmac: latestHmac, - } - if !updated { - // TODO - should we even call this an error? - walletStateResponse.Error = "Bad sequence number" - } - response, err = json.Marshal(walletStateResponse) - - if err != nil { - internalServiceErrorJson(w, err, "Error generating walletStateResponse") - return - } - - // Response Code: - // 200: Update successful - // 409: Update unsuccessful, probably due to new walletState's - // sequence not being 1 + current walletState's sequence - // - // Response Body: - // Current walletState (if it exists). If update successful, we just return - // the same one passed in. If update not successful, return the latest one - // from the db for the client to merge. - if updated { - fmt.Fprintf(w, string(response)) - } else { - http.Error(w, string(response), http.StatusConflict) - } -} diff --git a/server/wallet_state_test.go b/server/wallet_test.go similarity index 64% rename from server/wallet_state_test.go rename to server/wallet_test.go index 9adf2a2..84ab44b 100644 --- a/server/wallet_state_test.go +++ b/server/wallet_test.go @@ -12,8 +12,8 @@ func TestServerGetWalletErrors(t *testing.T) { t.Fatalf("Test me: GetWallet fails for various reasons (malformed, auth, db fail)") } -func TestServerGetWalletStateParams(t *testing.T) { - t.Fatalf("Test me: getWalletStateParams") +func TestServerGetWalletParams(t *testing.T) { + t.Fatalf("Test me: getWalletParams") } func TestServerPostWalletSuccess(t *testing.T) { @@ -25,17 +25,17 @@ func TestServerPostWalletTooLate(t *testing.T) { } func TestServerPostWalletErrors(t *testing.T) { - // (malformed json, db fail, auth token not found, walletstate invalid (via stub, make sure the validation function is even called), sequence too high, device id doesn't match token device id) + // (malformed json, db fail, auth token not found, wallet metadata invalid (via stub, make sure the validation function is even called), sequence too high, device id doesn't match token device id) // Client sends sequence != 1 for first entry // Client sends sequence == x + 10 for xth entry or whatever t.Fatalf("Test me: PostWallet fails for various reasons") } -func TestServerValidateWalletStateRequest(t *testing.T) { +func TestServerValidateWalletRequest(t *testing.T) { // also add a basic test case for this in TestServerAuthHandlerSuccess to make sure it's called at all - t.Fatalf("Test me: Implement and test WalletStateRequest.validate()") + t.Fatalf("Test me: Implement and test WalletRequest.validate()") } -func TestServerHandleWalletState(t *testing.T) { - t.Fatalf("Test me: Call the get or post function as appropriate. Alternately: call handleWalletState for the existing tests.") +func TestServerHandleWallet(t *testing.T) { + t.Fatalf("Test me: Call the get or post function as appropriate. Alternately: call handleWallet for the existing tests.") } diff --git a/store/store.go b/store/store.go index 44e7880..c18fd40 100644 --- a/store/store.go +++ b/store/store.go @@ -18,8 +18,8 @@ var ( ErrDuplicateToken = fmt.Errorf("Token already exists for this user and device") ErrNoToken = fmt.Errorf("Token does not exist for this user and device") - ErrDuplicateWalletState = fmt.Errorf("WalletState already exists for this user") - ErrNoWalletState = fmt.Errorf("WalletState does not exist for this user at this sequence") + ErrDuplicateWallet = fmt.Errorf("Wallet already exists for this user") + ErrNoWallet = fmt.Errorf("Wallet does not exist for this user at this sequence") ErrDuplicateEmail = fmt.Errorf("Email already exists for this user") ErrDuplicateAccount = fmt.Errorf("User already has an account") @@ -30,9 +30,9 @@ var ( // For test stubs type StoreInterface interface { SaveToken(*auth.AuthToken) error - GetToken(auth.AuthTokenString) (*auth.AuthToken, error) - SetWalletState(auth.UserId, string, int, wallet.WalletStateHmac) (string, wallet.WalletStateHmac, bool, error) - GetWalletState(auth.UserId) (string, wallet.WalletStateHmac, error) + GetToken(auth.TokenString) (*auth.AuthToken, error) + SetWallet(auth.UserId, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) (wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac, bool, error) + GetWallet(auth.UserId) (wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac, error) GetUserId(auth.Email, auth.Password) (auth.UserId, error) CreateAccount(auth.Email, auth.Password) (err error) } @@ -50,12 +50,10 @@ func (s *Store) Init(fileName string) { } func (s *Store) Migrate() error { - // We store `sequence` as a seprate field in the `wallet_state` table, even - // though it's also saved as part of the `walle_state_blob` column. We do - // this for transaction safety. For instance, let's say two different clients - // are trying to update the sequence from 5 to 6. The update command will - // specify "WHERE sequence=5". Only one of these commands will succeed, and - // the other will get back an error. + // We use the `sequence` field for transaction safety. For instance, let's + // say two different clients are trying to update the sequence from 5 to 6. + // The update command will specify "WHERE sequence=5". Only one of these + // commands will succeed, and the other will get back an error. // We use AUTOINCREMENT against the protestations of people on the Internet // who claim that INTEGER PRIMARY KEY automatically has autoincrment, and @@ -80,9 +78,9 @@ func (s *Store) Migrate() error { expiration DATETIME NOT NULL, PRIMARY KEY (user_id, device_id) ); - CREATE TABLE IF NOT EXISTS wallet_states( + CREATE TABLE IF NOT EXISTS wallets( user_id INTEGER NOT NULL, - wallet_state_blob TEXT NOT NULL, + encrypted_wallet TEXT NOT NULL, sequence INTEGER NOT NULL, hmac TEXT NOT NULL, PRIMARY KEY (user_id) @@ -108,7 +106,7 @@ func (s *Store) Migrate() error { // (which I did previously)? // // TODO Put the timestamp in the token to avoid duplicates over time. And/or just use a library! Someone solved this already. -func (s *Store) GetToken(token auth.AuthTokenString) (*auth.AuthToken, error) { +func (s *Store) GetToken(token auth.TokenString) (*auth.AuthToken, error) { expirationCutoff := time.Now().UTC() rows, err := s.db.Query( @@ -204,13 +202,13 @@ func (s *Store) SaveToken(token *auth.AuthToken) (err error) { return } -////////////////// -// Wallet State // -////////////////// +//////////// +// Wallet // +//////////// -func (s *Store) GetWalletState(userId auth.UserId) (walletStateJson string, hmac wallet.WalletStateHmac, err error) { +func (s *Store) GetWallet(userId auth.UserId) (encryptedWallet wallet.EncryptedWallet, sequence wallet.Sequence, hmac wallet.WalletHmac, err error) { rows, err := s.db.Query( - "SELECT wallet_state_blob, hmac FROM wallet_states WHERE user_id=?", + "SELECT encrypted_wallet, sequence, hmac FROM wallets WHERE user_id=?", userId, ) if err != nil { @@ -220,26 +218,27 @@ func (s *Store) GetWalletState(userId auth.UserId) (walletStateJson string, hmac for rows.Next() { err = rows.Scan( - &walletStateJson, + &encryptedWallet, + &sequence, &hmac, ) return } - err = ErrNoWalletState + err = ErrNoWallet return } -func (s *Store) insertFirstWalletState( +func (s *Store) insertFirstWallet( userId auth.UserId, - walletStateJson string, - hmac wallet.WalletStateHmac, + encryptedWallet wallet.EncryptedWallet, + hmac wallet.WalletHmac, ) (err error) { - // This will only be used to attempt to insert the first wallet state - // (sequence=1). The database will enforce that this will not be set - // if this user already has a walletState. + // This will only be used to attempt to insert the first wallet (sequence=1). + // The database will enforce that this will not be set if this user already + // has a wallet. _, err = s.db.Exec( - "INSERT INTO wallet_states (user_id, wallet_state_blob, sequence, hmac) values(?,?,?,?)", - userId, walletStateJson, 1, hmac, + "INSERT INTO wallets (user_id, encrypted_wallet, sequence, hmac) values(?,?,?,?)", + userId, encryptedWallet, 1, hmac, ) var sqliteErr sqlite3.Error @@ -247,26 +246,26 @@ func (s *Store) insertFirstWalletState( // I initially expected to need to check for ErrConstraintUnique. // Maybe for psql it will be? if errors.Is(sqliteErr.ExtendedCode, sqlite3.ErrConstraintPrimaryKey) { - err = ErrDuplicateWalletState + err = ErrDuplicateWallet } } return } -func (s *Store) updateWalletStateToSequence( +func (s *Store) updateWalletToSequence( userId auth.UserId, - walletStateJson string, - sequence int, - hmac wallet.WalletStateHmac, + encryptedWallet wallet.EncryptedWallet, + sequence wallet.Sequence, + hmac wallet.WalletHmac, ) (err error) { - // This will be used for wallet states with sequence > 1. + // This will be used for wallets with sequence > 1. // Use the database to enforce that we only update if we are incrementing the sequence. // This way, if two clients attempt to update at the same time, it will return - // ErrNoWalletState for the second one. + // ErrNoWallet for the second one. res, err := s.db.Exec( - "UPDATE wallet_states SET wallet_state_blob=?, sequence=?, hmac=? WHERE user_id=? AND sequence=?", - walletStateJson, sequence, hmac, userId, sequence-1, + "UPDATE wallets SET encrypted_wallet=?, sequence=?, hmac=? WHERE user_id=? AND sequence=?", + encryptedWallet, sequence, hmac, userId, sequence-1, ) if err != nil { return @@ -277,49 +276,59 @@ func (s *Store) updateWalletStateToSequence( return } if numRows == 0 { - err = ErrNoWalletState + err = ErrNoWallet } return } -// Assumption: walletState has been validated (sequence >=1, etc) -// Assumption: Sequence matches walletState.Sequence() -// Sequence is only passed in here to avoid deserializing walletStateJson again -// WalletState *struct* is not passed in because the clients need the exact string to match the hmac -func (s *Store) SetWalletState( +// Assumption: Sequence has been validated (>=1) +func (s *Store) SetWallet( userId auth.UserId, - walletStateJson string, - sequence int, - hmac wallet.WalletStateHmac, -) (latestWalletStateJson string, latestHmac wallet.WalletStateHmac, updated bool, err error) { + encryptedWallet wallet.EncryptedWallet, + sequence wallet.Sequence, + hmac wallet.WalletHmac, + // TODO `sequenceCorrect` should probably be replaced with `status`, that can + // equal `Updated` or `SequenceMismatch`. Maybe with a message for the API. + // Like an error, but not, because the function still returns a value. + // Right now, we have: + // `sequenceCorrect==true` and `err==nil` implies it updated. + // We could also have: + // `sequenceMismatch==true` or `err!=nil` implying it didn't update. + // Or: + // `updated==false` and `err=nil` implying the sequence mismatched. + // I don't like this implication stuff, the "status" should be explicit so + // we don't make bugs. +) (latestEncryptedWallet wallet.EncryptedWallet, latestSequence wallet.Sequence, latestHmac wallet.WalletHmac, sequenceCorrect bool, err error) { if sequence == 1 { // If sequence == 1, the client assumed that this is our first - // walletState. Try to insert. If we get a conflict, the client + // wallet. Try to insert. If we get a conflict, the client // assumed incorrectly and we proceed below to return the latest - // walletState from the db. - err = s.insertFirstWalletState(userId, walletStateJson, hmac) + // wallet from the db. + err = s.insertFirstWallet(userId, encryptedWallet, hmac) if err == nil { // Successful update - latestWalletStateJson = walletStateJson + latestEncryptedWallet = encryptedWallet + latestSequence = sequence latestHmac = hmac - updated = true + sequenceCorrect = true return - } else if err != ErrDuplicateWalletState { + } else if err != ErrDuplicateWallet { // Unsuccessful update for reasons other than sequence conflict return } } else { - // If sequence > 1, the client assumed that it is replacing walletState - // with sequence - 1. Explicitly try to update the walletState with + // If sequence > 1, the client assumed that it is replacing wallet + // with sequence - 1. Explicitly try to update the wallet with // sequence - 1. If we updated no rows, the client assumed incorrectly - // and we proceed below to return the latest walletState from the db. - err = s.updateWalletStateToSequence(userId, walletStateJson, sequence, hmac) + // and we proceed below to return the latest wallet from the db. + err = s.updateWalletToSequence(userId, encryptedWallet, sequence, hmac) if err == nil { - latestWalletStateJson = walletStateJson + latestEncryptedWallet = encryptedWallet + latestSequence = sequence latestHmac = hmac - updated = true + sequenceCorrect = true return - } else if err != ErrNoWalletState { + } else if err != ErrNoWallet { return } } @@ -329,9 +338,9 @@ func (s *Store) SetWalletState( // version right away so the requesting client can take care of it. // // Note that this means that `err` will not be `nil` at this point, but we - // already accounted for it with `updated=false`. Instead, we'll pass on any - // errors from calling `GetWalletState`. - latestWalletStateJson, latestHmac, err = s.GetWalletState(userId) + // already accounted for it with `sequenceCorrect=false`. Instead, we'll pass + // on any errors from calling `GetWallet`. + latestEncryptedWallet, latestSequence, latestHmac, err = s.GetWallet(userId) return } diff --git a/store/store_test.go b/store/store_test.go index 210d06e..372c4f7 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -305,16 +305,16 @@ func TestStoreTimeZones(t *testing.T) { t.Fatalf("Test me") } -func TestStoreSetWalletStateSuccess(t *testing.T) { +func TestStoreSetWalletSuccess(t *testing.T) { /* Sequence 1 - works via insert Sequence 2 - works via update Sequence 3 - works via update */ - t.Fatalf("Test me: WalletState Set successes") + t.Fatalf("Test me: Wallet Set successes") } -func TestStoreSetWalletStateFail(t *testing.T) { +func TestStoreSetWalletFail(t *testing.T) { /* Sequence 1 - fails via insert - fail by having something there already Sequence 2 - fails via update - fail by not having something there already @@ -323,31 +323,31 @@ func TestStoreSetWalletStateFail(t *testing.T) { Maybe some of the above gets put off to wallet util */ - t.Fatalf("Test me: WalletState Set failures") + t.Fatalf("Test me: Wallet Set failures") } -func TestStoreInsertWalletStateSuccess(t *testing.T) { - t.Fatalf("Test me: WalletState insert successes") +func TestStoreInsertWalletSuccess(t *testing.T) { + t.Fatalf("Test me: Wallet insert successes") } -func TestStoreInsertWalletStateFail(t *testing.T) { - t.Fatalf("Test me: WalletState insert failures") +func TestStoreInsertWalletFail(t *testing.T) { + t.Fatalf("Test me: Wallet insert failures") } -func TestStoreUpdateWalletStateSuccess(t *testing.T) { - t.Fatalf("Test me: WalletState update successes") +func TestStoreUpdateWalletSuccess(t *testing.T) { + t.Fatalf("Test me: Wallet update successes") } -func TestStoreUpdateWalletStateFail(t *testing.T) { - t.Fatalf("Test me: WalletState update failures") +func TestStoreUpdateWalletFail(t *testing.T) { + t.Fatalf("Test me: Wallet update failures") } -func TestStoreGetWalletStateSuccess(t *testing.T) { - t.Fatalf("Test me: WalletState get success") +func TestStoreGetWalletSuccess(t *testing.T) { + t.Fatalf("Test me: Wallet get success") } -func TestStoreGetWalletStateFail(t *testing.T) { - t.Fatalf("Test me: WalletState get failures") +func TestStoreGetWalletFail(t *testing.T) { + t.Fatalf("Test me: Wallet get failures") } func TestStoreSetEmailSuccess(t *testing.T) { diff --git a/test_client/README.md b/test_client/README.md index 903c692..eed1e62 100644 --- a/test_client/README.md +++ b/test_client/README.md @@ -14,15 +14,6 @@ Set up two clients with the same account (which won't exist on the server yet). >>> c2.set_account("joe2@example.com", "123abc2") ``` -Each device will have a device_id which will be used in the wallet state metadata to mark which device created a given version. This is used in the `lastSynced` field (see below). - -``` ->>> c1.device_id -'974690df-85a6-481d-9015-6293226db8c9' ->>> c2.device_id -'545643c9-ee47-443d-b260-cb9178b8646c' -``` - Register the account on the server with one of the clients. ``` @@ -34,59 +25,45 @@ Now that the account exists, grab an auth token with both clients. ``` >>> c1.get_auth_token() -Got auth token: 941e5159a2caff15f0bdc1c0e6da92691d3073543dbfae810cfe57d51c35f0e0 +Got auth token: a489d5cacc0a3db4811c34d203683482d90c605b03ae007fa5ae32ef17252bd9 >>> c2.get_auth_token() -Got auth token: b323a18e51263ac052777ca68de716c1f3b4983bf4c918477e355f637c8ea2d4 +Got auth token: 1fe687db8ab493ed260f499b674cfa49edefd3c03a718905c62d3f850dc50567 ``` ## Syncing -Create a new wallet state (wallet + metadata) and post it to the server. Note that after posting, it says it "got" a new wallet state. This is because the post endpoint also returns the latest version. The purpose of this will be explained in "Conflicts" below. +Create a new wallet + metadata (we'll wrap it in a struct we'll call `WalletState` in this client) and POST them to the server. The metadata (as of now) in the walletstate is only `sequence`. This increments for every POSTed wallet. This is bookkeeping to prevent certain syncing errors. -The fields in the walletstate are: - -* `encryptedWallet` - the actual encrypted wallet data -* `lastSynced` - a mapping between deviceId and the latest sequence number that it _created_. This is bookkeeping to prevent certain syncing errors. -* `deviceId` - the device that made _this_ wallet state version (NOTE this admittedly seems redundant with `lastSynced` and may be removed) +Note that after POSTing, it says it "got" a new wallet. This is because the POST endpoint also returns the latest version. The purpose of this will be explained in "Conflicts" below. ``` >>> c1.new_wallet_state() ->>> c1.post_wallet_state() +>>> c1.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': '', - 'lastSynced': {'974690df-85a6-481d-9015-6293226db8c9': 1}} +WalletState(sequence=1, encrypted_wallet='-') ``` -With the other client, get it from the server. Note that both clients have the same data now. +With the other client, GET it from the server. Note that both clients have the same data now. ``` ->>> c2.get_wallet_state() +>>> c2.get_wallet() Got latest walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': '', - 'lastSynced': {'974690df-85a6-481d-9015-6293226db8c9': 1}} +WalletState(sequence=1, encrypted_wallet='-') ``` ## Updating -Push a new version, get it with the other client. Even though we haven't edited the encrypted wallet yet, each version of a wallet _state_ has an incremented sequence number, and the deviceId that created it. +Push a new version, GET it with the other client. Even though we haven't edited the encrypted wallet yet, we can still increment the sequence number. ``` ->>> c2.post_wallet_state() +>>> c2.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': '', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 1}} ->>> c1.get_wallet_state() +WalletState(sequence=2, encrypted_wallet='-') +>>> c1.get_wallet() Got latest walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': '', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 1}} +WalletState(sequence=2, encrypted_wallet='-') ``` ## Wallet Changes @@ -95,30 +72,24 @@ For demo purposes, this test client represents each change to the wallet by appe ``` >>> c1.cur_encrypted_wallet() -'' +'-' >>> c1.change_encrypted_wallet() >>> c1.cur_encrypted_wallet() -':2fbE' +'-:cfF6' ``` The wallet is synced between the clients. ``` ->>> c1.post_wallet_state() +>>> c1.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': ':2fbE', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 3}} ->>> c2.get_wallet_state() +WalletState(sequence=3, encrypted_wallet='-:cfF6') +>>> c2.get_wallet() Got latest walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': ':2fbE', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 3}} +WalletState(sequence=3, encrypted_wallet='-:cfF6') >>> c2.cur_encrypted_wallet() -':2fbE' +'-:cfF6' ``` ## Merging Changes @@ -129,86 +100,70 @@ Both clients create changes. They now have diverging wallets. >>> c1.change_encrypted_wallet() >>> c2.change_encrypted_wallet() >>> c1.cur_encrypted_wallet() -':2fbE:BD62' +'-:cfF6:565b' >>> c2.cur_encrypted_wallet() -':2fbE:e7ac' +'-:cfF6:6De1' ``` -One client posts its change first. The other client pulls that change, and _merges_ those changes on top of the changes it had saved locally. - -The _merge base_ that a given client uses is the last version that it successfully got from or posted to the server. You can see the merge base here: the first part of the wallet which does not change from this merge. +One client POSTs its change first. ``` ->>> c1.post_wallet_state() +>>> c1.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': ':2fbE:BD62', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 4}} ->>> c2.get_wallet_state() +WalletState(sequence=4, encrypted_wallet='-:cfF6:565b') +``` + +The other client pulls that change, and _merges_ those changes on top of the changes it had saved locally. + +The _merge base_ that a given client uses is the last version that it successfully got from or POSTed to the server. You can see the merge base here: `"-:cfF6"`, the first part of the wallet which both clients had in common before the merge. + +``` +>>> c2.get_wallet() Got latest walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': ':2fbE:BD62', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 2, - '974690df-85a6-481d-9015-6293226db8c9': 4}} +WalletState(sequence=4, encrypted_wallet='-:cfF6:565b') >>> c2.cur_encrypted_wallet() -':2fbE:BD62:e7ac' +'-:cfF6:565b:6De1' ``` -Finally, the client with the merged wallet pushes it to the server, and the other client gets the update. +Finally, the client with the merged wallet pushes it to the server, and the other client GETs the update. ``` ->>> c2.post_wallet_state() +>>> c2.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': ':2fbE:BD62:e7ac', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 5, - '974690df-85a6-481d-9015-6293226db8c9': 4}} ->>> c1.get_wallet_state() +WalletState(sequence=5, encrypted_wallet='-:cfF6:565b:6De1') +>>> c1.get_wallet() Got latest walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': ':2fbE:BD62:e7ac', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 5, - '974690df-85a6-481d-9015-6293226db8c9': 4}} +WalletState(sequence=5, encrypted_wallet='-:cfF6:565b:6De1') >>> c1.cur_encrypted_wallet() -':2fbE:BD62:e7ac' +'-:cfF6:565b:6De1' ``` ## Conflicts -A client cannot post if it is not up to date. It needs to merge in any new changes on the server before posting its own changes. For convenience, if a conflicting post request is made, the server responds with the latest version of the wallet state (just like a GET request). This way the client doesn't need to make a second request to perform the merge. +A client cannot POST if it is not up to date. It needs to merge in any new changes on the server before POSTing its own changes. For convenience, if a conflicting POST request is made, the server responds with the latest version of the wallet state (just like a GET request). This way the client doesn't need to make a second request to perform the merge. -(If a non-conflicting post request is made, it responds with the same wallet state that the client just posted, as it is now the server's current wallet state) +(If a non-conflicting POST request is made, it responds with the same wallet state that the client just POSTed, as it is now the server's current wallet state) ``` >>> c2.change_encrypted_wallet() ->>> c2.post_wallet_state() +>>> c2.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': ':2fbE:BD62:e7ac:4EEf', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 6, - '974690df-85a6-481d-9015-6293226db8c9': 4}} +WalletState(sequence=6, encrypted_wallet='-:cfF6:565b:6De1:053a') >>> c1.change_encrypted_wallet() ->>> c1.post_wallet_state() -Wallet state out of date. Getting updated wallet state. Try again. +>>> c1.post_wallet() +Wallet state out of date. Getting updated wallet state. Try posting again after this. Got new walletState: -{'deviceId': '545643c9-ee47-443d-b260-cb9178b8646c', - 'encryptedWallet': ':2fbE:BD62:e7ac:4EEf', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 6, - '974690df-85a6-481d-9015-6293226db8c9': 4}} +WalletState(sequence=6, encrypted_wallet='-:cfF6:565b:6De1:053a') ``` -Now the merge is complete, and the client can make a second post request containing the merged wallet. +Now the merge is complete, and the client can make a second POST request containing the merged wallet. ``` ->>> c1.post_wallet_state() +>>> c1.post_wallet() Successfully updated wallet state on server Got new walletState: -{'deviceId': '974690df-85a6-481d-9015-6293226db8c9', - 'encryptedWallet': ':2fbE:BD62:e7ac:4EEf:DC86', - 'lastSynced': {'545643c9-ee47-443d-b260-cb9178b8646c': 6, - '974690df-85a6-481d-9015-6293226db8c9': 7}} +WalletState(sequence=7, encrypted_wallet='-:cfF6:565b:6De1:053a:6774') ``` diff --git a/test_client/gen-readme.py b/test_client/gen-readme.py index 7c1c40a..9bceddd 100644 --- a/test_client/gen-readme.py +++ b/test_client/gen-readme.py @@ -32,15 +32,6 @@ c1.set_account("joe2@example.com", "123abc2") c2.set_account("joe2@example.com", "123abc2") """) -print(""" -Each device will have a device_id which will be used in the wallet state metadata to mark which device created a given version. This is used in the `lastSynced` field (see below). -""") - -code_block(""" -c1.device_id -c2.device_id -""") - print(""" Register the account on the server with one of the clients. """) @@ -58,42 +49,36 @@ c1.get_auth_token() c2.get_auth_token() """) -# TODO - wait isn't it redundant to have the `deviceId` field, for the same reason it's redundant to have the `sequence` field? - print(""" ## Syncing -Create a new wallet state (wallet + metadata) and post it to the server. Note that after posting, it says it "got" a new wallet state. This is because the post endpoint also returns the latest version. The purpose of this will be explained in "Conflicts" below. +Create a new wallet + metadata (we'll wrap it in a struct we'll call `WalletState` in this client) and POST them to the server. The metadata (as of now) in the walletstate is only `sequence`. This increments for every POSTed wallet. This is bookkeeping to prevent certain syncing errors. -The fields in the walletstate are: - -* `encryptedWallet` - the actual encrypted wallet data -* `lastSynced` - a mapping between deviceId and the latest sequence number that it _created_. This is bookkeeping to prevent certain syncing errors. -* `deviceId` - the device that made _this_ wallet state version (NOTE this admittedly seems redundant with `lastSynced` and may be removed) +Note that after POSTing, it says it "got" a new wallet. This is because the POST endpoint also returns the latest version. The purpose of this will be explained in "Conflicts" below. """) code_block(""" c1.new_wallet_state() -c1.post_wallet_state() +c1.post_wallet() """) print(""" -With the other client, get it from the server. Note that both clients have the same data now. +With the other client, GET it from the server. Note that both clients have the same data now. """) code_block(""" -c2.get_wallet_state() +c2.get_wallet() """) print(""" ## Updating -Push a new version, get it with the other client. Even though we haven't edited the encrypted wallet yet, each version of a wallet _state_ has an incremented sequence number, and the deviceId that created it. +Push a new version, GET it with the other client. Even though we haven't edited the encrypted wallet yet, we can still increment the sequence number. """) code_block(""" -c2.post_wallet_state() -c1.get_wallet_state() +c2.post_wallet() +c1.get_wallet() """) print(""" @@ -113,8 +98,8 @@ The wallet is synced between the clients. """) code_block(""" -c1.post_wallet_state() -c2.get_wallet_state() +c1.post_wallet() +c2.get_wallet() c2.cur_encrypted_wallet() """) @@ -124,6 +109,8 @@ print(""" Both clients create changes. They now have diverging wallets. """) +merge_base = c2.cur_encrypted_wallet() + code_block(""" c1.change_encrypted_wallet() c2.change_encrypted_wallet() @@ -132,46 +119,53 @@ c2.cur_encrypted_wallet() """) print(""" -One client posts its change first. The other client pulls that change, and _merges_ those changes on top of the changes it had saved locally. - -The _merge base_ that a given client uses is the last version that it successfully got from or posted to the server. You can see the merge base here: the first part of the wallet which does not change from this merge. +One client POSTs its change first. """) code_block(""" -c1.post_wallet_state() -c2.get_wallet_state() +c1.post_wallet() +""") + +print(""" +The other client pulls that change, and _merges_ those changes on top of the changes it had saved locally. + +The _merge base_ that a given client uses is the last version that it successfully got from or POSTed to the server. You can see the merge base here: `"%s"`, the first part of the wallet which both clients had in common before the merge. +""" % merge_base) + +code_block(""" +c2.get_wallet() c2.cur_encrypted_wallet() """) print(""" -Finally, the client with the merged wallet pushes it to the server, and the other client gets the update. +Finally, the client with the merged wallet pushes it to the server, and the other client GETs the update. """) code_block(""" -c2.post_wallet_state() -c1.get_wallet_state() +c2.post_wallet() +c1.get_wallet() c1.cur_encrypted_wallet() """) print(""" ## Conflicts -A client cannot post if it is not up to date. It needs to merge in any new changes on the server before posting its own changes. For convenience, if a conflicting post request is made, the server responds with the latest version of the wallet state (just like a GET request). This way the client doesn't need to make a second request to perform the merge. +A client cannot POST if it is not up to date. It needs to merge in any new changes on the server before POSTing its own changes. For convenience, if a conflicting POST request is made, the server responds with the latest version of the wallet state (just like a GET request). This way the client doesn't need to make a second request to perform the merge. -(If a non-conflicting post request is made, it responds with the same wallet state that the client just posted, as it is now the server's current wallet state) +(If a non-conflicting POST request is made, it responds with the same wallet state that the client just POSTed, as it is now the server's current wallet state) """) code_block(""" c2.change_encrypted_wallet() -c2.post_wallet_state() +c2.post_wallet() c1.change_encrypted_wallet() -c1.post_wallet_state() +c1.post_wallet() """) print(""" -Now the merge is complete, and the client can make a second post request containing the merged wallet. +Now the merge is complete, and the client can make a second POST request containing the merged wallet. """) code_block(""" -c1.post_wallet_state() +c1.post_wallet() """) diff --git a/test_client/test_client.py b/test_client/test_client.py index 5f74e66..0b87302 100755 --- a/test_client/test_client.py +++ b/test_client/test_client.py @@ -1,32 +1,41 @@ #!/bin/python3 +from collections import namedtuple import random, string, json, uuid, requests, hashlib from pprint import pprint +CURRENT_VERSION = 1 + BASE_URL = 'http://localhost:8090' AUTH_URL = BASE_URL + '/auth/full' REGISTER_URL = BASE_URL + '/signup' -WALLET_STATE_URL = BASE_URL + '/wallet-state' +WALLET_URL = BASE_URL + '/wallet' -def wallet_state_sequence(wallet_state): - if 'deviceId' not in wallet_state: - return 0 - return wallet_state['lastSynced'][wallet_state['deviceId']] +# TODO - We should have: +# * self.last_synced_wallet_state - as described +# * self.current_wallet_state - WalletState(cur_encrypted_wallet(), sequence + 1) - and current_wallet_state +# We don't need it yet but we'd be avoiding the entire point of the syncing system. At least keep it around in this demo. -# TODO - do this correctly -def create_login_password(root_password): - return hashlib.sha256(root_password.encode('utf-8')).hexdigest()[:32] +WalletState = namedtuple('WalletState', ['sequence', 'encrypted_wallet']) -# TODO - actually the SDK will do this for now -def create_encryption_key(root_password): - return hashlib.sha256(root_password.encode('utf-8')).hexdigest()[32:] +# TODO - do this correctly. This is a hack example. +def derive_login_password(root_password): + return hashlib.sha256(root_password.encode('utf-8')).hexdigest()[:10] -# TODO - do this correctly -def check_hmac(wallet_state, encryption_key, hmac): - return hmac == 'Good HMAC' +# TODO - do this correctly. This is a hack example. +def derive_sdk_password(root_password): + return hashlib.sha256(root_password.encode('utf-8')).hexdigest()[10:20] -# TODO - do this correctly -def create_hmac(wallet_state, encryption_key): - return 'Good HMAC' +# TODO - do this correctly. This is a hack example. +def derive_hmac_key(root_password): + return hashlib.sha256(root_password.encode('utf-8')).hexdigest()[20:] + +# TODO - do this correctly. This is a hack example. +def create_hmac(wallet_state, hmac_key): + input_str = hmac_key + ':' + str(wallet_state.sequence) + ':' + wallet_state.encrypted_wallet + return hashlib.sha256(input_str.encode('utf-8')).hexdigest() + +def check_hmac(wallet_state, hmac_key, hmac): + return hmac == create_hmac(wallet_state, hmac_key) class Client(): def _validate_new_wallet_state(self, new_wallet_state): @@ -37,20 +46,9 @@ class Client(): return True # Make sure that the new sequence is overall later. - if wallet_state_sequence(new_wallet_state) <= wallet_state_sequence(self.wallet_state): + if new_wallet_state.sequence <= self.wallet_state.sequence: return False - for dev_id in self.wallet_state['lastSynced']: - if dev_id == self.device_id: - # Check if the new wallet has the latest changes from this device - if new_wallet_state['lastSynced'][dev_id] != self.wallet_state['lastSynced'][dev_id]: - return False - else: - # Check if the new wallet somehow regressed on any of the other devices - # This most likely means a bug in another client - if new_wallet_state['lastSynced'][dev_id] < self.wallet_state['lastSynced'][dev_id]: - return False - return True def __init__(self): @@ -76,9 +74,9 @@ class Client(): # on the server. def new_wallet_state(self): # camel-cased to ease json interop - self.wallet_state = {'lastSynced': {}, 'encryptedWallet': ''} + self.wallet_state = WalletState(sequence=0, encrypted_wallet='-') - # TODO - actual encryption with encryption_key + # TODO - actual encryption with encryption_key - or maybe not. self._encrypted_wallet_local_changes = '' def set_account(self, email, root_password): @@ -88,7 +86,7 @@ class Client(): def register(self): body = json.dumps({ 'email': self.email, - 'password': create_login_password(self.root_password), + 'password': derive_login_password(self.root_password), }) response = requests.post(REGISTER_URL, body) if response.status_code != 201: @@ -100,7 +98,7 @@ class Client(): def get_auth_token(self): body = json.dumps({ 'email': self.email, - 'password': create_login_password(self.root_password), + 'password': derive_login_password(self.root_password), 'deviceId': self.device_id, }) response = requests.post(AUTH_URL, body) @@ -116,26 +114,29 @@ class Client(): # you might end up overwriting one with a lower sequence entirely. Maybe we # want to annotate them with which account we're talking about. Again, we # should see how LBRY Desktop/SDK deal with it. - def get_wallet_state(self): + def get_wallet(self): params = { 'token': self.auth_token, } - response = requests.get(WALLET_STATE_URL, params=params) + response = requests.get(WALLET_URL, params=params) if response.status_code != 200: + # TODO check response version on client side now print ('Error', response.status_code) print (response.content) return - new_wallet_state_str = json.loads(response.content)['walletStateJson'] - new_wallet_state = json.loads(new_wallet_state_str) - encryption_key = create_encryption_key(self.root_password) - hmac = json.loads(response.content)['hmac'] - if not check_hmac(new_wallet_state_str, encryption_key, hmac): + hmac_key = derive_hmac_key(self.root_password) + + new_wallet_state = WalletState( + encrypted_wallet=response.json()['encryptedWallet'], + sequence=response.json()['sequence'], + ) + hmac = response.json()['hmac'] + if not check_hmac(new_wallet_state, hmac_key, hmac): print ('Error - bad hmac on new wallet') print (response.content) return - # In reality, we'd examine, merge, verify, validate etc this new wallet state. if self.wallet_state != new_wallet_state and not self._validate_new_wallet_state(new_wallet_state): print ('Error - new wallet does not validate') print (response.content) @@ -151,7 +152,7 @@ class Client(): print ("Got latest walletState:") pprint(self.wallet_state) - def post_wallet_state(self): + def post_wallet(self): # Create a *new* wallet state, indicating that it was last updated by this # device, with the updated sequence, and include our local encrypted wallet changes. # Don't set self.wallet_state to this until we know that it's accepted by @@ -160,48 +161,48 @@ class Client(): print ("No wallet state to post.") return - submitted_wallet_state = { - "deviceId": self.device_id, - "lastSynced": dict(self.wallet_state['lastSynced']), - "encryptedWallet": self.cur_encrypted_wallet(), - } - submitted_wallet_state['lastSynced'][self.device_id] = wallet_state_sequence(self.wallet_state) + 1 + hmac_key = derive_hmac_key(self.root_password) - encryption_key = create_encryption_key(self.root_password) - - submitted_wallet_state_str = json.dumps(submitted_wallet_state) - submitted_wallet_state_hmac = create_hmac(submitted_wallet_state_str, encryption_key) - body = json.dumps({ + submitted_wallet_state = WalletState( + encrypted_wallet=self.cur_encrypted_wallet(), + sequence=self.wallet_state.sequence + 1 + ) + wallet_request = { + 'version': CURRENT_VERSION, 'token': self.auth_token, - 'walletStateJson': submitted_wallet_state_str, - 'hmac': submitted_wallet_state_hmac - }) - response = requests.post(WALLET_STATE_URL, body) + "encryptedWallet": submitted_wallet_state.encrypted_wallet, + "sequence": submitted_wallet_state.sequence, + "hmac": create_hmac(submitted_wallet_state, hmac_key), + } + + response = requests.post(WALLET_URL, json.dumps(wallet_request)) if response.status_code == 200: + # TODO check response version on client side now # Our local changes are no longer local, so we reset them self._encrypted_wallet_local_changes = '' print ('Successfully updated wallet state on server') elif response.status_code == 409: - print ('Wallet state out of date. Getting updated wallet state. Try again.') + print ('Wallet state out of date. Getting updated wallet state. Try posting again after this.') # Don't return yet! We got the updated state here, so we still process it below. else: print ('Error', response.status_code) print (response.content) return - # Now we get a new wallet state back as a response - # TODO - factor this into the same thing as the get_wallet_state function + # Now we get a new wallet back as a response + # TODO - factor this code into the same thing as the get_wallet function - new_wallet_state_str = json.loads(response.content)['walletStateJson'] - new_wallet_state_hmac = json.loads(response.content)['hmac'] - new_wallet_state = json.loads(new_wallet_state_str) - if not check_hmac(new_wallet_state_str, encryption_key, new_wallet_state_hmac): + new_wallet_state = WalletState( + encrypted_wallet=response.json()['encryptedWallet'], + sequence=response.json()['sequence'], + ) + hmac = response.json()['hmac'] + if not check_hmac(new_wallet_state, hmac_key, hmac): print ('Error - bad hmac on new wallet') print (response.content) return - # In reality, we'd examine, merge, verify, validate etc this new wallet state. if submitted_wallet_state != new_wallet_state and not self._validate_new_wallet_state(new_wallet_state): print ('Error - new wallet does not validate') print (response.content) @@ -227,4 +228,4 @@ class Client(): # The local changes on top of whatever came from the server # If we pull new changes from server, we "rebase" these on top of it # If we push changes, the full "rebased" version gets committed to the server - return self.wallet_state['encryptedWallet'] + self._encrypted_wallet_local_changes + return self.wallet_state.encrypted_wallet + self._encrypted_wallet_local_changes diff --git a/wallet/wallet.go b/wallet/wallet.go index 7df3891..64a9706 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -1,39 +1,5 @@ package wallet -import "orblivion/lbry-id/auth" - -// Currently a small package but given other packages it makes imports easier. -// Also this might grow substantially over time - -// For test stubs -type WalletUtilInterface interface { - ValidateWalletStateMetadata(walletState *WalletStateMetadata) bool -} - -type WalletUtil struct{} - -// This is a subset of the WalletState structure, only the metadata fields. We -// don't need access to the encrypted wallet. -type WalletStateMetadata struct { - DeviceId auth.DeviceId `json:"deviceId"` - LastSynced map[auth.DeviceId]int `json:"lastSynced"` -} - -type WalletStateHmac string - -// TODO - These "validate" functions could/should be methods. Though I think -// we'd lose mockability for testing, since the method isn't the -// WalletUtilInterface. -// Mainly the job of the clients but we may as well short-circuit problems -// here before saving them. -func (wu *WalletUtil) ValidateWalletStateMetadata(walletState *WalletStateMetadata) bool { - - // TODO - nonempty fields, up to date, etc - return true -} - -// Assumptions: `ws` has been validated -// Avoid having to check for error -func (ws *WalletStateMetadata) Sequence() int { - return ws.LastSynced[ws.DeviceId] -} +type EncryptedWallet string +type WalletHmac string +type Sequence uint32 diff --git a/wallet/wallet_test.go b/wallet/wallet_test.go deleted file mode 100644 index 3291bde..0000000 --- a/wallet/wallet_test.go +++ /dev/null @@ -1,19 +0,0 @@ -package wallet - -import ( - "testing" -) - -// Test stubs for now - -func TestWalletSequence(t *testing.T) { - t.Fatalf("Test me: test that walletState.Sequence() == walletState.lastSynced[wallet.DeviceId]") -} - -func TestWalletValidateWalletState(t *testing.T) { - // walletState.DeviceId in walletState.lastSynced - // Sequence for lastSynced all > 1 - t.Fatalf("Test me: Implement and test validateWalletState.") -} - -// TODO - other wallet integrity stuff? particularly related to sequence?