[electron] require reauth after scopes change (#276)

Co-authored-by: Ben Polinsky <[email protected]>

.github/workflows/ci.yaml

@@ -30,12 +30,10 @@ jobs:
       - name: Setup node
         uses: actions/setup-node@v2
         with:
-          node-version: "18.12.0"
+          node-version: 18.x
 
       - name: Setup pnpm
         uses: pnpm/action-setup@v4
-        with:
-          version: latest
 
       - name: Install packages
         run: pnpm install

.github/workflows/codeql-analysis.yaml

@@ -20,7 +20,7 @@ jobs:
 
     steps:
       - name: Checkout branch
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 

.github/workflows/create-release.yaml

@@ -11,19 +11,17 @@ jobs:
     name: Release packages
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           token: ${{ secrets.IMJS_ADMIN_GH_TOKEN }}
 
       - name: Install pnpm
         uses: pnpm/action-setup@v4
-        with:
-          version: latest
 
       - name: Use Node.js 18
         uses: actions/setup-node@v3
         with:
-          node-version: 18.16.x
+          node-version: 18.x
           registry-url: https://registry.npmjs.org/
 
       - name: Install dependencies

.gitignore

@@ -9,7 +9,6 @@ dist
 
 # testing
 .nyc_output
-.parcel-cache
 junit_results.xml
 /**/test-results/**/*
 generated-docs

.pipelines/integration-test.yaml

@@ -34,7 +34,7 @@ jobs:
   - task: NodeTool@0
     displayName: Use Node 18
     inputs:
-      versionSpec: 18.x
+      versionSpec: 18.16.x
 
   - script: npm install -g pnpm@9
     displayName: Install pnpm

change/@itwin-electron-authorization-aff4393d-f27d-43de-8277-d8e1d47d14d4.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Delete stored token if application scopes change",
+  "packageName": "@itwin/electron-authorization",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@itwin-oidc-signin-tool-c779af30-967f-4a82-8c03-07e6eb78b688.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "",
+  "packageName": "@itwin/oidc-signin-tool",
+  "email": "[email protected]",
+  "dependentChangeType": "none"
+}

package.json

@@ -35,5 +35,6 @@
   "devDependencies": {
     "beachball": "^2.49.1",
     "lage": "^2.11.13"
-  }
-}
+  },
+  "packageManager": "[email protected]+sha256.09a8fe31a34fda706354680619f4002f4ccef6dadff93240d24ef6c831f0fd28"
+}

packages/electron/package.json

@@ -27,8 +27,8 @@
     "test": "mocha \"./lib/cjs/test/**/*.test.js\"",
     "test:integration": "pnpm test:integration:build && pnpm exec playwright test",
     "test:integration:start": "pnpm test:integration:build && electron dist/integration-test/test-app/index.js",
-    "test:integration:build": "pnpm test:integration:parcel && tsc -p ./src/integration-test/",
-    "test:integration:parcel": "parcel build src/integration-test/test-app/index.html --dist-dir dist/integration-test/test-app --public-url ./",
+    "test:integration:build": "pnpm test:integration:vite && tsc -p ./src/integration-test/",
+    "test:integration:vite": "vite build",
     "pack": "npm pack",
     "rebuild": "npm run clean && npm run build"
   },
@@ -66,13 +66,11 @@
     "eslint": "^8.56.0",
     "mocha": "^10.2.0",
     "nyc": "^17.0.0",
-    "parcel": "~2.12.0",
-    "path-browserify": "~1.0.1",
-    "process": "~0.11.10",
     "rimraf": "^3.0.2",
     "sinon": "^15.0.1",
     "source-map-support": "^0.5.9",
