[Backport] Fix OIDC login callback handling on Element Desktop (#33337)

.github/workflows/tests.yml

@@ -93,7 +93,7 @@ jobs:
               if: env.ENABLE_COVERAGE == 'true'
               uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
               with:
-                  name: coverage-${{ matrix.runner }}
+                  name: coverage-jest-${{ matrix.runner }}
                   path: |
                       apps/web/coverage
                       !apps/web/coverage/lcov-report
@@ -124,9 +124,10 @@ jobs:
         name: Vitest
         strategy:
             matrix:
-                package:
-                    - shared-components
-                    - module-api
+                path:
+                    - apps/desktop
+                    - packages/shared-components
+                    - packages/module-api
         runs-on: ubuntu-24.04
         steps:
             - name: Checkout code
@@ -149,30 +150,39 @@ jobs:
               uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5
               with:
                   path: |
-                      packages/${{ matrix.package }}/node_modules/.cache
-                      packages/${{ matrix.package }}/node_modules/.vite/vitest
-                  key: ${{ hashFiles('pnpm-lock.yaml') }}
+                      ${{ matrix.path }}/node_modules/.cache
+                      ${{ matrix.path }}/node_modules/.vite/vitest
+                  key: ${{ matrix.path }}-${{ hashFiles('pnpm-lock.yaml') }}
 
             - name: Setup playwright
               uses: ./.github/actions/setup-playwright
-              if: matrix.package == 'shared-components'
+              if: matrix.path == 'packages/shared-components'
               with:
                   write-cache: ${{ github.event_name != 'merge_group' }}
 
             - name: Run tests
-              working-directory: "packages/${{ matrix.package }}"
+              working-directory: ${{ matrix.path }}
               run: pnpm test:unit --coverage=$ENABLE_COVERAGE
 
             # Dump the disk usage on failure, because this job seems to fail with disk fills sometimes
             - name: df
-              run: df
+              run: df -h && df -i
               if: ${{ failure() }}
 
+            - name: Calculate artifact name
+              if: env.ENABLE_COVERAGE == 'true'
+              id: artifact
+              run: |
+                  NAME=$(basename "$MATRIX_PATH")
+                  echo "name=$NAME" >> $GITHUB_OUTPUT
+              env:
+                  MATRIX_PATH: ${{ matrix.path }}
+
             - name: Upload Artifact
               if: env.ENABLE_COVERAGE == 'true'
               uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
               with:
-                  name: coverage-${{ matrix.package }}
+                  name: coverage-${{ steps.artifact.outputs.name }}
                   path: |
-                      packages/${{ matrix.package }}/coverage
-                      !packages/${{ matrix.package }}/coverage/lcov-report
+                      ${{ matrix.path }}/coverage
+                      !${{ matrix.path }}/coverage/lcov-report

apps/desktop/.eslintrc.cjs

@@ -86,5 +86,12 @@ module.exports = {
                 "@typescript-eslint/no-non-null-assertion": "off",
             },
         },
+        {
+            files: ["src/**/*.test.ts", "electron-builder.ts", "vitest.config.ts"],
+            extends: ["plugin:matrix-org/typescript"],
+            parserOptions: {
+                project: ["tsconfig.node.json"],
+            },
+        },
     ],
 };

apps/desktop/electron-builder.ts

@@ -52,6 +52,7 @@ interface Variant extends Metadata {
 }
 
 type Writable<T> = NonNullable<
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
     T extends Function ? T : T extends object ? { -readonly [K in keyof T]: Writable<T[K]> } : T
 >;
 
