Skip to content

Commit 3cfc10e

Browse files
authored
Make sure authentication methods are retrieved if token is invalid but not expired (DSpace#4663)
1 parent 04cf0ea commit 3cfc10e

File tree

3 files changed

+33
-7
lines changed

3 files changed

+33
-7
lines changed

src/app/core/auth/auth.actions.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,16 @@ export class AuthenticatedAction implements Action {
6969
public type: string = AuthActionTypes.AUTHENTICATED;
7070
payload: AuthTokenInfo;
7171

72-
constructor(token: AuthTokenInfo) {
72+
/**
73+
* Whether we should consider the given authentication info final.
74+
* If the backend restarted we may have a token that hasn't expired yet, but it will be invalid anyway.
75+
* In this case we'll have to check twice.
76+
*/
77+
checkAgain: boolean;
78+
79+
constructor(token: AuthTokenInfo, checkAgain = false) {
7380
this.payload = token;
81+
this.checkAgain = checkAgain;
7482
}
7583
}
7684

src/app/core/auth/auth.effects.spec.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,25 +161,37 @@ describe('AuthEffects', () => {
161161

162162
describe('when token is valid', () => {
163163
it('should return a AUTHENTICATED_SUCCESS action in response to a AUTHENTICATED action', () => {
164-
actions = hot('--a-', { a: { type: AuthActionTypes.AUTHENTICATED, payload: token } });
164+
actions = hot('--a-', { a: new AuthenticatedAction(token) });
165165

166166
const expected = cold('--b-', { b: new AuthenticatedSuccessAction(true, token, EPersonMock._links.self.href) });
167167

168168
expect(authEffects.authenticated$).toBeObservable(expected);
169169
});
170170
});
171171

172-
describe('when token is not valid', () => {
172+
describe('when token is expired', () => {
173173
it('should return a AUTHENTICATED_ERROR action in response to a AUTHENTICATED action', () => {
174174
spyOn((authEffects as any).authService, 'authenticatedUser').and.returnValue(observableThrow(new Error('Message Error test')));
175175

176-
actions = hot('--a-', { a: { type: AuthActionTypes.AUTHENTICATED, payload: token } });
176+
actions = hot('--a-', { a: new AuthenticatedAction(token) });
177177

178178
const expected = cold('--b-', { b: new AuthenticatedErrorAction(new Error('Message Error test')) });
179179

180180
expect(authEffects.authenticated$).toBeObservable(expected);
181181
});
182182
});
183+
184+
describe('when token is not valid but also not expired (~ cookie)', () => {
185+
it('should return a AUTHENTICATED_ERROR action in response to a AUTHENTICATED action', () => {
186+
spyOn((authEffects as any).authService, 'authenticatedUser').and.returnValue(observableThrow(new Error('Message Error test')));
187+
188+
actions = hot('--a-', { a: new AuthenticatedAction(token, true) });
189+
190+
const expected = cold('--b-', { b: new CheckAuthenticationTokenCookieAction() });
191+
192+
expect(authEffects.authenticated$).toBeObservable(expected);
193+
});
194+
});
183195
});
184196

185197
describe('authenticatedSuccess$', () => {
@@ -215,7 +227,7 @@ describe('AuthEffects', () => {
215227

216228
actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN } });
217229

218-
const expected = cold('--b-', { b: new AuthenticatedAction(token) });
230+
const expected = cold('--b-', { b: new AuthenticatedAction(token, true) });
219231

220232
expect(authEffects.checkToken$).toBeObservable(expected);
221233
});

src/app/core/auth/auth.effects.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,13 @@ export class AuthEffects {
125125
switchMap((action: AuthenticatedAction) => {
126126
return this.authService.authenticatedUser(action.payload).pipe(
127127
map((userHref: string) => new AuthenticatedSuccessAction((userHref !== null), action.payload, userHref)),
128-
catchError((error: unknown) => errorToAuthAction$(AuthenticatedErrorAction, error)),
128+
catchError((error: unknown) => {
129+
if (action.checkAgain) {
130+
return of(new CheckAuthenticationTokenCookieAction());
131+
} else {
132+
return errorToAuthAction$(AuthenticatedErrorAction, error);
133+
}
134+
}),
129135
);
130136
}),
131137
));
@@ -180,7 +186,7 @@ export class AuthEffects {
180186
public checkToken$: Observable<Action> = createEffect(() => this.actions$.pipe(ofType(AuthActionTypes.CHECK_AUTHENTICATION_TOKEN),
181187
switchMap(() => {
182188
return this.authService.hasValidAuthenticationToken().pipe(
183-
map((token: AuthTokenInfo) => new AuthenticatedAction(token)),
189+
map((token: AuthTokenInfo) => new AuthenticatedAction(token, true)),
184190
catchError((error: unknown) => of(new CheckAuthenticationTokenCookieAction())),
185191
);
186192
}),

0 commit comments

Comments
 (0)