Skip to content

Commit

Permalink
feat(auth): set and remove auth token immediately (#2456)
Browse files Browse the repository at this point in the history 
Previously, we saw race condition errors when the token wasn't immediately
available after a successful login. There were times
where multiple effects would await AuthLoginSuccess, and depending
on internal angular mechanisms, certain effects would run in an
ill-defined order, creating unexpected consequences.
  • Loading branch information
@damienwebdev
damienwebdev committed May 25, 2023
1 parent d112bcd commit fc6c85d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 129 deletions.
130 changes: 55 additions & 75 deletions libs/auth/state/src/effects/login.effects.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,38 @@ describe('@daffodil/auth/state | DaffAuthLoginEffects', () => {
it('should notify state that the login was successful', () => {
expect(effects.login$).toBeObservable(expected);
});

it('should store the auth token', () => {
expect(effects.login$).toBeObservable(expected);
expect(setAuthTokenSpy).toHaveBeenCalledWith(mockAuth.token);
});

describe('but the token storage fails', () => {
beforeEach(() => {
setAuthTokenSpy.and.callFake(throwStorageError);
const mockAuthLoginFailureAction = new DaffAuthLoginFailure({
code: 'DAFF_STORAGE_FAILURE', recoverable: false, message: 'Storage of auth token has failed.',
});
expected = cold('--b', { b: mockAuthLoginFailureAction });
});

it('should return a DaffAuthStorageFailure', () => {
expect(effects.login$).toBeObservable(expected);
});

describe('unless the storage service throws a server side error', () => {
beforeEach(() => {
const error = new DaffServerSideStorageError('Server side');
const serverSideAction = new DaffAuthServerSide(daffTransformErrorToStateError(error));
setAuthTokenSpy.and.throwError(error);
expected = cold('--(a)', { a: serverSideAction });
});

it('should dispatch a server side action', () => {
expect(effects.login$).toBeObservable(expected);
});
});
});
});

describe('and the login fails', () => {
Expand Down Expand Up @@ -155,103 +187,51 @@ describe('@daffodil/auth/state | DaffAuthLoginEffects', () => {
it('should notify state that the logout succeeded', () => {
expect(effects.logout$).toBeObservable(expected);
});
});

describe('and the logout fails', () => {
beforeEach(() => {
const error = new DaffAuthInvalidAPIResponseError('Failed to log out');
const response = cold('#', {}, error);
daffLoginDriver.logout.and.returnValue(response);
const mockAuthLogoutFailureAction = new DaffAuthLogoutFailure(daffTransformErrorToStateError(error));

actions$ = hot('--a', { a: mockAuthLogoutAction });
expected = cold('--b', { b: mockAuthLogoutFailureAction });
});

it('should notify state that the logout failed', () => {
it('should remove the auth token from storage', () => {
expect(effects.logout$).toBeObservable(expected);
expect(removeAuthTokenSpy).toHaveBeenCalledWith();
});
});
});

describe('storeAuthToken$ | storing the auth token after a successful login', () => {
let expected;
let authLoginSuccessAction;

beforeEach(() => {
authLoginSuccessAction = new DaffAuthLoginSuccess(mockAuth);
actions$ = hot('--a', { a: authLoginSuccessAction });
expected = cold('---');
});

it('should set the auth token in storage', () => {
expect(effects.storeAuthToken$).toBeObservable(expected);
expect(setAuthTokenSpy).toHaveBeenCalledWith(mockAuth.token);
});

describe('and the storage service throws an error', () => {
beforeEach(() => {
setAuthTokenSpy.and.callFake(throwStorageError);
describe('unless the storage service throws an error', () => {
beforeEach(() => {
removeAuthTokenSpy.and.callFake(throwStorageError);

expected = cold('--(b|)', { b: authStorageFailureAction });
});
expected = cold('--(b)', { b: authStorageFailureAction });
});

it('should return a DaffAuthStorageFailure', () => {
expect(effects.storeAuthToken$).toBeObservable(expected);
it('should return a DaffAuthStorageFailure', () => {
expect(effects.logout$).toBeObservable(expected);
});
});

describe('and the storage service throws a server side error', () => {
describe('unless the storage service throws a server side error', () => {
beforeEach(() => {
const error = new DaffServerSideStorageError('Server side');
const serverSideAction = new DaffAuthServerSide(daffTransformErrorToStateError(error));
setAuthTokenSpy.and.throwError(error);
expected = cold('--(a|)', { a: serverSideAction });
removeAuthTokenSpy.and.throwError(error);
expected = cold('--(a)', { a: serverSideAction });
});

it('should dispatch a server side action', () => {
expect(effects.storeAuthToken$).toBeObservable(expected);
expect(effects.logout$).toBeObservable(expected);
});
});
});
});

describe('removeAuthToken$', () => {
let expected;
let authLogoutSuccessAction: DaffAuthLogoutSuccess;

beforeEach(() => {
authLogoutSuccessAction = new DaffAuthLogoutSuccess();
actions$ = hot('--a', { a: authLogoutSuccessAction });
expected = cold('---');
});

it('should remove the auth token from storage', () => {
expect(effects.removeAuthToken$).toBeObservable(expected);
expect(removeAuthTokenSpy).toHaveBeenCalledWith();
});

describe('and the storage service throws an error', () => {
describe('and the logout fails', () => {
beforeEach(() => {
removeAuthTokenSpy.and.callFake(throwStorageError);

expected = cold('--(b|)', { b: authStorageFailureAction });
});

it('should return a DaffAuthStorageFailure', () => {
expect(effects.removeAuthToken$).toBeObservable(expected);
});
});
const error = new DaffAuthInvalidAPIResponseError('Failed to log out');
const response = cold('#', {}, error);
daffLoginDriver.logout.and.returnValue(response);
const mockAuthLogoutFailureAction = new DaffAuthLogoutFailure(daffTransformErrorToStateError(error));

describe('and the storage service throws a server side error', () => {
beforeEach(() => {
const error = new DaffServerSideStorageError('Server side');
const serverSideAction = new DaffAuthServerSide(daffTransformErrorToStateError(error));
removeAuthTokenSpy.and.throwError(error);
expected = cold('--(a|)', { a: serverSideAction });
actions$ = hot('--a', { a: mockAuthLogoutAction });
expected = cold('--b', { b: mockAuthLogoutFailureAction });
});

it('should dispatch a server side action', () => {
expect(effects.removeAuthToken$).toBeObservable(expected);
it('should notify state that the logout failed', () => {
expect(effects.logout$).toBeObservable(expected);
});
});
});
Expand Down
80 changes: 26 additions & 54 deletions libs/auth/state/src/effects/login.effects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ import {
createEffect,
ofType,
} from '@ngrx/effects';
import {
of,
Observable,
EMPTY,
} from 'rxjs';
import { of } from 'rxjs';
import {
switchMap,
map,
Expand All @@ -38,15 +34,14 @@ import { ErrorTransformer } from '@daffodil/core/state';

import {
DaffAuthLoginActionTypes,
DaffAuthLogin,
DaffAuthLoginSuccess,
DaffAuthLoginFailure,
DaffAuthLogoutSuccess,
DaffAuthLogoutFailure,
DaffAuthLogout,
DaffAuthStorageFailure,
DaffAuthServerSide,
DaffAuthLoginActions,
DaffAuthStorageFailure,
} from '../actions/public_api';

@Injectable()
Expand All @@ -64,13 +59,20 @@ export class DaffAuthLoginEffects<
login$ = createEffect(() => this.actions$.pipe(
ofType(DaffAuthLoginActionTypes.LoginAction),
switchMap((action) =>
this.loginDriver.login(action.loginInfo).pipe(
map((resp) => new DaffAuthLoginSuccess<U>(resp),
this.loginDriver.login(action.loginInfo)
.pipe(
map((resp) => new DaffAuthLoginSuccess<U>(resp)),
tap((success) => this.storage.setAuthToken(success.auth.token)),
catchError((error: DaffError) => {
switch (true) {
case error instanceof DaffServerSideStorageError:
return of(new DaffAuthServerSide(this.errorMatcher(error)));
case error instanceof DaffStorageServiceError:
default:
return of(new DaffAuthLoginFailure(this.errorMatcher(error)));
}
}),
),
catchError((error: DaffError) =>
of(new DaffAuthLoginFailure(this.errorMatcher(error))),
),
),
),
));

Expand All @@ -79,48 +81,18 @@ export class DaffAuthLoginEffects<
switchMap((action: DaffAuthLogout) =>
this.loginDriver.logout().pipe(
map(() => new DaffAuthLogoutSuccess()),
catchError((error: DaffError) =>
of(new DaffAuthLogoutFailure(this.errorMatcher(error))),
),
tap(() => this.storage.removeAuthToken()),
catchError((error: DaffError) => {
switch (true) {
case error instanceof DaffServerSideStorageError:
return of(new DaffAuthServerSide(this.errorMatcher(error)));
case error instanceof DaffStorageServiceError:
return of(new DaffAuthStorageFailure(this.errorMatcher(error)));
default:
return of(new DaffAuthLogoutFailure(this.errorMatcher(error)));
}
}),
),
),
));

removeAuthToken$ = createEffect(() => this.actions$.pipe(
ofType(
DaffAuthLoginActionTypes.LogoutSuccessAction,
),
tap(() => {
this.storage.removeAuthToken();
}),
switchMap(() => EMPTY),
catchError((error: Error) => {
switch (true) {
case error instanceof DaffServerSideStorageError:
return of(new DaffAuthServerSide(this.errorMatcher(error)));

case error instanceof DaffStorageServiceError:
default:
return of(new DaffAuthStorageFailure(this.errorMatcher(error)));
}
}),
), { dispatch: false });

storeAuthToken$ = createEffect(() => this.actions$.pipe(
ofType(DaffAuthLoginActionTypes.LoginSuccessAction),
tap((action) => {
this.storage.setAuthToken(action.auth.token);
}),
switchMap(() => EMPTY),
catchError((error: Error) => {
switch (true) {
case error instanceof DaffServerSideStorageError:
return of(new DaffAuthServerSide(this.errorMatcher(error)));

case error instanceof DaffStorageServiceError:
default:
return of(new DaffAuthStorageFailure(this.errorMatcher(error)));
}
}),
), { dispatch: false });
}

0 comments on commit fc6c85d

Please sign in to comment.