Module update to configure widget permissions

[maheichyk]

We would like to have a possibility to configure widget permissions in custom fork deployments.

These PRs: matrix-org/matrix-react-sdk#9931, matrix-org/matrix-react-sdk#9910 were used to try to update already existing API from matrix-react-sdk but it was agreed that module system should be used for that.

There doesn't seem to be an API currently in the module system that could be used to directly customize this behavior.

This PR suggests to update RuntimeModule with WidgetPermissions that will allow to specify widget permissions behavior in the module implementation.

In this case it will be possible to create a PR to matrix-react-sdk to:

• update ModuleRunner to read WidgetPermissions from the modules and expose it via getWidgetPermissions(), if several modules define WidgetPermissions then the decision which one to use (or maybe to fail) should be made.
• lines like https://github.com/matrix-org/matrix-react-sdk/blob/441ad40e5549470d390ef20fc347e8cb2af4f4f5/src/stores/widgets/StopGapWidgetDriver.ts#L175 should be updated to use getWidgetPermissions() from the ModuleRunner.

[andybalaam]

This change looks reasonably sensible from my limited knowledge. Some questions:

• @maheichyk please can we have some tests for this code. Perhaps just some confirmation that it's possible to create a DefaultWidgetPermissions and it has the expected properties, but if you can think of better tests, please go ahead :-)
• @turt2live you are the only contributor to this repo. Please could you either train me up sufficiently to review this, or review it yourself?
• @dkasak I think you (or someone on your team) might be interested in this from a security perspective?

[andybalaam]

I've arranged to meet @turt2live on Mon 6th Feb to learn enough about this to be able to review it.

[maheichyk]

@andybalaam I have added a simple test to check DefaultWidgetPermissions

[andybalaam]

Hi @maheichyk , thank you for your work on this.

Yesterday I learned a lot from @turt2live about what the module API is, and how it should work, so I'm ready to give you some feedback on this PR.

The main feedback is that we want the module API to work mainly via ModuleRunner.instance.invoke calls, rather than adding new methods to the interface. We think that what you are trying to do can be done that way.

Generally, calls to ModuleRunner.instance.invoke are "events" that pass in a modifiable object that you can use to influence what happens next.

Here are my notes on how we think this would work:

For preapproveCapabilities we think it should look a little like RoomViewLifecycle, something like this:

In matrix-react-sdk:

ModuleRunner.instance.invoke(
    WidgetLifecycle.CapabilitiesRequest,
    capabilitiesOpts,
    widgetInfo: WidgetInfo,
    requestedCapabilities: Set<string>,
)

In module API:

/**
 * All the info you need to identify a widget.
 * (Similar to Widget in the widget api but we don't want to have a dep
 * on that).
 */
class WidgetInfo {
// e.g. widgetId, widgetType etc.
}

export enum WidgetLifecycle {
    CapabilitiesRequest = "capabilities_request",
}

export type CapabilitiesOpts = {
    approvedCapabilities: Set<string>,
};

/**
 * Helper type that documents how the implement a capabilities listener.
 */
export type CapabilitiesListener = (
    capabilitiesOpts: CapabilitiesOpts,
    widgetInfo: WidgetInfo,
    requestedCapabilities: Set<string>,
) => void;

Then for isEmbeddingPreapproved we think it could be similar to JoinFromRoomPreview - just a yes or no:

WidgetLifecycle.PreLoad - widget is about to load - shall I prompt?
// opts object pre-populated with whether the prompt will be shown, and if you modify it, don't show it and remember for next time, and don't show the "revoke" button.
export type PreloadOpts = {
    shouldPrompt: boolean,
};

isIdentityRequestPreapproved would be similar again, but maybe with a 3-state:

enum ApprovalState {
    approved,
    declined,
    promptUser,
}
WidgetLifecycle.IdentityRequest
export type IdentityRequestOpts = {
    approvalState: ApprovalState,
};

In all 3 cases, the Opts variable would be passed to the module code with a value representing the current state, and the module would be able to modify its contents. Then matrix-react-sdk would behave according to the modified values.

Does this make sense? Do ask questions if you need clarification.

[maheichyk]

Hi @andybalaam thanks a lot for the explanations and details regarding module API, that is very helpful.

I created another PR to module API: #11 that is based on your input and a draft PR to matrix react sdk matrix-org/matrix-react-sdk#10121 that shows how these module API changes could be used. Locally I implemented a simple module that is called and approves permissions via these API changes as a proof of concept.

I have slightly modified your suggestions basically to use a single listener type ApprovalListener for both preload_request and identity_request and make the opts to contain a single field approved: boolean | undefined; that could be used by react sdk and module implementation. In my opinion it simplifies a bit the usage in react sdk and covers the necessary cases from module implementation to explicitly grant permission via true or false or keep a default behavior in case of undefined.

Maybe a small remark about the API usage, for example to request capabilities:

const opts: CapabilitiesOpts = { approvedCapabilities: undefined };
ModuleRunner.instance.invoke(WidgetLifecycle.CapabilitiesRequest, opts, this.forWidget, requested);
approved = opts.approvedCapabilities;

then it is quite easy to write a code that calls invoke with wrong params because types are not checked for invoke to match actual listener type. Maybe this could be improved.

[andybalaam]

Closing in favour of #11