From b728fc4c09cf580fc0b419131746ff7199dabef7 Mon Sep 17 00:00:00 2001 From: Elinor Date: Wed, 18 May 2022 13:21:23 +0300 Subject: [PATCH 1/6] Fix: Refactor sample queries (#1677) --- .../sample-queries/SampleQueries.spec.tsx | 3 - .../sidebar/sample-queries/SampleQueries.tsx | 472 +++++++----------- .../sample-queries/sample-query-utils.ts | 73 +++ src/types/query-runner.ts | 3 - 4 files changed, 249 insertions(+), 302 deletions(-) diff --git a/src/app/views/sidebar/sample-queries/SampleQueries.spec.tsx b/src/app/views/sidebar/sample-queries/SampleQueries.spec.tsx index 4db44a313..d1ea486d9 100644 --- a/src/app/views/sidebar/sample-queries/SampleQueries.spec.tsx +++ b/src/app/views/sidebar/sample-queries/SampleQueries.spec.tsx @@ -37,9 +37,6 @@ const renderSampleQueries = () => { error: { message: '' } - }, - intl: { - message: messages } } diff --git a/src/app/views/sidebar/sample-queries/SampleQueries.tsx b/src/app/views/sidebar/sample-queries/SampleQueries.tsx index 587e7c18d..a8a4306e3 100644 --- a/src/app/views/sidebar/sample-queries/SampleQueries.tsx +++ b/src/app/views/sidebar/sample-queries/SampleQueries.tsx @@ -4,105 +4,134 @@ import { GroupHeader, IColumn, Icon, IDetailsRowStyles, MessageBar, MessageBarType, SearchBox, SelectionMode, Spinner, SpinnerSize, styled, TooltipHost } from '@fluentui/react'; -import React, { Component } from 'react'; -import { FormattedMessage, injectIntl } from 'react-intl'; -import { connect } from 'react-redux'; -import { bindActionCreators, Dispatch } from 'redux'; +import React, { useEffect, useState } from 'react'; +import { FormattedMessage } from 'react-intl'; +import { useDispatch, useSelector } from 'react-redux'; import { geLocale } from '../../../../appLocale'; -import { componentNames, eventTypes, telemetry } from '../../../../telemetry'; +import { componentNames, telemetry } from '../../../../telemetry'; import { IQuery, ISampleQueriesProps, ISampleQuery } from '../../../../types/query-runner'; import { IRootState } from '../../../../types/root'; -import * as queryActionCreators from '../../../services/actions/query-action-creators'; -import * as queryInputActionCreators from '../../../services/actions/query-input-action-creators'; -import * as queryStatusActionCreators from '../../../services/actions/query-status-action-creator'; -import * as samplesActionCreators from '../../../services/actions/samples-action-creators'; import { GRAPH_URL } from '../../../services/graph-constants'; import { getStyleFor } from '../../../utils/http-methods.utils'; -import { validateExternalLink } from '../../../utils/external-link-validation'; import { generateGroupsFromList } from '../../../utils/generate-groups'; -import { sanitizeQueryUrl } from '../../../utils/query-url-sanitization'; import { substituteTokens } from '../../../utils/token-helpers'; import { classNames } from '../../classnames'; +import { + columns, isJsonString, performSearch, trackDocumentLinkClickedEvent, + trackSampleQueryClickEvent +} from './sample-query-utils'; import { sidebarStyles } from '../Sidebar.styles'; -import { isJsonString } from './sample-query-utils'; import { searchBoxStyles } from '../../../utils/searchbox.styles'; +import { fetchSamples } from '../../../services/actions/samples-action-creators'; +import { setQueryResponseStatus } from '../../../services/actions/query-status-action-creator'; +import { runQuery } from '../../../services/actions/query-action-creators'; +import { setSampleQuery } from '../../../services/actions/query-input-action-creators'; +import { translateMessage } from '../../../utils/translate-messages'; + +const unstyledSampleQueries = (sampleProps?: ISampleQueriesProps): JSX.Element => { + + const [selectedQuery, setSelectedQuery] = useState(null) + const { authToken, profile, samples } = + useSelector((state: IRootState) => state); + const tokenPresent = authToken.token; + const [sampleQueries, setSampleQueries] = useState(samples.queries); + const dispatch = useDispatch(); + const currentTheme = getTheme(); + + const { error, pending } = samples; + const groups = generateGroupsFromList(sampleQueries, 'category'); + + const classProps = { + styles: sampleProps!.styles, + theme: sampleProps!.theme + }; + const classes = classNames(classProps); -export class SampleQueries extends Component { - constructor(props: ISampleQueriesProps) { - super(props); - this.state = { - sampleQueries: [], - selectedQuery: null - }; - } - - public componentDidMount = () => { - const { queries } = this.props.samples; - if (queries && queries.length > 0) { - this.setState({ sampleQueries: queries }); + useEffect(() => { + if (samples.queries.length === 0) { + dispatch(fetchSamples()); } else { - this.props.actions!.fetchSamples(); + setSampleQueries(samples.queries) } + }, [samples.queries, tokenPresent]) + + const searchValueChanged = (_event: any, value?: string): void => { + const { queries } = samples; + const filteredQueries = value ? performSearch(queries, value) : queries; + setSampleQueries(filteredQueries); }; - public componentDidUpdate = (prevProps: ISampleQueriesProps) => { - if (prevProps.samples.queries !== this.props.samples.queries) { - this.setState({ sampleQueries: this.props.samples.queries }); - } + const onDocumentationLinkClicked = (item: ISampleQuery) => { + window.open(item.docLink, '_blank'); + trackDocumentLinkClickedEvent(item); }; - public searchValueChanged = (event: any, value?: string): void => { - const { queries } = this.props.samples; - let sampleQueries = queries; - if (value) { - const keyword = value.toLowerCase(); - sampleQueries = queries.filter((sample: any) => { - const name = sample.humanName.toLowerCase(); - const category = sample.category.toLowerCase(); - return name.includes(keyword) || category.includes(keyword); - }); + const querySelected = (query: ISampleQuery) => { + const queryVersion = query.requestUrl.substring(1, 5); + const sampleQuery: IQuery = { + sampleUrl: GRAPH_URL + query.requestUrl, + selectedVerb: query.method, + sampleBody: query.postBody, + sampleHeaders: query.headers || [], + selectedVersion: queryVersion + }; + substituteTokens(sampleQuery, profile!); + sampleQuery.sampleBody = getSampleBody(sampleQuery); + + if (query.tip) { + displayTipMessage(query); } - this.setState({ sampleQueries }); - }; - public onDocumentationLinkClicked = (item: ISampleQuery) => { - window.open(item.docLink, '_blank'); - this.trackDocumentLinkClickedEvent(item); + if (shouldRunQuery(query)) { + dispatch(runQuery(sampleQuery)); + } + + trackSampleQueryClickEvent(query); + dispatch(setSampleQuery(sampleQuery)); }; - private async trackDocumentLinkClickedEvent(item: ISampleQuery): Promise { - const properties: { [key: string]: any } = { - ComponentName: componentNames.DOCUMENTATION_LINK, - SampleId: item.id, - SampleName: item.humanName, - SampleCategory: item.category, - Link: item.docLink - }; - telemetry.trackEvent(eventTypes.LINK_CLICK_EVENT, properties); + const getSampleBody = (query: IQuery) => { + return query.sampleBody ? parseSampleBody() : undefined; + + function parseSampleBody() { + return isJsonString(query.sampleBody!) + ? JSON.parse(query.sampleBody!) + : query.sampleBody; + } + } + + const displayTipMessage = (query: ISampleQuery) => { + dispatch(setQueryResponseStatus({ + messageType: MessageBarType.warning, + statusText: 'Tip', + status: query.tip + })); + } - // Check if link throws error - validateExternalLink(item.docLink || '', componentNames.DOCUMENTATION_LINK, item.id); + const shouldRunQuery = (query: ISampleQuery) => { + if (query.tip && tokenPresent) { + return false; + } + if (!tokenPresent || query.method === 'GET') { + return true; + } + return false; } - public renderItemColumn = ( + + const renderItemColumn = ( item: ISampleQuery, index: number | undefined, column: IColumn | undefined ) => { - const classes = classNames(this.props); - const { - tokenPresent, - intl: { messages } - }: any = this.props; - if (column) { const queryContent = item[column.fieldName as keyof ISampleQuery] as string; - const signInText = messages['Sign In to try this sample']; + const signInText = translateMessage('Sign In to try this sample'); switch (column.key) { case 'authRequiredIcon': @@ -154,7 +183,7 @@ export class SampleQueries extends Component { > this.onDocumentationLinkClicked(item)} + onClick={() => onDocumentationLinkClicked(item)} className={classes.docLink} style={{ marginRight: '45%', @@ -208,15 +237,12 @@ export class SampleQueries extends Component { ); } } - }; + } - public renderRow = (props: any): any => { - const currentTheme = getTheme(); - const { tokenPresent } = this.props; - const classes = classNames(this.props); + const renderRow = (props: any): any => { let selectionDisabled = false; const customStyles: Partial = {}; - if (this.state.selectedQuery?.id === props.item.id) { + if (selectedQuery?.id === props.item.id) { customStyles.root = { backgroundColor: currentTheme.palette.neutralLight }; } @@ -231,9 +257,10 @@ export class SampleQueries extends Component { styles={customStyles} onClick={() => { if (!selectionDisabled) { - this.querySelected(props.item); + const query: ISampleQuery = props.item!; + querySelected(query); } - this.setState({ selectedQuery: props.item }) + setSelectedQuery(props.item) }} className={ classes.queryRow + @@ -248,68 +275,7 @@ export class SampleQueries extends Component { } }; - private querySelected = (query: any) => { - const { actions, tokenPresent, profile } = this.props; - const selectedQuery = query; - if (!selectedQuery) { - return; - } - - const queryVersion = selectedQuery.requestUrl.substring(1, 5); - const sampleQuery: IQuery = { - sampleUrl: GRAPH_URL + selectedQuery.requestUrl, - selectedVerb: selectedQuery.method, - sampleBody: selectedQuery.postBody, - sampleHeaders: selectedQuery.headers || [], - selectedVersion: queryVersion - }; - - substituteTokens(sampleQuery, profile); - - if (actions) { - if (sampleQuery.selectedVerb === 'GET') { - sampleQuery.sampleBody = JSON.parse('{}'); - if (tokenPresent) { - if (selectedQuery.tip) { - displayTipMessage(actions, selectedQuery); - } else { - actions.runQuery(sampleQuery); - } - } else { - actions.runQuery(sampleQuery); - } - this.trackSampleQueryClickEvent(selectedQuery); - } else { - if (sampleQuery.sampleBody) { - sampleQuery.sampleBody = isJsonString(sampleQuery.sampleBody) - ? JSON.parse(sampleQuery.sampleBody) - : sampleQuery.sampleBody; - } else { - sampleQuery.sampleBody = undefined; - } - - if (selectedQuery.tip) { - displayTipMessage(actions, selectedQuery); - } - } - actions.setSampleQuery(sampleQuery); - } - }; - - private trackSampleQueryClickEvent(selectedQuery: ISampleQuery) { - const sanitizedUrl = sanitizeQueryUrl(GRAPH_URL + selectedQuery.requestUrl); - telemetry.trackEvent( - eventTypes.LISTITEM_CLICK_EVENT, - { - ComponentName: componentNames.SAMPLE_QUERY_LIST_ITEM, - SampleId: selectedQuery.id, - SampleName: selectedQuery.humanName, - SampleCategory: selectedQuery.category, - QuerySignature: `${selectedQuery.method} ${sanitizedUrl}` - }); - } - - public renderGroupHeader = (props: any): any => { + const renderGroupHeader = (props: any): any => { const onToggleSelectGroup = () => { props.onToggleCollapse(props.group); }; @@ -333,180 +299,94 @@ export class SampleQueries extends Component { ); }; - private renderDetailsHeader() { + const renderDetailsHeader = () => { return
; } - public render() { - const { error, pending } = this.props.samples; - const { - intl: { messages } - }: any = this.props; - - const { sampleQueries } = this.state; - const classes = classNames(this.props); - const groups = generateGroupsFromList(sampleQueries, 'category'); - if (this.state.selectedQuery) { - const index = groups.findIndex(k => k.key === this.state.selectedQuery.category); - if (index !== -1) { - groups[index].isCollapsed = false; - } - } - - if (pending) { - return ( - - ); - } - - let maxWidthOfHumanName = 180; - if (window.innerWidth > 1280) { - maxWidthOfHumanName = 200; + if (selectedQuery) { + const index = groups.findIndex(k => k.key === selectedQuery.category); + if (index !== -1) { + groups[index].isCollapsed = false; } + } - window.onresize = () => { - if (window.innerWidth > 1280) { - maxWidthOfHumanName = 200; - } - }; - - const columns: IColumn[] = [ - { - key: 'button', - name: '', - fieldName: 'button', - minWidth: 15, - maxWidth: 25 - }, - { - key: 'authRequiredIcon', - name: '', - fieldName: 'authRequiredIcon', - minWidth: 20, - maxWidth: 20 - }, - { - key: 'method', - name: '', - fieldName: 'method', - minWidth: 20, - maxWidth: 50 - }, - { - key: 'humanName', - name: '', - fieldName: 'humanName', - minWidth: 100, - maxWidth: maxWidthOfHumanName - } - ]; - + if (pending) { return ( -
- -
- {error && ( - - - - )} + + ); + } + + return ( + + ); } -function displayTipMessage(actions: any, selectedQuery: ISampleQuery) { - actions.setQueryResponseStatus({ - messageType: MessageBarType.warning, - statusText: 'Tip', - status: selectedQuery.tip - }); -} - -function mapStateToProps({ authToken, profile, samples, theme }: IRootState) { - return { - tokenPresent: !!authToken.token, - profile, - samples, - appTheme: theme - }; -} - -function mapDispatchToProps(dispatch: Dispatch): object { - return { - actions: bindActionCreators( - { - ...queryActionCreators, - ...queryInputActionCreators, - ...samplesActionCreators, - ...queryStatusActionCreators - }, - dispatch - ) - }; -} - -// @ts-ignore -const styledSampleQueries = styled(SampleQueries, sidebarStyles); -// @ts-ignore -const IntlSampleQueries = injectIntl(styledSampleQueries); // @ts-ignore -export default connect(mapStateToProps, mapDispatchToProps)(IntlSampleQueries); +const SampleQueries = styled(unstyledSampleQueries, sidebarStyles); +export default SampleQueries; diff --git a/src/app/views/sidebar/sample-queries/sample-query-utils.ts b/src/app/views/sidebar/sample-queries/sample-query-utils.ts index cf5c515c4..65669fd14 100644 --- a/src/app/views/sidebar/sample-queries/sample-query-utils.ts +++ b/src/app/views/sidebar/sample-queries/sample-query-utils.ts @@ -1,3 +1,40 @@ +import { IColumn } from '@fluentui/react'; +import { telemetry, eventTypes, componentNames } from '../../../../telemetry'; +import { ISampleQuery } from '../../../../types/query-runner'; +import { GRAPH_URL } from '../../../services/graph-constants'; +import { validateExternalLink } from '../../../utils/external-link-validation'; +import { sanitizeQueryUrl } from '../../../utils/query-url-sanitization'; + +export const columns: IColumn[] = [ + { + key: 'button', + name: '', + fieldName: 'button', + minWidth: 15, + maxWidth: 25 + }, + { + key: 'authRequiredIcon', + name: '', + fieldName: 'authRequiredIcon', + minWidth: 20, + maxWidth: 20 + }, + { + key: 'method', + name: '', + fieldName: 'method', + minWidth: 20, + maxWidth: 50 + }, + { + key: 'humanName', + name: '', + fieldName: 'humanName', + minWidth: 100, + maxWidth: 200 + } +]; export function isJsonString(str: string): boolean { try { JSON.parse(str); @@ -6,3 +43,39 @@ export function isJsonString(str: string): boolean { return false; } } + +export function performSearch(queries: ISampleQuery[], value: string): ISampleQuery[] { + const keyword = value.toLowerCase(); + return queries.filter((sample: any) => { + const name = sample.humanName.toLowerCase(); + const category = sample.category.toLowerCase(); + return name.includes(keyword) || category.includes(keyword); + }); +} + +export const trackSampleQueryClickEvent = (query: ISampleQuery) => { + const sanitizedUrl = sanitizeQueryUrl(GRAPH_URL + query.requestUrl); + telemetry.trackEvent( + eventTypes.LISTITEM_CLICK_EVENT, + { + ComponentName: componentNames.SAMPLE_QUERY_LIST_ITEM, + SampleId: query.id, + SampleName: query.humanName, + SampleCategory: query.category, + QuerySignature: `${query.method} ${sanitizedUrl}` + }); +} + +export const trackDocumentLinkClickedEvent = async (item: ISampleQuery): Promise => { + const properties: { [key: string]: any } = { + ComponentName: componentNames.DOCUMENTATION_LINK, + SampleId: item.id, + SampleName: item.humanName, + SampleCategory: item.category, + Link: item.docLink + }; + telemetry.trackEvent(eventTypes.LINK_CLICK_EVENT, properties); + + // Check if link throws error + validateExternalLink(item.docLink || '', componentNames.DOCUMENTATION_LINK, item.id); +} diff --git a/src/types/query-runner.ts b/src/types/query-runner.ts index 08a7291b3..4c3304586 100644 --- a/src/types/query-runner.ts +++ b/src/types/query-runner.ts @@ -100,9 +100,6 @@ export interface ISampleQueriesProps { fetchSamples: Function; setQueryResponseStatus: Function; }; - intl: { - message: object; - }; } export const httpMethods: IDropdownOption[] = [ From 632faee25cbb7d9c36f6a5e27d71ae3d7d2781cc Mon Sep 17 00:00:00 2001 From: Millicent Achieng Date: Wed, 18 May 2022 13:54:13 +0300 Subject: [PATCH 2/6] Improve efficiency of query sanitization regexes (#1726) * Simplify query parameter sanitization regex * Decode query parameters separately during sanitization since decodeURIComponent cannot be used directly to parse query parameters from a URL and needs a bit of preparation * Update sanitization regex that checks if property name is valid * Update regex that matches filter segments during query sanitization * Update query sanitization regex to prevent catastrophic backtracking * Add tests to check for catastrophic backtracking * Simplify 'filter' query option value sanitization regex --- .../query-parameter-sanitization.spec.ts | 48 +++++++++++++++++-- src/app/utils/query-parameter-sanitization.ts | 26 +++++----- src/app/utils/query-url-sanitization.ts | 6 ++- 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/app/utils/query-parameter-sanitization.spec.ts b/src/app/utils/query-parameter-sanitization.spec.ts index 92c9e82d8..3a8e31e77 100644 --- a/src/app/utils/query-parameter-sanitization.spec.ts +++ b/src/app/utils/query-parameter-sanitization.spec.ts @@ -1,4 +1,4 @@ -import { isAllAlpha, sanitizeQueryParameter } from './query-parameter-sanitization'; +import { isAllAlpha, isPropertyName, sanitizeQueryParameter } from './query-parameter-sanitization'; describe('isAllAlpha should ', () => { const list = [ @@ -8,7 +8,7 @@ describe('isAllAlpha should ', () => { { key: '1a1', isAllAlphabetic: false } ]; - list.forEach(element => { + list.forEach((element) => { it(`return ${element.isAllAlphabetic} for ${element.key}`, () => { const key = isAllAlpha(element.key); expect(key).toBe(element.isAllAlphabetic); @@ -16,6 +16,38 @@ describe('isAllAlpha should ', () => { }); }); +describe('isPropertyOrEntityName should', () => { + const list = [ + { key: 'userPrincipalName', isPropertyOrEntityName: true }, + { key: 'microsoft.graph.user', isPropertyOrEntityName: true }, + { + key: 'microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry', + isPropertyOrEntityName: true + }, + { + key: 'microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry/isExpeditable', + isPropertyOrEntityName: true + }, + { + key: 'microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry/isExpeditable/', + isPropertyOrEntityName: false + }, + { + key: 'from/emailAddress/address', + isPropertyOrEntityName: true + }, + { key: '1234surname', isPropertyOrEntityName: false }, + { key: 'from/', isPropertyOrEntityName: false } + ]; + + list.forEach((element) => { + it(`return ${element.isPropertyOrEntityName} for ${element.key}`, () => { + const key = isPropertyName(element.key); + expect(key).toBe(element.isPropertyOrEntityName); + }); + }); +}); + describe('Sanitize Query Parameters should', () => { const list = [ // $top @@ -189,7 +221,7 @@ describe('Sanitize Query Parameters should', () => { }, { check: 'returns sanitized value with `all` collection operator for $filter query option', - queryParam: '$filter=ratings/all(r: r ge 3 and r le 5)', + queryParam: '$filter=ratings/all(r:r ge 3 and r le 5)', sanitizedQueryParam: '$filter=ratings/all(r: r ge and r le )' }, { @@ -197,6 +229,14 @@ describe('Sanitize Query Parameters should', () => { queryParam: '$filter=rooms/all(room: room/from/address eq \'street\' and room/baseRate lt 100.0)', sanitizedQueryParam: '$filter=rooms/all(room: room/from/address eq and room/baseRate lt )' }, + { + check: 'returns sanitized value for complex $filter query option value with potential catastrophic backtracking', + queryParam: + `$filter=isof('microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry') + and microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry/isExpeditable eq true`, + sanitizedQueryParam: + '$filter=isof() and microsoft.graph.windowsUpdates.qualityUpdateCatalogEntry/isExpeditable eq ' + }, // $expand { @@ -251,4 +291,4 @@ describe('Sanitize Query Parameters should', () => { }); }); -}); \ No newline at end of file +}); diff --git a/src/app/utils/query-parameter-sanitization.ts b/src/app/utils/query-parameter-sanitization.ts index edc450aea..0847287f7 100644 --- a/src/app/utils/query-parameter-sanitization.ts +++ b/src/app/utils/query-parameter-sanitization.ts @@ -6,7 +6,8 @@ const QUERY_FUNCTIONS = [ 'contains', 'substring', 'indexof', - 'concat' + 'concat', + 'isof' ]; const ARITHMETIC_OPERATORS = ['add', 'sub', 'mul', 'div', 'divby', 'mod']; const COMPARISON_OPERATORS = ['eq', 'ne', 'gt', 'ge', 'lt', 'le']; @@ -21,19 +22,18 @@ const POSITIVE_INTEGER_REGEX = /^[1-9]\d*$/; const MEDIA_TYPE_REGEX = /^(([a-z]+\/)?\w[\w+-.]*)$/i; // Matches the format key=value const KEY_VALUE_REGEX = /^[a-z]+=[a-z]+$/i; -// Matches property name patterns e.g. displayName or from/emailAddress/address or microsoft.graph.itemAttachment/item -const PROPERTY_NAME_REGEX = - /^((((microsoft.graph(.[a-z]+)+)|[a-z]+)(\/?\b[a-z]+\b)+)|[a-z]+)$/i; +// Matches property name patterns e.g. displayName or from/emailAddress/address +// or microsoft.graph.itemAttachment or microsoft.graph.itemAttachment/item +const PROPERTY_NAME_REGEX = /^[a-z]+([\.\/](?=([a-z]+))\2)*$/i; // Matches pattterns within quotes e.g "displayName: Gupta" const QUOTED_TEXT_REGEX = /^["']([^"]*)['"]$/; // Matches segments of $filter query option values e.g. isRead eq false will match isRead, eq, false // eslint-disable-next-line max-len -const FILTER_SEGMENT_REGEX = - /(((((microsoft.graph(.[a-z]+)+)|[a-z]+)(\/?\b[a-z]+\b)+)|[a-z]+)\(.*?\))|("[^\"]+")|('[^\']+')|\(.*?\)|[^\s]+/gi; +const FILTER_SEGMENT_REGEX = /\S+\(.*\)|\(.*?\)|\S+/gi; // Matches segments of $search query option e.g. // "description:One" AND ("displayName:Video" OR "displayName:Drive") will match // "description:One", AND, ("displayName:Video" OR "displayName:Drive") -const SEARCH_SEGMENT_REGEX = /\(.*\)|(['"][\w\s]+['"])|[^\s]+/g; +const SEARCH_SEGMENT_REGEX = /\(.*\)|(['"][\w\s]+['"])|\S+/g; // Matches comma separating segments of $expand query option e.g. children($select=id,name),customer // will match comma between children($select=id,name) and customer const EXPAND_SEGMENT_REGEX = /,(?![^()]*\))/g; @@ -54,7 +54,7 @@ function isKeyValuePair(str: string): boolean { return KEY_VALUE_REGEX.test(str); } -function isPropertyName(str: string): boolean { +export function isPropertyName(str: string): boolean { return PROPERTY_NAME_REGEX.test(str); } @@ -222,7 +222,7 @@ function sanitizeOrderByQueryOptionValue(queryOptionValue: string): string { // Check if sort direction has been included if (expressionParts.length > 1) { - let sortDirection = expressionParts[1].trim(); + let sortDirection = expressionParts[1].trim().toLowerCase(); if (sortDirection) { if (sortDirection !== 'asc' && sortDirection !== 'desc') { sortDirection = ''; @@ -384,7 +384,7 @@ function sanitizeFilterQueryOptionValue(queryParameterValue: string): string { if (LAMBDA_OPERATORS.includes(lambdaOperator)) { propertyName = propertyName.substring(0, propertyName.length - 4); if (!isPropertyName(propertyName)) { - propertyName = ''; + propertyName = ''; } let textWithinBrackets = segment .substring(openingBracketIndex + 1, segment.length - 1) @@ -433,7 +433,8 @@ function sanitizeFilterQueryOptionValue(queryParameterValue: string): string { if (!isPropertyName(propertyName)) { propertyName = ''; } - sanitizedQueryString += `${queryFunctionPrefix}(${propertyName}${commaIndex > 0 ? ',' : '' + sanitizedQueryString += `${queryFunctionPrefix}(${propertyName}${ + commaIndex > 0 ? ',' : '' })`; } else { sanitizedQueryString += `${queryFunctionPrefix}()`; @@ -459,7 +460,8 @@ function sanitizeFilterQueryOptionValue(queryParameterValue: string): string { COMPARISON_OPERATORS.includes(expectedOperator) || ARITHMETIC_OPERATORS.includes(expectedOperator) ) { - sanitizedQueryString += `${segment} ${filterSegments[index + 1] + sanitizedQueryString += `${segment} ${ + filterSegments[index + 1] } `; index += 2; continue; diff --git a/src/app/utils/query-url-sanitization.ts b/src/app/utils/query-url-sanitization.ts index 3180b6613..cab4522c4 100644 --- a/src/app/utils/query-url-sanitization.ts +++ b/src/app/utils/query-url-sanitization.ts @@ -146,8 +146,10 @@ function sanitizePathSegment(previousSegment: string, segment: string): string { * @param queryString */ function sanitizeQueryParameters(queryString: string): string { - // remove leading ? from query string - queryString = queryString.substring(1); + // remove leading ? from query string and decode + queryString = decodeURIComponent( + queryString.substring(1).replace(/\+/g, ' ') + ); return queryString.split('&').map(sanitizeQueryParameter).join('&'); } From 9b2891b493c475c8bf783b1799be44d3da9a09f0 Mon Sep 17 00:00:00 2001 From: EvansA Date: Thu, 19 May 2022 09:15:23 +0300 Subject: [PATCH 3/6] Task: auto merge dependabot PRs into one PR (#1735) --- .github/workflows/combine-prs.yml | 118 ++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 .github/workflows/combine-prs.yml diff --git a/.github/workflows/combine-prs.yml b/.github/workflows/combine-prs.yml new file mode 100644 index 000000000..166f96858 --- /dev/null +++ b/.github/workflows/combine-prs.yml @@ -0,0 +1,118 @@ +name: 'Combine PRs' + +# This workflow will merge all dependabot PRs into one branch and open a new PR against dependabot-upgrades +on: + workflow_dispatch: + inputs: + branchPrefix: + description: 'Branch prefix to find combinable PRs based on' + required: true + default: 'dependabot/' + combineBranchName: + description: 'Name of the branch to combine PRs into' + required: true + default: 'dependabot-upgrades' + +jobs: + # Update dependabot-upgrades branch so that it's in sync with dev + merge-dev-dependabot-upgrades: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set Git config + run: | + git config user.name github-actions + git config user.email github-actions@github.com + + - name: Merge dev to dependabot-upgrades + run: | + git fetch --unshallow + git pull origin + git checkout dependabot-upgrades + git merge --no-ff origin/dev --allow-unrelated-histories -m "Auto-merge dev to dependabot-upgrades" + git push + + # Get all PRs opened by dependabot + combine-prs: + runs-on: ubuntu-latest + steps: + - uses: actions/github-script@v3 + id: fetch-branch-names + name: Fetch branch names + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + const pulls = await github.paginate('GET /repos/:owner/:repo/pulls', { + owner: context.repo.owner, + repo: context.repo.repo + }); + branches = []; + prs = []; + base_branch = null; + for (const pull of pulls) { + const branch = pull['head']['ref']; + console.log('Pull for branch: ' + branch); + if (branch.startsWith('${{ github.event.inputs.branchPrefix }}')) { + console.log('Branch matched: ' + branch); + console.log('Adding branch to array: ' + branch); + branches.push(branch); + prs.push('#' + pull['number'] + ' ' + pull['title']); + } + } + + if (branches.length == 0) { + core.setFailed('No PRs/branches matched criteria'); + return; + } + + core.setOutput('base-branch', 'dev'); + core.setOutput('prs-string', prs.join('\n')); + + combined = branches.join(' ') + console.log('Combined: ' + combined); + return combined + + # Checks-out the repository under $GITHUB_WORKSPACE, so this job can access it + - uses: actions/checkout@v2.3.3 + with: + fetch-depth: 0 + + # Merges all dependabot PRs into the dependabot-upgrades branch + - name: Created combined branch + env: + BRANCHES_TO_COMBINE: ${{ steps.fetch-branch-names.outputs.result }} + COMBINE_BRANCH_NAME: ${{ github.event.inputs.combineBranchName }} + run: | + echo "$BRANCHES_TO_COMBINE" + sourcebranches="${BRANCHES_TO_COMBINE%\"}" + sourcebranches="${sourcebranches#\"}" + + git config pull.rebase false + git config user.name github-actions + git config user.email github-actions@github.com + + git fetch + git pull origin + git checkout $COMBINE_BRANCH_NAME + + git pull origin $sourcebranches --no-edit + git push -f origin $COMBINE_BRANCH_NAME + + # Opens a pull request to dev from dependabot-upgrades branch + - uses: actions/github-script@v3 + name: Create Combined Pull Request + env: + PRS_STRING: ${{ steps.fetch-branch-names.outputs.prs-string }} + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + const prString = process.env.PRS_STRING; + const body = 'This PR was created by the Combine PRs action by combining the following PRs:\n' + prString; + await github.pulls.create({ + owner: context.repo.owner, + repo: context.repo.repo, + title: 'Combined dependabot pull requests', + head: '${{ github.event.inputs.combineBranchName }}', + base: '${{ steps.fetch-branch-names.outputs.base-branch }}', + body: body + }); From e14ba4f7c216ed05a946ed5795daac33ff908b39 Mon Sep 17 00:00:00 2001 From: EvansA Date: Fri, 20 May 2022 11:28:12 +0300 Subject: [PATCH 4/6] Add history items as a batch (#1748) --- .../request-history-action-creators.spec.ts | 50 +++++++++++++++++-- .../request-history-action-creators.ts | 10 +++- .../reducers/request-history-reducers.spec.ts | 18 ++++++- .../reducers/request-history-reducers.ts | 5 ++ src/app/services/redux-constants.ts | 1 + src/index.tsx | 7 +-- 6 files changed, 79 insertions(+), 12 deletions(-) diff --git a/src/app/services/actions/request-history-action-creators.spec.ts b/src/app/services/actions/request-history-action-creators.spec.ts index 77f215a6e..3c08b087b 100644 --- a/src/app/services/actions/request-history-action-creators.spec.ts +++ b/src/app/services/actions/request-history-action-creators.spec.ts @@ -3,12 +3,14 @@ import thunk from 'redux-thunk'; import { addHistoryItem, viewHistoryItem, removeHistoryItem, - bulkRemoveHistoryItems + bulkRemoveHistoryItems, + bulkAddHistoryItems } from './request-history-action-creators'; import { ADD_HISTORY_ITEM_SUCCESS, VIEW_HISTORY_ITEM_SUCCESS, REMOVE_HISTORY_ITEM_SUCCESS, - REMOVE_ALL_HISTORY_ITEMS_SUCCESS + REMOVE_ALL_HISTORY_ITEMS_SUCCESS, + BULK_ADD_HISTORY_ITEMS_SUCCESS } from '../redux-constants'; import { IHistoryItem } from '../../../types/history'; @@ -134,8 +136,46 @@ describe('Request History Action Creators', () => { .then(() => { expect(store.getActions()).toEqual([expectedAction]); }) - }) -}); + }); + + it('dispatches BULK_ADD_HISTORY_ITEMS_SUCCESS when bulkAddHistoryItems is called', () => { + // Arrange + const historyItems = [ + { + index: 0, + statusText: 'Something worked!', + responseHeaders: [], + result: {}, + url: 'https://graph.microsoft.com/v1.0/me', + method: 'GET', + headers: [], + createdAt: '12345', + status: 200, + duration: 200 + }, + { + index: 1, + statusText: 'Another history item!', + responseHeaders: [], + result: {}, + url: 'https://graph.microsoft.com/v1.0/me/events', + method: 'GET', + headers: [], + createdAt: '12345', + status: 200, + duration: 200 + } + ] + + const expectedAction = { + type: BULK_ADD_HISTORY_ITEMS_SUCCESS, + response: historyItems + } -//Add tests for the async functions + const store = mockStore([]); + // Act and Assert + store.dispatch(bulkAddHistoryItems(historyItems)); + expect(store.getActions()).toEqual([expectedAction]); + }) +}); diff --git a/src/app/services/actions/request-history-action-creators.ts b/src/app/services/actions/request-history-action-creators.ts index 8bf28e253..8516b3ecd 100644 --- a/src/app/services/actions/request-history-action-creators.ts +++ b/src/app/services/actions/request-history-action-creators.ts @@ -5,7 +5,8 @@ import { ADD_HISTORY_ITEM_SUCCESS, REMOVE_ALL_HISTORY_ITEMS_SUCCESS, REMOVE_HISTORY_ITEM_SUCCESS, - VIEW_HISTORY_ITEM_SUCCESS + VIEW_HISTORY_ITEM_SUCCESS, + BULK_ADD_HISTORY_ITEMS_SUCCESS } from '../redux-constants'; export function addHistoryItem(historyItem: IHistoryItem): any { @@ -15,6 +16,13 @@ export function addHistoryItem(historyItem: IHistoryItem): any { }; } +export function bulkAddHistoryItems(historyItems: IHistoryItem[]): any { + return { + type: BULK_ADD_HISTORY_ITEMS_SUCCESS, + response: historyItems + }; +} + export function removeHistoryItem(historyItem: IHistoryItem): Function { delete historyItem.category; diff --git a/src/app/services/reducers/request-history-reducers.spec.ts b/src/app/services/reducers/request-history-reducers.spec.ts index 7fd606c41..b76fecf37 100644 --- a/src/app/services/reducers/request-history-reducers.spec.ts +++ b/src/app/services/reducers/request-history-reducers.spec.ts @@ -1,6 +1,6 @@ import { history } from './request-history-reducers'; import { - ADD_HISTORY_ITEM_SUCCESS, REMOVE_ALL_HISTORY_ITEMS_SUCCESS, + ADD_HISTORY_ITEM_SUCCESS, BULK_ADD_HISTORY_ITEMS_SUCCESS, REMOVE_ALL_HISTORY_ITEMS_SUCCESS, REMOVE_HISTORY_ITEM_SUCCESS } from '../redux-constants'; @@ -65,6 +65,22 @@ describe('Request History Reducer', () => { const newState = history(initialState, action); expect(newState).toEqual(expectedState); + }); + + it('should handle BULK_ADD_HISTORY_ITEMS_SUCCESS', () => { + const initialState: any = []; + const dummy = [ + { query: 'query', createdAt: new Date().toISOString() }, + { query: 'query2', createdAt: new Date().toISOString() } + ]; + const queryAction = { + type: BULK_ADD_HISTORY_ITEMS_SUCCESS, + response: dummy + }; + + const newState = history(initialState, queryAction); + + expect(newState).toEqual(dummy); }) }); diff --git a/src/app/services/reducers/request-history-reducers.ts b/src/app/services/reducers/request-history-reducers.ts index 4a467bb6f..499a2eded 100644 --- a/src/app/services/reducers/request-history-reducers.ts +++ b/src/app/services/reducers/request-history-reducers.ts @@ -2,6 +2,7 @@ import { IAction } from '../../../types/action'; import { IHistoryItem } from '../../../types/history'; import { ADD_HISTORY_ITEM_SUCCESS, + BULK_ADD_HISTORY_ITEMS_SUCCESS, REMOVE_ALL_HISTORY_ITEMS_SUCCESS, REMOVE_HISTORY_ITEM_SUCCESS } from '../redux-constants'; @@ -18,6 +19,10 @@ export function history(state: any[] = [], action: IAction): any { return historyItems; + case BULK_ADD_HISTORY_ITEMS_SUCCESS: + historyItems = [...state, ...action.response]; + return historyItems; + case REMOVE_HISTORY_ITEM_SUCCESS: return state.filter(historyItem => historyItem !== action.response); diff --git a/src/app/services/redux-constants.ts b/src/app/services/redux-constants.ts index 5492225b3..419e4b968 100644 --- a/src/app/services/redux-constants.ts +++ b/src/app/services/redux-constants.ts @@ -52,3 +52,4 @@ export const GET_POLICY_ERROR = 'GET_POLICY_ERROR'; export const GET_POLICY_PENDING = 'GET_POLICY_PENDING'; export const RESOURCEPATHS_ADD_SUCCESS = 'RESOURCEPATHS_ADD_SUCCESS'; export const RESOURCEPATHS_DELETE_SUCCESS = 'RESOURCEPATHS_DELETE_SUCCESS'; +export const BULK_ADD_HISTORY_ITEMS_SUCCESS = 'BULK_ADD_HISTORY_ITEMS_SUCCESS' diff --git a/src/index.tsx b/src/index.tsx index efb264f22..97963c241 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -19,7 +19,7 @@ import { getAuthTokenSuccess, getConsentedScopesSuccess } from './app/services/a import { setDevxApiUrl } from './app/services/actions/devxApi-action-creators'; import { setGraphExplorerMode } from './app/services/actions/explorer-mode-action-creator'; import { getGraphProxyUrl } from './app/services/actions/proxy-action-creator'; -import { addHistoryItem } from './app/services/actions/request-history-action-creators'; +import { bulkAddHistoryItems } from './app/services/actions/request-history-action-creators'; import { changeThemeSuccess } from './app/services/actions/theme-action-creator'; import { isValidHttpsUrl } from './app/utils/external-link-validation'; import App from './app/views/App'; @@ -35,7 +35,6 @@ import { loadGETheme } from './themes'; import { readTheme } from './themes/theme-utils'; import { IDevxAPI } from './types/devx-api'; import { Mode } from './types/enums'; -import { IHistoryItem } from './types/history'; import { changeTheme } from './app/services/actions/theme-action-creator'; import { fetchResources } from './app/services/actions/resource-explorer-action-creators'; @@ -140,9 +139,7 @@ if (devxApiUrl && isValidHttpsUrl(devxApiUrl)) { readHistoryData().then((data: any) => { if (data.length > 0) { - data.forEach((element: IHistoryItem) => { - appStore.dispatch(addHistoryItem(element)); - }); + appStore.dispatch(bulkAddHistoryItems(data)); } }); From 9f4be4424e4ca89956450bc55d31c9bebd4da0c2 Mon Sep 17 00:00:00 2001 From: Evans Aboge Date: Tue, 24 May 2022 14:31:44 +0300 Subject: [PATCH 5/6] fill up div during hover --- src/app/views/main-header/Help.tsx | 1 + src/app/views/main-header/MainHeader.styles.ts | 4 ++-- src/app/views/main-header/MainHeader.tsx | 4 ++-- src/app/views/main-header/settings/Settings.tsx | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/app/views/main-header/Help.tsx b/src/app/views/main-header/Help.tsx index 395ba39e6..c52e59f0b 100644 --- a/src/app/views/main-header/Help.tsx +++ b/src/app/views/main-header/Help.tsx @@ -108,6 +108,7 @@ export const Help = () => { content={translateMessage('Help')} id={getId()} calloutProps={{ gapSpace: 0 }} + styles={{ root: {flexGrow: '1', display: 'flex', alignItems: 'stretch' }}} > { }, feedbackIconAdjustmentStyles: { position: 'relative' as 'relative', - right: '-6px', top: '4px' }, tenantStyles: { @@ -32,7 +31,8 @@ export const mainHeaderStyles = (theme: ITheme) => { width: '50px', ':hover': { background: `${theme.palette.neutralLight} !important` - } + }, + flexGrow: '1' } } } diff --git a/src/app/views/main-header/MainHeader.tsx b/src/app/views/main-header/MainHeader.tsx index d15074ba4..faa86f5f2 100644 --- a/src/app/views/main-header/MainHeader.tsx +++ b/src/app/views/main-header/MainHeader.tsx @@ -96,8 +96,8 @@ export const MainHeader: React.FunctionComponent = (props: Mai style={tenantStyles} /> } - - + + diff --git a/src/app/views/main-header/settings/Settings.tsx b/src/app/views/main-header/settings/Settings.tsx index 8dae1f67a..77c2010eb 100644 --- a/src/app/views/main-header/settings/Settings.tsx +++ b/src/app/views/main-header/settings/Settings.tsx @@ -104,11 +104,12 @@ export const Settings: React.FunctionComponent = () => { }; return ( -
+
Date: Tue, 24 May 2022 17:11:28 +0300 Subject: [PATCH 6/6] refactor styles to styles files --- src/app/views/main-header/Help.tsx | 4 ++-- src/app/views/main-header/MainHeader.styles.ts | 16 ++++++++++++++++ src/app/views/main-header/MainHeader.tsx | 8 ++++---- src/app/views/main-header/settings/Settings.tsx | 7 ++++--- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/app/views/main-header/Help.tsx b/src/app/views/main-header/Help.tsx index c52e59f0b..39fbc26de 100644 --- a/src/app/views/main-header/Help.tsx +++ b/src/app/views/main-header/Help.tsx @@ -91,7 +91,7 @@ export const Help = () => { const calloutStyles: React.CSSProperties = { overflowY: 'hidden' } - const helpButtonStyles = mainHeaderStyles(currentTheme).iconButton; + const { iconButton: helpButtonStyles, tooltipStyles } = mainHeaderStyles(currentTheme); const menuProperties = { shouldFocusOnMount: true, @@ -108,7 +108,7 @@ export const Help = () => { content={translateMessage('Help')} id={getId()} calloutProps={{ gapSpace: 0 }} - styles={{ root: {flexGrow: '1', display: 'flex', alignItems: 'stretch' }}} + styles={ tooltipStyles } > { }, flexGrow: '1' } + }, + moreInformationStyles: { + flexGrow: 1, + flexShrink: 1, + flexBasis: '60px' + }, + settingsContainerStyles: { + display: 'flex', + alignItems: 'stretch' + }, + tooltipStyles: { + root: { + flexGrow: 1, + display: 'flex', + alignItems: 'stretch' + } } } } \ No newline at end of file diff --git a/src/app/views/main-header/MainHeader.tsx b/src/app/views/main-header/MainHeader.tsx index 9c66633b1..09dc6de47 100644 --- a/src/app/views/main-header/MainHeader.tsx +++ b/src/app/views/main-header/MainHeader.tsx @@ -43,7 +43,7 @@ export const MainHeader: React.FunctionComponent = (props: Mai const mobileScreen = props.mobileScreen; const currentTheme = getTheme(); const { rootStyles : itemAlignmentStackStyles, rightItemsStyles, - feedbackIconAdjustmentStyles, tenantStyles } = mainHeaderStyles(currentTheme); + feedbackIconAdjustmentStyles, tenantStyles, moreInformationStyles } = mainHeaderStyles(currentTheme); return ( @@ -98,9 +98,9 @@ export const MainHeader: React.FunctionComponent = (props: Mai style={tenantStyles} /> } - - - + + + diff --git a/src/app/views/main-header/settings/Settings.tsx b/src/app/views/main-header/settings/Settings.tsx index 77c2010eb..9855f9be4 100644 --- a/src/app/views/main-header/settings/Settings.tsx +++ b/src/app/views/main-header/settings/Settings.tsx @@ -92,7 +92,8 @@ export const Settings: React.FunctionComponent = () => { overflowY: 'hidden' } - const settingsButtonStyles = mainHeaderStyles(currentTheme).iconButton; + const { iconButton : settingsButtonStyles, settingsContainerStyles, + tooltipStyles} = mainHeaderStyles(currentTheme); const menuProperties = { shouldFocusOnMount: true, @@ -104,12 +105,12 @@ export const Settings: React.FunctionComponent = () => { }; return ( -
+