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.
This commit is contained in:
Travis Ralston 2020-11-25 18:39:11 -07:00
parent c91dc55bc1
commit 5da27aed94
5 changed files with 119 additions and 50 deletions

View file

@ -17,18 +17,17 @@ limitations under the License.
import React from 'react'; import React from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import {_t} from "../../../languageHandler"; import {_t} from "../../../languageHandler";
import SettingsStore from "../../../settings/SettingsStore";
import * as sdk from "../../../index"; import * as sdk from "../../../index";
import LabelledToggleSwitch from "../elements/LabelledToggleSwitch"; import LabelledToggleSwitch from "../elements/LabelledToggleSwitch";
import WidgetUtils from "../../../utils/WidgetUtils"; import {Widget} from "matrix-widget-api";
import {SettingLevel} from "../../../settings/SettingLevel"; import {OIDCState, WidgetPermissionStore} from "../../../stores/widgets/WidgetPermissionStore";
export default class WidgetOpenIDPermissionsDialog extends React.Component { export default class WidgetOpenIDPermissionsDialog extends React.Component {
static propTypes = { static propTypes = {
onFinished: PropTypes.func.isRequired, onFinished: PropTypes.func.isRequired,
widgetUrl: PropTypes.string.isRequired, widget: PropTypes.objectOf(Widget).isRequired,
widgetId: PropTypes.string.isRequired, widgetKind: PropTypes.string.isRequired, // WidgetKind from widget-api
isUserWidget: PropTypes.bool.isRequired, inRoomId: PropTypes.string,
}; };
constructor() { constructor() {
@ -51,16 +50,10 @@ export default class WidgetOpenIDPermissionsDialog extends React.Component {
if (this.state.rememberSelection) { if (this.state.rememberSelection) {
console.log(`Remembering ${this.props.widgetId} as allowed=${allowed} for OpenID`); console.log(`Remembering ${this.props.widgetId} as allowed=${allowed} for OpenID`);
const currentValues = SettingsStore.getValue("widgetOpenIDPermissions"); WidgetPermissionStore.instance.setOIDCState(
if (!currentValues.allow) currentValues.allow = []; this.props.widget, this.props.widgetKind, this.props.inRoomId,
if (!currentValues.deny) currentValues.deny = []; allowed ? OIDCState.Allowed : OIDCState.Denied,
);
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);
} }
this.props.onFinished(allowed); 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. " + "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 " + "By allowing this, the widget will be able to verify your user ID, but not " +
"perform actions as you.", { "perform actions as you.", {
widgetUrl: this.props.widgetUrl.split("?")[0], widgetUrl: this.props.widget.templateUrl.split("?")[0],
}, },
)} )}
</p> </p>

View file