@@ -74,7 +75,7 @@ if (process.env.VARIANT_PATH) {
 }
 
 for (const key in variant) {
-    console.log(`${key}: ${variant[key]}`);
+    console.log(`${key}: ${variant[key as keyof Variant]}`);
 }
 
 interface Configuration extends BaseConfiguration {

apps/desktop/package.json

@@ -32,8 +32,9 @@
         "lint": "pnpm lint:types && pnpm lint:js",
         "lint:js": "eslint --max-warnings 0 src hak playwright scripts",
         "lint:js-fix": "eslint --fix --max-warnings 0 src hak playwright scripts && prettier --log-level=warn --write .",
-        "lint:types": "pnpm lint:types:src && pnpm lint:types:test && pnpm lint:types:scripts && pnpm lint:types:hak",
+        "lint:types": "pnpm lint:types:src && pnpm lint:types:node && pnpm lint:types:test && pnpm lint:types:scripts && pnpm lint:types:hak",
         "lint:types:src": "tsc --noEmit",
+        "lint:types:node": "tsc --noEmit -p tsconfig.node.json",
         "lint:types:test": "tsc --noEmit -p playwright/tsconfig.json",
         "lint:types:scripts": "tsc --noEmit -p scripts/tsconfig.json",
         "lint:types:hak": "tsc --noEmit -p hak/tsconfig.json",
@@ -49,6 +50,7 @@
         "docker:install": "scripts/in-docker.sh pnpm install",
         "clean": "rimraf webapp.asar dist packages deploys lib",
         "hak": "node scripts/hak/index.ts",
+        "test:unit": "vitest",
         "test:playwright": "nx test:playwright --",
         "test:playwright:open": "nx test:playwright -- --ui",
         "test:playwright:screenshots": "nx test:playwright:screenshots --",
@@ -79,6 +81,7 @@
         "@types/pacote": "^11.1.1",
         "@typescript-eslint/eslint-plugin": "^8.0.0",
         "@typescript-eslint/parser": "^8.0.0",
+        "@vitest/coverage-v8": "^4.1.5",
         "app-builder-lib": "26.8.2",
         "chokidar": "^5.0.0",
         "detect-libc": "^2.0.0",
@@ -95,12 +98,16 @@
         "eslint-plugin-unicorn": "^56.0.0",
         "glob": "^13.0.0",
         "matrix-web-i18n": "catalog:",
+        "memfs": "^4.57.2",
         "mkdirp": "^3.0.0",
         "pacote": "^21.0.0",
         "prettier": "^3.0.0",
         "rimraf": "^6.0.0",
         "tar": "^7.5.8",
-        "typescript": "5.9.3"
+        "typescript": "5.9.3",
+        "vite": "^8.0.9",
+        "vitest": "^4.1.5",
+        "vitest-sonar-reporter": "^3.0.0"
     },
     "hakDependencies": {
         "matrix-seshat": "4.2.0"

apps/desktop/src/protocol.test.ts

@@ -0,0 +1,87 @@
+/*
+Copyright 2026 Element Creations Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { expect, describe, it, beforeEach, vi } from "vitest";
+import { fs as memfs, vol } from "memfs";
+
+import ProtocolHandler from "./protocol.js";
+
+const TEST_PROTOCOL = "test.proto";
+const TEST_SESSION_ID = "test_session_id";
+const USER_DATA_DIR = "/Users/name/Library/Application Support/Element";
+
+vi.mock("node:fs", () => ({ default: memfs }));
+vi.mock("electron", () => ({
+    app: {
+        getPath: vi.fn().mockReturnValue("/Users/name/Library/Application Support/Element"),
+        on: vi.fn(),
+    },
+    ipcMain: {
+        handle: vi.fn(),
+    },
+}));
+
+beforeEach(() => {
+    // Reset the state of the in-memory fs
+    vol.reset();
+});
+
+describe("ProtocolHandler", () => {
+    describe("getProfileFromDeeplink", () => {
+        const handler = new ProtocolHandler(TEST_PROTOCOL);
+
+        beforeEach(() => {
+            vol.fromJSON(
+                {
+                    "./sso-sessions.json": JSON.stringify({ [TEST_SESSION_ID]: USER_DATA_DIR }),
+                },
+                USER_DATA_DIR,
+            );
+        });
+
+        it("should handle legacy SSO URIs", () => {
+            expect(
+                handler.getProfileFromDeeplink([
+                    "Element.app",
+                    `element://vector/webapp/?element-desktop-ssoid=${TEST_SESSION_ID}`,
+                ]),
+            ).toBe(USER_DATA_DIR);
+        });
+
+        it("should handle OIDC URIs with response_mode=query", () => {
+            expect(
+                handler.getProfileFromDeeplink([
+                    "Element.app",
+                    `${TEST_PROTOCOL}:/vector/webapp/?no_universal_links=true&code=DEADBEEF&state=foobar:element-desktop-ssoid:${TEST_SESSION_ID}`,
+                ]),
+            ).toBe(USER_DATA_DIR);
+        });
+
+        it("should handle OIDC URIs with response_mode=fragment", () => {
+            expect(
+                handler.getProfileFromDeeplink([
+                    "Element.app",
+                    `${TEST_PROTOCOL}:/vector/webapp/?no_universal_links=true#code=DEADBEEF&state=foobar:element-desktop-ssoid:${TEST_SESSION_ID}`,
+                ]),
+            ).toBe(USER_DATA_DIR);
+        });
+
+        it("should handle malformed OIDC URIs gracefully", () => {
+            expect(
+                handler.getProfileFromDeeplink([
+                    "Element.app",
+                    `${TEST_PROTOCOL}:/vector/webapp/?no_universal_links=true#code=DEADBEEF:element-desktop-ssoid:${TEST_SESSION_ID}`,
+                ]),
+            ).toBeUndefined();
+        });
+
+        it("should handle unrelated URIs gracefully", () => {
+            expect(handler.getProfileFromDeeplink(["Element.app", `${TEST_PROTOCOL}:/vector/webapp/`])).toBeUndefined();
+            expect(handler.getProfileFromDeeplink(["Element.app", `test.unrelated:/vector/webapp/`])).toBeUndefined();
+        });
+    });
+});

apps/desktop/src/protocol.ts

@@ -97,7 +97,8 @@ export default class ProtocolHandler {
             const s = fs.readFileSync(storePath, { encoding: "utf8" });
             const o = JSON.parse(s);
             return typeof o === "object" ? o : {};
-        } catch {
+        } catch (e) {
+            console.warn("Unable to read protocol store, starting with empty store: ", e);
             return {};
         }
     }
@@ -130,10 +131,26 @@ export default class ProtocolHandler {
                 let sessionId = parsedUrl.searchParams.get(SEARCH_PARAM);
                 if (!sessionId) {
                     // In OIDC, we must shuttle the value in the `state` param rather than `element-desktop-ssoid`
-                    // We encode it as a suffix like `:element-desktop-ssoid:XXYYZZ`
-                    sessionId = parsedUrl.searchParams.get("state")!.split(`:${SEARCH_PARAM}:`)[1];
+                    // We encode it as a suffix like `:element-desktop-ssoid:XXYYZZ`.
+                    // The OIDC flow may have used response_mode=fragment or query, so we need to handle both cases.
+                    let searchParams = parsedUrl.searchParams;
+                    if (parsedUrl.hash.includes("=")) {
+                        const [params] = parsedUrl.hash.substring(1).split("?", 2);
+                        searchParams = new URLSearchParams(params);
+                    }
+
+                    const state = searchParams.get("state");
+                    if (state) {
+                        sessionId = state.split(`:${SEARCH_PARAM}:`)[1];
+                    }
                 }
-                console.log("Forwarding to profile: ", store[sessionId]);
+
+                if (!sessionId) {
+                    console.warn("Unable to read session ID in deeplink url:", deeplinkUrl);
+                    return undefined;
+                }
+
+                console.log("Forwarding to profile:", store[sessionId]);
                 return store[sessionId];
             }
         }

apps/desktop/tsconfig.json

@@ -14,5 +14,6 @@
         "lib": ["es2022", "es2024.promise"],
         "strict": true
     },
-    "include": ["./src/**/*.ts", "./src/**/*.cts"]
+    "include": ["./src/**/*.ts", "./src/**/*.cts"],
+    "exclude": ["./src/**/*.test.ts"]
 }

