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)

changes/27454-validation

@@ -0,0 +1 @@
+- Accept any string for the Fleet web URL

frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx

@@ -29,23 +29,6 @@ describe("FleetDetails - form", () => {
     ).toBeInTheDocument();
   });
 
-  it("validates the fleet web address field starts with https://", 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" }));
-
-    expect(handleSubmitSpy).not.toHaveBeenCalled();
-    expect(
-      screen.getByText("Fleet web address must start with https://")
-    ).toBeInTheDocument();
-  });
-
   it("submits the form when valid", async () => {
     const { user } = renderWithSetup(
       <FleetDetails handleSubmit={handleSubmitSpy} currentPage />

frontend/components/forms/RegistrationForm/FleetDetails/helpers.js

@@ -1,15 +1,22 @@
-import { size, startsWith } from "lodash";
+import { size } from "lodash";
+
+import validUrl from "components/forms/validators/valid_url";
 
 const validate = (formData) => {
   const errors = {};
   const { server_url: fleetWebAddress } = 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"],
+      allowAnyLocalHost: true,
+    })
+  ) {
+    errors.server_url =
+      "Fleet web address must be a valid https, http, or localhost URL";
   }
 
   const valid = !size(errors);

frontend/components/forms/validators/valid_url/valid_url.ts

@@ -6,8 +6,16 @@ interface IValidUrl {
   url: string;
   /**  Validate protocols specified */
   protocols?: ("http" | "https")[];
+  allowAnyLocalHost?: boolean;
 }
 
-export default ({ url, protocols }: IValidUrl): boolean => {
-  return isURL(url, { protocols });
+export default ({
+  url,
+  protocols,
+  allowAnyLocalHost = false,
+}: IValidUrl): boolean => {
+  if (allowAnyLocalHost && url.includes("localhost")) return true;
+  // this function also has a `require_valid_protocol` option, though as called it seems to already validate
+  // that the URL's protocol is one of those specified
+  return isURL(url, { protocols, require_protocol: !!protocols?.length });
 };

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) {

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
@@ -16,9 +17,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"],
+      allowAnyLocalHost: true,
+    })
+  ) {
+    errors.server_url = `${serverURL} is not a valid URL`;
+  }
+
+  return errors;
+};
+
 const WebAddress = ({
   appConfig,
   handleSubmit,
@@ -35,23 +52,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 +107,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 +118,7 @@ const WebAddress = ({
               <Button
                 type="submit"
                 variant="brand"
-                disabled={Object.keys(formErrors).length > 0 || disableChildren}
+                disabled={!!size(formErrors) || disableChildren}
                 className="button-wrap"
                 isLoading={isUpdatingSettings}
               >

server/fleet/errors.go

@@ -627,6 +627,9 @@ const (
 
 	// Labels
 	InvalidLabelSpecifiedErrMsg = "Invalid label name(s):"
+
+	// Config
+	InvalidServerURLMsg = "Server URL must use https or http scheme with valid host, or localhost."
 )
 
 // ConflictError is used to indicate a conflict, such as a UUID conflict in the DB.

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

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,33 @@ 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://")) {
+		// left-append valid scheme to leverage `url.Parse`
+		parsed, err := url.Parse("https://" + urlString)
+		if err != nil {
+			return err
+		}
+		// "localhost" only acceptable host if no valid scheme (protocol) provided.
+		// Hostname() will pull out the substring directly after the left-appended scheme, excluding
+		// port. This has the added benefit of catching an invalid scheme provided (e.g. ftp) and will correctly invalidate
+		if parsed.Hostname() != "localhost" {
+			return errors.New(fleet.InvalidServerURLMsg)
+		}
+		return nil
+	}
+
+	// valid scheme provided
+	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")
+	// host required with scheme if provided
+	if parsed.Host == "" {
+		return errors.New(fleet.InvalidServerURLMsg)
 	}
 
 	return nil