From 7749854eb736921c88469f31837a2516d104784d Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Wed, 10 Jun 2020 18:56:24 +0100 Subject: [PATCH] Remove account IDs in favor of just account names These were a "security" feature but pre-emptive and just add complications when we can implement secrets later --- cmd/rove-server/internal/api_test.go | 11 +++-------- cmd/rove-server/internal/routes.go | 13 +------------ cmd/rove-server/internal/routes_test.go | 12 ++++++------ cmd/rove-server/internal/server.go | 4 ++-- cmd/rove/main.go | 4 ++-- pkg/accounts/accounts.go | 18 ++++++------------ pkg/accounts/accounts_test.go | 11 +++-------- pkg/rove/api.go | 3 --- 8 files changed, 23 insertions(+), 53 deletions(-) diff --git a/cmd/rove-server/internal/api_test.go b/cmd/rove-server/internal/api_test.go index 16d6eb1..3e6a0c1 100644 --- a/cmd/rove-server/internal/api_test.go +++ b/cmd/rove-server/internal/api_test.go @@ -50,7 +50,6 @@ func TestServer_Register(t *testing.T) { r1, err := serv.Register(d1) assert.NoError(t, err) assert.True(t, r1.Success) - assert.NotZero(t, len(r1.Id)) d2 := rove.RegisterData{ Name: uuid.New().String(), @@ -58,7 +57,6 @@ func TestServer_Register(t *testing.T) { r2, err := serv.Register(d2) assert.NoError(t, err) assert.True(t, r2.Success) - assert.NotZero(t, len(r2.Id)) r3, err := serv.Register(d1) assert.NoError(t, err) @@ -72,7 +70,6 @@ func TestServer_Command(t *testing.T) { r1, err := serv.Register(d1) assert.NoError(t, err) assert.True(t, r1.Success) - assert.NotZero(t, len(r1.Id)) c := rove.CommandData{ Commands: []game.Command{ @@ -83,7 +80,7 @@ func TestServer_Command(t *testing.T) { }, }, } - r3, err := serv.Command(r1.Id, c) + r3, err := serv.Command(d1.Name, c) assert.NoError(t, err) assert.True(t, r3.Success) } @@ -95,9 +92,8 @@ func TestServer_Radar(t *testing.T) { r1, err := serv.Register(d1) assert.NoError(t, err) assert.True(t, r1.Success) - assert.NotZero(t, len(r1.Id)) - r3, err := serv.Radar(r1.Id) + r3, err := serv.Radar(d1.Name) assert.NoError(t, err) assert.True(t, r3.Success) } @@ -109,9 +105,8 @@ func TestServer_Rover(t *testing.T) { r1, err := serv.Register(d1) assert.NoError(t, err) assert.True(t, r1.Success) - assert.NotZero(t, len(r1.Id)) - r3, err := serv.Rover(r1.Id) + r3, err := serv.Rover(d1.Name) assert.NoError(t, err) assert.True(t, r3.Success) } diff --git a/cmd/rove-server/internal/routes.go b/cmd/rove-server/internal/routes.go index 7f7127d..e427e7e 100644 --- a/cmd/rove-server/internal/routes.go +++ b/cmd/rove-server/internal/routes.go @@ -6,7 +6,6 @@ import ( "io" "net/http" - "github.com/google/uuid" "github.com/mdiluz/rove/pkg/rove" "github.com/mdiluz/rove/pkg/version" ) @@ -87,7 +86,7 @@ func HandleRegister(s *Server, vars map[string]string, b io.ReadCloser, w io.Wri } else if acc, err := s.accountant.RegisterAccount(data.Name); err != nil { response.Error = err.Error() - } else if _, _, err := s.SpawnRoverForAccount(acc.Id); err != nil { + } else if _, _, err := s.SpawnRoverForAccount(acc.Name); err != nil { response.Error = err.Error() } else if err := s.SaveAll(); err != nil { @@ -95,7 +94,6 @@ func HandleRegister(s *Server, vars map[string]string, b io.ReadCloser, w io.Wri } else { // Save out the new accounts - response.Id = acc.Id.String() response.Success = true } @@ -120,9 +118,6 @@ func HandleCommand(s *Server, vars map[string]string, b io.ReadCloser, w io.Writ } else if len(id) == 0 { response.Error = "No account ID provided" - } else if id, err := uuid.Parse(id); err != nil { - response.Error = fmt.Sprintf("Provided account ID was invalid: %s", err) - } else if inst, err := s.accountant.GetRover(id); err != nil { response.Error = fmt.Sprintf("Provided account has no rover: %s", err) @@ -147,9 +142,6 @@ func HandleRadar(s *Server, vars map[string]string, b io.ReadCloser, w io.Writer if len(id) == 0 { response.Error = "No account ID provided" - } else if id, err := uuid.Parse(id); err != nil { - response.Error = fmt.Sprintf("Provided account ID was invalid: %s", err) - } else if inst, err := s.accountant.GetRover(id); err != nil { response.Error = fmt.Sprintf("Provided account has no rover: %s", err) @@ -179,9 +171,6 @@ func HandleRover(s *Server, vars map[string]string, b io.ReadCloser, w io.Writer if len(id) == 0 { response.Error = "No account ID provided" - } else if id, err := uuid.Parse(id); err != nil { - response.Error = fmt.Sprintf("Provided account ID was invalid: %s", err) - } else if inst, err := s.accountant.GetRover(id); err != nil { response.Error = fmt.Sprintf("Provided account has no rover: %s", err) diff --git a/cmd/rove-server/internal/routes_test.go b/cmd/rove-server/internal/routes_test.go index f765a11..7cb9995 100644 --- a/cmd/rove-server/internal/routes_test.go +++ b/cmd/rove-server/internal/routes_test.go @@ -66,7 +66,7 @@ func TestHandleCommand(t *testing.T) { assert.NoError(t, err, "Error registering account") // Spawn the rover rover for the account - _, inst, err := s.SpawnRoverForAccount(a.Id) + _, inst, err := s.SpawnRoverForAccount(a.Name) assert.NoError(t, s.world.WarpRover(inst, vector.Vector{})) attribs, err := s.world.RoverAttributes(inst) @@ -85,7 +85,7 @@ func TestHandleCommand(t *testing.T) { b, err := json.Marshal(data) assert.NoError(t, err, "Error marshalling data") - request, _ := http.NewRequest(http.MethodPost, path.Join("/", a.Id.String(), "/command"), bytes.NewReader(b)) + request, _ := http.NewRequest(http.MethodPost, path.Join("/", a.Name, "/command"), bytes.NewReader(b)) response := httptest.NewRecorder() s.router.ServeHTTP(response, request) @@ -118,7 +118,7 @@ func TestHandleRadar(t *testing.T) { assert.NoError(t, err, "Error registering account") // Spawn the rover rover for the account - attrib, id, err := s.SpawnRoverForAccount(a.Id) + attrib, id, err := s.SpawnRoverForAccount(a.Name) assert.NoError(t, err) // Warp this rover to 0,0 @@ -134,7 +134,7 @@ func TestHandleRadar(t *testing.T) { assert.NoError(t, s.world.Atlas.SetTile(rockPos, game.TileRock)) assert.NoError(t, s.world.Atlas.SetTile(emptyPos, game.TileEmpty)) - request, _ := http.NewRequest(http.MethodGet, path.Join("/", a.Id.String(), "/radar"), nil) + request, _ := http.NewRequest(http.MethodGet, path.Join("/", a.Name, "/radar"), nil) response := httptest.NewRecorder() s.router.ServeHTTP(response, request) @@ -172,10 +172,10 @@ func TestHandleRover(t *testing.T) { assert.NoError(t, err, "Error registering account") // Spawn one rover for the account - attribs, _, err := s.SpawnRoverForAccount(a.Id) + attribs, _, err := s.SpawnRoverForAccount(a.Name) assert.NoError(t, err) - request, _ := http.NewRequest(http.MethodGet, path.Join("/", a.Id.String(), "/rover"), nil) + request, _ := http.NewRequest(http.MethodGet, path.Join("/", a.Name, "/rover"), nil) response := httptest.NewRecorder() s.router.ServeHTTP(response, request) diff --git a/cmd/rove-server/internal/server.go b/cmd/rove-server/internal/server.go index 08c6eb6..746774c 100644 --- a/cmd/rove-server/internal/server.go +++ b/cmd/rove-server/internal/server.go @@ -281,7 +281,7 @@ func (s *Server) wrapHandler(method string, handler Handler) func(w http.Respons } // SpawnRoverForAccount spawns the rover rover for an account -func (s *Server) SpawnRoverForAccount(accountid uuid.UUID) (game.RoverAttributes, uuid.UUID, error) { +func (s *Server) SpawnRoverForAccount(account string) (game.RoverAttributes, uuid.UUID, error) { if inst, err := s.world.SpawnRover(); err != nil { return game.RoverAttributes{}, uuid.UUID{}, err @@ -289,7 +289,7 @@ func (s *Server) SpawnRoverForAccount(accountid uuid.UUID) (game.RoverAttributes return game.RoverAttributes{}, uuid.UUID{}, fmt.Errorf("No attributes found for created rover: %s", err) } else { - if err := s.accountant.AssignRover(accountid, inst); err != nil { + if err := s.accountant.AssignRover(account, inst); err != nil { // Try and clear up the rover if err := s.world.DestroyRover(inst); err != nil { fmt.Printf("Failed to destroy rover after failed rover assign: %s", err) diff --git a/cmd/rove/main.go b/cmd/rove/main.go index 7970cfc..f744410 100644 --- a/cmd/rove/main.go +++ b/cmd/rove/main.go @@ -125,8 +125,8 @@ func InnerMain(command string) error { return fmt.Errorf("Server returned failure: %s", response.Error) default: - fmt.Printf("Registered account with id: %s\n", response.Id) - config.Accounts[config.Host] = response.Id + fmt.Printf("Registered account with id: %s\n", d.Name) + config.Accounts[config.Host] = d.Name } case "move": diff --git a/pkg/accounts/accounts.go b/pkg/accounts/accounts.go index 3ab921f..3ca5505 100644 --- a/pkg/accounts/accounts.go +++ b/pkg/accounts/accounts.go @@ -13,9 +13,6 @@ type Account struct { // Name simply describes the account and must be unique Name string `json:"name"` - // Id represents a unique ID per account and is set one registered - Id uuid.UUID `json:"id"` - // Rover represents the rover that this account owns Rover uuid.UUID `json:"rover"` } @@ -26,40 +23,37 @@ type accountantData struct { // Accountant manages a set of accounts type Accountant struct { - Accounts map[uuid.UUID]Account `json:"accounts"` + Accounts map[string]Account `json:"accounts"` } // NewAccountant creates a new accountant func NewAccountant() *Accountant { return &Accountant{ - Accounts: make(map[uuid.UUID]Account), + Accounts: make(map[string]Account), } } // RegisterAccount adds an account to the set of internal accounts func (a *Accountant) RegisterAccount(name string) (acc Account, err error) { - // Set the account ID to a new UUID - acc.Id = uuid.New() + // Set the account name acc.Name = name // Verify this acount isn't already registered for _, a := range a.Accounts { if a.Name == acc.Name { return Account{}, fmt.Errorf("Account name already registered") - } else if a.Id == acc.Id { - return Account{}, fmt.Errorf("Account ID already registered") } } // Simply add the account to the map - a.Accounts[acc.Id] = acc + a.Accounts[acc.Name] = acc return } // AssignRover assigns rover ownership of an rover to an account -func (a *Accountant) AssignRover(account, rover uuid.UUID) error { +func (a *Accountant) AssignRover(account string, rover uuid.UUID) error { // Find the account matching the ID if this, ok := a.Accounts[account]; ok { @@ -73,7 +67,7 @@ func (a *Accountant) AssignRover(account, rover uuid.UUID) error { } // GetRover gets the rover rover for the account -func (a *Accountant) GetRover(account uuid.UUID) (uuid.UUID, error) { +func (a *Accountant) GetRover(account string) (uuid.UUID, error) { // Find the account matching the ID if this, ok := a.Accounts[account]; !ok { return uuid.UUID{}, fmt.Errorf("no account found for id: %s", account) diff --git a/pkg/accounts/accounts_test.go b/pkg/accounts/accounts_test.go index 3ab9795..0e0f3a6 100644 --- a/pkg/accounts/accounts_test.go +++ b/pkg/accounts/accounts_test.go @@ -36,11 +36,6 @@ func TestAccountant_RegisterAccount(t *testing.T) { t.Errorf("Missmatched account name after register, expected: %s, actual: %s", nameb, acca.Name) } - // Verify our accounts have differing IDs - if acca.Id == accb.Id { - t.Error("Duplicate account IDs fo separate accounts") - } - // Verify another request gets rejected _, err = accountant.RegisterAccount(namea) if err == nil { @@ -62,12 +57,12 @@ func TestAccountant_AssignGetRover(t *testing.T) { inst := uuid.New() - err = accountant.AssignRover(a.Id, inst) + err = accountant.AssignRover(a.Name, inst) if err != nil { t.Error("Failed to set rover for created account") - } else if accountant.Accounts[a.Id].Rover != inst { + } else if accountant.Accounts[a.Name].Rover != inst { t.Error("Rover for assigned account is incorrect") - } else if id, err := accountant.GetRover(a.Id); err != nil { + } else if id, err := accountant.GetRover(a.Name); err != nil { t.Error("Failed to get rover for account") } else if id != inst { t.Error("Fetched rover is incorrect for account") diff --git a/pkg/rove/api.go b/pkg/rove/api.go index b9b1d23..8ecb108 100644 --- a/pkg/rove/api.go +++ b/pkg/rove/api.go @@ -27,7 +27,6 @@ type StatusResponse struct { // API: /register method: POST // Register registers a user by name -// Responds with a unique ID for that user to be used in future requests func (s Server) Register(d RegisterData) (r RegisterResponse, err error) { err = s.Post("register", d, &r) return @@ -42,8 +41,6 @@ type RegisterData struct { type RegisterResponse struct { Success bool `json:"success"` Error string `json:"error,omitempty"` - - Id string `json:"id"` } // ==============================