Skip to content

Commit

Permalink
fix: login guard and me service initialization (#8840)
Browse files Browse the repository at this point in the history
* fix login guard and me service initialization

* fix static check
  • Loading branch information
floreks authored Mar 28, 2024
1 parent 7d1785f commit 172d5e0
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 45 deletions.
3 changes: 2 additions & 1 deletion modules/auth/pkg/routes/me/me.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ type ServiceAccount struct {
func me(request *http.Request) (*types.User, int, error) {
k8sClient, err := client.Client(request)
if err != nil {
return nil, http.StatusInternalServerError, err
code, err := errors.HandleError(err)
return nil, code, err
}

// Make sure that authorization token is valid
Expand Down
2 changes: 1 addition & 1 deletion modules/web/src/common/components/chips/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from '@angular/core';
import {MatDialog, MatDialogConfig} from '@angular/material/dialog';
import {StringMap} from '@api/root.shared';
// @ts-ignore
// @ts-expect-error

Check warning on line 26 in modules/web/src/common/components/chips/component.ts

View workflow job for this annotation

GitHub Actions / Static check

Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer
import cropUrl from 'crop-url';

import {GlobalSettingsService} from '../../services/global/globalsettings';
Expand Down
12 changes: 5 additions & 7 deletions modules/web/src/common/services/global/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ import {Inject, Injectable} from '@angular/core';
import {Router} from '@angular/router';
import {IConfig} from '@api/root.ui';
import {CookieService} from 'ngx-cookie-service';
import {of} from 'rxjs';
import {Observable} from 'rxjs';
import {switchMap} from 'rxjs/operators';
import {AuthResponse, CsrfToken, LoginSpec} from 'typings/root.api';
import {AuthResponse, CsrfToken, LoginSpec, User} from 'typings/root.api';
import {CONFIG_DI_TOKEN} from '../../../index.config';
import {CsrfTokenService} from './csrftoken';
import {KdStateService} from './state';
Expand All @@ -46,7 +45,7 @@ export class AuthService {
/**
* Sends a login request to the backend with filled in login spec structure.
*/
login(loginSpec: LoginSpec): Observable<void> {
login(loginSpec: LoginSpec): Observable<User> {
return this.csrfTokenService_
.getTokenForAction('login')
.pipe(
Expand All @@ -62,8 +61,7 @@ export class AuthService {
this.setTokenCookie_(authResponse.token);
}

this._meService.refresh();
return of(void 0);
return this._meService.refresh();
})
);
}
Expand Down Expand Up @@ -103,11 +101,11 @@ export class AuthService {
}

isAuthenticated(): boolean {
return this._meService.getUser()?.authenticated || this.hasTokenCookie();
return this._meService.getUser().authenticated || this.hasTokenCookie();
}

hasAuthHeader(): boolean {
return this._meService.getUser()?.authenticated && !this.hasTokenCookie();
return this._meService.getUser().authenticated && !this.hasTokenCookie();
}

private getTokenCookie(): string {
Expand Down
8 changes: 4 additions & 4 deletions modules/web/src/common/services/global/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {HttpClient} from '@angular/common/http';
import {Injectable} from '@angular/core';
import {AppConfig} from '@api/root.ui';
import {tap} from 'rxjs/operators';
import {lastValueFrom} from 'rxjs';

@Injectable()
export class LocalConfigLoaderService {
Expand All @@ -28,9 +29,8 @@ export class LocalConfigLoaderService {
}

init(): Promise<{}> {
return this.http_
.get('assets/config/config.json')
.pipe(tap(response => (this.appConfig_ = response as AppConfig)))
.toPromise();
return lastValueFrom(
this.http_.get('assets/config/config.json').pipe(tap(response => (this.appConfig_ = response as AppConfig)))
);
}
}
42 changes: 31 additions & 11 deletions modules/web/src/common/services/global/me.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,42 @@
* limitations under the License.
*/

import {Injectable} from '@angular/core';
import {EventEmitter, Injectable} from '@angular/core';
import {User} from '@api/root.api';
import {HttpClient} from '@angular/common/http';
import {interval, Observable} from 'rxjs';
import {startWith, switchMap} from 'rxjs/operators';
import {interval, lastValueFrom, Observable, of} from 'rxjs';
import {catchError, switchMap, take, tap} from 'rxjs/operators';

@Injectable()
export class MeService {
private readonly _endpoint = 'api/v1/me';
private _user: User;
private _interval = interval(30000);
private _user: User = {authenticated: false} as User;
private _interval = interval(30_000);
private _initialized = new EventEmitter<void>(true);

constructor(private readonly _http: HttpClient) {}

init(): void {
init(): Promise<void> {
// Immediately check user
this.me()
.pipe(
take(1),
catchError(_ => of({authenticated: false} as User))
)
.subscribe(me => {
this._user = me;
this._initialized.emit();
});

// Start interval refresh
this._interval
.pipe(
startWith({} as User),
switchMap(() => this.me())
switchMap(() => this.me()),
catchError(_ => of({authenticated: false} as User))
)
.subscribe(user => (this._user = user));
.subscribe(me => (this._user = me));

return lastValueFrom(this._initialized.pipe(take(1)));
}

me(): Observable<User> {
Expand All @@ -53,7 +68,12 @@ export class MeService {
this._user = {} as User;
}

refresh(): void {
this.me().subscribe(user => (this._user = user));
refresh(): Promise<User> {
return lastValueFrom(
this.me().pipe(
take(1),
tap(user => (this._user = user))
)
);
}
}
20 changes: 10 additions & 10 deletions modules/web/src/common/services/global/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import {MeService} from '@common/services/global/me';
})
export class GlobalServicesModule {
static injector: Injector;

constructor(injector: Injector) {
GlobalServicesModule.injector = injector;
}
Expand All @@ -100,15 +101,14 @@ export function init(
loader: LocalConfigLoaderService,
me: MeService
): Function {
return () => {
return loader.init().then(() => {
localSettings.init();
pinner.init();
config.init();
history.init();
theme.init();
me.init();
return globalSettings.init();
});
return async () => {
await loader.init();
localSettings.init();
pinner.init();
config.init();
history.init();
theme.init();
await me.init();
return await globalSettings.init();
};
}
6 changes: 0 additions & 6 deletions modules/web/src/common/services/guard/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {ActivatedRouteSnapshot, Router, UrlTree} from '@angular/router';
import {Observable, of} from 'rxjs';
import {AuthService} from '../global/authentication';
import {HistoryService} from '../global/history';
import {environment} from '@environments/environment';

@Injectable()
export class AuthGuard {
Expand All @@ -28,11 +27,6 @@ export class AuthGuard {
) {}

canActivate(_root: ActivatedRouteSnapshot): Observable<boolean | UrlTree> {
// Disable auth guard for dev mode
if (!environment.production) {
return of(true);
}

if (!this.authService_.isAuthenticated()) {
this.historyService_.pushState(this.router_.getCurrentNavigation());
return of(this.router_.parseUrl('login'));
Expand Down
8 changes: 4 additions & 4 deletions modules/web/src/common/services/guard/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import {AuthService} from '../global/authentication';
@Injectable()
export class LoginGuard {
constructor(
private readonly authService_: AuthService,
private readonly router_: Router
private readonly _authService: AuthService,
private readonly _router: Router
) {}

canActivate(): Observable<boolean | UrlTree> {
// If user is already authenticated do not allow login view access.
if (this.authService_.isAuthenticated()) {
return of(this.router_.parseUrl('workloads'));
if (this._authService.isAuthenticated()) {
return of(this._router.parseUrl('workloads'));
}

return of(true);
Expand Down
2 changes: 1 addition & 1 deletion modules/web/src/systemjs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface Window {
define: (name: string, deps: string[], definitionFn: () => any) => void;

System: {
// @ts-ignore
// @ts-expect-error

Check warning on line 19 in modules/web/src/systemjs.d.ts

View workflow job for this annotation

GitHub Actions / Static check

Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer
import: (path) => Promise<any>;
};
}

0 comments on commit 172d5e0

Please sign in to comment.