diff --git a/.github/actions/sign-release-tarball/action.yml b/.github/actions/sign-release-tarball/action.yml new file mode 100644 index 00000000000..cd663cd873c --- /dev/null +++ b/.github/actions/sign-release-tarball/action.yml @@ -0,0 +1,28 @@ +name: Sign Release Tarball +description: Generates signature for release tarball and uploads it as a release asset +inputs: + gpg-fingerprint: + description: Fingerprint of the GPG key to use for signing the tarball. + required: true + upload-url: + description: GitHub release upload URL to upload the signature file to. + required: true +runs: + using: composite + steps: + - name: Generate tarball signature + shell: bash + run: | + git -c tar.tar.gz.command='gzip -cn' archive --format=tar.gz --prefix="${REPO#*/}-${VERSION#v}/" -o "/tmp/${VERSION}.tar.gz" "${VERSION}" + gpg -u "$GPG_FINGERPRINT" --armor --output "${VERSION}.tar.gz.asc" --detach-sig "/tmp/${VERSION}.tar.gz" + rm "/tmp/${VERSION}.tar.gz" + env: + GPG_FINGERPRINT: ${{ inputs.gpg-fingerprint }} + REPO: ${{ github.repository }} + + - name: Upload tarball signature + if: ${{ inputs.upload-url }} + uses: shogo82148/actions-upload-release-asset@dccd6d23e64fd6a746dce6814c0bde0a04886085 # v1 + with: + upload_url: ${{ inputs.upload-url }} + asset_path: ${{ env.VERSION }}.tar.gz.asc diff --git a/.github/actions/upload-release-assets/action.yml b/.github/actions/upload-release-assets/action.yml new file mode 100644 index 00000000000..25eb7f03940 --- /dev/null +++ b/.github/actions/upload-release-assets/action.yml @@ -0,0 +1,41 @@ +name: Upload release assets +description: Uploads assets to an existing release and optionally signs them +inputs: + gpg-fingerprint: + description: Fingerprint of the GPG key to use for signing the assets, if any. + required: false + upload-url: + description: GitHub release upload URL to upload the assets to. + required: true + asset-path: + description: | + The path to the asset you want to upload, if any. You can use glob patterns here. + Will be GPG signed and an `.asc` file included in the release artifacts if `gpg-fingerprint` is set. + required: true +runs: + using: composite + steps: + - name: Sign assets + if: inputs.gpg-fingerprint + shell: bash + run: | + for FILE in $ASSET_PATH + do + gpg -u "$GPG_FINGERPRINT" --armor --output "$FILE".asc --detach-sig "$FILE" + done + env: + GPG_FINGERPRINT: ${{ inputs.gpg-fingerprint }} + ASSET_PATH: ${{ inputs.asset-path }} + + - name: Upload asset signatures + if: inputs.gpg-fingerprint + uses: shogo82148/actions-upload-release-asset@dccd6d23e64fd6a746dce6814c0bde0a04886085 # v1 + with: + upload_url: ${{ inputs.upload-url }} + asset_path: ${{ inputs.asset-path }}.asc + + - name: Upload assets + uses: shogo82148/actions-upload-release-asset@dccd6d23e64fd6a746dce6814c0bde0a04886085 # v1 + with: + upload_url: ${{ inputs.upload-url }} + asset_path: ${{ inputs.asset-path }} diff --git a/.github/workflows/cypress.yml b/.github/workflows/cypress.yml index a40711a9ca1..9fe20bc3ffd 100644 --- a/.github/workflows/cypress.yml +++ b/.github/workflows/cypress.yml @@ -20,7 +20,7 @@ jobs: # from creeping in. They take a long time to run and consume 4 concurrent runners. if: github.event.workflow_run.event == 'merge_group' - uses: matrix-org/matrix-react-sdk/.github/workflows/cypress.yaml@v3.83.0-rc.1 + uses: matrix-org/matrix-react-sdk/.github/workflows/cypress.yaml@03b01b4a50d0f3fbbfa6c1a9314ef2d346d089d4 permissions: actions: read issues: read @@ -33,7 +33,6 @@ jobs: TCMS_PASSWORD: ${{ secrets.TCMS_PASSWORD }} with: react-sdk-repository: matrix-org/matrix-react-sdk - rust-crypto: true # We want to make the cypress tests a required check for the merge queue. # diff --git a/.github/workflows/downstream-artifacts.yml b/.github/workflows/downstream-artifacts.yml index 09cd350de48..c58fccc9b6e 100644 --- a/.github/workflows/downstream-artifacts.yml +++ b/.github/workflows/downstream-artifacts.yml @@ -20,7 +20,7 @@ concurrency: jobs: build-element-web: name: Build element-web - uses: matrix-org/matrix-react-sdk/.github/workflows/element-web.yaml@v3.83.0 + uses: matrix-org/matrix-react-sdk/.github/workflows/element-web.yaml@v3.84.0 with: matrix-js-sdk-sha: ${{ github.sha }} react-sdk-repository: matrix-org/matrix-react-sdk diff --git a/.github/workflows/release-action.yml b/.github/workflows/release-action.yml new file mode 100644 index 00000000000..aba9ae28d46 --- /dev/null +++ b/.github/workflows/release-action.yml @@ -0,0 +1,321 @@ +name: Release Action +on: + workflow_call: + secrets: + ELEMENT_BOT_TOKEN: + required: true + NPM_TOKEN: + required: false + GPG_PASSPHRASE: + required: false + GPG_PRIVATE_KEY: + required: false + inputs: + final: + description: Make final release + required: true + default: false + type: boolean + npm: + description: Publish to npm + type: boolean + default: false + dependencies: + description: | + List of dependencies to update in `npm-dep=version` format. + `version` can be `"current"` to leave it at the current version. + type: string + required: false + include-changes: + description: Project to include changelog entries from in this release. + type: string + required: false + gpg-fingerprint: + description: Fingerprint of the GPG key to use for signing the git tag and assets, if any. + type: string + required: false + asset-path: + description: | + The path to the asset you want to upload, if any. You can use glob patterns here. + Will be GPG signed and an `.asc` file included in the release artifacts if `gpg-fingerprint` is set. + type: string + required: false + expected-asset-count: + description: The number of expected assets, including signatures, excluding generated zip & tarball. + type: number + required: false +jobs: + release: + name: Release + runs-on: ubuntu-latest + environment: Release + steps: + - name: Load GPG key + id: gpg + if: inputs.gpg-fingerprint + uses: crazy-max/ghaction-import-gpg@82a020f1f7f605c65dd2449b392a52c3fcfef7ef # v6 + with: + gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }} + passphrase: ${{ secrets.GPG_PASSPHRASE }} + fingerprint: ${{ inputs.gpg-fingerprint }} + + - name: Get draft release + id: release + uses: cardinalby/git-get-release-action@cedef2faf69cb7c55b285bad07688d04430b7ada # v1 + env: + GITHUB_TOKEN: ${{ github.token }} + with: + draft: true + latest: true + + - uses: actions/checkout@v4 + with: + ref: staging + token: ${{ secrets.ELEMENT_BOT_TOKEN }} + fetch-depth: 0 + + - name: Get actions scripts + uses: actions/checkout@v4 + with: + repository: matrix-org/matrix-js-sdk + persist-credentials: false + path: .action-repo + sparse-checkout: | + .github/actions + scripts/release + + - name: Prepare variables + id: prepare + run: | + echo "VERSION=$VERSION" >> $GITHUB_ENV + { + echo "RELEASE_NOTES<> $GITHUB_ENV + + HAS_DIST=0 + jq -e .scripts.dist package.json >/dev/null 2>&1 && HAS_DIST=1 + echo "has-dist-script=$HAS_DIST" >> $GITHUB_OUTPUT + env: + BODY: ${{ steps.release.outputs.body }} + VERSION: ${{ steps.release.outputs.tag_name }} + + - name: Finalise version + if: inputs.mode == 'final' + run: echo "VERSION=$(echo $VERSION | cut -d- -f1)" >> $GITHUB_ENV + + - name: Set up git + run: | + git config --global user.email "releases@riot.im" + git config --global user.name "RiotRobot" + + - uses: actions/setup-node@v3 + with: + cache: "yarn" + + - name: Install dependencies + run: "yarn install --frozen-lockfile" + + - name: Update dependencies + id: update-dependencies + if: inputs.dependencies + run: | + UPDATED=() + while IFS= read -r DEPENDENCY; do + [ -z "$DEPENDENCY" ] && continue + IFS="=" read -r PACKAGE UPDATE_VERSION <<< "$DEPENDENCY" + + CURRENT_VERSION=$(cat package.json | jq -r .dependencies[\"$PACKAGE\"]) + echo "Current $PACKAGE version is $CURRENT_VERSION" + + if [ "$CURRENT_VERSION" == "null" ] + then + echo "Unable to find $PACKAGE in package.json" + exit 1 + fi + + if [ "$UPDATE_VERSION" == "current" ] || [ "$UPDATE_VERSION" == "$CURRENT_VERSION" ] + then + echo "Not updating dependency $PACKAGE" + continue + fi + + echo "Upgrading $PACKAGE to $UPDATE_VERSION..." + yarn upgrade "$PACKAGE@$UPDATE_VERSION" --exact + git add -u + git commit -m "Upgrade $PACKAGE to $UPDATE_VERSION" + UPDATED+=("$PACKAGE") + done <<< "$DEPENDENCIES" + + JSON=$(jq --compact-output --null-input '$ARGS.positional' --args -- "${UPDATED[@]}") + echo "updated=$JSON" >> $GITHUB_OUTPUT + env: + DEPENDENCIES: ${{ inputs.dependencies }} + + - name: Prevent develop dependencies + if: inputs.dependencies + run: | + ret=0 + cat package.json | jq '.dependencies[]' | grep -q '#develop' || ret=$? + if [ "$ret" -eq 0 ]; then + echo "package.json contains develop dependencies. Refusing to release." + exit + fi + + - name: Bump package.json version + run: yarn version --no-git-tag-version --new-version "$VERSION" + + - name: Ingest upstream changes + if: | + inputs.dependencies && + inputs.include-changes && + contains(fromJSON(steps.update-dependencies.outputs.updated), inputs.include-changes) + uses: actions/github-script@v6 + env: + RELEASE_ID: ${{ steps.upstream-release.outputs.body }} + DEPENDENCY: ${{ inputs.include-changes }} + with: + retries: 3 + script: | + const { RELEASE_ID: releaseId, DEPENDENCY } = process.env; + const { owner, repo } = context.repo; + const script = require("./.action-repo/scripts/release/merge-release-notes.js"); + const notes = await script({ + github, + releaseId, + dependencies: [DEPENDENCY], + }); + core.exportVariable("RELEASE_NOTES", notes); + + - name: Add to CHANGELOG.md + if: inputs.mode == 'final' + run: | + mv CHANGELOG.md CHANGELOG.md.old + HEADER="Changes in [${VERSION#v}](https://github.com/${{ github.repository }}/releases/tag/$VERSION) ($(date '+%Y-%m-%d'))" + + { + echo "$HEADER" + printf '=%.0s' $(seq ${#HEADER}) + echo "" + echo "$RELEASE_NOTES" + echo "" + } > CHANGELOG.md + + cat CHANGELOG.md.old >> CHANGELOG.md + rm CHANGELOG.md.old + git add CHANGELOG.md + + - name: Run pre-release script to update package.json fields + run: | + ./.action-repo/scripts/release/pre-release.sh + git add package.json + + - name: Commit changes + run: git commit -m "$VERSION" + + - name: Build assets + if: steps.prepare.outputs.has-dist-script + run: DIST_VERSION="$VERSION" yarn dist + + - name: Upload release assets & signatures + if: inputs.asset-path + uses: ./.action-repo/.github/actions/upload-release-assets + with: + gpg-fingerprint: ${{ inputs.gpg-fingerprint }} + upload-url: ${{ steps.release.outputs.upload_url }} + asset-path: ${{ inputs.asset-path }} + + - name: Create signed tag + if: inputs.gpg-fingerprint + run: | + GIT_COMMITTER_EMAIL="$SIGNING_ID" GPG_TTY=$(tty) git tag -u "$SIGNING_ID" -m "Release $VERSION" "$VERSION" + env: + SIGNING_ID: ${{ steps.gpg.outputs.email }} + + - name: Generate & upload tarball signature + if: inputs.gpg-fingerprint + uses: ./.action-repo/.github/actions/sign-release-tarball + with: + gpg-fingerprint: ${{ inputs.gpg-fingerprint }} + upload-url: ${{ steps.release.outputs.upload_url }} + + # We defer pushing changes until after the release assets are built, + # signed & uploaded to improve the atomicity of this action. + - name: Push changes to staging + run: | + git push origin staging $TAG + git reset --hard + env: + TAG: ${{ inputs.gpg-fingerprint && env.VERSION || '' }} + + - name: Validate tarball signature + if: inputs.gpg-fingerprint + run: | + wget https://github.com/$GITHUB_REPOSITORY/archive/refs/tags/$VERSION.tar.gz + gpg --verify "$VERSION.tar.gz.asc" "$VERSION.tar.gz" + + - name: Validate release has expected assets + if: inputs.expected-asset-count + uses: actions/github-script@v6 + env: + RELEASE_ID: ${{ steps.release.outputs.id }} + EXPECTED_ASSET_COUNT: ${{ inputs.expected-asset-count }} + with: + retries: 3 + script: | + const { RELEASE_ID: release_id, EXPECTED_ASSET_COUNT } = process.env; + const { owner, repo } = context.repo; + + const { data: release } = await github.rest.repos.getRelease({ + owner, + repo, + release_id, + }); + + if (release.assets.length !== parseInt(EXPECTED_ASSET_COUNT, 10)) { + core.setFailed(`Found ${release.assets.length} assets but expected ${EXPECTED_ASSET_COUNT}`); + } + + - name: Merge to master + if: inputs.final + run: | + git checkout master + git merge -X theirs staging + git push origin master + + - name: Publish release + uses: actions/github-script@v6 + env: + RELEASE_ID: ${{ steps.release.outputs.id }} + FINAL: ${{ inputs.final }} + with: + retries: 3 + script: | + const { RELEASE_ID: release_id, RELEASE_NOTES, VERSION, FINAL } = process.env; + const { owner, repo } = context.repo; + + const opts = { + owner, + repo, + release_id, + tag_name: VERSION, + name: VERSION, + draft: false, + body: RELEASE_NOTES, + }; + + if (FINAL == "true") { + opts.prerelease = false; + opts.make_latest = true; + } + + github.rest.repos.updateRelease(opts); + + npm: + name: Publish to npm + needs: release + if: inputs.npm + uses: matrix-org/matrix-js-sdk/.github/workflows/release-npm.yml@develop + secrets: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/.github/workflows/release-gitflow.yml b/.github/workflows/release-gitflow.yml new file mode 100644 index 00000000000..5a75f9a751b --- /dev/null +++ b/.github/workflows/release-gitflow.yml @@ -0,0 +1,85 @@ +# Gitflow merge-back master->develop +name: Merge master -> develop +on: + push: + branches: [master] + workflow_call: + secrets: + ELEMENT_BOT_TOKEN: + required: true + inputs: + dependencies: + description: List of dependencies to reset. + type: string + required: false +concurrency: ${{ github.workflow }} +jobs: + merge: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + token: ${{ secrets.ELEMENT_BOT_TOKEN }} + fetch-depth: 0 + + - name: Get actions scripts + uses: actions/checkout@v4 + with: + repository: matrix-org/matrix-js-sdk + persist-credentials: false + path: .action-repo + sparse-checkout: | + scripts/release + + - uses: actions/setup-node@v3 + with: + cache: "yarn" + + - name: Install Deps + run: "yarn install --frozen-lockfile" + + - name: Set up git + run: | + git config --global user.email "releases@riot.im" + git config --global user.name "RiotRobot" + + - name: Merge to develop + run: | + git checkout develop + git merge -X ours master + + - name: Run post-merge-master script to revert package.json fields + run: ./.action-repo/scripts/release/post-merge-master.sh + + - name: Reset dependencies + if: inputs.dependencies + run: | + while IFS= read -r PACKAGE; do + [ -z "$PACKAGE" ] && continue + + CURRENT_VERSION=$(cat package.json | jq -r .dependencies[\"$PACKAGE\"]) + echo "Current $PACKAGE version is $CURRENT_VERSION" + + if [ "$CURRENT_VERSION" == "null" ] + then + echo "Unable to find $PACKAGE in package.json" + exit 1 + fi + + if [ "$CURRENT_VERSION" == "develop" ] + then + echo "Not updating dependency $PACKAGE" + continue + fi + + echo "Resetting $1 to develop branch..." + yarn add "github:matrix-org/$PACKAGE#develop" + git add -u + git commit -m "Reset $PACKAGE back to develop branch" + done <<< "$DEPENDENCIES" + env: + DEPENDENCIES: ${{ inputs.dependencies }} + FINAL: ${{ inputs.final }} + + - name: Push changes + run: git push origin develop diff --git a/.github/workflows/release-master.yml b/.github/workflows/release-master.yml deleted file mode 100644 index b328af07093..00000000000 --- a/.github/workflows/release-master.yml +++ /dev/null @@ -1,56 +0,0 @@ -# Gitflow merge-back master->develop -name: Merge master -> develop -on: - push: - branches: [master] -concurrency: ${{ github.workflow }} -jobs: - merge: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - token: ${{ secrets.ELEMENT_BOT_TOKEN }} - fetch-depth: 0 - - - uses: actions/setup-node@v3 - with: - cache: "yarn" - - - name: Install Deps - run: "yarn install --frozen-lockfile" - - - name: Set up git - run: | - git config --global user.email "releases@riot.im" - git config --global user.name "RiotRobot" - - - name: Merge to develop - run: | - git checkout develop - git merge -X ours master - - - name: Update package.json fields - run: | - # When merging to develop, we need revert the `main` and `typings` fields if we adjusted them previously. - for i in main typings browser - do - # If a `lib` prefixed value is present, it means we adjusted the field - # earlier at publish time, so we should revert it now. - if [ "$(jq -r ".matrix_lib_$i" package.json)" != "null" ]; then - # If there's a `src` prefixed value, use that, otherwise delete. - # This is used to delete the `typings` field and reset `main` back to the TypeScript source. - src_value=$(jq -r ".matrix_src_$i" package.json) - if [ "$src_value" != "null" ]; then - jq ".$i = .matrix_src_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - else - jq "del(.$i)" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - fi - fi - done - - - name: Commit and push changes - run: | - git add package.json - git commit -m "Resetting package fields for development" - git push origin develop diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 66ba2b7e857..cc523b78111 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,121 +23,11 @@ on: concurrency: ${{ github.workflow }} jobs: release: - runs-on: ubuntu-latest - steps: - - name: Get draft release - id: release - uses: cardinalby/git-get-release-action@cedef2faf69cb7c55b285bad07688d04430b7ada # v1 - env: - GITHUB_TOKEN: ${{ github.token }} - with: - draft: true - latest: true - - - uses: actions/checkout@v4 - with: - ref: staging - token: ${{ secrets.ELEMENT_BOT_TOKEN }} - fetch-depth: 0 - - - uses: actions/setup-node@v3 - with: - cache: "yarn" - - - name: Install Deps - run: "yarn install --frozen-lockfile" - - - name: Load version - run: echo "VERSION=$VERSION" >> $GITHUB_ENV - env: - VERSION: ${{ steps.release.outputs.tag_name }} - - - name: Finalise version - if: inputs.mode == 'final' - run: echo "VERSION=$(echo $VERSION | cut -d- -f1)" >> $GITHUB_ENV - - - name: Bump package.json version - run: yarn version --no-git-tag-version --new-version "$VERSION" - - - name: Add to CHANGELOG.md - if: inputs.mode == 'final' - run: | - mv CHANGELOG.md CHANGELOG.md.old - HEADER="Changes in [${VERSION#v}](https://github.com/${{ github.repository }}/releases/tag/$VERSION) ($(date '+%Y-%m-%d'))" - - { - echo "$HEADER" - printf '=%.0s' $(seq ${#HEADER}) - echo "" - echo "$BODY" - echo "" - } > CHANGELOG.md - - cat CHANGELOG.md.old >> CHANGELOG.md - rm CHANGELOG.md.old - env: - BODY: ${{ steps.release.outputs.body }} - - # For the published and dist versions of the package, - # we copy the `matrix_lib_main` and `matrix_lib_typings` fields to `main` and `typings` (if they exist). - # This small bit of gymnastics allows us to use the TypeScript source directly for development without - # needing to build before linting or testing. - - name: Update package.json fields - run: | - for i in main typings browser - do - lib_value=$(jq -r ".matrix_lib_$i" package.json) - if [ "$lib_value" != "null" ]; then - jq ".$i = .matrix_lib_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - fi - done - - - name: Set up git - run: | - git config --global user.email "releases@riot.im" - git config --global user.name "RiotRobot" - - - name: Commit and push changes - run: | - git add package.json CHANGELOG.md - git commit -m "$VERSION" - git push origin staging - - - name: Merge to master - if: inputs.mode == 'final' - run: | - git checkout master - git merge -X theirs staging - git push origin master - - - name: Publish release - uses: actions/github-script@v6 - id: my-script - env: - RELEASE_ID: ${{ steps.release.outputs.id }} - MODE: ${{ inputs.mode }} - with: - result-encoding: string - retries: 3 - script: | - let { RELEASE_ID: release_id, VERSION, MODE } = process.env; - const { owner, repo } = context.repo; - - const opts = { - owner, - repo, - release_id, - tag_name: VERSION, - name: VERSION, - draft: false, - }; - - if (MODE === "final") { - opts.prerelease = false; - opts.make_latest = true; - } - - github.rest.repos.updateRelease(opts); + uses: matrix-org/matrix-js-sdk/.github/workflows/release-action.yml@develop + secrets: inherit + with: + final: ${{ inputs.mode == 'final' }} + npm: ${{ inputs.npm }} docs: name: Publish Documentation @@ -184,11 +74,3 @@ jobs: git commit -m "Update docs" git push working-directory: _docs - - npm: - name: Publish - needs: release - if: inputs.npm - uses: matrix-org/matrix-js-sdk/.github/workflows/release-npm.yml@develop - secrets: - NPM_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md index b4305e66031..f48d02866b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +Changes in [30.0.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v30.0.0) (2023-11-07) +================================================================================================== + +## 🚨 BREAKING CHANGES + * Refactor & make base64 functions browser-safe ([\#3818](https://github.com/matrix-org/matrix-js-sdk/pull/3818)). + +## 🦖 Deprecations + * Deprecate `MatrixEvent.toJSON` ([\#3801](https://github.com/matrix-org/matrix-js-sdk/pull/3801)). + +## ✨ Features + * Element-R: Add the git sha of the binding crate to `CryptoApi#getVersion` ([\#3838](https://github.com/matrix-org/matrix-js-sdk/pull/3838)). Contributed by @florianduros. + * Element-R: Wire up `globalBlacklistUnverifiedDevices` field to rust crypto encryption settings ([\#3790](https://github.com/matrix-org/matrix-js-sdk/pull/3790)). Fixes vector-im/element-web#26315. Contributed by @florianduros. + * Element-R: Wire up room rotation ([\#3807](https://github.com/matrix-org/matrix-js-sdk/pull/3807)). Fixes vector-im/element-web#26318. Contributed by @florianduros. + * Element-R: Add current version of the rust-sdk and vodozemac ([\#3825](https://github.com/matrix-org/matrix-js-sdk/pull/3825)). Contributed by @florianduros. + * Element-R: Wire up room history visibility ([\#3805](https://github.com/matrix-org/matrix-js-sdk/pull/3805)). Fixes vector-im/element-web#26319. Contributed by @florianduros. + * Element-R: log when we send to-device messages ([\#3810](https://github.com/matrix-org/matrix-js-sdk/pull/3810)). + +## 🐛 Bug Fixes + * Fix reemitter not being correctly wired on user objects created in storage classes ([\#3796](https://github.com/matrix-org/matrix-js-sdk/pull/3796)). Contributed by @MidhunSureshR. + * Element-R: silence log errors when viewing a pending event ([\#3824](https://github.com/matrix-org/matrix-js-sdk/pull/3824)). + * Don't emit a closed event if the indexeddb is closed by Element ([\#3832](https://github.com/matrix-org/matrix-js-sdk/pull/3832)). Fixes vector-im/element-web#25941. Contributed by @dhenneke. + * Element-R: silence log errors when viewing a decryption failure ([\#3821](https://github.com/matrix-org/matrix-js-sdk/pull/3821)). + Changes in [29.1.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v29.1.0) (2023-10-24) ================================================================================================== diff --git a/package.json b/package.json index 43cbcd95226..3b5f4f741e6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "29.1.0", + "version": "30.0.0", "description": "Matrix Client-Server SDK for Javascript", "engines": { "node": ">=18.0.0" @@ -95,7 +95,7 @@ "babelify": "^10.0.0", "debug": "^4.3.4", "domexception": "^4.0.0", - "eslint": "8.52.0", + "eslint": "8.53.0", "eslint-config-google": "^0.14.0", "eslint-config-prettier": "^9.0.0", "eslint-import-resolver-typescript": "^3.5.1", diff --git a/post-release.sh b/post-release.sh index 50e9579b4dc..b639770db57 100755 --- a/post-release.sh +++ b/post-release.sh @@ -10,28 +10,6 @@ set -e jq --version > /dev/null || (echo "jq is required: please install it"; kill $$) if [ "$(git branch -lr | grep origin/develop -c)" -ge 1 ]; then - # When merging to develop, we need revert the `main` and `typings` fields if we adjusted them previously. - for i in main typings browser - do - # If a `lib` prefixed value is present, it means we adjusted the field - # earlier at publish time, so we should revert it now. - if [ "$(jq -r ".matrix_lib_$i" package.json)" != "null" ]; then - # If there's a `src` prefixed value, use that, otherwise delete. - # This is used to delete the `typings` field and reset `main` back - # to the TypeScript source. - src_value=$(jq -r ".matrix_src_$i" package.json) - if [ "$src_value" != "null" ]; then - jq ".$i = .matrix_src_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - else - jq "del(.$i)" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - fi - fi - done - - if [ -n "$(git ls-files --modified package.json)" ]; then - echo "Committing develop package.json" - git commit package.json -m "Resetting package fields for development" - fi - + "$(dirname "$0")/scripts/release/post-merge-master.sh" git push origin develop fi diff --git a/release.sh b/release.sh index c341ee9f17c..83916a381f0 100755 --- a/release.sh +++ b/release.sh @@ -175,18 +175,7 @@ echo "yarn version" # manually commit the result. yarn version --no-git-tag-version --new-version "$release" -# For the published and dist versions of the package, we copy the -# `matrix_lib_main` and `matrix_lib_typings` fields to `main` and `typings` (if -# they exist). This small bit of gymnastics allows us to use the TypeScript -# source directly for development without needing to build before linting or -# testing. -for i in main typings browser -do - lib_value=$(jq -r ".matrix_lib_$i" package.json) - if [ "$lib_value" != "null" ]; then - jq ".$i = .matrix_lib_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json - fi -done +"$(dirname "$0")/scripts/release/pre-release.sh" # commit yarn.lock if it exists, is versioned, and is modified if [[ -f yarn.lock && $(git status --porcelain yarn.lock | grep '^ M') ]]; diff --git a/scripts/release/merge-release-notes.js b/scripts/release/merge-release-notes.js new file mode 100755 index 00000000000..f98aa46e945 --- /dev/null +++ b/scripts/release/merge-release-notes.js @@ -0,0 +1,79 @@ +#!/usr/bin/env node + +const fs = require("fs"); + +async function getRelease(github, dependency) { + const upstreamPackageJson = JSON.parse(fs.readFileSync(`./node_modules/${dependency}/package.json`, "utf8")); + const [owner, repo] = upstreamPackageJson.repository.url.split("/").slice(-2); + const tag = `v${upstreamPackageJson.version}`; + + const response = await github.rest.repos.getReleaseByTag({ + owner, + repo, + tag, + }); + return response.data; +} + +const main = async ({ github, releaseId, dependencies }) => { + const { GITHUB_REPOSITORY } = process.env; + const [owner, repo] = GITHUB_REPOSITORY.split("/"); + + const sections = new Map(); + let heading = null; + for (const dependency of dependencies) { + const release = await getRelease(github, dependency); + for (const line of release.body.split("\n")) { + if (line.startsWith("#")) { + heading = line; + sections.set(heading, []); + continue; + } + if (heading && line) { + sections.get(heading).push(line); + } + } + } + + const { data: release } = await github.rest.repos.getRelease({ + owner, + repo, + release_id: releaseId, + }); + + heading = null; + const output = []; + for (const line of [...release.body.split("\n"), null]) { + if (line === null || line.startsWith("#")) { + if (heading && sections.has(heading)) { + const lastIsBlank = !output.at(-1)?.trim(); + if (lastIsBlank) output.pop(); + output.push(...sections.get(heading)); + if (lastIsBlank) output.push(""); + } + heading = line; + } + output.push(line); + } + + return output.join("\n"); +}; + +// This is just for testing locally +// Needs environment variables GITHUB_TOKEN & GITHUB_REPOSITORY +if (require.main === module) { + const { Octokit } = require("@octokit/rest"); + const github = new Octokit({ auth: process.env.GITHUB_TOKEN }); + if (process.argv.length < 4) { + // eslint-disable-next-line no-console + console.error("Usage: node merge-release-notes.js owner/repo:release_id npm-package-name ..."); + process.exit(1); + } + const [releaseId, ...dependencies] = process.argv.slice(2); + main({ github, releaseId, dependencies }).then((output) => { + // eslint-disable-next-line no-console + console.log(output); + }); +} + +module.exports = main; diff --git a/scripts/release/post-merge-master.sh b/scripts/release/post-merge-master.sh new file mode 100755 index 00000000000..e76d6422111 --- /dev/null +++ b/scripts/release/post-merge-master.sh @@ -0,0 +1,22 @@ +#!/bin/bash + +# When merging to develop, we need revert the `main` and `typings` fields if we adjusted them previously. +for i in main typings browser +do + # If a `lib` prefixed value is present, it means we adjusted the field earlier at publish time, so we should revert it now. + if [ "$(jq -r ".matrix_lib_$i" package.json)" != "null" ]; then + # If there's a `src` prefixed value, use that, otherwise delete. + # This is used to delete the `typings` field and reset `main` back to the TypeScript source. + src_value=$(jq -r ".matrix_src_$i" package.json) + if [ "$src_value" != "null" ]; then + jq ".$i = .matrix_src_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json + else + jq "del(.$i)" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json + fi + fi +done + +if [ -n "$(git ls-files --modified package.json)" ]; then + echo "Committing develop package.json" + git commit package.json -m "Resetting package fields for development" +fi diff --git a/scripts/release/pre-release.sh b/scripts/release/pre-release.sh new file mode 100755 index 00000000000..6b47ef180ff --- /dev/null +++ b/scripts/release/pre-release.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +# For the published and dist versions of the package, +# we copy the `matrix_lib_main` and `matrix_lib_typings` fields to `main` and `typings` (if they exist). +# This small bit of gymnastics allows us to use the TypeScript source directly for development without +# needing to build before linting or testing. + +for i in main typings browser +do + lib_value=$(jq -r ".matrix_lib_$i" package.json) + if [ "$lib_value" != "null" ]; then + jq ".$i = .matrix_lib_$i" package.json > package.json.new && mv package.json.new package.json && yarn prettier --write package.json + fi +done diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 0cd01000b73..09cf2ebfe52 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -692,7 +692,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); it("prepareToEncrypt", async () => { - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + const homeserverUrl = aliceClient.getHomeserverUrl(); + keyResponder = new E2EKeyResponder(homeserverUrl); + keyResponder.addKeyReceiver("@alice:localhost", keyReceiver); + + const testDeviceKeys = getTestOlmAccountKeys(testOlmAccount, "@bob:xyz", "DEVICE_ID"); + keyResponder.addDeviceKeys(testDeviceKeys); + await startClientAndAwaitFirstSync(); aliceClient.setGlobalErrorOnUnknownDevices(false); @@ -700,10 +706,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, syncResponder.sendOrQueueSyncResponse(getSyncResponse(["@bob:xyz"])); await syncPromise(aliceClient); - // we expect alice first to query bob's keys... - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - - // ... and then claim one of his OTKs + // Alice should claim one of Bob's OTKs expectAliceKeyClaim(getTestKeysClaimResponse("@bob:xyz")); // fire off the prepare request @@ -720,18 +723,20 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, it("Alice sends a megolm message with GlobalErrorOnUnknownDevices=false", async () => { aliceClient.setGlobalErrorOnUnknownDevices(false); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + const homeserverUrl = aliceClient.getHomeserverUrl(); + keyResponder = new E2EKeyResponder(homeserverUrl); + keyResponder.addKeyReceiver("@alice:localhost", keyReceiver); + + const testDeviceKeys = getTestOlmAccountKeys(testOlmAccount, "@bob:xyz", "DEVICE_ID"); + keyResponder.addDeviceKeys(testDeviceKeys); + await startClientAndAwaitFirstSync(); // Alice shares a room with Bob syncResponder.sendOrQueueSyncResponse(getSyncResponse(["@bob:xyz"])); await syncPromise(aliceClient); - // Once we send the message, Alice will check Bob's device list (twice, because reasons) ... - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - - // ... and claim one of his OTKs ... + // ... and claim one of Bob's OTKs ... expectAliceKeyClaim(getTestKeysClaimResponse("@bob:xyz")); // ... and send an m.room_key message @@ -746,18 +751,20 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, it("We should start a new megolm session after forceDiscardSession", async () => { aliceClient.setGlobalErrorOnUnknownDevices(false); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + const homeserverUrl = aliceClient.getHomeserverUrl(); + keyResponder = new E2EKeyResponder(homeserverUrl); + keyResponder.addKeyReceiver("@alice:localhost", keyReceiver); + + const testDeviceKeys = getTestOlmAccountKeys(testOlmAccount, "@bob:xyz", "DEVICE_ID"); + keyResponder.addDeviceKeys(testDeviceKeys); + await startClientAndAwaitFirstSync(); // Alice shares a room with Bob syncResponder.sendOrQueueSyncResponse(getSyncResponse(["@bob:xyz"])); await syncPromise(aliceClient); - // Once we send the message, Alice will check Bob's device list (twice, because reasons) ... - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - - // ... and claim one of his OTKs ... + // ... and claim one of Bob's OTKs ... expectAliceKeyClaim(getTestKeysClaimResponse("@bob:xyz")); // ... and send an m.room_key message @@ -2052,13 +2059,17 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); } - oldBackendOnly("Sending an event initiates a member list sync", async () => { + it("Sending an event initiates a member list sync", async () => { + const homeserverUrl = aliceClient.getHomeserverUrl(); + keyResponder = new E2EKeyResponder(homeserverUrl); + keyResponder.addKeyReceiver("@alice:localhost", keyReceiver); + + const testDeviceKeys = getTestOlmAccountKeys(testOlmAccount, "@bob:xyz", "DEVICE_ID"); + keyResponder.addDeviceKeys(testDeviceKeys); + // we expect a call to the /members list... const memberListPromise = expectMembershipRequest(ROOM_ID, ["@bob:xyz"]); - // then a request for bob's devices... - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - // then a to-device with the room_key const inboundGroupSessionPromise = expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession); @@ -2071,13 +2082,17 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, await Promise.all([sendPromise, megolmMessagePromise, memberListPromise]); }); - oldBackendOnly("loading the membership list inhibits a later load", async () => { + it("loading the membership list inhibits a later load", async () => { + const homeserverUrl = aliceClient.getHomeserverUrl(); + keyResponder = new E2EKeyResponder(homeserverUrl); + keyResponder.addKeyReceiver("@alice:localhost", keyReceiver); + + const testDeviceKeys = getTestOlmAccountKeys(testOlmAccount, "@bob:xyz", "DEVICE_ID"); + keyResponder.addDeviceKeys(testDeviceKeys); + const room = aliceClient.getRoom(ROOM_ID)!; await Promise.all([room.loadMembersIfNeeded(), expectMembershipRequest(ROOM_ID, ["@bob:xyz"])]); - // expect a request for bob's devices... - expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - // then a to-device with the room_key const inboundGroupSessionPromise = expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession); diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index d7592803050..1e4f8d38eb3 100644 --- a/spec/test-utils/test-utils.ts +++ b/spec/test-utils/test-utils.ts @@ -315,6 +315,7 @@ export interface IMessageOpts { event?: boolean; relatesTo?: IEventRelation; ts?: number; + unsigned?: IUnsigned; } /** diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts index 046dea947a1..545c3923b85 100644 --- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts +++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts @@ -17,6 +17,7 @@ limitations under the License. import { EventTimeline, EventType, MatrixClient, MatrixError, MatrixEvent, Room } from "../../../src"; import { CallMembershipData } from "../../../src/matrixrtc/CallMembership"; import { MatrixRTCSession, MatrixRTCSessionEvent } from "../../../src/matrixrtc/MatrixRTCSession"; +import { EncryptionKeysEventContent } from "../../../src/matrixrtc/types"; import { randomString } from "../../../src/randomstring"; import { makeMockRoom, makeMockRoomState, mockRTCEvent } from "./mocks"; @@ -420,6 +421,55 @@ describe("MatrixRTCSession", () => { } }); + it("Rotates key if a member leaves", async () => { + jest.useFakeTimers(); + try { + const member2 = Object.assign({}, membershipTemplate, { + device_id: "BBBBBBB", + }); + const mockRoom = makeMockRoom([membershipTemplate, member2]); + sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); + + const onMyEncryptionKeyChanged = jest.fn(); + sess.on( + MatrixRTCSessionEvent.EncryptionKeyChanged, + (_key: Uint8Array, _idx: number, participantId: string) => { + if (participantId === `${client.getUserId()}:${client.getDeviceId()}`) { + onMyEncryptionKeyChanged(); + } + }, + ); + + const keysSentPromise1 = new Promise((resolve) => { + sendEventMock.mockImplementation((_roomId, _evType, payload) => resolve(payload)); + }); + + sess.joinRoomSession([mockFocus], true); + const firstKeysPayload = await keysSentPromise1; + expect(firstKeysPayload.keys).toHaveLength(1); + + sendEventMock.mockClear(); + + const keysSentPromise2 = new Promise((resolve) => { + sendEventMock.mockImplementation((_roomId, _evType, payload) => resolve(payload)); + }); + + mockRoom.getLiveTimeline().getState = jest + .fn() + .mockReturnValue(makeMockRoomState([membershipTemplate], mockRoom.roomId, undefined)); + sess.onMembershipUpdate(); + + jest.advanceTimersByTime(10000); + + const secondKeysPayload = await keysSentPromise2; + + expect(secondKeysPayload.keys).toHaveLength(2); + expect(onMyEncryptionKeyChanged).toHaveBeenCalledTimes(2); + } finally { + jest.useRealTimers(); + } + }); + it("Doesn't re-send key immediately", async () => { const realSetImmediate = setImmediate; jest.useFakeTimers(); @@ -643,4 +693,26 @@ describe("MatrixRTCSession", () => { expect(bobKeys[3]).toBeFalsy(); expect(bobKeys[4]).toEqual(Buffer.from("this is the key", "utf-8")); }); + + it("ignores keys event for the local participant", () => { + const mockRoom = makeMockRoom([membershipTemplate]); + sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); + sess.onCallEncryption({ + getType: jest.fn().mockReturnValue("io.element.call.encryption_keys"), + getContent: jest.fn().mockReturnValue({ + device_id: client.getDeviceId(), + call_id: "", + keys: [ + { + index: 4, + key: "dGhpcyBpcyB0aGUga2V5", + }, + ], + }), + getSender: jest.fn().mockReturnValue(client.getUserId()), + } as unknown as MatrixEvent); + + const myKeys = sess.getKeysForParticipant(client.getUserId()!, client.getDeviceId()!)!; + expect(myKeys).toBeFalsy(); + }); }); diff --git a/spec/unit/models/event.spec.ts b/spec/unit/models/event.spec.ts index 5ff9d745da1..efb2404f6a6 100644 --- a/spec/unit/models/event.spec.ts +++ b/spec/unit/models/event.spec.ts @@ -14,19 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MockedObject } from "jest-mock"; - import { MatrixEvent, MatrixEventEvent } from "../../../src/models/event"; import { emitPromise } from "../../test-utils/test-utils"; import { Crypto, IEventDecryptionResult } from "../../../src/crypto"; -import { - IAnnotatedPushRule, - MatrixClient, - PushRuleActionName, - Room, - THREAD_RELATION_TYPE, - TweakName, -} from "../../../src"; +import { IAnnotatedPushRule, PushRuleActionName, TweakName } from "../../../src"; describe("MatrixEvent", () => { it("should create copies of itself", () => { @@ -70,264 +61,31 @@ describe("MatrixEvent", () => { expect(a.toSnapshot().isEquivalentTo(b)).toBe(false); }); - describe("redaction", () => { - it("should prune clearEvent when being redacted", () => { - const ev = createEvent("$event1:server", "Test"); - - expect(ev.getContent().body).toBe("Test"); - expect(ev.getWireContent().body).toBe("Test"); - ev.makeEncrypted("m.room.encrypted", { ciphertext: "xyz" }, "", ""); - expect(ev.getContent().body).toBe("Test"); - expect(ev.getWireContent().body).toBeUndefined(); - expect(ev.getWireContent().ciphertext).toBe("xyz"); - - const mockClient = {} as unknown as MockedObject; - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const redaction = createRedaction(ev.getId()!); - - ev.makeRedacted(redaction, room); - expect(ev.getContent().body).toBeUndefined(); - expect(ev.getWireContent().body).toBeUndefined(); - expect(ev.getWireContent().ciphertext).toBeUndefined(); - }); - - it("should remain in the main timeline when redacted", async () => { - // Given an event in the main timeline - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const ev = createEvent("$event1:server"); - - await room.addLiveEvents([ev]); - await room.createThreadsTimelineSets(); - expect(ev.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([ev.getId()]); - - // When I redact it - const redaction = createRedaction(ev.getId()!); - ev.makeRedacted(redaction, room); - - // Then it remains in the main timeline - expect(ev.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([ev.getId()]); - }); - - it("should keep thread roots in both timelines when redacted", async () => { - // Given a thread exists - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const threadRoot = createEvent("$threadroot:server"); - const ev = createThreadedEvent("$event1:server", threadRoot.getId()!); - - await room.addLiveEvents([threadRoot, ev]); - await room.createThreadsTimelineSets(); - expect(threadRoot.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]); - - // When I redact the thread root - const redaction = createRedaction(ev.getId()!); - threadRoot.makeRedacted(redaction, room); - - // Then it remains in the main timeline and the thread - expect(threadRoot.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]); - }); - - it("should move into the main timeline when redacted", async () => { - // Given an event in a thread - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const threadRoot = createEvent("$threadroot:server"); - const ev = createThreadedEvent("$event1:server", threadRoot.getId()!); - - await room.addLiveEvents([threadRoot, ev]); - await room.createThreadsTimelineSets(); - expect(ev.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]); - - // When I redact it - const redaction = createRedaction(ev.getId()!); - ev.makeRedacted(redaction, room); - - // Then it disappears from the thread and appears in the main timeline - expect(ev.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId()]); - expect(threadLiveEventIds(room, 0)).not.toContain(ev.getId()); - }); - - it("should move reactions to a redacted event into the main timeline", async () => { - // Given an event in a thread with a reaction - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const threadRoot = createEvent("$threadroot:server"); - const ev = createThreadedEvent("$event1:server", threadRoot.getId()!); - const reaction = createReactionEvent("$reaction:server", ev.getId()!); - - await room.addLiveEvents([threadRoot, ev, reaction]); - await room.createThreadsTimelineSets(); - expect(reaction.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId(), reaction.getId()]); - - // When I redact the event - const redaction = createRedaction(ev.getId()!); - ev.makeRedacted(redaction, room); - - // Then the reaction moves into the main timeline - expect(reaction.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId(), reaction.getId()]); - expect(threadLiveEventIds(room, 0)).not.toContain(reaction.getId()); + it("should prune clearEvent when being redacted", () => { + const ev = new MatrixEvent({ + type: "m.room.message", + content: { + body: "Test", + }, + event_id: "$event1:server", }); - it("should move edits of a redacted event into the main timeline", async () => { - // Given an event in a thread with a reaction - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const threadRoot = createEvent("$threadroot:server"); - const ev = createThreadedEvent("$event1:server", threadRoot.getId()!); - const edit = createEditEvent("$edit:server", ev.getId()!); - - await room.addLiveEvents([threadRoot, ev, edit]); - await room.createThreadsTimelineSets(); - expect(edit.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId(), edit.getId()]); - - // When I redact the event - const redaction = createRedaction(ev.getId()!); - ev.makeRedacted(redaction, room); - - // Then the edit moves into the main timeline - expect(edit.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId(), edit.getId()]); - expect(threadLiveEventIds(room, 0)).not.toContain(edit.getId()); - }); + expect(ev.getContent().body).toBe("Test"); + expect(ev.getWireContent().body).toBe("Test"); + ev.makeEncrypted("m.room.encrypted", { ciphertext: "xyz" }, "", ""); + expect(ev.getContent().body).toBe("Test"); + expect(ev.getWireContent().body).toBeUndefined(); + expect(ev.getWireContent().ciphertext).toBe("xyz"); - it("should move reactions to replies to replies a redacted event into the main timeline", async () => { - // Given an event in a thread with a reaction - const mockClient = createMockClient(); - const room = new Room("!roomid:e.xyz", mockClient, "myname"); - const threadRoot = createEvent("$threadroot:server"); - const ev = createThreadedEvent("$event1:server", threadRoot.getId()!); - const reply1 = createReplyEvent("$reply1:server", ev.getId()!); - const reply2 = createReplyEvent("$reply2:server", reply1.getId()!); - const reaction = createReactionEvent("$reaction:server", reply2.getId()!); - - await room.addLiveEvents([threadRoot, ev, reply1, reply2, reaction]); - await room.createThreadsTimelineSets(); - expect(reaction.threadRootId).toEqual(threadRoot.getId()); - expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]); - expect(threadLiveEventIds(room, 0)).toEqual([ - threadRoot.getId(), - ev.getId(), - reply1.getId(), - reply2.getId(), - reaction.getId(), - ]); - - // When I redact the event - const redaction = createRedaction(ev.getId()!); - ev.makeRedacted(redaction, room); - - // Then the replies move to the main thread and the reaction disappears - expect(reaction.threadRootId).toBeUndefined(); - expect(mainTimelineLiveEventIds(room)).toEqual([ - threadRoot.getId(), - ev.getId(), - reply1.getId(), - reply2.getId(), - reaction.getId(), - ]); - expect(threadLiveEventIds(room, 0)).not.toContain(reply1.getId()); - expect(threadLiveEventIds(room, 0)).not.toContain(reply2.getId()); - expect(threadLiveEventIds(room, 0)).not.toContain(reaction.getId()); + const redaction = new MatrixEvent({ + type: "m.room.redaction", + redacts: ev.getId(), }); - function createMockClient(): MatrixClient { - return { - supportsThreads: jest.fn().mockReturnValue(true), - decryptEventIfNeeded: jest.fn().mockReturnThis(), - getUserId: jest.fn().mockReturnValue("@user:server"), - } as unknown as MockedObject; - } - - function createEvent(eventId: string, body?: string): MatrixEvent { - return new MatrixEvent({ - type: "m.room.message", - content: { - body: body ?? eventId, - }, - event_id: eventId, - }); - } - - function createThreadedEvent(eventId: string, threadRootId: string): MatrixEvent { - return new MatrixEvent({ - type: "m.room.message", - content: { - "body": eventId, - "m.relates_to": { - rel_type: THREAD_RELATION_TYPE.name, - event_id: threadRootId, - }, - }, - event_id: eventId, - }); - } - - function createEditEvent(eventId: string, repliedToId: string): MatrixEvent { - return new MatrixEvent({ - type: "m.room.message", - content: { - "body": "Edited", - "m.new_content": { - body: "Edited", - }, - "m.relates_to": { - event_id: repliedToId, - rel_type: "m.replace", - }, - }, - event_id: eventId, - }); - } - - function createReplyEvent(eventId: string, repliedToId: string): MatrixEvent { - return new MatrixEvent({ - type: "m.room.message", - content: { - "m.relates_to": { - event_id: repliedToId, - key: "x", - rel_type: "m.in_reply_to", - }, - }, - event_id: eventId, - }); - } - - function createReactionEvent(eventId: string, reactedToId: string): MatrixEvent { - return new MatrixEvent({ - type: "m.reaction", - content: { - "m.relates_to": { - event_id: reactedToId, - key: "x", - rel_type: "m.annotation", - }, - }, - event_id: eventId, - }); - } - - function createRedaction(redactedEventid: string): MatrixEvent { - return new MatrixEvent({ - type: "m.room.redaction", - redacts: redactedEventid, - }); - } + ev.makeRedacted(redaction); + expect(ev.getContent().body).toBeUndefined(); + expect(ev.getWireContent().body).toBeUndefined(); + expect(ev.getWireContent().ciphertext).toBeUndefined(); }); describe("applyVisibilityEvent", () => { @@ -572,19 +330,3 @@ describe("MatrixEvent", () => { expect(stateEvent.threadRootId).toBeUndefined(); }); }); - -function mainTimelineLiveEventIds(room: Room): Array { - return room - .getLiveTimeline() - .getEvents() - .map((e) => e.getId()!); -} - -function threadLiveEventIds(room: Room, threadIndex: number): Array { - return room - .getThreads() - [threadIndex].getUnfilteredTimelineSet() - .getLiveTimeline() - .getEvents() - .map((e) => e.getId()!); -} diff --git a/spec/unit/read-receipt.spec.ts b/spec/unit/read-receipt.spec.ts index 953643c8cf9..ea27d515219 100644 --- a/spec/unit/read-receipt.spec.ts +++ b/spec/unit/read-receipt.spec.ts @@ -225,6 +225,7 @@ describe("Read receipt", () => { it("should not allow an older unthreaded receipt to clobber a `main` threaded one", () => { const userId = client.getSafeUserId(); const room = new Room(ROOM_ID, client, userId); + room.findEventById = jest.fn().mockReturnValue({} as MatrixEvent); const unthreadedReceipt: WrappedReceipt = { eventId: "$olderEvent", diff --git a/spec/unit/room-state.spec.ts b/spec/unit/room-state.spec.ts index 6c60d08f908..435d2e33ddc 100644 --- a/spec/unit/room-state.spec.ts +++ b/spec/unit/room-state.spec.ts @@ -27,7 +27,6 @@ import { M_BEACON } from "../../src/@types/beacon"; import { MatrixClient } from "../../src/client"; import { DecryptionError } from "../../src/crypto/algorithms"; import { defer } from "../../src/utils"; -import { Room } from "../../src/models/room"; describe("RoomState", function () { const roomId = "!foo:bar"; @@ -363,11 +362,9 @@ describe("RoomState", function () { }); it("does not add redacted beacon info events to state", () => { - const mockClient = {} as unknown as MockedObject; const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId); const redactionEvent = new MatrixEvent({ type: "m.room.redaction" }); - const room = new Room(roomId, mockClient, userA); - redactedBeaconEvent.makeRedacted(redactionEvent, room); + redactedBeaconEvent.makeRedacted(redactionEvent); const emitSpy = jest.spyOn(state, "emit"); state.setStateEvents([redactedBeaconEvent]); @@ -397,13 +394,11 @@ describe("RoomState", function () { }); it("destroys and removes redacted beacon events", () => { - const mockClient = {} as unknown as MockedObject; const beaconId = "$beacon1"; const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId); const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId); const redactionEvent = new MatrixEvent({ type: "m.room.redaction", redacts: beaconEvent.getId() }); - const room = new Room(roomId, mockClient, userA); - redactedBeaconEvent.makeRedacted(redactionEvent, room); + redactedBeaconEvent.makeRedacted(redactionEvent); state.setStateEvents([beaconEvent]); const beaconInstance = state.beacons.get(getBeaconInfoIdentifier(beaconEvent)); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index b1db43f5dcd..f32cf5fb6ca 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -1746,6 +1746,7 @@ describe("Room", function () { it("should acknowledge if an event has been read", function () { const ts = 13787898424; room.addReceipt(mkReceipt(roomId, [mkRecord(eventToAck.getId()!, "m.read", userB, ts)])); + room.findEventById = jest.fn().mockReturnValue({} as MatrixEvent); expect(room.hasUserReadEvent(userB, eventToAck.getId()!)).toEqual(true); }); it("return false for an unknown event", function () { @@ -3147,106 +3148,195 @@ describe("Room", function () { const client = new TestClient(userA).client; const room = new Room(roomId, client, userA); - it("handles missing receipt type", () => { - room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { - return receiptType === ReceiptType.ReadPrivate ? ({ eventId: "eventId" } as WrappedReceipt) : null; - }; + describe("invalid receipts", () => { + beforeEach(() => { + // Clear the spies on logger.warn + jest.clearAllMocks(); + }); - expect(room.getEventReadUpTo(userA)).toEqual("eventId"); - }); + it("ignores receipts pointing at missing events", () => { + // Given a receipt exists + room.getReadReceiptForUserId = (): WrappedReceipt | null => { + return { eventId: "missingEventId" } as WrappedReceipt; + }; + // But the event ID it contains does not refer to an event we have + room.findEventById = jest.fn().mockReturnValue(null); - describe("prefers newer receipt", () => { - it("should compare correctly using timelines", () => { - room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { - if (receiptType === ReceiptType.ReadPrivate) { - return { eventId: "eventId1" } as WrappedReceipt; - } - if (receiptType === ReceiptType.Read) { - return { eventId: "eventId2" } as WrappedReceipt; - } - return null; + // When we ask what they have read + // Then we say "nothing" + expect(room.getEventReadUpTo(userA)).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith("Ignoring receipt for missing event with id missingEventId"); + }); + + it("ignores receipts pointing at the wrong thread", () => { + // Given a threaded receipt exists + room.getReadReceiptForUserId = (): WrappedReceipt | null => { + return { eventId: "wrongThreadEventId", data: { ts: 0, thread_id: "thread1" } } as WrappedReceipt; }; + // But the event it refers to is in a thread + room.findEventById = jest.fn().mockReturnValue({ threadRootId: "thread2" } as MatrixEvent); + + // When we ask what they have read + // Then we say "nothing" + expect(room.getEventReadUpTo(userA)).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith( + "Ignoring receipt because its thread_id (thread1) disagrees with the thread root (thread2) " + + "of the referenced event (event ID = wrongThreadEventId)", + ); + }); - for (let i = 1; i <= 2; i++) { - room.getUnfilteredTimelineSet = () => - ({ - compareEventOrdering: (event1, event2) => { - return event1 === `eventId${i}` ? 1 : -1; - }, - } as EventTimelineSet); + it("accepts unthreaded receipts pointing at an event in a thread", () => { + // Given an unthreaded receipt exists + room.getReadReceiptForUserId = (): WrappedReceipt | null => { + return { eventId: "inThreadEventId" } as WrappedReceipt; + }; + // And the event it refers to is in a thread + room.findEventById = jest.fn().mockReturnValue({ threadRootId: "thread2" } as MatrixEvent); - expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`); - } + // When we ask what they have read + // Then we say the event + expect(room.getEventReadUpTo(userA)).toEqual("inThreadEventId"); }); - describe("correctly compares by timestamp", () => { - it("should correctly compare, if we have all receipts", () => { - for (let i = 1; i <= 2; i++) { - room.getUnfilteredTimelineSet = () => - ({ - compareEventOrdering: (_1, _2) => null, - } as EventTimelineSet); - room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { - if (receiptType === ReceiptType.ReadPrivate) { - return { eventId: "eventId1", data: { ts: i === 1 ? 2 : 1 } } as WrappedReceipt; - } - if (receiptType === ReceiptType.Read) { - return { eventId: "eventId2", data: { ts: i === 2 ? 2 : 1 } } as WrappedReceipt; - } - return null; - }; + it("accepts main thread receipts pointing at an event in main timeline", () => { + // Given a threaded receipt exists, in main thread + room.getReadReceiptForUserId = (): WrappedReceipt | null => { + return { eventId: "mainThreadEventId", data: { ts: 12, thread_id: "main" } } as WrappedReceipt; + }; + // And the event it refers to is in a thread + room.findEventById = jest.fn().mockReturnValue({ threadRootId: undefined } as MatrixEvent); - expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`); - } - }); + // When we ask what they have read + // Then we say the event + expect(room.getEventReadUpTo(userA)).toEqual("mainThreadEventId"); + }); - it("should correctly compare, if private read receipt is missing", () => { - room.getUnfilteredTimelineSet = () => - ({ - compareEventOrdering: (_1, _2) => null, - } as EventTimelineSet); - room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { - if (receiptType === ReceiptType.Read) { - return { eventId: "eventId2", data: { ts: 1 } } as WrappedReceipt; - } - return null; - }; + it("accepts main thread receipts pointing at a thread root", () => { + // Given a threaded receipt exists, in main thread + room.getReadReceiptForUserId = (): WrappedReceipt | null => { + return { eventId: "rootId", data: { ts: 12, thread_id: "main" } } as WrappedReceipt; + }; + // And the event it refers to is in a thread, because it is a thread root + room.findEventById = jest + .fn() + .mockReturnValue({ isThreadRoot: true, threadRootId: "thread1" } as MatrixEvent); - expect(room.getEventReadUpTo(userA)).toEqual(`eventId2`); - }); + // When we ask what they have read + // Then we say the event + expect(room.getEventReadUpTo(userA)).toEqual("rootId"); }); + }); - describe("fallback precedence", () => { - beforeAll(() => { - room.getUnfilteredTimelineSet = () => - ({ - compareEventOrdering: (_1, _2) => null, - } as EventTimelineSet); - }); + describe("valid receipts", () => { + beforeEach(() => { + // When we look up the event referred to by the receipt, it exists + room.findEventById = jest.fn().mockReturnValue({} as MatrixEvent); + }); - it("should give precedence to m.read.private", () => { + it("handles missing receipt type", () => { + room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { + return receiptType === ReceiptType.ReadPrivate ? ({ eventId: "eventId" } as WrappedReceipt) : null; + }; + expect(room.getEventReadUpTo(userA)).toEqual("eventId"); + }); + + describe("prefers newer receipt", () => { + it("should compare correctly using timelines", () => { room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { if (receiptType === ReceiptType.ReadPrivate) { - return { eventId: "eventId1", data: { ts: 123 } }; + return { eventId: "eventId1" } as WrappedReceipt; } if (receiptType === ReceiptType.Read) { - return { eventId: "eventId2", data: { ts: 123 } }; + return { eventId: "eventId2" } as WrappedReceipt; } return null; }; - expect(room.getEventReadUpTo(userA)).toEqual(`eventId1`); + for (let i = 1; i <= 2; i++) { + room.getUnfilteredTimelineSet = () => + ({ + compareEventOrdering: (event1: string, _event2: string) => { + return event1 === `eventId${i}` ? 1 : -1; + }, + findEventById: jest.fn().mockReturnValue({} as MatrixEvent), + } as unknown as EventTimelineSet); + + expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`); + } }); - it("should give precedence to m.read", () => { - room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { - if (receiptType === ReceiptType.Read) { - return { eventId: "eventId3" } as WrappedReceipt; + describe("correctly compares by timestamp", () => { + it("should correctly compare, if we have all receipts", () => { + for (let i = 1; i <= 2; i++) { + room.getUnfilteredTimelineSet = () => + ({ + compareEventOrdering: () => null, + findEventById: jest.fn().mockReturnValue({} as MatrixEvent), + } as unknown as EventTimelineSet); + room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { + if (receiptType === ReceiptType.ReadPrivate) { + return { eventId: "eventId1", data: { ts: i === 1 ? 2 : 1 } } as WrappedReceipt; + } + if (receiptType === ReceiptType.Read) { + return { eventId: "eventId2", data: { ts: i === 2 ? 2 : 1 } } as WrappedReceipt; + } + return null; + }; + + expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`); } - return null; - }; + }); + + it("should correctly compare, if private read receipt is missing", () => { + room.getUnfilteredTimelineSet = () => + ({ + compareEventOrdering: () => null, + findEventById: jest.fn().mockReturnValue({} as MatrixEvent), + } as unknown as EventTimelineSet); + room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { + if (receiptType === ReceiptType.Read) { + return { eventId: "eventId2", data: { ts: 1 } } as WrappedReceipt; + } + return null; + }; + + expect(room.getEventReadUpTo(userA)).toEqual(`eventId2`); + }); + }); - expect(room.getEventReadUpTo(userA)).toEqual(`eventId3`); + describe("fallback precedence", () => { + beforeAll(() => { + room.getUnfilteredTimelineSet = () => + ({ + compareEventOrdering: () => null, + findEventById: jest.fn().mockReturnValue({} as MatrixEvent), + } as unknown as EventTimelineSet); + }); + + it("should give precedence to m.read.private", () => { + room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { + if (receiptType === ReceiptType.ReadPrivate) { + return { eventId: "eventId1", data: { ts: 123 } }; + } + if (receiptType === ReceiptType.Read) { + return { eventId: "eventId2", data: { ts: 123 } }; + } + return null; + }; + + expect(room.getEventReadUpTo(userA)).toEqual(`eventId1`); + }); + + it("should give precedence to m.read", () => { + room.getReadReceiptForUserId = (userId, ignore, receiptType): WrappedReceipt | null => { + if (receiptType === ReceiptType.Read) { + return { eventId: "eventId3" } as WrappedReceipt; + } + return null; + }; + + expect(room.getEventReadUpTo(userA)).toEqual(`eventId3`); + }); }); }); }); @@ -3564,7 +3654,7 @@ describe("Room", function () { expect(room.polls.get(pollStartEvent.getId()!)).toBeTruthy(); const redactedEvent = new MatrixEvent({ type: "m.room.redaction" }); - pollStartEvent.makeRedacted(redactedEvent, room); + pollStartEvent.makeRedacted(redactedEvent); await flushPromises(); diff --git a/spec/unit/rust-crypto/OutgoingRequestsManager.spec.ts b/spec/unit/rust-crypto/OutgoingRequestsManager.spec.ts new file mode 100644 index 00000000000..9c59c2f416b --- /dev/null +++ b/spec/unit/rust-crypto/OutgoingRequestsManager.spec.ts @@ -0,0 +1,237 @@ +/* +Copyright 2023 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 { Mocked } from "jest-mock"; +import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-wasm"; + +import { OutgoingRequest, OutgoingRequestProcessor } from "../../../src/rust-crypto/OutgoingRequestProcessor"; +import { OutgoingRequestsManager } from "../../../src/rust-crypto/OutgoingRequestsManager"; +import { defer, IDeferred } from "../../../src/utils"; +import { logger } from "../../../src/logger"; + +describe("OutgoingRequestsManager", () => { + /** the OutgoingRequestsManager implementation under test */ + let manager: OutgoingRequestsManager; + + /** a mock OutgoingRequestProcessor */ + let processor: Mocked; + + /** a mocked-up OlmMachine which manager is connected to */ + let olmMachine: Mocked; + + beforeEach(async () => { + olmMachine = { + outgoingRequests: jest.fn(), + } as unknown as Mocked; + + processor = { + makeOutgoingRequest: jest.fn(), + } as unknown as Mocked; + + manager = new OutgoingRequestsManager(logger, olmMachine, processor); + }); + + describe("Call doProcessOutgoingRequests", () => { + it("The call triggers handling of the machine outgoing requests", async () => { + const request1 = new RustSdkCryptoJs.KeysQueryRequest("foo", "{}"); + const request2 = new RustSdkCryptoJs.KeysUploadRequest("foo2", "{}"); + olmMachine.outgoingRequests.mockImplementationOnce(async () => { + return [request1, request2]; + }); + + processor.makeOutgoingRequest.mockImplementationOnce(async () => { + return; + }); + + await manager.doProcessOutgoingRequests(); + + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(1); + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(2); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request1); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request2); + }); + + it("Stack and batch calls to doProcessOutgoingRequests while one is already running", async () => { + const request1 = new RustSdkCryptoJs.KeysQueryRequest("foo", "{}"); + const request2 = new RustSdkCryptoJs.KeysUploadRequest("foo2", "{}"); + const request3 = new RustSdkCryptoJs.KeysBackupRequest("foo3", "{}", "1"); + + const firstOutgoingRequestDefer = defer(); + + olmMachine.outgoingRequests + .mockImplementationOnce(async (): Promise => { + return firstOutgoingRequestDefer.promise; + }) + .mockImplementationOnce(async () => { + return [request3]; + }); + + const firstRequest = manager.doProcessOutgoingRequests(); + + // stack 2 additional requests while the first one is still running + const secondRequest = manager.doProcessOutgoingRequests(); + const thirdRequest = manager.doProcessOutgoingRequests(); + + // let the first request complete + firstOutgoingRequestDefer.resolve([request1, request2]); + + await firstRequest; + await secondRequest; + await thirdRequest; + + // outgoingRequests should be called twice in total, as the second and third requests are + // processed in the same loop. + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(2); + + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(3); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request1); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request2); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request3); + }); + + it("Process 3 consecutive calls to doProcessOutgoingRequests while not blocking previous ones", async () => { + const request1 = new RustSdkCryptoJs.KeysQueryRequest("foo", "{}"); + const request2 = new RustSdkCryptoJs.KeysUploadRequest("foo2", "{}"); + const request3 = new RustSdkCryptoJs.KeysBackupRequest("foo3", "{}", "1"); + + // promises which will resolve when OlmMachine.outgoingRequests is called + const outgoingRequestCalledPromises: Promise[] = []; + + // deferreds which will provide the results of OlmMachine.outgoingRequests + const outgoingRequestResultDeferreds: IDeferred[] = []; + + for (let i = 0; i < 3; i++) { + const resultDeferred = defer(); + const calledPromise = new Promise((resolve) => { + olmMachine.outgoingRequests.mockImplementationOnce(() => { + resolve(); + return resultDeferred.promise; + }); + }); + outgoingRequestCalledPromises.push(calledPromise); + outgoingRequestResultDeferreds.push(resultDeferred); + } + + const call1 = manager.doProcessOutgoingRequests(); + + // First call will start an iteration and for now is awaiting on outgoingRequests + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(1); + + // Make a new call now: this will request a new iteration + const call2 = manager.doProcessOutgoingRequests(); + + // let the first iteration complete + outgoingRequestResultDeferreds[0].resolve([request1]); + + // The first call should now complete + await call1; + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(1); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request1); + + // Wait for the second iteration to fire and be waiting on `outgoingRequests` + await outgoingRequestCalledPromises[1]; + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(2); + + // Stack a new call that should be processed in an additional iteration. + const call3 = manager.doProcessOutgoingRequests(); + + outgoingRequestResultDeferreds[1].resolve([request2]); + await call2; + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(2); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request2); + + // Wait for the third iteration to fire and be waiting on `outgoingRequests` + await outgoingRequestCalledPromises[2]; + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(3); + outgoingRequestResultDeferreds[2].resolve([request3]); + await call3; + + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(3); + expect(processor.makeOutgoingRequest).toHaveBeenCalledWith(request3); + + // ensure that no other iteration is going on + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(3); + }); + + it("Should not bubble exceptions if server request is rejected", async () => { + const request = new RustSdkCryptoJs.KeysQueryRequest("foo", "{}"); + olmMachine.outgoingRequests.mockImplementationOnce(async () => { + return [request]; + }); + + processor.makeOutgoingRequest.mockImplementationOnce(async () => { + throw new Error("Some network error"); + }); + + await manager.doProcessOutgoingRequests(); + + expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(1); + }); + }); + + describe("Calling stop on the manager should stop ongoing work", () => { + it("When the manager is stopped after outgoingRequests() call, do not make sever requests", async () => { + const request1 = new RustSdkCryptoJs.KeysQueryRequest("foo", "{}"); + + const firstOutgoingRequestDefer = defer(); + + olmMachine.outgoingRequests.mockImplementationOnce(async (): Promise => { + return firstOutgoingRequestDefer.promise; + }); + + const firstRequest = manager.doProcessOutgoingRequests(); + + // stop + manager.stop(); + + // let the first request complete + firstOutgoingRequestDefer.resolve([request1]); + + await firstRequest; + + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(0); + }); + + it("When the manager is stopped while doing server calls, it should stop before the next sever call", async () => { + const request1 = new RustSdkCryptoJs.KeysQueryRequest("11", "{}"); + const request2 = new RustSdkCryptoJs.KeysUploadRequest("12", "{}"); + + const firstRequestDefer = defer(); + + olmMachine.outgoingRequests.mockImplementationOnce(async (): Promise => { + return [request1, request2]; + }); + + processor.makeOutgoingRequest + .mockImplementationOnce(async () => { + manager.stop(); + return firstRequestDefer.promise; + }) + .mockImplementationOnce(async () => { + return; + }); + + const firstRequest = manager.doProcessOutgoingRequests(); + + firstRequestDefer.resolve(); + + await firstRequest; + + // should have been called once but not twice + expect(processor.makeOutgoingRequest).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index c450331187e..1044c71a0c0 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -50,6 +50,7 @@ import { import * as testData from "../../test-utils/test-data"; import { defer } from "../../../src/utils"; import { logger } from "../../../src/logger"; +import { OutgoingRequestsManager } from "../../../src/rust-crypto/OutgoingRequestsManager"; const TEST_USER = "@alice:example.com"; const TEST_DEVICE_ID = "TEST_DEVICE"; @@ -347,6 +348,8 @@ describe("RustCrypto", () => { makeOutgoingRequest: jest.fn(), } as unknown as Mocked; + const outgoingRequestsManager = new OutgoingRequestsManager(logger, olmMachine, outgoingRequestProcessor); + rustCrypto = new RustCrypto( logger, olmMachine, @@ -357,6 +360,7 @@ describe("RustCrypto", () => { {} as CryptoCallbacks, ); rustCrypto["outgoingRequestProcessor"] = outgoingRequestProcessor; + rustCrypto["outgoingRequestsManager"] = outgoingRequestsManager; }); it("should poll for outgoing messages and send them", async () => { @@ -395,50 +399,6 @@ describe("RustCrypto", () => { await awaitCallToMakeOutgoingRequest(); expect(olmMachine.outgoingRequests).toHaveBeenCalledTimes(2); }); - - it("stops looping when stop() is called", async () => { - for (let i = 0; i < 5; i++) { - outgoingRequestQueue.push([new KeysQueryRequest("1234", "{}")]); - } - - let makeRequestPromise = awaitCallToMakeOutgoingRequest(); - - rustCrypto.onSyncCompleted({}); - - expect(rustCrypto["outgoingRequestLoopRunning"]).toBeTruthy(); - - // go a couple of times round the loop - let resolveMakeRequest = await makeRequestPromise; - makeRequestPromise = awaitCallToMakeOutgoingRequest(); - resolveMakeRequest(); - - resolveMakeRequest = await makeRequestPromise; - makeRequestPromise = awaitCallToMakeOutgoingRequest(); - resolveMakeRequest(); - - // a second sync while this is going on shouldn't make any difference - rustCrypto.onSyncCompleted({}); - - resolveMakeRequest = await makeRequestPromise; - outgoingRequestProcessor.makeOutgoingRequest.mockReset(); - resolveMakeRequest(); - - // now stop... - rustCrypto.stop(); - - // which should (eventually) cause the loop to stop with no further calls to outgoingRequests - olmMachine.outgoingRequests.mockReset(); - - await new Promise((resolve) => { - setTimeout(resolve, 100); - }); - expect(rustCrypto["outgoingRequestLoopRunning"]).toBeFalsy(); - expect(outgoingRequestProcessor.makeOutgoingRequest).not.toHaveBeenCalled(); - expect(olmMachine.outgoingRequests).not.toHaveBeenCalled(); - - // we sent three, so there should be 2 left - expect(outgoingRequestQueue.length).toEqual(2); - }); }); describe(".getEventEncryptionInfo", () => { diff --git a/spec/unit/timeline-window.spec.ts b/spec/unit/timeline-window.spec.ts index f786a513d64..ddb2a48d3b9 100644 --- a/spec/unit/timeline-window.spec.ts +++ b/spec/unit/timeline-window.spec.ts @@ -22,12 +22,15 @@ import { Room } from "../../src/models/room"; import { EventTimeline } from "../../src/models/event-timeline"; import { TimelineIndex, TimelineWindow } from "../../src/timeline-window"; import { mkMessage } from "../test-utils/test-utils"; +import { MatrixEvent } from "../../src/models/event"; const ROOM_ID = "roomId"; const USER_ID = "userId"; const mockClient = { getEventTimeline: jest.fn(), paginateEventTimeline: jest.fn(), + supportsThreads: jest.fn(), + getUserId: jest.fn().mockReturnValue(USER_ID), } as unknown as MockedObject; /* @@ -64,6 +67,23 @@ function addEventsToTimeline(timeline: EventTimeline, numEvents: number, toStart } } +function createEvents(numEvents: number): Array { + const ret = []; + + for (let i = 0; i < numEvents; i++) { + ret.push( + mkMessage({ + room: ROOM_ID, + user: USER_ID, + event: true, + unsigned: { age: 1 }, + }), + ); + } + + return ret; +} + /* * create a pair of linked timelines */ @@ -412,4 +432,46 @@ describe("TimelineWindow", function () { expect(timelineWindow.canPaginate(EventTimeline.FORWARDS)).toBe(true); }); }); + + function idsOf(events: Array): Array { + return events.map((e) => (e ? e.getId() ?? "MISSING_ID" : "MISSING_EVENT")); + } + + describe("removing events", () => { + it("should shorten if removing an event within the window makes it overflow", function () { + // Given a room with events in two timelines + const room = new Room(ROOM_ID, mockClient, USER_ID, { timelineSupport: true }); + const timelineSet = room.getUnfilteredTimelineSet(); + const liveTimeline = room.getLiveTimeline(); + const oldTimeline = room.addTimeline(); + liveTimeline.setNeighbouringTimeline(oldTimeline, EventTimeline.BACKWARDS); + oldTimeline.setNeighbouringTimeline(liveTimeline, EventTimeline.FORWARDS); + + const oldEvents = createEvents(5); + const liveEvents = createEvents(5); + const [, , e3, e4, e5] = oldEvents; + const [, e7, e8, e9, e10] = liveEvents; + room.addLiveEvents(liveEvents); + room.addEventsToTimeline(oldEvents, true, oldTimeline); + + // And 2 windows over the timelines in this room + const oldWindow = new TimelineWindow(mockClient, timelineSet); + oldWindow.load(e5.getId(), 6); + expect(idsOf(oldWindow.getEvents())).toEqual(idsOf([e5, e4, e3])); + + const newWindow = new TimelineWindow(mockClient, timelineSet); + newWindow.load(e9.getId(), 4); + expect(idsOf(newWindow.getEvents())).toEqual(idsOf([e7, e8, e9, e10])); + + // When I remove an event + room.removeEvent(e8.getId()!); + + // Then the affected timeline is shortened (because it would have + // been too long with the removed event gone) + expect(idsOf(newWindow.getEvents())).toEqual(idsOf([e7, e9, e10])); + + // And the unaffected one is not + expect(idsOf(oldWindow.getEvents())).toEqual(idsOf([e5, e4, e3])); + }); + }); }); diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index 43348ff3ec2..bba60704379 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -26,13 +26,21 @@ import { MatrixError, MatrixEvent } from "../matrix"; import { randomString, secureRandomBase64Url } from "../randomstring"; import { EncryptionKeysEventContent } from "./types"; import { decodeBase64, encodeUnpaddedBase64 } from "../base64"; -import { isNumber } from "../utils"; const MEMBERSHIP_EXPIRY_TIME = 60 * 60 * 1000; const MEMBER_EVENT_CHECK_PERIOD = 2 * 60 * 1000; // How often we check to see if we need to re-send our member event const CALL_MEMBER_EVENT_RETRY_DELAY_MIN = 3000; const UPDATE_ENCRYPTION_KEY_THROTTLE = 3000; +// A delay after a member leaves before we create and publish a new key, because people +// tend to leave calls at the same time +const MAKE_KEY_DELAY = 3000; +// The delay between creating and sending a new key and starting to encrypt with it. This gives others +// a chance to receive the new key to minimise the chance they don't get media they can't decrypt. +// The total time between a member leaving and the call switching to new keys is therefore +// MAKE_KEY_DELAY + SEND_KEY_DELAY +const USE_KEY_DELAY = 5000; + const getParticipantId = (userId: string, deviceId: string): string => `${userId}:${deviceId}`; const getParticipantIdFromMembership = (m: CallMembership): string => getParticipantId(m.sender!, m.deviceId); @@ -85,6 +93,8 @@ export class MatrixRTCSession extends TypedEventEmitter; private expiryTimeout?: ReturnType; private keysEventUpdateTimeout?: ReturnType; + private makeNewKeyTimeout?: ReturnType; + private setNewKeyTimeouts = new Set>(); private activeFoci: Focus[] | undefined; @@ -253,6 +263,15 @@ export class MatrixRTCSession extends TypedEventEmitter { + this.setNewKeyTimeouts.delete(useKeyTimeout); + logger.info(`Delayed-emitting key changed event for ${participantId} idx ${encryptionKeyIndex}`); + this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, keyBin, encryptionKeyIndex, participantId); + }, USE_KEY_DELAY); + this.setNewKeyTimeouts.add(useKeyTimeout); + } else { + this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, keyBin, encryptionKeyIndex, participantId); + } } /** * Generate a new sender key and add it at the next available index + * @param delayBeforeUse - If true, wait for a short period before settign the key for the + * media encryptor to use. If false, set the key immediately. */ - private makeNewSenderKey(): void { + private makeNewSenderKey(delayBeforeUse = false): void { const userId = this.client.getUserId(); const deviceId = this.client.getDeviceId(); @@ -328,7 +371,7 @@ export class MatrixRTCSession extends TypedEventEmitter m.sender === this.client.getUserId() && m.deviceId === this.client.getDeviceId(); - const callMembersChanged = - oldMemberships - .filter((m) => !isMyMembership(m)) - .map(getParticipantIdFromMembership) - .sort() - .join() !== - this.memberships - .filter((m) => !isMyMembership(m)) - .map(getParticipantIdFromMembership) - .sort() - .join(); - - if (callMembersChanged && this.isJoined()) { - this.requestKeyEventSend(); + + if (this.isJoined() && this.makeNewKeyTimeout === undefined) { + const oldMebershipIds = new Set( + oldMemberships.filter((m) => !isMyMembership(m)).map(getParticipantIdFromMembership), + ); + const newMebershipIds = new Set( + this.memberships.filter((m) => !isMyMembership(m)).map(getParticipantIdFromMembership), + ); + + const anyLeft = Array.from(oldMebershipIds).some((x) => !newMebershipIds.has(x)); + const anyJoined = Array.from(newMebershipIds).some((x) => !oldMebershipIds.has(x)); + + if (anyLeft) { + logger.debug(`Member(s) have left: queueing sender key rotation`); + this.makeNewKeyTimeout = setTimeout(this.onRotateKeyTimeout, MAKE_KEY_DELAY); + } else if (anyJoined) { + logger.debug(`New member(s) have joined: re-sending keys`); + this.requestKeyEventSend(); + } } this.setExpiryTimer(); @@ -708,4 +765,13 @@ export class MatrixRTCSession extends TypedEventEmitter { + this.makeNewKeyTimeout = undefined; + logger.info("Making new sender key for key rotation"); + this.makeNewSenderKey(true); + // send immediately: if we're about to start sending with a new key, it's + // important we get it out to others as soon as we can. + this.sendEncryptionKeysEvent(); + }; } diff --git a/src/matrixrtc/MatrixRTCSessionManager.ts b/src/matrixrtc/MatrixRTCSessionManager.ts index ba1eb0fa1dd..e0ca3142b69 100644 --- a/src/matrixrtc/MatrixRTCSessionManager.ts +++ b/src/matrixrtc/MatrixRTCSessionManager.ts @@ -121,7 +121,9 @@ export class MatrixRTCSessionManager extends TypedEventEmitter void; + [RoomEvent.Redaction]: (event: MatrixEvent, room: Room) => void; /** * Fires when an event that was previously redacted isn't anymore. * This happens when the redaction couldn't be sent and @@ -2114,12 +2113,6 @@ export class Room extends ReadReceipt { * Relations (other than m.thread), redactions, replies to a thread root live only in the main timeline * Relations, redactions, replies where the parent cannot be found live in no timelines but should be aggregated regardless. * Otherwise, the event lives in the main timeline only. - * - * Note: when a redaction is applied, the redacted event, events relating - * to it, and the redaction event itself, will all move to the main thread. - * This method classifies them as inside the thread of the redacted event. - * They are moved later as part of makeRedacted. - * This will change if MSC3389 is merged. */ public eventShouldLiveIn( event: MatrixEvent, @@ -2336,8 +2329,7 @@ export class Room extends ReadReceipt { // if we know about this event, redact its contents now. const redactedEvent = redactId ? this.findEventById(redactId) : undefined; if (redactedEvent) { - const threadRootId = redactedEvent.threadRootId; - redactedEvent.makeRedacted(event, this); + redactedEvent.makeRedacted(event); // If this is in the current state, replace it with the redacted version if (redactedEvent.isState()) { @@ -2350,7 +2342,7 @@ export class Room extends ReadReceipt { } } - this.emit(RoomEvent.Redaction, event, this, threadRootId); + this.emit(RoomEvent.Redaction, event, this); // TODO: we stash user displaynames (among other things) in // RoomMember objects which are then attached to other events @@ -2503,7 +2495,7 @@ export class Room extends ReadReceipt { } if (redactedEvent) { redactedEvent.markLocallyRedacted(event); - this.emit(RoomEvent.Redaction, event, this, redactedEvent.threadRootId); + this.emit(RoomEvent.Redaction, event, this); } } } else { diff --git a/src/models/thread.ts b/src/models/thread.ts index 6b8069c9a5f..1da3fca558b 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -228,8 +228,8 @@ export class Thread extends ReadReceipt => { - if (threadRootId !== this.id) return; // ignore redactions for other timelines + private onRedaction = async (event: MatrixEvent): Promise => { + if (event.threadRootId !== this.id) return; // ignore redactions for other timelines if (this.replyCount <= 0) { for (const threadEvent of this.timeline) { this.clearEventMetadata(threadEvent); diff --git a/src/rust-crypto/OutgoingRequestsManager.ts b/src/rust-crypto/OutgoingRequestsManager.ts new file mode 100644 index 00000000000..e3ca9066d0d --- /dev/null +++ b/src/rust-crypto/OutgoingRequestsManager.ts @@ -0,0 +1,141 @@ +/* +Copyright 2023 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 { OlmMachine } from "@matrix-org/matrix-sdk-crypto-wasm"; + +import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; +import { Logger } from "../logger"; +import { defer, IDeferred } from "../utils"; + +/** + * OutgoingRequestsManager: responsible for processing outgoing requests from the OlmMachine. + * Ensure that only one loop is going on at once, and that the requests are processed in order. + */ +export class OutgoingRequestsManager { + /** whether {@link stop} has been called */ + private stopped = false; + + /** whether {@link outgoingRequestLoop} is currently running */ + private outgoingRequestLoopRunning = false; + + /** + * If there are additional calls to doProcessOutgoingRequests() while there is a current call running + * we need to remember in order to call `doProcessOutgoingRequests` again (as there could be new requests). + * + * If this is defined, it is an indication that we need to do another iteration; in this case the deferred + * will resolve once that next iteration completes. If it is undefined, there have been no new calls + * to `doProcessOutgoingRequests` since the current iteration started. + */ + private nextLoopDeferred?: IDeferred; + + public constructor( + private readonly logger: Logger, + private readonly olmMachine: OlmMachine, + public readonly outgoingRequestProcessor: OutgoingRequestProcessor, + ) {} + + /** + * Shut down as soon as possible the current loop of outgoing requests processing. + */ + public stop(): void { + this.stopped = true; + } + + /** + * Process the OutgoingRequests from the OlmMachine. + * + * This should be called at the end of each sync, to process any OlmMachine OutgoingRequests created by the rust sdk. + * In some cases if OutgoingRequests need to be sent immediately, this can be called directly. + * + * Calls to doProcessOutgoingRequests() are processed synchronously, one after the other, in order. + * If doProcessOutgoingRequests() is called while another call is still being processed, it will be queued. + * Multiple calls to doProcessOutgoingRequests() when a call is already processing will be batched together. + */ + public doProcessOutgoingRequests(): Promise { + // Flag that we need at least one more iteration of the loop. + // + // It is important that we do this even if the loop is currently running. There is potential for a race whereby + // a request is added to the queue *after* `OlmMachine.outgoingRequests` checks the queue, but *before* it + // returns. In such a case, the item could sit there unnoticed for some time. + // + // In order to circumvent the race, we set a flag which tells the loop to go round once again even if the + // queue appears to be empty. + if (!this.nextLoopDeferred) { + this.nextLoopDeferred = defer(); + } + + // ... and wait for it to complete. + const result = this.nextLoopDeferred.promise; + + // set the loop going if it is not already. + if (!this.outgoingRequestLoopRunning) { + this.outgoingRequestLoop().catch((e) => { + // this should not happen; outgoingRequestLoop should return any errors via `nextLoopDeferred`. + /* istanbul ignore next */ + this.logger.error("Uncaught error in outgoing request loop", e); + }); + } + return result; + } + + private async outgoingRequestLoop(): Promise { + /* istanbul ignore if */ + if (this.outgoingRequestLoopRunning) { + throw new Error("Cannot run two outgoing request loops"); + } + this.outgoingRequestLoopRunning = true; + try { + while (!this.stopped && this.nextLoopDeferred) { + const deferred = this.nextLoopDeferred; + + // reset `nextLoopDeferred` so that any future calls to `doProcessOutgoingRequests` are queued + // for another additional iteration. + this.nextLoopDeferred = undefined; + + // make the requests and feed the results back to the `nextLoopDeferred` + await this.processOutgoingRequests().then(deferred.resolve, deferred.reject); + } + } finally { + this.outgoingRequestLoopRunning = false; + } + + if (this.nextLoopDeferred) { + // the loop was stopped, but there was a call to `doProcessOutgoingRequests`. Make sure that + // we reject the promise in case anything is waiting for it. + this.nextLoopDeferred.reject(new Error("OutgoingRequestsManager was stopped")); + } + } + + /** + * Make a single request to `olmMachine.outgoingRequests` and do the corresponding requests. + */ + private async processOutgoingRequests(): Promise { + if (this.stopped) return; + + const outgoingRequests: OutgoingRequest[] = await this.olmMachine.outgoingRequests(); + + for (const request of outgoingRequests) { + if (this.stopped) return; + try { + await this.outgoingRequestProcessor.makeOutgoingRequest(request); + } catch (e) { + // as part of the loop we silently ignore errors, but log them. + // The rust sdk will retry the request later as it won't have been marked as sent. + this.logger.error(`Failed to process outgoing request ${request.type}: ${e}`); + } + } + } +} diff --git a/src/rust-crypto/RoomEncryptor.ts b/src/rust-crypto/RoomEncryptor.ts index c384a933586..4e0dc95b32c 100644 --- a/src/rust-crypto/RoomEncryptor.ts +++ b/src/rust-crypto/RoomEncryptor.ts @@ -23,6 +23,7 @@ import { HistoryVisibility as RustHistoryVisibility, ToDeviceRequest, } from "@matrix-org/matrix-sdk-crypto-wasm"; +import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-wasm"; import { EventType } from "../@types/event"; import { IContent, MatrixEvent } from "../models/event"; @@ -30,8 +31,8 @@ import { Room } from "../models/room"; import { Logger, logger } from "../logger"; import { KeyClaimManager } from "./KeyClaimManager"; import { RoomMember } from "../models/room-member"; -import { OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; import { HistoryVisibility } from "../@types/partials"; +import { OutgoingRequestsManager } from "./OutgoingRequestsManager"; /** * RoomEncryptor: responsible for encrypting messages to a given room @@ -41,21 +42,34 @@ import { HistoryVisibility } from "../@types/partials"; export class RoomEncryptor { private readonly prefixedLogger: Logger; + /** whether the room members have been loaded and tracked for the first time */ + private lazyLoadedMembersResolved = false; + /** * @param olmMachine - The rust-sdk's OlmMachine * @param keyClaimManager - Our KeyClaimManager, which manages the queue of one-time-key claim requests - * @param outgoingRequestProcessor - The OutgoingRequestProcessor, which sends outgoing requests + * @param outgoingRequestManager - The OutgoingRequestManager, which manages the queue of outgoing requests. * @param room - The room we want to encrypt for * @param encryptionSettings - body of the m.room.encryption event currently in force in this room */ public constructor( private readonly olmMachine: OlmMachine, private readonly keyClaimManager: KeyClaimManager, - private readonly outgoingRequestProcessor: OutgoingRequestProcessor, + private readonly outgoingRequestManager: OutgoingRequestsManager, private readonly room: Room, private encryptionSettings: IContent, ) { this.prefixedLogger = logger.getChild(`[${room.roomId} encryption]`); + + // start tracking devices for any users already known to be in this room. + // Do not load members here, would defeat lazy loading. + const members = room.getJoinedMembers(); + // At this point just mark the known members as tracked, it might not be the full list of members + // because of lazy loading. This is fine, because we will get a member list update when sending a message for + // the first time, see `RoomEncryptor#ensureEncryptionSession` + this.olmMachine.updateTrackedUsers(members.map((u) => new RustSdkCryptoJs.UserId(u.userId))).then(() => { + this.prefixedLogger.debug(`Updated tracked users for room ${room.roomId}`); + }); } /** @@ -104,6 +118,28 @@ export class RoomEncryptor { } const members = await this.room.getEncryptionTargetMembers(); + + // If this is the first time we are sending a message to the room, we may not yet have seen all the members + // (so the Crypto SDK might not have a device list for them). So, if this is the first time we are encrypting + // for this room, give the SDK the full list of members, to be on the safe side. + // + // This could end up being racy (if two calls to ensureEncryptionSession happen at the same time), but that's + // not a particular problem, since `OlmMachine.updateTrackedUsers` just adds any users that weren't already tracked. + if (!this.lazyLoadedMembersResolved) { + await this.olmMachine.updateTrackedUsers(members.map((u) => new RustSdkCryptoJs.UserId(u.userId))); + this.lazyLoadedMembersResolved = true; + this.prefixedLogger.debug(`Updated tracked users for room ${this.room.roomId}`); + } + + // Query keys in case we don't have them for newly tracked members. + // This must be done before ensuring sessions. If not the devices of these users are not + // known yet and will not get the room key. + // We don't have API to only get the keys queries related to this member list, so we just + // process the pending requests from the olmMachine. (usually these are processed + // at the end of the sync, but we can't wait for that). + // XXX future improvement process only KeysQueryRequests for the tracked users. + await this.outgoingRequestManager.doProcessOutgoingRequests(); + this.prefixedLogger.debug( `Encrypting for users (shouldEncryptForInvitedMembers: ${this.room.shouldEncryptForInvitedMembers()}):`, members.map((u) => `${u.userId} (${u.membership})`), @@ -143,7 +179,7 @@ export class RoomEncryptor { ); if (shareMessages) { for (const m of shareMessages) { - await this.outgoingRequestProcessor.makeOutgoingRequest(m); + await this.outgoingRequestManager.outgoingRequestProcessor.makeOutgoingRequest(m); } } } diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 4d986480e35..33eb1988b5d 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -27,7 +27,7 @@ import { BackupDecryptor, CryptoBackend, OnSyncCompletedData } from "../common-c import { Logger } from "../logger"; import { ClientPrefix, IHttpOpts, MatrixHttpApi, Method } from "../http-api"; import { RoomEncryptor } from "./RoomEncryptor"; -import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; +import { OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; import { KeyClaimManager } from "./KeyClaimManager"; import { encodeUri, MapWithDefault } from "../utils"; import { @@ -72,6 +72,7 @@ import { ClientStoppedError } from "../errors"; import { ISignatures } from "../@types/signed"; import { encodeBase64 } from "../base64"; import { DecryptionError } from "../crypto/algorithms"; +import { OutgoingRequestsManager } from "./OutgoingRequestsManager"; const ALL_VERIFICATION_METHODS = ["m.sas.v1", "m.qr_code.scan.v1", "m.qr_code.show.v1", "m.reciprocate.v1"]; @@ -93,16 +94,6 @@ export class RustCrypto extends TypedEventEmitter = {}; @@ -111,6 +102,7 @@ export class RustCrypto extends TypedEventEmitter = {}; // When did we last try to check the server for a given session id? @@ -143,6 +135,12 @@ export class RustCrypto extends TypedEventEmitter new RustSdkCryptoJs.UserId(u.userId))); } /** called by the sync loop after processing each sync. @@ -1290,7 +1285,9 @@ export class RustCrypto extends TypedEventEmitter { + this.logger.warn("onSyncCompleted: Error processing outgoing requests", e); + }); } /** @@ -1540,68 +1537,10 @@ export class RustCrypto extends TypedEventEmitter { - this.logger.error("Error processing outgoing-message requests from rust crypto-sdk", e); + this.outgoingRequestsManager.doProcessOutgoingRequests().catch((e) => { + this.logger.warn("onKeyVerificationRequest: Error processing outgoing requests", e); }); } - - private async outgoingRequestLoopInner(): Promise { - /* istanbul ignore if */ - if (this.outgoingRequestLoopRunning) { - throw new Error("Cannot run two outgoing request loops"); - } - this.outgoingRequestLoopRunning = true; - try { - while (!this.stopped) { - // we clear the "one more loop" flag just before calling `OlmMachine.outgoingRequests()`, so we can tell - // if `this.outgoingRequestLoop()` was called while `OlmMachine.outgoingRequests()` was running. - this.outgoingRequestLoopOneMoreLoop = false; - - const outgoingRequests: Object[] = await this.olmMachine.outgoingRequests(); - - if (this.stopped) { - // we've been told to stop while `outgoingRequests` was running: exit the loop without processing - // any of the returned requests (anything important will happen next time the client starts.) - return; - } - - if (outgoingRequests.length === 0 && !this.outgoingRequestLoopOneMoreLoop) { - // `OlmMachine.outgoingRequests` returned no messages, and there was no call to - // `this.outgoingRequestLoop()` while it was running. We can stop the loop for a while. - return; - } - - for (const msg of outgoingRequests) { - await this.outgoingRequestProcessor.makeOutgoingRequest(msg as OutgoingRequest); - } - } - } finally { - this.outgoingRequestLoopRunning = false; - } - } } class EventDecryptor { diff --git a/src/timeline-window.ts b/src/timeline-window.ts index 5f0d885696e..0022ae87169 100644 --- a/src/timeline-window.ts +++ b/src/timeline-window.ts @@ -21,6 +21,7 @@ import { logger } from "./logger"; import { MatrixClient } from "./client"; import { EventTimelineSet } from "./models/event-timeline-set"; import { MatrixEvent } from "./models/event"; +import { Room, RoomEvent } from "./models/room"; /** * @internal @@ -74,6 +75,10 @@ export class TimelineWindow { * are received from /sync; you should arrange to call {@link TimelineWindow#paginate} * on {@link RoomEvent.Timeline} events. * + *

