implement screen name/password validation

foodgroup/admin.go

@@ -154,18 +154,21 @@ func (s AdminService) InfoChangeRequest(ctx context.Context, sess *state.Session
 
 	// validateProposedName ensures that the name is valid
 	var validateProposedName = func(name state.DisplayScreenName) (ok bool, errorCode uint16) {
-		// proposed name is too long
-		if len(name) > 16 {
+		err := name.ValidateAIMHandle()
+		switch {
+		case errors.Is(err, state.ErrAIMHandleLength):
+			// proposed name is too long
 			return false, wire.AdminInfoErrorInvalidNickNameLength
+		case errors.Is(err, state.ErrAIMHandleInvalidFormat):
+			// character or spacing issues
+			return false, wire.AdminInfoErrorInvalidNickName
 		}
+
 		// proposed name does not match session name (e.g. malicious client)
 		if name.IdentScreenName() != sess.IdentScreenName() {
 			return false, wire.AdminInfoErrorValidateNickName
 		}
-		// proposed name ends in a space
-		if name[len(name)-1] == 32 {
-			return false, wire.AdminInfoErrorInvalidNickName
-		}
+
 		return true, 0
 	}
 

server/http/mgmt_api.go

@@ -201,15 +201,20 @@ func postUserHandler(w http.ResponseWriter, r *http.Request, userManager UserMan
 	}
 
 	sn := state.DisplayScreenName(input.ScreenName)
+
+	if err := sn.ValidateAIMHandle(); err != nil {
+		http.Error(w, fmt.Sprintf("invalid screen name: %s", err), http.StatusBadRequest)
+		return
+	}
+
 	user := state.User{
 		AuthKey:           newUUID().String(),
 		DisplayScreenName: sn,
 		IdentScreenName:   sn.IdentScreenName(),
 	}
 
 	if err := user.HashPassword(input.Password); err != nil {
-		logger.Error("error hashing user password in POST /user", "err", err.Error())
-		http.Error(w, "internal server error", http.StatusInternalServerError)
+		http.Error(w, fmt.Sprintf("invalid password: %s", err), http.StatusBadRequest)
 		return
 	}
 

server/http/mgmt_api_test.go

@@ -149,71 +149,99 @@ func TestUserHandler_GET(t *testing.T) {
 }
 
 func TestUserHandler_POST(t *testing.T) {
+	type insertUserParams struct {
+		user state.User
+		err  error
+	}
 	tt := []struct {
-		name           string
-		body           string
-		UUID           uuid.UUID
-		user           state.User
-		userHandlerErr error
-		want           string
-		statusCode     int
+		name             string
+		body             string
+		UUID             uuid.UUID
+		insertUserParams []insertUserParams
+		want             string
+		statusCode       int
 	}{
 		{
 			name: "with valid user",
 			body: `{"screen_name":"userA", "password":"thepassword"}`,
 			UUID: uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b"),
-			user: func() state.User {
-				user := state.User{
-					AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
-					DisplayScreenName: "userA",
-					IdentScreenName:   state.NewIdentScreenName("userA"),
-				}
-				assert.NoError(t, user.HashPassword("thepassword"))
-				return user
-			}(),
+			insertUserParams: []insertUserParams{
+				{
+					user: func() state.User {
+						user := state.User{
+							AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
+							DisplayScreenName: "userA",
+							IdentScreenName:   state.NewIdentScreenName("userA"),
+						}
+						assert.NoError(t, user.HashPassword("thepassword"))
+						return user
+					}(),
+				},
+			},
 			want:       `User account created successfully.`,
 			statusCode: http.StatusCreated,
 		},
 		{
 			name:       "with malformed body",
 			body:       `{"screen_name":"userA", "password":"thepassword"`,
-			user:       state.User{},
 			want:       `malformed input`,
 			statusCode: http.StatusBadRequest,
 		},
 		{
 			name: "user handler error",
 			body: `{"screen_name":"userA", "password":"thepassword"}`,
 			UUID: uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b"),
-			user: func() state.User {
-				user := state.User{
-					AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
-					DisplayScreenName: "userA",
-					IdentScreenName:   state.NewIdentScreenName("userA"),
-				}
-				assert.NoError(t, user.HashPassword("thepassword"))
-				return user
-			}(),
-			userHandlerErr: io.EOF,
-			want:           `internal server error`,
-			statusCode:     http.StatusInternalServerError,
+			insertUserParams: []insertUserParams{
+				{
+					user: func() state.User {
+						user := state.User{
+							AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
+							DisplayScreenName: "userA",
+							IdentScreenName:   state.NewIdentScreenName("userA"),
+						}
+						assert.NoError(t, user.HashPassword("thepassword"))
+						return user
+					}(),
+					err: io.EOF,
+				},
+			},
+			want:       `internal server error`,
+			statusCode: http.StatusInternalServerError,
 		},
 		{
 			name: "duplicate user",
 			body: `{"screen_name":"userA", "password":"thepassword"}`,
 			UUID: uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b"),
-			user: func() state.User {
-				user := state.User{
-					AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
-					DisplayScreenName: "userA",
-					IdentScreenName:   state.NewIdentScreenName("userA"),
-				}
-				assert.NoError(t, user.HashPassword("thepassword"))
-				return user
-			}(),
-			userHandlerErr: state.ErrDupUser,
-			want:           `user already exists`,
-			statusCode:     http.StatusConflict,
+			insertUserParams: []insertUserParams{
+				{
+					user: func() state.User {
+						user := state.User{
+							AuthKey:           uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b").String(),
+							DisplayScreenName: "userA",
+							IdentScreenName:   state.NewIdentScreenName("userA"),
+						}
+						assert.NoError(t, user.HashPassword("thepassword"))
+						return user
+					}(),
+					err: state.ErrDupUser,
+				},
+			},
+			want:       `user already exists`,
+			statusCode: http.StatusConflict,
+		},
+		{
+			name:       "invalid AIM screen name",
+			body:       `{"screen_name":"a", "password":"thepassword"}`,
+			UUID:       uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b"),
+			want:       `invalid screen name: screen name must be between 3 and 16 characters`,
+			statusCode: http.StatusBadRequest,
+		},
+		{
+			name:       "invalid AIM password",
+			body:       `{"screen_name":"userA", "password":"1"}`,
+			UUID:       uuid.MustParse("07c70701-ba68-49a9-9f9b-67a53816e37b"),
+			want:       `invalid password: password length must be between 4-16 characters`,
+			statusCode: http.StatusBadRequest,
 		},
 	}
 
@@ -223,10 +251,10 @@ func TestUserHandler_POST(t *testing.T) {
 			responseRecorder := httptest.NewRecorder()
 
 			userManager := newMockUserManager(t)
-			if tc.user.IdentScreenName.String() != "" {
+			for _, params := range tc.insertUserParams {
 				userManager.EXPECT().
-					InsertUser(tc.user).
-					Return(tc.userHandlerErr)
+					InsertUser(params.user).
+					Return(params.err)
 			}
 
 			newUUID := func() uuid.UUID { return tc.UUID }

state/user.go

@@ -3,7 +3,9 @@ package state
 import (
 	"bytes"
 	"errors"
+	"strconv"
 	"strings"
+	"unicode"
 
 	"github.com/google/uuid"
 
@@ -58,6 +60,50 @@ func NewIdentScreenName(screenName string) IdentScreenName {
 // This includes the original casing and spacing as defined by the user.
 type DisplayScreenName string
 
+var (
+	ErrAIMHandleLength        = errors.New("screen name must be between 3 and 16 characters")
+	ErrAIMHandleInvalidFormat = errors.New("screen name must start with a letter, cannot end with a space, and must contain only letters, numbers, and spaces")
+	ErrICQUINInvalidFormat    = errors.New("uin must be a number in the range 10000-2147483646")
+)
+
+// ValidateAIMHandle returns an error if the instance is not a valid AIM screen name.
+// Possible errors:
+//   - ErrAIMHandleLength: if the screen name is not between 3 and 16
+//     characters
+//   - ErrAIMHandleInvalidFormat: if the screen name does not start with a
+//     letter, ends with a space, or contains invalid characters
+func (s DisplayScreenName) ValidateAIMHandle() error {
+	if len(s) < 3 || len(s) > 16 {
+		return ErrAIMHandleLength
+	}
+
+	// Must start with a letter, cannot end with a space,
+	// and must contain only letters, numbers, and spaces
+	if !unicode.IsLetter(rune(s[0])) || s[len(s)-1] == ' ' {
+		return ErrAIMHandleInvalidFormat
+	}
+
+	for _, ch := range s {
+		if !unicode.IsLetter(ch) && !unicode.IsDigit(ch) && ch != ' ' {
+			return ErrAIMHandleInvalidFormat
+		}
+	}
+
+	return nil
+}
+
+// ValidateICQHandle returns an error if the instance is not a valid ICQ UIN.
+// Possible errors:
+//   - ErrICQUINInvalidFormat: if the UIN is not a number or is not in the valid
+//     range
+func (s DisplayScreenName) ValidateICQHandle() error {
+	uin, err := strconv.Atoi(string(s))
+	if err != nil || uin < 10000 || uin > 2147483646 {
+		return ErrICQUINInvalidFormat
+	}
+	return nil
+}
+
 // IdentScreenName converts the DisplayScreenName to an IdentScreenName by applying
 // the normalization process defined in NewIdentScreenName.
 func (s DisplayScreenName) IdentScreenName() IdentScreenName {
@@ -118,7 +164,32 @@ func (u *User) ValidateRoastedPass(roastedPass []byte) bool {
 // HashPassword computes MD5 hashes of the user's password. It computes both
 // weak and strong variants and stores them in the struct.
 func (u *User) HashPassword(passwd string) error {
+	if err := validateAIMPassword(passwd); err != nil {
+		return err
+	}
 	u.WeakMD5Pass = wire.WeakMD5PasswordHash(passwd, u.AuthKey)
 	u.StrongMD5Pass = wire.StrongMD5PasswordHash(passwd, u.AuthKey)
 	return nil
 }
+
+// validateAIMPassword returns an error if the AIM password is invalid.
+// A valid password is 4-16 characters long. The minimum password length is
+// set here for software preservation purposes; operators should set more
+// stringent password requirements.
+func validateAIMPassword(pass string) error {
+	if len(pass) < 4 || len(pass) > 16 {
+		return errors.New("password length must be between 4-16 characters")
+	}
+	return nil
+}
+
+// validateICQPassword returns an error if the ICQ password is invalid.
+// A valid password is 1-8 characters long. The minimum password length is set
+// here for software preservation purposes; operators should set more stringent
+// password requirements.
+func validateICQPassword(pass string) error {
+	if len(pass) < 1 || len(pass) > 8 {
+		return errors.New("password must be between 1 and 8 characters")
+	}
+	return nil
+}