@ -246,7 +246,7 @@ export class StopGapWidget extends EventEmitter {
public start(iframe: HTMLIFrameElement) { public start(iframe: HTMLIFrameElement) {
if (this.started) return; if (this.started) return;
const allowedCapabilities = this.appTileProps.whitelistCapabilities || []; 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 = new ClientWidgetApi(this.mockWidget, iframe, driver);
this.messaging.on("preparing", () => this.emit("preparing")); this.messaging.on("preparing", () => this.emit("preparing"));
this.messaging.on("ready", () => this.emit("ready")); this.messaging.on("ready", () => this.emit("ready"));

View file

@ -30,13 +30,12 @@ import { iterableDiff, iterableUnion } from "../../utils/iterables";
import { MatrixClientPeg } from "../../MatrixClientPeg"; import { MatrixClientPeg } from "../../MatrixClientPeg";
import ActiveRoomObserver from "../../ActiveRoomObserver"; import ActiveRoomObserver from "../../ActiveRoomObserver";
import Modal from "../../Modal"; import Modal from "../../Modal";
import WidgetUtils from "../../utils/WidgetUtils";
import SettingsStore from "../../settings/SettingsStore";
import WidgetOpenIDPermissionsDialog from "../../components/views/dialogs/WidgetOpenIDPermissionsDialog"; import WidgetOpenIDPermissionsDialog from "../../components/views/dialogs/WidgetOpenIDPermissionsDialog";
import WidgetCapabilitiesPromptDialog, { import WidgetCapabilitiesPromptDialog, {
getRememberedCapabilitiesForWidget, getRememberedCapabilitiesForWidget,
} from "../../components/views/dialogs/WidgetCapabilitiesPromptDialog"; } from "../../components/views/dialogs/WidgetCapabilitiesPromptDialog";
import { WidgetPermissionCustomisations } from "../../customisations/WidgetPermissions"; import { WidgetPermissionCustomisations } from "../../customisations/WidgetPermissions";
import { OIDCState, WidgetPermissionStore } from "./WidgetPermissionStore";
// TODO: Purge this from the universe // TODO: Purge this from the universe
@ -44,7 +43,12 @@ export class StopGapWidgetDriver extends WidgetDriver {
private allowedCapabilities: Set<Capability>; private allowedCapabilities: Set<Capability>;
// TODO: Refactor widgetKind into the Widget class // 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(); super();
// Always allow screenshots to be taken because it's a client-induced flow. The widget can't // 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<IOpenIDUpdate>) { public async askOpenID(observer: SimpleObservable<IOpenIDUpdate>) {
const isUserWidget = this.forWidgetKind !== WidgetKind.Room; // modal and account widgets are "user" widgets const isUserWidget = this.forWidgetKind !== WidgetKind.Room; // modal and account widgets are "user" widgets
const rawUrl = this.forWidget.templateUrl; 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<IOpenIDCredentials> => { const getToken = (): Promise<IOpenIDCredentials> => {
return MatrixClientPeg.get().getOpenIdToken(); return MatrixClientPeg.get().getOpenIdToken();
}; };
const settings = SettingsStore.getValue("widgetOpenIDPermissions"); if (oidcState === OIDCState.Denied) {
if (settings?.deny?.includes(widgetSecurityKey)) {
return observer.update({state: OpenIDRequestState.Blocked}); return observer.update({state: OpenIDRequestState.Blocked});
} }
if (settings?.allow?.includes(widgetSecurityKey)) { if (oidcState === OIDCState.Allowed) {
return observer.update({state: OpenIDRequestState.Allowed, token: await getToken()}); return observer.update({state: OpenIDRequestState.Allowed, token: await getToken()});
} }
observer.update({state: OpenIDRequestState.PendingUserConfirmation}); observer.update({state: OpenIDRequestState.PendingUserConfirmation});
Modal.createTrackedDialog("OpenID widget permissions", '', WidgetOpenIDPermissionsDialog, { Modal.createTrackedDialog("OpenID widget permissions", '', WidgetOpenIDPermissionsDialog, {
widgetUrl: rawUrl, widget: this.forWidget,
widgetId: this.forWidget.id, widgetKind: this.forWidgetKind,
isUserWidget: isUserWidget, inRoomId: this.inRoomId,
onFinished: async (confirm) => { onFinished: async (confirm) => {
if (!confirm) { if (!confirm) {

View file

@ -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);
}
}

View file

@ -22,7 +22,6 @@ import SdkConfig from "../SdkConfig";
import dis from '../dispatcher/dispatcher'; import dis from '../dispatcher/dispatcher';
import WidgetEchoStore from '../stores/WidgetEchoStore'; import WidgetEchoStore from '../stores/WidgetEchoStore';
import SettingsStore from "../settings/SettingsStore"; import SettingsStore from "../settings/SettingsStore";
import ActiveWidgetStore from "../stores/ActiveWidgetStore";
import {IntegrationManagers} from "../integrations/IntegrationManagers"; import {IntegrationManagers} from "../integrations/IntegrationManagers";
import {Room} from "matrix-js-sdk/src/models/room"; import {Room} from "matrix-js-sdk/src/models/room";
import {WidgetType} from "../widgets/WidgetType"; import {WidgetType} from "../widgets/WidgetType";
@ -457,27 +456,6 @@ export default class WidgetUtils {
return capWhitelist; 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} = {}) { static getLocalJitsiWrapperUrl(opts: {forLocalRender?: boolean, auth?: string} = {}) {
// NB. we can't just encodeURIComponent all of these because the $ signs need to be there // NB. we can't just encodeURIComponent all of these because the $ signs need to be there
const queryStringParts = [ const queryStringParts = [