Pull request 2022: 6280-password-length

Updates #6280.

Squashed commit of the following:

commit 85014e2
Merge: 2d93201 5f61b55
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Oct 5 15:58:48 2023 +0300

    Merge branch 'master' into 6280-password-length

commit 2d93201
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Oct 5 15:43:05 2023 +0300

    client: rm dep

commit 3b11d10
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Oct 5 15:22:58 2023 +0300

    client: imp i18n

commit f88dfc9
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Oct 5 15:17:56 2023 +0300

    all: imp i18n, names

commit a7874f5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Oct 5 15:07:10 2023 +0300

    all: fix passwd check

CHANGELOG.md

@@ -29,17 +29,18 @@ NOTE: Add new changes BELOW THIS COMMENT.
 - Ability to specify for how long clients should cache a filtered response,
   using the *Blocked response TTL* field on the *DNS settings* page ([#4569]).
 
-[#1700]: https://github.com/AdguardTeam/AdGuardHome/issues/1700
-[#4569]: https://github.com/AdguardTeam/AdGuardHome/issues/4569
-
 ### Fixed
 
+- Improper validation of password length ([#6280]).
 - Wrong algorithm for filtering self addresses from the list of private upstream
   DNS servers ([#6231]).
 - An accidental change in DNS rewrite priority ([#6226]).
 
+[#1700]: https://github.com/AdguardTeam/AdGuardHome/issues/1700
+[#4569]: https://github.com/AdguardTeam/AdGuardHome/issues/4569
 [#6226]: https://github.com/AdguardTeam/AdGuardHome/issues/6226
 [#6231]: https://github.com/AdguardTeam/AdGuardHome/issues/6231
+[#6280]: https://github.com/AdguardTeam/AdGuardHome/issues/6280
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

client/package.json

@@ -42,7 +42,6 @@
         "redux-actions": "^2.6.5",
         "redux-form": "^8.3.5",
         "redux-thunk": "^2.3.0",
-        "string-length": "^5.0.1",
         "timezones-list": "^3.0.2",
         "url-polyfill": "^1.1.9"
     },

client/src/__locales/en.json

@@ -659,7 +659,7 @@
     "parental_control": "Parental Control",
     "safe_browsing": "Safe Browsing",
     "served_from_cache": "{{value}} <i>(served from cache)</i>",
-    "form_error_password_length": "Password must be at least {{value}} characters long",
+    "form_error_password_length": "Password must be {{min}} to {{max}} characters long",
     "anonymizer_notification": "<0>Note:</0> IP anonymization is enabled. You can disable it in <1>General settings</1>.",
     "confirm_dns_cache_clear": "Are you sure you want to clear DNS cache?",
     "cache_cleared": "DNS cache successfully cleared",

client/src/helpers/constants.js

@@ -27,6 +27,7 @@ export const R_WIN_ABSOLUTE_PATH = /^([a-zA-Z]:)?(\\|\/)(?:[^\\/:*?"<>|\x00]+\\)
 export const R_CLIENT_ID = /^[a-z0-9-]{1,63}$/;
 
 export const MIN_PASSWORD_LENGTH = 8;
+export const MAX_PASSWORD_LENGTH = 72;
 
 export const HTML_PAGES = {
     INSTALL: '/install.html',

client/src/helpers/validators.js

@@ -1,5 +1,4 @@
 import i18next from 'i18next';
-import stringLength from 'string-length';
 
 import {
     MAX_PORT,
@@ -14,6 +13,7 @@ import {
     UNSAFE_PORTS,
     R_CLIENT_ID,
     R_DOMAIN,
+    MAX_PASSWORD_LENGTH,
     MIN_PASSWORD_LENGTH,
 } from './constants';
 import { ip4ToInt, isValidAbsolutePath } from './form';
@@ -325,14 +325,33 @@ export const validateIpv4InCidr = (valueIp, allValues) => {
     return undefined;
 };
 
+/**
+ * @param value {string}
+ * @returns {number}
+ */
+const utf8StringLength = (value) => {
+    const encoder = new TextEncoder();
+    const view = encoder.encode(value);
+
+    return view.length;
+};
+
 /**
  * @param value {string}
  * @returns {Function}
  */
 export const validatePasswordLength = (value) => {
-    if (value && stringLength(value) < MIN_PASSWORD_LENGTH) {
-        return i18next.t('form_error_password_length', { value: MIN_PASSWORD_LENGTH });
+    if (value) {
+        const length = utf8StringLength(value);
+        if (length < MIN_PASSWORD_LENGTH || length > MAX_PASSWORD_LENGTH) {
+            // TODO: Make the i18n clearer with regards to bytes vs. characters.
+            return i18next.t('form_error_password_length', {
+                min: MIN_PASSWORD_LENGTH,
+                max: MAX_PASSWORD_LENGTH,
+            });
+        }
     }
+
     return undefined;
 };
 

internal/home/auth.go

@@ -644,24 +644,27 @@ func optionalAuthHandler(handler http.Handler) http.Handler {
 	return &authHandler{handler}
 }
 
-// UserAdd - add new user
-func (a *Auth) UserAdd(u *webUser, password string) {
+// Add adds a new user with the given password.
+func (a *Auth) Add(u *webUser, password string) (err error) {
 	if len(password) == 0 {
-		return
+		return errors.Error("empty password")
 	}
 
 	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
 	if err != nil {
-		log.Error("bcrypt.GenerateFromPassword: %s", err)
-		return
+		return fmt.Errorf("generating hash: %w", err)
 	}
+
 	u.PasswordHash = string(hash)
 
 	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	a.users = append(a.users, *u)
-	a.lock.Unlock()
 
-	log.Debug("auth: added user: %s", u.Name)
+	log.Debug("auth: added user with login %q", u.Name)
+
+	return nil
 }
 
 // findUser returns a user if there is one.

internal/home/auth_test.go

@@ -47,7 +47,8 @@ func TestAuth(t *testing.T) {
 	s := session{}
 
 	user := webUser{Name: "name"}
-	a.UserAdd(&user, "password")
+	err := a.Add(&user, "password")
+	require.NoError(t, err)
 
 	assert.Equal(t, checkSessionNotFound, a.checkSession("notfound"))
 	a.RemoveSession("notfound")

internal/home/controlinstall.go

@@ -417,6 +417,18 @@ func (web *webAPI) handleInstallConfigure(w http.ResponseWriter, r *http.Request
 	config.DNS.BindHosts = []netip.Addr{req.DNS.IP}
 	config.DNS.Port = req.DNS.Port
 
+	u := &webUser{
+		Name: req.Username,
+	}
+	err = Context.auth.Add(u, req.Password)
+	if err != nil {
+		Context.firstRun = true
+		copyInstallSettings(config, curConfig)
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "%s", err)
+
+		return
+	}
+
 	// TODO(e.burkov): StartMods() should be put in a separate goroutine at the
 	// moment we'll allow setting up TLS in the initial configuration or the
 	// configuration itself will use HTTPS protocol, because the underlying
@@ -430,11 +442,6 @@ func (web *webAPI) handleInstallConfigure(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	u := &webUser{
-		Name: req.Username,
-	}
-	Context.auth.UserAdd(u, req.Password)
-
 	err = config.write()
 	if err != nil {
 		Context.firstRun = true