Skip to content

Commit

Permalink
refactor: captcha as configurable property on api service methods
Browse files Browse the repository at this point in the history
  • Loading branch information
dhhyi committed Apr 27, 2020
1 parent 8bb7fe4 commit 5b3c641
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 62 deletions.
49 changes: 30 additions & 19 deletions src/app/core/services/api/api.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,25 +148,9 @@ describe('Api Service', () => {
req.flush({});
expect(req.request.method).toEqual('DELETE');
});

it('should set Captcha V2 authorization header key when apiService.appendCaptchaHeaders method is called.', () => {
const httpHeader = apiService.appendCaptchaHeaders('captchatoken', undefined);

expect(httpHeader.get(ApiService.AUTHORIZATION_HEADER_KEY)).toEqual(
'CAPTCHA g-recaptcha-response=captchatoken foo=bar'
);
});

it('should set Captcha V3 authorization header key when apiService.appendCaptchaHeaders method is called.', () => {
const httpHeader = apiService.appendCaptchaHeaders('captchatoken', 'create_account');

expect(httpHeader.get(ApiService.AUTHORIZATION_HEADER_KEY)).toEqual(
'CAPTCHA recaptcha_token=captchatoken action=create_account'
);
});
});

describe('Api Service', () => {
describe('API Service Helper Methods', () => {
describe('constructUrlForPath()', () => {
const BASE_URL = 'http://example.org/site';
const JP = { lang: 'jp', currency: 'YEN' } as Locale;
Expand Down Expand Up @@ -223,7 +207,7 @@ describe('Api Service', () => {
});
});

describe('Api Service', () => {
describe('API Service Pipable Operators', () => {
let httpTestingController: HttpTestingController;
let apiService: ApiService;

Expand Down Expand Up @@ -408,7 +392,7 @@ describe('Api Service', () => {
});
});

describe('Api Service', () => {
describe('API Service Headers', () => {
const REST_URL = 'http://www.example.org/WFS/site/-';
let apiService: ApiService;
let store$: Store<{}>;
Expand Down Expand Up @@ -509,5 +493,32 @@ describe('Api Service', () => {
const req = httpTestingController.expectOne(`${REST_URL}/dummy`);
expect(req.request.headers.has(ApiService.TOKEN_HEADER_KEY)).toBeFalse();
});

it('should set Captcha V2 authorization header key when captcha is supplied without captchaAction', () => {
apiService.get('dummy', { captcha: { captcha: 'captchatoken' } }).subscribe(fail, fail, fail);

const req = httpTestingController.expectOne(`${REST_URL}/dummy`);
expect(req.request.headers.get(ApiService.AUTHORIZATION_HEADER_KEY)).toMatchInlineSnapshot(
`"CAPTCHA g-recaptcha-response=captchatoken foo=bar"`
);
});

it('should set Captcha V3 authorization header key when captcha is supplied', () => {
apiService
.get('dummy', { captcha: { captcha: 'captchatoken', captchaAction: 'create_account' } })
.subscribe(fail, fail, fail);

const req = httpTestingController.expectOne(`${REST_URL}/dummy`);
expect(req.request.headers.get(ApiService.AUTHORIZATION_HEADER_KEY)).toMatchInlineSnapshot(
`"CAPTCHA recaptcha_token=captchatoken action=create_account"`
);
});

it('should not set header when captcha config object is empty', () => {
apiService.get('dummy', { captcha: {} }).subscribe(fail, fail, fail);

const req = httpTestingController.expectOne(`${REST_URL}/dummy`);
expect(req.request.headers.get(ApiService.AUTHORIZATION_HEADER_KEY)).toBeFalsy();
});
});
});
52 changes: 28 additions & 24 deletions src/app/core/services/api/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export interface AvailableOptions {
headers?: HttpHeaders;
skipApiErrorHandling?: boolean;
runExclusively?: boolean;
captcha?: { captcha?: string; captchaAction?: string };
}

@Injectable({ providedIn: 'root' })
Expand Down Expand Up @@ -141,17 +142,42 @@ export class ApiService {
: headers;
}

/**
- * sets the request header for the appropriate captcha service
- @param captcha captcha token for captcha V2 and V3
- @param captchaAction captcha action for captcha V3
- @returns HttpHeader http header with captcha Authorization key
- */
private appendCaptchaTokenToHeaders(headers: HttpHeaders, captcha: string, captchaAction: string) {
if (captchaAction) {
// captcha V3
return headers.set(
ApiService.AUTHORIZATION_HEADER_KEY,
`CAPTCHA recaptcha_token=${captcha} action=${captchaAction}`
);
} else {
// captcha V2
// TODO: remove second parameter 'foo=bar' that currently only resolves a shortcoming of the server side implemenation that still requires two parameters
return headers.set(ApiService.AUTHORIZATION_HEADER_KEY, `CAPTCHA g-recaptcha-response=${captcha} foo=bar`);
}
}

/**
* merges supplied and default headers
*/
private constructHeaders(options?: { headers?: HttpHeaders }): HttpHeaders {
private constructHeaders(options?: AvailableOptions): HttpHeaders {
const defaultHeaders = new HttpHeaders().set('content-type', 'application/json').set('Accept', 'application/json');

let newHeaders = defaultHeaders;
if (options && options.headers) {
newHeaders = options.headers.keys().reduce((acc, key) => acc.set(key, options.headers.get(key)), defaultHeaders);
}
return this.appendAPITokenToHeaders(newHeaders);

if (options && options.captcha && options.captcha.captcha) {
return this.appendCaptchaTokenToHeaders(newHeaders, options.captcha.captcha, options.captcha.captchaAction);
} else {
return this.appendAPITokenToHeaders(newHeaders);
}
}

private wrapHttpCall<T>(httpCall: () => Observable<T>, options: AvailableOptions) {
Expand Down Expand Up @@ -285,26 +311,4 @@ export class ApiService {
options
);
}

/**
* provides the request header for the appropriate captcha service
@param captcha captcha token for captcha V2 and V3
@param captchaAction captcha action for captcha V3
@returns HttpHeader http header with captcha Authorization key
*/
appendCaptchaHeaders(captcha: string, captchaAction: string): HttpHeaders {
let headers = new HttpHeaders();
// captcha V3
if (captchaAction) {
headers = headers.set(
ApiService.AUTHORIZATION_HEADER_KEY,
`CAPTCHA recaptcha_token=${captcha} action=${captchaAction}`
);
// captcha V2
} else {
// TODO: remove second parameter 'foo=bar' that currently only resolves a shortcoming of the server side implemenation that still requires two parameters
headers = headers.set(ApiService.AUTHORIZATION_HEADER_KEY, `CAPTCHA g-recaptcha-response=${captcha} foo=bar`);
}
return headers;
}
}
6 changes: 2 additions & 4 deletions src/app/core/services/contact/contact.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable } from '@angular/core';
import { pick } from 'lodash-es';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';

Expand All @@ -25,12 +26,9 @@ export class ContactService {
createContactRequest(contactData: Contact): Observable<void> {
const options: AvailableOptions = {
skipApiErrorHandling: true,
captcha: pick(contactData, ['captcha', 'captchaAction']),
};

if (contactData.captcha) {
options.headers = this.apiService.appendCaptchaHeaders(contactData.captcha, contactData.captchaAction);
}

return this.apiService.post(`contact`, { ...contactData, captcha: undefined, captchaAction: undefined }, options);
}
}
4 changes: 2 additions & 2 deletions src/app/core/services/user/user.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('User Service', () => {
});

it("should create a new private user when 'createUser' is called with type 'PrivateCustomer'", done => {
when(apiServiceMock.post(anyString(), anything())).thenReturn(of({}));
when(apiServiceMock.post(anyString(), anything(), anything())).thenReturn(of({}));

const payload = {
customer: { customerNo: '4711', type: 'PrivateCustomer' } as Customer,
Expand All @@ -90,7 +90,7 @@ describe('User Service', () => {
} as CustomerRegistrationType;

userService.createUser(payload).subscribe(() => {
verify(apiServiceMock.post('customers', anything())).once();
verify(apiServiceMock.post('customers', anything(), anything())).once();
done();
});
});
Expand Down
17 changes: 5 additions & 12 deletions src/app/core/services/user/user.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { HttpHeaders } from '@angular/common/http';
import { Injectable } from '@angular/core';
import b64u from 'b64u';
import { pick } from 'lodash-es';
import { EMPTY, Observable, throwError } from 'rxjs';
import { catchError, map } from 'rxjs/operators';

Expand Down Expand Up @@ -95,14 +96,9 @@ export class UserService {
};
}

if (body.captcha) {
return this.apiService.post<void>('customers', newCustomer, {
headers: this.apiService.appendCaptchaHeaders(body.captcha, body.captchaAction),
});
// without captcha
} else {
return this.apiService.post<void>('customers', newCustomer);
}
return this.apiService.post('customers', newCustomer, {
captcha: pick(body, ['captcha', 'captchaAction']),
});
}

/**
Expand Down Expand Up @@ -195,12 +191,9 @@ export class UserService {
requestPasswordReminder(data: PasswordReminder) {
const options: AvailableOptions = {
skipApiErrorHandling: true,
captcha: pick(data, ['captcha', 'captchaAction']),
};

if (data.captcha) {
options.headers = this.apiService.appendCaptchaHeaders(data.captcha, data.captchaAction);
}

return this.apiService.post('security/reminder', { answer: '', ...data }, options);
}

Expand Down
2 changes: 1 addition & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@
"from": "lodash.*"
},
{
"import": "^(?!(range|uniq|memoize|once|groupBy|countBy|flatten|isEqual|intersection|omit)$).*",
"import": "^(?!(range|uniq|memoize|once|groupBy|countBy|flatten|isEqual|intersection|omit|pick)$).*",
"from": "lodash.*"
},
{
Expand Down

0 comments on commit 5b3c641

Please sign in to comment.