-    "typescript": "~5.3.3"
+    "typescript": "~5.3.3",
+    "vite": "^6.0.3"
   },
   "peerDependencies": {
     "@itwin/core-bentley": "^3.3.0 || ^4.0.0",

packages/electron/src/integration-test/integration.test.ts

@@ -37,7 +37,7 @@ const userDataPath = getElectronUserDataPath();
 let electronApp: ElectronApplication;
 let electronPage: Page;
 const testHelper = new TestHelper(signInOptions);
-const tokenStore = new RefreshTokenStore(getTokenStoreFileName(),getTokenStoreKey(), userDataPath);
+const tokenStore = new RefreshTokenStore(getTokenStoreFileName(), getTokenStoreKey(), userDataPath);
 
 function getTokenStoreKey(issuerUrl?: string): string {
   const authority = new URL(issuerUrl ?? "https://ims.bentley.com");
@@ -93,7 +93,8 @@ test("sign in successful", async ({ browser }) => {
   const page = await browser.newPage();
   await testHelper.checkStatus(electronPage, false);
   await testHelper.clickSignIn(electronPage);
-  await testHelper.signIn(page, await getUrl(electronApp));
+  const url = await getUrl(electronApp);
+  await testHelper.signIn(page, url);
   await page.waitForLoadState("networkidle");
   await testHelper.checkStatus(electronPage, true);
   await page.close();
@@ -110,3 +111,17 @@ test("sign out successful", async ({ browser }) => {
   await testHelper.checkStatus(electronPage, false);
   await page.close();
 });
+
+test("when scopes change, sign in is required", async ({ browser }) => {
+  const page = await browser.newPage();
+  await testHelper.clickSignIn(electronPage);
+  await testHelper.signIn(page, await getUrl(electronApp));
+  await page.waitForLoadState("networkidle");
+  await testHelper.checkStatus(electronPage, true);
+
+  // Admittedly this is cheating: no user would interact
+  // with the tokenStore directly, but this is a tough
+  // case to test otherwise.
+  await tokenStore.load("itwin-platform realitydata:read");
+  await testHelper.checkStatus(electronPage, false);
+});

packages/electron/src/integration-test/test-app/index.html

@@ -1,19 +1,19 @@
 <!DOCTYPE html>
 <html>
-  <head>
-    <meta charset="UTF-8" />
-    <!-- https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP -->
-    <meta
-      http-equiv="Content-Security-Policy"
-      content="default-src 'self'; script-src 'self'"
-    />
-    <title>Hello World!</title>
-    <script type="module" src="./renderer.ts"></script>
-  </head>
-  <body>
-    <button data-testid="signIn" id="signIn">Sign In</button>
-    <button data-testid="signOut" id="signOut">Sign Out</button>
-    <button data-testid="getStatus" id="getStatus">Get Sign-in Status</button>
-    <h2 data-testid="status" id="status"></h2>
-  </body>
-</html>
+
+<head>
+  <meta charset="UTF-8" />
+  <!-- https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP -->
+  <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'" />
+  <title>Hello World!</title>
+  <script type="module" src="./renderer.ts"></script>
+</head>
+
+<body>
+  <button data-testid="signIn" id="signIn">Sign In</button>
+  <button data-testid="signOut" id="signOut">Sign Out</button>
+  <button data-testid="getStatus" id="getStatus">Get Sign-in Status</button>
+  <h2 data-testid="status" id="status"></h2>
+</body>
+
+</html>

packages/electron/src/integration-test/tsconfig.json

@@ -1,7 +1,7 @@
 {
   "compilerOptions": {
-    "module": "node16",
-    "moduleResolution": "node16",
+    "module": "NodeNext",
+    "moduleResolution": "nodenext",
     "lib": ["DOM"],
     "target": "ES5",
     "skipLibCheck": true,
@@ -10,4 +10,4 @@
   },
   "include": ["./test-app/index.ts", "../renderer/ElectronPreload.ts"],
   "exclude": ["node_modules"]
-}
+}

packages/electron/src/main/Client.ts

@@ -302,7 +302,8 @@ export class ElectronMainAuthorization implements AuthorizationClient {
    * @return AccessToken if it's possible to get a valid access token, and undefined otherwise.
    */
   private async loadAccessToken(): Promise<AccessToken> {
-    const refreshToken = await this._refreshTokenStore.load();
+    const refreshToken = await this._refreshTokenStore.load(this._scopes);
+
     if (!refreshToken)
       return "";
 
@@ -526,7 +527,7 @@ export class ElectronMainAuthorization implements AuthorizationClient {
   ): Promise<AccessToken> {
     this._refreshToken = tokenResponse.refreshToken;
     if (this._refreshToken)
-      await this._refreshTokenStore.save(this._refreshToken);
+      await this._refreshTokenStore.save(this._refreshToken, this._scopes);
 
     const expiresAtMilliseconds =
       (tokenResponse.issuedAt + (tokenResponse.expiresIn ?? 0)) * 1000;

packages/electron/src/main/TokenStore.ts

@@ -7,6 +7,7 @@
 import { safeStorage } from "electron";
 // eslint-disable-next-line @typescript-eslint/naming-convention
 const Store = require("electron-store"); // eslint-disable-line @typescript-eslint/no-var-requires
+
 /**
  * Utility class used to store and read OAuth refresh tokens.
  * @internal
@@ -34,31 +35,8 @@ export class RefreshTokenStore {
     });
   }
 
-  private async getUserName(): Promise<string | undefined> {
-    if (!this._userName) {
-      this._userName = await (await import("username")).username();
-    }
-
-    return this._userName;
-  }
-
-  // Note: this is intentionally made async in case this code doesn't run in electron's main process and safeStorage must be polyfilled with async function
-  private async encryptRefreshToken(token: string): Promise<Buffer> {
-    return safeStorage.encryptString(token);
-  }
-
-  // Note: this is intentionally made async in case this code doesn't run in electron's main process and safeStorage must be polyfilled with async function
-  private async decryptRefreshToken(encryptedToken: Buffer): Promise<string> {
-    return safeStorage.decryptString(Buffer.from(encryptedToken));
-  }
-
-  private async getKey(): Promise<string> {
-    const userName = await this.getUserName();
-    return `${this._appStorageKey}${userName}`;
-  }
-
-  /** Load refresh token if available */
-  public async load(): Promise<string | undefined> {
+  /** (Load) refresh token if available */
+  public async load(scopes?: string): Promise<string | undefined> {
     const userName = await this.getUserName();
     if (!userName)
       return undefined;
@@ -67,19 +45,26 @@ export class RefreshTokenStore {
     if (!this._store.has(key)) {
       return undefined;
     }
+
+    if (scopes && !(await this.scopesMatch(scopes)))
+      return;
+
     const encryptedToken = this._store.get(key);
     const refreshToken = await this.decryptRefreshToken(encryptedToken).catch(() => undefined);
+
     return refreshToken;
   }
 
   /** Save refresh token after signin */
-  public async save(refreshToken: string): Promise<void> {
+  public async save(refreshToken: string, scopes?: string): Promise<void> {
     const userName = await this.getUserName();
     if (!userName)
       return;
     const encryptedToken = await this.encryptRefreshToken(refreshToken);
     const key = await this.getKey();
     this._store.set(key, encryptedToken);
+    if (scopes)
+      this._store.set(`${key}:scopes`, scopes);
   }
 
   /** Delete refresh token after signout */
@@ -90,5 +75,40 @@ export class RefreshTokenStore {
 
     const key = await this.getKey();
     await this._store.delete(key);
+    await this._store.delete(`${key}:scopes`);
+  }
+
+  private async getUserName(): Promise<string | undefined> {
+    if (!this._userName) {
+      this._userName = await (await import("username")).username();
+    }
+
+    return this._userName;
+  }
+
+  // Note: this is intentionally made async in case this code doesn't run in electron's main process and safeStorage must be polyfilled with async function
+  private async encryptRefreshToken(token: string): Promise<Buffer> {
+    return safeStorage.encryptString(token);
+  }
+
+  // Note: this is intentionally made async in case this code doesn't run in electron's main process and safeStorage must be polyfilled with async function
+  private async decryptRefreshToken(encryptedToken: Buffer): Promise<string> {
+    return safeStorage.decryptString(Buffer.from(encryptedToken));
+  }
+
+  private async getKey(): Promise<string> {
+    const userName = await this.getUserName();
+    return `${this._appStorageKey}${userName}`;
+  }
+
+  private async scopesMatch(scopes: string): Promise<boolean> {
+    const key = await this.getKey();
+    const savedScopes = this._store.get(`${key}:scopes`);
+    if (savedScopes) {
+      return savedScopes.split(" ").sort().join(" ") === scopes.split(" ").sort().join(" ");
+    }
+
+    // no stored scopes, so all good
+    return true;
   }
 }