diff --git a/auth.go b/auth.go index 2eda757..374dd7b 100644 --- a/auth.go +++ b/auth.go @@ -86,7 +86,7 @@ func AuthAuthenticate(app *App) func(c echo.Context) error { playerName := req.Username var player Player - result := app.DB.Preload("Clients").Preload("User").First(&player, "name = ?", playerName) + result := app.DB.Preload("User").First(&player, "name = ?", playerName) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { return c.JSONBlob(http.StatusUnauthorized, invalidCredentialsBlob) diff --git a/common.go b/common.go index 8de1df3..593580a 100644 --- a/common.go +++ b/common.go @@ -149,26 +149,6 @@ func (app *App) CachedPostJSON(url string, body []byte, ttl int) (RequestCacheVa return response, nil } -type Error error - -func IsErrorUniqueFailed(err error) bool { - if err == nil { - return false - } - // Work around https://stackoverflow.com/questions/75489773/why-do-i-get-second-argument-to-errors-as-should-not-be-error-build-error-in - e := (errors.New("UNIQUE constraint failed")).(Error) - return errors.As(err, &e) -} - -func IsErrorUniqueFailedField(err error, field string) bool { - if err == nil { - return false - } - - // The Go programming language 😎 - return err.Error() == "UNIQUE constraint failed: "+field -} - func (app *App) GetSkinPath(hash string) string { dir := path.Join(app.Config.StateDirectory, "skin") return path.Join(dir, fmt.Sprintf("%s.png", hash)) diff --git a/db.go b/db.go index 83ce6dc..ada4eda 100644 --- a/db.go +++ b/db.go @@ -2,6 +2,7 @@ package main import ( "database/sql" + "errors" "fmt" "gorm.io/driver/sqlite" "gorm.io/gorm" @@ -12,23 +13,38 @@ import ( "time" ) -func OpenDB(config *Config) (*gorm.DB, error) { - dbPath := path.Join(config.StateDirectory, "drasl.db") - _, err := os.Stat(dbPath) - alreadyExisted := err == nil +const CURRENT_USER_VERSION = 4 - db := Unwrap(gorm.Open(sqlite.Open(dbPath), &gorm.Config{ - Logger: logger.Default.LogMode(logger.Silent), - })) - err = migrate(db, alreadyExisted) - if err != nil { - return nil, fmt.Errorf("Error migrating database: %w", err) +const PLAYER_NAME_TAKEN_BY_USERNAME_ERROR = "PLAYER_NAME_TAKEN_BY_USERNAME" +const USERNAME_TAKEN_BY_PLAYER_NAME_ERROR = "USERNAME_TAKEN_BY_PLAYER_NAME" + +type Error error + +func IsErrorUniqueFailed(err error) bool { + if err == nil { + return false } - - return db, nil + // Work around https://stackoverflow.com/questions/75489773/why-do-i-get-second-argument-to-errors-as-should-not-be-error-build-error-in + e := (errors.New("UNIQUE constraint failed")).(Error) + return errors.As(err, &e) || IsErrorPlayerNameTakenByUsername(err) || IsErrorUsernameTakenByPlayerName(err) } -const CURRENT_USER_VERSION = 4 +func IsErrorUniqueFailedField(err error, field string) bool { + if err == nil { + return false + } + + // The Go programming language 😎 + return err.Error() == "UNIQUE constraint failed: "+field +} + +func IsErrorUsernameTakenByPlayerName(err error) bool { + return err.Error() == USERNAME_TAKEN_BY_PLAYER_NAME_ERROR +} + +func IsErrorPlayerNameTakenByUsername(err error) bool { + return err.Error() == PLAYER_NAME_TAKEN_BY_USERNAME_ERROR +} type V1User struct { IsAdmin bool @@ -108,6 +124,22 @@ type V4User = User type V4Player = Player type V4Client = Client +func OpenDB(config *Config) (*gorm.DB, error) { + dbPath := path.Join(config.StateDirectory, "drasl.db") + _, err := os.Stat(dbPath) + alreadyExisted := err == nil + + db := Unwrap(gorm.Open(sqlite.Open(dbPath), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + })) + err = migrate(db, alreadyExisted) + if err != nil { + return nil, fmt.Errorf("Error migrating database: %w", err) + } + + return db, nil +} + func setUserVersion(tx *gorm.DB, userVersion uint) error { // PRAGMA user_version = ? doesn't work here return tx.Exec(fmt.Sprintf("PRAGMA user_version = %d", userVersion)).Error @@ -275,6 +307,75 @@ func migrate(db *gorm.DB, alreadyExisted bool) error { return err } + err = tx.Exec(fmt.Sprintf(` + DROP TRIGGER IF EXISTS v4_insert_unique_username; + CREATE TRIGGER v4_insert_unique_username + BEFORE INSERT ON users + FOR EACH ROW + BEGIN + -- We have to reimplement the regular "UNIQUE constraint + -- failed" errors here too since we want them to take priority + SELECT RAISE(ABORT, 'UNIQUE constraint failed: users.username') + WHERE EXISTS( + SELECT 1 FROM users WHERE username = NEW.username AND uuid != NEW.uuid + ); + + SELECT RAISE(ABORT, '%[1]s') + WHERE EXISTS( + SELECT 1 from players WHERE name == NEW.username AND user_uuid != NEW.uuid + ); + END; + + DROP TRIGGER IF EXISTS v4_update_unique_username; + CREATE TRIGGER v4_update_unique_username + BEFORE UPDATE ON users + FOR EACH ROW + BEGIN + SELECT RAISE(ABORT, 'UNIQUE constraint failed: users.username') + WHERE EXISTS( + SELECT 1 FROM users WHERE username = NEW.username AND uuid != NEW.uuid + ); + + SELECT RAISE(ABORT, '%[1]s') + WHERE EXISTS( + SELECT 1 from players WHERE name == NEW.username AND user_uuid != NEW.uuid + ); + END; + + DROP TRIGGER IF EXISTS v4_insert_unique_player_name; + CREATE TRIGGER v4_insert_unique_player_name + BEFORE INSERT ON players + BEGIN + SELECT RAISE(ABORT, 'UNIQUE constraint failed: players.name') + WHERE EXISTS( + SELECT 1 from players WHERE name == NEW.name AND uuid != NEW.uuid + ); + + SELECT RAISE(ABORT, '%[2]s') + WHERE EXISTS( + SELECT 1 from users WHERE username == NEW.name AND uuid != NEW.user_uuid + ); + END; + + DROP TRIGGER IF EXISTS v4_update_unique_player_name; + CREATE TRIGGER v4_update_unique_player_name + BEFORE UPDATE ON players + BEGIN + SELECT RAISE(ABORT, 'UNIQUE constraint failed: players.name') + WHERE EXISTS( + SELECT 1 from players WHERE name == NEW.name AND uuid != NEW.uuid + ); + + SELECT RAISE(ABORT, '%[2]s') + WHERE EXISTS( + SELECT 1 from users WHERE username == NEW.name AND uuid != NEW.user_uuid + ); + END; + `, USERNAME_TAKEN_BY_PLAYER_NAME_ERROR, PLAYER_NAME_TAKEN_BY_USERNAME_ERROR)).Error + if err != nil { + return err + } + if err := setUserVersion(tx, userVersion); err != nil { return err } diff --git a/front.go b/front.go index 655a613..8ab71b2 100644 --- a/front.go +++ b/front.go @@ -179,10 +179,6 @@ func getReturnURL(app *App, c *echo.Context) string { if (*c).FormValue("returnUrl") != "" { return (*c).FormValue("returnUrl") } - // TODO no idea why this is here - // if (*c).QueryParam("returnUrl") != "" { - // return (*c).QueryParam("username") - // } return app.FrontEndURL } @@ -336,7 +332,7 @@ func FrontAdmin(app *App) func(c echo.Context) error { return withBrowserAdmin(app, func(c echo.Context, user *User) error { var users []User - result := app.DB.Preload("Players").Find(&users) + result := app.DB.Find(&users) if result.Error != nil { return result.Error } @@ -465,7 +461,7 @@ func FrontUser(app *App) func(c echo.Context) error { adminView := false if targetUUID == "" || targetUUID == user.UUID { var targetUserStruct User - result := app.DB.Preload("Players").First(&targetUserStruct, "uuid = ?", user.UUID) + result := app.DB.First(&targetUserStruct, "uuid = ?", user.UUID) if result.Error != nil { return result.Error } @@ -476,7 +472,7 @@ func FrontUser(app *App) func(c echo.Context) error { } adminView = true var targetUserStruct User - result := app.DB.Preload("Players").First(&targetUserStruct, "uuid = ?", targetUUID) + result := app.DB.First(&targetUserStruct, "uuid = ?", targetUUID) if result.Error != nil { returnURL, err := url.JoinPath(app.FrontEndURL, "web/admin") if err != nil { @@ -942,6 +938,8 @@ func FrontRegister(app *App) func(c echo.Context) error { func addDestination(url_ string, destination string) (string, error) { if destination == "" { return url_, nil + } else if url_ == destination { + return url_, nil } else { urlStruct, err := url.Parse(url_) if err != nil { diff --git a/front_test.go b/front_test.go index 204831c..2caa599 100644 --- a/front_test.go +++ b/front_test.go @@ -20,7 +20,7 @@ import ( var FAKE_BROWSER_TOKEN = "deadbeef" -var EXISTING_USERNAME = "existing" +var EXISTING_PLAYER_NAME = "existing" func setupRegistrationExistingPlayerTS(requireSkinVerification bool, requireInvite bool) *TestSuite { ts := &TestSuite{} @@ -48,7 +48,7 @@ func setupRegistrationExistingPlayerTS(requireSkinVerification bool, requireInvi } ts.Setup(config) - ts.CreateTestUser(ts.AuxApp, ts.AuxServer, EXISTING_USERNAME) + ts.CreateTestUser(ts.AuxApp, ts.AuxServer, EXISTING_PLAYER_NAME) return ts } @@ -90,26 +90,38 @@ func (ts *TestSuite) registrationShouldSucceed(t *testing.T, rec *httptest.Respo assert.Equal(t, http.StatusSeeOther, rec.Code) assert.Equal(t, "", getErrorMessage(rec)) assert.NotEqual(t, "", getCookie(rec, "browserToken").Value) - assert.Equal(t, ts.App.FrontEndURL+"/web/profile", rec.Header().Get("Location")) + assert.Equal(t, ts.App.FrontEndURL+"/web/user", rec.Header().Get("Location")) } -func (ts *TestSuite) updateShouldFail(t *testing.T, rec *httptest.ResponseRecorder, errorMessage string, returnURL string) { +func (ts *TestSuite) updateUserShouldFail(t *testing.T, rec *httptest.ResponseRecorder, errorMessage string, returnURL string) { assert.Equal(t, http.StatusSeeOther, rec.Code) assert.Equal(t, errorMessage, getErrorMessage(rec)) assert.Equal(t, returnURL, rec.Header().Get("Location")) } -func (ts *TestSuite) updateShouldSucceed(t *testing.T, rec *httptest.ResponseRecorder) { +func (ts *TestSuite) updateUserShouldSucceed(t *testing.T, rec *httptest.ResponseRecorder) { assert.Equal(t, http.StatusSeeOther, rec.Code) assert.Equal(t, "", getErrorMessage(rec)) - assert.Equal(t, ts.App.FrontEndURL+"/web/profile", rec.Header().Get("Location")) + assert.Equal(t, ts.App.FrontEndURL+"/web/user", rec.Header().Get("Location")) +} + +func (ts *TestSuite) updatePlayerShouldFail(t *testing.T, rec *httptest.ResponseRecorder, errorMessage string, returnURL string) { + assert.Equal(t, http.StatusSeeOther, rec.Code) + assert.Equal(t, errorMessage, getErrorMessage(rec)) + assert.Equal(t, returnURL, rec.Header().Get("Location")) +} + +func (ts *TestSuite) updatePlayerShouldSucceed(t *testing.T, rec *httptest.ResponseRecorder, playerUUID string) { + assert.Equal(t, http.StatusSeeOther, rec.Code) + assert.Equal(t, "", getErrorMessage(rec)) + assert.Equal(t, ts.App.FrontEndURL+"/web/player/"+playerUUID, rec.Header().Get("Location")) } func (ts *TestSuite) loginShouldSucceed(t *testing.T, rec *httptest.ResponseRecorder) { assert.Equal(t, http.StatusSeeOther, rec.Code) assert.Equal(t, "", getErrorMessage(rec)) assert.NotEqual(t, "", getCookie(rec, "browserToken").Value) - assert.Equal(t, ts.App.FrontEndURL+"/web/profile", rec.Header().Get("Location")) + assert.Equal(t, ts.App.FrontEndURL+"/web/user", rec.Header().Get("Location")) } func (ts *TestSuite) loginShouldFail(t *testing.T, rec *httptest.ResponseRecorder, errorMessage string) { @@ -133,7 +145,8 @@ func TestFront(t *testing.T) { t.Run("Test web app manifest", ts.testWebManifest) t.Run("Test registration as new player", ts.testRegistrationNewPlayer) t.Run("Test registration as new player, chosen UUID, chosen UUID not allowed", ts.testRegistrationNewPlayerChosenUUIDNotAllowed) - t.Run("Test profile update", ts.testUpdate) + t.Run("Test user update", ts.testUserUpdate) + t.Run("Test player update", ts.testPlayerUpdate) t.Run("Test creating/deleting invites", ts.testNewInviteDeleteInvite) t.Run("Test login, logout", ts.testLoginLogout) t.Run("Test delete account", ts.testDeleteAccount) @@ -217,31 +230,31 @@ func TestFront(t *testing.T) { t.Run("Test registration as existing player, no skin verification", ts.testRegistrationExistingPlayerNoVerification) } - { - // Registration as existing player allowed, skin verification required - ts := setupRegistrationExistingPlayerTS(true, false) - defer ts.Teardown() - - t.Run("Test registration as existing player, with skin verification", ts.testRegistrationExistingPlayerWithVerification) - } - { - // Invite required, new player - ts := &TestSuite{} - - config := testConfig() - config.RegistrationNewPlayer.RequireInvite = true - ts.Setup(config) - defer ts.Teardown() - - t.Run("Test registration as new player, invite only", ts.testRegistrationNewPlayerInvite) - } - { - // Invite required, existing player, skin verification - ts := setupRegistrationExistingPlayerTS(true, true) - defer ts.Teardown() - - t.Run("Test registration as existing player, with skin verification, invite only", ts.testRegistrationExistingPlayerInvite) - } + // { + // // Registration as existing player allowed, skin verification required + // ts := setupRegistrationExistingPlayerTS(true, false) + // defer ts.Teardown() + // + // t.Run("Test registration as existing player, with skin verification", ts.testRegistrationExistingPlayerWithVerification) + // } + // { + // // Invite required, new player + // ts := &TestSuite{} + // + // config := testConfig() + // config.RegistrationNewPlayer.RequireInvite = true + // ts.Setup(config) + // defer ts.Teardown() + // + // t.Run("Test registration as new player, invite only", ts.testRegistrationNewPlayerInvite) + // } + // { + // // Invite required, existing player, skin verification + // ts := setupRegistrationExistingPlayerTS(true, true) + // defer ts.Teardown() + // + // t.Run("Test registration as existing player, with skin verification, invite only", ts.testRegistrationExistingPlayerInvite) + // } } func (ts *TestSuite) testRateLimit(t *testing.T) { @@ -313,21 +326,14 @@ func (ts *TestSuite) testRegistrationNewPlayer(t *testing.T) { // Get the profile { - // TODO use ts.Get here - req := httptest.NewRequest(http.MethodGet, "/web/profile", nil) - req.AddCookie(browserTokenCookie) - rec = httptest.NewRecorder() - ts.Server.ServeHTTP(rec, req) + rec := ts.Get(t, ts.Server, "/web/user", []http.Cookie{*browserTokenCookie}, nil) assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, "", getErrorMessage(rec)) } // Get admin page { - req := httptest.NewRequest(http.MethodGet, "/web/admin", nil) - req.AddCookie(browserTokenCookie) - rec = httptest.NewRecorder() - ts.Server.ServeHTTP(rec, req) + rec := ts.Get(t, ts.Server, "/web/admin", []http.Cookie{*browserTokenCookie}, nil) assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, "", getErrorMessage(rec)) } @@ -350,10 +356,7 @@ func (ts *TestSuite) testRegistrationNewPlayer(t *testing.T) { assert.False(t, user.IsAdmin) // Getting admin page should fail and redirect back to / - req := httptest.NewRequest(http.MethodGet, "/web/admin", nil) - req.AddCookie(browserTokenCookie) - rec = httptest.NewRecorder() - ts.Server.ServeHTTP(rec, req) + rec = ts.Get(t, ts.Server, "/web/admin", []http.Cookie{*browserTokenCookie}, nil) assert.Equal(t, http.StatusSeeOther, rec.Code) assert.Equal(t, "You are not an admin.", getErrorMessage(rec)) assert.Equal(t, ts.App.FrontEndURL, rec.Header().Get("Location")) @@ -370,14 +373,15 @@ func (ts *TestSuite) testRegistrationNewPlayer(t *testing.T) { } { // Test case insensitivity: try registering again with the "same" - // username, but uppercase + // username, but uppercase. Usernames are case-sensitive, but player + // names are. form := url.Values{} form.Set("username", usernameAUppercase) form.Set("password", TEST_PASSWORD) form.Set("returnUrl", ts.App.FrontEndURL+"/web/registration") rec := ts.PostForm(t, ts.Server, "/web/register", form, nil, nil) - ts.registrationShouldFail(t, rec, "That username is taken.", returnURL) + ts.registrationShouldFail(t, rec, "That username is in use as the name of another user's player.", returnURL) } { // Registration with a too-long username should fail @@ -400,7 +404,7 @@ func (ts *TestSuite) testRegistrationNewPlayer(t *testing.T) { ts.registrationShouldFail(t, rec, "Invalid password: can't be blank", returnURL) } { - // Registration from an existing account should fail + // Registration from an existing player should fail form := url.Values{} form.Set("username", usernameC) form.Set("password", TEST_PASSWORD) @@ -409,7 +413,7 @@ func (ts *TestSuite) testRegistrationNewPlayer(t *testing.T) { form.Set("returnUrl", returnURL) rec := ts.PostForm(t, ts.Server, "/web/register", form, nil, nil) - ts.registrationShouldFail(t, rec, "Registration from an existing account is not allowed.", returnURL) + ts.registrationShouldFail(t, rec, "Registration from an existing player is not allowed.", returnURL) } } @@ -446,14 +450,16 @@ func (ts *TestSuite) testRegistrationNewPlayerChosenUUID(t *testing.T) { form.Set("returnUrl", ts.App.FrontEndURL+"/web/registration") rec := ts.PostForm(t, ts.Server, "/web/register", form, nil, nil) - // Registration should succeed, grant a browserToken, and redirect to profile + // Registration should succeed, grant a browserToken, and redirect to user page assert.NotEqual(t, "", getCookie(rec, "browserToken")) ts.registrationShouldSucceed(t, rec) - // Check that the user has been created with the UUID + // Check that the user has been created and has a player with the chosen UUID var user User - result := ts.App.DB.First(&user, "uuid = ?", uuid) + result := ts.App.DB.First(&user, "username = ?", usernameA) assert.Nil(t, result.Error) + assert.Equal(t, 1, len(user.Players)) + assert.Equal(t, uuid, user.Players[0].UUID) } { // Try registering again with the same UUID @@ -558,7 +564,7 @@ func (ts *TestSuite) solveSkinChallenge(t *testing.T, username string) *http.Coo assert.Nil(t, err) var auxUser User - result := ts.AuxApp.DB.Preload("Players").First(&auxUser, "username = ?", username) + result := ts.AuxApp.DB.First(&auxUser, "username = ?", username) assert.Nil(t, result.Error) // Bypass the controller for setting the skin here, we can test that with the rest of /update @@ -569,7 +575,7 @@ func (ts *TestSuite) solveSkinChallenge(t *testing.T, username string) *http.Coo } func (ts *TestSuite) testRegistrationExistingPlayerInvite(t *testing.T) { - username := EXISTING_USERNAME + username := EXISTING_PLAYER_NAME { // Registration without an invite should fail returnURL := ts.App.FrontEndURL + "/web/registration" @@ -683,8 +689,8 @@ func (ts *TestSuite) testLoginLogout(t *testing.T) { assert.Nil(t, result.Error) assert.Equal(t, *UnmakeNullString(&user.BrowserToken), browserTokenCookie.Value) - // Get profile - req := httptest.NewRequest(http.MethodGet, "/web/profile", nil) + // Get user page + req := httptest.NewRequest(http.MethodGet, "/web/user", nil) req.AddCookie(browserTokenCookie) rec = httptest.NewRecorder() ts.Server.ServeHTTP(rec, req) @@ -708,12 +714,12 @@ func (ts *TestSuite) testLoginLogout(t *testing.T) { ts.loginShouldFail(t, rec, "Incorrect password!") } { - // GET /profile without valid BrowserToken should fail - req := httptest.NewRequest(http.MethodGet, "/web/profile", nil) + // GET /web/user without valid BrowserToken should fail + req := httptest.NewRequest(http.MethodGet, "/web/user", nil) rec := httptest.NewRecorder() ts.Server.ServeHTTP(rec, req) assert.Equal(t, http.StatusSeeOther, rec.Code) - assert.Equal(t, ts.App.FrontEndURL, rec.Header().Get("Location")) + assert.Equal(t, ts.App.FrontEndURL+"?destination=%2Fweb%2Fuser", rec.Header().Get("Location")) assert.Equal(t, "You are not logged in.", getErrorMessage(rec)) // Logout without valid BrowserToken should fail @@ -725,7 +731,7 @@ func (ts *TestSuite) testLoginLogout(t *testing.T) { } func (ts *TestSuite) testRegistrationExistingPlayerNoVerification(t *testing.T) { - username := EXISTING_USERNAME + username := EXISTING_PLAYER_NAME returnURL := ts.App.FrontEndURL + "/web/registration" // Register from the existing account @@ -737,14 +743,19 @@ func (ts *TestSuite) testRegistrationExistingPlayerNoVerification(t *testing.T) rec := ts.PostForm(t, ts.Server, "/web/register", form, nil, nil) ts.registrationShouldSucceed(t, rec) - // Check that the user has been created with the same UUID - var auxUser User - result := ts.AuxApp.DB.First(&auxUser, "username = ?", username) + // Check that the new user was created and has a player with the same UUID + // as the player on the auxiliary server + var auxPlayer Player + result := ts.AuxApp.DB.First(&auxPlayer, "name = ?", username) assert.Nil(t, result.Error) + var user User result = ts.App.DB.First(&user, "username = ?", username) assert.Nil(t, result.Error) - assert.Equal(t, auxUser.UUID, user.UUID) + assert.Equal(t, 1, len(user.Players)) + player := user.Players[0] + assert.Equal(t, auxPlayer.UUID, player.UUID) + { // Registration as a new user should fail form := url.Values{} @@ -752,7 +763,7 @@ func (ts *TestSuite) testRegistrationExistingPlayerNoVerification(t *testing.T) form.Set("password", TEST_PASSWORD) form.Set("returnUrl", returnURL) rec := ts.PostForm(t, ts.Server, "/web/register", form, nil, nil) - ts.registrationShouldFail(t, rec, "Registration without some existing account is not allowed.", returnURL) + ts.registrationShouldFail(t, rec, "Registration without some existing player is not allowed.", returnURL) } { // Registration with a missing existing account should fail @@ -768,7 +779,7 @@ func (ts *TestSuite) testRegistrationExistingPlayerNoVerification(t *testing.T) } func (ts *TestSuite) testRegistrationExistingPlayerWithVerification(t *testing.T) { - username := EXISTING_USERNAME + username := EXISTING_PLAYER_NAME returnURL := ts.App.FrontEndURL + "/web/registration" { // Registration without setting a skin should fail @@ -782,7 +793,7 @@ func (ts *TestSuite) testRegistrationExistingPlayerWithVerification(t *testing.T } { // Get challenge skin with invalid username should fail - req := httptest.NewRequest(http.MethodGet, "/web/challenge-skin?username=AReallyReallyReallyLongUsername&returnUrl="+ts.App.FrontEndURL+"/web/registration", nil) + req := httptest.NewRequest(http.MethodGet, "/web/register-challenge?username=AReallyReallyReallyLongUsername&returnUrl="+ts.App.FrontEndURL+"/web/registration", nil) rec := httptest.NewRecorder() ts.Server.ServeHTTP(rec, req) assert.Equal(t, http.StatusSeeOther, rec.Code) @@ -816,13 +827,16 @@ func (ts *TestSuite) testRegistrationExistingPlayerWithVerification(t *testing.T ts.registrationShouldSucceed(t, rec) // Check that the user has been created with the same UUID + var auxPlayer Player + result := ts.AuxApp.DB.First(&auxPlayer, "name = ?", username) + assert.Nil(t, result.Error) + var user User - result := ts.App.DB.First(&user, "username = ?", username) + result = ts.App.DB.First(&user, "username = ?", username) assert.Nil(t, result.Error) - var auxUser User - result = ts.AuxApp.DB.First(&auxUser, "username = ?", username) - assert.Nil(t, result.Error) - assert.Equal(t, auxUser.UUID, user.UUID) + assert.Equal(t, 1, len(user.Players)) + player := user.Players[0] + assert.Equal(t, auxPlayer.UUID, player.UUID) } } } @@ -867,12 +881,94 @@ func (ts *TestSuite) testNewInviteDeleteInvite(t *testing.T) { assert.Equal(t, 0, len(invites)) } -func (ts *TestSuite) testUpdate(t *testing.T) { - username := "testUpdate" - takenUsername := "testUpdateTaken" +func (ts *TestSuite) testUserUpdate(t *testing.T) { + username := "userUpdate" + takenUsername := "userUpdateTaken" user, browserTokenCookie := ts.CreateTestUser(ts.App, ts.Server, username) takenUser, takenBrowserTokenCookie := ts.CreateTestUser(ts.App, ts.Server, takenUsername) + assert.Equal(t, "en", user.PreferredLanguage) + user.IsAdmin = true + assert.Nil(t, ts.App.DB.Save(&user).Error) + + { + // Successful update + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + assert.Nil(t, writer.WriteField("preferredLanguage", "es")) + assert.Nil(t, writer.WriteField("password", "newpassword")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/user")) + + rec := ts.PostMultipart(t, ts.Server, "/web/update-user", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldSucceed(t, rec) + + var updatedUser User + result := ts.App.DB.First(&updatedUser, "uuid = ?", user.UUID) + assert.Nil(t, result.Error) + assert.Equal(t, "es", updatedUser.PreferredLanguage) + + // Make sure we can log in with the new password + form := url.Values{} + form.Set("username", username) + form.Set("password", "newpassword") + form.Set("returnUrl", ts.App.FrontEndURL+"/web/registration") + rec = ts.PostForm(t, ts.Server, "/web/login", form, nil, nil) + ts.loginShouldSucceed(t, rec) + browserTokenCookie = getCookie(rec, "browserToken") + } + { + // As an admin, test updating another user's account + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", takenUser.UUID)) + assert.Nil(t, writer.WriteField("preferredLanguage", "es")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/user")) + assert.Nil(t, writer.Close()) + rec := ts.PostMultipart(t, ts.Server, "/web/update-user", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldSucceed(t, rec) + } + { + // Non-admin should not be able to edit another user + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", user.UUID)) + assert.Nil(t, writer.WriteField("preferredLanguage", "es")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/user")) + assert.Nil(t, writer.Close()) + rec := ts.PostMultipart(t, ts.Server, "/web/update-user", body, writer, []http.Cookie{*takenBrowserTokenCookie}, nil) + ts.updateUserShouldFail(t, rec, "You are not an admin.", ts.App.FrontEndURL) + } + { + // Invalid preferred language should fail + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("preferredLanguage", "xx")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/user")) + assert.Nil(t, writer.Close()) + rec := ts.PostMultipart(t, ts.Server, "/web/update-user", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldFail(t, rec, "Invalid preferred language.", ts.App.FrontEndURL+"/web/user") + } + { + // Setting an invalid password should fail + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("password", "short")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/user")) + assert.Nil(t, writer.Close()) + rec := ts.PostMultipart(t, ts.Server, "/web/update-user", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldFail(t, rec, "Invalid password: password must be longer than 8 characters", ts.App.FrontEndURL+"/web/user") + } +} + +func (ts *TestSuite) testPlayerUpdate(t *testing.T) { + playerName := "playerUpdate" + takenPlayerName := "pUpdateTaken" + user, browserTokenCookie := ts.CreateTestUser(ts.Server, playerName) + player := user.Players[0] + takenUser, takenBrowserTokenCookie := ts.CreateTestUser(ts.Server, takenPlayerName) + takenPlayer := takenUser.Players[0] + sum := blake3.Sum256(RED_SKIN) redSkinHash := hex.EncodeToString(sum[:]) @@ -888,10 +984,11 @@ func (ts *TestSuite) testUpdate(t *testing.T) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("playerName", "newTestUpdate")) - assert.Nil(t, writer.WriteField("fallbackPlayer", "newTestUpdate")) - assert.Nil(t, writer.WriteField("preferredLanguage", "es")) - assert.Nil(t, writer.WriteField("password", "newpassword")) + newPlayerName := "newPlayerUpdate" + + assert.Nil(t, writer.WriteField("uuid", player.UUID)) + assert.Nil(t, writer.WriteField("playerName", newPlayerName)) + assert.Nil(t, writer.WriteField("fallbackPlayer", newPlayerName)) assert.Nil(t, writer.WriteField("skinModel", "slim")) skinFileField, err := writer.CreateFormFile("skinFile", "redSkin.png") assert.Nil(t, err) @@ -903,78 +1000,71 @@ func (ts *TestSuite) testUpdate(t *testing.T) { _, err = capeFileField.Write(RED_CAPE) assert.Nil(t, err) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldSucceed(t, rec) + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldSucceed(t, rec, player.UUID) - var updatedUser User - result := ts.App.DB.First(&updatedUser, "player_name = ?", "newTestUpdate") + var updatedPlayer Player + result := ts.App.DB.First(&updatedPlayer, "name = ?", newPlayerName) assert.Nil(t, result.Error) - assert.Equal(t, "es", updatedUser.PreferredLanguage) - assert.Equal(t, "slim", updatedUser.Players[0].SkinModel) - assert.Equal(t, redSkinHash, *UnmakeNullString(&updatedUser.Players[0].SkinHash)) - assert.Equal(t, redCapeHash, *UnmakeNullString(&updatedUser.Players[0].CapeHash)) - - // Make sure we can log in with the new password - form := url.Values{} - form.Set("username", username) - form.Set("password", "newpassword") - form.Set("returnUrl", ts.App.FrontEndURL+"/web/registration") - rec = ts.PostForm(t, ts.Server, "/web/login", form, nil, nil) - ts.loginShouldSucceed(t, rec) - browserTokenCookie = getCookie(rec, "browserToken") + assert.Equal(t, "slim", updatedPlayer.SkinModel) + assert.Equal(t, redSkinHash, *UnmakeNullString(&updatedPlayer.SkinHash)) + assert.Equal(t, redCapeHash, *UnmakeNullString(&updatedPlayer.CapeHash)) } { - // As an admin, test updating another user's profile + // As an admin, test updating another user's player body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("uuid", takenUser.UUID)) - assert.Nil(t, writer.WriteField("preferredLanguage", "es")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("uuid", takenPlayer.UUID)) + assert.Nil(t, writer.WriteField("skinModel", "slim")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+takenPlayer.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldSucceed(t, rec) + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldSucceed(t, rec, takenPlayer.UUID) } { - // Non-admin should not be able to edit another user's profile + // Non-admin should not be able to edit another user's player body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("uuid", user.UUID)) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("preferredLanguage", "es")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*takenBrowserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "You are not an admin.", ts.App.FrontEndURL) + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*takenBrowserTokenCookie}, nil) + ts.updatePlayerShouldFail(t, rec, "Can't update a player belonging to another user unless you're an admin.", ts.App.FrontEndURL+"/web/player/"+player.UUID) } { // Deleting skin should succeed body := &bytes.Buffer{} writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("deleteSkin", "on")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldSucceed(t, rec) - var updatedUser User - result := ts.App.DB.First(&updatedUser, "username = ?", username) + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldSucceed(t, rec, player.UUID) + + var updatedPlayer Player + result := ts.App.DB.First(&updatedPlayer, "uuid = ?", player.UUID) assert.Nil(t, result.Error) - assert.Nil(t, UnmakeNullString(&updatedUser.Players[0].SkinHash)) - assert.NotNil(t, UnmakeNullString(&updatedUser.Players[0].CapeHash)) - assert.Nil(t, ts.App.SetSkinAndSave(&updatedUser.Players[0], bytes.NewReader(RED_SKIN))) + assert.Nil(t, UnmakeNullString(&updatedPlayer.SkinHash)) + assert.NotNil(t, UnmakeNullString(&updatedPlayer.CapeHash)) + assert.Nil(t, ts.App.SetSkinAndSave(&updatedPlayer, bytes.NewReader(RED_SKIN))) } { // Deleting cape should succeed body := &bytes.Buffer{} writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("deleteCape", "on")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldSucceed(t, rec) - var updatedUser User - result := ts.App.DB.Preload("Players").First(&updatedUser, "username = ?", username) - updatedPlayer := updatedUser.Players[0] + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldSucceed(t, rec, player.UUID) + + var updatedPlayer Player + result := ts.App.DB.First(&updatedPlayer, "uuid = ?", player.UUID) assert.Nil(t, result.Error) assert.Nil(t, UnmakeNullString(&updatedPlayer.CapeHash)) assert.NotNil(t, UnmakeNullString(&updatedPlayer.SkinHash)) @@ -984,11 +1074,12 @@ func (ts *TestSuite) testUpdate(t *testing.T) { // Invalid player name should fail body := &bytes.Buffer{} writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("playerName", "AReallyReallyReallyLongUsername")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Invalid player name: can't be longer than 16 characters", ts.App.FrontEndURL+"/web/profile") + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldFail(t, rec, "Invalid player name: can't be longer than 16 characters", ts.App.FrontEndURL+"/web/player/"+player.UUID) } { // Setting a skin from URL should fail for non-admin (config.AllowTextureFromURL is false by default) @@ -1005,51 +1096,36 @@ func (ts *TestSuite) testUpdate(t *testing.T) { // Invalid fallback player should fail body := &bytes.Buffer{} writer := multipart.NewWriter(body) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("fallbackPlayer", "521759201-invalid-uuid-057219")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Invalid fallback player: not a valid player name or UUID", ts.App.FrontEndURL+"/web/profile") + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldFail(t, rec, "Invalid fallback player: not a valid player name or UUID", ts.App.FrontEndURL+"/web/player/"+player.UUID) } { - // Invalid preferred language should fail + // Changing to a taken player name should fail body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("preferredLanguage", "xx")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) + assert.Nil(t, writer.WriteField("playerName", takenPlayerName)) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Invalid preferred language.", ts.App.FrontEndURL+"/web/profile") - } - { - // Changing to a taken username should fail - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("playerName", takenUsername)) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) - assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "That player name is taken.", ts.App.FrontEndURL+"/web/profile") - } - { - // Setting an invalid password should fail - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("password", "short")) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) - assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Invalid password: password must be longer than 8 characters", ts.App.FrontEndURL+"/web/profile") + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldFail(t, rec, "That player name is taken.", ts.App.FrontEndURL+"/web/player/"+player.UUID) } } func (ts *TestSuite) testUpdateSkinsCapesNotAllowed(t *testing.T) { - username := "updateNoSkinCape" - _, browserTokenCookie := ts.CreateTestUser(ts.App, ts.Server, username) + playerName := "updateNoSkinCape" + user, browserTokenCookie := ts.CreateTestUser(ts.App, ts.Server, playerName) + player := user.Players[0] + { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.WriteField("skinModel", "classic")) skinFileField, err := writer.CreateFormFile("skinFile", "redSkin.png") assert.Nil(t, err) @@ -1057,33 +1133,32 @@ func (ts *TestSuite) testUpdateSkinsCapesNotAllowed(t *testing.T) { assert.Nil(t, err) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Setting a skin is not allowed.", ts.App.FrontEndURL+"/web/profile") + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldFail(t, rec, "Setting a skin texture is not allowed.", ts.App.FrontEndURL+"/web/player/"+player.UUID) - // The user should not have a skin set - var user User - result := ts.App.DB.First(&user, "username = ?", username) + // The player should not have a skin set + result := ts.App.DB.First(&player, "uuid = ?", player.UUID) assert.Nil(t, result.Error) assert.Nil(t, UnmakeNullString(&user.Players[0].SkinHash)) } { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) capeFileField, err := writer.CreateFormFile("capeFile", "redCape.png") assert.Nil(t, err) _, err = capeFileField.Write(RED_CAPE) assert.Nil(t, err) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldFail(t, rec, "Setting a cape is not allowed.", ts.App.FrontEndURL+"/web/profile") + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updateUserShouldFail(t, rec, "Setting a cape texture is not allowed.", ts.App.FrontEndURL+"/web/player/"+player.UUID) - // The user should not have a cape set - var user User - result := ts.App.DB.First(&user, "username = ?", username) + // The player should not have a cape set + result := ts.App.DB.First(&player, "uuid = ?", player.UUID) assert.Nil(t, result.Error) - assert.Nil(t, UnmakeNullString(&user.Players[0].CapeHash)) + assert.Nil(t, UnmakeNullString(&player.CapeHash)) } } @@ -1105,14 +1180,15 @@ func (ts *TestSuite) testTextureFromURL(t *testing.T) { skinURL, err := ts.AuxApp.SkinURL(skinHash) assert.Nil(t, err) + assert.Nil(t, writer.WriteField("uuid", player.UUID)) assert.Nil(t, writer.WriteField("skinUrl", skinURL)) - assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/profile")) + assert.Nil(t, writer.WriteField("returnUrl", ts.App.FrontEndURL+"/web/player/"+player.UUID)) assert.Nil(t, writer.Close()) - rec := ts.PostMultipart(t, ts.Server, "/web/update", body, writer, []http.Cookie{*browserTokenCookie}, nil) - ts.updateShouldSucceed(t, rec) + rec := ts.PostMultipart(t, ts.Server, "/web/update-player", body, writer, []http.Cookie{*browserTokenCookie}, nil) + ts.updatePlayerShouldSucceed(t, rec, player.UUID) - assert.Nil(t, ts.App.DB.First(&user, "username = ?", username).Error) - assert.Equal(t, skinHash, *UnmakeNullString(&user.Players[0].SkinHash)) + assert.Nil(t, ts.App.DB.First(&player, "uuid = ?", player.UUID).Error) + assert.Equal(t, skinHash, *UnmakeNullString(&player.SkinHash)) } func (ts *TestSuite) testDeleteAccount(t *testing.T) { @@ -1122,7 +1198,7 @@ func (ts *TestSuite) testDeleteAccount(t *testing.T) { ts.CreateTestUser(ts.App, ts.Server, usernameA) { var user User - result := ts.App.DB.Preload("Players").First(&user, "username = ?", usernameA) + result := ts.App.DB.First(&user, "username = ?", usernameA) assert.Nil(t, result.Error) player := user.Players[0] @@ -1137,7 +1213,7 @@ func (ts *TestSuite) testDeleteAccount(t *testing.T) { // Check that usernameB has been created var otherUser User - result = ts.App.DB.Preload("Players").First(&otherUser, "username = ?", usernameB) + result = ts.App.DB.First(&otherUser, "username = ?", usernameB) assert.Nil(t, result.Error) otherPlayer := user.Players[0] @@ -1174,7 +1250,7 @@ func (ts *TestSuite) testDeleteAccount(t *testing.T) { // Check that usernameB has been created var otherUser User - result := ts.App.DB.Preload("Players").First(&otherUser, "username = ?", usernameB) + result := ts.App.DB.First(&otherUser, "username = ?", usernameB) assert.Nil(t, result.Error) otherPlayer := otherUser.Players[0] diff --git a/main.go b/main.go index 06d38c6..8c0267a 100644 --- a/main.go +++ b/main.go @@ -81,12 +81,14 @@ func makeRateLimiter(app *App) echo.MiddlewareFunc { Skipper: func(c echo.Context) bool { switch c.Path() { case "/", + "/web/create-player", "/web/delete-user", "/web/delete-player", "/web/login", "/web/logout", "/web/register", - "/web/update": + "/web/update-user", + "/web/update-player": return false default: return true diff --git a/model.go b/model.go index fc1c246..3c06812 100644 --- a/model.go +++ b/model.go @@ -370,7 +370,7 @@ type User struct { MaxPlayerCount int } -func (user *User) BeforeDelete(tx *gorm.DB) (err error) { +func (user *User) BeforeDelete(tx *gorm.DB) error { var players []Player if err := tx.Where("user_uuid = ?", user.UUID).Find(&players).Error; err != nil { return err @@ -381,6 +381,10 @@ func (user *User) BeforeDelete(tx *gorm.DB) (err error) { return nil } +func (user *User) AfterFind(tx *gorm.DB) error { + return tx.Find(&user.Players, "user_uuid = ?", user.UUID).Error +} + type Player struct { UUID string `gorm:"primaryKey"` Name string `gorm:"unique;not null;type:text collate nocase"` @@ -408,6 +412,10 @@ func (player *Player) BeforeDelete(tx *gorm.DB) (err error) { return nil } +func (player *Player) AfterFind(tx *gorm.DB) error { + return tx.Find(&player.Clients, "player_uuid = ?", player.UUID).Error +} + type Client struct { UUID string `gorm:"primaryKey"` ClientToken string diff --git a/player.go b/player.go index 67bdd1d..a6b669d 100644 --- a/player.go +++ b/player.go @@ -28,8 +28,8 @@ func (app *App) getTexture( callerIsAdmin := caller != nil && caller.IsAdmin if textureReader != nil || textureURL != nil { - allowed := true - if textureType == TextureTypeCape { + allowed := false + if textureType == TextureTypeSkin { allowed = app.Config.AllowSkins } else if textureType == TextureTypeCape { allowed = app.Config.AllowCapes @@ -95,7 +95,7 @@ func (app *App) CreatePlayer( defer tx.Rollback() var user User - if err := tx.Preload("Players").First(&user, "uuid = ?", userUUID).Error; err != nil { + if err := tx.First(&user, "uuid = ?", userUUID).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { return Player{}, NewBadRequestUserError("User not found.") } @@ -200,14 +200,20 @@ func (app *App) CreatePlayer( CreatedAt: time.Now(), NameLastChangedAt: time.Now(), } - user.Players = append(user.Players, player) - - if err := tx.Save(&user).Error; err != nil { + if err := tx.Create(&player).Error; err != nil { if IsErrorUniqueFailedField(err, "players.name") { return Player{}, NewBadRequestUserError("That player name is taken.") } else if IsErrorUniqueFailedField(err, "players.uuid") { return Player{}, NewBadRequestUserError("That UUID is taken.") + } else if IsErrorPlayerNameTakenByUsername(err) { + return Player{}, NewBadRequestUserError("That player name is in use as another user's username.") + } else { + return Player{}, err } + } + + user.Players = append(user.Players, player) + if err := tx.Save(&user).Error; err != nil { return Player{}, err } if err := tx.Commit().Error; err != nil { @@ -326,8 +332,10 @@ func (app *App) UpdatePlayer( err = app.DB.Save(&player).Error if err != nil { - if IsErrorUniqueFailed(err) { + if IsErrorUniqueFailedField(err, "players.name") { return Player{}, NewBadRequestUserError("That player name is taken.") + } else if IsErrorPlayerNameTakenByUsername(err) { + return Player{}, NewBadRequestUserError("That player name is in use as another user's username.") } return Player{}, err } diff --git a/services.go b/services.go index 7619f96..1609e16 100644 --- a/services.go +++ b/services.go @@ -452,7 +452,7 @@ func ServicesNameAvailability(app *App) func(c echo.Context) error { errorMessage := fmt.Sprintf("checkNameAvailability.profileName: %s, checkNameAvailability.profileName: Invalid profile name", err.Error()) return MakeErrorResponse(&c, http.StatusBadRequest, Ptr("CONSTRAINT_VIOLATION"), Ptr(errorMessage)) } - var otherPlayer User + var otherPlayer Player result := app.DB.First(&otherPlayer, "name = ?", playerName) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { diff --git a/services_test.go b/services_test.go index 8d9409a..50e5576 100644 --- a/services_test.go +++ b/services_test.go @@ -43,7 +43,7 @@ func TestServices(t *testing.T) { // Set the red skin on the aux user var user User - assert.Nil(t, ts.AuxApp.DB.Preload("Players").First(&user, "username = ?", TEST_USERNAME).Error) + assert.Nil(t, ts.AuxApp.DB.First(&user, "username = ?", TEST_USERNAME).Error) player := user.Players[0] assert.Nil(t, ts.AuxApp.SetSkinAndSave(&player, bytes.NewReader(RED_SKIN))) @@ -339,7 +339,7 @@ func (ts *TestSuite) testServicesChangeName(t *testing.T) { assert.Equal(t, newName, response.Name) // New name should be in the database - assert.Nil(t, ts.App.DB.First(player, "uuid = ?", player.UUID).Error) + assert.Nil(t, ts.App.DB.First(&player, "uuid = ?", player.UUID).Error) assert.Equal(t, newName, player.Name) // Change it back diff --git a/user.go b/user.go index 9c138cc..4247688 100644 --- a/user.go +++ b/user.go @@ -205,8 +205,22 @@ func (app *App) CreateUser( return User{}, err } + tx := app.DB.Begin() + defer tx.Rollback() + + if err := tx.Create(&user).Error; err != nil { + if IsErrorUniqueFailedField(err, "users.username") { + return User{}, NewBadRequestUserError("That username is taken.") + } else if IsErrorUsernameTakenByPlayerName(err) { + return User{}, NewBadRequestUserError("That username is in use as the name of another user's player.") + } else { + return User{}, err + } + } + player := Player{ UUID: playerUUID, + UserUUID: user.UUID, Clients: []Client{}, Name: *playerName, OfflineUUID: offlineUUID, @@ -219,23 +233,21 @@ func (app *App) CreateUser( } user.Players = append(user.Players, player) - tx := app.DB.Begin() - defer tx.Rollback() - - result := tx.Create(&user) - if result.Error != nil { - if IsErrorUniqueFailedField(result.Error, "users.username") { - return User{}, NewBadRequestUserError("That username is taken.") - } else if IsErrorUniqueFailedField(result.Error, "users.uuid") { + if err := tx.Create(&player).Error; err != nil { + if IsErrorUniqueFailedField(err, "players.name") { + return User{}, NewBadRequestUserError("That player name is taken.") + } else if IsErrorUniqueFailedField(err, "players.uuid") { return User{}, NewBadRequestUserError("That UUID is taken.") + } else if IsErrorPlayerNameTakenByUsername(err) { + return User{}, NewBadRequestUserError("That player name is in use as another user's username.") + } else { + return User{}, err } - return User{}, result.Error } if invite != nil { - result = tx.Delete(invite) - if result.Error != nil { - return User{}, result.Error + if err := tx.Delete(invite).Error; err != nil { + return User{}, err } } @@ -351,7 +363,11 @@ func (app *App) DeleteUser(user *User) error { oldSkinHashes := make([]*string, 0, len(user.Players)) oldCapeHashes := make([]*string, 0, len(user.Players)) - for _, player := range user.Players { + var players []Player + if err := app.DB.Where("user_uuid = ?", user.UUID).Find(&players).Error; err != nil { + return err + } + for _, player := range players { oldSkinHashes = append(oldSkinHashes, UnmakeNullString(&player.SkinHash)) oldCapeHashes = append(oldCapeHashes, UnmakeNullString(&player.CapeHash)) }