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

Add oauth2 fields behind a feature flag #609

Closed
wants to merge 2 commits into from
Closed
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
1 change: 1 addition & 0 deletions dev/custom.ini
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,7 @@ homepage=true
topnav=true
multi-http=true
synthetics-scenes=true
synthetics-oauth2=true

[navigation.app_standalone_pages]
# WORKS
Expand Down
138 changes: 137 additions & 1 deletion src/components/http/HttpSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from '@grafana/ui';
import { css } from '@emotion/css';
import { useFormContext, Controller, useFieldArray } from 'react-hook-form';
import { HttpVersion, CheckType, HttpRegexValidationType, HttpMethod } from 'types';
import { HttpVersion, CheckType, HttpRegexValidationType, HttpMethod, FeatureName } from 'types';
import { Collapse } from 'components/Collapse';
import {
HTTP_COMPRESSION_ALGO_OPTIONS,
Expand All @@ -33,6 +33,7 @@ import { NameValueInput } from 'components/NameValueInput';
import { validateBearerToken, validateHTTPBody, validateHTTPHeaderName, validateHTTPHeaderValue } from 'validation';
import { GrafanaTheme2 } from '@grafana/data';
import { HorizontalCheckboxField } from 'components/HorizonalCheckboxField';
import { useFeatureFlag } from 'hooks/useFeatureFlag';

const httpVersionOptions = [
{
Expand Down Expand Up @@ -114,6 +115,8 @@ const generateValidStatusCodes = () => {

const validStatusCodes = generateValidStatusCodes();
const REGEX_FIELD_NAME = 'settings.http.regexValidations';
const SCOPES_FIELD_NAME = 'settings.http.oauth2.scopes';
const ENDPOINT_PARAMS_FIELD_NAME = 'settings.http.oauth2.endpointParams';

const getStyles = (theme: GrafanaTheme2) => ({
validationGroup: css`
Expand Down Expand Up @@ -164,11 +167,24 @@ export const HttpSettingsForm = ({ isEditor }: Props) => {

const bearerToken = watch('settings.http.bearerToken');
const basicAuth = watch('settings.http.basicAuth');
const oauth2 = watch('settings.http.oauth2');

const [includeBearerToken, setIncludeBearerToken] = useState(Boolean(bearerToken));
const [includeOauth2, setIncludeOauth2] = useState(Boolean(oauth2));
const [includeBasicAuth, setIncludeBasicAuth] = useState(Boolean(basicAuth));
const {
fields: scopesFields,
append: appendScope,
remove: removeScope,
} = useFieldArray({ control, name: SCOPES_FIELD_NAME });
const {
fields: endpointParamFields,
append: appendEndpointParam,
remove: removeEndpointParam,
} = useFieldArray({ control, name: ENDPOINT_PARAMS_FIELD_NAME });
const { fields, append, remove } = useFieldArray({ control, name: REGEX_FIELD_NAME });
const styles = useStyles2(getStyles);
const { isEnabled: oauth2Enabled } = useFeatureFlag(FeatureName.Oauth2);

return (
<Container>
Expand Down Expand Up @@ -318,7 +334,127 @@ export const HttpSettingsForm = ({ isEditor }: Props) => {
</HorizontalGroup>
)}
</VerticalGroup>
{oauth2Enabled && (
<VerticalGroup spacing="xs">
<HorizontalCheckboxField
label="Use Oauth2"
id="http-settings-oauth2"
disabled={!isEditor}
className={
!includeOauth2
? undefined
: css`
margin-bottom: 1px;
`
}
value={includeOauth2}
onChange={() => setIncludeOauth2(!includeOauth2)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed when testing this my expectation of what this checkbox does versus what it is actually doing don't quite line up. If you tick this, start filling in the form, change your mind and untick it my expectation would be that none of the fields would be sent with the request but I can see that they are still.

It is worth noting, I think it is good to persist the fields in case someone accidentally unticks this and then wants to get back the information they have just added but I think it is a fair expectation that once they submit with this box unticked that any information within the oauth2 fields is lost.

/>
{includeOauth2 && (
<VerticalGroup spacing="xs">
<Field label="Client ID" description="The public identifier for your app">
<Input
{...register('settings.http.oauth2.clientId')}
type="text"
id="http-settings-oauth2-clientId"
placeholder="Client ID"
disabled={!isEditor}
/>
</Field>
<Field label="Client secret" description="The secret known only to the server">
<Input
{...register('settings.http.oauth2.clientSecret')}
type="text"
id="https-settings-http-oauth2-client-secret"
placeholder="Client secret"
disabled={!isEditor}
/>
</Field>
<Field label="Token URL" description="Where to request a token from">
<Input
{...register('settings.http.oauth2.tokenUrl')}
type="text"
placeholder="Token url"
disabled={!isEditor}
/>
</Field>
<Field label="Endpoint params">
<VerticalGroup spacing="sm">
{endpointParamFields.map((field, index) => {
return (
<HorizontalGroup key={field.id}>
<VerticalGroup spacing="xs">
<Label>Name</Label>
<Input
{...register(`${ENDPOINT_PARAMS_FIELD_NAME}.${index}.name`)}
type="text"
placeholder="Endpoint param name"
disabled={!isEditor}
/>
</VerticalGroup>
<VerticalGroup spacing="xs">
<Label>Value</Label>
<Input
{...register(`${ENDPOINT_PARAMS_FIELD_NAME}.${index}.value`)}
type="text"
placeholder="Endpoint param value"
disabled={!isEditor}
/>
</VerticalGroup>
<IconButton name="minus-circle" onClick={() => removeEndpointParam(index)} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a 'Delete' tooltip. (Same as the IconButton below). This button also isn't centred but top aligned.

A screenshot of the Synthetic Monitoring plugin with two input fields and the delete icon is aligned vertically to the top of the inputs.

</HorizontalGroup>
);
})}
<Button
type="button"
icon="plus"
variant="secondary"
size="sm"
disabled={!isEditor}
onClick={() => appendEndpointParam({ name: '', value: '' })}
>
Add endpoint param
</Button>
</VerticalGroup>
</Field>

<Field label="Scopes">
<VerticalGroup spacing="sm">
{scopesFields.map((field, index) => {
return (
<HorizontalGroup key={field.id}>
<Input
{...register(`${SCOPES_FIELD_NAME}.${index}`)}
type="text"
placeholder="Scope"
disabled={!isEditor}
/>
<IconButton name="minus-circle" onClick={() => removeScope(index)} />
</HorizontalGroup>
);
})}

<Button
type="button"
icon="plus"
variant="secondary"
size="sm"
disabled={!isEditor}
onClick={() => {
console.log('appending scope');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left in by mistake? 😃

appendScope('');
}}
>
Add scope
</Button>
</VerticalGroup>
</Field>
</VerticalGroup>
)}
</VerticalGroup>
)}
</Collapse>

<Collapse
label="Validation"
onToggle={() => setShowValidation(!showValidation)}
Expand Down
13 changes: 12 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export interface HttpSettings {
// Authentication
bearerToken?: string;
basicAuth?: BasicAuth;
oauth2Config?: Oauth2Config;

// validations
failIfSSL?: boolean;
Expand All @@ -193,10 +194,19 @@ export interface HttpSettings {
failIfBodyNotMatchesRegexp?: string[];
failIfHeaderMatchesRegexp?: HeaderMatch[];
failIfHeaderNotMatchesRegexp?: HeaderMatch[];

cacheBustingQueryParamName?: string;
}

export interface Oauth2Config {
clientId: string;
clientSecret: string;
tokenURL: string;
scopes?: string[];
endpointParams?: Label[];
// tlsConfig: TLSConfig;
// proxyUrl: string;
}

interface HttpHeaderFormValue {
name: string;
value: string;
Expand Down Expand Up @@ -525,6 +535,7 @@ export enum FeatureName {
UnifiedAlerting = 'ngalert',
MultiHttp = 'multi-http',
Scenes = 'synthetics-scenes',
Oauth2 = 'synthetics-oauth2',
}

export interface UsageValues {
Expand Down