Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Support for MSC3882 revision 1 #10443

Merged
merged 8 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/components/views/settings/devices/LoginWithQRSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@ limitations under the License.
*/

import React from "react";
import {
IMSC3882GetLoginTokenCapability,
IServerVersions,
UNSTABLE_MSC3882_CAPABILITY,
} from "matrix-js-sdk/src/matrix";

import type { IServerVersions } from "matrix-js-sdk/src/matrix";
import { _t } from "../../../../languageHandler";
import AccessibleButton from "../../elements/AccessibleButton";
import SettingsSubsection from "../shared/SettingsSubsection";

interface IProps {
onShowQr: () => void;
versions?: IServerVersions;
// we can't use the capabilities type from the js-sdk because it isn't exported
capabilities?: Record<string, any>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything to stop us from exporting it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing insurmountable:

The is a name clash on ICapabilities with https://github.com/matrix-org/matrix-js-sdk/blob/b6699ba4c3463c84589d12cde18a25ed502c56b1/src/embedded.ts#L47 which describes embedded client capabilities.

So, it the homeserver capabilities response would need exporting as another name but this wouldn't be a breaking change.

I can make another PR for this if you like?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make another PR for this if you like?

👍 Same name would just be an inconvenience by using an alias. But if creating a new export you can just rename it to something more specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created matrix-org/matrix-js-sdk#3266 to do the export.

}

export default class LoginWithQRSection extends React.Component<IProps> {
Expand All @@ -33,7 +39,10 @@ export default class LoginWithQRSection extends React.Component<IProps> {

public render(): JSX.Element | null {
// Needs server support for MSC3882 and MSC3886:
const msc3882Supported = !!this.props.versions?.unstable_features?.["org.matrix.msc3882"];
// in r0 of MSC3882 it is exposed as a feature flag, but in r1 it is a capability
const capability = UNSTABLE_MSC3882_CAPABILITY.findIn<IMSC3882GetLoginTokenCapability>(this.props.capabilities);
const msc3882Supported =
!!this.props.versions?.unstable_features?.["org.matrix.msc3882"] || !!capability?.enabled;
const msc3886Supported = !!this.props.versions?.unstable_features?.["org.matrix.msc3886"];
const offerShowQr = msc3882Supported && msc3886Supported;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ interface IState {
invitedRoomIds: Set<string>;
showLoginWithQR: Mode | null;
versions?: IServerVersions;
// we can't use the capabilities type from the js-sdk because it isn't exported
capabilities?: Record<string, any>;
}

export default class SecurityUserSettingsTab extends React.Component<IProps, IState> {
Expand Down Expand Up @@ -116,6 +118,9 @@ export default class SecurityUserSettingsTab extends React.Component<IProps, ISt
MatrixClientPeg.get()
.getVersions()
.then((versions) => this.setState({ versions }));
MatrixClientPeg.get()
.getCapabilities()
.then((capabilities) => this.setState({ capabilities }));
}

public componentWillUnmount(): void {
Expand Down Expand Up @@ -393,7 +398,11 @@ export default class SecurityUserSettingsTab extends React.Component<IProps, ISt
</span>
<DevicesPanel />
</div>
<LoginWithQRSection onShowQr={this.onShowQRClicked} versions={this.state.versions} />
<LoginWithQRSection
onShowQr={this.onShowQRClicked}
versions={this.state.versions}
capabilities={this.state.capabilities}
/>
</>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ const SessionManagerTab: React.FC = () => {
const userId = matrixClient.getUserId();
const currentUserMember = (userId && matrixClient.getUser(userId)) || undefined;
const clientVersions = useAsyncMemo(() => matrixClient.getVersions(), [matrixClient]);
const capabilities = useAsyncMemo(async () => matrixClient?.getCapabilities(), [matrixClient]);

const onDeviceExpandToggle = (deviceId: ExtendedDevice["device_id"]): void => {
if (expandedDeviceIds.includes(deviceId)) {
Expand Down Expand Up @@ -279,7 +280,7 @@ const SessionManagerTab: React.FC = () => {
/>
</SettingsSubsection>
)}
<LoginWithQRSection onShowQr={onShowQrClicked} versions={clientVersions} />
<LoginWithQRSection onShowQr={onShowQrClicked} versions={clientVersions} capabilities={capabilities} />
</SettingsTab>
);
};
Expand Down
6 changes: 4 additions & 2 deletions test/components/views/settings/devices/LoginWithQR-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { cleanup, render, waitFor } from "@testing-library/react";
import { mocked } from "jest-mock";
import React from "react";
import { MSC3906Rendezvous, RendezvousFailureReason } from "matrix-js-sdk/src/rendezvous";
import { LoginTokenPostResponse } from "matrix-js-sdk/src/@types/auth";

import LoginWithQR, { Click, Mode, Phase } from "../../../../../src/components/views/auth/LoginWithQR";
import type { MatrixClient } from "matrix-js-sdk/src/matrix";
Expand Down Expand Up @@ -87,8 +88,9 @@ describe("<LoginWithQR />", () => {
jest.spyOn(MSC3906Rendezvous.prototype, "verifyNewDeviceOnExistingDevice").mockResolvedValue(undefined);
client.requestLoginToken.mockResolvedValue({
login_token: "token",
expires_in: 1000,
});
expires_in: 1000, // this is as per MSC3882 r0
expires_in_ms: 1000 * 1000, // this is as per MSC3882 r1
} as LoginTokenPostResponse); // we force the type here so that it works with versions of js-sdk that don't have r1 support yet
});

afterEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import { render } from "@testing-library/react";
import { mocked } from "jest-mock";
import { IServerVersions, MatrixClient } from "matrix-js-sdk/src/matrix";
import { IServerVersions, MatrixClient, UNSTABLE_MSC3882_CAPABILITY } from "matrix-js-sdk/src/matrix";
import React from "react";

import LoginWithQRSection from "../../../../../src/components/views/settings/devices/LoginWithQRSection";
Expand Down Expand Up @@ -69,6 +69,23 @@ describe("<LoginWithQRSection />", () => {
const { container } = render(getComponent({ versions: makeVersions({ "org.matrix.msc3882": true }) }));
expect(container).toMatchSnapshot();
});

it("only MSC3882 r1 enabled", async () => {
const { container } = render(
getComponent({ capabilities: { [UNSTABLE_MSC3882_CAPABILITY.name]: { enabled: true } } }),
);
expect(container).toMatchSnapshot();
});

it("MSC3886 + MSC3882 r1 disabled", async () => {
const { container } = render(
getComponent({
versions: makeVersions({ "org.matrix.msc3886": true }),
capabilities: { [UNSTABLE_MSC3882_CAPABILITY.name]: { enabled: false } },
}),
);
expect(container).toMatchSnapshot();
});
});

