refactor(api): extract method configMuxBuilder.postAdjustGui and add test coverage (#9979)

This is extracted from PR #9175. This deduplicates `SetPassword` calls and makes `postAdjustGui` a single place where PR #9175 can add another adjustment step for sanitizing changes to WebAuthn credentials. This also adds tests to validate that the refactored logic was not broken.
2025-03-09 21:31:06 +01:00
parent 435f2d2178
commit 893071d2ba
2 changed files with 141 additions and 15 deletions
@@ -751,6 +751,129 @@ func TestHTTPLogin(t *testing.T) {
 	testWith(false, http.StatusOK, http.StatusOK, "/")
 	testWith(false, http.StatusOK, http.StatusForbidden, "/meta.js")
 	testWith(false, http.StatusNotFound, http.StatusForbidden, "/any-path/that/does/nooooooot/match-any/noauth-pattern")
+
+	t.Run("Password change invalidates old and enables new password", func(t *testing.T) {
+		t.Parallel()
+
+		// This test needs a longer-than-default shutdown timeout to finish saving
+		// config changes when running on GitHub Actions
+		shutdownTimeout := time.Second
+
+		initConfig := func(password string, t *testing.T) config.Wrapper {
+			gui := config.GUIConfiguration{
+				RawAddress: "127.0.0.1:0",
+				APIKey:     testAPIKey,
+				User:       "user",
+			}
+			if err := gui.SetPassword(password); err != nil {
+				t.Fatal(err, "Failed to set initial password")
+			}
+			cfg := config.Configuration{
+				GUI: gui,
+			}
+
+			tmpFile, err := os.CreateTemp("", "syncthing-testConfig-Password-*")
+			if err != nil {
+				t.Fatal(err, "Failed to create tmpfile for test")
+			}
+			w := config.Wrap(tmpFile.Name(), cfg, protocol.LocalDeviceID, events.NoopLogger)
+			tmpFile.Close()
+			cfgCtx, cfgCancel := context.WithCancel(context.Background())
+			go w.Serve(cfgCtx)
+			t.Cleanup(func() {
+				os.Remove(tmpFile.Name())
+				cfgCancel()
+			})
+			return w
+		}
+
+		initialPassword := "Asdf123!"
+		newPassword := "123!asdF"
+
+		t.Run("when done via /rest/config", func(t *testing.T) {
+			t.Parallel()
+
+			w := initConfig(initialPassword, t)
+			{
+				baseURL, cancel, err := startHTTPWithShutdownTimeout(w, shutdownTimeout)
+				cfgPath := baseURL + "/rest/config"
+				path := baseURL + "/meta.js"
+				t.Cleanup(cancel)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				resp := httpGetBasicAuth(path, "user", initialPassword)
+				if resp.StatusCode != http.StatusOK {
+					t.Fatalf("Unexpected non-200 return code %d for auth with initial password", resp.StatusCode)
+				}
+
+				cfg := w.RawCopy()
+				cfg.GUI.Password = newPassword
+				httpRequest(http.MethodPut, cfgPath, cfg, "", "", testAPIKey, "", "", "", nil, t)
+			}
+			{
+				baseURL, cancel, err := startHTTP(w)
+				path := baseURL + "/meta.js"
+				t.Cleanup(cancel)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				resp := httpGetBasicAuth(path, "user", initialPassword)
+				if resp.StatusCode != http.StatusForbidden {
+					t.Errorf("Unexpected non-403 return code %d for request authed with old password", resp.StatusCode)
+				}
+
+				resp2 := httpGetBasicAuth(path, "user", newPassword)
+				if resp2.StatusCode != http.StatusOK {
+					t.Errorf("Unexpected non-200 return code %d for request authed with new password", resp2.StatusCode)
+				}
+			}
+		})
+
+		t.Run("when done via /rest/config/gui", func(t *testing.T) {
+			t.Parallel()
+
+			w := initConfig(initialPassword, t)
+			{
+				baseURL, cancel, err := startHTTPWithShutdownTimeout(w, shutdownTimeout)
+				cfgPath := baseURL + "/rest/config/gui"
+				path := baseURL + "/meta.js"
+				t.Cleanup(cancel)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				resp := httpGetBasicAuth(path, "user", initialPassword)
+				if resp.StatusCode != http.StatusOK {
+					t.Fatalf("Unexpected non-200 return code %d for auth with initial password", resp.StatusCode)
+				}
+
+				cfg := w.RawCopy()
+				cfg.GUI.Password = newPassword
+				httpRequest(http.MethodPut, cfgPath, cfg.GUI, "", "", testAPIKey, "", "", "", nil, t)
+			}
+			{
+				baseURL, cancel, err := startHTTP(w)
+				path := baseURL + "/meta.js"
+				t.Cleanup(cancel)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				resp := httpGetBasicAuth(path, "user", initialPassword)
+				if resp.StatusCode != http.StatusForbidden {
+					t.Errorf("Unexpected non-403 return code %d for request authed with old password", resp.StatusCode)
+				}
+
+				resp2 := httpGetBasicAuth(path, "user", newPassword)
+				if resp2.StatusCode != http.StatusOK {
+					t.Errorf("Unexpected non-200 return code %d for request authed with new password", resp2.StatusCode)
+				}
+			}
+		})
+	})
 }

 func TestHtmlFormLogin(t *testing.T) {
@@ -318,13 +318,10 @@ func (c *configMuxBuilder) adjustConfig(w http.ResponseWriter, r *http.Request)
 	var errMsg string
 	var status int
 	waiter, err := c.cfg.Modify(func(cfg *config.Configuration) {
-		if to.GUI.Password != cfg.GUI.Password {
-			if err := to.GUI.SetPassword(to.GUI.Password); err != nil {
-				l.Warnln("hashing password:", err)
-				errMsg = err.Error()
-				status = http.StatusInternalServerError
-				return
-			}
+		if err := c.postAdjustGui(&cfg.GUI, &to.GUI); err != nil {
+			errMsg = err.Error()
+			status = http.StatusInternalServerError
+			return
 		}
 		*cfg = to
 	})
@@ -391,7 +388,6 @@ func (c *configMuxBuilder) adjustOptions(w http.ResponseWriter, r *http.Request,
 }

 func (c *configMuxBuilder) adjustGUI(w http.ResponseWriter, r *http.Request, gui config.GUIConfiguration) {
-	oldPassword := gui.Password
 	err := unmarshalTo(r.Body, &gui)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
@@ -400,13 +396,10 @@ func (c *configMuxBuilder) adjustGUI(w http.ResponseWriter, r *http.Request, gui
 	var errMsg string
 	var status int
 	waiter, err := c.cfg.Modify(func(cfg *config.Configuration) {
-		if gui.Password != oldPassword {
-			if err := gui.SetPassword(gui.Password); err != nil {
-				l.Warnln("hashing password:", err)
-				errMsg = err.Error()
-				status = http.StatusInternalServerError
-				return
-			}
+		if err := c.postAdjustGui(&cfg.GUI, &gui); err != nil {
+			errMsg = err.Error()
+			status = http.StatusInternalServerError
+			return
 		}
 		cfg.GUI = gui
 	})
@@ -419,6 +412,16 @@ func (c *configMuxBuilder) adjustGUI(w http.ResponseWriter, r *http.Request, gui
 	c.finish(w, waiter)
 }

+func (c *configMuxBuilder) postAdjustGui(from *config.GUIConfiguration, to *config.GUIConfiguration) error {
+	if to.Password != from.Password {
+		if err := to.SetPassword(to.Password); err != nil {
+			l.Warnln("hashing password:", err)
+			return err
+		}
+	}
+	return nil
+}
+
 func (c *configMuxBuilder) adjustLDAP(w http.ResponseWriter, r *http.Request, ldap config.LDAPConfiguration) {
 	if err := unmarshalTo(r.Body, &ldap); err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)