fix: overhaul auth to avoid issues

Closes #336
2026-01-09 09:12:52 +00:00 · 2022-11-28 16:15:33 +01:00
parent 6720265410
commit 99e8d6c9bc
10 changed files with 231 additions and 112 deletions
--- a/src/app/modules/auth/auth-callback/page/auth-callback-page.component.html
+++ b/src/app/modules/auth/auth-callback/page/auth-callback-page.component.html
@@ -1,5 +1,7 @@
 <div class="centeredMessageContainer">
  <p>
-    {{ 'auth.messages' + '.' + providerType + '.' + 'authorizing' | translate }}
+    {{
+      'auth.messages' + '.' + PROVIDER_TYPE + '.' + 'authorizing' | translate
+    }}
  </p>
 </div>
--- a/src/app/modules/auth/auth-callback/page/auth-callback-page.component.ts
+++ b/src/app/modules/auth/auth-callback/page/auth-callback-page.component.ts
@@ -26,7 +26,7 @@ import {AuthHelperService} from '../../auth-helper.service';
  styleUrls: ['auth-callback-page.component.scss'],
 })
 export class AuthCallbackPageComponent implements OnInit, OnDestroy {
-  providerType: SCAuthorizationProviderType = 'default';
+  PROVIDER_TYPE: SCAuthorizationProviderType = 'default';

  private authEvents: Subscription;

@@ -38,10 +38,10 @@ export class AuthCallbackPageComponent implements OnInit, OnDestroy {

  ngOnInit() {
    this.authEvents = this.authHelper
-      .getProvider(this.providerType)
+      .getProvider(this.PROVIDER_TYPE)
      .events$.subscribe((action: IAuthAction) => this.postCallback(action));
    this.authHelper
-      .getProvider(this.providerType)
+      .getProvider(this.PROVIDER_TYPE)
      .authorizationCallback(window.location.origin + this.router.url);
  }

--- a/src/app/modules/auth/auth.service.ts
+++ b/src/app/modules/auth/auth.service.ts
@@ -369,6 +369,9 @@ export abstract class AuthService implements IAuthService {
      await this.configuration,
      new TokenRequest(requestJSON),
    );
+    if (!token.accessToken) {
+      throw new Error('No Access Token Defined In Refresh Response');
+    }
    await this.storage.setItem(
      TOKEN_RESPONSE_KEY,
      JSON.stringify(token.toJson()),
--- a/src/app/modules/auth/default-auth.service.spec.ts
+++ b/src/app/modules/auth/default-auth.service.spec.ts
@@ -0,0 +1,113 @@
+/*
+ * Copyright (C) 2022 StApps
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, version 3.
+ *
+ * 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
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+import {TestBed} from '@angular/core/testing';
+import {ConfigProvider} from '../config/config.provider';
+import {StorageProvider} from '../storage/storage.provider';
+import {DefaultAuthService} from './default-auth.service';
+import {Browser} from 'ionic-appauth';
+import {nowInSeconds, Requestor, StorageBackend} from '@openid/appauth';
+import {TranslateService} from '@ngx-translate/core';
+import {LoggerConfig, LoggerModule, NGXLogger} from 'ngx-logger';
+import {StAppsWebHttpClient} from '../data/stapps-web-http-client.provider';
+import {HttpClientModule} from '@angular/common/http';
+import {IonicStorage} from 'ionic-appauth/lib';
+
+describe('AuthService', () => {
+  let defaultAuthService: DefaultAuthService;
+  let storageBackendSpy: jasmine.SpyObj<StorageBackend>;
+  const storageProviderSpy = jasmine.createSpyObj('StorageProvider', [
+    'init',
+    'get',
+    'has',
+    'put',
+    'search',
+  ]);
+  const translateServiceSpy = jasmine.createSpyObj('TranslateService', [
+    'setDefaultLang',
+    'use',
+  ]);
+
+  beforeEach(() => {
+    storageBackendSpy = jasmine.createSpyObj('StorageBackend', ['getItem']);
+
+    TestBed.configureTestingModule({
+      imports: [HttpClientModule, LoggerModule],
+      providers: [
+        NGXLogger,
+        StAppsWebHttpClient,
+        LoggerConfig,
+        {
+          provide: TranslateService,
+          useValue: translateServiceSpy,
+        },
+        {
+          provide: StorageProvider,
+          useValue: storageProviderSpy,
+        },
+        IonicStorage,
+        ConfigProvider,
+        Browser,
+        {
+          provide: StorageBackend,
+          useValue: storageBackendSpy,
+        },
+        Requestor,
+      ],
+    });
+    defaultAuthService = TestBed.inject(DefaultAuthService);
+  });
+
+  describe('loadTokenFromStorage', () => {
+    it('should provide false through isAuthenticated$ when there is no token response', async () => {
+      // eslint-disable-next-line unicorn/no-null
+      storageBackendSpy.getItem.and.returnValue(Promise.resolve(null));
+      let loggedInHolder;
+      defaultAuthService.isAuthenticated$.subscribe(loggedIn => {
+        loggedInHolder = loggedIn;
+      });
+      await defaultAuthService.loadTokenFromStorage();
+
+      expect(loggedInHolder).toBeFalse();
+    });
+
+    it('should provide true through isAuthenticated$ when access token is valid', async () => {
+      const validToken = `{"access_token":"AT-XXXX","refresh_token":"RT-XXXX","scope":"","token_type":"bearer","issued_at":${nowInSeconds()},"expires_in":"${
+        8 * 60 * 60
+      }"}`;
+      storageBackendSpy.getItem.and.returnValue(Promise.resolve(validToken));
+      let loggedInHolder;
+      defaultAuthService.isAuthenticated$.subscribe(loggedIn => {
+        loggedInHolder = loggedIn;
+      });
+      await defaultAuthService.loadTokenFromStorage();
+
+      expect(loggedInHolder).toBeTrue();
+    });
+
+    it('should provide false through isAuthenticated$ when access token is invalid', async () => {
+      const invalidToken = `{"access_token":"AT-INVALID-XXXX","refresh_token":"RT-XXXX","scope":"","token_type":"bearer","issued_at":${
+        nowInSeconds() - 9 * 60 * 60
+      },"expires_in":"${8 * 60 * 60}"}`;
+      storageBackendSpy.getItem.and.returnValue(Promise.resolve(invalidToken));
+      let loggedInHolder;
+      defaultAuthService.isAuthenticated$.subscribe(loggedIn => {
+        loggedInHolder = loggedIn;
+      });
+      await defaultAuthService.loadTokenFromStorage();
+
+      expect(loggedInHolder).toBeFalse();
+    });
+  });
+});
--- a/src/app/modules/auth/default-auth.service.ts
+++ b/src/app/modules/auth/default-auth.service.ts
@@ -69,9 +69,15 @@ export class DefaultAuthService extends AuthService {
  }

  public async signOut() {
-    await this.storage.removeItem(TOKEN_RESPONSE_KEY).catch(error => {
+    await this.revokeTokens().catch(error => {
      this.notifyActionListers(AuthActionBuilder.SignOutFailed(error));
    });
    this.notifyActionListers(AuthActionBuilder.SignOutSuccess());
  }
+
+  public async revokeTokens() {
+    // Note: only locally
+    await this.storage.removeItem(TOKEN_RESPONSE_KEY);
+    this.notifyActionListers(AuthActionBuilder.RevokeTokensSuccess());
+  }
 }
--- a/src/app/modules/auth/paia/auth-callback/page/paiaauth-callback-page.component.ts
+++ b/src/app/modules/auth/paia/auth-callback/page/paiaauth-callback-page.component.ts
@@ -10,7 +10,7 @@ import {AuthHelperService} from '../../../auth-helper.service';
  styleUrls: ['../../../auth-callback/page/auth-callback-page.component.scss'],
 })
 export class PAIAAuthCallbackPageComponent extends AuthCallbackPageComponent {
-  providerType = 'paia' as SCAuthorizationProviderType;
+  PROVIDER_TYPE = 'paia' as SCAuthorizationProviderType;

  constructor(
    navCtrl: NavController,
--- a/src/app/modules/auth/paia/paia-auth.service.ts
+++ b/src/app/modules/auth/paia/paia-auth.service.ts
@@ -10,6 +10,7 @@ import {
  Requestor,
  StorageBackend,
  StringMap,
+  TokenResponse,
 } from '@openid/appauth';
 import {
  AuthActions,
@@ -36,8 +37,8 @@ import {ConfigProvider} from '../../config/config.provider';
 import {getClientConfig, getEndpointsConfig} from '../auth.provider.methods';
 import {Injectable} from '@angular/core';

-const TOKEN_KEY = 'auth_paia_token';
-const AUTH_EXPIRY_BUFFER = 10 * 60 * -1; // 10 mins in seconds
+const TOKEN_RESPONSE_KEY = 'paia_token_response';
+const AUTH_EXPIRY_BUFFER = 10 * 60 * -1; // 10 minutes in seconds

 export interface IAuthService {
  signIn(authExtras?: StringMap, state?: string): void;
@@ -153,9 +154,6 @@ export class PAIAAuthService {
  }

  protected notifyActionListers(action: IPAIAAuthAction) {
-    this._authSubjectV2.next(action);
-    this._authSubject.notify(action);
-
    /* eslint-disable unicorn/no-useless-undefined */
    switch (action.action) {
      case AuthActions.SignInFailed:
@@ -177,7 +175,9 @@ export class PAIAAuthService {
        break;
      case AuthActions.LoadTokenFromStorageSuccess:
        this._tokenSubject.next(action.tokenResponse);
-        this._authenticatedSubject.next(true);
+        this._authenticatedSubject.next(
+          (action.tokenResponse as TokenResponse).isValid(0),
+        );
        this._initComplete.next(true);
        break;
      case AuthActions.RevokeTokensSuccess:
@@ -190,6 +190,9 @@ export class PAIAAuthService {
        this._userSubject.next(undefined);
        break;
    }
+
+    this._authSubjectV2.next(action);
+    this._authSubject.notify(action);
  }

  protected setupAuthorizationNotifier() {
@@ -272,13 +275,16 @@ export class PAIAAuthService {
        await this.configuration,
        new PAIATokenRequest(requestJSON),
      );
-    await this.storage.setItem(TOKEN_KEY, JSON.stringify(token.toJson()));
+    await this.storage.setItem(
+      TOKEN_RESPONSE_KEY,
+      JSON.stringify(token.toJson()),
+    );
    this.notifyActionListers(PAIAAuthActionBuilder.SignInSuccess(token));
  }

  public async revokeTokens() {
    // Note: only locally
-    await this.storage.removeItem(TOKEN_KEY);
+    await this.storage.removeItem(TOKEN_RESPONSE_KEY);
    this.notifyActionListers(PAIAAuthActionBuilder.RevokeTokensSuccess());
  }

@@ -292,7 +298,7 @@ export class PAIAAuthService {
  protected async internalLoadTokenFromStorage() {
    let token: PAIATokenResponse | undefined;
    const tokenResponseString: string | null = await this.storage.getItem(
-      TOKEN_KEY,
+      TOKEN_RESPONSE_KEY,
    );

    if (tokenResponseString != undefined) {
@@ -355,6 +361,9 @@ export class PAIAAuthService {
      return this._tokenSubject.value;
    }

-    throw new Error('Unable To Obtain Valid Token');
+    const error = new Error('Unable To Obtain Valid Token');
+    this.notifyActionListers(PAIAAuthActionBuilder.SignInFailed(error));
+
+    throw error;
  }
 }
--- a/src/app/modules/profile/page/profile-page-section.component.ts
+++ b/src/app/modules/profile/page/profile-page-section.component.ts
@@ -16,7 +16,7 @@
 import {Component, Input, OnDestroy, OnInit} from '@angular/core';
 import {SCSection} from './sections';
 import {AuthHelperService} from '../../auth/auth-helper.service';
-import {Subscription} from 'rxjs';
+import {Observable, Subscription} from 'rxjs';
 import {SCAuthorizationProviderType} from '@openstapps/core';
 import Swiper from 'swiper';

@@ -42,25 +42,25 @@ export class ProfilePageSectionComponent implements OnInit, OnDestroy {

  slidesFillScreen = false;

+  data: {
+    [key in SCAuthorizationProviderType]: {loggedIn$: Observable<boolean>};
+  } = {
+    default: {
+      loggedIn$: this.authHelper.getProvider('default').isAuthenticated$,
+    },
+    paia: {
+      loggedIn$: this.authHelper.getProvider('paia').isAuthenticated$,
+    },
+  };
+
  constructor(private authHelper: AuthHelperService) {}

  ngOnInit() {
    if (this.item.authProvider) {
-      const provider = this.item.authProvider;
      this.subscriptions.push(
-        this.authHelper
-          .getProvider(provider as 'default')
-          .token$.subscribe(_token => {
-            this.authHelper
-              .getProvider(provider)
-              .getValidToken()
-              .then(() => {
-                this.isLoggedIn = true;
-              })
-              .catch(_error => {
-                this.isLoggedIn = false;
-              });
-          }),
+        this.data[this.item.authProvider].loggedIn$.subscribe(loggedIn => {
+          this.isLoggedIn = loggedIn;
+        }),
      );
    }
  }
--- a/src/app/modules/profile/page/profile-page.component.ts
+++ b/src/app/modules/profile/page/profile-page.component.ts
@@ -14,9 +14,7 @@
 */

 import {Component, OnInit} from '@angular/core';
-import {IonicUserInfoHandler} from 'ionic-appauth';
-import {Requestor, TokenResponse} from '@openid/appauth';
-import {Subscription} from 'rxjs';
+import {Observable, of, Subscription} from 'rxjs';
 import {AuthHelperService} from '../../auth/auth-helper.service';
 import {
  SCAuthorizationProviderType,
@@ -28,6 +26,7 @@ import {ScheduleProvider} from '../../calendar/schedule.provider';
 import moment from 'moment';
 import {SCIcon} from '../../../util/ion-icon/icon';
 import {profilePageSections} from './sections';
+import {filter, map} from 'rxjs/operators';

 const CourseCard = {
  collapsed: SCIcon`expand_more`,
@@ -46,11 +45,20 @@ interface MyCoursesTodayInterface {
  styleUrls: ['profile-page.scss'],
 })
 export class ProfilePageComponent implements OnInit {
-  data: {[key in SCAuthorizationProviderType]: {loggedIn: boolean}} = {
-    default: {loggedIn: false},
-    paia: {loggedIn: false},
+  data: {
+    [key in SCAuthorizationProviderType]: {loggedIn$: Observable<boolean>};
+  } = {
+    default: {loggedIn$: of(false)},
+    paia: {loggedIn$: of(false)},
  };

+  user$ = this.authHelper.getProvider('default').user$.pipe(
+    filter(user => typeof user !== 'undefined'),
+    map(userInfo => {
+      return this.authHelper.getUserFromUserInfo(userInfo as object);
+    }),
+  );
+
  sections = profilePageSections;

  logins: SCAuthorizationProviderType[] = [];
@@ -70,67 +78,24 @@ export class ProfilePageComponent implements OnInit {
  subscriptions: Subscription[] = [];

  constructor(
-    private requestor: Requestor,
    private authHelper: AuthHelperService,
    private route: ActivatedRoute,
    protected readonly scheduleProvider: ScheduleProvider,
  ) {}

  ngOnInit() {
+    this.data.default.loggedIn$ =
+      this.authHelper.getProvider('default').isAuthenticated$;
+    this.data.paia.loggedIn$ =
+      this.authHelper.getProvider('paia').isAuthenticated$;
+
    this.subscriptions.push(
-      this.authHelper.getProvider('default').token$.subscribe(_token => {
-        this.authHelper
-          .getProvider('default')
-          .getValidToken()
-          .then(token => {
-            this.data.default.loggedIn = true;
-            this.getUserInfo(token);
-          })
-          .catch(_error => {
-            this.data.default.loggedIn = false;
-          });
-      }),
-      this.authHelper.getProvider('paia').token$.subscribe(_token => {
-        this.authHelper
-          .getProvider('paia')
-          .getValidToken()
-          .then(_token => {
-            this.data.paia.loggedIn = true;
-          })
-          .catch(_error => {
-            this.data.paia.loggedIn = false;
-          });
-      }),
      this.route.queryParamMap.subscribe(queryParameters => {
        this.originPath = queryParameters.get('origin_path');
      }),
    );

    this.getMyCourses();
-
-    for (const dataKey in this.data) {
-      switch (dataKey) {
-        case 'default':
-          this.logins.push('default');
-          break;
-        case 'paia':
-          this.logins.push('paia');
-          break;
-      }
-    }
-  }
-
-  getUserInfo(token: TokenResponse) {
-    const userInfoHandler = new IonicUserInfoHandler(this.requestor);
-
-    userInfoHandler
-      .performUserInfoRequest(
-        this.authHelper.getProvider('default').localConfiguration,
-        token,
-      )
-      .then(userInfo => {
-        this.userInfo = this.authHelper.getUserFromUserInfo(userInfo);
-      });
  }

  async getMyCourses() {
@@ -184,4 +149,20 @@ export class ProfilePageComponent implements OnInit {
      ? await this.authHelper.setOriginPath(this.originPath)
      : await this.authHelper.deleteOriginPath();
  }
+
+  ionViewWillEnter() {
+    this.authHelper
+      .getProvider('default')
+      .getValidToken()
+      .then(() => void this.authHelper.getProvider('default').loadUserInfo())
+      .catch(() => {
+        // noop
+      });
+    this.authHelper
+      .getProvider('paia')
+      .getValidToken()
+      .catch(() => {
+        // noop
+      });
+  }
 }
--- a/src/app/modules/profile/page/profile-page.html
+++ b/src/app/modules/profile/page/profile-page.html
@@ -28,9 +28,9 @@
      <ion-card class="user-card">
        <ion-card-header>
          <ion-img src="assets/imgs/header.svg"></ion-img>
-          <span>
+          <span *ngIf="user$ | async as userInfo">
            {{
-              userInfo?.role
+              userInfo.role
                ? (userInfo?.role | titlecase)
                : ('profile.role_guest' | translate | titlecase)
            }}
@@ -45,35 +45,40 @@
            <ion-row>
              <ion-col size="3"></ion-col>
              <ion-col
-                *ngIf="data.default.loggedIn; else logInPrompt"
+                *ngIf="
+                  data.default.loggedIn$ | async as loggedIn;
+                  else logInPrompt
+                "
                size="9"
                class="main-info"
              >
-                <ion-text class="full-name">
-                  {{ userInfo?.name }}
-                </ion-text>
-                <div class="matriculation-number">
-                  <ion-label>
-                    {{ 'profile.userInfo.studentId' | translate | uppercase }}
-                  </ion-label>
-                  <ion-text>
-                    {{ userInfo?.studentId }}
+                <ng-container *ngIf="user$ | async as userInfo">
+                  <ion-text class="full-name">
+                    {{ userInfo?.name }}
                  </ion-text>
-                </div>
-                <div class="user-name">
-                  <ion-label>
-                    {{ 'profile.userInfo.username' | translate | uppercase }}
-                  </ion-label>
-                  <ion-text>{{ userInfo?.id }}</ion-text>
-                </div>
-                <div class="email">
-                  <ion-label>
-                    {{ 'profile.userInfo.email' | translate | uppercase }}
-                  </ion-label>
-                  <ion-text>
-                    {{ userInfo?.email }}
-                  </ion-text>
-                </div>
+                  <div class="matriculation-number">
+                    <ion-label>
+                      {{ 'profile.userInfo.studentId' | translate | uppercase }}
+                    </ion-label>
+                    <ion-text>
+                      {{ userInfo?.studentId }}
+                    </ion-text>
+                  </div>
+                  <div class="user-name">
+                    <ion-label>
+                      {{ 'profile.userInfo.username' | translate | uppercase }}
+                    </ion-label>
+                    <ion-text>{{ userInfo?.id }}</ion-text>
+                  </div>
+                  <div class="email">
+                    <ion-label>
+                      {{ 'profile.userInfo.email' | translate | uppercase }}
+                    </ion-label>
+                    <ion-text>
+                      {{ userInfo?.email }}
+                    </ion-text>
+                  </div>
+                </ng-container>
              </ion-col>
              <ng-template #logInPrompt>
                <ion-col size="9">