From a119c888f7a5a404bddc3bdc62d3dee6982445f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Silke=20Gr=C3=BCber?= Date: Thu, 16 Apr 2020 09:57:26 +0200 Subject: [PATCH] fix: set captcha authorization key for 'contact us' REST requests (#200) --- .../integration/pages/contact/contact.page.ts | 2 +- src/app/core/models/captcha/captcha.model.ts | 7 ++++ src/app/core/models/contact/contact.model.ts | 4 ++- .../core/models/customer/customer.model.ts | 5 ++- .../password-reminder.model.ts | 6 ++-- src/app/core/services/api/api.service.spec.ts | 27 +++++++++++++++ src/app/core/services/api/api.service.ts | 33 ++++++++++++++++-- .../core/services/contact/contact.service.ts | 10 ++++-- .../core/services/user/user.service.spec.ts | 4 +-- src/app/core/services/user/user.service.ts | 34 +++---------------- .../contact-form/contact-form.component.html | 2 +- .../contact-form.component.spec.ts | 2 +- .../contact-form/contact-form.component.ts | 19 +++-------- .../pages/contact/contact-page.component.ts | 4 +-- .../request-reminder-form.component.ts | 2 +- .../registration-form.component.ts | 4 +-- 16 files changed, 101 insertions(+), 64 deletions(-) create mode 100644 src/app/core/models/captcha/captcha.model.ts diff --git a/e2e/cypress/integration/pages/contact/contact.page.ts b/e2e/cypress/integration/pages/contact/contact.page.ts index d97ac8f01ca..ae0246415a4 100644 --- a/e2e/cypress/integration/pages/contact/contact.page.ts +++ b/e2e/cypress/integration/pages/contact/contact.page.ts @@ -28,7 +28,7 @@ export class ContactPage { this.phoneInput.clear().type(phone).blur(); // tslint:disable-next-line:ban cy.get('select[data-testing-id="subject"]').select(subject); - cy.get('textarea[data-testing-id="comments"]').clear().type(comments).blur(); + cy.get('textarea[data-testing-id="comment"]').clear().type(comments).blur(); return this; } diff --git a/src/app/core/models/captcha/captcha.model.ts b/src/app/core/models/captcha/captcha.model.ts new file mode 100644 index 00000000000..e76f869ed5b --- /dev/null +++ b/src/app/core/models/captcha/captcha.model.ts @@ -0,0 +1,7 @@ +/** + * The contact request to send, when a customer want to get in contact with the shop + */ +export interface Captcha { + captcha?: string; + captchaAction?: string; +} diff --git a/src/app/core/models/contact/contact.model.ts b/src/app/core/models/contact/contact.model.ts index 1d02a3e1328..da015c3060b 100644 --- a/src/app/core/models/contact/contact.model.ts +++ b/src/app/core/models/contact/contact.model.ts @@ -1,7 +1,9 @@ +import { Captcha } from 'ish-core/models/captcha/captcha.model'; + /** * The contact request to send, when a customer want to get in contact with the shop */ -export interface Contact { +export interface Contact extends Captcha { name: string; type?: string; email: string; diff --git a/src/app/core/models/customer/customer.model.ts b/src/app/core/models/customer/customer.model.ts index 0587358a69e..4d6fce16163 100644 --- a/src/app/core/models/customer/customer.model.ts +++ b/src/app/core/models/customer/customer.model.ts @@ -1,4 +1,5 @@ import { Address } from 'ish-core/models/address/address.model'; +import { Captcha } from 'ish-core/models/captcha/captcha.model'; import { Credentials } from 'ish-core/models/credentials/credentials.model'; import { User } from 'ish-core/models/user/user.model'; @@ -29,9 +30,7 @@ export interface CustomerUserType { /** * registration request data type */ -export interface CustomerRegistrationType extends CustomerUserType { +export interface CustomerRegistrationType extends CustomerUserType, Captcha { credentials: Credentials; address: Address; - captchaResponse?: string; - captchaAction?: string; } diff --git a/src/app/core/models/password-reminder/password-reminder.model.ts b/src/app/core/models/password-reminder/password-reminder.model.ts index 9e470189e34..0ed747b8985 100644 --- a/src/app/core/models/password-reminder/password-reminder.model.ts +++ b/src/app/core/models/password-reminder/password-reminder.model.ts @@ -1,8 +1,8 @@ -export interface PasswordReminder { +import { Captcha } from 'ish-core/models/captcha/captcha.model'; + +export interface PasswordReminder extends Captcha { email: string; firstName: string; lastName: string; answer?: string; - captcha?: string; - captchaAction?: string; } diff --git a/src/app/core/services/api/api.service.spec.ts b/src/app/core/services/api/api.service.spec.ts index 6a78bc105c0..dc206853318 100644 --- a/src/app/core/services/api/api.service.spec.ts +++ b/src/app/core/services/api/api.service.spec.ts @@ -487,5 +487,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(); + }); }); }); diff --git a/src/app/core/services/api/api.service.ts b/src/app/core/services/api/api.service.ts index 97671969ea5..7eef4dd2c32 100644 --- a/src/app/core/services/api/api.service.ts +++ b/src/app/core/services/api/api.service.ts @@ -4,6 +4,7 @@ import { Store, select } from '@ngrx/store'; import { Observable, OperatorFunction, Subject, forkJoin, of, throwError } from 'rxjs'; import { catchError, concatMap, defaultIfEmpty, filter, map, switchMap, tap, throwIfEmpty } from 'rxjs/operators'; +import { Captcha } from 'ish-core/models/captcha/captcha.model'; import { Link } from 'ish-core/models/link/link.model'; import { Locale } from 'ish-core/models/locale/locale.model'; import { getCurrentLocale, getICMServerURL, getRestEndpoint } from 'ish-core/store/configuration'; @@ -105,6 +106,7 @@ export interface AvailableOptions { headers?: HttpHeaders; skipApiErrorHandling?: boolean; runExclusively?: boolean; + captcha?: Captcha; } @Injectable({ providedIn: 'root' }) @@ -137,17 +139,44 @@ 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) { + // testing token gets 'null' from captcha service, so we accept it as a valid value here + if (captchaAction !== undefined) { + // 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); + + // testing token gets 'null' from captcha service, so we accept it as a valid value here + if (options && options.captcha && options.captcha.captcha !== undefined) { + return this.appendCaptchaTokenToHeaders(newHeaders, options.captcha.captcha, options.captcha.captchaAction); + } else { + return this.appendAPITokenToHeaders(newHeaders); + } } private wrapHttpCall(httpCall: () => Observable, options: AvailableOptions) { diff --git a/src/app/core/services/contact/contact.service.ts b/src/app/core/services/contact/contact.service.ts index 08ee6acdb9b..28f711c2b38 100644 --- a/src/app/core/services/contact/contact.service.ts +++ b/src/app/core/services/contact/contact.service.ts @@ -1,9 +1,10 @@ import { Injectable } from '@angular/core'; +import { pick } from 'lodash-es'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { Contact } from 'ish-core/models/contact/contact.model'; -import { ApiService, unpackEnvelope } from 'ish-core/services/api/api.service'; +import { ApiService, AvailableOptions, unpackEnvelope } from 'ish-core/services/api/api.service'; @Injectable({ providedIn: 'root' }) export class ContactService { @@ -23,6 +24,11 @@ export class ContactService { * Send contact us request, when a customer want to get in contact with the shop */ createContactRequest(contactData: Contact): Observable { - return this.apiService.post(`contact`, contactData, { skipApiErrorHandling: true }); + const options: AvailableOptions = { + skipApiErrorHandling: true, + captcha: pick(contactData, ['captcha', 'captchaAction']), + }; + + return this.apiService.post(`contact`, { ...contactData, captcha: undefined, captchaAction: undefined }, options); } } diff --git a/src/app/core/services/user/user.service.spec.ts b/src/app/core/services/user/user.service.spec.ts index 08a9f5088e1..03d3e83202b 100644 --- a/src/app/core/services/user/user.service.spec.ts +++ b/src/app/core/services/user/user.service.spec.ts @@ -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, @@ -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(); }); }); diff --git a/src/app/core/services/user/user.service.ts b/src/app/core/services/user/user.service.ts index 3a323a9d372..6a0724fad98 100644 --- a/src/app/core/services/user/user.service.ts +++ b/src/app/core/services/user/user.service.ts @@ -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'; @@ -97,14 +98,9 @@ export class UserService { }; } - if (body.captchaResponse) { - return this.apiService.post('customers', newCustomer, { - headers: this.appendCaptchaHeaders(body.captchaResponse, body.captchaAction), - }); - // without captcha - } else { - return this.apiService.post('customers', newCustomer); - } + return this.apiService.post('customers', newCustomer, { + captcha: pick(body, ['captcha', 'captchaAction']), + }); } /** @@ -197,12 +193,9 @@ export class UserService { requestPasswordReminder(data: PasswordReminder) { const options: AvailableOptions = { skipApiErrorHandling: true, + captcha: pick(data, ['captcha', 'captchaAction']), }; - if (data.captcha) { - options.headers = this.appendCaptchaHeaders(data.captcha, data.captchaAction); - } - return this.apiService.post('security/reminder', { answer: '', ...data }, options); } @@ -216,21 +209,4 @@ export class UserService { }; return this.apiService.post('security/password', data, options); } - - // provides the request header for the appropriate captcha service - private 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; - } } diff --git a/src/app/pages/contact/contact-form/contact-form.component.html b/src/app/pages/contact/contact-form/contact-form.component.html index 5d2bab861e5..49961553020 100644 --- a/src/app/pages/contact/contact-form/contact-form.component.html +++ b/src/app/pages/contact/contact-form/contact-form.component.html @@ -29,7 +29,7 @@ [translateOptionLabels]="true" > { component.contactForm.get('phone').setValue('123456'); component.contactForm.get('order').setValue('456789'); component.contactForm.get('subject').setValue('Return'); - component.contactForm.get('comments').setValue('want to return stuff'); + component.contactForm.get('comment').setValue('want to return stuff'); component.submitForm(); verify(emitter.emit(anything())).once(); }); diff --git a/src/app/pages/contact/contact-form/contact-form.component.ts b/src/app/pages/contact/contact-form/contact-form.component.ts index 7fd0e8562d8..94315c62475 100644 --- a/src/app/pages/contact/contact-form/contact-form.component.ts +++ b/src/app/pages/contact/contact-form/contact-form.component.ts @@ -22,7 +22,7 @@ export class ContactFormComponent implements OnChanges, OnInit { @Input() subjects: string[] = []; @Input() user: User; /** The contact request to send. */ - @Output() request = new EventEmitter<{ contact: Contact; captcha?: string }>(); + @Output() request = new EventEmitter(); subjectOptions: SelectOption[]; @@ -43,18 +43,9 @@ export class ContactFormComponent implements OnChanges, OnInit { /** emit contact request, when for is valid or mark form as dirty, when form is invalid */ submitForm() { if (this.contactForm.valid) { - const formValue = this.contactForm.value; - const contact: Contact = { - name: formValue.name, - email: formValue.email, - phone: formValue.phone, - subject: formValue.subject, - comment: formValue.comments, - order: formValue.order, - }; + const contact: Contact = this.contactForm.value; - /* ToDo: send captcha data if captcha is supported by REST, see #IS-28299 */ - this.request.emit({ contact }); + this.request.emit(contact); } else { markAsDirtyRecursive(this.contactForm); this.submitted = true; @@ -77,9 +68,9 @@ export class ContactFormComponent implements OnChanges, OnInit { phone: [phone, Validators.required], order: [''], subject: ['', Validators.required], - comments: ['', Validators.required], + comment: ['', Validators.required], captcha: [''], - captchaAction: ['contact_us'], + captchaAction: ['contactUs'], }); } diff --git a/src/app/pages/contact/contact-page.component.ts b/src/app/pages/contact/contact-page.component.ts index 102b78e8eff..b1829cd9c17 100644 --- a/src/app/pages/contact/contact-page.component.ts +++ b/src/app/pages/contact/contact-page.component.ts @@ -52,8 +52,8 @@ export class ContactPageComponent implements OnInit, OnDestroy { } /** dispatch contact request */ - createRequest(request: { contact: Contact; captcha?: string }) { - this.accountFacade.createContact(request.contact); + createRequest(contact: Contact) { + this.accountFacade.createContact(contact); this.router.navigate([], { queryParams: { submitted: true } }); } diff --git a/src/app/pages/forgot-password/request-reminder-form/request-reminder-form.component.ts b/src/app/pages/forgot-password/request-reminder-form/request-reminder-form.component.ts index 85e4ef2c5ec..0a5a87ad405 100644 --- a/src/app/pages/forgot-password/request-reminder-form/request-reminder-form.component.ts +++ b/src/app/pages/forgot-password/request-reminder-form/request-reminder-form.component.ts @@ -33,7 +33,7 @@ export class RequestReminderFormComponent implements OnInit { firstName: new FormControl('', Validators.required), lastName: new FormControl('', Validators.required), captcha: new FormControl(''), - captchaAction: new FormControl('forgot_password'), + captchaAction: new FormControl('forgotPassword'), }); } diff --git a/src/app/pages/registration/registration-form/registration-form.component.ts b/src/app/pages/registration/registration-form/registration-form.component.ts index 3570df37317..ed8a55f5c92 100644 --- a/src/app/pages/registration/registration-form/registration-form.component.ts +++ b/src/app/pages/registration/registration-form/registration-form.component.ts @@ -74,7 +74,7 @@ export class RegistrationFormComponent implements OnInit, OnChanges { birthday: [''], termsAndConditions: [false, [Validators.required, Validators.pattern('true')]], captcha: [''], - captchaAction: ['create_account'], + captchaAction: ['register'], address: this.afs.getFactory('default').getGroup({ isBusinessAddress: this.businessCustomerRegistration }), // filled dynamically when country code changes }); @@ -147,7 +147,7 @@ export class RegistrationFormComponent implements OnInit, OnChanges { } const registration: CustomerRegistrationType = { customer, user, credentials, address }; - registration.captchaResponse = this.form.get('captcha').value; + registration.captcha = this.form.get('captcha').value; registration.captchaAction = this.form.get('captchaAction').value; this.create.emit(registration);