From 22fbed760e2dd5ca8c6ebbbd07f6fae95492860b Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 3 Feb 2023 18:15:59 +0100 Subject: [PATCH] 98863: Prevent retrieval of the domains on TYPE_REQUEST_FORGOT form & fixed error message when no emails are set --- .../register-email-form.component.html | 6 ++-- .../register-email-form.component.spec.ts | 18 ++++++++-- .../register-email-form.component.ts | 33 +++++++++++-------- src/assets/i18n/en.json5 | 2 +- 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index 8cc0d293cf..8a388a3187 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -20,9 +20,9 @@ {{ MESSAGE_PREFIX + '.email.error.not-email-form' | translate }} - - - {{ MESSAGE_PREFIX + '.email.error.not-valid-domain' | translate: { domains: validMailDomains } }} + + {{ MESSAGE_PREFIX + '.email.error.not-valid-domain' | translate: { domains: validMailDomains.join(', ') } }} + diff --git a/src/app/register-email-form/register-email-form.component.spec.ts b/src/app/register-email-form/register-email-form.component.spec.ts index 585e69405a..8a527135bc 100644 --- a/src/app/register-email-form/register-email-form.component.spec.ts +++ b/src/app/register-email-form/register-email-form.component.spec.ts @@ -12,8 +12,12 @@ import { EpersonRegistrationService } from '../core/data/eperson-registration.se import { By } from '@angular/platform-browser'; import { RouterStub } from '../shared/testing/router.stub'; import { NotificationsServiceStub } from '../shared/testing/notifications-service.stub'; -import { RegisterEmailFormComponent } from './register-email-form.component'; -import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; +import { + RegisterEmailFormComponent, + TYPE_REQUEST_REGISTER, + TYPE_REQUEST_FORGOT +} from './register-email-form.component'; +import { createSuccessfulRemoteDataObject$, createFailedRemoteDataObject$ } from '../shared/remote-data.utils'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { ConfigurationProperty } from '../core/shared/configuration-property.model'; @@ -44,6 +48,7 @@ describe('RegisterEmailFormComponent', () => { ] })) }); + jasmine.getEnv().allowRespy(true); TestBed.configureTestingModule({ imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), ReactiveFormsModule], @@ -69,6 +74,15 @@ describe('RegisterEmailFormComponent', () => { const elem = fixture.debugElement.queryAll(By.css('input#email'))[0].nativeElement; expect(elem).toBeDefined(); }); + + it('should not retrieve the validDomains for TYPE_REQUEST_FORGOT', () => { + spyOn(configurationDataService, 'findByPropertyName'); + comp.typeRequest = TYPE_REQUEST_FORGOT; + + comp.ngOnInit(); + + expect(configurationDataService.findByPropertyName).not.toHaveBeenCalled(); + }); }); describe('email validation', () => { it('should be invalid when no email is present', () => { diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index bb10aaed38..808657ab38 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -1,4 +1,4 @@ -import {Component, Input, OnInit} from '@angular/core'; +import { Component, Input, OnInit, OnDestroy } from '@angular/core'; import {EpersonRegistrationService} from '../core/data/eperson-registration.service'; import {NotificationsService} from '../shared/notifications/notifications.service'; import {TranslateService} from '@ngx-translate/core'; @@ -7,8 +7,9 @@ import { FormBuilder, FormControl, FormGroup, Validators, ValidatorFn } from '@a import {Registration} from '../core/shared/registration.model'; import {RemoteData} from '../core/data/remote-data'; import {ConfigurationDataService} from '../core/data/configuration-data.service'; -import {getAllCompletedRemoteData} from '../core/shared/operators'; +import { getAllSucceededRemoteDataPayload } from '../core/shared/operators'; import { ConfigurationProperty } from '../core/shared/configuration-property.model'; +import { Subscription } from 'rxjs'; export const TYPE_REQUEST_FORGOT = 'forgot'; export const TYPE_REQUEST_REGISTER = 'register'; @@ -20,7 +21,7 @@ export const TYPE_REQUEST_REGISTER = 'register'; /** * Component responsible to render an email registration form. */ -export class RegisterEmailFormComponent implements OnInit { +export class RegisterEmailFormComponent implements OnDestroy, OnInit { /** * The form containing the mail address @@ -42,6 +43,8 @@ export class RegisterEmailFormComponent implements OnInit { validMailDomains: string[]; TYPE_REQUEST_REGISTER = TYPE_REQUEST_REGISTER; + subscriptions: Subscription[] = []; + constructor( private epersonRegistrationService: EpersonRegistrationService, private notificationService: NotificationsService, @@ -52,6 +55,10 @@ export class RegisterEmailFormComponent implements OnInit { ) { } + ngOnDestroy(): void { + this.subscriptions.forEach((sub: Subscription) => sub.unsubscribe()); + } + ngOnInit(): void { const validators: ValidatorFn[] = [ Validators.required, @@ -64,13 +71,13 @@ export class RegisterEmailFormComponent implements OnInit { }) }); this.validMailDomains = []; - this.configurationService.findByPropertyName('authentication-password.domain.valid') - .pipe(getAllCompletedRemoteData()) - .subscribe((remoteData: RemoteData) => { - if (remoteData.payload) { - for (const remoteValue of remoteData.payload.values) { + if (this.typeRequest === TYPE_REQUEST_REGISTER) { + this.subscriptions.push(this.configurationService.findByPropertyName('authentication-password.domain.valid') + .pipe(getAllSucceededRemoteDataPayload()) + .subscribe((configurationProperty: ConfigurationProperty) => { + for (const remoteValue of configurationProperty.values) { this.validMailDomains.push(remoteValue); - if (this.validMailDomains.length !== 0 && this.typeRequest === TYPE_REQUEST_REGISTER) { + if (this.validMailDomains.length !== 0) { this.form.get('email').setValidators([ ...validators, Validators.pattern(this.validMailDomains.map((domain: string) => '(^.*' + domain.replace(new RegExp('\\.', 'g'), '\\.') + '$)').join('|')), @@ -78,8 +85,8 @@ export class RegisterEmailFormComponent implements OnInit { this.form.updateValueAndValidity(); } } - } - }); + })); + } } /** @@ -87,7 +94,7 @@ export class RegisterEmailFormComponent implements OnInit { */ register() { if (!this.form.invalid) { - this.epersonRegistrationService.registerEmail(this.email.value, this.typeRequest).subscribe((response: RemoteData) => { + this.subscriptions.push(this.epersonRegistrationService.registerEmail(this.email.value, this.typeRequest).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); @@ -100,7 +107,7 @@ export class RegisterEmailFormComponent implements OnInit { this.notificationService.error('title', response.errorMessage); } } - ); + )); } } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 7a011ecff5..7253d09077 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3013,7 +3013,7 @@ "register-page.registration.email.error.required": "Please fill in an email address", - "register-page.registration.email.error.not-email-form": "Please fill in a valid email address. ", + "register-page.registration.email.error.not-email-form": "Please fill in a valid email address.", "register-page.registration.email.error.not-valid-domain": "Use email with allowed domains: {{ domains }}",