Reset focus to trigger element when pane is closed (#217)

* Fix aria-labels in AddCollectionPane and DeleteCollectionConfirmationPane

* Fix broken delete

.github/workflows/ci.yml

@@ -3,8 +3,8 @@ on:
   push:
     branches:
       - master
-      - hotfix/**
-      - release/**
+      - hotfix/*
+      - release/*
   pull_request:
     branches:
       - master
@@ -196,6 +196,26 @@ jobs:
         shell: bash
         env:
           NODE_TLS_REJECT_UNAUTHORIZED: 0
+  endtoendpuppeteer:
+    name: "End to end puppeteer tests"
+    needs: [lint, format, compile, unittest]
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Use Node.js 12.x
+        uses: actions/setup-node@v1
+        with:
+          node-version: 12.x
+      - name: End to End Puppeteer Tests
+        run: |
+          npm ci
+          npm start &
+          npm run wait-for-server
+          npm run test:e2e
+        shell: bash
+        env:
+          NODE_TLS_REJECT_UNAUTHORIZED: 0
+          PORTAL_RUNNER_CONNECTION_STRING: ${{ secrets.CONNECTION_STRING_SQL }}
   nuget:
     name: Publish Nuget
     if: github.ref == 'refs/heads/master' || contains(github.ref, 'hotfix/') || contains(github.ref, 'release/')

src/Common/MessageHandler.ts

@@ -48,15 +48,16 @@ export function sendCachedDataMessage<TResponseDataModel>(
 
 export function sendMessage(data: any): void {
   if (canSendMessage()) {
-    // We try to find data explorer window first, then fallback to current window
-    const portalChildWindow = getDataExplorerWindow(window) || window;
-    portalChildWindow.parent.postMessage(
-      {
-        signature: "pcIframe",
-        data: data
-      },
-      portalChildWindow.document.referrer
-    );
+    const dataExplorerWindow = getDataExplorerWindow(window);
+    if (dataExplorerWindow) {
+      dataExplorerWindow.parent.postMessage(
+        {
+          signature: "pcIframe",
+          data: data
+        },
+        dataExplorerWindow.document.referrer
+      );
+    }
   }
 }
 
@@ -65,21 +66,15 @@ export const getDataExplorerWindow = (currentWindow: Window): Window | undefined
   // Start with the current window and traverse up the parent hierarchy to find a window
   // with `dataExplorerPlatform` property
   let dataExplorerWindow: Window | undefined = currentWindow;
-
-  try {
-    // TODO: Need to `any` here since the window imports Explorer which can't be in strict mode yet
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    while (dataExplorerWindow && (dataExplorerWindow as any).dataExplorerPlatform == undefined) {
-      // If a window does not have a parent, its parent property is a reference to itself.
-      if (dataExplorerWindow.parent == dataExplorerWindow) {
-        dataExplorerWindow = undefined;
-      } else {
-        dataExplorerWindow = dataExplorerWindow.parent;
-      }
+  // TODO: Need to `any` here since the window imports Explorer which can't be in strict mode yet
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  while (dataExplorerWindow && (dataExplorerWindow as any).dataExplorerPlatform == undefined) {
+    // If a window does not have a parent, its parent property is a reference to itself.
+    if (dataExplorerWindow.parent == dataExplorerWindow) {
+      dataExplorerWindow = undefined;
+    } else {
+      dataExplorerWindow = dataExplorerWindow.parent;
     }
-  } catch (error) {
-    // This can happen if we come across parent from a different origin
-    dataExplorerWindow = undefined;
   }
 
   return dataExplorerWindow;

src/ConfigContext.ts

@@ -6,7 +6,7 @@ export enum Platform {
 
 interface ConfigContext {
   platform: Platform;
-  allowedParentFrameOrigins: RegExp;
+  allowedParentFrameOrigins: string[];
   gitSha?: string;
   proxyPath?: string;
   AAD_ENDPOINT: string;
@@ -30,7 +30,12 @@ interface ConfigContext {
 // Default configuration
 let configContext: Readonly<ConfigContext> = {
   platform: Platform.Portal,
-  allowedParentFrameOrigins: /^https:\/\/portal\.azure\.com$|^https:\/\/portal\.azure\.us$|^https:\/\/portal\.azure\.cn$|^https:\/\/portal\.microsoftazure\.de$|^https:\/\/.+\.portal\.azure\.com$|^https:\/\/.+\.portal\.azure\.us$|^https:\/\/.+\.portal\.azure\.cn$|^https:\/\/.+\.portal\.microsoftazure\.de$|^https:\/\/main\.documentdb\.ext\.azure\.com$|^https:\/\/main\.documentdb\.ext\.microsoftazure\.de$|^https:\/\/main\.documentdb\.ext\.azure\.cn$|^https:\/\/main\.documentdb\.ext\.azure\.us$/,
+  allowedParentFrameOrigins: [
+    `^https:\\/\\/cosmos.azure.(com|cn|us)$`,
+    `^https:\\/\\/[\\.\\w]+.portal.azure.(com|cn|us)$`,
+    `^https:\\/\\/[\\.\\w]+.ext.azure.(com|cn|us)$`,
+    `^https:\\/\\/[\\.\\w]+microsoftazure.de$`
+  ],
   // Webpack injects this at build time
   gitSha: process.env.GIT_SHA,
   hostedExplorerURL: "https://cosmos.azure.com/",
@@ -73,8 +78,13 @@ export async function initializeConfiguration(): Promise<ConfigContext> {
     const response = await fetch("./config.json");
     if (response.status === 200) {
       try {
-        const externalConfig = await response.json();
+        const { allowedParentFrameOrigins, ...externalConfig } = await response.json();
         Object.assign(configContext, externalConfig);
+        if (allowedParentFrameOrigins && allowedParentFrameOrigins.length > 0) {
+          updateConfigContext({
+            allowedParentFrameOrigins: [...configContext.allowedParentFrameOrigins, ...allowedParentFrameOrigins]
+          });
+        }
       } catch (error) {
         console.error("Unable to parse json in config file");
         console.error(error);

src/Explorer/Controls/DynamicList/DynamicListComponent.ts

@@ -86,6 +86,7 @@ export class DynamicListViewModel extends WaitsForTemplateViewModel {
   public onRemoveItemKeyPress = (data: any, event: KeyboardEvent, source: any): boolean => {
     if (event.keyCode === KeyCodes.Enter || event.keyCode === KeyCodes.Space) {
       this.removeItem(data, event);
+      (document.querySelector(".dynamicListItem:last-of-type input") as HTMLElement).focus();
       event.stopPropagation();
       return false;
     }
@@ -94,7 +95,7 @@ export class DynamicListViewModel extends WaitsForTemplateViewModel {
 
   public addItem(): void {
     this.listItems.push({ value: ko.observable("") });
-    document.getElementById("uniqueKeyItems").focus();
+    (document.querySelector(".dynamicListItem:last-of-type input") as HTMLElement).focus();
   }
 
   public onAddItemKeyPress = (source: any, event: KeyboardEvent): boolean => {

src/Explorer/Panes/AddCollectionPane.html

@@ -115,10 +115,10 @@
 
           <!-- Database provisioned throughput - Start -->
           <!-- ko if: canConfigureThroughput -->
-          <div class="databaseProvision" aria-label="New database provision support"
+          <div class="databaseProvision" aria-label="Provision database throughput"
               data-bind="visible: databaseCreateNew">
               <input tabindex="0" type="checkbox" data-test="addCollectionPane-databaseSharedThroughput"
-                  id="addCollection-databaseSharedThroughput" title="Provision shared throughput"
+                  id="addCollection-databaseSharedThroughput" title="Provision database throughput"
                   data-bind="checked: databaseCreateNewShared" />
               <span class="databaseProvisionText" for="databaseSharedThroughput">Provision database throughput</span>
               <span class="infoTooltip" role="tooltip" tabindex="0">

src/Explorer/Panes/AddCollectionPane.ts

@@ -608,7 +608,7 @@ export default class AddCollectionPane extends ContextualPaneBase {
         return true;
       }
 
-      if (this.container.isPreferredApiMongoDB()) {
+      if (this.container.isPreferredApiMongoDB() && this.container.hasStorageAnalyticsAfecFeature()) {
         return true;
       }
 

src/Explorer/Panes/ContextualPaneBase.ts

@@ -9,6 +9,7 @@ import Explorer from "../Explorer";
 
 // TODO: Use specific actions for logging telemetry data
 export abstract class ContextualPaneBase extends WaitsForTemplateViewModel {
+  private initalFocusedElement: HTMLElement | undefined;
   public id: string;
   public container: Explorer;
   public firstFieldHasFocus: ko.Observable<boolean>;
@@ -49,9 +50,11 @@ export abstract class ContextualPaneBase extends WaitsForTemplateViewModel {
     this.visible(false);
     this.isExecuting(false);
     this.resetData();
+    this.resetFocus();
   }
 
   public open() {
+    this.initalFocusedElement = document.activeElement as HTMLElement;
     this.visible(true);
     this.firstFieldHasFocus(true);
     this.resizePane();
@@ -123,4 +126,11 @@ export abstract class ContextualPaneBase extends WaitsForTemplateViewModel {
 
     $(paneElement).height(newPaneElementHeight);
   }
+
+  private resetFocus(): void {
+    if (this.initalFocusedElement) {
+      this.initalFocusedElement.focus();
+      this.initalFocusedElement = undefined;
+    }
+  }
 }

src/Explorer/Panes/DeleteCollectionConfirmationPane.html

@@ -67,8 +67,7 @@
                 name="collectionIdConfirmation"
                 required
                 class="collid"
-                data-bind="value: collectionIdConfirmation, hasFocus: firstFieldHasFocus"
-                aria-label="Confirm by typing the collection id"
+                data-bind="value: collectionIdConfirmation, hasFocus: firstFieldHasFocus, attr: { 'aria-label': collectionIdConfirmationText }"
               />
             </p>
           </div>

src/Explorer/Panes/Tables/AddTableEntityPane.ts

@@ -80,7 +80,7 @@ export default class AddTableEntityPane extends TableEntityPane {
       this.updateIsActionEnabled();
       super.open();
     }
-    const focusElement = document.getElementById("addTableEntityValue");
+    const focusElement = document.getElementById("closeAddEntityPane");
     focusElement && focusElement.focus();
   }
 

src/Explorer/Panes/Tables/TableAddEntityPane.html

@@ -21,6 +21,7 @@
         <div class="firstdivbg headerline">
           <span role="heading" aria-level="2" data-bind="text: title"></span>
           <div
+            id="closeAddEntityPane"
             class="closeImg"
             role="button"
             aria-label="Close pane"

src/Utils/MessageValidation.test.ts

@@ -0,0 +1,21 @@
+import { isInvalidParentFrameOrigin } from "./MessageValidation";
+
+test.each`
+  domain                                             | expected
+  ${"https://cosmos.azure.com"}                      | ${false}
+  ${"https://cosmos.azure.us"}                       | ${false}
+  ${"https://cosmos.azure.cn"}                       | ${false}
+  ${"https://cosmos.microsoftazure.de"}              | ${false}
+  ${"https://subdomain.portal.azure.com"}            | ${false}
+  ${"https://subdomain.portal.azure.us"}             | ${false}
+  ${"https://subdomain.portal.azure.cn"}             | ${false}
+  ${"https://subdomain.microsoftazure.de"}           | ${false}
+  ${"https://main.documentdb.ext.azure.com"}         | ${false}
+  ${"https://main.documentdb.ext.azure.us"}          | ${false}
+  ${"https://main.documentdb.ext.azure.cn"}          | ${false}
+  ${"https://main.documentdb.ext.microsoftazure.de"} | ${false}
+  ${"https://random.domain"}                         | ${true}
+  ${"https://malicious.cloudapp.azure.com"}          | ${true}
+`("returns $expected when called with $domain", ({ domain, expected }) => {
+  expect(isInvalidParentFrameOrigin({ origin: domain } as MessageEvent)).toBe(expected);
+});

src/Utils/MessageValidation.ts

@@ -4,13 +4,18 @@ export function isInvalidParentFrameOrigin(event: MessageEvent): boolean {
   return !isValidOrigin(configContext.allowedParentFrameOrigins, event);
 }
 
-function isValidOrigin(allowedOrigins: RegExp, event: MessageEvent): boolean {
+function isValidOrigin(allowedOrigins: string[], event: MessageEvent): boolean {
   const eventOrigin = (event && event.origin) || "";
   const windowOrigin = (window && window.origin) || "";
   if (eventOrigin === windowOrigin) {
     return true;
   }
 
-  const result = allowedOrigins && allowedOrigins.test(eventOrigin);
-  return result;
+  for (const origin of allowedOrigins) {
+    const result = new RegExp(origin).test(eventOrigin);
+    if (result) {
+      return true;
+    }
+  }
+  return false;
 }