Skip to content

Commit

Permalink
feat(logout): new logout implementation using /logout/v2 API (fabric8…
Browse files Browse the repository at this point in the history
…-ui#3513)

* feat(logout): new logout implementation using /logout/v2 API

* fix(types): add better type checking for fetch
  • Loading branch information
rohitkrai03 authored and karthikjeeyar committed Feb 6, 2019
1 parent 26d9437 commit a50f6f2
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 16 deletions.
32 changes: 30 additions & 2 deletions packages/fabric8-ui/src/app/shared/login.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import { never as observableNever, of } from 'rxjs';
import { createMock } from 'testing/mock';
import { LoginService } from './login.service';
import { WindowService } from './window.service';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';

describe('LoginService', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule.withRoutes([])],
imports: [RouterTestingModule.withRoutes([]), HttpClientTestingModule],
providers: [
LoginService,
{
Expand All @@ -24,7 +25,7 @@ describe('LoginService', () => {
},
},
{ provide: LocalStorageService, useValue: createMock(LocalStorageService) },
{ provide: AUTH_API_URL, useValue: 'http://example.com' },
{ provide: AUTH_API_URL, useValue: 'http://example.com/' },
{
provide: Broadcaster,
useFactory: () => {
Expand Down Expand Up @@ -105,4 +106,31 @@ describe('LoginService', () => {
});
});
});

describe('logout', () => {
// Fix the test warning related to ngzone.
it('should handle logout', () => {
const windowService: jasmine.SpyObj<WindowService> = TestBed.get(WindowService);
windowService.getNativeWindow.and.returnValue({
location: {
origin: 'mock-origin',
href: '',
},
});

const loginService: LoginService = TestBed.get(LoginService);
loginService.logout();

const authService: jasmine.SpyObj<AuthenticationService> = TestBed.get(AuthenticationService);
authService.logout.and.stub();

const controller = TestBed.get(HttpTestingController);

const req = controller.expectOne('http://example.com/logout/v2?redirect=mock-origin');
expect(req.request.method).toEqual('GET');
req.flush({ redirect_location: 'mock-location' });

expect(authService.logout).toBeCalled();
});
});
});
12 changes: 9 additions & 3 deletions packages/fabric8-ui/src/app/shared/login.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { AUTH_API_URL, AuthenticationService, UserService } from 'ngx-login-clie
import { of } from 'rxjs';
import { catchError } from 'rxjs/operators';
import { WindowService } from './window.service';
import { HttpClient } from '@angular/common/http';