describe("should render panel", () => {
Expand All @@ -83,5 +100,19 @@ describe("<LoginWithQRSection />", () => {
);
expect(container).toMatchSnapshot();
});

it("MSC3882 r1 + MSC3886", async () => {
const { container } = render(
getComponent({
versions: makeVersions({
"org.matrix.msc3886": true,
}),
capabilities: {
[UNSTABLE_MSC3882_CAPABILITY.name]: { enabled: true },
},
}),
);
expect(container).toMatchSnapshot();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<LoginWithQRSection /> should not render MSC3886 + MSC3882 r1 disabled 1`] = `<div />`;

exports[`<LoginWithQRSection /> should not render no support at all 1`] = `<div />`;

exports[`<LoginWithQRSection /> should not render only MSC3882 enabled 1`] = `<div />`;

exports[`<LoginWithQRSection /> should not render only MSC3882 r1 enabled 1`] = `<div />`;

exports[`<LoginWithQRSection /> should render panel MSC3882 + MSC3886 1`] = `
<div>
<div
Expand Down Expand Up @@ -41,3 +45,41 @@ exports[`<LoginWithQRSection /> should render panel MSC3882 + MSC3886 1`] = `
</div>
</div>
`;

exports[`<LoginWithQRSection /> should render panel MSC3882 r1 + MSC3886 1`] = `
<div>
<div
class="mx_SettingsSubsection"
>
<div
class="mx_SettingsSubsectionHeading"
>
<h3
class="mx_Heading_h3 mx_SettingsSubsectionHeading_heading"
>
Sign in with QR code
</h3>
</div>
<div
class="mx_SettingsSubsection_content"
>
<div
class="mx_LoginWithQRSection"
>
<p
class="mx_SettingsTab_subsectionText"
>
You can use this device to sign in a new device with a QR code. You will need to scan the QR code shown on this device with your device that's signed out.
</p>
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
role="button"
tabindex="0"
>
Show QR code
</div>
</div>
</div>
</div>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { fireEvent, render } from "@testing-library/react";
import { UNSTABLE_MSC3882_CAPABILITY } from "matrix-js-sdk/src/matrix";
import React from "react";

import SecurityUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/SecurityUserSettingsTab";
Expand Down Expand Up @@ -45,10 +46,14 @@ describe("<SecurityUserSettingsTab />", () => {
getIgnoredUsers: jest.fn(),
getVersions: jest.fn().mockResolvedValue({
unstable_features: {
"org.matrix.msc3882": true,
"org.matrix.msc3886": true,
},
}),
getCapabilities: jest.fn().mockResolvedValue({
[UNSTABLE_MSC3882_CAPABILITY.name]: {
enabled: true,
},
}),
});

const getComponent = () => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
PUSHER_DEVICE_ID,
PUSHER_ENABLED,
IAuthData,
UNSTABLE_MSC3882_CAPABILITY,
} from "matrix-js-sdk/src/matrix";

import { clearAllModals } from "../../../../../test-utils";
Expand Down Expand Up @@ -97,6 +98,7 @@ describe("<SessionManagerTab />", () => {
setPusher: jest.fn(),
setLocalNotificationSettings: jest.fn(),
getVersions: jest.fn().mockResolvedValue({}),
getCapabilities: jest.fn().mockResolvedValue({}),
});

const defaultProps = {};
Expand Down Expand Up @@ -1375,10 +1377,14 @@ describe("<SessionManagerTab />", () => {
mockClient.getVersions.mockResolvedValue({
versions: [],
unstable_features: {
"org.matrix.msc3882": true,
"org.matrix.msc3886": true,
},
});
mockClient.getCapabilities.mockResolvedValue({
[UNSTABLE_MSC3882_CAPABILITY.name]: {
enabled: true,
},
});
});

it("renders qr code login section", async () => {
Expand Down