Note that constructing an instance of this class for a room adds a + * listener for RoomEvent.Timeline events which is never removed. In theory + * this should not cause a leak since the EventEmitter uses weak mappings. + * * @param client - MatrixClient to be used for context/pagination * requests. * @@ -87,6 +92,7 @@ export class TimelineWindow { opts: IOpts = {}, ) { this.windowLimit = opts.windowLimit || 1000; + timelineSet.room?.on(RoomEvent.Timeline, this.onTimelineEvent.bind(this)); } /** @@ -193,6 +199,23 @@ export class TimelineWindow { return false; } + private onTimelineEvent(_event?: MatrixEvent, _room?: Room, _atStart?: boolean, removed?: boolean): void { + if (removed) { + this.onEventRemoved(); + } + } + + /** + * If an event was removed, meaning this window is longer than the timeline, + * shorten the window. + */ + private onEventRemoved(): void { + const events = this.getEvents(); + if (events.length > 0 && events[events.length - 1] === undefined && this.end) { + this.end.index--; + } + } + /** * Check if this window can be extended * diff --git a/yarn.lock b/yarn.lock index d8b397672ca..6e6ea3a0dfc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1249,10 +1249,10 @@ resolved "https://registry.yarnpkg.com/@eslint-community/regexpp/-/regexpp-4.6.2.tgz#1816b5f6948029c5eaacb0703b850ee0cb37d8f8" integrity sha512-pPTNuaAG3QMH+buKyBIGJs3g/S5y0caxw0ygM3YyE6yJFySwiGGSzA+mM3KJ8QQvzeLh3blwgSonkFjgQdxzMw== -"@eslint/eslintrc@^2.1.2": - version "2.1.2" - resolved "https://registry.yarnpkg.com/@eslint/eslintrc/-/eslintrc-2.1.2.tgz#c6936b4b328c64496692f76944e755738be62396" - integrity sha512-+wvgpDsrB1YqAMdEUCcnTlpfVBH7Vqn6A/NT3D8WVXFIaKMlErPIZT3oCIAVCOtarRpMtelZLqJeU3t7WY6X6g== +"@eslint/eslintrc@^2.1.3": + version "2.1.3" + resolved "https://registry.yarnpkg.com/@eslint/eslintrc/-/eslintrc-2.1.3.tgz#797470a75fe0fbd5a53350ee715e85e87baff22d" + integrity sha512-yZzuIG+jnVu6hNSzFEN07e8BxF3uAzYtQb6uDkaYZLo6oYZDCq454c5kB8zxnzfCYyP4MIuyBn10L0DqwujTmA== dependencies: ajv "^6.12.4" debug "^4.3.2" @@ -1264,10 +1264,10 @@ minimatch "^3.1.2" strip-json-comments "^3.1.1" -"@eslint/js@8.52.0": - version "8.52.0" - resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.52.0.tgz#78fe5f117840f69dc4a353adf9b9cd926353378c" - integrity sha512-mjZVbpaeMZludF2fsWLD0Z9gCref1Tk4i9+wddjRvpUNqqcndPkBD09N/Mapey0b3jaXbLm2kICwFv2E64QinA== +"@eslint/js@8.53.0": + version "8.53.0" + resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.53.0.tgz#bea56f2ed2b5baea164348ff4d5a879f6f81f20d" + integrity sha512-Kn7K8dx/5U6+cT1yEhpX1w4PCSg0M+XyRILPgvwcEBjerFWCwQj5sbr3/VmxqV0JGHCBCzyd6LxypEuehypY1w== "@humanwhocodes/config-array@^0.11.13": version "0.11.13" @@ -3267,15 +3267,15 @@ eslint-visitor-keys@^3.4.1: resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-3.4.2.tgz#8c2095440eca8c933bedcadf16fefa44dbe9ba5f" integrity sha512-8drBzUEyZ2llkpCA67iYrgEssKDUu68V8ChqqOfFupIaG/LCVPUT+CoGJpT77zJprs4T/W7p07LP7zAIMuweVw== -eslint@8.52.0: - version "8.52.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-8.52.0.tgz#d0cd4a1fac06427a61ef9242b9353f36ea7062fc" - integrity sha512-zh/JHnaixqHZsolRB/w9/02akBk9EPrOs9JwcTP2ek7yL5bVvXuRariiaAjjoJ5DvuwQ1WAE/HsMz+w17YgBCg== +eslint@8.53.0: + version "8.53.0" + resolved "https://registry.yarnpkg.com/eslint/-/eslint-8.53.0.tgz#14f2c8244298fcae1f46945459577413ba2697ce" + integrity sha512-N4VuiPjXDUa4xVeV/GC/RV3hQW9Nw+Y463lkWaKKXKYMvmRiRDAtfpuPFLN+E1/6ZhyR8J2ig+eVREnYgUsiag== dependencies: "@eslint-community/eslint-utils" "^4.2.0" "@eslint-community/regexpp" "^4.6.1" - "@eslint/eslintrc" "^2.1.2" - "@eslint/js" "8.52.0" + "@eslint/eslintrc" "^2.1.3" + "@eslint/js" "8.53.0" "@humanwhocodes/config-array" "^0.11.13" "@humanwhocodes/module-importer" "^1.0.1" "@nodelib/fs.walk" "^1.2.8"