diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c4a40e..68af508 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,14 +6,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - xxxx-xx-xx ### Added +- Allow to remove a skin without removing all user information - New StatsD metrics: - Counters: - `ely.skinsystem.{hostname}.app.profiles.request` +### Fixed +- Adjusted Mojang usernames filter to be stickier according to their docs + ### Changed - Bumped Go version to 1.21. ### Removed +- Removed mentioning and processing of skin uploading as a file, as this functionality was never implemented and was not planned to be implemented - StatsD metrics: - Gauges: - `ely.skinsystem.{hostname}.app.redis.pool.available` diff --git a/README.md b/README.md index 50081a1..2bdf03f 100644 --- a/README.md +++ b/README.md @@ -328,10 +328,9 @@ You can obtain token by executing `docker-compose run --rm app token`. #### `POST /api/skins` -> **Warning**: skin uploading via `skin` field is not implemented for now. +Endpoint allows you to create or update skin record for a username. -Endpoint allows you to create or update skin record for a username. To upload skin, you have to send multipart -form data. `form-urlencoded` also supported, but, as you may know, it doesn't support files uploading. +The request body must be encoded as `application/x-www-form-urlencoded`. **Request params:** @@ -345,8 +344,9 @@ form data. `form-urlencoded` also supported, but, as you may know, it doesn't su | isSlim | bool | Does skin have slim arms (Alex model). | | mojangTextures | string | Mojang textures field. It must be a base64 encoded json string. Not required. | | mojangSignature | string | Signature for Mojang textures, which is required when `mojangTextures` passed. | -| url | string | Actual url of the skin. You have to pass this parameter or `skin`. | -| skin | file | Skin file. You have to pass this parameter or `url`. | +| url | string | Actual url of the skin. | + +**Important**: all parameters are always read at least as their default values. So, if you only want to update the username and not pass the skin data it will reset all skin information. If you want to keep the data, you should always pass the full set of parameters. If successful you'll receive `201` status code. In the case of failure there will be `400` status code and errors list as json: diff --git a/db/redis/redis.go b/db/redis/redis.go index 235e84f..e801ecf 100644 --- a/db/redis/redis.go +++ b/db/redis/redis.go @@ -73,6 +73,12 @@ func findByUsername(ctx context.Context, conn radix.Conn, username string) (*mod return nil, err } + // Some old data causing issues in the production. + // TODO: remove after investigation will be finished + if skin.Uuid == "" { + return nil, nil + } + skin.OldUsername = skin.Username return skin, nil diff --git a/http/api.go b/http/api.go index a01bcdc..321eb87 100644 --- a/http/api.go +++ b/http/api.go @@ -19,14 +19,6 @@ const UUID_ANY = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$ var regexUuidAny = regexp.MustCompile(UUID_ANY) func init() { - govalidator.AddCustomRule("skinUploadingNotAvailable", func(field string, rule string, message string, value interface{}) error { - if message == "" { - message = "Skin uploading is temporary unavailable" - } - - return errors.New(message) - }) - // Add ability to validate any possible uuid form govalidator.AddCustomRule("uuid_any", func(field string, rule string, message string, value interface{}) error { str := value.(string) @@ -163,50 +155,41 @@ func (ctx *Api) findIdentityOrCleanup(identityId int, username string) (*model.S } func validatePostSkinRequest(request *http.Request) map[string][]string { - const maxMultipartMemory int64 = 32 << 20 - const oneOfSkinOrUrlMessage = "One of url or skin should be provided, but not both" - - _ = request.ParseMultipartForm(maxMultipartMemory) + _ = request.ParseForm() validationRules := govalidator.MapData{ - "identityId": {"required", "numeric", "min:1"}, - "username": {"required"}, - "uuid": {"required", "uuid_any"}, - "skinId": {"required", "numeric", "min:1"}, - "url": {"url"}, - "file:skin": {"ext:png", "size:24576", "mime:image/png"}, - "is1_8": {"bool"}, - "isSlim": {"bool"}, + "identityId": {"required", "numeric", "min:1"}, + "username": {"required"}, + "uuid": {"required", "uuid_any"}, + "skinId": {"required", "numeric"}, + "url": {}, + "is1_8": {"bool"}, + "isSlim": {"bool"}, + "mojangTextures": {}, + "mojangSignature": {}, } - shouldAppendSkinRequiredError := false url := request.Form.Get("url") - _, _, skinErr := request.FormFile("skin") - if (url != "" && skinErr == nil) || (url == "" && skinErr != nil) { - shouldAppendSkinRequiredError = true - } else if skinErr == nil { - validationRules["file:skin"] = append(validationRules["file:skin"], "skinUploadingNotAvailable") - } else if url != "" { + if url == "" { + validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:0,0") + } else { + validationRules["url"] = append(validationRules["url"], "url") + validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:1,") validationRules["is1_8"] = append(validationRules["is1_8"], "required") validationRules["isSlim"] = append(validationRules["isSlim"], "required") } mojangTextures := request.Form.Get("mojangTextures") if mojangTextures != "" { - validationRules["mojangSignature"] = []string{"required"} + validationRules["mojangSignature"] = append(validationRules["mojangSignature"], "required") } validator := govalidator.New(govalidator.Options{ Request: request, Rules: validationRules, RequiredDefault: false, - FormSize: maxMultipartMemory, }) validationResults := validator.Validate() - if shouldAppendSkinRequiredError { - validationResults["url"] = append(validationResults["url"], oneOfSkinOrUrlMessage) - validationResults["skin"] = append(validationResults["skin"], oneOfSkinOrUrlMessage) - } if len(validationResults) != 0 { return validationResults diff --git a/http/api_test.go b/http/api_test.go index 52484c1..b07a459 100644 --- a/http/api_test.go +++ b/http/api_test.go @@ -6,7 +6,6 @@ import ( "errors" "io" "io/ioutil" - "mime/multipart" "net/http" "net/http/httptest" "net/url" @@ -136,6 +135,41 @@ var postSkinTestsCases = []*postSkinTestCase{ suite.Empty(body) }, }, + { + Name: "Update exists identity by changing textures data to empty", + Form: bytes.NewBufferString(url.Values{ + "identityId": {"1"}, + "username": {"mock_username"}, + "uuid": {"0f657aa8-bfbe-415d-b700-5750090d3af3"}, + "skinId": {"0"}, + "is1_8": {"0"}, + "isSlim": {"0"}, + "url": {""}, + "mojangTextures": {""}, + "mojangSignature": {""}, + }.Encode()), + BeforeTest: func(suite *apiTestSuite) { + suite.SkinsRepository.On("FindSkinByUserId", 1).Return(createSkinModel("mock_username", false), nil) + suite.SkinsRepository.On("SaveSkin", mock.MatchedBy(func(model *model.Skin) bool { + suite.Equal(1, model.UserId) + suite.Equal("mock_username", model.Username) + suite.Equal("0f657aa8-bfbe-415d-b700-5750090d3af3", model.Uuid) + suite.Equal(0, model.SkinId) + suite.False(model.Is1_8) + suite.False(model.IsSlim) + suite.Equal("", model.Url) + suite.Equal("", model.MojangTextures) + suite.Equal("", model.MojangSignature) + + return true + })).Times(1).Return(nil) + }, + AfterTest: func(suite *apiTestSuite, response *http.Response) { + suite.Equal(201, response.StatusCode) + body, _ := io.ReadAll(response.Body) + suite.Equal("", string(body)) + }, + }, { Name: "Update exists identity by changing its identityId", Form: bytes.NewBufferString(url.Values{ @@ -271,7 +305,7 @@ func (suite *apiTestSuite) TestPostSkin() { "skinId": [ "The skinId field is required", "The skinId field must be numeric", - "The skinId field must be minimum 1 char" + "The skinId field must be numeric value between 0 and 0" ], "username": [ "The username field is required" @@ -280,54 +314,12 @@ func (suite *apiTestSuite) TestPostSkin() { "The uuid field is required", "The uuid field must contain valid UUID" ], - "url": [ - "One of url or skin should be provided, but not both" - ], - "skin": [ - "One of url or skin should be provided, but not both" - ], "mojangSignature": [ "The mojangSignature field is required" ] } }`, string(body)) }) - - suite.RunSubTest("Upload textures with skin as file", func() { - inputBody := &bytes.Buffer{} - writer := multipart.NewWriter(inputBody) - - part, _ := writer.CreateFormFile("skin", "char.png") - _, _ = part.Write(loadSkinFile()) - - _ = writer.WriteField("identityId", "1") - _ = writer.WriteField("username", "mock_user") - _ = writer.WriteField("uuid", "0f657aa8-bfbe-415d-b700-5750090d3af3") - _ = writer.WriteField("skinId", "5") - - err := writer.Close() - if err != nil { - panic(err) - } - - req := httptest.NewRequest("POST", "http://chrly/skins", inputBody) - req.Header.Add("Content-Type", writer.FormDataContentType()) - w := httptest.NewRecorder() - - suite.App.Handler().ServeHTTP(w, req) - - resp := w.Result() - defer resp.Body.Close() - suite.Equal(400, resp.StatusCode) - responseBody, _ := ioutil.ReadAll(resp.Body) - suite.JSONEq(`{ - "errors": { - "skin": [ - "Skin uploading is temporary unavailable" - ] - } - }`, string(responseBody)) - }) } /************************************** diff --git a/http/skinsystem.go b/http/skinsystem.go index 041c209..ab94995 100644 --- a/http/skinsystem.go +++ b/http/skinsystem.go @@ -6,7 +6,6 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" - "github.com/elyby/chrly/utils" "io" "net/http" "strings" @@ -16,6 +15,7 @@ import ( "github.com/elyby/chrly/api/mojang" "github.com/elyby/chrly/model" + "github.com/elyby/chrly/utils" ) var timeNow = time.Now @@ -275,12 +275,7 @@ func (ctx *Skinsystem) getProfile(request *http.Request, proxy bool) (*profile, } profile := &profile{ - Id: "", - Username: "", - Textures: &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding - CapeFile: nil, - MojangTextures: "", - MojangSignature: "", + Textures: &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding } if skin != nil { @@ -288,7 +283,7 @@ func (ctx *Skinsystem) getProfile(request *http.Request, proxy bool) (*profile, profile.Username = skin.Username } - if skin != nil && skin.SkinId != 0 { + if skin != nil && skin.Url != "" { profile.Textures.Skin = &mojang.SkinTexturesResponse{ Url: skin.Url, } @@ -350,6 +345,8 @@ func (ctx *Skinsystem) getProfile(request *http.Request, proxy bool) (*profile, profile.Id = mojangProfile.Id profile.Username = mojangProfile.Name } + } else if profile.Id != "" { + return profile, nil } else { return nil, nil } diff --git a/model/skin.go b/model/skin.go index 95254fd..c97e3b9 100644 --- a/model/skin.go +++ b/model/skin.go @@ -4,7 +4,7 @@ type Skin struct { UserId int `json:"userId"` Uuid string `json:"uuid"` Username string `json:"username"` - SkinId int `json:"skinId"` + SkinId int `json:"skinId"` // deprecated Url string `json:"url"` Is1_8 bool `json:"is1_8"` IsSlim bool `json:"isSlim"` diff --git a/mojangtextures/mojang_textures.go b/mojangtextures/mojang_textures.go index c092169..0e153d9 100644 --- a/mojangtextures/mojang_textures.go +++ b/mojangtextures/mojang_textures.go @@ -60,8 +60,8 @@ func (c *broadcaster) BroadcastAndRemove(username string, result *broadcastResul delete(c.listeners, username) } -// https://help.mojang.com/customer/portal/articles/928638 -var allowedUsernamesRegex = regexp.MustCompile(`^[\w_]{3,16}$`) +// https://help.minecraft.net/hc/en-us/articles/4408950195341#h_01GE5JX1Z0CZ833A7S54Y195KV +var allowedUsernamesRegex = regexp.MustCompile(`(?i)^[0-9a-z_]{3,16}$`) type UUIDsProvider interface { GetUuid(username string) (*mojang.ProfileInfo, error)