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
This commit is contained in:
Marc Di Luzio 2020-06-10 18:56:24 +01:00
parent b3b369f608
commit 7749854eb7
8 changed files with 23 additions and 53 deletions

View file

@ -50,7 +50,6 @@ func TestServer_Register(t *testing.T) {
r1, err := serv.Register(d1) r1, err := serv.Register(d1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, r1.Success) assert.True(t, r1.Success)
assert.NotZero(t, len(r1.Id))
d2 := rove.RegisterData{ d2 := rove.RegisterData{
Name: uuid.New().String(), Name: uuid.New().String(),
@ -58,7 +57,6 @@ func TestServer_Register(t *testing.T) {
r2, err := serv.Register(d2) r2, err := serv.Register(d2)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, r2.Success) assert.True(t, r2.Success)
assert.NotZero(t, len(r2.Id))
r3, err := serv.Register(d1) r3, err := serv.Register(d1)
assert.NoError(t, err) assert.NoError(t, err)
@ -72,7 +70,6 @@ func TestServer_Command(t *testing.T) {
r1, err := serv.Register(d1) r1, err := serv.Register(d1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, r1.Success) assert.True(t, r1.Success)
assert.NotZero(t, len(r1.Id))
c := rove.CommandData{ c := rove.CommandData{
Commands: []game.Command{ 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.NoError(t, err)
assert.True(t, r3.Success) assert.True(t, r3.Success)
} }
@ -95,9 +92,8 @@ func TestServer_Radar(t *testing.T) {
r1, err := serv.Register(d1) r1, err := serv.Register(d1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, r1.Success) 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.NoError(t, err)
assert.True(t, r3.Success) assert.True(t, r3.Success)
} }
@ -109,9 +105,8 @@ func TestServer_Rover(t *testing.T) {
r1, err := serv.Register(d1) r1, err := serv.Register(d1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, r1.Success) 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.NoError(t, err)
assert.True(t, r3.Success) assert.True(t, r3.Success)
} }

View file

@ -6,7 +6,6 @@ import (
"io" "io"
"net/http" "net/http"
"github.com/google/uuid"
"github.com/mdiluz/rove/pkg/rove" "github.com/mdiluz/rove/pkg/rove"
"github.com/mdiluz/rove/pkg/version" "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 { } else if acc, err := s.accountant.RegisterAccount(data.Name); err != nil {
response.Error = err.Error() 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() response.Error = err.Error()
} else if err := s.SaveAll(); err != nil { } 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 { } else {
// Save out the new accounts // Save out the new accounts
response.Id = acc.Id.String()
response.Success = true 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 { } else if len(id) == 0 {
response.Error = "No account ID provided" 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 { } else if inst, err := s.accountant.GetRover(id); err != nil {
response.Error = fmt.Sprintf("Provided account has no rover: %s", err) 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 { if len(id) == 0 {
response.Error = "No account ID provided" 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 { } else if inst, err := s.accountant.GetRover(id); err != nil {
response.Error = fmt.Sprintf("Provided account has no rover: %s", err) 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 { if len(id) == 0 {
response.Error = "No account ID provided" 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 { } else if inst, err := s.accountant.GetRover(id); err != nil {
response.Error = fmt.Sprintf("Provided account has no rover: %s", err) response.Error = fmt.Sprintf("Provided account has no rover: %s", err)

View file

@ -66,7 +66,7 @@ func TestHandleCommand(t *testing.T) {
assert.NoError(t, err, "Error registering account") assert.NoError(t, err, "Error registering account")
// Spawn the rover rover for the 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{})) assert.NoError(t, s.world.WarpRover(inst, vector.Vector{}))
attribs, err := s.world.RoverAttributes(inst) attribs, err := s.world.RoverAttributes(inst)
@ -85,7 +85,7 @@ func TestHandleCommand(t *testing.T) {
b, err := json.Marshal(data) b, err := json.Marshal(data)
assert.NoError(t, err, "Error marshalling 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() response := httptest.NewRecorder()
s.router.ServeHTTP(response, request) s.router.ServeHTTP(response, request)
@ -118,7 +118,7 @@ func TestHandleRadar(t *testing.T) {
assert.NoError(t, err, "Error registering account") assert.NoError(t, err, "Error registering account")
// Spawn the rover rover for the 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) assert.NoError(t, err)
// Warp this rover to 0,0 // 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(rockPos, game.TileRock))
assert.NoError(t, s.world.Atlas.SetTile(emptyPos, game.TileEmpty)) 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() response := httptest.NewRecorder()
s.router.ServeHTTP(response, request) s.router.ServeHTTP(response, request)
@ -172,10 +172,10 @@ func TestHandleRover(t *testing.T) {
assert.NoError(t, err, "Error registering account") assert.NoError(t, err, "Error registering account")
// Spawn one rover for the account // Spawn one rover for the account
attribs, _, err := s.SpawnRoverForAccount(a.Id) attribs, _, err := s.SpawnRoverForAccount(a.Name)
assert.NoError(t, err) 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() response := httptest.NewRecorder()
s.router.ServeHTTP(response, request) s.router.ServeHTTP(response, request)

View file

@ -281,7 +281,7 @@ func (s *Server) wrapHandler(method string, handler Handler) func(w http.Respons
} }
// SpawnRoverForAccount spawns the rover rover for an account // 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 { if inst, err := s.world.SpawnRover(); err != nil {
return game.RoverAttributes{}, uuid.UUID{}, err 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) return game.RoverAttributes{}, uuid.UUID{}, fmt.Errorf("No attributes found for created rover: %s", err)
} else { } 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 // Try and clear up the rover
if err := s.world.DestroyRover(inst); err != nil { if err := s.world.DestroyRover(inst); err != nil {
fmt.Printf("Failed to destroy rover after failed rover assign: %s", err) fmt.Printf("Failed to destroy rover after failed rover assign: %s", err)

View file

@ -125,8 +125,8 @@ func InnerMain(command string) error {
return fmt.Errorf("Server returned failure: %s", response.Error) return fmt.Errorf("Server returned failure: %s", response.Error)
default: default:
fmt.Printf("Registered account with id: %s\n", response.Id) fmt.Printf("Registered account with id: %s\n", d.Name)
config.Accounts[config.Host] = response.Id config.Accounts[config.Host] = d.Name
} }
case "move": case "move":

View file

@ -13,9 +13,6 @@ type Account struct {
// Name simply describes the account and must be unique // Name simply describes the account and must be unique
Name string `json:"name"` 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 represents the rover that this account owns
Rover uuid.UUID `json:"rover"` Rover uuid.UUID `json:"rover"`
} }
@ -26,40 +23,37 @@ type accountantData struct {
// Accountant manages a set of accounts // Accountant manages a set of accounts
type Accountant struct { type Accountant struct {
Accounts map[uuid.UUID]Account `json:"accounts"` Accounts map[string]Account `json:"accounts"`
} }
// NewAccountant creates a new accountant // NewAccountant creates a new accountant
func NewAccountant() *Accountant { func NewAccountant() *Accountant {
return &Accountant{ return &Accountant{
Accounts: make(map[uuid.UUID]Account), Accounts: make(map[string]Account),
} }
} }
// RegisterAccount adds an account to the set of internal accounts // RegisterAccount adds an account to the set of internal accounts
func (a *Accountant) RegisterAccount(name string) (acc Account, err error) { func (a *Accountant) RegisterAccount(name string) (acc Account, err error) {
// Set the account ID to a new UUID // Set the account name
acc.Id = uuid.New()
acc.Name = name acc.Name = name
// Verify this acount isn't already registered // Verify this acount isn't already registered
for _, a := range a.Accounts { for _, a := range a.Accounts {
if a.Name == acc.Name { if a.Name == acc.Name {
return Account{}, fmt.Errorf("Account name already registered") 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 // Simply add the account to the map
a.Accounts[acc.Id] = acc a.Accounts[acc.Name] = acc
return return
} }
// AssignRover assigns rover ownership of an rover to an account // 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 // Find the account matching the ID
if this, ok := a.Accounts[account]; ok { 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 // 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 // Find the account matching the ID
if this, ok := a.Accounts[account]; !ok { if this, ok := a.Accounts[account]; !ok {
return uuid.UUID{}, fmt.Errorf("no account found for id: %s", account) return uuid.UUID{}, fmt.Errorf("no account found for id: %s", account)

View file

@ -36,11 +36,6 @@ func TestAccountant_RegisterAccount(t *testing.T) {
t.Errorf("Missmatched account name after register, expected: %s, actual: %s", nameb, acca.Name) 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 // Verify another request gets rejected
_, err = accountant.RegisterAccount(namea) _, err = accountant.RegisterAccount(namea)
if err == nil { if err == nil {
@ -62,12 +57,12 @@ func TestAccountant_AssignGetRover(t *testing.T) {
inst := uuid.New() inst := uuid.New()
err = accountant.AssignRover(a.Id, inst) err = accountant.AssignRover(a.Name, inst)
if err != nil { if err != nil {
t.Error("Failed to set rover for created account") 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") 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") t.Error("Failed to get rover for account")
} else if id != inst { } else if id != inst {
t.Error("Fetched rover is incorrect for account") t.Error("Fetched rover is incorrect for account")

View file

@ -27,7 +27,6 @@ type StatusResponse struct {
// API: /register method: POST // API: /register method: POST
// Register registers a user by name // 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) { func (s Server) Register(d RegisterData) (r RegisterResponse, err error) {
err = s.Post("register", d, &r) err = s.Post("register", d, &r)
return return
@ -42,8 +41,6 @@ type RegisterData struct {
type RegisterResponse struct { type RegisterResponse struct {
Success bool `json:"success"` Success bool `json:"success"`
Error string `json:"error,omitempty"` Error string `json:"error,omitempty"`
Id string `json:"id"`
} }
// ============================== // ==============================