Angular - Improve Authentication Token Handling #24050
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances authentication token handling in Angular by implementing a SharedWorker-based token storage system. The changes improve cross-tab token synchronization and move sensitive tokens from localStorage to in-memory storage for better security.
- Introduces a SharedWorker for cross-tab token synchronization
- Implements MemoryTokenStorageService to store sensitive tokens in memory instead of localStorage
- Adds error handling for token refresh operations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| token-storage.worker.ts | New SharedWorker implementation for cross-tab token storage synchronization |
| memory-token-storage.service.ts | New service that stores sensitive OAuth tokens in memory with SharedWorker support |
| storage.factory.ts | Updated to use MemoryTokenStorageService instead of AbpLocalStorageService |
| oauth.service.ts | Added try-catch error handling around token refresh operation |
| index.ts | Added export for the new MemoryTokenStorageService |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| return this.oAuthService.refreshToken(); | ||
| } catch (error) { | ||
| return Promise.resolve(); |
There was a problem hiding this comment.
The catch block silently swallows errors without logging or providing any feedback. Consider logging the error or returning a rejected promise with an appropriate error message to help with debugging authentication issues.
| return Promise.resolve(); | |
| console.error('Error during token refresh:', error); | |
| return Promise.reject(error); |
| } | ||
|
|
||
| if (this.useSharedWorker && this.port) { | ||
| // this.cache.delete(key); |
There was a problem hiding this comment.
Remove commented-out code. If this line is needed, it should be uncommented and executed; otherwise, it should be deleted entirely.
| // this.cache.delete(key); |
| // @ts-ignore | ||
| if (typeof SharedWorker !== 'undefined') { | ||
| try { | ||
| // @ts-ignore | ||
| this.worker = new SharedWorker( |
There was a problem hiding this comment.
Replace @ts-ignore comments with proper TypeScript type declarations or type guards. The repeated use of @ts-ignore suppresses type checking and can hide potential runtime errors.
| } | ||
| }; | ||
|
|
||
| console.log('✅ Using SharedWorker for cross-tab token storage'); |
There was a problem hiding this comment.
Remove or replace console.log statements in production code. Consider using a proper logging service or removing this debug statement entirely.
| console.log('✅ Using SharedWorker for cross-tab token storage'); |
|
|
||
| console.log('✅ Using SharedWorker for cross-tab token storage'); | ||
| } catch (error) { | ||
| console.warn('⚠️ SharedWorker failed:', error); |
There was a problem hiding this comment.
Consider using a proper logging service instead of console.warn for error reporting in production code. This would allow better error tracking and debugging capabilities.
| this.useSharedWorker = false; | ||
| } | ||
| } else { | ||
| console.warn('⚠️ SharedWorker not supported'); |
There was a problem hiding this comment.
Consider using a proper logging service instead of console.warn. This would provide consistent logging throughout the application.
Description
Resolves #23930
Improvements