From 748b5bcd519c5658c0591d5da2b8697eb063aa34 Mon Sep 17 00:00:00 2001
From: jacobshandling <61553566+jacobshandling@users.noreply.github.com>
Date: Thu, 27 Mar 2025 13:56:38 -0700
Subject: [PATCH] Full-stack: Make "Server url" validation conditions
 consistent across Fleet, update Web Address form validation and submission
 logic per Fleet best practices (`frontend/docs/patterns.md`) (#27455)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## For #27454

Consider Fleet web URL to be valid if it:

- (Front end and back end): uses “https://” or “http://” scheme
 and
- (Front end) accepts only valid or "localhost" hosts (e.g., "a.b.cc" or
"localhost", but not "a.b")
- (Back end) accepts any host (e.g., "localhost", "a.b.cc", or even
"a.b")


### Setup flow UI URL validation:

![setup](https://github.com/user-attachments/assets/34a428d2-5731-46f2-b708-c88b790e3667)

### Org settings UI URL validation:

![org-settings](https://github.com/user-attachments/assets/147916c8-9c5b-4ae7-9e14-625c65b42d0a)

### Server URL validation:
<img width="1464" alt="invalid-url-server"
src="https://github.com/user-attachments/assets/83a112e1-6318-4b09-864d-fe66a223835d"
/>

### Invalid Fleet server URL in DB error:

![invalid-url-in-db](https://github.com/user-attachments/assets/aae591fb-6cc3-49bd-8556-22129be4c2c4)


- [x] Changes file added for user-visible changes in `changes/`,
- [x] Added/updated automated tests
- [ ] A detailed QA plan exists on the associated ticket (if it isn't
there, work with the product group's QA engineer to add it)
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Jacob Shandling <jacob@fleetdm.com>
---
 changes/27454-validation                      |  1 +
 .../FleetDetails/FleetDetails.tests.jsx       | 43 +++++++++----
 .../RegistrationForm/FleetDetails/helpers.js  | 18 ++++--
 .../forms/validators/valid_url/valid_url.ts   |  9 ++-
 frontend/docs/patterns.md                     |  8 ++-
 .../cards/WebAddress/WebAddress.tsx           | 60 ++++++++++++++-----
 frontend/utilities/error_messages.ts          |  3 +
 server/fleet/errors.go                        |  3 +
 server/service/appconfig.go                   | 14 +++--
 server/service/validation_setup.go            | 19 ++++--
 10 files changed, 133 insertions(+), 45 deletions(-)
 create mode 100644 changes/27454-validation
 create mode 100644 frontend/utilities/error_messages.ts

diff --git a/changes/27454-validation b/changes/27454-validation
new file mode 100644
index 0000000000..fd08c518e3
--- /dev/null
+++ b/changes/27454-validation
@@ -0,0 +1 @@
+- Accept any "http://" or "https://" prefixed Fleet web URL
diff --git a/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx b/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx
index 7ca5b9249d..e447bfeaf3 100644
--- a/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx
+++ b/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx
@@ -5,6 +5,8 @@ import { renderWithSetup } from "test/test-utils";
 
 import FleetDetails from "components/forms/RegistrationForm/FleetDetails";
 
+import INVALID_SERVER_URL_MESSAGE from "utilities/error_messages";
+
 describe("FleetDetails - form", () => {
   const handleSubmitSpy = jest.fn();
   it("renders", () => {
@@ -29,24 +31,30 @@ describe("FleetDetails - form", () => {
     ).toBeInTheDocument();
   });
 
-  it("validates the fleet web address field starts with https://", async () => {
+  it("validates the Fleet server URL field starts with 'https://' or 'http://'", async () => {
     const { user } = renderWithSetup(
       <FleetDetails handleSubmit={handleSubmitSpy} currentPage />
     );
 
-    await user.type(
-      screen.getByRole("textbox", { name: "Fleet web address" }),
-      "http://gnar.Fleet.co"
-    );
-    await user.click(screen.getByRole("button", { name: "Next" }));
+    const inputField = screen.getByRole("textbox", {
+      name: "Fleet web address",
+    });
+    const nextButton = screen.getByRole("button", { name: "Next" });
+
+    await user.type(inputField, "gnar.Fleet.co");
+    await user.click(nextButton);
 
     expect(handleSubmitSpy).not.toHaveBeenCalled();
-    expect(
-      screen.getByText("Fleet web address must start with https://")
-    ).toBeInTheDocument();
+    expect(screen.getByText(INVALID_SERVER_URL_MESSAGE)).toBeInTheDocument();
+
+    await user.type(inputField, "localhost:8080");
+    await user.click(nextButton);
+
+    expect(handleSubmitSpy).not.toHaveBeenCalled();
+    expect(screen.getByText(INVALID_SERVER_URL_MESSAGE)).toBeInTheDocument();
   });
 
-  it("submits the form when valid", async () => {
+  it("submits the form with valid https link", async () => {
     const { user } = renderWithSetup(
       <FleetDetails handleSubmit={handleSubmitSpy} currentPage />
     );
@@ -61,4 +69,19 @@ describe("FleetDetails - form", () => {
       server_url: "https://gnar.Fleet.co",
     });
   });
+  it("submits the form with valid http link", async () => {
+    const { user } = renderWithSetup(
+      <FleetDetails handleSubmit={handleSubmitSpy} currentPage />
+    );
+    // when
+    await user.type(
+      screen.getByRole("textbox", { name: "Fleet web address" }),
+      "http://localhost:8080"
+    );
+    await user.click(screen.getByRole("button", { name: "Next" }));
+    // then
+    expect(handleSubmitSpy).toHaveBeenCalledWith({
+      server_url: "http://localhost:8080",
+    });
+  });
 });
diff --git a/frontend/components/forms/RegistrationForm/FleetDetails/helpers.js b/frontend/components/forms/RegistrationForm/FleetDetails/helpers.js
index f66547dfa7..d1c08acf1c 100644
--- a/frontend/components/forms/RegistrationForm/FleetDetails/helpers.js
+++ b/frontend/components/forms/RegistrationForm/FleetDetails/helpers.js
@@ -1,4 +1,8 @@
-import { size, startsWith } from "lodash";
+import { size } from "lodash";
+
+import validUrl from "components/forms/validators/valid_url";
+
+import INVALID_SERVER_URL_MESSAGE from "utilities/error_messages";
 
 const validate = (formData) => {
   const errors = {};
@@ -6,10 +10,14 @@ const validate = (formData) => {
 
   if (!fleetWebAddress) {
     errors.server_url = "Fleet web address must be completed";
-  }
-
-  if (fleetWebAddress && !startsWith(fleetWebAddress, "https://")) {
-    errors.server_url = "Fleet web address must start with https://";
+  } else if (
+    !validUrl({
+      url: fleetWebAddress,
+      protocols: ["http", "https"],
+      allowLocalHost: true,
+    })
+  ) {
+    errors.server_url = INVALID_SERVER_URL_MESSAGE;
   }
 
   const valid = !size(errors);
diff --git a/frontend/components/forms/validators/valid_url/valid_url.ts b/frontend/components/forms/validators/valid_url/valid_url.ts
index 1707e991f1..4b94cd76ca 100644
--- a/frontend/components/forms/validators/valid_url/valid_url.ts
+++ b/frontend/components/forms/validators/valid_url/valid_url.ts
@@ -6,8 +6,13 @@ interface IValidUrl {
   url: string;
   /**  Validate protocols specified */
   protocols?: ("http" | "https")[];
+  allowLocalHost?: boolean;
 }
 
-export default ({ url, protocols }: IValidUrl): boolean => {
-  return isURL(url, { protocols });
+export default ({ url, protocols, allowLocalHost = false }: IValidUrl) => {
+  return isURL(url, {
+    protocols,
+    require_protocol: !!protocols?.length,
+    require_tld: !allowLocalHost,
+  });
 };
diff --git a/frontend/docs/patterns.md b/frontend/docs/patterns.md
index f417de7482..f17bad4f5e 100644
--- a/frontend/docs/patterns.md
+++ b/frontend/docs/patterns.md
@@ -208,9 +208,10 @@ When building a React-controlled form:
 - Use the native HTML `form` element to wrap the form.
 - Use a `Button` component with `type="submit"` for its submit button.
 - Write a submit handler, e.g. `handleSubmit`, that accepts an `evt:
-React.FormEvent<HTMLFormElement>` argument and, critically, calls `evt.preventDefault()` in its
-body. This prevents the HTML `form`'s default submit behavior from interfering with our custom
+React.FormEvent<HTMLFormElement>` argument and, critically:
+  - calls `evt.preventDefault()` in its body. This prevents the HTML `form`'s default submit behavior from interfering with our custom
 handler's logic.
+  - does nothing (e.g., returns `null`) if the form is in an invalid state, preventing submission by any means.
 - Assign that handler to the `form`'s `onSubmit` property (*not* the submit button's `onClick`)
 
 ### Data validation
@@ -248,7 +249,9 @@ const onInputChange = ({ name, value }: IFormField) => {
   // new errors are only set onBlur
   const errsToSet: Record<string, string> = {};
   Object.keys(formErrors).forEach((k) => {
+    // @ts-ignore
     if (newErrs[k]) {
+      // @ts-ignore
       errsToSet[k] = newErrs[k];
     }
   });
@@ -270,7 +273,6 @@ const onInputBlur = () => {
 ```tsx
 const onFormSubmit = (evt: React.MouseEvent<HTMLFormElement>) => {
   evt.preventDefault();
-
   // return null if there are errors
   const errs = validateFormData(formData);
   if (Object.keys(errs).length > 0) {
diff --git a/frontend/pages/admin/OrgSettingsPage/cards/WebAddress/WebAddress.tsx b/frontend/pages/admin/OrgSettingsPage/cards/WebAddress/WebAddress.tsx
index f18a9b5948..c8f61836be 100644
--- a/frontend/pages/admin/OrgSettingsPage/cards/WebAddress/WebAddress.tsx
+++ b/frontend/pages/admin/OrgSettingsPage/cards/WebAddress/WebAddress.tsx
@@ -1,4 +1,5 @@
 import React, { useState } from "react";
+import { size } from "lodash";
 
 import Button from "components/buttons/Button";
 // @ts-ignore
@@ -7,6 +8,8 @@ import validUrl from "components/forms/validators/valid_url";
 import SectionHeader from "components/SectionHeader";
 import GitOpsModeTooltipWrapper from "components/GitOpsModeTooltipWrapper";
 
+import INVALID_SERVER_URL_MESSAGE from "utilities/error_messages";
+
 import { IAppConfigFormProps, IFormField } from "../constants";
 
 interface IWebAddressFormData {
@@ -16,9 +19,25 @@ interface IWebAddressFormData {
 interface IWebAddressFormErrors {
   server_url?: string | null;
 }
-
 const baseClass = "app-config-form";
 
+const validateFormData = ({ serverURL }: IWebAddressFormData) => {
+  const errors: IWebAddressFormErrors = {};
+  if (!serverURL) {
+    errors.server_url = "Fleet server URL must be present";
+  } else if (
+    !validUrl({
+      url: serverURL,
+      protocols: ["http", "https"],
+      allowLocalHost: true,
+    })
+  ) {
+    errors.server_url = INVALID_SERVER_URL_MESSAGE;
+  }
+
+  return errors;
+};
+
 const WebAddress = ({
   appConfig,
   handleSubmit,
@@ -35,23 +54,34 @@ const WebAddress = ({
   const [formErrors, setFormErrors] = useState<IWebAddressFormErrors>({});
 
   const onInputChange = ({ name, value }: IFormField) => {
-    setFormData({ ...formData, [name]: value });
-    setFormErrors({});
+    const newFormData = { ...formData, [name]: value };
+    setFormData(newFormData);
+    const newErrs = validateFormData(newFormData);
+    // only set errors that are updates of existing errors
+    // new errors are only set onBlur
+    const errsToSet: Record<string, string> = {};
+    Object.keys(formErrors).forEach((k) => {
+      // @ts-ignore
+      if (newErrs[k]) {
+        // @ts-ignore
+        errsToSet[k] = newErrs[k];
+      }
+    });
+    setFormErrors(errsToSet);
   };
 
-  const validateForm = () => {
-    const errors: IWebAddressFormErrors = {};
-    if (!serverURL) {
-      errors.server_url = "Fleet server URL must be present";
-    } else if (!validUrl({ url: serverURL, protocols: ["http", "https"] })) {
-      errors.server_url = `${serverURL} is not a valid URL`;
-    }
-
-    setFormErrors(errors);
+  const onInputBlur = () => {
+    setFormErrors(validateFormData(formData));
   };
 
-  const onFormSubmit = (evt: React.MouseEvent<HTMLFormElement>) => {
+  const onFormSubmit = (evt: React.FormEvent<HTMLFormElement>) => {
     evt.preventDefault();
+    // return null if there are errors
+    const errs = validateFormData(formData);
+    if (size(errs)) {
+      setFormErrors(errs);
+      return;
+    }
 
     // Formatting of API not UI
     const formDataToSubmit = {
@@ -79,7 +109,7 @@ const WebAddress = ({
             name="serverURL"
             value={serverURL}
             parseTarget
-            onBlur={validateForm}
+            onBlur={onInputBlur}
             error={formErrors.server_url}
             tooltip="The base URL of this instance for use in Fleet links."
             disabled={gitOpsModeEnabled}
@@ -90,7 +120,7 @@ const WebAddress = ({
               <Button
                 type="submit"
                 variant="brand"
-                disabled={Object.keys(formErrors).length > 0 || disableChildren}
+                disabled={!!size(formErrors) || disableChildren}
                 className="button-wrap"
                 isLoading={isUpdatingSettings}
               >
diff --git a/frontend/utilities/error_messages.ts b/frontend/utilities/error_messages.ts
new file mode 100644
index 0000000000..e9086678a3
--- /dev/null
+++ b/frontend/utilities/error_messages.ts
@@ -0,0 +1,3 @@
+const INVALID_SERVER_URL_MESSAGE = `Fleet server address must be a valid “https” or “http” URL.`;
+
+export default INVALID_SERVER_URL_MESSAGE;
diff --git a/server/fleet/errors.go b/server/fleet/errors.go
index f04ceba4aa..deffb10ead 100644
--- a/server/fleet/errors.go
+++ b/server/fleet/errors.go
@@ -627,6 +627,9 @@ const (
 
 	// Labels
 	InvalidLabelSpecifiedErrMsg = "Invalid label name(s):"
+
+	// Config
+	InvalidServerURLMsg = `Fleet server URL must use “https” or “http”.`
 )
 
 // ConflictError is used to indicate a conflict, such as a UUID conflict in the DB.
diff --git a/server/service/appconfig.go b/server/service/appconfig.go
index 4745af4999..4800a0d92f 100644
--- a/server/service/appconfig.go
+++ b/server/service/appconfig.go
@@ -417,6 +417,10 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle
 	}
 	if appConfig.ServerSettings.ServerURL == "" {
 		invalid.Append("server_url", "Fleet server URL must be present")
+	} else {
+		if err := ValidateServerURL(appConfig.ServerSettings.ServerURL); err != nil {
+			invalid.Append("server_url", "Couldn't update settings: "+err.Error())
+		}
 	}
 
 	if appConfig.ActivityExpirySettings.ActivityExpiryEnabled && appConfig.ActivityExpirySettings.ActivityExpiryWindow < 1 {
@@ -924,8 +928,8 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle
 }
 
 func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet.AppConfig, oldAppConfig *fleet.AppConfig,
-	appConfig *fleet.AppConfig, invalid *fleet.InvalidArgumentError) (appConfigCAStatus, error) {
-
+	appConfig *fleet.AppConfig, invalid *fleet.InvalidArgumentError,
+) (appConfigCAStatus, error) {
 	var invalidLicense bool
 	fleetLicense, _ := license.FromContext(ctx)
 	if newAppConfig.Integrations.NDESSCEPProxy.Set && newAppConfig.Integrations.NDESSCEPProxy.Valid && !fleetLicense.IsPremium() {
@@ -1249,7 +1253,8 @@ func (svc *Service) populateCustomSCEPChallenges(ctx context.Context, remainingO
 // filterDeletedDigiCertCAs identifies deleted DigiCert integrations in the provided configs.
 // It mutates the provided result to set a deleted status where applicable and returns a list of the remaining (non-deleted) integrations.
 func filterDeletedDigiCertCAs(oldAppConfig *fleet.AppConfig, newAppConfig *fleet.AppConfig,
-	result *appConfigCAStatus) []fleet.DigiCertIntegration {
+	result *appConfigCAStatus,
+) []fleet.DigiCertIntegration {
 	remainingOldCAs := make([]fleet.DigiCertIntegration, 0, len(oldAppConfig.Integrations.DigiCert.Value))
 	for _, oldCA := range oldAppConfig.Integrations.DigiCert.Value {
 		var found bool
@@ -1271,7 +1276,8 @@ func filterDeletedDigiCertCAs(oldAppConfig *fleet.AppConfig, newAppConfig *fleet
 // filterDeletedCustomSCEPCAs identifies deleted custom SCEP integrations in the provided configs.
 // It mutates the provided result to set a deleted status where applicable and returns a list of the remaining (non-deleted) integrations.
 func filterDeletedCustomSCEPCAs(oldAppConfig *fleet.AppConfig, newAppConfig *fleet.AppConfig,
-	result *appConfigCAStatus) []fleet.CustomSCEPProxyIntegration {
+	result *appConfigCAStatus,
+) []fleet.CustomSCEPProxyIntegration {
 	remainingOldCAs := make([]fleet.CustomSCEPProxyIntegration, 0, len(oldAppConfig.Integrations.CustomSCEPProxy.Value))
 	for _, oldCA := range oldAppConfig.Integrations.CustomSCEPProxy.Value {
 		var found bool
diff --git a/server/service/validation_setup.go b/server/service/validation_setup.go
index cb757d7b98..8b4a46d33b 100644
--- a/server/service/validation_setup.go
+++ b/server/service/validation_setup.go
@@ -18,7 +18,7 @@ func (mw validationMiddleware) NewAppConfig(ctx context.Context, payload fleet.A
 	} else {
 		serverURLString = cleanupURL(payload.ServerSettings.ServerURL)
 	}
-	if err := validateServerURL(serverURLString); err != nil {
+	if err := ValidateServerURL(serverURLString); err != nil {
 		invalid.Append("server_url", err.Error())
 	}
 	if invalid.HasErrors() {
@@ -27,14 +27,21 @@ func (mw validationMiddleware) NewAppConfig(ctx context.Context, payload fleet.A
 	return mw.Service.NewAppConfig(ctx, payload)
 }
 
-func validateServerURL(urlString string) error {
-	serverURL, err := url.Parse(urlString)
+func ValidateServerURL(urlString string) error {
+	// TODO - implement more robust URL validation here
+
+	// no valid scheme provided
+	if !(strings.HasPrefix(urlString, "http://") || strings.HasPrefix(urlString, "https://")) {
+		return errors.New(fleet.InvalidServerURLMsg)
+	}
+
+	// valid scheme provided - require host
+	parsed, err := url.Parse(urlString)
 	if err != nil {
 		return err
 	}
-
-	if serverURL.Scheme != "https" && !strings.Contains(serverURL.Host, "localhost") {
-		return errors.New("url scheme must be https")
+	if parsed.Host == "" {
+		return errors.New(fleet.InvalidServerURLMsg)
 	}
 
 	return nil