Skip to content

Commit b0f5b12

Browse files
damienboddamienbod
authored
Merge pull request #1483 from Tom-V/tomv/fix-concurrent-renew-with-authorize-1479
fix concurrent issue with renew and normal code flow
2 parents 9457024 + 25e3d15 commit b0f5b12

10 files changed

+296
-168
lines changed

‎projects/angular-auth-oidc-client/src/lib/callback/code-flow-callback.service.spec.ts‎

Lines changed: 101 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('CodeFlowCallbackService ', () => {
3939
expect(codeFlowCallbackService).toBeTruthy();
4040
});
4141

42-
describe('authorizedCallbackWithCode', () => {
42+
describe('authenticatedCallbackWithCode', () => {
4343
it('calls flowsService.processCodeFlowCallback with correct url', () => {
4444
const spy = spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(of(null));
4545
//spyOn(configurationProvider, 'getOpenIDConfiguration').and.returnValue({ triggerAuthorizationResultEvent: true });
@@ -53,111 +53,105 @@ describe('CodeFlowCallbackService ', () => {
5353
expect(spy).toHaveBeenCalledOnceWith('some-url1', config, [config]);
5454
});
5555

56-
it(
57-
'does nothing if triggerAuthorizationResultEvent is true and isRenewProcess is true',
58-
waitForAsync(() => {
59-
const callbackContext = {
60-
code: '',
61-
refreshToken: '',
62-
state: '',
63-
sessionState: null,
64-
authResult: null,
65-
isRenewProcess: true,
66-
jwtKeys: null,
67-
validationResult: null,
68-
existingIdToken: '',
69-
};
70-
const spy = spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(of(callbackContext));
71-
const routerSpy = spyOn(router, 'navigateByUrl');
72-
const config = {
73-
configId: 'configId1',
74-
triggerAuthorizationResultEvent: true,
75-
};
76-
77-
codeFlowCallbackService.authenticatedCallbackWithCode('some-url2', config, [config]).subscribe(() => {
78-
expect(spy).toHaveBeenCalledOnceWith('some-url2', config, [config]);
79-
expect(routerSpy).not.toHaveBeenCalled();
80-
});
81-
})
82-
);
83-
84-
it(
85-
'calls router if triggerAuthorizationResultEvent is false and isRenewProcess is false',
86-
waitForAsync(() => {
87-
const callbackContext = {
88-
code: '',
89-
refreshToken: '',
90-
state: '',
91-
sessionState: null,
92-
authResult: null,
93-
isRenewProcess: false,
94-
jwtKeys: null,
95-
validationResult: null,
96-
existingIdToken: '',
97-
};
98-
const spy = spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(of(callbackContext));
99-
const routerSpy = spyOn(router, 'navigateByUrl');
100-
const config = {
101-
configId: 'configId1',
102-
triggerAuthorizationResultEvent: false,
103-
postLoginRoute: 'postLoginRoute',
104-
};
105-
106-
codeFlowCallbackService.authenticatedCallbackWithCode('some-url3', config, [config]).subscribe(() => {
107-
expect(spy).toHaveBeenCalledOnceWith('some-url3', config, [config]);
108-
expect(routerSpy).toHaveBeenCalledOnceWith('postLoginRoute');
109-
});
110-
})
111-
);
112-
113-
it(
114-
'resetSilentRenewRunning and stopPeriodicallTokenCheck in case of error',
115-
waitForAsync(() => {
116-
spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(throwError(() => new Error('error')));
117-
const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');
118-
const stopPeriodicallTokenCheckSpy = spyOn(intervalService, 'stopPeriodicTokenCheck');
119-
120-
const config = {
121-
configId: 'configId1',
122-
triggerAuthorizationResultEvent: false,
123-
postLoginRoute: 'postLoginRoute',
124-
};
125-
126-
codeFlowCallbackService.authenticatedCallbackWithCode('some-url4', config, [config]).subscribe({
127-
error: (err) => {
128-
expect(resetSilentRenewRunningSpy).toHaveBeenCalled();
129-
expect(stopPeriodicallTokenCheckSpy).toHaveBeenCalled();
130-
expect(err).toBeTruthy();
131-
},
132-
});
133-
})
134-
);
135-
136-
it(
137-
`navigates to unauthorizedRoute in case of error and in case of error and
138-
triggerAuthorizationResultEvent is false`,
139-
waitForAsync(() => {
140-
spyOn(flowsDataService, 'isSilentRenewRunning').and.returnValue(false);
141-
spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(throwError(() => new Error('error')));
142-
const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');
143-
const stopPeriodicallTokenCheckSpy = spyOn(intervalService, 'stopPeriodicTokenCheck');
144-
const routerSpy = spyOn(router, 'navigateByUrl');
145-
146-
const config = {
147-
configId: 'configId1',
148-
triggerAuthorizationResultEvent: false,
149-
unauthorizedRoute: 'unauthorizedRoute',
150-
};
151-
152-
codeFlowCallbackService.authenticatedCallbackWithCode('some-url5', config, [config]).subscribe({
153-
error: (err) => {
154-
expect(resetSilentRenewRunningSpy).toHaveBeenCalled();
155-
expect(stopPeriodicallTokenCheckSpy).toHaveBeenCalled();
156-
expect(err).toBeTruthy();
157-
expect(routerSpy).toHaveBeenCalledOnceWith('unauthorizedRoute');
158-
},
159-
});
160-
})
161-
);
56+
it('does only call resetCodeFlowInProgress if triggerAuthorizationResultEvent is true and isRenewProcess is true', waitForAsync(() => {
57+
const callbackContext = {
58+
code: '',
59+
refreshToken: '',
60+
state: '',
61+
sessionState: null,
62+
authResult: null,
63+
isRenewProcess: true,
64+
jwtKeys: null,
65+
validationResult: null,
66+
existingIdToken: '',
67+
};
68+
const spy = spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(of(callbackContext));
69+
const flowsDataSpy = spyOn(flowsDataService, 'resetCodeFlowInProgress');
70+
const routerSpy = spyOn(router, 'navigateByUrl');
71+
const config = {
72+
configId: 'configId1',
73+
triggerAuthorizationResultEvent: true,
74+
};
75+
76+
codeFlowCallbackService.authenticatedCallbackWithCode('some-url2', config, [config]).subscribe(() => {
77+
expect(spy).toHaveBeenCalledOnceWith('some-url2', config, [config]);
78+
expect(routerSpy).not.toHaveBeenCalled();
79+
expect(flowsDataSpy).toHaveBeenCalled();
80+
});
81+
}));
82+
83+
it('calls router and resetCodeFlowInProgress if triggerAuthorizationResultEvent is false and isRenewProcess is false', waitForAsync(() => {
84+
const callbackContext = {
85+
code: '',
86+
refreshToken: '',
87+
state: '',
88+
sessionState: null,
89+
authResult: null,
90+
isRenewProcess: false,
91+
jwtKeys: null,
92+
validationResult: null,
93+
existingIdToken: '',
94+
};
95+
const spy = spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(of(callbackContext));
96+
const flowsDataSpy = spyOn(flowsDataService, 'resetCodeFlowInProgress');
97+
const routerSpy = spyOn(router, 'navigateByUrl');
98+
const config = {
99+
configId: 'configId1',
100+
triggerAuthorizationResultEvent: false,
101+
postLoginRoute: 'postLoginRoute',
102+
};
103+
104+
codeFlowCallbackService.authenticatedCallbackWithCode('some-url3', config, [config]).subscribe(() => {
105+
expect(spy).toHaveBeenCalledOnceWith('some-url3', config, [config]);
106+
expect(routerSpy).toHaveBeenCalledOnceWith('postLoginRoute');
107+
expect(flowsDataSpy).toHaveBeenCalled();
108+
});
109+
}));
110+
111+
it('resetSilentRenewRunning, resetCodeFlowInProgress and stopPeriodicallTokenCheck in case of error', waitForAsync(() => {
112+
spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(throwError(() => new Error('error')));
113+
const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');
114+
const resetCodeFlowInProgressSpy = spyOn(flowsDataService, 'resetCodeFlowInProgress');
115+
const stopPeriodicallTokenCheckSpy = spyOn(intervalService, 'stopPeriodicTokenCheck');
116+
117+
const config = {
118+
configId: 'configId1',
119+
triggerAuthorizationResultEvent: false,
120+
postLoginRoute: 'postLoginRoute',
121+
};
122+
123+
codeFlowCallbackService.authenticatedCallbackWithCode('some-url4', config, [config]).subscribe({
124+
error: (err) => {
125+
expect(resetSilentRenewRunningSpy).toHaveBeenCalled();
126+
expect(resetCodeFlowInProgressSpy).toHaveBeenCalled();
127+
expect(stopPeriodicallTokenCheckSpy).toHaveBeenCalled();
128+
expect(err).toBeTruthy();
129+
},
130+
});
131+
}));
132+
133+
it(`navigates to unauthorizedRoute in case of error and in case of error and
134+
triggerAuthorizationResultEvent is false`, waitForAsync(() => {
135+
spyOn(flowsDataService, 'isSilentRenewRunning').and.returnValue(false);
136+
spyOn(flowsService, 'processCodeFlowCallback').and.returnValue(throwError(() => new Error('error')));
137+
const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');
138+
const stopPeriodicallTokenCheckSpy = spyOn(intervalService, 'stopPeriodicTokenCheck');
139+
const routerSpy = spyOn(router, 'navigateByUrl');
140+
141+
const config = {
142+
configId: 'configId1',
143+
triggerAuthorizationResultEvent: false,
144+
unauthorizedRoute: 'unauthorizedRoute',
145+
};
146+
147+
codeFlowCallbackService.authenticatedCallbackWithCode('some-url5', config, [config]).subscribe({
148+
error: (err) => {
149+
expect(resetSilentRenewRunningSpy).toHaveBeenCalled();
150+
expect(stopPeriodicallTokenCheckSpy).toHaveBeenCalled();
151+
expect(err).toBeTruthy();
152+
expect(routerSpy).toHaveBeenCalledOnceWith('unauthorizedRoute');
153+
},
154+
});
155+
}));
162156
});
163157
});

‎projects/angular-auth-oidc-client/src/lib/callback/code-flow-callback.service.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ export class CodeFlowCallbackService {
2727

2828
return this.flowsService.processCodeFlowCallback(urlToCheck, config, allConfigs).pipe(
2929
tap((callbackContext) => {
30+
this.flowsDataService.resetCodeFlowInProgress(config);
3031
if (!triggerAuthorizationResultEvent && !callbackContext.isRenewProcess) {
3132
this.router.navigateByUrl(postLoginRoute);
3233
}
3334
}),
3435
catchError((error) => {
3536
this.flowsDataService.resetSilentRenewRunning(config);
37+
this.flowsDataService.resetCodeFlowInProgress(config);
3638
this.intervalService.stopPeriodicTokenCheck();
3739
if (!triggerAuthorizationResultEvent && !isRenewProcess) {
3840
this.router.navigateByUrl(unauthorizedRoute);

‎projects/angular-auth-oidc-client/src/lib/callback/periodically-token-check.service.spec.ts‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,32 +77,26 @@ describe('PeriodicallyTokenCheckService', () => {
7777
});
7878

7979
describe('startTokenValidationPeriodically', () => {
80-
it(
81-
'returns if no config has silentrenew enabled',
82-
waitForAsync(() => {
83-
const configs = [
84-
{ silentRenew: false, configId: 'configId1' },
85-
{ silentRenew: false, configId: 'configId2' },
86-
];
80+
it('returns if no config has silentrenew enabled', waitForAsync(() => {
81+
const configs = [
82+
{ silentRenew: false, configId: 'configId1' },
83+
{ silentRenew: false, configId: 'configId2' },
84+
];
8785

88-
const result = periodicallyTokenCheckService.startTokenValidationPeriodically(configs, configs[0]);
86+
const result = periodicallyTokenCheckService.startTokenValidationPeriodically(configs, configs[0]);
8987

90-
expect(result).toBeUndefined();
91-
})
92-
);
88+
expect(result).toBeUndefined();
89+
}));
9390

94-
it(
95-
'returns if runTokenValidationRunning',
96-
waitForAsync(() => {
97-
const configs = [{ silentRenew: true, configId: 'configId1' }];
91+
it('returns if runTokenValidationRunning', waitForAsync(() => {
92+
const configs = [{ silentRenew: true, configId: 'configId1' }];
9893

99-
spyOn(intervalService, 'isTokenValidationRunning').and.returnValue(true);
94+
spyOn(intervalService, 'isTokenValidationRunning').and.returnValue(true);
10095

101-
const result = periodicallyTokenCheckService.startTokenValidationPeriodically(configs, configs[0]);
96+
const result = periodicallyTokenCheckService.startTokenValidationPeriodically(configs, configs[0]);
10297

103-
expect(result).toBeUndefined();
104-
})
105-
);
98+
expect(result).toBeUndefined();
99+
}));
106100

107101
it('interval calls resetSilentRenewRunning when current flow is CodeFlowWithRefreshTokens', fakeAsync(() => {
108102
const configs = [{ silentRenew: true, configId: 'configId1', tokenRefreshInSeconds: 1 }];
@@ -201,6 +195,17 @@ describe('PeriodicallyTokenCheckService', () => {
201195
expect(result).toBeFalse();
202196
});
203197

198+
it('returns false when code flow is in progress', () => {
199+
spyOn(authStateService, 'getIdToken').and.returnValue('idToken');
200+
spyOn(flowsDataService, 'isSilentRenewRunning').and.returnValue(false);
201+
spyOn(flowsDataService, 'isCodeFlowInProgress').and.returnValue(true);
202+
spyOn(userService, 'getUserDataFromStore').and.returnValue('some-userdata');
203+
204+
const result = (periodicallyTokenCheckService as any).shouldStartPeriodicallyCheckForConfig({ configId: 'configId1' });
205+
206+
expect(result).toBeFalse();
207+
});
208+
204209
it('returns false when there is no userdata from the store', () => {
205210
spyOn(authStateService, 'getIdToken').and.returnValue('idToken');
206211
spyOn(flowsDataService, 'isSilentRenewRunning').and.returnValue(true);

‎projects/angular-auth-oidc-client/src/lib/callback/periodically-token-check.service.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,15 @@ export class PeriodicallyTokenCheckService {
154154
private shouldStartPeriodicallyCheckForConfig(config: OpenIdConfiguration): boolean {
155155
const idToken = this.authStateService.getIdToken(config);
156156
const isSilentRenewRunning = this.flowsDataService.isSilentRenewRunning(config);
157+
const isCodeFlowinProgress = this.flowsDataService.isCodeFlowInProgress(config);
157158
const userDataFromStore = this.userService.getUserDataFromStore(config);
158159

159160
this.loggerService.logDebug(
160161
config,
161-
`Checking: silentRenewRunning: ${isSilentRenewRunning} - has idToken: ${!!idToken} - has userData: ${!!userDataFromStore}`
162+
`Checking: silentRenewRunning: ${isSilentRenewRunning}, isCodeFlowInProgress: ${isCodeFlowinProgress} - has idToken: ${!!idToken} - has userData: ${!!userDataFromStore}`
162163
);
163164

164-
const shouldBeExecuted = !!userDataFromStore && !isSilentRenewRunning && !!idToken;
165+
const shouldBeExecuted = !!userDataFromStore && !isSilentRenewRunning && !!idToken && !isCodeFlowinProgress;
165166

166167
if (!shouldBeExecuted) {
167168
return false;

‎projects/angular-auth-oidc-client/src/lib/flows/flows-data.service.spec.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,63 @@ describe('Flows Data Service', () => {
131131
});
132132
});
133133

134+
describe('isCodeFlowInProgress', () => {
135+
it('checks code flow is in progress and returns result', () => {
136+
const config = {
137+
configId: 'configId1',
138+
};
139+
140+
jasmine.clock().uninstall();
141+
jasmine.clock().install();
142+
const baseTime = new Date();
143+
jasmine.clock().mockDate(baseTime);
144+
145+
const storageObject = {
146+
state: 'in progress',
147+
};
148+
149+
spyOn(storagePersistenceService, 'read').withArgs('storageCodeFlowInProgress', config).and.returnValue(JSON.stringify(storageObject));
150+
const spyWrite = spyOn(storagePersistenceService, 'write');
151+
152+
const isCodeFlowInProgressResult = service.isCodeFlowInProgress(config);
153+
154+
expect(spyWrite).not.toHaveBeenCalled();
155+
expect(isCodeFlowInProgressResult).toBeTrue();
156+
});
157+
158+
it('state object does not exist returns false result', () => {
159+
spyOn(storagePersistenceService, 'read').withArgs('storageCodeFlowInProgress', { configId: 'configId1' }).and.returnValue(null);
160+
161+
const isCodeFlowInProgressResult = service.isCodeFlowInProgress({ configId: 'configId1' });
162+
expect(isCodeFlowInProgressResult).toBeFalse();
163+
});
164+
});
165+
166+
describe('setCodeFlowInProgress', () => {
167+
it('set setCodeFlowInProgress to `in progress` when called', () => {
168+
jasmine.clock().uninstall();
169+
jasmine.clock().install();
170+
const baseTime = new Date();
171+
jasmine.clock().mockDate(baseTime);
172+
173+
const storageObject = {
174+
state: 'in progress',
175+
};
176+
177+
const spy = spyOn(storagePersistenceService, 'write');
178+
service.setCodeFlowInProgress({ configId: 'configId1' });
179+
expect(spy).toHaveBeenCalledOnceWith('storageCodeFlowInProgress', JSON.stringify(storageObject), { configId: 'configId1' });
180+
});
181+
});
182+
183+
describe('resetCodeFlowInProgress', () => {
184+
it('set resetCodeFlowInProgress to empty string when called', () => {
185+
const spy = spyOn(storagePersistenceService, 'write');
186+
service.resetCodeFlowInProgress({ configId: 'configId1' });
187+
expect(spy).toHaveBeenCalledOnceWith('storageCodeFlowInProgress', '', { configId: 'configId1' });
188+
});
189+
});
190+
134191
describe('isSilentRenewRunning', () => {
135192
it('silent renew process timeout exceeded reset state object and returns false result', () => {
136193
const config = {

0 commit comments

Comments
 (0)