@Injectable()
export class LoginService {
Expand All @@ -23,6 +24,7 @@ export class LoginService {
constructor(
windowService: WindowService,
private router: Router,
private http: HttpClient,
private localStorage: LocalStorageService,
@Inject(AUTH_API_URL) private apiUrl: string,
private broadcaster: Broadcaster,
Expand Down Expand Up @@ -71,9 +73,13 @@ export class LoginService {
}

public logout() {
this.authService.logout();
this.window.location.href =
this.apiUrl + 'logout?redirect=' + encodeURIComponent(this.window.location.origin);
const logoutUrl =
this.apiUrl + 'logout/v2?redirect=' + encodeURIComponent(this.window.location.origin);

this.http.get(logoutUrl).subscribe((res: { redirect_location: string }) => {
this.authService.logout();
window.location.href = res.redirect_location;
});
}

public login() {
Expand Down
2 changes: 1 addition & 1 deletion packages/toolchain/src/app/api/__mocks__/jsonapi.client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JsonApiError } from '../jsonapi.client';
import { JsonApiError } from '../http.client';

export const fetchDocument = (url: string) => {
if (url.indexOf('throwError') >= 0) {
Expand Down
6 changes: 3 additions & 3 deletions packages/toolchain/src/app/api/__tests__/api-urls.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ jest.mock('../internal/api.config.ts');

describe('api.urls', () => {
it('should get logout url', () => {
expect(urls.getLogoutUrl('')).toBe('/auth-api-url/logout');
expect(urls.getLogoutUrl('foobar')).toBe('/auth-api-url/logout?redirect=foobar');
expect(urls.getLogoutUrl('foo&bar')).toBe('/auth-api-url/logout?redirect=foo%26bar');
expect(urls.getLogoutUrl('')).toBe('/auth-api-url/logout/v2');
expect(urls.getLogoutUrl('foobar')).toBe('/auth-api-url/logout/v2?redirect=foobar');
expect(urls.getLogoutUrl('foo&bar')).toBe('/auth-api-url/logout/v2?redirect=foo%26bar');
});

it('should get login authorize url', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/toolchain/src/app/api/api-urls.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AUTH_API_URL, FEATURE_TOGGLES_API_URL, WIT_API_URL } from './internal/api.config';

export const getLogoutUrl = (redirect: string) =>
`${AUTH_API_URL}logout${redirect ? `?redirect=${encodeURIComponent(redirect)}` : ''}`;
`${AUTH_API_URL}logout/v2${redirect ? `?redirect=${encodeURIComponent(redirect)}` : ''}`;
export const getLoginAuthorizeUrl = () => `${WIT_API_URL}login/authorize`;
export const getCurrentUserSpacesUrl = () => `${WIT_API_URL}user/spaces`;
export const getCurrentUserUrl = () => `${AUTH_API_URL}user`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ErrorsDocument,
ErrorObject,
JsonApiError as IJsonApiError,
MetaObject,
} from './models/jsonapi';

export class JsonApiError extends Error implements IJsonApiError {
Expand All @@ -13,9 +14,13 @@ export class JsonApiError extends Error implements IJsonApiError {
}
}

export async function fetch<T>(url: string): Promise<T> {
return (await axiosClient.get<T>(url)).data;
}

export async function fetchDocument(url: string): Promise<DataDocument> {
try {
return (await axiosClient.get<DataDocument>(url)).data;
return fetch<DataDocument>(url);
} catch (e) {
const axiosError = e as AxiosError;
const { response } = axiosError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,17 @@ import { AuthenticationActionTypes } from '../actionTypes';
import { RedirectActionTypes } from '../../middleware/redirect';
import { LocalStorageActionTypes } from '../../middleware/localStorage';
import * as token from '../../../api/token';
import { getLoginAuthorizeUrl, getLogoutUrl } from '../../../api/api-urls';
import { getLoginAuthorizeUrl } from '../../../api/api-urls';
import { AppState } from '../../appState';

jest.mock('../../wit/actions');

jest.mock('../../../api/http.client', () => ({
fetch: jest.fn((url) => ({
redirect_location: 'foobar',
})),
}));

let mockToken;
let mockParsedToken;
let mockTokenInfo: token.TokenInfo;
Expand Down Expand Up @@ -48,7 +54,7 @@ describe('authentication actions', () => {
expect(await getAction(store, RedirectActionTypes.REDIRECT)).toEqual({
type: RedirectActionTypes.REDIRECT,
payload: {
url: getLogoutUrl('foobar'),
url: 'foobar',
},
});
expect(token.setAuthToken).toHaveBeenCalledWith(null);
Expand Down
7 changes: 5 additions & 2 deletions packages/toolchain/src/app/redux/authentication/actions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { push } from 'connected-react-router';
import { fetch } from '../../api/http.client';
import { getLoginAuthorizeUrl, getLogoutUrl } from '../../api/api-urls';
import {
setAuthToken,
Expand Down Expand Up @@ -75,8 +76,10 @@ export function login(url: string = `${window.location.pathname}${location.searc
}

export function logout(): ThunkAction {
return function(dispatch) {
return async function(dispatch) {
const logoutUrl = getLogoutUrl(window.location.origin);
const result = await fetch<{ redirect_location: string }>(logoutUrl);
setAuthToken(null);
dispatch(redirect(getLogoutUrl(window.location.origin)));
dispatch(redirect(result.redirect_location));
};
}
2 changes: 1 addition & 1 deletion packages/toolchain/src/app/redux/jsonapi/actions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ThunkAction } from 'redux-thunk';
import { DataDocument, ErrorObject, JsonApiError } from '../../api/models/jsonapi';
import { fetchDocument as fetchDocumentApi } from '../../api/jsonapi.client';
import { fetchDocument as fetchDocumentApi } from '../../api/http.client';
import { AppState } from '../appState';
import { createAction, ActionsUnion } from '../utils';
import { JsonapiActionTypes } from './actionTypes';
Expand Down

0 comments on commit a50f6f2

Please sign in to comment.