Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check https before allowing https redirect setting #12628

Merged
merged 3 commits into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/* @flow */

import React, { Component } from "react";
import { t } from "ttag";

import SettingToggle from "./SettingToggle";

type Props = {
onChange: (value: any) => void,
setting: {},
settingValues: { "site-url": string },
};

type State = {
status: string,
};

const VERIFIED = "verified";
const CHECKING = "checking";
const NOT_CHECKED = "not_checked";
const FAILED = "failed";

export default class HttpsOnlyWidget extends Component {
props: Props;
state: State;

constructor(props: Props) {
super(props);
this.state = {
status: NOT_CHECKED,
};
}

checkHttps() {
const req = new XMLHttpRequest();
req.timeout = 10000; // don't make the user wait >10s
req.addEventListener("load", () => this.setState({ status: VERIFIED }));
req.addEventListener("error", () => this.setState({ status: FAILED }));
req.open("GET", this.props.settingValues["site-url"] + "/api/health");
this.setState({ status: CHECKING });
req.send();
}

componentDidMount() {
this.checkHttps();
}

componentDidUpdate(prevProps: Props) {
if (
prevProps.settingValues["site-url"] !==
this.props.settingValues["site-url"]
) {
this.checkHttps();
}
}

render() {
const { status } = this.state;
return (
<div>
{status === VERIFIED ? (
<SettingToggle {...this.props} />
) : status === CHECKING ? (
t`Checking HTTPS...`
) : status === FAILED ? (
t`It looks like HTTPS is not properly configured`
) : null // NOT_CHECKED
}
</div>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* @flow */

import React, { Component } from "react";

import InputWithSelectPrefix from "metabase/components/InputWithSelectPrefix";

type Props = {
onChange: (value: any) => void,
setting: { value: string },
};

export default class SiteUrlWidget extends Component {
props: Props;

render() {
const { setting, onChange } = this.props;
return (
<InputWithSelectPrefix
value={setting.value}
onChange={e => onChange(e.target.value)}
prefixes={["https://", "http://"]}
defaultPrefix="http://"
caseInsensitivePrefix={true}
/>
);
}
}
6 changes: 5 additions & 1 deletion frontend/src/metabase/admin/settings/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { createSelector } from "reselect";
import MetabaseSettings from "metabase/lib/settings";
import { t } from "ttag";
import CustomGeoJSONWidget from "./components/widgets/CustomGeoJSONWidget";
import SiteUrlWidget from "./components/widgets/SiteUrlWidget";
import HttpsOnlyWidget from "./components/widgets/HttpsOnlyWidget";
import {
PublicLinksDashboardListing,
PublicLinksQuestionListing,
Expand Down Expand Up @@ -48,12 +50,14 @@ const SECTIONS = updateSectionsWithPlugins({
key: "site-url",
display_name: t`Site URL`,
type: "string",
widget: SiteUrlWidget,
},
{
key: "redirect-all-requests-to-https",
display_name: t`Redirect to HTTPS`,
type: "boolean",
note: t`This value only takes effect if Site URL is HTTPS`,
getHidden: ({ "site-url": url }) => !/^https:\/\//.test(url),
widget: HttpsOnlyWidget,
},
{
key: "admin-email",
Expand Down
91 changes: 91 additions & 0 deletions frontend/src/metabase/components/InputWithSelectPrefix.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/* @flow */

import React, { Component } from "react";

import Select, { Option } from "./Select";
import InputBlurChange from "./InputBlurChange";

type Props = {
onChange: (value: any) => void,
value: string,
prefixes: string[],
defaultPrefix: string,
caseInsensitivePrefix?: boolean,
};

type State = {
prefix: string,
rest: string,
};

export default class InputWithSelectPrefix extends Component {
props: Props;
state: State;

constructor(props: Props) {
super(props);
this.state = { prefix: "", rest: "" };
}

componentDidMount() {
this.setPrefixAndRestFromValue();
}

setPrefixAndRestFromValue() {
const {
value,
prefixes,
defaultPrefix,
caseInsensitivePrefix = false,
} = this.props;
if (value) {
const prefix = prefixes.find(
caseInsensitivePrefix
? p => value.toLowerCase().startsWith(p.toLowerCase())
: p => value.startsWith(p),
);
this.setState(
prefix
? { prefix, rest: value.slice(prefix.length) }
: { prefix: defaultPrefix, rest: value },
);
}
}

componentDidUpdate(prevProps: Props, prevState: State) {
const { prefix, rest } = this.state;
if (prevState.rest !== rest || prevState.prefix !== prefix) {
const value = prefix + rest;
this.props.onChange({ target: { value } });
}
if (prevProps.value !== this.props.value) {
this.setPrefixAndRestFromValue();
}
}

render() {
const { prefixes, defaultPrefix } = this.props;
const { prefix, rest } = this.state;
return (
<div className="flex align-stretch SettingsInput Form-input p0">
<Select
className="border-right"
value={prefix || defaultPrefix}
onChange={e => this.setState({ prefix: e.target.value })}
buttonProps={{ className: "borderless" }}
>
{prefixes.map(p => (
<Option value={p}>{p}</Option>
))}
</Select>
<InputBlurChange
type="text"
className="Form-input flex-full borderless"
value={rest}
placeholder={"foo"}
onBlurChange={e => this.setState({ rest: e.target.value })}
/>
</div>
);
}
}
5 changes: 5 additions & 0 deletions frontend/src/metabase/components/Select.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ export default class Select extends Component {
// PopoverWithTrigger props
isInitiallyOpen: PropTypes.bool,

// SelectButton props
buttonProps: PropTypes.object,

// AccordianList props
searchProp: PropTypes.string,
searchCaseInsensitive: PropTypes.bool,
Expand Down Expand Up @@ -156,6 +159,7 @@ export default class Select extends Component {

render() {
const {
buttonProps,
className,
placeholder,
searchProp,
Expand All @@ -179,6 +183,7 @@ export default class Select extends Component {
<SelectButton
className="full-width"
hasValue={selectedNames.length > 0}
{...buttonProps}
>
{selectedNames.length > 0
? selectedNames.map((name, index) => (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { signInAsAdmin, restore, openOrdersTable } from "__support__/cypress";
import {
signInAsAdmin,
restore,
popover,
openOrdersTable,
} from "__support__/cypress";

describe("scenarios > admin > settings", () => {
before(restore);
Expand Down Expand Up @@ -37,6 +42,57 @@ describe("scenarios > admin > settings", () => {
cy.wait("@saveSettings");
});

it("should check for working https before enabling a redirect", () => {
cy.visit("/admin/settings/general");
cy.server();
cy.route("GET", "**/api/health", "ok").as("httpsCheck");

// settings have loaded, but there's no redirect setting visible
cy.contains("Site URL");
cy.contains("Redirect to HTTPS").should("not.exist");

// switch site url to use https
cy.contains("Site URL")
.parent()
.parent()
.find(".AdminSelect")
.click();
popover()
.contains("https://")
.click();

cy.wait("@httpsCheck");
cy.contains("Redirect to HTTPS")
.parent()
.parent()
.contains("Disabled");

restore(); // avoid leaving https site url
});

it("should display an error if the https redirect check fails", () => {
cy.visit("/admin/settings/general");
cy.server();
// return 500 on https check
cy.route({ method: "GET", url: "**/api/health", status: 500 }).as(
"httpsCheck",
);

// switch site url to use https
cy.contains("Site URL")
.parent()
.parent()
.find(".AdminSelect")
.click();
popover()
.contains("https://")
.click();

cy.wait("@httpsCheck");
cy.contains("It looks like HTTPS is not properly configured");
restore(); // avoid leaving https site url
});

it("should update the formatting", () => {
cy.server();
cy.route("PUT", "**/custom-formatting").as("saveFormatting");
Expand Down
Loading