-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Angular - Improve Authentication Token Handling #24050
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
base: dev
Are you sure you want to change the base?
Changes from all commits
13fd817
43218a8
69f21ef
026336e
125537f
e087db8
5f026aa
e4e8da3
e1a68fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { DOCUMENT, inject, Injectable } from '@angular/core'; | ||
| import { OAuthStorage } from 'angular-oauth2-oidc'; | ||
| import { AbpLocalStorageService } from '@abp/ng.core'; | ||
|
|
||
| @Injectable({ | ||
| providedIn: 'root', | ||
| }) | ||
| export class MemoryTokenStorageService implements OAuthStorage { | ||
| private keysShouldStoreInMemory = [ | ||
| 'access_token', 'id_token', 'expires_at', | ||
| 'id_token_claims_obj', 'id_token_expires_at', | ||
| 'id_token_stored_at', 'access_token_stored_at', | ||
| 'abpOAuthClientId', 'granted_scopes' | ||
| ]; | ||
|
|
||
| private worker?: any; | ||
| private port?: MessagePort; | ||
| private cache = new Map<string, string>(); | ||
| private localStorageService = inject(AbpLocalStorageService); | ||
| private _document = inject(DOCUMENT); | ||
| private useSharedWorker = false; | ||
|
|
||
| constructor() { | ||
| this.initializeStorage(); | ||
| } | ||
|
|
||
| private initializeStorage(): void { | ||
| // @ts-ignore | ||
| if (typeof SharedWorker !== 'undefined') { | ||
| try { | ||
| // @ts-ignore | ||
| this.worker = new SharedWorker( | ||
|
Comment on lines
+28
to
+32
|
||
| new URL( | ||
| '../workers/token-storage.worker.js', | ||
| import.meta.url | ||
| ), | ||
| { name: 'oauth-token-storage', type: "module" } | ||
| ); | ||
|
|
||
| this.port = this.worker.port; | ||
| this.port.start(); | ||
| this.useSharedWorker = true; | ||
|
|
||
| this.port.onmessage = (event) => { | ||
| const { action, key, value } = event.data; | ||
|
|
||
| switch (action) { | ||
| case 'set': | ||
| this.checkAuthStateChanges(key); | ||
| this.cache.set(key, value); | ||
| break; | ||
| case 'remove': | ||
| this.cache.delete(key); | ||
| this.refreshDocument(); | ||
| break; | ||
| case 'clear': | ||
| this.cache.clear(); | ||
| this.refreshDocument(); | ||
| break; | ||
| } | ||
| }; | ||
| } catch (error) { | ||
| this.useSharedWorker = false; | ||
| } | ||
| } else { | ||
| this.useSharedWorker = false; | ||
| } | ||
| } | ||
|
|
||
| getItem(key: string): string | null { | ||
| if (!this.keysShouldStoreInMemory.includes(key)) { | ||
| return this.localStorageService.getItem(key); | ||
| } | ||
| return this.cache.get(key) || null; | ||
| } | ||
|
|
||
| setItem(key: string, value: string): void { | ||
| if (!this.keysShouldStoreInMemory.includes(key)) { | ||
| this.localStorageService.setItem(key, value); | ||
| return; | ||
| } | ||
|
|
||
| if (this.useSharedWorker && this.port) { | ||
| this.cache.set(key, value); | ||
| this.port.postMessage({ action: 'set', key, value }); | ||
| } else { | ||
| this.cache.set(key, value); | ||
| } | ||
| } | ||
|
|
||
| removeItem(key: string): void { | ||
| if (!this.keysShouldStoreInMemory.includes(key)) { | ||
| this.localStorageService.removeItem(key); | ||
| return; | ||
| } | ||
|
|
||
| if (this.useSharedWorker && this.port) { | ||
| this.cache.delete(key); | ||
| this.port.postMessage({ action: 'remove', key }); | ||
| } else { | ||
| this.cache.delete(key); | ||
| } | ||
| } | ||
|
|
||
| clear(): void { | ||
| if (this.useSharedWorker && this.port) { | ||
| this.port.postMessage({ action: 'clear' }); | ||
| } | ||
| this.cache.clear(); | ||
| } | ||
|
|
||
| private checkAuthStateChanges = (key: string) => { | ||
| if (key === 'access_token' && !this.cache.get('access_token')) { | ||
| this.refreshDocument(); | ||
| } | ||
| } | ||
|
|
||
| private refreshDocument(): void { | ||
| this._document.defaultView?.location.reload(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| // ESM SharedWorker | ||
|
|
||
| const tokenStore = new Map(); | ||
| const ports = new Set(); | ||
|
|
||
| self.onconnect = (event) => { | ||
| const port = event.ports[0]; | ||
| ports.add(port); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like there’s a potential memory leak issue here. Steps to reproduce:
At this point, you’ll notice that the ports array contains 5 elements instead of 3.
To fix this issue, we probably need to introduce a new |
||
| port.onmessage = (e) => { | ||
| const { action, key, value } = e.data; | ||
| switch (action) { | ||
| case 'set': | ||
| if (key && value !== undefined) { | ||
| tokenStore.set(key, value); | ||
| ports.forEach(p => { | ||
| if (p !== port) { | ||
| p.postMessage({ action: 'set', key, value }); | ||
| } | ||
| }); | ||
| } | ||
| break; | ||
| case 'remove': | ||
| if (key) { | ||
| tokenStore.delete(key); | ||
| ports.forEach(p => { | ||
| if (p !== port) { | ||
| p.postMessage({ action: 'remove', key }); | ||
| } | ||
| }); | ||
| } | ||
| break; | ||
| case 'clear': | ||
| tokenStore.clear(); | ||
| ports.forEach(p => { | ||
| if (p !== port) { | ||
| p.postMessage({ action: 'clear' }); | ||
| } | ||
| }); | ||
| break; | ||
| case 'get': | ||
| if (key) { | ||
| const value = tokenStore.get(key) ?? null; | ||
| port.postMessage({ action: 'get', key, value }); | ||
| } | ||
| break; | ||
| } | ||
| }; | ||
|
|
||
| port.start(); | ||
| }; | ||
|
|
||
| export {}; | ||

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.
private worker?: SharedWorker;It would be better to add a proper type for worker instead of leaving it as an optional SharedWorker. This improves clarity and type safety.