-
-
Notifications
You must be signed in to change notification settings - Fork 28
feat(desktop): show team dashboard link #1490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds team dashboard functionality for desktop users by redirecting to the web version, fixes user profile data mapping issues, and modifies the JWT token authentication flow to always re-issue tokens while preventing duplicate requests on page refresh.
Key Changes:
- Desktop users now open team dashboard in external browser instead of navigating within the app
- JWT tokens are re-issued on every authentication request instead of reusing existing valid tokens
- Page refresh protection added via localStorage to prevent duplicate token generation
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
libs/shared/ui-core/src/app/HeaderNavbar.tsx |
Opens team dashboard link in external browser for desktop users |
libs/desktop-types/src/lib/desktop-app.types.ts |
Expands user profile schema to include additional fields (userId, emailVerified, picture, billingAccount, entitlements, teamMembership) |
libs/auth/acl/src/lib/acl.ts |
Adds Team read permission for desktop billing/admin users |
apps/landing/hooks/desktop-auth.hooks.ts |
Implements localStorage-based check to prevent re-authentication on page refresh |
apps/jetstream-desktop/src/services/persistence.service.ts |
Fixes swapped email/name mapping and adds picture and teamMembership to user profile |
apps/jetstream-desktop/src/browser/browser.ts |
Changes hostname check to host check for better port handling in external link detection |
apps/api/src/app/controllers/desktop-app.controller.ts |
Removes token reuse logic and always issues new JWT tokens |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
031bdcb to
282024f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
prisma/migrations/20251228000000_add_token_hash_to_web_extension_token/migration.sql
Show resolved
Hide resolved
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
282024f to
175f179
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
apps/api/src/app/routes/openapi.routes.ts:581
- The path should be '/desktop-app/auth/logout' not '/desktop-app/logout' to match the actual route definition. The actual route is defined as '/auth/logout' under the '/desktop-app' prefix.
'/desktop-app/logout': {
delete: { ...getRequest({ ...desktopController.logout.validators, tags: ['desktop'] }) },
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prisma/migrations/20251228000000_add_token_hash_to_web_extension_token/migration.sql
Show resolved
Hide resolved
175f179 to
caa0f94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prisma/migrations/20251228000000_add_token_hash_to_web_extension_token/migration.sql
Show resolved
Hide resolved
6cf0367 to
e18b7f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 21 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
Allow access to the team dashboard from the desktop app If allowed, a link to the web-app is shown in the menu
e18b7f7 to
c25beaa
Compare
c25beaa to
a0c3ec0
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
a0c3ec0 to
d18eb0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
apps/jetstream-e2e/src/tests/authentication/external-auth-logged-in.spec.ts
Outdated
Show resolved
Hide resolved
prisma/migrations/20251228000000_add_token_hash_to_web_extension_token/migration.sql
Show resolved
Hide resolved
JWT tokens are used for desktop/web-extension to ensure access These tokens provide access to history data, but nothing else, they were stored unencrypted in the DB The code supports both encrypted and unencrypted tokens, so existing tokens will continue to work, but new tokens will be encrypted before being stored in the DB Tokens are encrypted as they are used if in plaintext form Fixed openapi specifications
Ensure all external clients (desktop/web extension) utilize authorization header instead of the body Update external auth to pull from body as a fallback for backwards compatibility Fix logout response handling
d18eb0a to
f1e355c
Compare
This pull request implements several security and maintainability improvements to the authentication flow for both the desktop app and web extension, focusing on token management, validation, and logging. The changes include migrating stored tokens to an encrypted format, reworking token reuse logic, improving error handling, and modernizing the way device and token information is passed in API requests. Additionally, logging is standardized across controllers for better observability.
Authentication and Token Management Improvements
web-extension.db.ts,jwt-token-encryption.serviceusage) [1] [2]desktop-app.controller.ts,web-extension.controller.ts) [1] [2]routeDefinitionin both controllers) [1] [2] [3] [4]API Response and Logging Enhancements
desktop-app.controller.ts,web-extension.controller.ts) [1] [2]console.errorwith the sharedloggerutility in asset download endpoints. (desktop-assets.controller.ts) [1] [2] [3]Code Quality and Maintainability
desktop-app.controller.ts,web-extension.controller.ts) [1] [2] [3] [4]desktop-app.controller.ts,web-extension.controller.ts) [1] [2]These changes significantly improve the security, reliability, and maintainability of authentication flows for client applications.