Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

feat(logout): new logout implementation using /logout/v2 API #3513

Merged
merged 2 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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