From a348c870103d43d38dfbe3460c15c364f0d5592f Mon Sep 17 00:00:00 2001 From: Masum ULU Date: Tue, 16 May 2023 17:30:08 +0300 Subject: [PATCH 1/6] Redirect to previous url after login Allowed --- .../oauth/src/lib/guards/oauth.guard.ts | 10 +++++-- .../lib/strategies/auth-code-flow-strategy.ts | 10 ++++++- .../src/lib/strategies/auth-flow-strategy.ts | 30 +++++++++++++++++-- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/npm/ng-packs/packages/oauth/src/lib/guards/oauth.guard.ts b/npm/ng-packs/packages/oauth/src/lib/guards/oauth.guard.ts index 357e861ba0..4b2d95af74 100644 --- a/npm/ng-packs/packages/oauth/src/lib/guards/oauth.guard.ts +++ b/npm/ng-packs/packages/oauth/src/lib/guards/oauth.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { CanActivate, UrlTree } from '@angular/router'; +import { CanActivate, UrlTree, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; import { OAuthService } from 'angular-oauth2-oidc'; import { Observable } from 'rxjs'; import { AuthService, IAuthGuard } from '@abp/ng.core'; @@ -10,13 +10,17 @@ import { AuthService, IAuthGuard } from '@abp/ng.core'; export class AbpOAuthGuard implements CanActivate, IAuthGuard { constructor(private oauthService: OAuthService, private authService: AuthService) {} - canActivate(): Observable | boolean | UrlTree { + canActivate( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot, + ): Observable | boolean | UrlTree { const hasValidAccessToken = this.oauthService.hasValidAccessToken(); if (hasValidAccessToken) { return true; } - this.authService.navigateToLogin(); + const params = { returnUrl: state.url }; + this.authService.navigateToLogin(params); return false; } } diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts index 946d1295ca..ab70febb3c 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts @@ -14,7 +14,10 @@ export class AuthCodeFlowStrategy extends AuthFlowStrategy { } navigateToLogin(queryParams?: Params) { - this.oAuthService.initCodeFlow('', this.getCultureParams(queryParams)); + this.oAuthService.initCodeFlow( + this.getAdditionalState(queryParams), + this.getCultureParams(queryParams), + ); } checkIfInternalAuth(queryParams?: Params) { @@ -31,6 +34,11 @@ export class AuthCodeFlowStrategy extends AuthFlowStrategy { return of(null); } + private getAdditionalState(queryParams?: Params): string { + const { returnUrl } = queryParams; + return returnUrl; + } + private getCultureParams(queryParams?: Params) { const lang = this.sessionState.getLanguage(); const culture = { culture: lang, 'ui-culture': lang }; diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts index 7a07cb8985..039565f8cf 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts @@ -1,5 +1,5 @@ import { Injector } from '@angular/core'; -import { Params } from '@angular/router'; +import { Params, Router } from '@angular/router'; import { AuthConfig, OAuthErrorEvent, @@ -7,7 +7,7 @@ import { OAuthStorage, } from 'angular-oauth2-oidc'; import { Observable, of } from 'rxjs'; -import { filter, switchMap, tap } from 'rxjs/operators'; +import { filter, map, switchMap, tap } from 'rxjs/operators'; import { ConfigStateService, EnvironmentService, @@ -30,6 +30,7 @@ export abstract class AuthFlowStrategy { protected oAuthConfig!: AuthConfig; protected sessionState: SessionStateService; protected tenantKey: string; + protected router: Router; abstract checkIfInternalAuth(queryParams?: Params): boolean; @@ -52,6 +53,7 @@ export abstract class AuthFlowStrategy { this.sessionState = injector.get(SessionStateService); this.oAuthConfig = this.environment.getEnvironment().oAuthConfig || {}; this.tenantKey = injector.get(TENANT_KEY); + this.router = injector.get(Router); this.listenToOauthErrors(); } @@ -68,6 +70,8 @@ export abstract class AuthFlowStrategy { .pipe(filter(event => event.type === 'token_refresh_error')) .subscribe(() => this.navigateToLogin()); + this.navigateToPreviousUrl(); + return this.oAuthService .loadDiscoveryDocument() .then(() => { @@ -80,6 +84,28 @@ export abstract class AuthFlowStrategy { .catch(this.catchError); } + protected navigateToPreviousUrl() { + this.oAuthService.events + .pipe( + filter(event => event.type === 'token_received' && !!this.oAuthService.state), + map(() => { + const redirect_uri = decodeURIComponent(this.oAuthService.state); + + if (redirect_uri && redirect_uri !== '/') { + return redirect_uri; + } + return '/'; + }), + switchMap(redirectUri => + this.configState.getOne$('currentUser').pipe( + filter(user => Boolean(user?.isAuthenticated)), + tap(() => this.router.navigate([redirectUri])), + ), + ), + ) + .subscribe(); + } + protected refreshToken() { return this.oAuthService.refreshToken().catch(() => clearOAuthStorage()); } From b44301bd65684ae4a9f63982a69eb78b116b0792 Mon Sep 17 00:00:00 2001 From: Masum ULU Date: Tue, 16 May 2023 22:13:58 +0300 Subject: [PATCH 2/6] navigateToLogin refactored --- .../src/lib/strategies/auth-code-flow-strategy.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts index ab70febb3c..184ef59dca 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-code-flow-strategy.ts @@ -14,10 +14,10 @@ export class AuthCodeFlowStrategy extends AuthFlowStrategy { } navigateToLogin(queryParams?: Params) { - this.oAuthService.initCodeFlow( - this.getAdditionalState(queryParams), - this.getCultureParams(queryParams), - ); + const additionalState = queryParams.returnUrl; + const cultureParams = this.getCultureParams(queryParams); + + this.oAuthService.initCodeFlow(additionalState, cultureParams); } checkIfInternalAuth(queryParams?: Params) { @@ -34,11 +34,6 @@ export class AuthCodeFlowStrategy extends AuthFlowStrategy { return of(null); } - private getAdditionalState(queryParams?: Params): string { - const { returnUrl } = queryParams; - return returnUrl; - } - private getCultureParams(queryParams?: Params) { const lang = this.sessionState.getLanguage(); const culture = { culture: lang, 'ui-culture': lang }; From 85d244d44aa44b9837c64294f9bad7c82da38c0f Mon Sep 17 00:00:00 2001 From: Masum ULU Date: Tue, 16 May 2023 23:10:11 +0300 Subject: [PATCH 3/6] fix build error & improve navigateToPreviousUrl method --- .../packages/identity/ng-package.json | 3 +- .../src/lib/strategies/auth-flow-strategy.ts | 44 +++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/npm/ng-packs/packages/identity/ng-package.json b/npm/ng-packs/packages/identity/ng-package.json index a0824f39e5..68f95303e4 100644 --- a/npm/ng-packs/packages/identity/ng-package.json +++ b/npm/ng-packs/packages/identity/ng-package.json @@ -6,6 +6,7 @@ }, "allowedNonPeerDependencies": [ "@abp/ng.theme.shared", - "@abp/ng.permission-management" + "@abp/ng.permission-management", + "@abp/ng.components" ] } diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts index 039565f8cf..9199fbf7cf 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts @@ -33,11 +33,8 @@ export abstract class AuthFlowStrategy { protected router: Router; abstract checkIfInternalAuth(queryParams?: Params): boolean; - abstract navigateToLogin(queryParams?: Params): void; - abstract logout(queryParams?: Params): Observable; - abstract login(params?: LoginParams | Params): Observable; private catchError = (err: HttpErrorResponse) => { @@ -84,26 +81,29 @@ export abstract class AuthFlowStrategy { .catch(this.catchError); } - protected navigateToPreviousUrl() { - this.oAuthService.events - .pipe( - filter(event => event.type === 'token_received' && !!this.oAuthService.state), - map(() => { - const redirect_uri = decodeURIComponent(this.oAuthService.state); - - if (redirect_uri && redirect_uri !== '/') { - return redirect_uri; - } - return '/'; - }), - switchMap(redirectUri => - this.configState.getOne$('currentUser').pipe( - filter(user => Boolean(user?.isAuthenticated)), - tap(() => this.router.navigate([redirectUri])), + protected navigateToPreviousUrl(): void { + const { responseType } = this.oAuthConfig; + if (responseType === 'code') { + this.oAuthService.events + .pipe( + filter(event => event.type === 'token_received' && !!this.oAuthService.state), + map(() => { + const redirect_uri = decodeURIComponent(this.oAuthService.state); + + if (redirect_uri && redirect_uri !== '/') { + return redirect_uri; + } + return '/'; + }), + switchMap(redirectUri => + this.configState.getOne$('currentUser').pipe( + filter(user => Boolean(user?.isAuthenticated)), + tap(() => this.router.navigate([redirectUri])), + ), ), - ), - ) - .subscribe(); + ) + .subscribe(); + } } protected refreshToken() { From 36d96d2401b962dd0bb8f277fe486fda2d4422ba Mon Sep 17 00:00:00 2001 From: Masum ULU Date: Wed, 17 May 2023 21:32:11 +0300 Subject: [PATCH 4/6] fix condition line for error component --- .../src/lib/handlers/error.handler.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/npm/ng-packs/packages/theme-shared/src/lib/handlers/error.handler.ts b/npm/ng-packs/packages/theme-shared/src/lib/handlers/error.handler.ts index 4f833fe0f1..adbbba4ddf 100644 --- a/npm/ng-packs/packages/theme-shared/src/lib/handlers/error.handler.ts +++ b/npm/ng-packs/packages/theme-shared/src/lib/handlers/error.handler.ts @@ -123,9 +123,11 @@ export class ErrorHandler { } private executeErrorHandler = (error: any) => { - const returnValue = this.httpErrorHandler(this.injector, error); + const errHandler = this.httpErrorHandler(this.injector, error); + const isObservable = errHandler instanceof Observable; + const response = isObservable ? errHandler : of(null); - return (returnValue instanceof Observable ? returnValue : of(null)).pipe( + return response.pipe( catchError(err => { this.handleError(err); return of(null); @@ -139,6 +141,11 @@ export class ErrorHandler { defaultValue: DEFAULT_ERROR_MESSAGES.defaultError.title, }; + if (err instanceof HttpErrorResponse && err.headers.get('Abp-Tenant-Resolve-Error')) { + this.authService.logout().subscribe(); + return; + } + if (err instanceof HttpErrorResponse && err.headers.get('_AbpErrorFormat')) { const confirmation$ = this.showErrorWithRequestBody(body); @@ -147,8 +154,6 @@ export class ErrorHandler { this.navigateToLogin(); }); } - } if(err instanceof HttpErrorResponse && err.headers.get('Abp-Tenant-Resolve-Error')){ - this.authService.logout().subscribe(); } else { switch (err.status) { case 401: @@ -178,7 +183,7 @@ export class ErrorHandler { status: 403, }); break; - case 404:{ + case 404: this.canCreateCustomError(404) ? this.show404Page() : this.showError( @@ -191,9 +196,7 @@ export class ErrorHandler { defaultValue: DEFAULT_ERROR_MESSAGES.defaultError404.title, }, ); - break; - } - + break; case 500: this.createErrorComponent({ title: { From 24d04f36481d2220a6adb74710bd7c7cf7623dbf Mon Sep 17 00:00:00 2001 From: Masum ULU <49063256+masumulu28@users.noreply.github.com> Date: Wed, 24 May 2023 10:56:16 +0300 Subject: [PATCH 5/6] Check style changed --- .../packages/oauth/src/lib/strategies/auth-flow-strategy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts index 9199fbf7cf..f326f71a21 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts @@ -97,7 +97,7 @@ export abstract class AuthFlowStrategy { }), switchMap(redirectUri => this.configState.getOne$('currentUser').pipe( - filter(user => Boolean(user?.isAuthenticated)), + filter(user => !!user?.isAuthenticated), tap(() => this.router.navigate([redirectUri])), ), ), From 30af7af840e75a6199718783052af76e9c1456c8 Mon Sep 17 00:00:00 2001 From: Masum ULU Date: Wed, 24 May 2023 11:18:33 +0300 Subject: [PATCH 6/6] fix memory leak --- .../packages/oauth/src/lib/strategies/auth-flow-strategy.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts index f326f71a21..59315449b0 100644 --- a/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts +++ b/npm/ng-packs/packages/oauth/src/lib/strategies/auth-flow-strategy.ts @@ -7,7 +7,7 @@ import { OAuthStorage, } from 'angular-oauth2-oidc'; import { Observable, of } from 'rxjs'; -import { filter, map, switchMap, tap } from 'rxjs/operators'; +import { filter, map, switchMap, take, tap } from 'rxjs/operators'; import { ConfigStateService, EnvironmentService, @@ -87,6 +87,7 @@ export abstract class AuthFlowStrategy { this.oAuthService.events .pipe( filter(event => event.type === 'token_received' && !!this.oAuthService.state), + take(1), map(() => { const redirect_uri = decodeURIComponent(this.oAuthService.state);