-
Notifications
You must be signed in to change notification settings - Fork 4
[NAE-2241] Anonymous access refactor #316
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: release/7.0.0-rev10
Are you sure you want to change the base?
Changes from 5 commits
9cdebe0
41cc8fb
8babfe3
f0a63a8
ffb3b5e
44be3b6
3949d93
0305882
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 |
|---|---|---|
| @@ -1,28 +1,28 @@ | ||
| import {TestBed} from '@angular/core/testing'; | ||
| import {NoopAnimationsModule} from '@angular/platform-browser/animations'; | ||
| import {HttpClientTestingModule} from '@angular/common/http/testing'; | ||
| import {RouterTestingModule} from '@angular/router/testing'; | ||
| import {AnonymousService} from './anonymous.service'; | ||
| import {ConfigurationService} from '../../configuration/configuration.service'; | ||
| import {TestConfigurationService} from '../../utility/tests/test-config'; | ||
|
|
||
| describe('AnonymousService', () => { | ||
| let service: AnonymousService; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| imports: [NoopAnimationsModule, HttpClientTestingModule, RouterTestingModule.withRoutes([])], | ||
| providers: [{provide: ConfigurationService, useClass: TestConfigurationService}] | ||
| }); | ||
| service = TestBed.inject(AnonymousService); | ||
| }); | ||
|
|
||
| it('should be created', () => { | ||
| expect(service).toBeTruthy(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| TestBed.resetTestingModule(); | ||
| }); | ||
| }); | ||
|
|
||
| // import {TestBed} from '@angular/core/testing'; | ||
| // import {NoopAnimationsModule} from '@angular/platform-browser/animations'; | ||
| // import {HttpClientTestingModule} from '@angular/common/http/testing'; | ||
| // import {RouterTestingModule} from '@angular/router/testing'; | ||
| // import {AnonymousService} from './anonymous.service'; | ||
| // import {ConfigurationService} from '../../configuration/configuration.service'; | ||
| // import {TestConfigurationService} from '../../utility/tests/test-config'; | ||
| // | ||
| // describe('AnonymousService', () => { | ||
| // let service: AnonymousService; | ||
| // | ||
| // beforeEach(() => { | ||
| // TestBed.configureTestingModule({ | ||
| // imports: [NoopAnimationsModule, HttpClientTestingModule, RouterTestingModule.withRoutes([])], | ||
| // providers: [{provide: ConfigurationService, useClass: TestConfigurationService}] | ||
| // }); | ||
| // service = TestBed.inject(AnonymousService); | ||
| // }); | ||
| // | ||
| // it('should be created', () => { | ||
| // expect(service).toBeTruthy(); | ||
| // }); | ||
| // | ||
| // afterEach(() => { | ||
| // TestBed.resetTestingModule(); | ||
| // }); | ||
| // }); | ||
| // | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,43 +9,43 @@ import {BehaviorSubject, Observable} from 'rxjs'; | |
| }) | ||
| export class AnonymousService implements OnDestroy { | ||
|
|
||
| public static readonly JWT_BEARER_HEADER_DEFAULT = 'X-Jwt-Token'; | ||
| protected readonly _jwtHeader: string; | ||
| public static readonly X_ANONYMOUS_TOKEN = 'X-Anonymous-Token'; | ||
| protected readonly _anonymousTokenHeader: string; | ||
| protected _storage: Storage; | ||
| protected _tokenSet: BehaviorSubject<boolean>; | ||
|
|
||
| constructor(protected _config: ConfigurationService) { | ||
| this._jwtHeader = this._config.get().providers.auth.jwtBearer ? | ||
| this._config.get().providers.auth.jwtBearer : AnonymousService.JWT_BEARER_HEADER_DEFAULT; | ||
| this._anonymousTokenHeader = this._config.get().providers.auth.anonymous ? | ||
| this._config.get().providers.auth.anonymous : AnonymousService.X_ANONYMOUS_TOKEN; | ||
| this._storage = this.resolveStorage(this._config.get().providers.auth['local']); | ||
| this._tokenSet = new BehaviorSubject<boolean>(false); | ||
| } | ||
|
|
||
| get jwtHeader(): string { | ||
| return this._jwtHeader; | ||
| get anonymousTokenHeader(): string { | ||
| return this._anonymousTokenHeader; | ||
| } | ||
|
|
||
| get tokenSet(): Observable<boolean> { | ||
| return this._tokenSet.asObservable(); | ||
| } | ||
|
|
||
| public getToken(): string { | ||
| return this._storage.getItem(this._jwtHeader); | ||
| return this._storage.getItem(this._anonymousTokenHeader); | ||
| } | ||
|
|
||
| public setToken(token: string): void { | ||
| this._storage.setItem(this._jwtHeader, token); | ||
| this._storage.setItem(this._anonymousTokenHeader, token); | ||
| if (!this._tokenSet.getValue()) | ||
| this._tokenSet.next(true); | ||
| } | ||
|
|
||
| public removeToken(): void { | ||
| this._storage.removeItem(this._jwtHeader); | ||
| this._storage.removeItem(this._anonymousTokenHeader); | ||
| this._tokenSet.next(false); | ||
| } | ||
|
|
||
| ngOnDestroy(): void { | ||
| localStorage.removeItem(this._jwtHeader); | ||
| localStorage.removeItem(this._anonymousTokenHeader); | ||
| this._tokenSet.complete(); | ||
|
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. Use resolved storage in
🛠️ Proposed fix- localStorage.removeItem(this._anonymousTokenHeader);
+ this._storage.removeItem(this._anonymousTokenHeader);🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -10,7 +10,7 @@ export * from './authentication.module'; | |||
|
|
||||
| /* SERVICES */ | ||||
| export * from './anonymous/anonymous.service'; | ||||
| export * from './services/anonymous-authentication-interceptor' | ||||
| // export * from './services/anonymous-authentication-interceptor' | ||||
|
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. 🧹 Nitpick | 🔵 Trivial Remove the commented export from the public API surface. Keeping commented exports in a public API file creates dead code and ambiguity. If this export is intentionally retired, delete it outright. 🧹 Suggested cleanup-// export * from './services/anonymous-authentication-interceptor'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| export * from './services/authentication-interceptor' | ||||
| export * from './proxyAuthentication.service' | ||||
|
|
||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,58 +1,58 @@ | ||
| import {inject, TestBed} from '@angular/core/testing'; | ||
| import {ConfigurationService} from '../../configuration/configuration.service'; | ||
| import {TestConfigurationService} from '../../utility/tests/test-config'; | ||
| import {HTTP_INTERCEPTORS, HttpClient, HttpHeaders} from '@angular/common/http'; | ||
| import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; | ||
| import {RouterTestingModule} from '@angular/router/testing'; | ||
| import {NoopAnimationsModule} from '@angular/platform-browser/animations'; | ||
| import {LoggerService} from '../../logger/services/logger.service'; | ||
| import {AnonymousService} from '../anonymous/anonymous.service'; | ||
| import {AnonymousAuthenticationInterceptor} from './anonymous-authentication-interceptor'; | ||
|
|
||
| describe('AnonymousAuthenticationInterceptor', () => { | ||
| let service: AnonymousService; | ||
| let warnSpy: jasmine.Spy; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| imports: [HttpClientTestingModule, NoopAnimationsModule, RouterTestingModule.withRoutes([])], | ||
| providers: [ | ||
| {provide: ConfigurationService, useClass: TestConfigurationService}, | ||
| AnonymousService, | ||
| { | ||
| provide: HTTP_INTERCEPTORS, | ||
| useClass: AnonymousAuthenticationInterceptor, | ||
| multi: true | ||
| } | ||
| ] | ||
| }); | ||
| service = TestBed.inject(AnonymousService); | ||
| warnSpy = spyOn(TestBed.inject(LoggerService), 'warn'); | ||
| }); | ||
|
|
||
| describe('intercept HTTP request', () => { | ||
| it('should add JWT bearer to Headers', (done) => { | ||
| inject([HttpClient, HttpTestingController], | ||
| (http: HttpClient, mock: HttpTestingController) => { | ||
|
|
||
| service.setToken('jwt-token'); | ||
| http.get('/api').subscribe(response => { | ||
| expect(response).toBeTruthy(); | ||
| done(); | ||
| }); | ||
| const request = mock.expectOne(req => (req.headers.has('X-Jwt-Token'))); | ||
|
|
||
| request.flush({data: 'test'}, {headers: new HttpHeaders({'X-Jwt-Token': 'tokenos'})}); | ||
| mock.verify(); | ||
| })(); | ||
| }); | ||
| afterEach(inject([HttpTestingController], (mock: HttpTestingController) => { | ||
| mock.verify(); | ||
| TestBed.resetTestingModule(); | ||
| })); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| TestBed.resetTestingModule(); | ||
| }); | ||
| }); | ||
| // import {inject, TestBed} from '@angular/core/testing'; | ||
| // import {ConfigurationService} from '../../configuration/configuration.service'; | ||
| // import {TestConfigurationService} from '../../utility/tests/test-config'; | ||
| // import {HTTP_INTERCEPTORS, HttpClient, HttpHeaders} from '@angular/common/http'; | ||
| // import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; | ||
| // import {RouterTestingModule} from '@angular/router/testing'; | ||
| // import {NoopAnimationsModule} from '@angular/platform-browser/animations'; | ||
| // import {LoggerService} from '../../logger/services/logger.service'; | ||
| // import {AnonymousService} from '../anonymous/anonymous.service'; | ||
| // import {AnonymousAuthenticationInterceptor} from './anonymous-authentication-interceptor'; | ||
| // | ||
| // describe('AnonymousAuthenticationInterceptor', () => { | ||
| // let service: AnonymousService; | ||
| // let warnSpy: jasmine.Spy; | ||
| // | ||
| // beforeEach(() => { | ||
| // TestBed.configureTestingModule({ | ||
| // imports: [HttpClientTestingModule, NoopAnimationsModule, RouterTestingModule.withRoutes([])], | ||
| // providers: [ | ||
| // {provide: ConfigurationService, useClass: TestConfigurationService}, | ||
| // AnonymousService, | ||
| // { | ||
| // provide: HTTP_INTERCEPTORS, | ||
| // useClass: AnonymousAuthenticationInterceptor, | ||
| // multi: true | ||
| // } | ||
| // ] | ||
| // }); | ||
| // service = TestBed.inject(AnonymousService); | ||
| // warnSpy = spyOn(TestBed.inject(LoggerService), 'warn'); | ||
| // }); | ||
| // | ||
| // describe('intercept HTTP request', () => { | ||
| // it('should add JWT bearer to Headers', (done) => { | ||
| // inject([HttpClient, HttpTestingController], | ||
| // (http: HttpClient, mock: HttpTestingController) => { | ||
| // | ||
| // service.setToken('jwt-token'); | ||
| // http.get('/api').subscribe(response => { | ||
| // expect(response).toBeTruthy(); | ||
| // done(); | ||
| // }); | ||
| // const request = mock.expectOne(req => (req.headers.has('X-Jwt-Token'))); | ||
| // | ||
| // request.flush({data: 'test'}, {headers: new HttpHeaders({'X-Jwt-Token': 'tokenos'})}); | ||
| // mock.verify(); | ||
| // })(); | ||
| // }); | ||
| // afterEach(inject([HttpTestingController], (mock: HttpTestingController) => { | ||
| // mock.verify(); | ||
| // TestBed.resetTestingModule(); | ||
| // })); | ||
| // }); | ||
| // | ||
| // afterEach(() => { | ||
| // TestBed.resetTestingModule(); | ||
| // }); | ||
| // }); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,41 @@ | ||
| import {Injectable} from '@angular/core'; | ||
| import {HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse} from '@angular/common/http'; | ||
| import {Observable, throwError} from 'rxjs'; | ||
| import {catchError, tap} from 'rxjs/operators'; | ||
| import {AnonymousService} from '../anonymous/anonymous.service'; | ||
|
|
||
| @Injectable() | ||
| export class AnonymousAuthenticationInterceptor implements HttpInterceptor { | ||
|
|
||
| constructor(protected _anonymousService: AnonymousService) {} | ||
|
|
||
| intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> { | ||
| const jwtAuthToken = this._anonymousService.getToken(); | ||
|
|
||
| if (!this._anonymousService) { | ||
| next.handle(req); | ||
| } | ||
|
|
||
| if (!!jwtAuthToken) { | ||
| req = req.clone({ | ||
| headers: req.headers.set(this._anonymousService.jwtHeader, jwtAuthToken) | ||
| }); | ||
| } | ||
| return next.handle(req).pipe( | ||
| tap(event => { | ||
| if (event instanceof HttpResponse) { | ||
| if (event.headers.has(this._anonymousService.jwtHeader)) { | ||
| this._anonymousService.setToken(event.headers.get(this._anonymousService.jwtHeader)); | ||
| } | ||
| } | ||
| }), | ||
| catchError(errorEvent => { | ||
| if (errorEvent instanceof HttpErrorResponse && errorEvent.status === 401) { | ||
| console.debug('Authentication token is invalid. Clearing session token'); | ||
| this._anonymousService.removeToken(); | ||
| } | ||
| return throwError(errorEvent); | ||
| }) | ||
| ); | ||
| } | ||
| } | ||
| // import {Injectable} from '@angular/core'; | ||
| // import {HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse} from '@angular/common/http'; | ||
| // import {Observable, throwError} from 'rxjs'; | ||
| // import {catchError, tap} from 'rxjs/operators'; | ||
| // import {AnonymousService} from '../anonymous/anonymous.service'; | ||
| // | ||
| // @Injectable() | ||
| // export class AnonymousAuthenticationInterceptor implements HttpInterceptor { | ||
| // | ||
| // constructor(protected _anonymousService: AnonymousService) {} | ||
| // | ||
| // intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> { | ||
| // const xAnonymousToken = this._anonymousService.getToken(); | ||
| // | ||
| // if (!this._anonymousService) { | ||
| // next.handle(req); | ||
| // } | ||
| // | ||
| // if (!!xAnonymousToken) { | ||
| // req = req.clone({ | ||
| // headers: req.headers.set(this._anonymousService.anonymousTokenHeader, xAnonymousToken) | ||
| // }); | ||
| // } | ||
| // return next.handle(req).pipe( | ||
| // tap(event => { | ||
| // if (event instanceof HttpResponse) { | ||
| // if (event.headers.has(this._anonymousService.anonymousTokenHeader)) { | ||
| // this._anonymousService.setToken(event.headers.get(this._anonymousService.anonymousTokenHeader)); | ||
| // } | ||
| // } | ||
| // }), | ||
| // catchError(errorEvent => { | ||
| // if (errorEvent instanceof HttpErrorResponse && errorEvent.status === 401) { | ||
| // console.debug('Authentication token is invalid. Clearing session token'); | ||
| // this._anonymousService.removeToken(); | ||
| // } | ||
| // return throwError(errorEvent); | ||
| // }) | ||
| // ); | ||
| // } | ||
| // } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -11,15 +11,15 @@ import {Observable, throwError} from 'rxjs'; | |||
| import {catchError, tap} from 'rxjs/operators'; | ||||
| import {SessionService} from '../session/services/session.service'; | ||||
| import {RedirectService} from '../../routing/redirect-service/redirect.service'; | ||||
| import {AnonymousService} from '../anonymous/anonymous.service'; | ||||
| // import {AnonymousService} from '../anonymous/anonymous.service'; | ||||
|
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. 🛠️ Refactor suggestion | 🟠 Major Remove commented-out import. Delete the commented-out import rather than leaving dead code. If the ♻️ Proposed fix-// import {AnonymousService} from '../anonymous/anonymous.service';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| import {SessionIdleTimerService} from "../session/services/session-idle-timer.service"; | ||||
|
|
||||
| @Injectable() | ||||
| export class AuthenticationInterceptor implements HttpInterceptor { | ||||
|
|
||||
| constructor(private _session: SessionService, | ||||
| private _redirect: RedirectService, | ||||
| private _anonymousService: AnonymousService, | ||||
| // private _anonymousService: AnonymousService, | ||||
| private idleTimerService: SessionIdleTimerService) { | ||||
| } | ||||
|
Comment on lines
20
to
24
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. 🛠️ Refactor suggestion | 🟠 Major Remove commented-out constructor parameter. The commented-out ♻️ Proposed fix constructor(private _session: SessionService,
private _redirect: RedirectService,
- // private _anonymousService: AnonymousService,
private idleTimerService: SessionIdleTimerService) {
}🤖 Prompt for AI Agents |
||||
|
|
||||
|
|
@@ -37,7 +37,7 @@ export class AuthenticationInterceptor implements HttpInterceptor { | |||
| return next.handle(req).pipe( | ||||
| tap(event => { | ||||
| if (event instanceof HttpResponse) { | ||||
| if (event.headers.has(this._session.sessionHeader) && !event.headers.has(this._anonymousService.jwtHeader)) { | ||||
| if (event.headers.has(this._session.sessionHeader)) { | ||||
| this._session.setVerifiedToken(event.headers.get(this._session.sessionHeader)); | ||||
| } | ||||
| } | ||||
|
|
||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: netgrif/components
Length of output: 4437
Restore active tests for
AnonymousService.The
AnonymousServiceis still active and fully implemented, but its test file is entirely commented out, leaving the service without test coverage. Restore the test suite to maintain coverage for the active service.🤖 Prompt for AI Agents