Skip to content

Commit 1b167bd

Browse files
committed
fix: Reverted interceptor changes (#3213)
* Reverted interceptor changes * Fixed merge conflict
1 parent a3a2080 commit 1b167bd

File tree

3 files changed

+143
-123
lines changed

3 files changed

+143
-123
lines changed

src/app/core/interceptors/httpInterceptor.spec.ts

Lines changed: 50 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -139,20 +139,11 @@ describe('HttpConfigInterceptor', () => {
139139
});
140140
});
141141

142-
describe('expiringSoon():', () => {
143-
it('should return true if token is expiring soon', () => {
144-
jwtHelperService.getExpirationDate.and.returnValue(new Date('2023-03-03T06:50:11.644Z'));
142+
it('expiringSoon(): should check if token is expiring soon', () => {
143+
jwtHelperService.getExpirationDate.and.returnValue(new Date('2023-03-03T06:50:11.644Z'));
145144

146-
const result = httpInterceptor.expiringSoon(authResData2.accessToken);
147-
expect(result).toBeTrue();
148-
});
149-
150-
it('should return true if an error occurs while checking token expiration', () => {
151-
jwtHelperService.getExpirationDate.and.throwError('Error in getting expiration date');
152-
153-
const result = httpInterceptor.expiringSoon(authResData2.accessToken);
154-
expect(result).toBeTrue();
155-
});
145+
const result = httpInterceptor.expiringSoon(authResData2.accessToken);
146+
expect(result).toBeTrue();
156147
});
157148

158149
describe('refreshAccessToken():', () => {
@@ -179,26 +170,13 @@ describe('HttpConfigInterceptor', () => {
179170
httpInterceptor.refreshAccessToken().subscribe({
180171
error: (err) => {
181172
expect(err).toBeTruthy();
182-
expect(userEventService.logout).not.toHaveBeenCalled();
183-
expect(secureStorageService.clearAll).not.toHaveBeenCalled();
184-
expect(storageService.clearAll).not.toHaveBeenCalled();
173+
expect(userEventService.logout).toHaveBeenCalledTimes(1);
174+
expect(secureStorageService.clearAll).toHaveBeenCalledTimes(1);
175+
expect(storageService.clearAll).toHaveBeenCalledTimes(1);
185176
done();
186177
},
187178
});
188179
});
189-
190-
it('should return null if refresh token is null or undefined', (done) => {
191-
tokenService.getRefreshToken.and.resolveTo(null); // Simulate a null refresh token
192-
193-
httpInterceptor.refreshAccessToken().subscribe((res) => {
194-
expect(res).toBeNull();
195-
expect(tokenService.getRefreshToken).toHaveBeenCalledTimes(1);
196-
expect(routerAuthService.fetchAccessToken).not.toHaveBeenCalled();
197-
expect(routerAuthService.newAccessToken).not.toHaveBeenCalled();
198-
expect(tokenService.getAccessToken).not.toHaveBeenCalled();
199-
done();
200-
});
201-
});
202180
});
203181

204182
describe('getAccessToken():', () => {
@@ -256,44 +234,67 @@ describe('HttpConfigInterceptor', () => {
256234
.intercept(new HttpRequest('GET', 'https://app.fylehq.com/'), { handle: () => of(null) })
257235
.subscribe((res) => {
258236
expect(res).toBeNull();
259-
expect(httpInterceptor.secureUrl).toHaveBeenCalledTimes(1);
237+
expect(httpInterceptor.secureUrl).toHaveBeenCalledTimes(2);
260238
expect(httpInterceptor.getAccessToken).toHaveBeenCalledTimes(1);
261239
expect(deviceService.getDeviceInfo).toHaveBeenCalledTimes(1);
262240
done();
263241
});
264242
});
265243

266-
it('should handle unauthorized error if no access token is available', (done) => {
267-
spyOn(httpInterceptor, 'secureUrl').and.returnValue(true);
268-
spyOn(httpInterceptor, 'getAccessToken').and.returnValue(of(null)); // Simulate no access token
244+
it('should refresh token if the next handler errors out', (done) => {
245+
spyOn(httpInterceptor, 'expiringSoon').and.returnValue(true);
246+
spyOn(httpInterceptor, 'refreshAccessToken').and.returnValue(of(authResData2.refresh_token));
247+
spyOn(httpInterceptor, 'getAccessToken').and.returnValue(of(authResData2.accessToken));
248+
deviceService.getDeviceInfo.and.returnValue(of(extendedDeviceInfoMockData));
269249

270250
httpInterceptor
271-
.intercept(new HttpRequest('GET', 'https://app.fylehq.com/'), { handle: () => of(null) })
251+
.intercept(new HttpRequest('GET', 'https://app.fylehq.com/'), {
252+
handle: () =>
253+
throwError(
254+
() =>
255+
new HttpErrorResponse({
256+
status: 200,
257+
})
258+
),
259+
})
272260
.subscribe({
273-
next: () => fail('Expected an error, but got a success response'),
274261
error: (err) => {
275-
expect(err.status).toBe(401);
276-
expect(err.error).toBe('Unauthorized');
277-
expect(httpInterceptor.secureUrl).toHaveBeenCalledTimes(1);
262+
expect(err).toBeTruthy();
263+
expect(httpInterceptor.expiringSoon).toHaveBeenCalledTimes(1);
264+
expect(httpInterceptor.refreshAccessToken).toHaveBeenCalledTimes(1);
278265
expect(httpInterceptor.getAccessToken).toHaveBeenCalledTimes(1);
266+
expect(deviceService.getDeviceInfo).toHaveBeenCalledTimes(1);
279267
done();
280268
},
281269
});
282270
});
283271

284-
it('should pass through requests for non-secure URLs', (done) => {
285-
spyOn(httpInterceptor, 'secureUrl').and.returnValue(false);
286-
287-
const nextHandleSpy = jasmine.createSpy().and.returnValue(of(null));
272+
it('should clear cache if the next handler error out but the token is not expiring soon', (done) => {
273+
spyOn(httpInterceptor, 'expiringSoon').and.returnValue(false);
274+
spyOn(httpInterceptor, 'getAccessToken').and.returnValue(of(authResData2.accessToken));
275+
deviceService.getDeviceInfo.and.returnValue(of(extendedDeviceInfoMockData));
288276

289277
httpInterceptor
290-
.intercept(new HttpRequest('GET', 'http://example.com'), { handle: nextHandleSpy })
291-
.subscribe((res) => {
292-
expect(res).toBeNull();
293-
expect(httpInterceptor.secureUrl).toHaveBeenCalledTimes(1);
294-
expect(nextHandleSpy).toHaveBeenCalled();
295-
done();
278+
.intercept(new HttpRequest('GET', 'https://app.fylehq.com/'), {
279+
handle: () =>
280+
throwError(
281+
() =>
282+
new HttpErrorResponse({
283+
status: 401,
284+
})
285+
),
286+
})
287+
.subscribe({
288+
error: (err) => {
289+
expect(err).toBeTruthy();
290+
expect(httpInterceptor.expiringSoon).toHaveBeenCalledTimes(1);
291+
expect(httpInterceptor.getAccessToken).toHaveBeenCalledTimes(1);
292+
expect(userEventService.logout).toHaveBeenCalledTimes(1);
293+
expect(secureStorageService.clearAll).toHaveBeenCalledTimes(1);
294+
expect(storageService.clearAll).toHaveBeenCalledTimes(1);
295+
},
296296
});
297+
done();
297298
});
298299

299300
it('should throw an error if the next handler returns a 404 and device information could be retrived', (done) => {
@@ -318,7 +319,7 @@ describe('HttpConfigInterceptor', () => {
318319
.subscribe({
319320
error: (err) => {
320321
expect(err).toBeTruthy();
321-
expect(httpInterceptor.expiringSoon).not.toHaveBeenCalled();
322+
expect(httpInterceptor.expiringSoon).toHaveBeenCalledTimes(1);
322323
expect(httpInterceptor.getAccessToken).toHaveBeenCalledTimes(1);
323324
},
324325
});

src/app/core/interceptors/httpInterceptor.ts

Lines changed: 92 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,33 @@
1-
/* eslint-disable @typescript-eslint/no-explicit-any */
21
import {
32
HttpErrorResponse,
43
HttpEvent,
54
HttpHandler,
65
HttpInterceptor,
76
HttpParameterCodec,
7+
HttpParams,
88
HttpRequest,
99
} from '@angular/common/http';
1010
import { Injectable } from '@angular/core';
11-
import { BehaviorSubject, Observable, from, of, throwError } from 'rxjs';
12-
import { catchError, filter, switchMap, take } from 'rxjs/operators';
13-
import * as dayjs from 'dayjs';
11+
12+
import { BehaviorSubject, Observable, forkJoin, from, iif, of, throwError } from 'rxjs';
13+
import { catchError, concatMap, filter, mergeMap, take } from 'rxjs/operators';
14+
1415
import { JwtHelperService } from '../services/jwt-helper.service';
16+
17+
import * as dayjs from 'dayjs';
1518
import { globalCacheBusterNotifier } from 'ts-cacheable';
1619
import { DeviceService } from '../services/device.service';
1720
import { RouterAuthService } from '../services/router-auth.service';
1821
import { SecureStorageService } from '../services/secure-storage.service';
1922
import { StorageService } from '../services/storage.service';
2023
import { TokenService } from '../services/token.service';
2124
import { UserEventService } from '../services/user-event.service';
25+
2226
@Injectable()
2327
export class HttpConfigInterceptor implements HttpInterceptor {
2428
public accessTokenCallInProgress = false;
2529

26-
public accessTokenSubject = new BehaviorSubject<string | null>(null);
30+
public accessTokenSubject = new BehaviorSubject<string>(null);
2731

2832
constructor(
2933
private jwtHelperService: JwtHelperService,
@@ -51,115 +55,130 @@ export class HttpConfigInterceptor implements HttpInterceptor {
5155
expiringSoon(accessToken: string): boolean {
5256
try {
5357
const expiryDate = dayjs(this.jwtHelperService.getExpirationDate(accessToken));
54-
const now = dayjs();
55-
const differenceSeconds = expiryDate.diff(now, 'seconds');
58+
const now = dayjs(new Date());
59+
const differenceSeconds = expiryDate.diff(now, 'second');
5660
const maxRefreshDifferenceSeconds = 2 * 60;
5761
return differenceSeconds < maxRefreshDifferenceSeconds;
5862
} catch (err) {
5963
return true;
6064
}
6165
}
6266

63-
refreshAccessToken(): Observable<string | null> {
67+
refreshAccessToken(): Observable<string> {
6468
return from(this.tokenService.getRefreshToken()).pipe(
65-
switchMap((refreshToken) => {
66-
if (refreshToken) {
67-
return from(this.routerAuthService.fetchAccessToken(refreshToken)).pipe(
68-
switchMap((authResponse) => from(this.routerAuthService.newAccessToken(authResponse.access_token))),
69-
switchMap(() => from(this.tokenService.getAccessToken())),
70-
catchError((error: HttpErrorResponse) => this.handleError(error)) // Handle refresh errors
71-
);
72-
} else {
73-
return of(null);
74-
}
75-
})
69+
concatMap((refreshToken) => this.routerAuthService.fetchAccessToken(refreshToken)),
70+
catchError((error) => {
71+
this.userEventService.logout();
72+
this.secureStorageService.clearAll();
73+
this.storageService.clearAll();
74+
globalCacheBusterNotifier.next();
75+
return throwError(error);
76+
}),
77+
concatMap((authResponse) => this.routerAuthService.newAccessToken(authResponse.access_token)),
78+
concatMap(() => from(this.tokenService.getAccessToken()))
7679
);
7780
}
7881

79-
handleError(error: HttpErrorResponse): Observable<never> {
80-
if (error.status === 401) {
81-
this.userEventService.logout();
82-
this.secureStorageService.clearAll();
83-
this.storageService.clearAll();
84-
globalCacheBusterNotifier.next();
85-
}
86-
return throwError(error); // Rethrow the error to the caller
87-
}
88-
8982
/**
9083
* This method get current accessToken from Storage, check if this token is expiring or not.
9184
* If the token is expiring it will get another accessToken from API and return the new accessToken
9285
* If multiple API call initiated then `this.accessTokenCallInProgress` will block multiple access_token call
9386
* Reference: https://stackoverflow.com/a/57638101
9487
*/
95-
getAccessToken(): Observable<string | null> {
88+
getAccessToken(): Observable<string> {
9689
return from(this.tokenService.getAccessToken()).pipe(
97-
switchMap((accessToken) => {
98-
if (accessToken && !this.expiringSoon(accessToken)) {
99-
return of(accessToken);
100-
}
101-
if (!this.accessTokenCallInProgress) {
102-
this.accessTokenCallInProgress = true;
103-
this.accessTokenSubject.next(null);
104-
return this.refreshAccessToken().pipe(
105-
switchMap((newAccessToken) => {
106-
this.accessTokenCallInProgress = false;
107-
this.accessTokenSubject.next(newAccessToken);
108-
return of(newAccessToken);
109-
})
110-
);
90+
concatMap((accessToken) => {
91+
if (this.expiringSoon(accessToken)) {
92+
if (!this.accessTokenCallInProgress) {
93+
this.accessTokenCallInProgress = true;
94+
this.accessTokenSubject.next(null);
95+
return this.refreshAccessToken().pipe(
96+
concatMap((newAccessToken) => {
97+
this.accessTokenCallInProgress = false;
98+
this.accessTokenSubject.next(newAccessToken);
99+
return of(newAccessToken);
100+
})
101+
);
102+
} else {
103+
return this.accessTokenSubject.pipe(
104+
filter((result) => result !== null),
105+
take(1),
106+
concatMap(() => from(this.tokenService.getAccessToken()))
107+
);
108+
}
111109
} else {
112-
// If a refresh is already in progress, wait for it to complete
113-
return this.accessTokenSubject.pipe(
114-
filter((result) => result !== null),
115-
take(1),
116-
switchMap(() => from(this.tokenService.getAccessToken()))
117-
);
110+
return of(accessToken);
118111
}
119112
})
120113
);
121114
}
122115

123116
getUrlWithoutQueryParam(url: string): string {
124-
// Remove query parameters from the URL
125-
return url.split('?')[0].split(';')[0].substring(0, 200);
126-
}
127-
128-
intercept(request: HttpRequest<string>, next: HttpHandler): Observable<HttpEvent<string>> {
129-
if (this.secureUrl(request.url)) {
130-
return this.getAccessToken().pipe(
131-
switchMap((accessToken) => {
132-
if (!accessToken) {
133-
return this.handleError({ status: 401, error: 'Unauthorized' } as HttpErrorResponse);
134-
}
135-
return this.executeHttpRequest(request, next, accessToken);
136-
})
137-
);
138-
} else {
139-
return next.handle(request);
117+
const queryIndex = Math.min(
118+
url.indexOf('?') !== -1 ? url.indexOf('?') : url.length,
119+
url.indexOf(';') !== -1 ? url.indexOf(';') : url.length
120+
);
121+
if (queryIndex !== url.length) {
122+
url = url.substring(0, queryIndex);
123+
}
124+
if (url.length > 200) {
125+
url = url.substring(0, 200);
140126
}
127+
return url;
141128
}
142129

143-
executeHttpRequest(request: HttpRequest<any>, next: HttpHandler, accessToken: string): Observable<HttpEvent<any>> {
144-
return from(this.deviceService.getDeviceInfo()).pipe(
145-
switchMap((deviceInfo) => {
130+
intercept(request: HttpRequest<string>, next: HttpHandler): Observable<HttpEvent<string>> {
131+
return forkJoin({
132+
token: iif(() => this.secureUrl(request.url), this.getAccessToken(), of(null)),
133+
deviceInfo: from(this.deviceService.getDeviceInfo()),
134+
}).pipe(
135+
concatMap(({ token, deviceInfo }) => {
136+
if (token && this.secureUrl(request.url)) {
137+
request = request.clone({ headers: request.headers.set('Authorization', 'Bearer ' + token) });
138+
const params = new HttpParams({ encoder: new CustomEncoder(), fromString: request.params.toString() });
139+
request = request.clone({ params });
140+
}
146141
const appVersion = deviceInfo.appVersion || '0.0.0';
147142
const osVersion = deviceInfo.osVersion;
148143
const operatingSystem = deviceInfo.operatingSystem;
149-
const mobileModifiedAppVersion = `fyle-mobile::${appVersion}::${operatingSystem}::${osVersion}`;
144+
const mobileModifiedappVersion = `fyle-mobile::${appVersion}::${operatingSystem}::${osVersion}`;
150145
request = request.clone({
151-
headers: request.headers.set('Authorization', `Bearer ${accessToken}`),
152146
setHeaders: {
153-
'X-App-Version': mobileModifiedAppVersion,
147+
'X-App-Version': mobileModifiedappVersion,
154148
'X-Page-Url': this.getUrlWithoutQueryParam(window.location.href),
155149
'X-Source-Identifier': 'mobile_app',
156150
},
157151
});
158-
return next.handle(request).pipe(catchError((error: HttpErrorResponse) => this.handleError(error)));
152+
153+
return next.handle(request).pipe(
154+
catchError((error) => {
155+
if (error instanceof HttpErrorResponse) {
156+
if (this.expiringSoon(token)) {
157+
return from(this.refreshAccessToken()).pipe(
158+
mergeMap((newToken) => {
159+
request = request.clone({ headers: request.headers.set('Authorization', 'Bearer ' + newToken) });
160+
return next.handle(request);
161+
})
162+
);
163+
} else if (
164+
(error.status === 404 && error.headers.get('X-Mobile-App-Blocked') === 'true') ||
165+
error.status === 401
166+
) {
167+
this.userEventService.logout();
168+
this.secureStorageService.clearAll();
169+
this.storageService.clearAll();
170+
globalCacheBusterNotifier.next();
171+
return throwError(error);
172+
}
173+
}
174+
return throwError(error);
175+
})
176+
);
159177
})
160178
);
161179
}
162180
}
181+
163182
export class CustomEncoder implements HttpParameterCodec {
164183
encodeKey(key: string): string {
165184
return encodeURIComponent(key);

src/app/fyle/personal-cards/personal-cards.page.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ export class PersonalCardsPage implements OnInit, AfterViewInit {
452452
if (this.selectionMode) {
453453
this.switchSelectionMode();
454454
}
455-
this.selectedTrasactionType = event.detail.value;
455+
this.selectedTrasactionType = event.detail.value as string;
456456
this.acc = [];
457457
const params = this.loadData$.getValue();
458458
const queryParams = params.queryParams || {};

0 commit comments

Comments
 (0)