Merge pull request #1546 from atmire/Issue-1136-Password-Registration-link-fixes

Redirect to 404 page for forgot password or registration link with used tokens

src/app/core/data/eperson-registration.service.spec.ts

@@ -90,10 +90,12 @@ describe('EpersonRegistrationService', () => {
       const expected = service.searchByToken('test-token');
 
       expect(expected).toBeObservable(cold('(a|)', {
-        a: Object.assign(new Registration(), {
-          email: registrationWithUser.email,
-          token: 'test-token',
-          user: registrationWithUser.user
+        a: jasmine.objectContaining({
+          payload: Object.assign(new Registration(), {
+            email: registrationWithUser.email,
+            token: 'test-token',
+            user: registrationWithUser.user
+          })
         })
       }));
     });

src/app/core/data/eperson-registration.service.ts

@@ -79,7 +79,7 @@ export class EpersonRegistrationService {
    * Search a registration based on the provided token
    * @param token
    */
-  searchByToken(token: string): Observable<Registration> {
+  searchByToken(token: string): Observable<RemoteData<Registration>> {
     const requestId = this.requestService.generateRequestId();
 
     const href$ = this.getTokenSearchEndpoint(token).pipe(
@@ -97,15 +97,14 @@ export class EpersonRegistrationService {
     });
 
     return this.rdbService.buildSingle<Registration>(href$).pipe(
-      skipWhile((rd: RemoteData<Registration>) => rd.isStale),
-      getFirstSucceededRemoteData(),
-      map((restResponse: RemoteData<Registration>) => {
-        return Object.assign(new Registration(), {
-          email: restResponse.payload.email, token: token, user: restResponse.payload.user
-        });
-      }),
+      map((rd) => {
+        if (rd.hasSucceeded && hasValue(rd.payload)) {
+          return Object.assign(rd, { payload: Object.assign(rd.payload, { token }) });
+        } else {
+          return rd;
+        }
+      })
     );
-
   }
 
 }

src/app/forgot-password/forgot-password-form/forgot-password-form.component.html

@@ -1,4 +1,4 @@
-<div class="container">
+<div class="container" *ngIf="(registration$ |async)">
     <h3 class="mb-4">{{'forgot-password.form.head' | translate}}</h3>
     <div class="card mb-4">
         <div class="card-header">{{'forgot-password.form.identification.header' | translate}}</div>
@@ -33,4 +33,4 @@
                     (click)="submit()">{{'forgot-password.form.submit' | translate}}</button>
         </div>
     </div>
-</div>
+</div>

src/app/forgot-password/forgot-password-form/forgot-password-form.component.spec.ts

@@ -16,7 +16,11 @@ import { Registration } from '../../core/shared/registration.model';
 import { ForgotPasswordFormComponent } from './forgot-password-form.component';
 import { By } from '@angular/platform-browser';
 import { AuthenticateAction } from '../../core/auth/auth.actions';
-import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
+import {
+  createFailedRemoteDataObject$,
+  createSuccessfulRemoteDataObject,
+  createSuccessfulRemoteDataObject$
+} from '../../shared/remote-data.utils';
 
 describe('ForgotPasswordFormComponent', () => {
   let comp: ForgotPasswordFormComponent;
@@ -36,7 +40,7 @@ describe('ForgotPasswordFormComponent', () => {
 
   beforeEach(waitForAsync(() => {
 
-    route = {data: observableOf({registration: registration})};
+    route = {data: observableOf({registration: createSuccessfulRemoteDataObject(registration)})};
     router = new RouterStub();
     notificationsService = new NotificationsServiceStub();
 

src/app/forgot-password/forgot-password-form/forgot-password-form.component.ts

@@ -11,7 +11,10 @@ import { Store } from '@ngrx/store';
 import { CoreState } from '../../core/core.reducers';
 import { RemoteData } from '../../core/data/remote-data';
 import { EPerson } from '../../core/eperson/models/eperson.model';
-import { getFirstCompletedRemoteData } from '../../core/shared/operators';
+import {
+  getFirstCompletedRemoteData,
+  getFirstSucceededRemoteDataPayload,
+} from '../../core/shared/operators';
 
 @Component({
   selector: 'ds-forgot-password-form',
@@ -48,7 +51,8 @@ export class ForgotPasswordFormComponent {
 
   ngOnInit(): void {
     this.registration$ = this.route.data.pipe(
-      map((data) => data.registration as Registration),
+      map((data) => data.registration as RemoteData<Registration>),
+      getFirstSucceededRemoteDataPayload(),
     );
     this.registration$.subscribe((registration: Registration) => {
       this.email = registration.email;

src/app/forgot-password/forgot-password-routing.module.ts

@@ -1,9 +1,9 @@
 import { NgModule } from '@angular/core';
 import { RouterModule } from '@angular/router';
 import { ItemPageResolver } from '../item-page/item-page.resolver';
-import { RegistrationResolver } from '../register-email-form/registration.resolver';
 import { ThemedForgotPasswordFormComponent } from './forgot-password-form/themed-forgot-password-form.component';
 import { ThemedForgotEmailComponent } from './forgot-password-email/themed-forgot-email.component';
+import { RegistrationGuard } from '../register-page/registration.guard';
 
 @NgModule({
   imports: [
@@ -16,12 +16,11 @@ import { ThemedForgotEmailComponent } from './forgot-password-email/themed-forgo
       {
         path: ':token',
         component: ThemedForgotPasswordFormComponent,
-        resolve: {registration: RegistrationResolver}
+        canActivate: [ RegistrationGuard ],
       }
     ])
   ],
   providers: [
-    RegistrationResolver,
     ItemPageResolver,
   ]
 })

src/app/register-email-form/registration.resolver.spec.ts

@@ -1,8 +1,8 @@
 import { RegistrationResolver } from './registration.resolver';
 import { EpersonRegistrationService } from '../core/data/eperson-registration.service';
-import { of as observableOf } from 'rxjs';
 import { Registration } from '../core/shared/registration.model';
 import { first } from 'rxjs/operators';
+import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 
 describe('RegistrationResolver', () => {
   let resolver: RegistrationResolver;
@@ -13,7 +13,7 @@ describe('RegistrationResolver', () => {
 
   beforeEach(() => {
     epersonRegistrationService = jasmine.createSpyObj('epersonRegistrationService', {
-      searchByToken: observableOf(registration)
+      searchByToken: createSuccessfulRemoteDataObject$(registration)
     });
     resolver = new RegistrationResolver(epersonRegistrationService);
   });
@@ -23,9 +23,9 @@ describe('RegistrationResolver', () => {
         .pipe(first())
         .subscribe(
           (resolved) => {
-            expect(resolved.token).toEqual(token);
-            expect(resolved.email).toEqual('test@email.org');
-            expect(resolved.user).toEqual('user-uuid');
+            expect(resolved.payload.token).toEqual(token);
+            expect(resolved.payload.email).toEqual('test@email.org');
+            expect(resolved.payload.user).toEqual('user-uuid');
             done();
           }
         );

src/app/register-email-form/registration.resolver.ts

@@ -3,18 +3,22 @@ import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/r
 import { EpersonRegistrationService } from '../core/data/eperson-registration.service';
 import { Registration } from '../core/shared/registration.model';
 import { Observable } from 'rxjs';
+import { RemoteData } from '../core/data/remote-data';
+import { getFirstCompletedRemoteData } from '../core/shared/operators';
 
 @Injectable()
 /**
  * Resolver to resolve a Registration object based on the provided token
  */
-export class RegistrationResolver implements Resolve<Registration> {
+export class RegistrationResolver implements Resolve<RemoteData<Registration>> {
 
   constructor(private epersonRegistrationService: EpersonRegistrationService) {
   }
 
-  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<Registration> {
+  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<RemoteData<Registration>> {
     const token = route.params.token;
-    return this.epersonRegistrationService.searchByToken(token);
+    return this.epersonRegistrationService.searchByToken(token).pipe(
+      getFirstCompletedRemoteData(),
+    );
   }
 }

src/app/register-page/create-profile/create-profile.component.html

@@ -1,4 +1,4 @@
-<div class="container">
+<div class="container" *ngIf="(registration$ |async)">
     <h3 class="mb-4">{{'register-page.create-profile.header' | translate}}</h3>
     <div class="card mb-4">
         <div class="card-header">{{'register-page.create-profile.identification.header' | translate}}</div>

src/app/register-page/create-profile/create-profile.component.spec.ts

@@ -21,7 +21,11 @@ import {
   END_USER_AGREEMENT_METADATA_FIELD,
   EndUserAgreementService
 } from '../../core/end-user-agreement/end-user-agreement.service';
-import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
+import {
+  createFailedRemoteDataObject$,
+  createSuccessfulRemoteDataObject,
+  createSuccessfulRemoteDataObject$
+} from '../../shared/remote-data.utils';
 
 describe('CreateProfileComponent', () => {
   let comp: CreateProfileComponent;
@@ -106,7 +110,7 @@ describe('CreateProfileComponent', () => {
     };
     epersonWithAgreement = Object.assign(new EPerson(), valuesWithAgreement);
 
-    route = {data: observableOf({registration: registration})};
+    route = {data: observableOf({registration: createSuccessfulRemoteDataObject(registration)})};
     router = new RouterStub();
     notificationsService = new NotificationsServiceStub();
 

src/app/register-page/create-profile/create-profile.component.ts

@@ -19,7 +19,7 @@ import {
   END_USER_AGREEMENT_METADATA_FIELD,
   EndUserAgreementService
 } from '../../core/end-user-agreement/end-user-agreement.service';
-import { getFirstCompletedRemoteData } from '../../core/shared/operators';
+import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators';
 
 /**
  * Component that renders the create profile page to be used by a user registering through a token
@@ -56,7 +56,8 @@ export class CreateProfileComponent implements OnInit {
 
   ngOnInit(): void {
     this.registration$ = this.route.data.pipe(
-      map((data) => data.registration as Registration),
+      map((data) => data.registration as RemoteData<Registration>),
+      getFirstSucceededRemoteDataPayload(),
     );
     this.registration$.subscribe((registration: Registration) => {
       this.email = registration.email;

src/app/register-page/register-page-routing.module.ts

@@ -2,9 +2,9 @@ import { NgModule } from '@angular/core';
 import { RouterModule } from '@angular/router';
 import { RegisterEmailComponent } from './register-email/register-email.component';
 import { ItemPageResolver } from '../item-page/item-page.resolver';
-import { RegistrationResolver } from '../register-email-form/registration.resolver';
 import { EndUserAgreementCookieGuard } from '../core/end-user-agreement/end-user-agreement-cookie.guard';
 import { ThemedCreateProfileComponent } from './create-profile/themed-create-profile.component';
+import { RegistrationGuard } from './registration.guard';
 
 @NgModule({
   imports: [
@@ -17,13 +17,14 @@ import { ThemedCreateProfileComponent } from './create-profile/themed-create-pro
       {
         path: ':token',
         component: ThemedCreateProfileComponent,
-        resolve: {registration: RegistrationResolver},
-        canActivate: [EndUserAgreementCookieGuard]
+        canActivate: [
+          RegistrationGuard,
+          EndUserAgreementCookieGuard,
+        ],
       }
     ])
   ],
   providers: [
-    RegistrationResolver,
     ItemPageResolver
   ]
 })

src/app/register-page/registration.guard.spec.ts

@@ -0,0 +1,106 @@
+import { RegistrationGuard } from './registration.guard';
+import { EpersonRegistrationService } from '../core/data/eperson-registration.service';
+import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
+import { AuthService } from '../core/auth/auth.service';
+import { Location } from '@angular/common';
+import {
+  createFailedRemoteDataObject$,
+  createSuccessfulRemoteDataObject,
+} from '../shared/remote-data.utils';
+import { Registration } from '../core/shared/registration.model';
+import { of as observableOf } from 'rxjs/internal/observable/of';
+import { RemoteData } from '../core/data/remote-data';
+
+describe('RegistrationGuard', () => {
+  let guard: RegistrationGuard;
+
+  let epersonRegistrationService: EpersonRegistrationService;
+  let router: Router;
+  let authService: AuthService;
+
+  let registration: Registration;
+  let registrationRD: RemoteData<Registration>;
+  let currentUrl: string;
+
+  let startingRouteData: any;
+  let route: ActivatedRouteSnapshot;
+  let state: RouterStateSnapshot;
+
+  beforeEach(() => {
+    registration = Object.assign(new Registration(), {
+      email: 'test@email.com',
+      token: 'testToken',
+      user: 'testUser',
+    });
+    registrationRD = createSuccessfulRemoteDataObject(registration);
+    currentUrl = 'test-current-url';
+
+    startingRouteData = {
+      existingData: 'some-existing-data',
+    };
+    route = Object.assign(new ActivatedRouteSnapshot(), {
+      data: Object.assign({}, startingRouteData),
+      params: {
+        token: 'testToken',
+      },
+    });
+    state = Object.assign({
+      url: currentUrl,
+    });
+
+    epersonRegistrationService = jasmine.createSpyObj('epersonRegistrationService', {
+      searchByToken: observableOf(registrationRD),
+    });
+    router = jasmine.createSpyObj('router', {
+      navigateByUrl: Promise.resolve(),
+    }, {
+      url: currentUrl,
+    });
+    authService = jasmine.createSpyObj('authService', {
+      isAuthenticated: observableOf(false),
+      setRedirectUrl: {},
+    });
+
+    guard = new RegistrationGuard(epersonRegistrationService, router, authService);
+  });
+
+  describe('canActivate', () => {
+    describe('when searchByToken returns a successful response', () => {
+      beforeEach(() => {
+        (epersonRegistrationService.searchByToken as jasmine.Spy).and.returnValue(observableOf(registrationRD));
+      });
+
+      it('should return true', (done) => {
+        guard.canActivate(route, state).subscribe((result) => {
+          expect(result).toEqual(true);
+          done();
+        });
+      });
+
+      it('should add the response to the route\'s data', (done) => {
+        guard.canActivate(route, state).subscribe(() => {
+          expect(route.data).toEqual({ ...startingRouteData, registration: registrationRD });
+          done();
+        });
+      });
+
+      it('should not redirect', (done) => {
+        guard.canActivate(route, state).subscribe(() => {
+          expect(router.navigateByUrl).not.toHaveBeenCalled();
+          done();
+        });
+      });
+    });
+
+    describe('when searchByToken returns a 404 response', () => {
+      beforeEach(() => {
+        (epersonRegistrationService.searchByToken as jasmine.Spy).and.returnValue(createFailedRemoteDataObject$('Not Found', 404));
+      });
+
+      it('should redirect', () => {
+        guard.canActivate(route, state).subscribe();
+        expect(router.navigateByUrl).toHaveBeenCalled();
+      });
+    });
+  });
+});

src/app/register-page/registration.guard.ts

@@ -0,0 +1,43 @@
+import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router';
+import { Injectable } from '@angular/core';
+import { Observable } from 'rxjs/internal/Observable';
+import { EpersonRegistrationService } from '../core/data/eperson-registration.service';
+import { AuthService } from '../core/auth/auth.service';
+import { map } from 'rxjs/operators';
+import { getFirstCompletedRemoteData, redirectOn4xx } from '../core/shared/operators';
+import { Location } from '@angular/common';
+
+@Injectable({
+  providedIn: 'root'
+})
+/**
+ * A guard responsible for redirecting to 4xx pages upon retrieving a Registration object
+ * The guard also adds the resulting RemoteData<Registration> object to the route's data for further usage in components
+ * The reason this is a guard and not a resolver, is because it has to run before the EndUserAgreementCookieGuard
+ */
+export class RegistrationGuard implements CanActivate {
+  constructor(private epersonRegistrationService: EpersonRegistrationService,
+              private router: Router,
+              private authService: AuthService) {
+  }
+
+  /**
+   * Can the user activate the route? Returns true if the provided token resolves to an existing Registration, false if
+   * not. Redirects to 4xx page on 4xx error. Adds the resulting RemoteData<Registration> object to the route's
+   * data.registration property
+   * @param route
+   * @param state
+   */
+  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
+    const token = route.params.token;
+    return this.epersonRegistrationService.searchByToken(token).pipe(
+      getFirstCompletedRemoteData(),
+      redirectOn4xx(this.router, this.authService),
+      map((rd) => {
+        route.data = { ...route.data, registration: rd };
+        return rd.hasSucceeded;
+      }),
+    );
+  }
+
+}