Skip to content

Commit

Permalink
NAS-131829 / 25.04 / Adds error log for middleware and efficient calls (
Browse files Browse the repository at this point in the history
  • Loading branch information
RehanY147 authored Jan 5, 2025
1 parent 58f308a commit 464a06e
Show file tree
Hide file tree
Showing 35 changed files with 302 additions and 124 deletions.
6 changes: 3 additions & 3 deletions src/app/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import { Router, NavigationEnd, RouterOutlet } from '@angular/router';
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
import { filter, tap } from 'rxjs';
import { WINDOW } from 'app/helpers/window.helper';
import { AuthService } from 'app/modules/auth/auth.service';
import { LayoutService } from 'app/modules/layout/layout.service';
import { PingService } from 'app/modules/websocket/ping.service';
import { DetectBrowserService } from 'app/services/detect-browser.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

@UntilDestroy()
@Component({
Expand All @@ -24,13 +24,13 @@ export class AppComponent implements OnInit {
constructor(
public title: Title,
private router: Router,
private authService: AuthService,
private wsStatus: WebSocketStatusService,
private detectBrowser: DetectBrowserService,
private layoutService: LayoutService,
private pingService: PingService,
@Inject(WINDOW) private window: Window,
) {
this.authService.isAuthenticated$.pipe(untilDestroyed(this)).subscribe((isAuthenticated) => {
this.wsStatus.isAuthenticated$.pipe(untilDestroyed(this)).subscribe((isAuthenticated) => {
this.isAuthenticated = isAuthenticated;
});
this.title.setTitle('TrueNAS - ' + this.window.location.hostname);
Expand Down
4 changes: 3 additions & 1 deletion src/app/core/testing/classes/mock-api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { Job } from 'app/interfaces/job.interface';
import { ApiService } from 'app/modules/websocket/api.service';
import { SubscriptionManagerService } from 'app/modules/websocket/subscription-manager.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

/**
* Better than just expect.anything() because it allows null and undefined.
Expand All @@ -47,10 +48,11 @@ export class MockApiService extends ApiService {

constructor(
wsHandler: WebSocketHandlerService,
wsStatus: WebSocketStatusService,
subscriptionManager: SubscriptionManagerService,
translate: TranslateService,
) {
super(wsHandler, subscriptionManager, translate);
super(wsHandler, wsStatus, subscriptionManager, translate);

this.call = jest.fn();
this.job = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { SystemInfo } from 'app/interfaces/system-info.interface';
import { ApiService } from 'app/modules/websocket/api.service';
import { SubscriptionManagerService } from 'app/modules/websocket/subscription-manager.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

@Injectable({
providedIn: 'root',
Expand All @@ -20,10 +21,11 @@ export class MockEnclosureApiService extends ApiService {

constructor(
wsManager: WebSocketHandlerService,
wsStatus: WebSocketStatusService,
subscriptionManager: SubscriptionManagerService,
translate: TranslateService,
) {
super(wsManager, subscriptionManager, translate);
super(wsManager, wsStatus, subscriptionManager, translate);

console.warn('MockEnclosureApiService is in effect. Some calls will be mocked');
}
Expand Down
1 change: 0 additions & 1 deletion src/app/core/testing/utils/empty-auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { AuthService } from 'app/modules/auth/auth.service';

export class EmptyAuthService {
readonly authToken$ = getMissingInjectionErrorObservable(AuthService.name);
readonly isAuthenticated$ = getMissingInjectionErrorObservable(AuthService.name);
readonly user$ = getMissingInjectionErrorObservable(AuthService.name);
readonly isSysAdmin$ = getMissingInjectionErrorObservable(AuthService.name);
readonly userTwoFactorConfig$ = getMissingInjectionErrorObservable(AuthService.name);
Expand Down
10 changes: 8 additions & 2 deletions src/app/core/testing/utils/mock-api.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Job } from 'app/interfaces/job.interface';
import { ApiService } from 'app/modules/websocket/api.service';
import { SubscriptionManagerService } from 'app/modules/websocket/subscription-manager.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

/**
* This is a sugar syntax for creating simple api mocks.
Expand Down Expand Up @@ -49,11 +50,12 @@ export function mockApi(
{
provide: ApiService,
useFactory: (
wsStatus: WebSocketStatusService,
wsHandler: WebSocketHandlerService,
translate: TranslateService,
) => {
const subscriptionManager = {} as SubscriptionManagerService;
const mockApiService = new MockApiService(wsHandler, subscriptionManager, translate);
const mockApiService = new MockApiService(wsHandler, wsStatus, subscriptionManager, translate);
(mockResponses || []).forEach((mockResponse) => {
if (mockResponse.type === MockApiResponseType.Call) {
mockApiService.mockCall(mockResponse.method, mockResponse.response);
Expand All @@ -66,7 +68,11 @@ export function mockApi(
});
return mockApiService;
},
deps: [WebSocketHandlerService, TranslateService],
deps: [WebSocketStatusService, WebSocketHandlerService, TranslateService],
},
{
provide: WebSocketStatusService,
useValue: ({} as WebSocketStatusService),
},
{
provide: MockApiService,
Expand Down
9 changes: 5 additions & 4 deletions src/app/core/testing/utils/mock-auth.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { Role } from 'app/enums/role.enum';
import { LoggedInUser } from 'app/interfaces/ds-cache.interface';
import { AuthService } from 'app/modules/auth/auth.service';
import { ApiService } from 'app/modules/websocket/api.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { TokenLastUsedService } from 'app/services/token-last-used.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

export const dummyUser = {
privilege: {
Expand Down Expand Up @@ -40,13 +40,14 @@ export function mockAuth(
provide: AuthService,
useFactory: () => {
const mockService = new MockAuthService(
createSpyObject(WebSocketHandlerService, {
isConnected$: of(true),
}),
createSpyObject(Store),
createSpyObject(ApiService),
createSpyObject(TokenLastUsedService),
createSpyObject(Window),
createSpyObject(WebSocketStatusService, {
isConnected$: of(true),
isAuthenticated$: of(false),
}),
);

mockService.setUser(user as LoggedInUser);
Expand Down
4 changes: 2 additions & 2 deletions src/app/modules/auth/auth-guard.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '@ngneat/spectator/jest';
import { BehaviorSubject } from 'rxjs';
import { AuthGuardService } from 'app/modules/auth/auth-guard.service';
import { AuthService } from 'app/modules/auth/auth.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

describe('AuthGuardService', () => {
const redirectUrl = 'storage/disks';
Expand All @@ -17,7 +17,7 @@ describe('AuthGuardService', () => {
let spectator: SpectatorService<AuthGuardService>;
const createService = createServiceFactory({
service: AuthGuardService,
providers: [mockProvider(AuthService, { isAuthenticated$ })],
providers: [mockProvider(WebSocketStatusService, { isAuthenticated$ })],
});

beforeEach(() => {
Expand Down
6 changes: 3 additions & 3 deletions src/app/modules/auth/auth-guard.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Inject, Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
import { WINDOW } from 'app/helpers/window.helper';
import { AuthService } from 'app/modules/auth/auth.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

@UntilDestroy()
@Injectable({
Expand All @@ -12,10 +12,10 @@ export class AuthGuardService {
isAuthenticated = false;
constructor(
private router: Router,
private authService: AuthService,
private wsStatus: WebSocketStatusService,
@Inject(WINDOW) private window: Window,
) {
this.authService.isAuthenticated$.pipe(untilDestroyed(this)).subscribe((isLoggedIn) => {
this.wsStatus.isAuthenticated$.pipe(untilDestroyed(this)).subscribe((isLoggedIn) => {
this.isAuthenticated = isLoggedIn;
});
}
Expand Down
35 changes: 16 additions & 19 deletions src/app/modules/auth/auth.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import {
StorageStrategyStub, withLocalStorage,
} from 'ngx-webstorage';
import * as rxjs from 'rxjs';
import { firstValueFrom, of } from 'rxjs';
import {
BehaviorSubject, firstValueFrom,
} from 'rxjs';
import { TestScheduler } from 'rxjs/testing';
import { MockApiService } from 'app/core/testing/classes/mock-api.service';
import { mockCall, mockApi } from 'app/core/testing/utils/mock-api.utils';
Expand All @@ -21,7 +23,7 @@ import { LoggedInUser } from 'app/interfaces/ds-cache.interface';
import { Preferences } from 'app/interfaces/preferences.interface';
import { AuthService } from 'app/modules/auth/auth.service';
import { ApiService } from 'app/modules/websocket/api.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';

const authMeUser = {
pw_dir: 'dir',
Expand All @@ -40,9 +42,12 @@ const authMeUser = {
},
} as LoggedInUser;

const mockWsStatus = new WebSocketStatusService();

describe('AuthService', () => {
let spectator: SpectatorService<AuthService>;
let testScheduler: TestScheduler;
let timer$: BehaviorSubject<0>;
const createService = createServiceFactory({
service: AuthService,
providers: [
Expand All @@ -59,9 +64,10 @@ describe('AuthService', () => {
},
} as LoginExResponse),
]),
mockProvider(WebSocketHandlerService, {
isConnected$: of(true),
}),
{
provide: WebSocketStatusService,
useValue: mockWsStatus,
},
{
provide: STORAGE_STRATEGIES,
useFactory: () => new StorageStrategyStub(LocalStorageStrategy.strategyName),
Expand All @@ -78,11 +84,14 @@ describe('AuthService', () => {
testScheduler = new TestScheduler((actual, expected) => {
expect(actual).toEqual(expected);
});
mockWsStatus.setConnectionStatus(true);
timer$ = new BehaviorSubject(0);
jest.spyOn(rxjs, 'timer').mockReturnValue(timer$.asObservable());
});

describe('Login', () => {
it('initializes auth session with triggers and token with username/password login', () => {
jest.spyOn(rxjs, 'timer').mockReturnValueOnce(of(0));
timer$.next(0);

const obs$ = spectator.service.login('dummy', 'dummy');

Expand All @@ -91,10 +100,6 @@ describe('AuthService', () => {
'(a|)',
{ a: LoginResult.Success },
);
expectObservable(spectator.service.isAuthenticated$).toBe(
'c',
{ c: true },
);
expectObservable(spectator.service.authToken$).toBe(
'd',
{ d: 'DUMMY_TOKEN' },
Expand All @@ -109,7 +114,7 @@ describe('AuthService', () => {
});

it('initializes auth session with triggers and token with token login', () => {
jest.spyOn(rxjs, 'timer').mockReturnValueOnce(of(0));
timer$.next(0);

const obs$ = spectator.service.loginWithToken();

Expand All @@ -118,10 +123,6 @@ describe('AuthService', () => {
'(a|)',
{ a: LoginResult.Success },
);
expectObservable(spectator.service.isAuthenticated$).toBe(
'c',
{ c: true },
);
expectObservable(spectator.service.authToken$).toBe(
'd',
{ d: 'DUMMY_TOKEN' },
Expand All @@ -146,10 +147,6 @@ describe('AuthService', () => {
a: undefined,
},
);
expectObservable(spectator.service.isAuthenticated$).toBe(
'c',
{ c: false },
);
expectObservable(spectator.service.authToken$).toBe(
'|',
{},
Expand Down
34 changes: 11 additions & 23 deletions src/app/modules/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { Store } from '@ngrx/store';
import { LocalStorage } from 'ngx-webstorage';
import {
BehaviorSubject,
combineLatest,
filter,
map,
Observable,
Expand All @@ -23,8 +22,8 @@ import { LoginExMechanism, LoginExResponse, LoginExResponseType } from 'app/inte
import { LoggedInUser } from 'app/interfaces/ds-cache.interface';
import { GlobalTwoFactorConfig } from 'app/interfaces/two-factor-config.interface';
import { ApiService } from 'app/modules/websocket/api.service';
import { WebSocketHandlerService } from 'app/modules/websocket/websocket-handler.service';
import { TokenLastUsedService } from 'app/services/token-last-used.service';
import { WebSocketStatusService } from 'app/services/websocket-status.service';
import { AppState } from 'app/store';
import { adminUiInitialized } from 'app/store/admin-panel/admin.actions';

Expand All @@ -51,19 +50,8 @@ export class AuthService {
return Boolean(this.token) && this.token !== 'null';
}

private isLoggedIn$ = new BehaviorSubject<boolean>(false);

private generateTokenSubscription: Subscription | null;

readonly isAuthenticated$ = combineLatest([
this.wsManager.isConnected$,
this.isLoggedIn$.asObservable(),
]).pipe(
switchMap(([isConnected, isLoggedIn]) => {
return of(isConnected && isLoggedIn);
}),
);

readonly user$ = this.loggedInUser$.asObservable();

/**
Expand All @@ -82,11 +70,11 @@ export class AuthService {
private cachedGlobalTwoFactorConfig: GlobalTwoFactorConfig | null;

constructor(
private wsManager: WebSocketHandlerService,
private store$: Store<AppState>,
private api: ApiService,
private tokenLastUsedService: TokenLastUsedService,
@Inject(WINDOW) private window: Window,
private wsStatus: WebSocketStatusService,
) {
this.setupAuthenticationUpdate();
this.setupWsConnectionUpdate();
Expand Down Expand Up @@ -183,7 +171,7 @@ export class AuthService {
tap(() => {
this.clearAuthToken();
this.api.clearSubscriptions();
this.isLoggedIn$.next(false);
this.wsStatus.setLoginStatus(false);
}),
);
}
Expand All @@ -202,18 +190,18 @@ export class AuthService {
this.loggedInUser$.next(result.user_info);

if (!result.user_info?.privilege?.webui_access) {
this.isLoggedIn$.next(false);
this.wsStatus.setLoginStatus(false);
return of(LoginResult.NoAccess);
}

this.isLoggedIn$.next(true);
this.wsStatus.setLoginStatus(true);
this.window.sessionStorage.setItem('loginBannerDismissed', 'true');
return this.authToken$.pipe(
take(1),
map(() => LoginResult.Success),
);
}
this.isLoggedIn$.next(false);
this.wsStatus.setLoginStatus(false);

if (result.response_type === LoginExResponseType.OtpRequired) {
return of(LoginResult.NoOtp);
Expand All @@ -226,8 +214,8 @@ export class AuthService {
private setupPeriodicTokenGeneration(): void {
if (!this.generateTokenSubscription || this.generateTokenSubscription.closed) {
this.generateTokenSubscription = timer(0, this.tokenRegenerationTimeMillis).pipe(
switchMap(() => this.isAuthenticated$.pipe(take(1))),
filter((isAuthenticated) => isAuthenticated),
switchMap(() => this.wsStatus.isAuthenticated$.pipe(take(1))),
filter(Boolean),
switchMap(() => this.api.call('auth.generate_token')),
tap((token) => this.latestTokenGenerated$.next(token)),
).subscribe();
Expand All @@ -243,7 +231,7 @@ export class AuthService {
}

private setupAuthenticationUpdate(): void {
this.isAuthenticated$.subscribe({
this.wsStatus.isAuthenticated$.subscribe({
next: (isAuthenticated) => {
if (isAuthenticated) {
this.store$.dispatch(adminUiInitialized());
Expand All @@ -260,8 +248,8 @@ export class AuthService {
}

private setupWsConnectionUpdate(): void {
this.wsManager.isConnected$.pipe(filter((isConnected) => !isConnected)).subscribe(() => {
this.isLoggedIn$.next(false);
this.wsStatus.isConnected$.pipe(filter((isConnected) => !isConnected)).subscribe(() => {
this.wsStatus.setLoginStatus(false);
});
}

Expand Down
Loading

0 comments on commit 464a06e

Please sign in to comment.