From 5da27aed945aae35c9f977c03d139c4a8ba00bda Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 25 Nov 2020 18:39:11 -0700 Subject: [PATCH] Replace the concept of a Widget Security Key with an OIDC state The security key naming/practice was misguided, so let's call it what it is (a settings key) and abstract away the complexity to a new store. Fixes https://github.com/vector-im/element-web/issues/15820 while we're here. --- .../dialogs/WidgetOpenIDPermissionsDialog.js | 27 ++---- src/stores/widgets/StopGapWidget.ts | 2 +- src/stores/widgets/StopGapWidgetDriver.ts | 25 +++-- src/stores/widgets/WidgetPermissionStore.ts | 93 +++++++++++++++++++ src/utils/WidgetUtils.ts | 22 ----- 5 files changed, 119 insertions(+), 50 deletions(-) create mode 100644 src/stores/widgets/WidgetPermissionStore.ts diff --git a/src/components/views/dialogs/WidgetOpenIDPermissionsDialog.js b/src/components/views/dialogs/WidgetOpenIDPermissionsDialog.js index e793b85079..7ed3d04318 100644 --- a/src/components/views/dialogs/WidgetOpenIDPermissionsDialog.js +++ b/src/components/views/dialogs/WidgetOpenIDPermissionsDialog.js @@ -17,18 +17,17 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; import {_t} from "../../../languageHandler"; -import SettingsStore from "../../../settings/SettingsStore"; import * as sdk from "../../../index"; import LabelledToggleSwitch from "../elements/LabelledToggleSwitch"; -import WidgetUtils from "../../../utils/WidgetUtils"; -import {SettingLevel} from "../../../settings/SettingLevel"; +import {Widget} from "matrix-widget-api"; +import {OIDCState, WidgetPermissionStore} from "../../../stores/widgets/WidgetPermissionStore"; export default class WidgetOpenIDPermissionsDialog extends React.Component { static propTypes = { onFinished: PropTypes.func.isRequired, - widgetUrl: PropTypes.string.isRequired, - widgetId: PropTypes.string.isRequired, - isUserWidget: PropTypes.bool.isRequired, + widget: PropTypes.objectOf(Widget).isRequired, + widgetKind: PropTypes.string.isRequired, // WidgetKind from widget-api + inRoomId: PropTypes.string, }; constructor() { @@ -51,16 +50,10 @@ export default class WidgetOpenIDPermissionsDialog extends React.Component { if (this.state.rememberSelection) { console.log(`Remembering ${this.props.widgetId} as allowed=${allowed} for OpenID`); - const currentValues = SettingsStore.getValue("widgetOpenIDPermissions"); - if (!currentValues.allow) currentValues.allow = []; - if (!currentValues.deny) currentValues.deny = []; - - const securityKey = WidgetUtils.getWidgetSecurityKey( - this.props.widgetId, - this.props.widgetUrl, - this.props.isUserWidget); - (allowed ? currentValues.allow : currentValues.deny).push(securityKey); - SettingsStore.setValue("widgetOpenIDPermissions", null, SettingLevel.DEVICE, currentValues); + WidgetPermissionStore.instance.setOIDCState( + this.props.widget, this.props.widgetKind, this.props.inRoomId, + allowed ? OIDCState.Allowed : OIDCState.Denied, + ); } this.props.onFinished(allowed); @@ -84,7 +77,7 @@ export default class WidgetOpenIDPermissionsDialog extends React.Component { "A widget located at %(widgetUrl)s would like to verify your identity. " + "By allowing this, the widget will be able to verify your user ID, but not " + "perform actions as you.", { - widgetUrl: this.props.widgetUrl.split("?")[0], + widgetUrl: this.props.widget.templateUrl.split("?")[0], }, )}

diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts index 3485e153e1..29d63cf3fa 100644 --- a/src/stores/widgets/StopGapWidget.ts +++ b/src/stores/widgets/StopGapWidget.ts @@ -246,7 +246,7 @@ export class StopGapWidget extends EventEmitter { public start(iframe: HTMLIFrameElement) { if (this.started) return; const allowedCapabilities = this.appTileProps.whitelistCapabilities || []; - const driver = new StopGapWidgetDriver( allowedCapabilities, this.mockWidget, this.kind); + const driver = new StopGapWidgetDriver(allowedCapabilities, this.mockWidget, this.kind, this.roomId); this.messaging = new ClientWidgetApi(this.mockWidget, iframe, driver); this.messaging.on("preparing", () => this.emit("preparing")); this.messaging.on("ready", () => this.emit("ready")); diff --git a/src/stores/widgets/StopGapWidgetDriver.ts b/src/stores/widgets/StopGapWidgetDriver.ts index 59cdbfe3e5..b6e2f6c681 100644 --- a/src/stores/widgets/StopGapWidgetDriver.ts +++ b/src/stores/widgets/StopGapWidgetDriver.ts @@ -30,13 +30,12 @@ import { iterableDiff, iterableUnion } from "../../utils/iterables"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import ActiveRoomObserver from "../../ActiveRoomObserver"; import Modal from "../../Modal"; -import WidgetUtils from "../../utils/WidgetUtils"; -import SettingsStore from "../../settings/SettingsStore"; import WidgetOpenIDPermissionsDialog from "../../components/views/dialogs/WidgetOpenIDPermissionsDialog"; import WidgetCapabilitiesPromptDialog, { getRememberedCapabilitiesForWidget, } from "../../components/views/dialogs/WidgetCapabilitiesPromptDialog"; import { WidgetPermissionCustomisations } from "../../customisations/WidgetPermissions"; +import { OIDCState, WidgetPermissionStore } from "./WidgetPermissionStore"; // TODO: Purge this from the universe @@ -44,7 +43,12 @@ export class StopGapWidgetDriver extends WidgetDriver { private allowedCapabilities: Set; // TODO: Refactor widgetKind into the Widget class - constructor(allowedCapabilities: Capability[], private forWidget: Widget, private forWidgetKind: WidgetKind) { + constructor( + allowedCapabilities: Capability[], + private forWidget: Widget, + private forWidgetKind: WidgetKind, + private inRoomId?: string, + ) { super(); // Always allow screenshots to be taken because it's a client-induced flow. The widget can't @@ -114,26 +118,27 @@ export class StopGapWidgetDriver extends WidgetDriver { public async askOpenID(observer: SimpleObservable) { const isUserWidget = this.forWidgetKind !== WidgetKind.Room; // modal and account widgets are "user" widgets const rawUrl = this.forWidget.templateUrl; - const widgetSecurityKey = WidgetUtils.getWidgetSecurityKey(this.forWidget.id, rawUrl, isUserWidget); + const oidcState = WidgetPermissionStore.instance.getOIDCState( + this.forWidget, this.forWidgetKind, this.inRoomId, + ); const getToken = (): Promise => { return MatrixClientPeg.get().getOpenIdToken(); }; - const settings = SettingsStore.getValue("widgetOpenIDPermissions"); - if (settings?.deny?.includes(widgetSecurityKey)) { + if (oidcState === OIDCState.Denied) { return observer.update({state: OpenIDRequestState.Blocked}); } - if (settings?.allow?.includes(widgetSecurityKey)) { + if (oidcState === OIDCState.Allowed) { return observer.update({state: OpenIDRequestState.Allowed, token: await getToken()}); } observer.update({state: OpenIDRequestState.PendingUserConfirmation}); Modal.createTrackedDialog("OpenID widget permissions", '', WidgetOpenIDPermissionsDialog, { - widgetUrl: rawUrl, - widgetId: this.forWidget.id, - isUserWidget: isUserWidget, + widget: this.forWidget, + widgetKind: this.forWidgetKind, + inRoomId: this.inRoomId, onFinished: async (confirm) => { if (!confirm) { diff --git a/src/stores/widgets/WidgetPermissionStore.ts b/src/stores/widgets/WidgetPermissionStore.ts new file mode 100644 index 0000000000..c2c30911c9 --- /dev/null +++ b/src/stores/widgets/WidgetPermissionStore.ts @@ -0,0 +1,93 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AsyncStore } from "../AsyncStore"; +import { ActionPayload } from "../../dispatcher/payloads"; +import defaultDispatcher from "../../dispatcher/dispatcher"; +import SettingsStore from "../../settings/SettingsStore"; +import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; +import { IWidget, Widget, WidgetKind } from "matrix-widget-api"; +import { MatrixClientPeg } from "../../MatrixClientPeg"; +import WidgetUtils from "../../utils/WidgetUtils"; +import { SettingLevel } from "../../settings/SettingLevel"; + +export enum OIDCState { + Allowed, // user has set the remembered value as allowed + Denied, // user has set the remembered value as disallowed + Unknown, // user has not set a remembered value +} + +export class WidgetPermissionStore { + private static internalInstance: WidgetPermissionStore; + + private constructor() { + } + + public static get instance(): WidgetPermissionStore { + if (!WidgetPermissionStore.internalInstance) { + WidgetPermissionStore.internalInstance = new WidgetPermissionStore(); + } + return WidgetPermissionStore.internalInstance; + } + + // TODO (all functions here): Merge widgetKind with the widget definition + + private packSettingKey(widget: Widget, kind: WidgetKind, roomId?: string): string { + let location = roomId; + if (kind !== WidgetKind.Room) { + location = MatrixClientPeg.get().getUserId(); + } + if (kind === WidgetKind.Modal) { + location = '*MODAL*-' + location; // to guarantee differentiation from whatever spawned it + } + if (!location) { + throw new Error("Failed to determine a location to check the widget's OIDC state with"); + } + + return encodeURIComponent(`${location}::${widget.templateUrl}`); + } + + public getOIDCState(widget: Widget, kind: WidgetKind, roomId?: string): OIDCState { + const settingsKey = this.packSettingKey(widget, kind, roomId); + const settings = SettingsStore.getValue("widgetOpenIDPermissions"); + if (settings?.deny?.includes(settingsKey)) { + return OIDCState.Denied; + } + if (settings?.allow?.includes(settingsKey)) { + return OIDCState.Allowed; + } + return OIDCState.Unknown; + } + + public setOIDCState(widget: Widget, kind: WidgetKind, roomId: string, newState: OIDCState) { + const settingsKey = this.packSettingKey(widget, kind, roomId); + + const currentValues = SettingsStore.getValue("widgetOpenIDPermissions"); + if (!currentValues.allow) currentValues.allow = []; + if (!currentValues.deny) currentValues.deny = []; + + if (newState === OIDCState.Allowed) { + currentValues.allow.push(settingsKey); + } else if (newState === OIDCState.Denied) { + currentValues.deny.push(settingsKey); + } else { + currentValues.allow = currentValues.allow.filter(c => c !== settingsKey); + currentValues.deny = currentValues.deny.filter(c => c !== settingsKey); + } + + SettingsStore.setValue("widgetOpenIDPermissions", null, SettingLevel.DEVICE, currentValues); + } +} diff --git a/src/utils/WidgetUtils.ts b/src/utils/WidgetUtils.ts index 526c2d5ce7..986c68342c 100644 --- a/src/utils/WidgetUtils.ts +++ b/src/utils/WidgetUtils.ts @@ -22,7 +22,6 @@ import SdkConfig from "../SdkConfig"; import dis from '../dispatcher/dispatcher'; import WidgetEchoStore from '../stores/WidgetEchoStore'; import SettingsStore from "../settings/SettingsStore"; -import ActiveWidgetStore from "../stores/ActiveWidgetStore"; import {IntegrationManagers} from "../integrations/IntegrationManagers"; import {Room} from "matrix-js-sdk/src/models/room"; import {WidgetType} from "../widgets/WidgetType"; @@ -457,27 +456,6 @@ export default class WidgetUtils { return capWhitelist; } - static getWidgetSecurityKey(widgetId: string, widgetUrl: string, isUserWidget: boolean): string { - let widgetLocation = ActiveWidgetStore.getRoomId(widgetId); - - if (isUserWidget) { - const userWidget = WidgetUtils.getUserWidgetsArray() - .find((w) => w.id === widgetId && w.content && w.content.url === widgetUrl); - - if (!userWidget) { - throw new Error("No matching user widget to form security key"); - } - - widgetLocation = userWidget.sender; - } - - if (!widgetLocation) { - throw new Error("Failed to locate where the widget resides"); - } - - return encodeURIComponent(`${widgetLocation}::${widgetUrl}`); - } - static getLocalJitsiWrapperUrl(opts: {forLocalRender?: boolean, auth?: string} = {}) { // NB. we can't just encodeURIComponent all of these because the $ signs need to be there const queryStringParts = [