From c75c4dce5b2f7d795d09d98f463d4dacbb5d2f66 Mon Sep 17 00:00:00 2001 From: "aditya.chandel" <8075870+adityachandelgit@users.noreply.github.com> Date: Fri, 15 Aug 2025 21:02:01 -0600 Subject: [PATCH] OIDC Authentication Revamp: Smarter Init, Error Handling, and Fallback --- .../security/DualJwtAuthenticationFilter.java | 2 +- .../config/security/SecurityConfig.java | 2 +- booklore-ui/src/app/auth-initializer.ts | 19 +++- .../core/component/login/login.component.html | 4 +- .../core/component/login/login.component.ts | 8 +- .../authentication-settings.component.ts | 8 +- .../app/core/service/app-settings.service.ts | 103 ++++++++++++++++-- .../src/app/core/service/auth.service.ts | 18 ++- .../src/app/public-app-settings.service.ts | 42 ------- 9 files changed, 133 insertions(+), 73 deletions(-) delete mode 100644 booklore-ui/src/app/public-app-settings.service.ts diff --git a/booklore-api/src/main/java/com/adityachandel/booklore/config/security/DualJwtAuthenticationFilter.java b/booklore-api/src/main/java/com/adityachandel/booklore/config/security/DualJwtAuthenticationFilter.java index 141b8db38..e2ad35a57 100644 --- a/booklore-api/src/main/java/com/adityachandel/booklore/config/security/DualJwtAuthenticationFilter.java +++ b/booklore-api/src/main/java/com/adityachandel/booklore/config/security/DualJwtAuthenticationFilter.java @@ -52,7 +52,7 @@ public class DualJwtAuthenticationFilter extends OncePerRequestFilter { String path = request.getRequestURI(); - if (path.startsWith("/api/v1/opds/") || path.equals("/api/v1/auth/refresh")) { + if (path.startsWith("/api/v1/opds/") || path.equals("/api/v1/auth/refresh") || path.equals("/api/v1/setup/status")) { chain.doFilter(request, response); return; } diff --git a/booklore-api/src/main/java/com/adityachandel/booklore/config/security/SecurityConfig.java b/booklore-api/src/main/java/com/adityachandel/booklore/config/security/SecurityConfig.java index b1ca05f9e..0abcdcd47 100644 --- a/booklore-api/src/main/java/com/adityachandel/booklore/config/security/SecurityConfig.java +++ b/booklore-api/src/main/java/com/adityachandel/booklore/config/security/SecurityConfig.java @@ -48,7 +48,7 @@ public class SecurityConfig { "/ws/**", "/api/v1/auth/**", "/api/v1/public-settings", - "/api/v1/setup/**", + "/api/v1/setup/status", "/api/v1/books/*/cover", "/api/v1/books/*/backup-cover", "/api/v1/opds/*/cover.jpg", diff --git a/booklore-ui/src/app/auth-initializer.ts b/booklore-ui/src/app/auth-initializer.ts index 4dc9d084a..24e9c3b0a 100644 --- a/booklore-ui/src/app/auth-initializer.ts +++ b/booklore-ui/src/app/auth-initializer.ts @@ -1,8 +1,8 @@ import {inject} from '@angular/core'; import {OAuthService} from 'angular-oauth2-oidc'; import {AuthService, websocketInitializer} from './core/service/auth.service'; +import {AppSettingsService} from './core/service/app-settings.service'; import {AuthInitializationService} from './auth-initialization-service'; -import {PublicAppSettingService} from './public-app-settings.service'; const OIDC_BYPASS_KEY = 'booklore-oidc-bypass'; const OIDC_ERROR_COUNT_KEY = 'booklore-oidc-error-count'; @@ -21,17 +21,22 @@ function withTimeout(promise: Promise, timeoutMs: number): Promise { export function initializeAuthFactory() { return () => { const oauthService = inject(OAuthService); - const publicAppSettingService = inject(PublicAppSettingService); + const appSettingsService = inject(AppSettingsService); const authService = inject(AuthService); const authInitService = inject(AuthInitializationService); return new Promise((resolve) => { - const sub = publicAppSettingService.publicAppSettings$.subscribe(publicSettings => { + const sub = appSettingsService.publicAppSettings$.subscribe(publicSettings => { if (publicSettings) { + const forceLocalOnly = new URLSearchParams(window.location.search).get('localOnly') === 'true'; const oidcBypassed = localStorage.getItem(OIDC_BYPASS_KEY) === 'true'; const errorCount = parseInt(localStorage.getItem(OIDC_ERROR_COUNT_KEY) || '0', 10); - if (publicSettings.oidcEnabled && publicSettings.oidcProviderDetails && !oidcBypassed && errorCount < MAX_OIDC_RETRIES) { + if (!forceLocalOnly && + publicSettings.oidcEnabled && + publicSettings.oidcProviderDetails && + !oidcBypassed && + errorCount < MAX_OIDC_RETRIES) { const details = publicSettings.oidcProviderDetails; oauthService.configure({ @@ -53,7 +58,7 @@ export function initializeAuthFactory() { localStorage.removeItem(OIDC_ERROR_COUNT_KEY); if (oauthService.hasValidAccessToken()) { - authService.tokenSubject.next(oauthService.getAccessToken()) + authService.tokenSubject.next(oauthService.getAccessToken()); console.log('[OIDC] Valid access token found after tryLogin'); oauthService.setupAutomaticSilentRefresh(); websocketInitializer(authService); @@ -90,7 +95,9 @@ export function initializeAuthFactory() { resolve(); }); } else { - if (oidcBypassed) { + if (forceLocalOnly) { + console.warn('[OIDC] Forced local-only login via ?localOnly=true'); + } else if (oidcBypassed) { console.log('[OIDC] OIDC is manually bypassed, using local authentication only'); } else if (errorCount >= MAX_OIDC_RETRIES) { console.log(`[OIDC] OIDC automatically bypassed due to ${errorCount} consecutive errors`); diff --git a/booklore-ui/src/app/core/component/login/login.component.html b/booklore-ui/src/app/core/component/login/login.component.html index 5ed2a1c00..ca8d4ab5b 100644 --- a/booklore-ui/src/app/core/component/login/login.component.html +++ b/booklore-ui/src/app/core/component/login/login.component.html @@ -2,7 +2,7 @@ style="background: linear-gradient(60deg, var(--primary-color) 0%, #1e3a8a 100%);">
-
+
@@ -72,6 +73,7 @@ [toggleMask]="true" class="mb-4" [fluid]="true" + autocomplete="current-password" >
diff --git a/booklore-ui/src/app/core/component/login/login.component.ts b/booklore-ui/src/app/core/component/login/login.component.ts index 7fbd5c1f1..ff501bcee 100644 --- a/booklore-ui/src/app/core/component/login/login.component.ts +++ b/booklore-ui/src/app/core/component/login/login.component.ts @@ -1,4 +1,4 @@ -import {Component, inject, OnInit, OnDestroy} from '@angular/core'; +import {Component, inject, OnDestroy, OnInit} from '@angular/core'; import {AuthService} from '../../service/auth.service'; import {Router} from '@angular/router'; import {FormsModule} from '@angular/forms'; @@ -8,9 +8,9 @@ import {Message} from 'primeng/message'; import {InputText} from 'primeng/inputtext'; import {OAuthService} from 'angular-oauth2-oidc'; import {Observable, Subject} from 'rxjs'; -import {filter, take, takeUntil} from 'rxjs/operators'; -import {PublicAppSettings, PublicAppSettingService} from '../../../public-app-settings.service'; +import {filter, take} from 'rxjs/operators'; import {getOidcErrorCount, isOidcBypassed, resetOidcBypass} from '../../../auth-initializer'; +import {AppSettingsService, PublicAppSettings} from '../../service/app-settings.service'; @Component({ selector: 'app-login', @@ -37,7 +37,7 @@ export class LoginComponent implements OnInit, OnDestroy { private authService = inject(AuthService); private oAuthService = inject(OAuthService); - private appSettingsService = inject(PublicAppSettingService); + private appSettingsService = inject(AppSettingsService); private router = inject(Router); publicAppSettings$: Observable = this.appSettingsService.publicAppSettings$; diff --git a/booklore-ui/src/app/core/security/oauth2-management/authentication-settings.component.ts b/booklore-ui/src/app/core/security/oauth2-management/authentication-settings.component.ts index 4ce2ceb7f..9f75f7e8d 100644 --- a/booklore-ui/src/app/core/security/oauth2-management/authentication-settings.component.ts +++ b/booklore-ui/src/app/core/security/oauth2-management/authentication-settings.component.ts @@ -113,13 +113,7 @@ export class AuthenticationSettingsComponent implements OnInit { toggleOidcEnabled(): void { if (!this.isOidcFormComplete()) return; - const payload = [ - { - key: AppSettingKey.OIDC_ENABLED, - newValue: this.oidcEnabled - } - ]; - this.appSettingsService.saveSettings(payload).subscribe({ + this.appSettingsService.toggleOidcEnabled(this.oidcEnabled).subscribe({ next: () => this.messageService.add({ severity: 'success', summary: 'Saved', diff --git a/booklore-ui/src/app/core/service/app-settings.service.ts b/booklore-ui/src/app/core/service/app-settings.service.ts index 91dfd7bbf..853c0871e 100644 --- a/booklore-ui/src/app/core/service/app-settings.service.ts +++ b/booklore-ui/src/app/core/service/app-settings.service.ts @@ -1,17 +1,27 @@ -import {inject, Injectable} from '@angular/core'; +import {inject, Injectable, Injector} from '@angular/core'; import {HttpClient} from '@angular/common/http'; import {BehaviorSubject, Observable, of} from 'rxjs'; -import {AppSettings} from '../model/app-settings.model'; -import {API_CONFIG} from '../../config/api-config'; import {catchError, finalize, shareReplay, tap} from 'rxjs/operators'; +import {API_CONFIG} from '../../config/api-config'; +import {AppSettings, OidcProviderDetails} from '../model/app-settings.model'; +import {AuthService} from './auth.service'; + +export interface PublicAppSettings { + oidcEnabled: boolean; + oidcProviderDetails: OidcProviderDetails; +} @Injectable({providedIn: 'root'}) export class AppSettingsService { private http = inject(HttpClient); + private injector = inject(Injector); + private readonly apiUrl = `${API_CONFIG.BASE_URL}/api/v1/settings`; + private readonly publicApiUrl = `${API_CONFIG.BASE_URL}/api/v1/public-settings`; private loading$: Observable | null = null; private appSettingsSubject = new BehaviorSubject(null); + appSettings$ = this.appSettingsSubject.asObservable().pipe( tap(state => { if (!state && !this.loading$) { @@ -24,9 +34,27 @@ export class AppSettingsService { }) ); + private publicLoading$: Observable | null = null; + private publicAppSettingsSubject = new BehaviorSubject(null); + + publicAppSettings$ = this.publicAppSettingsSubject.asObservable().pipe( + tap(state => { + if (!state && !this.publicLoading$) { + this.publicLoading$ = this.fetchPublicSettings().pipe( + shareReplay(1), + finalize(() => (this.publicLoading$ = null)) + ); + this.publicLoading$.subscribe(); + } + }) + ); + private fetchAppSettings(): Observable { return this.http.get(this.apiUrl).pipe( - tap(settings => this.appSettingsSubject.next(settings)), + tap(settings => { + this.appSettingsSubject.next(settings); + this.syncPublicSettings(settings); + }), catchError(err => { console.error('Error loading app settings:', err); this.appSettingsSubject.next(null); @@ -35,6 +63,32 @@ export class AppSettingsService { ); } + private fetchPublicSettings(): Observable { + return this.http.get(this.publicApiUrl).pipe( + tap(settings => this.publicAppSettingsSubject.next(settings)), + catchError(err => { + console.error('Failed to fetch public settings', err); + throw err; + }) + ); + } + + private syncPublicSettings(appSettings: AppSettings): void { + const updatedPublicSettings: PublicAppSettings = { + oidcEnabled: appSettings.oidcEnabled, + oidcProviderDetails: appSettings.oidcProviderDetails + }; + const current = this.publicAppSettingsSubject.value; + + if ( + !current || + current.oidcEnabled !== updatedPublicSettings.oidcEnabled || + JSON.stringify(current.oidcProviderDetails) !== JSON.stringify(updatedPublicSettings.oidcProviderDetails) + ) { + this.publicAppSettingsSubject.next(updatedPublicSettings); + } + } + saveSettings(settings: { key: string; newValue: any }[]): Observable { const payload = settings.map(setting => ({ name: setting.key, @@ -43,11 +97,18 @@ export class AppSettingsService { return this.http.put(this.apiUrl, payload).pipe( tap(() => { - this.loading$ = this.fetchAppSettings().pipe( - shareReplay(1), - finalize(() => (this.loading$ = null)) - ); - this.loading$.subscribe(); + const current = this.appSettingsSubject.value; + if (current) { + settings.forEach(s => (current as any)[s.key] = s.newValue); + this.appSettingsSubject.next({...current}); + this.syncPublicSettings(current); + } else { + this.loading$ = this.fetchAppSettings().pipe( + shareReplay(1), + finalize(() => (this.loading$ = null)) + ); + this.loading$.subscribe(); + } }), catchError(err => { console.error('Error saving settings:', err); @@ -55,4 +116,28 @@ export class AppSettingsService { }) ); } + + toggleOidcEnabled(enabled: boolean): Observable { + const payload = [{name: 'OIDC_ENABLED', value: enabled}]; + return this.http.put(this.apiUrl, payload).pipe( + tap(() => { + const current = this.appSettingsSubject.value; + if (current) { + current.oidcEnabled = enabled; + this.appSettingsSubject.next({...current}); + this.syncPublicSettings(current); + } + if (!enabled) { + setTimeout(() => { + const authService = this.injector.get(AuthService); + authService.clearOIDCTokens(); + }); + } + }), + catchError(err => { + console.error('Error toggling OIDC:', err); + return of(); + }) + ); + } } diff --git a/booklore-ui/src/app/core/service/auth.service.ts b/booklore-ui/src/app/core/service/auth.service.ts index a7adc243b..f7f593ce8 100644 --- a/booklore-ui/src/app/core/service/auth.service.ts +++ b/booklore-ui/src/app/core/service/auth.service.ts @@ -1,10 +1,10 @@ import {inject, Injectable, Injector} from '@angular/core'; import {HttpClient} from '@angular/common/http'; -import {BehaviorSubject, Observable, Subject, tap} from 'rxjs'; +import {BehaviorSubject, Observable, tap} from 'rxjs'; import {RxStompService} from '../../shared/websocket/rx-stomp.service'; import {API_CONFIG} from '../../config/api-config'; import {createRxStompConfig} from '../../shared/websocket/rx-stomp.config'; -import {OAuthService} from 'angular-oauth2-oidc'; +import {OAuthService, OAuthStorage} from 'angular-oauth2-oidc'; import {Router} from '@angular/router'; @Injectable({ @@ -18,6 +18,7 @@ export class AuthService { private http = inject(HttpClient); private injector = inject(Injector); private oAuthService = inject(OAuthService); + private oAuthStorage = inject(OAuthStorage); private router = inject(Router); public tokenSubject = new BehaviorSubject(this.getOidcAccessToken() || this.getInternalAccessToken()); @@ -64,9 +65,22 @@ export class AuthService { return localStorage.getItem('refreshToken_Internal'); } + clearOIDCTokens(): void { + const hasInternalTokens = this.getInternalAccessToken() || this.getInternalRefreshToken(); + if (!hasInternalTokens) { + this.oAuthStorage.removeItem("access_token"); + this.oAuthStorage.removeItem("refresh_token"); + this.oAuthStorage.removeItem("id_token"); + this.router.navigate(['/login']); + } + } + logout(): void { localStorage.removeItem('accessToken_Internal'); localStorage.removeItem('refreshToken_Internal'); + this.oAuthStorage.removeItem("access_token"); + this.oAuthStorage.removeItem("refresh_token"); + this.oAuthStorage.removeItem("id_token"); this.tokenSubject.next(null); this.getRxStompService().deactivate(); this.router.navigate(['/login']); diff --git a/booklore-ui/src/app/public-app-settings.service.ts b/booklore-ui/src/app/public-app-settings.service.ts deleted file mode 100644 index b1b40eba3..000000000 --- a/booklore-ui/src/app/public-app-settings.service.ts +++ /dev/null @@ -1,42 +0,0 @@ -import {inject, Injectable} from '@angular/core'; -import {API_CONFIG} from './config/api-config'; -import {BehaviorSubject, Observable} from 'rxjs'; -import {HttpClient} from '@angular/common/http'; -import {OidcProviderDetails} from './core/model/app-settings.model'; -import {catchError, finalize, shareReplay, tap} from 'rxjs/operators'; - -export interface PublicAppSettings { - oidcEnabled: boolean; - oidcProviderDetails: OidcProviderDetails; -} - -@Injectable({providedIn: 'root'}) -export class PublicAppSettingService { - private http = inject(HttpClient); - private readonly url = `${API_CONFIG.BASE_URL}/api/v1/public-settings`; - - private loading$: Observable | null = null; - private publicAppSettingsSubject = new BehaviorSubject(null); - - publicAppSettings$ = this.publicAppSettingsSubject.asObservable().pipe( - tap(state => { - if (!state && !this.loading$) { - this.loading$ = this.fetchPublicSettings().pipe( - shareReplay(1), - finalize(() => (this.loading$ = null)) - ); - this.loading$.subscribe(); - } - }) - ); - - private fetchPublicSettings(): Observable { - return this.http.get(this.url).pipe( - tap(settings => this.publicAppSettingsSubject.next(settings)), - catchError(err => { - console.error('Failed to fetch public settings', err); - throw err; - }) - ); - } -}