Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 21 Packages Projects Releases Wiki Activity

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)

Browse Source 
## 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>
This commit is contained in:
jacobshandling
2025-03-27 13:56:38 -07:00
committed by GitHub
co-authored by Jacob Shandling
parent 4290dbbd43
commit 748b5bcd51
10 changed files with 133 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/27454-validation
+1
View File
@@ -0,0 +1 @@
- Accept any "http://" or "https://" prefixed Fleet web URL
frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.tests.jsx
+33 -10
View File
@@ -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",
});
});
});
frontend/components/forms/RegistrationForm/FleetDetails/helpers.js
+13 -5
View File
@@ -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);
frontend/components/forms/validators/valid_url/valid_url.ts
+7 -2
View File
@@ -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,
});
};
frontend/docs/patterns.md
+5 -3
View File
@@ -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
+45 -15
View File
@@ -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}
>
frontend/utilities/error_messages.ts
+3
View File
@@ -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;
server/fleet/errors.go
+3
View File
@@ -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.
server/service/appconfig.go
+10 -4
View File
@@ -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
server/service/validation_setup.go
+13 -6
View File
@@ -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