apps/desktop/tsconfig.node.json

@@ -0,0 +1,15 @@
+{
+    "compilerOptions": {
+        "resolveJsonModule": true,
+        "module": "nodenext",
+        "moduleResolution": "NodeNext",
+        "target": "es2022",
+        "sourceMap": false,
+        "typeRoots": [],
+        "types": [],
+        "skipLibCheck": true,
+        "noEmit": true,
+        "strict": true
+    },
+    "include": ["./electron-builder.ts", "./vitest.config.ts", "./src/**/*.d.ts", "./src/**/*.test.ts"]
+}

apps/desktop/vitest.config.ts

@@ -0,0 +1,64 @@
+/*
+Copyright 2026 Element Creations Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { defineConfig } from "vitest/config";
+import { type UserConfig } from "vite";
+import { type Reporter } from "vitest/reporters";
+import { env } from "node:process";
+
+const reporters: NonNullable<UserConfig["test"]>["reporters"] = [["default"]];
+
+const slowTestReporter: Reporter = {
+    onTestRunEnd(testModules, unhandledErrors, reason) {
+        const tests = testModules
+            .flatMap((m) => Array.from(m.children.allTests()))
+            .filter((test) => test.diagnostic()?.slow);
+        tests.sort((x, y) => x.diagnostic()!.duration! - y.diagnostic()!.duration!);
+        tests.reverse();
+
+        if (tests.length > 0) {
+            console.warn("Slowest 10 tests:");
+        }
+        for (const t of tests.slice(0, 10)) {
+            console.warn(`${t.module.moduleId} > ${t.fullName}: ${t.diagnostic()?.duration.toFixed(0)}ms`);
+        }
+    },
+};
+
+// if we're running under GHA, enable the GHA & Sonar reporters
+if (env["GITHUB_ACTIONS"] !== undefined) {
+    reporters.push(["github-actions", { silent: false }]);
+    reporters.push([
+        "vitest-sonar-reporter",
+        {
+            outputFile: "coverage/sonar-report.xml",
+            onWritePath: (path): string => `apps/desktop/${path}`,
+        },
+    ]);
+
+    // if we're running against the develop branch, also enable the slow test reporter
+    if (env["GITHUB_REF"] == "refs/heads/develop") {
+        reporters.push(slowTestReporter);
+    }
+}
+
+export default defineConfig({
+    test: {
+        coverage: {
+            provider: "v8",
+            include: ["src/**/*"],
+            // The coverage report currently chokes on this file as it doesn't process it as TypeScript
+            exclude: ["src/preload.cts"],
+            reporter: [["lcov", { projectRoot: "../../" }]],
+        },
+        environment: "node",
+        reporters,
+        globals: true,
+        pool: "threads",
+        include: ["src/**/*.test.ts"],
+    },
+});

