From cadb89f00a8c24aef161bc61101ab500d4afa216 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 22 Dec 2023 01:56:02 +0100 Subject: [PATCH] Fixes #40. Allow to upload profile information without a skin Remove skin file uploading stubs --- CHANGELOG.md | 2 ++ README.md | 10 +++--- http/api.go | 49 ++++++++++------------------ http/api_test.go | 80 +++++++++++++++++++++------------------------- http/skinsystem.go | 13 +++----- model/skin.go | 2 +- 6 files changed, 65 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66ae8da..e7e675e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ 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` @@ -14,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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 18f8090..8db7f9e 100644 --- a/README.md +++ b/README.md @@ -344,10 +344,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:** @@ -361,8 +360,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/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"`