Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit support for OIDC silent renew across the web template and the web-viewer-test app by introducing a dedicated silent callback page and wiring a new silentRedirectUri environment variable into the auth client configuration.
Changes:
- Add
signin-silent.html+signin-silent.tscallback entrypoints for silent renew. - Add
IMJS_AUTH_CLIENT_SILENT_REDIRECT_URIenv var (typing + docs + sample.env) and pass it intoBrowserAuthorizationClientconfiguration. - Configure the template Vite build for a multi-page output including the silent callback HTML.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/templates/web/vite.config.mts | Adds multi-page Rollup input for signin-silent.html (but currently uses __dirname in .mts). |
| packages/templates/web/src/vite-env.d.ts | Adds typing for IMJS_AUTH_CLIENT_SILENT_REDIRECT_URI. |
| packages/templates/web/src/signin-silent.ts | Adds a silent callback handler entrypoint. |
| packages/templates/web/src/components/Authorization.tsx | Passes silentRedirectUri to BrowserAuthorizationClient. |
| packages/templates/web/signin-silent.html | Adds the silent callback HTML entry. |
| packages/templates/web/README.md | Documents the new silent redirect env var and how to configure it. |
| packages/templates/web/.env | Adds the new env var placeholder. |
| packages/apps/web-viewer-test/src/signin-silent.ts | Adds a silent callback handler entrypoint. |
| packages/apps/web-viewer-test/src/services/auth/AuthorizationClient.ts | Passes silentRedirectUri into the auth client configuration. |
| packages/apps/web-viewer-test/signin-silent.html | Adds the silent callback HTML entry. |
| packages/apps/web-viewer-test/README.md | Documents silent redirect env var configuration. |
| packages/apps/web-viewer-test/.env | Sets a default local IMJS_AUTH_CLIENT_SILENT_REDIRECT_URI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rollupOptions: { | ||
| input: { | ||
| main: resolve(__dirname, "index.html"), | ||
| "signin-silent": resolve(__dirname, "signin-silent.html"), | ||
| }, |
There was a problem hiding this comment.
vite.config.mts is an ESM/TypeScript config (.mts), so __dirname is not defined in Node ESM and this will throw at config-load time. Compute the directory from import.meta.url (e.g., via fileURLToPath/dirname) or use new URL("index.html", import.meta.url) style paths instead of __dirname.
|
|
||
| import { BrowserAuthorizationClient } from "@itwin/browser-authorization"; | ||
|
|
||
| BrowserAuthorizationClient.handleSignInCallback().catch(() => window.close()); |
There was a problem hiding this comment.
The codebase uses client.handleSigninCallback() (note casing/spelling: Signin) elsewhere, but this new page calls BrowserAuthorizationClient.handleSignInCallback() (SignIn) as a static. Please verify the exact API name; if handleSignInCallback doesn’t exist this will fail at runtime/compile-time. Prefer using the same callback handler naming used by the existing redirect flow for consistency.
| BrowserAuthorizationClient.handleSignInCallback().catch(() => window.close()); | |
| BrowserAuthorizationClient.handleSigninCallback().catch(() => window.close()); |
|
|
||
| import { BrowserAuthorizationClient } from "@itwin/browser-authorization"; | ||
|
|
||
| BrowserAuthorizationClient.handleSignInCallback().catch(() => window.close()); |
There was a problem hiding this comment.
The codebase uses client.handleSigninCallback() (note casing/spelling: Signin) elsewhere, but this new page calls BrowserAuthorizationClient.handleSignInCallback() (SignIn) as a static. Please verify the exact API name; if handleSignInCallback doesn’t exist this will fail at runtime/compile-time. Prefer using the same callback handler naming used by the existing redirect flow for consistency.
| BrowserAuthorizationClient.handleSignInCallback().catch(() => window.close()); | |
| BrowserAuthorizationClient.handleSigninCallback().catch(() => window.close()); |
| responseType: "code", | ||
| silentRedirectUri: import.meta.env.IMJS_AUTH_CLIENT_SILENT_REDIRECT_URI, | ||
| }; |
There was a problem hiding this comment.
import.meta.env.IMJS_AUTH_CLIENT_SILENT_REDIRECT_URI is referenced here, but packages/apps/web-viewer-test/src/vite-env.d.ts doesn’t declare this env var in ImportMetaEnv, so this will fail type-checking. Add the new optional env key to that file (or adjust env typing) to keep TS builds green.
| <script type="module" src="/src/signin-silent.ts"></script> | ||
| </body> |
There was a problem hiding this comment.
This app adds a new standalone HTML entry (signin-silent.html), but vite.config.ts for web-viewer-test currently doesn’t configure a multi-page Rollup input. In production builds this page may not be emitted to dist/, which would break silent renew. Consider adding build.rollupOptions.input for this HTML entry (similar to the template’s config).
Add explicit support for silent renew of auth tokens to web template.
(Desktop applications use refresh token)