From 580ebee362ffb1ad495f3d76520cdfa9f6e54a8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thea=20Sch=C3=B6bl?= Date: Thu, 28 Nov 2024 18:04:08 +0000 Subject: [PATCH] fix: user gets logged out when their token expires Closes #230 --- .../app/src/app/modules/auth/auth.service.ts | 15 +++- .../modules/auth/paia/paia-auth.service.ts | 13 +++ .../app/modules/profile/id-cards.provider.ts | 6 +- .../src/app/modules/profile/id-cards.spec.ts | 6 +- .../page/profile-page-section.component.ts | 79 +++++++------------ .../profile/page/profile-page-section.html | 12 +-- 6 files changed, 67 insertions(+), 64 deletions(-) diff --git a/frontend/app/src/app/modules/auth/auth.service.ts b/frontend/app/src/app/modules/auth/auth.service.ts index cca6a5d1..1b38118f 100644 --- a/frontend/app/src/app/modules/auth/auth.service.ts +++ b/frontend/app/src/app/modules/auth/auth.service.ts @@ -86,6 +86,8 @@ export abstract class AuthService implements IAuthService { private _authenticatedSubject = new BehaviorSubject(false); + private _loggedInSubject = new BehaviorSubject(false); + private _initComplete = new BehaviorSubject(false); protected tokenHandler: TokenRequestHandler; @@ -133,6 +135,13 @@ export abstract class AuthService implements IAuthService { return this._authenticatedSubject.asObservable(); } + /** + * Similar to isAuthenticated$, but will also return true if the token is expired + */ + get isLoggedIn$(): Observable { + return this._loggedInSubject.asObservable(); + } + get initComplete$(): Observable { return this._initComplete.asObservable(); } @@ -183,19 +192,20 @@ export abstract class AuthService implements IAuthService { protected notifyActionListers(action: IAuthAction) { /* eslint-disable unicorn/no-useless-undefined */ switch (action.action) { - case AuthActions.RefreshFailed: case AuthActions.SignInFailed: case AuthActions.SignOutSuccess: case AuthActions.SignOutFailed: { this._tokenSubject.next(undefined); this._userSubject.next(undefined); this._authenticatedSubject.next(false); + this._loggedInSubject.next(false); break; } case AuthActions.LoadTokenFromStorageFailed: { this._tokenSubject.next(undefined); this._userSubject.next(undefined); this._authenticatedSubject.next(false); + this._loggedInSubject.next(false); this._initComplete.next(true); break; } @@ -203,11 +213,13 @@ export abstract class AuthService implements IAuthService { case AuthActions.RefreshSuccess: { this._tokenSubject.next(action.tokenResponse); this._authenticatedSubject.next(true); + this._loggedInSubject.next(true); break; } case AuthActions.LoadTokenFromStorageSuccess: { this._tokenSubject.next(action.tokenResponse); this._authenticatedSubject.next((action.tokenResponse as TokenResponse).isValid(0)); + this._loggedInSubject.next(true); this._initComplete.next(true); break; } @@ -442,7 +454,6 @@ export abstract class AuthService implements IAuthService { public async refreshToken() { await this.requestTokenRefresh().catch(error => { - this.storage.removeItem(TOKEN_RESPONSE_KEY); this.notifyActionListers(AuthActionBuilder.RefreshFailed(error)); }); } diff --git a/frontend/app/src/app/modules/auth/paia/paia-auth.service.ts b/frontend/app/src/app/modules/auth/paia/paia-auth.service.ts index 252ab1eb..6de1bf6a 100644 --- a/frontend/app/src/app/modules/auth/paia/paia-auth.service.ts +++ b/frontend/app/src/app/modules/auth/paia/paia-auth.service.ts @@ -81,6 +81,8 @@ export class PAIAAuthService { private _authenticatedSubject = new BehaviorSubject(false); + private _loggedIn = new BehaviorSubject(false); + private _initComplete = new BehaviorSubject(false); protected tokenHandler: PAIATokenRequestHandler; @@ -118,6 +120,13 @@ export class PAIAAuthService { return this._authenticatedSubject.asObservable(); } + /** + * Similar to isAuthenticated$, but will also return true if the token is expired + */ + get isLoggedIn$(): Observable { + return this._loggedIn.asObservable(); + } + get initComplete$(): Observable { return this._initComplete.asObservable(); } @@ -170,23 +179,27 @@ export class PAIAAuthService { this._tokenSubject.next(undefined); this._userSubject.next(undefined); this._authenticatedSubject.next(false); + this._loggedIn.next(false); break; } case AuthActions.LoadTokenFromStorageFailed: { this._tokenSubject.next(undefined); this._userSubject.next(undefined); this._authenticatedSubject.next(false); + this._loggedIn.next(false); this._initComplete.next(true); break; } case AuthActions.SignInSuccess: { this._tokenSubject.next(action.tokenResponse); this._authenticatedSubject.next(true); + this._loggedIn.next(true); break; } case AuthActions.LoadTokenFromStorageSuccess: { this._tokenSubject.next(action.tokenResponse); this._authenticatedSubject.next((action.tokenResponse as TokenResponse).isValid(0)); + this._loggedIn.next(true); this._initComplete.next(true); break; } diff --git a/frontend/app/src/app/modules/profile/id-cards.provider.ts b/frontend/app/src/app/modules/profile/id-cards.provider.ts index c6e1a32c..93bf189f 100644 --- a/frontend/app/src/app/modules/profile/id-cards.provider.ts +++ b/frontend/app/src/app/modules/profile/id-cards.provider.ts @@ -23,9 +23,9 @@ export class IdCardsProvider { this.encryptedStorageProvider.get('id-cards') as Promise, ).pipe(filter(it => it !== undefined)); - return auth.isAuthenticated$.pipe( - mergeMap(isAuthenticated => - isAuthenticated + return auth.isLoggedIn$.pipe( + mergeMap(isLoggedIn => + isLoggedIn ? feature ? storedIdCards.pipe( concatWith( diff --git a/frontend/app/src/app/modules/profile/id-cards.spec.ts b/frontend/app/src/app/modules/profile/id-cards.spec.ts index 4553a34b..911a5581 100644 --- a/frontend/app/src/app/modules/profile/id-cards.spec.ts +++ b/frontend/app/src/app/modules/profile/id-cards.spec.ts @@ -7,7 +7,7 @@ import {SCAuthorizationProviderType} from '@openstapps/core'; import {EncryptedStorageProvider} from '../storage/encrypted-storage.provider'; class FakeAuth { - isAuthenticated$ = new BehaviorSubject(false); + isLoggedIn$ = new BehaviorSubject(false); // eslint-disable-next-line @typescript-eslint/no-empty-function getValidToken() {} @@ -42,7 +42,7 @@ describe('IdCards', () => { }); it('should emit network result when logged in', async () => { - fakeAuth.isAuthenticated$.next(true); + fakeAuth.isLoggedIn$.next(true); httpClient.get = jasmine.createSpy().and.returnValue(of(['abc'])); fakeAuth.getValidToken = jasmine.createSpy().and.resolveTo({accessToken: 'fake-token'}); const provider = new IdCardsProvider(authHelper, configProvider, httpClient, encryptedStorageProvider); @@ -63,7 +63,7 @@ describe('IdCards', () => { expect(await firstValueFrom(observable)).toEqual([]); httpClient.get = jasmine.createSpy().and.returnValue(of(['abc'])); fakeAuth.getValidToken = jasmine.createSpy().and.resolveTo({accessToken: 'fake-token'}); - fakeAuth.isAuthenticated$.next(true); + fakeAuth.isLoggedIn$.next(true); // this is counter-intuitive, but because we unsubscribed above the first value // will now contain the network result. expect(await firstValueFrom(observable)).toEqual(['abc' as never]); diff --git a/frontend/app/src/app/modules/profile/page/profile-page-section.component.ts b/frontend/app/src/app/modules/profile/page/profile-page-section.component.ts index 7ff56bd3..d8bf4114 100644 --- a/frontend/app/src/app/modules/profile/page/profile-page-section.component.ts +++ b/frontend/app/src/app/modules/profile/page/profile-page-section.component.ts @@ -6,74 +6,56 @@ * * This program is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * FITNESS FOR A PARTICULAr purpose. see the gnu general public license for * more details. * - * You should have received a copy of the GNU General Public License along with - * this program. If not, see . + * you should have received a copy of the gnu general public license along with + * this program. if not, see . */ -import {Component, DestroyRef, inject, Input, OnInit} from '@angular/core'; +import {Component, inject, input} from '@angular/core'; import {SCSection} from '../../../../config/profile-page-sections'; import {AuthHelperService} from '../../auth/auth-helper.service'; -import {Observable} from 'rxjs'; -import {SCAuthorizationProviderType} from '@openstapps/core'; +import {mergeMap, of} from 'rxjs'; import Swiper from 'swiper'; -import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; +import {toObservable, toSignal} from '@angular/core/rxjs-interop'; @Component({ selector: 'stapps-profile-page-section', templateUrl: 'profile-page-section.html', styleUrls: ['profile-page-section.scss'], }) -export class ProfilePageSectionComponent implements OnInit { - @Input() item: SCSection; +export class ProfilePageSectionComponent { + item = input.required(); - @Input() minSlideWidth = 110; + minSlideWidth = input(110); - isLoggedIn: boolean; + authHelper = inject(AuthHelperService); + + loggedIn = toSignal( + toObservable(this.item).pipe( + mergeMap(item => + item.authProvider ? this.authHelper.getProvider(item.authProvider).isLoggedIn$ : of(false), + ), + ), + ); isEnd = false; isBeginning = true; - slidesPerView: number; + slidesPerView?: number; slidesFillScreen = false; - data: { - [key in SCAuthorizationProviderType]: {loggedIn$: Observable}; - } = { - default: { - loggedIn$: this.authHelper.getProvider('default').isAuthenticated$, - }, - paia: { - loggedIn$: this.authHelper.getProvider('paia').isAuthenticated$, - }, - }; - - destroy$ = inject(DestroyRef); - - constructor(private authHelper: AuthHelperService) {} - - ngOnInit() { - if (this.item.authProvider) { - this.data[this.item.authProvider].loggedIn$ - .pipe(takeUntilDestroyed(this.destroy$)) - .subscribe(loggedIn => { - this.isLoggedIn = loggedIn; - }); - } - } - activeIndexChange(swiper: Swiper) { this.isBeginning = swiper.isBeginning; this.isEnd = swiper.isEnd; - this.slidesFillScreen = this.slidesPerView >= swiper.slides.length; + this.slidesFillScreen = this.slidesPerView! >= swiper.slides.length; } resizeSwiper(resizeEvent: ResizeObserverEntry, swiper: Swiper) { const slidesPerView = - Math.floor((resizeEvent.contentRect.width - this.minSlideWidth / 2) / this.minSlideWidth) + 0.5; + Math.floor((resizeEvent.contentRect.width - this.minSlideWidth() / 2) / this.minSlideWidth()) + 0.5; if (slidesPerView > 1 && slidesPerView !== this.slidesPerView) { this.slidesPerView = slidesPerView; @@ -84,16 +66,13 @@ export class ProfilePageSectionComponent implements OnInit { } async toggleLogIn() { - if (!this.item.authProvider) return; - await (this.isLoggedIn ? this.signOut(this.item.authProvider) : this.signIn(this.item.authProvider)); - } - - async signIn(providerType: SCAuthorizationProviderType) { - await this.authHelper.getProvider(providerType).signIn(); - } - - async signOut(providerType: SCAuthorizationProviderType) { - await this.authHelper.getProvider(providerType).signOut(); - await this.authHelper.endBrowserSession(providerType); + const providerType = this.item().authProvider; + if (!providerType) return; + if (this.loggedIn()) { + await this.authHelper.getProvider(providerType).signOut(); + await this.authHelper.endBrowserSession(providerType); + } else { + await this.authHelper.getProvider(providerType).signIn(); + } } } diff --git a/frontend/app/src/app/modules/profile/page/profile-page-section.html b/frontend/app/src/app/modules/profile/page/profile-page-section.html index 308b92ad..5b46f123 100644 --- a/frontend/app/src/app/modules/profile/page/profile-page-section.html +++ b/frontend/app/src/app/modules/profile/page/profile-page-section.html @@ -13,24 +13,24 @@ ~ this program. If not, see . --> - - @if (item.authProvider) { + + @if (item().authProvider) { - @if (isLoggedIn) { + @if (loggedIn()) { } @else { } - {{ 'profile.buttons.default.log_' + (isLoggedIn ? 'out' : 'in') | translate }} + {{ 'profile.buttons.default.log_' + (loggedIn() ? 'out' : 'in') | translate }} } - @for (link of item.links; track link) { + @for (link of item().links; track link) {