From caf9194f3627b69a391b235b71b14d5e387cb511 Mon Sep 17 00:00:00 2001 From: lotte Date: Wed, 12 Sep 2018 11:38:08 +0200 Subject: [PATCH] Fixes for authentication (awaiting fixes in EPerson REST endpoint) --- src/app/core/auth/auth-object-factory.ts | 8 +++--- .../auth-response-parsing.service.spec.ts | 19 +++++++++---- .../auth/auth-response-parsing.service.ts | 9 +++--- src/app/core/auth/auth.effects.spec.ts | 1 - src/app/core/auth/auth.service.spec.ts | 10 +++++-- src/app/core/auth/auth.service.ts | 28 ++++++++++++++----- src/app/core/auth/models/auth-status.model.ts | 5 ++-- .../models/normalized-auth-status.model.ts | 22 +++++++++------ src/app/core/auth/server-auth.service.ts | 11 ++++---- .../cache/models/normalized-object-factory.ts | 8 ++++++ src/app/core/data/data.service.ts | 7 +++-- .../eperson/models/NormalizedEperson.model.ts | 2 +- .../eperson/models/NormalizedGroup.model.ts | 3 +- .../auth-nav-menu/auth-nav-menu.component.ts | 1 + .../mocks/mock-remote-data-build.service.ts | 4 ++- .../testing/auth-request-service-stub.ts | 5 ++-- src/app/shared/testing/auth-service-stub.ts | 3 +- 17 files changed, 96 insertions(+), 50 deletions(-) diff --git a/src/app/core/auth/auth-object-factory.ts b/src/app/core/auth/auth-object-factory.ts index c3e70eaaac..02330ca5b3 100644 --- a/src/app/core/auth/auth-object-factory.ts +++ b/src/app/core/auth/auth-object-factory.ts @@ -1,14 +1,14 @@ import { AuthType } from './auth-type'; import { GenericConstructor } from '../shared/generic-constructor'; import { NormalizedAuthStatus } from './models/normalized-auth-status.model'; -import { NormalizedDSpaceObject } from '../cache/models/normalized-dspace-object.model'; -import { NormalizedEpersonModel } from '../eperson/models/NormalizedEperson.model'; +import { NormalizedEperson } from '../eperson/models/NormalizedEperson.model'; +import { NormalizedObject } from '../cache/models/normalized-object.model'; export class AuthObjectFactory { - public static getConstructor(type): GenericConstructor { + public static getConstructor(type): GenericConstructor { switch (type) { case AuthType.Eperson: { - return NormalizedEpersonModel + return NormalizedEperson } case AuthType.Status: { diff --git a/src/app/core/auth/auth-response-parsing.service.spec.ts b/src/app/core/auth/auth-response-parsing.service.spec.ts index f7d899a9bc..f6dd87e99a 100644 --- a/src/app/core/auth/auth-response-parsing.service.spec.ts +++ b/src/app/core/auth/auth-response-parsing.service.spec.ts @@ -8,12 +8,13 @@ import { CoreState } from '../core.reducers'; import { AuthStatus } from './models/auth-status.model'; import { AuthResponseParsingService } from './auth-response-parsing.service'; import { AuthGetRequest, AuthPostRequest } from '../data/request.models'; +import { getMockStore } from '../../shared/mocks/mock-store'; -describe('ConfigResponseParsingService', () => { +describe('AuthResponseParsingService', () => { let service: AuthResponseParsingService; - const EnvConfig = {} as GlobalConfig; - const store = {} as Store; + const EnvConfig = {cache: {msToLive: 1000}} as GlobalConfig; + const store = getMockStore() as Store; const objectCacheService = new ObjectCacheService(store); beforeEach(() => { @@ -86,13 +87,19 @@ describe('ConfigResponseParsingService', () => { type: 'eperson', uuid: '4dc70ab5-cd73-492f-b007-3179d2d9296b', _links: { - self: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/eperson/epersons/4dc70ab5-cd73-492f-b007-3179d2d9296b' + self: { + href: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/eperson/epersons/4dc70ab5-cd73-492f-b007-3179d2d9296b' + } } } }, _links: { - eperson: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/eperson/epersons/4dc70ab5-cd73-492f-b007-3179d2d9296b', - self: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/authn/status' + eperson: { + href: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/eperson/epersons/4dc70ab5-cd73-492f-b007-3179d2d9296b' + }, + self: { + href: 'https://hasselt-dspace.dev01.4science.it/dspace-spring-rest/api/authn/status' + } } }, statusCode: '200' diff --git a/src/app/core/auth/auth-response-parsing.service.ts b/src/app/core/auth/auth-response-parsing.service.ts index 80c1b2eeca..8efa36f9e2 100644 --- a/src/app/core/auth/auth-response-parsing.service.ts +++ b/src/app/core/auth/auth-response-parsing.service.ts @@ -12,22 +12,23 @@ import { ResponseParsingService } from '../data/parsing.service'; import { RestRequest } from '../data/request.models'; import { AuthType } from './auth-type'; import { AuthStatus } from './models/auth-status.model'; +import { NormalizedAuthStatus } from './models/normalized-auth-status.model'; @Injectable() export class AuthResponseParsingService extends BaseResponseParsingService implements ResponseParsingService { protected objectFactory = AuthObjectFactory; - protected toCache = false; + protected toCache = true; constructor(@Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, - protected objectCache: ObjectCacheService,) { + protected objectCache: ObjectCacheService) { super(); } parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links) && (data.statusCode === '200' || data.statusCode === 'OK')) { - const response = this.process(data.payload, request.href); - return new AuthStatusResponse(response[Object.keys(response)[0]][0], data.statusCode); + const response = this.process(data.payload, request.href); + return new AuthStatusResponse(response, data.statusCode); } else { return new AuthStatusResponse(data.payload as AuthStatus, data.statusCode); } diff --git a/src/app/core/auth/auth.effects.spec.ts b/src/app/core/auth/auth.effects.spec.ts index 3b569e523f..b862ae77fe 100644 --- a/src/app/core/auth/auth.effects.spec.ts +++ b/src/app/core/auth/auth.effects.spec.ts @@ -30,7 +30,6 @@ import { EpersonMock } from '../../shared/testing/eperson-mock'; describe('AuthEffects', () => { let authEffects: AuthEffects; let actions: Observable; - const authServiceStub = new AuthServiceStub(); const store: Store = jasmine.createSpyObj('store', { /* tslint:disable:no-empty */ diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index b54f65078e..b238bef033 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -22,6 +22,8 @@ import { Eperson } from '../eperson/models/eperson.model'; import { EpersonMock } from '../../shared/testing/eperson-mock'; import { AppState } from '../../app.reducer'; import { ClientCookieService } from '../../shared/services/client-cookie.service'; +import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-data-build.service'; describe('AuthService test', () => { @@ -44,7 +46,7 @@ describe('AuthService test', () => { authToken: token, user: EpersonMock }; - + const rdbService = getMockRemoteDataBuildService(); describe('', () => { beforeEach(() => { @@ -61,6 +63,7 @@ describe('AuthService test', () => { {provide: Router, useValue: routerStub}, {provide: ActivatedRoute, useValue: routeStub}, {provide: Store, useValue: mockStore}, + {provide: RemoteDataBuildService, useValue: rdbService}, CookieService, AuthService ], @@ -121,6 +124,7 @@ describe('AuthService test', () => { {provide: AuthRequestService, useValue: authRequest}, {provide: REQUEST, useValue: {}}, {provide: Router, useValue: routerStub}, + {provide: RemoteDataBuildService, useValue: rdbService}, CookieService ] }).compileComponents(); @@ -132,7 +136,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, authReqService, router, cookieService, store); + authService = new AuthService({}, window, authReqService, router, cookieService, store, rdbService); })); it('should return true when user is logged in', () => { @@ -191,7 +195,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, authReqService, router, cookieService, store); + authService = new AuthService({}, window, authReqService, router, cookieService, store, rdbService); storage = (authService as any).storage; spyOn(storage, 'get'); spyOn(storage, 'remove'); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 2848b54b50..f19651e0dd 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -7,7 +7,7 @@ import { RouterReducerState } from '@ngrx/router-store'; import { Store } from '@ngrx/store'; import { CookieAttributes } from 'js-cookie'; import { Observable } from 'rxjs/Observable'; -import { map, withLatestFrom } from 'rxjs/operators'; +import { map, switchMap, withLatestFrom } from 'rxjs/operators'; import { Eperson } from '../eperson/models/eperson.model'; import { AuthRequestService } from './auth-request.service'; @@ -17,11 +17,18 @@ import { AuthStatus } from './models/auth-status.model'; import { AuthTokenInfo, TOKENITEM } from './models/auth-token-info.model'; import { isEmpty, isNotEmpty, isNotNull, isNotUndefined } from '../../shared/empty.util'; import { CookieService } from '../../shared/services/cookie.service'; -import { getAuthenticationToken, getRedirectUrl, isAuthenticated, isTokenRefreshing } from './selectors'; +import { + getAuthenticationToken, + getRedirectUrl, + isAuthenticated, + isTokenRefreshing +} from './selectors'; import { AppState, routerStateSelector } from '../../app.reducer'; import { ResetAuthenticationMessagesAction, SetRedirectUrlAction } from './auth.actions'; import { NativeWindowRef, NativeWindowService } from '../../shared/services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; +import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { NormalizedEperson } from '../eperson/models/NormalizedEperson.model'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -45,7 +52,9 @@ export class AuthService { protected authRequestService: AuthRequestService, protected router: Router, protected storage: CookieService, - protected store: Store) { + protected store: Store, + protected rdbService: RemoteDataBuildService + ) { this.store.select(isAuthenticated) .startWith(false) .subscribe((authenticated: boolean) => this._authenticated = authenticated); @@ -123,14 +132,19 @@ export class AuthService { headers = headers.append('Accept', 'application/json'); headers = headers.append('Authorization', `Bearer ${token.accessToken}`); options.headers = headers; - return this.authRequestService.getRequest('status', options) - .map((status: AuthStatus) => { + return this.authRequestService.getRequest('status', options).pipe( + switchMap((status: AuthStatus) => { + if (status.authenticated) { - return status.eperson[0]; + + // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole... + const person$ = this.rdbService.buildSingle(status.eperson.toString()); + // person$.subscribe(() => console.log('test')); + return person$.pipe(map((eperson) => eperson.payload)); } else { throw(new Error('Not authenticated')); } - }); + })) } /** diff --git a/src/app/core/auth/models/auth-status.model.ts b/src/app/core/auth/models/auth-status.model.ts index 22c9d14718..bf90b82bf6 100644 --- a/src/app/core/auth/models/auth-status.model.ts +++ b/src/app/core/auth/models/auth-status.model.ts @@ -1,7 +1,8 @@ import { AuthError } from './auth-error.model'; import { AuthTokenInfo } from './auth-token-info.model'; -import { DSpaceObject } from '../../shared/dspace-object.model'; import { Eperson } from '../../eperson/models/eperson.model'; +import { RemoteData } from '../../data/remote-data'; +import { Observable } from 'rxjs/Observable'; export class AuthStatus { @@ -13,7 +14,7 @@ export class AuthStatus { error?: AuthError; - eperson: Eperson[]; + eperson: Observable>; token?: AuthTokenInfo; diff --git a/src/app/core/auth/models/normalized-auth-status.model.ts b/src/app/core/auth/models/normalized-auth-status.model.ts index 19952f7c70..f7f6ab5a9e 100644 --- a/src/app/core/auth/models/normalized-auth-status.model.ts +++ b/src/app/core/auth/models/normalized-auth-status.model.ts @@ -1,12 +1,18 @@ import { AuthStatus } from './auth-status.model'; import { autoserialize, autoserializeAs, inheritSerialization } from 'cerialize'; -import { mapsTo } from '../../cache/builders/build-decorators'; -import { NormalizedDSpaceObject } from '../../cache/models/normalized-dspace-object.model'; -import { Eperson } from '../../eperson/models/eperson.model'; +import { mapsTo, relationship } from '../../cache/builders/build-decorators'; +import { ResourceType } from '../../shared/resource-type'; +import { NormalizedObject } from '../../cache/models/normalized-object.model'; +import { IDToUUIDSerializer } from '../../cache/id-to-uuid-serializer'; @mapsTo(AuthStatus) -@inheritSerialization(NormalizedDSpaceObject) -export class NormalizedAuthStatus extends NormalizedDSpaceObject { +@inheritSerialization(NormalizedObject) +export class NormalizedAuthStatus extends NormalizedObject { + @autoserialize + id: string; + + @autoserializeAs(new IDToUUIDSerializer('auth-status'), 'id') + uuid: string; /** * True if REST API is up and running, should never return false @@ -20,7 +26,7 @@ export class NormalizedAuthStatus extends NormalizedDSpaceObject { @autoserialize authenticated: boolean; - @autoserializeAs(Eperson) - eperson: Eperson[]; - + @relationship(ResourceType.Eperson, false) + @autoserialize + eperson: string; } diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 96ee2e355a..833df4b9d2 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -1,3 +1,4 @@ +import {first, map} from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { Observable } from 'rxjs/Observable'; @@ -32,14 +33,14 @@ export class ServerAuthService extends AuthService { headers = headers.append('X-Forwarded-For', clientIp); options.headers = headers; - return this.authRequestService.getRequest('status', options) - .map((status: AuthStatus) => { + return this.authRequestService.getRequest('status', options).pipe( + map((status: AuthStatus) => { if (status.authenticated) { return status.eperson[0]; } else { throw(new Error('Not authenticated')); } - }); + })); } /** @@ -53,8 +54,8 @@ export class ServerAuthService extends AuthService { * Redirect to the route navigated before the login */ public redirectToPreviousUrl() { - this.getRedirectUrl() - .first() + this.getRedirectUrl().pipe( + first()) .subscribe((redirectUrl) => { if (isNotEmpty(redirectUrl)) { // override the route reuse strategy diff --git a/src/app/core/cache/models/normalized-object-factory.ts b/src/app/core/cache/models/normalized-object-factory.ts index df67a1f2ce..fc35dffca8 100644 --- a/src/app/core/cache/models/normalized-object-factory.ts +++ b/src/app/core/cache/models/normalized-object-factory.ts @@ -8,6 +8,8 @@ import { ResourceType } from '../../shared/resource-type'; import { NormalizedObject } from './normalized-object.model'; import { NormalizedBitstreamFormat } from './normalized-bitstream-format.model'; import { NormalizedResourcePolicy } from './normalized-resource-policy.model'; +import { NormalizedEperson } from '../../eperson/models/NormalizedEperson.model'; +import { NormalizedGroup } from '../../eperson/models/NormalizedGroup.model'; export class NormalizedObjectFactory { public static getConstructor(type: ResourceType): GenericConstructor { @@ -33,6 +35,12 @@ export class NormalizedObjectFactory { case ResourceType.ResourcePolicy: { return NormalizedResourcePolicy } + case ResourceType.Eperson: { + return NormalizedEperson + } + case ResourceType.Group: { + return NormalizedGroup + } default: { return undefined; } diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index c7588a5231..c6b2c51134 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -1,3 +1,4 @@ +import { filter, take } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; @@ -60,9 +61,9 @@ export abstract class DataService const hrefObs = this.halService.getEndpoint(this.linkPath).filter((href: string) => isNotEmpty(href)) .flatMap((endpoint: string) => this.getFindAllHref(endpoint, options)); - hrefObs - .filter((href: string) => hasValue(href)) - .take(1) + hrefObs.pipe( + filter((href: string) => hasValue(href)), + take(1)) .subscribe((href: string) => { const request = new FindAllRequest(this.requestService.generateRequestId(), href, options); this.requestService.configure(request); diff --git a/src/app/core/eperson/models/NormalizedEperson.model.ts b/src/app/core/eperson/models/NormalizedEperson.model.ts index 0c0b2490d6..bdcd069eb8 100644 --- a/src/app/core/eperson/models/NormalizedEperson.model.ts +++ b/src/app/core/eperson/models/NormalizedEperson.model.ts @@ -8,7 +8,7 @@ import { ResourceType } from '../../shared/resource-type'; @mapsTo(Eperson) @inheritSerialization(NormalizedDSpaceObject) -export class NormalizedEpersonModel extends NormalizedDSpaceObject implements CacheableObject, ListableObject { +export class NormalizedEperson extends NormalizedDSpaceObject implements CacheableObject, ListableObject { @autoserialize public handle: string; diff --git a/src/app/core/eperson/models/NormalizedGroup.model.ts b/src/app/core/eperson/models/NormalizedGroup.model.ts index 24f7da8eab..be5995d9c5 100644 --- a/src/app/core/eperson/models/NormalizedGroup.model.ts +++ b/src/app/core/eperson/models/NormalizedGroup.model.ts @@ -2,13 +2,12 @@ import { autoserialize, inheritSerialization } from 'cerialize'; import { CacheableObject } from '../../cache/object-cache.reducer'; import { ListableObject } from '../../../shared/object-collection/shared/listable-object.model'; import { NormalizedDSpaceObject } from '../../cache/models/normalized-dspace-object.model'; -import { Eperson } from './eperson.model'; import { mapsTo } from '../../cache/builders/build-decorators'; import { Group } from './group.model'; @mapsTo(Group) @inheritSerialization(NormalizedDSpaceObject) -export class NormalizedGroupModel extends NormalizedDSpaceObject implements CacheableObject, ListableObject { +export class NormalizedGroup extends NormalizedDSpaceObject implements CacheableObject, ListableObject { @autoserialize public handle: string; diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts index 1c376258fb..05dfd2d872 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts @@ -10,6 +10,7 @@ import { isNotUndefined } from '../empty.util'; import { getAuthenticatedUser, isAuthenticated, isAuthenticationLoading } from '../../core/auth/selectors'; import { Eperson } from '../../core/eperson/models/eperson.model'; import { LOGIN_ROUTE, LOGOUT_ROUTE } from '../../core/auth/auth.service'; +import { RemoteData } from '../../core/data/remote-data'; @Component({ selector: 'ds-auth-nav-menu', diff --git a/src/app/shared/mocks/mock-remote-data-build.service.ts b/src/app/shared/mocks/mock-remote-data-build.service.ts index c10032eb94..8f93fe2d96 100644 --- a/src/app/shared/mocks/mock-remote-data-build.service.ts +++ b/src/app/shared/mocks/mock-remote-data-build.service.ts @@ -5,6 +5,7 @@ import { ResponseCacheEntry } from '../../core/cache/response-cache.reducer'; import { RemoteData } from '../../core/data/remote-data'; import { RequestEntry } from '../../core/data/request.reducer'; import { hasValue } from '../empty.util'; +import { NormalizedObject } from '../../core/cache/models/normalized-object.model'; export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observable>): RemoteDataBuildService { return { @@ -17,7 +18,8 @@ export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observab payload } as RemoteData))) } - } + }, + buildSingle: (href$: string | Observable) => Observable.of(new RemoteData(false, false, true, undefined, {})) } as RemoteDataBuildService; } diff --git a/src/app/shared/testing/auth-request-service-stub.ts b/src/app/shared/testing/auth-request-service-stub.ts index 2c47068af4..4f525463c5 100644 --- a/src/app/shared/testing/auth-request-service-stub.ts +++ b/src/app/shared/testing/auth-request-service-stub.ts @@ -5,6 +5,7 @@ import { AuthTokenInfo } from '../../core/auth/models/auth-token-info.model'; import { Eperson } from '../../core/eperson/models/eperson.model'; import { isNotEmpty } from '../empty.util'; import { EpersonMock } from './eperson-mock'; +import { RemoteData } from '../../core/data/remote-data'; export class AuthRequestServiceStub { protected mockUser: Eperson = EpersonMock; @@ -26,7 +27,7 @@ export class AuthRequestServiceStub { if (this.validateToken(token)) { authStatusStub.authenticated = true; authStatusStub.token = this.mockTokenInfo; - authStatusStub.eperson = [this.mockUser]; + authStatusStub.eperson = Observable.of(new RemoteData(false, false, true, undefined, this.mockUser)); } else { authStatusStub.authenticated = false; } @@ -45,7 +46,7 @@ export class AuthRequestServiceStub { if (this.validateToken(token)) { authStatusStub.authenticated = true; authStatusStub.token = this.mockTokenInfo; - authStatusStub.eperson = [this.mockUser]; + authStatusStub.eperson = Observable.of(new RemoteData(false, false, true, undefined, this.mockUser)); } else { authStatusStub.authenticated = false; } diff --git a/src/app/shared/testing/auth-service-stub.ts b/src/app/shared/testing/auth-service-stub.ts index c7d5556910..07157c8623 100644 --- a/src/app/shared/testing/auth-service-stub.ts +++ b/src/app/shared/testing/auth-service-stub.ts @@ -3,6 +3,7 @@ import { Observable } from 'rxjs/Observable'; import { AuthTokenInfo } from '../../core/auth/models/auth-token-info.model'; import { EpersonMock } from './eperson-mock'; import { Eperson } from '../../core/eperson/models/eperson.model'; +import { RemoteData } from '../../core/data/remote-data'; export class AuthServiceStub { @@ -19,7 +20,7 @@ export class AuthServiceStub { authStatus.okay = true; authStatus.authenticated = true; authStatus.token = this.token; - authStatus.eperson = [EpersonMock]; + authStatus.eperson = Observable.of(new RemoteData(false, false, true, undefined, EpersonMock)); return Observable.of(authStatus); } else { console.log('error');