Merge branch 'master' into v5

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`

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
+The request body must be encoded as `application/x-www-form-urlencoded`.
-form data. `form-urlencoded` also supported, but, as you may know, it doesn't support files uploading.
 
 **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`.             |
+| url             | string | Actual url of the skin.                                                        |
-| skin            | file   | Skin file. You have to pass this parameter or `url`.                           |
+
+**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:

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

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
+	_ = request.ParseForm()
-	const oneOfSkinOrUrlMessage = "One of url or skin should be provided, but not both"
-
-	_ = request.ParseMultipartForm(maxMultipartMemory)
 
 	validationRules := govalidator.MapData{
-		"identityId": {"required", "numeric", "min:1"},
+		"identityId":      {"required", "numeric", "min:1"},
-		"username":   {"required"},
+		"username":        {"required"},
-		"uuid":       {"required", "uuid_any"},
+		"uuid":            {"required", "uuid_any"},
-		"skinId":     {"required", "numeric", "min:1"},
+		"skinId":          {"required", "numeric"},
-		"url":        {"url"},
+		"url":             {},
-		"file:skin":  {"ext:png", "size:24576", "mime:image/png"},
+		"is1_8":           {"bool"},
-		"is1_8":      {"bool"},
+		"isSlim":          {"bool"},
-		"isSlim":     {"bool"},
+		"mojangTextures":  {},
+		"mojangSignature": {},
 	}
 
-	shouldAppendSkinRequiredError := false
 	url := request.Form.Get("url")
-	_, _, skinErr := request.FormFile("skin")
+	if url == "" {
-	if (url != "" && skinErr == nil) || (url == "" && skinErr != nil) {
+		validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:0,0")
-		shouldAppendSkinRequiredError = true
+	} else {
-	} else if skinErr == nil {
+		validationRules["url"] = append(validationRules["url"], "url")
-		validationRules["file:skin"] = append(validationRules["file:skin"], "skinUploadingNotAvailable")
+		validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:1,")
-	} else if url != "" {
 		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

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))
-	})
 }
 
 /**************************************

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:              "",
+		Textures: &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding
-		Username:        "",
-		Textures:        &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding
-		CapeFile:        nil,
-		MojangTextures:  "",
-		MojangSignature: "",
 	}
 
 	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
 	}

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"`

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
+// https://help.minecraft.net/hc/en-us/articles/4408950195341#h_01GE5JX1Z0CZ833A7S54Y195KV
-var allowedUsernamesRegex = regexp.MustCompile(`^[\w_]{3,16}$`)
+var allowedUsernamesRegex = regexp.MustCompile(`(?i)^[0-9a-z_]{3,16}$`)
 
 type UUIDsProvider interface {
 	GetUuid(username string) (*mojang.ProfileInfo, error)