sonar-project.properties

@@ -5,8 +5,14 @@ sonar.organization=element-hq
 #sonar.sourceEncoding=UTF-8
 
 sonar.sources=.
-sonar.tests=apps/web/test,apps/web/playwright,apps/desktop/playwright,packages
-sonar.test.inclusions=apps/web/test/*,apps/web/playwright/*,apps/desktop/playwright/*,packages/*/src/**/*.test.*,packages/*/src/test/**/*
+sonar.tests=apps/web/test,apps/web/playwright,apps/desktop,packages
+sonar.test.inclusions=\
+  apps/web/test/*,\
+  apps/web/playwright/*,\
+  apps/desktop/playwright/*,\
+  apps/desktop/src/**/*.test.ts,\
+  packages/*/src/**/*.test.*,\
+  packages/*/src/test/**/*
 sonar.exclusions=\
   apps/web/__mocks__,\
   docs,\
@@ -32,16 +38,23 @@ sonar.coverage.exclusions=\
   apps/web/playwright/**/*,\
   apps/web/res/**/*,\
   apps/web/scripts/**/*,\
-  apps/desktop/scripts/**/*,\
-  apps/desktop/playwright/**/*,\
-  apps/desktop/hak/**/*,\
-  scripts/**/*,\
+  apps/web/__mocks__/**/*,\
+  apps/web/I18nWebpackPlugin.ts,\
+  apps/web/recorder-worklet-loader.cjs,\
   apps/web/src/components/views/dialogs/devtools/**/*,\
   apps/web/src/utils/SessionLock.ts,\
   apps/web/src/**/*.d.ts,\
   apps/web/src/vector/mobile_guide/**/*,\
+  apps/desktop/electron-builder.ts,\
+  apps/desktop/scripts/**/*,\
+  apps/desktop/playwright/**/*,\
+  apps/desktop/hak/**/*,\
   packages/shared-components/src/test/**/*,\
   packages/shared-components/src/**/*.stories.tsx,\
+  packages/shared-components/scripts/**/*,\
   packages/playwright-common/**/*,\
-  **/*.config.ts
+  scripts/**/*,\
+  docs/**/*,\
+  **/*.config.*,\
+  knip.ts
 sonar.testExecutionReportPaths=apps/web/coverage/jest-sonar-report.xml