From caf9194f3627b69a391b235b71b14d5e387cb511 Mon Sep 17 00:00:00 2001 From: lotte Date: Wed, 12 Sep 2018 11:38:08 +0200 Subject: [PATCH 1/8] 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'); From 1f336f29fdf6941d8f35ddb94a0a7715008cfafc Mon Sep 17 00:00:00 2001 From: lotte Date: Wed, 12 Sep 2018 16:11:04 +0200 Subject: [PATCH 2/8] small fixes for authentication --- src/app/core/auth/auth-object-factory.ts | 7 ++--- src/app/core/auth/auth-type.ts | 2 +- src/app/core/auth/auth.actions.ts | 14 +++++----- src/app/core/auth/auth.effects.spec.ts | 4 +-- src/app/core/auth/auth.effects.ts | 6 ++--- src/app/core/auth/auth.reducer.spec.ts | 26 +++++++++---------- src/app/core/auth/auth.reducer.ts | 4 +-- src/app/core/auth/auth.service.spec.ts | 10 +++---- src/app/core/auth/auth.service.ts | 14 +++++----- src/app/core/auth/models/auth-status.model.ts | 4 +-- .../models/normalized-auth-status.model.ts | 2 +- src/app/core/auth/server-auth.service.ts | 20 +++++++++----- .../cache/models/normalized-object-factory.ts | 8 +++--- .../data/base-response-parsing.service.ts | 22 ++++++++++++++++ src/app/core/eperson/models/eperson.model.ts | 2 +- ...n.model.ts => normalized-eperson.model.ts} | 6 ++--- ...oup.model.ts => normalized-group.model.ts} | 0 src/app/core/shared/resource-type.ts | 2 +- .../auth-nav-menu.component.spec.ts | 5 ++-- .../auth-nav-menu/auth-nav-menu.component.ts | 4 +-- .../shared/log-in/log-in.component.spec.ts | 8 +++--- .../shared/log-out/log-out.component.spec.ts | 8 +++--- .../testing/auth-request-service-stub.ts | 10 +++---- src/app/shared/testing/auth-service-stub.ts | 10 +++---- src/app/shared/testing/eperson-mock.ts | 4 +-- 25 files changed, 115 insertions(+), 87 deletions(-) rename src/app/core/eperson/models/{NormalizedEperson.model.ts => normalized-eperson.model.ts} (88%) rename src/app/core/eperson/models/{NormalizedGroup.model.ts => normalized-group.model.ts} (100%) diff --git a/src/app/core/auth/auth-object-factory.ts b/src/app/core/auth/auth-object-factory.ts index 02330ca5b3..b6df1fac34 100644 --- a/src/app/core/auth/auth-object-factory.ts +++ b/src/app/core/auth/auth-object-factory.ts @@ -1,14 +1,15 @@ import { AuthType } from './auth-type'; import { GenericConstructor } from '../shared/generic-constructor'; import { NormalizedAuthStatus } from './models/normalized-auth-status.model'; -import { NormalizedEperson } from '../eperson/models/NormalizedEperson.model'; +import { NormalizedEPerson } from '../eperson/models/normalized-eperson.model'; import { NormalizedObject } from '../cache/models/normalized-object.model'; +import { EPerson } from '../eperson/models/eperson.model'; export class AuthObjectFactory { public static getConstructor(type): GenericConstructor { switch (type) { - case AuthType.Eperson: { - return NormalizedEperson + case AuthType.EPerson: { + return NormalizedEPerson } case AuthType.Status: { diff --git a/src/app/core/auth/auth-type.ts b/src/app/core/auth/auth-type.ts index b8879ae445..9a248da91f 100644 --- a/src/app/core/auth/auth-type.ts +++ b/src/app/core/auth/auth-type.ts @@ -1,4 +1,4 @@ export enum AuthType { - Eperson = 'eperson', + EPerson = 'eperson', Status = 'status' } diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index 207e8fae70..d0969d38d4 100644 --- a/src/app/core/auth/auth.actions.ts +++ b/src/app/core/auth/auth.actions.ts @@ -5,7 +5,7 @@ import { Action } from '@ngrx/store'; import { type } from '../../shared/ngrx/type'; // import models -import { Eperson } from '../eperson/models/eperson.model'; +import { EPerson } from '../eperson/models/eperson.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; export const AuthActionTypes = { @@ -76,10 +76,10 @@ export class AuthenticatedSuccessAction implements Action { payload: { authenticated: boolean; authToken: AuthTokenInfo; - user: Eperson + user: EPerson }; - constructor(authenticated: boolean, authToken: AuthTokenInfo, user: Eperson) { + constructor(authenticated: boolean, authToken: AuthTokenInfo, user: EPerson) { this.payload = { authenticated, authToken, user }; } } @@ -250,9 +250,9 @@ export class RefreshTokenErrorAction implements Action { */ export class RegistrationAction implements Action { public type: string = AuthActionTypes.REGISTRATION; - payload: Eperson; + payload: EPerson; - constructor(user: Eperson) { + constructor(user: EPerson) { this.payload = user; } } @@ -278,9 +278,9 @@ export class RegistrationErrorAction implements Action { */ export class RegistrationSuccessAction implements Action { public type: string = AuthActionTypes.REGISTRATION_SUCCESS; - payload: Eperson; + payload: EPerson; - constructor(user: Eperson) { + constructor(user: EPerson) { this.payload = user; } } diff --git a/src/app/core/auth/auth.effects.spec.ts b/src/app/core/auth/auth.effects.spec.ts index b862ae77fe..d75b407516 100644 --- a/src/app/core/auth/auth.effects.spec.ts +++ b/src/app/core/auth/auth.effects.spec.ts @@ -25,7 +25,7 @@ import { AuthServiceStub } from '../../shared/testing/auth-service-stub'; import { AuthService } from './auth.service'; import { TruncatablesState } from '../../shared/truncatable/truncatable.reducer'; -import { EpersonMock } from '../../shared/testing/eperson-mock'; +import { EPersonMock } from '../../shared/testing/eperson-mock'; describe('AuthEffects', () => { let authEffects: AuthEffects; @@ -104,7 +104,7 @@ describe('AuthEffects', () => { it('should return a AUTHENTICATED_SUCCESS action in response to a AUTHENTICATED action', () => { actions = hot('--a-', {a: {type: AuthActionTypes.AUTHENTICATED, payload: token}}); - const expected = cold('--b-', {b: new AuthenticatedSuccessAction(true, token, EpersonMock)}); + const expected = cold('--b-', {b: new AuthenticatedSuccessAction(true, token, EPersonMock)}); expect(authEffects.authenticated$).toBeObservable(expected); }); diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index e2d6c80b5e..82def702eb 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/src/app/core/auth/auth.effects.ts @@ -28,7 +28,7 @@ import { RegistrationErrorAction, RegistrationSuccessAction } from './auth.actions'; -import { Eperson } from '../eperson/models/eperson.model'; +import { EPerson } from '../eperson/models/eperson.model'; import { AuthStatus } from './models/auth-status.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { AppState } from '../../app.reducer'; @@ -63,7 +63,7 @@ export class AuthEffects { .ofType(AuthActionTypes.AUTHENTICATED) .switchMap((action: AuthenticatedAction) => { return this.authService.authenticatedUser(action.payload) - .map((user: Eperson) => new AuthenticatedSuccessAction((user !== null), action.payload, user)) + .map((user: EPerson) => new AuthenticatedSuccessAction((user !== null), action.payload, user)) .catch((error) => Observable.of(new AuthenticatedErrorAction(error))); }); @@ -88,7 +88,7 @@ export class AuthEffects { .debounceTime(500) // to remove when functionality is implemented .switchMap((action: RegistrationAction) => { return this.authService.create(action.payload) - .map((user: Eperson) => new RegistrationSuccessAction(user)) + .map((user: EPerson) => new RegistrationSuccessAction(user)) .catch((error) => Observable.of(new RegistrationErrorAction(error))); }); diff --git a/src/app/core/auth/auth.reducer.spec.ts b/src/app/core/auth/auth.reducer.spec.ts index f148f3ac8d..ca2ba00036 100644 --- a/src/app/core/auth/auth.reducer.spec.ts +++ b/src/app/core/auth/auth.reducer.spec.ts @@ -21,7 +21,7 @@ import { SetRedirectUrlAction } from './auth.actions'; import { AuthTokenInfo } from './models/auth-token-info.model'; -import { EpersonMock } from '../../shared/testing/eperson-mock'; +import { EPersonMock } from '../../shared/testing/eperson-mock'; describe('authReducer', () => { @@ -107,7 +107,7 @@ describe('authReducer', () => { loading: true, info: undefined }; - const action = new AuthenticatedSuccessAction(true, mockTokenInfo, EpersonMock); + const action = new AuthenticatedSuccessAction(true, mockTokenInfo, EPersonMock); const newState = authReducer(initialState, action); state = { authenticated: true, @@ -116,7 +116,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; expect(newState).toEqual(state); }); @@ -182,7 +182,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; const action = new LogOutAction(); @@ -199,7 +199,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; const action = new LogOutSuccessAction(); @@ -225,7 +225,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; const action = new LogOutErrorAction(mockError); @@ -237,7 +237,7 @@ describe('authReducer', () => { error: 'Test error message', loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; expect(newState).toEqual(state); }); @@ -250,7 +250,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; const newTokenInfo = new AuthTokenInfo('Refreshed token'); const action = new RefreshTokenAction(newTokenInfo); @@ -262,7 +262,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock, + user: EPersonMock, refreshing: true }; expect(newState).toEqual(state); @@ -276,7 +276,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock, + user: EPersonMock, refreshing: true }; const newTokenInfo = new AuthTokenInfo('Refreshed token'); @@ -289,7 +289,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock, + user: EPersonMock, refreshing: false }; expect(newState).toEqual(state); @@ -303,7 +303,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock, + user: EPersonMock, refreshing: true }; const action = new RefreshTokenErrorAction(); @@ -329,7 +329,7 @@ describe('authReducer', () => { error: undefined, loading: false, info: undefined, - user: EpersonMock + user: EPersonMock }; state = { diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index 0c5e36ce91..98827d842e 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -12,7 +12,7 @@ import { SetRedirectUrlAction } from './auth.actions'; // import models -import { Eperson } from '../eperson/models/eperson.model'; +import { EPerson } from '../eperson/models/eperson.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; /** @@ -46,7 +46,7 @@ export interface AuthState { refreshing?: boolean; // the authenticated user - user?: Eperson; + user?: EPerson; } /** diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index b238bef033..c943a815e7 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -18,8 +18,8 @@ import { AuthRequestServiceStub } from '../../shared/testing/auth-request-servic import { AuthRequestService } from './auth-request.service'; import { AuthStatus } from './models/auth-status.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; -import { Eperson } from '../eperson/models/eperson.model'; -import { EpersonMock } from '../../shared/testing/eperson-mock'; +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'; @@ -44,7 +44,7 @@ describe('AuthService test', () => { loaded: true, loading: false, authToken: token, - user: EpersonMock + user: EPersonMock }; const rdbService = getMockRemoteDataBuildService(); describe('', () => { @@ -82,7 +82,7 @@ describe('AuthService test', () => { }); it('should return the authenticated user object when user token is valid', () => { - authService.authenticatedUser(new AuthTokenInfo('test_token')).subscribe((user: Eperson) => { + authService.authenticatedUser(new AuthTokenInfo('test_token')).subscribe((user: EPerson) => { expect(user).toBeDefined(); }); }); @@ -188,7 +188,7 @@ describe('AuthService test', () => { loaded: true, loading: false, authToken: expiredToken, - user: EpersonMock + user: EPersonMock }; store .subscribe((state) => { diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index f19651e0dd..ea73ff9e1b 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -9,7 +9,7 @@ import { CookieAttributes } from 'js-cookie'; import { Observable } from 'rxjs/Observable'; import { map, switchMap, withLatestFrom } from 'rxjs/operators'; -import { Eperson } from '../eperson/models/eperson.model'; +import { EPerson } from '../eperson/models/eperson.model'; import { AuthRequestService } from './auth-request.service'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; @@ -28,7 +28,7 @@ import { ResetAuthenticationMessagesAction, SetRedirectUrlAction } from './auth. 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'; +import { NormalizedEPerson } from '../eperson/models/normalized-eperson.model'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -107,7 +107,7 @@ export class AuthService { if (status.authenticated) { return status; } else { - throw(new Error('Invalid email or password')); + Observable.throw(new Error('Invalid email or password')); } }) @@ -125,7 +125,7 @@ export class AuthService { * Returns the authenticated user * @returns {User} */ - public authenticatedUser(token: AuthTokenInfo): Observable { + public authenticatedUser(token: AuthTokenInfo): Observable { // Determine if the user has an existing auth session on the server const options: HttpOptions = Object.create({}); let headers = new HttpHeaders(); @@ -136,10 +136,8 @@ export class AuthService { switchMap((status: AuthStatus) => { if (status.authenticated) { - // 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')); + const person$ = this.rdbService.buildSingle(status.eperson.toString()); return person$.pipe(map((eperson) => eperson.payload)); } else { throw(new Error('Not authenticated')); @@ -202,7 +200,7 @@ export class AuthService { * Create a new user * @returns {User} */ - public create(user: Eperson): Observable { + public create(user: EPerson): Observable { // Normally you would do an HTTP request to POST the user // details and then return the new user object // but, let's just return the new user for this example. diff --git a/src/app/core/auth/models/auth-status.model.ts b/src/app/core/auth/models/auth-status.model.ts index bf90b82bf6..b8ccf9ed6d 100644 --- a/src/app/core/auth/models/auth-status.model.ts +++ b/src/app/core/auth/models/auth-status.model.ts @@ -1,6 +1,6 @@ import { AuthError } from './auth-error.model'; import { AuthTokenInfo } from './auth-token-info.model'; -import { Eperson } from '../../eperson/models/eperson.model'; +import { EPerson } from '../../eperson/models/eperson.model'; import { RemoteData } from '../../data/remote-data'; import { Observable } from 'rxjs/Observable'; @@ -14,7 +14,7 @@ export class AuthStatus { error?: AuthError; - eperson: Observable>; + 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 f7f6ab5a9e..b8dd2aa23e 100644 --- a/src/app/core/auth/models/normalized-auth-status.model.ts +++ b/src/app/core/auth/models/normalized-auth-status.model.ts @@ -26,7 +26,7 @@ export class NormalizedAuthStatus extends NormalizedObject { @autoserialize authenticated: boolean; - @relationship(ResourceType.Eperson, false) + @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 833df4b9d2..8d1bf1673c 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -1,4 +1,4 @@ -import {first, map} from 'rxjs/operators'; +import { first, map, switchMap } from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { Observable } from 'rxjs/Observable'; @@ -9,7 +9,8 @@ import { isNotEmpty } from '../../shared/empty.util'; import { AuthService } from './auth.service'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { CheckAuthenticationTokenAction } from './auth.actions'; -import { Eperson } from '../eperson/models/eperson.model'; +import { EPerson } from '../eperson/models/eperson.model'; +import { NormalizedEPerson } from '../eperson/models/normalized-eperson.model'; /** * The auth service. @@ -21,7 +22,7 @@ export class ServerAuthService extends AuthService { * Returns the authenticated user * @returns {User} */ - public authenticatedUser(token: AuthTokenInfo): Observable { + public authenticatedUser(token: AuthTokenInfo): Observable { // Determine if the user has an existing auth session on the server const options: HttpOptions = Object.create({}); let headers = new HttpHeaders(); @@ -34,13 +35,18 @@ export class ServerAuthService extends AuthService { options.headers = headers; return this.authRequestService.getRequest('status', options).pipe( - map((status: AuthStatus) => { + 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')); + Observable.throw(new Error('Not authenticated')); } - })); + })) } /** diff --git a/src/app/core/cache/models/normalized-object-factory.ts b/src/app/core/cache/models/normalized-object-factory.ts index fc35dffca8..5c5ebf50aa 100644 --- a/src/app/core/cache/models/normalized-object-factory.ts +++ b/src/app/core/cache/models/normalized-object-factory.ts @@ -8,8 +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'; +import { NormalizedEPerson } from '../../eperson/models/normalized-eperson.model'; +import { NormalizedGroup } from '../../eperson/models/normalized-group.model'; export class NormalizedObjectFactory { public static getConstructor(type: ResourceType): GenericConstructor { @@ -35,8 +35,8 @@ export class NormalizedObjectFactory { case ResourceType.ResourcePolicy: { return NormalizedResourcePolicy } - case ResourceType.Eperson: { - return NormalizedEperson + case ResourceType.EPerson: { + return NormalizedEPerson } case ResourceType.Group: { return NormalizedGroup diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index 050b3c2da5..bdd18f8a61 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/src/app/core/data/base-response-parsing.service.ts @@ -7,6 +7,8 @@ import { GlobalConfig } from '../../../config/global-config.interface'; import { GenericConstructor } from '../shared/generic-constructor'; import { PaginatedList } from './paginated-list'; import { NormalizedObject } from '../cache/models/normalized-object.model'; +import { ResourceType } from '../shared/resource-type'; +import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; function isObjectLevel(halObj: any) { return isNotEmpty(halObj._links) && hasValue(halObj._links.self); @@ -34,6 +36,7 @@ export abstract class BaseResponseParsingService { } else if (Array.isArray(data)) { return this.processArray(data, requestHref); } else if (isObjectLevel(data)) { + data = this.fixBadEPersonRestResponse(data); const object = this.deserialize(data); if (isNotEmpty(data._embedded)) { Object @@ -53,6 +56,7 @@ export abstract class BaseResponseParsingService { } }); } + this.cache(object, requestHref); return object; } @@ -145,4 +149,22 @@ export abstract class BaseResponseParsingService { } return obj[keys[0]]; } + + /* TODO remove when REST response for epersons is fixed */ + private fixBadEPersonRestResponse(obj: any): any { + if (obj.type === ResourceType.EPerson) { + const groups = obj.groups; + const normGroups = []; + if (isNotEmpty(groups)) { + groups.forEach((group) => { + const parts = ['eperson', 'groups', group.uuid]; + const href = new RESTURLCombiner(this.EnvConfig, ...parts).toString(); + normGroups.push(href); + } + ) + } + return Object.assign({}, obj, { groups: normGroups }); + } + return obj; + } } diff --git a/src/app/core/eperson/models/eperson.model.ts b/src/app/core/eperson/models/eperson.model.ts index 373fb42792..45d26761b0 100644 --- a/src/app/core/eperson/models/eperson.model.ts +++ b/src/app/core/eperson/models/eperson.model.ts @@ -1,7 +1,7 @@ import { DSpaceObject } from '../../shared/dspace-object.model'; import { Group } from './group.model'; -export class Eperson extends DSpaceObject { +export class EPerson extends DSpaceObject { public handle: string; diff --git a/src/app/core/eperson/models/NormalizedEperson.model.ts b/src/app/core/eperson/models/normalized-eperson.model.ts similarity index 88% rename from src/app/core/eperson/models/NormalizedEperson.model.ts rename to src/app/core/eperson/models/normalized-eperson.model.ts index bdcd069eb8..9d0fa428e9 100644 --- a/src/app/core/eperson/models/NormalizedEperson.model.ts +++ b/src/app/core/eperson/models/normalized-eperson.model.ts @@ -2,13 +2,13 @@ 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 { EPerson } from './eperson.model'; import { mapsTo, relationship } from '../../cache/builders/build-decorators'; import { ResourceType } from '../../shared/resource-type'; -@mapsTo(Eperson) +@mapsTo(EPerson) @inheritSerialization(NormalizedDSpaceObject) -export class NormalizedEperson 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/normalized-group.model.ts similarity index 100% rename from src/app/core/eperson/models/NormalizedGroup.model.ts rename to src/app/core/eperson/models/normalized-group.model.ts diff --git a/src/app/core/shared/resource-type.ts b/src/app/core/shared/resource-type.ts index 71053f628b..e67f3339de 100644 --- a/src/app/core/shared/resource-type.ts +++ b/src/app/core/shared/resource-type.ts @@ -6,7 +6,7 @@ export enum ResourceType { Item = 'item', Collection = 'collection', Community = 'community', - Eperson = 'eperson', + EPerson = 'eperson', Group = 'group', ResourcePolicy = 'resourcePolicy' } diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts index 8b9f7c8775..1c59c02346 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts @@ -5,7 +5,7 @@ import { By } from '@angular/platform-browser'; import { Store, StoreModule } from '@ngrx/store'; import { authReducer, AuthState } from '../../core/auth/auth.reducer'; -import { EpersonMock } from '../testing/eperson-mock'; +import { EPersonMock } from '../testing/eperson-mock'; import { TranslateModule } from '@ngx-translate/core'; import { AppState } from '../../app.reducer'; import { AuthNavMenuComponent } from './auth-nav-menu.component'; @@ -13,6 +13,7 @@ import { HostWindowServiceStub } from '../testing/host-window-service-stub'; import { HostWindowService } from '../host-window.service'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { AuthTokenInfo } from '../../core/auth/models/auth-token-info.model'; +import { EPerson } from '../../core/eperson/models/eperson.model'; describe('AuthNavMenuComponent', () => { @@ -31,7 +32,7 @@ describe('AuthNavMenuComponent', () => { loaded: true, loading: false, authToken: new AuthTokenInfo('test_token'), - user: EpersonMock + user: EPersonMock }; let routerState = { url: '/home' 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 05dfd2d872..887729a7af 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 @@ -8,7 +8,7 @@ import { HostWindowService } from '../host-window.service'; import { AppState, routerStateSelector } from '../../app.reducer'; import { isNotUndefined } from '../empty.util'; import { getAuthenticatedUser, isAuthenticated, isAuthenticationLoading } from '../../core/auth/selectors'; -import { Eperson } from '../../core/eperson/models/eperson.model'; +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'; @@ -35,7 +35,7 @@ export class AuthNavMenuComponent implements OnInit { public showAuth = Observable.of(false); - public user: Observable; + public user: Observable; constructor(private store: Store, private windowService: HostWindowService) { diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index dc4a0be1c6..dd2aea35d5 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -7,8 +7,8 @@ import { Store, StoreModule } from '@ngrx/store'; import { LogInComponent } from './log-in.component'; import { authReducer } from '../../core/auth/auth.reducer'; -import { EpersonMock } from '../testing/eperson-mock'; -import { Eperson } from '../../core/eperson/models/eperson.model'; +import { EPersonMock } from '../testing/eperson-mock'; +import { EPerson } from '../../core/eperson/models/eperson.model'; import { TranslateModule } from '@ngx-translate/core'; import { AuthService } from '../../core/auth/auth.service'; import { AuthServiceStub } from '../testing/auth-service-stub'; @@ -19,7 +19,7 @@ describe('LogInComponent', () => { let component: LogInComponent; let fixture: ComponentFixture; let page: Page; - let user: Eperson; + let user: EPerson; const authState = { authenticated: false, @@ -28,7 +28,7 @@ describe('LogInComponent', () => { }; beforeEach(() => { - user = EpersonMock; + user = EPersonMock; }); beforeEach(async(() => { diff --git a/src/app/shared/log-out/log-out.component.spec.ts b/src/app/shared/log-out/log-out.component.spec.ts index ad609f0aea..94780ead5a 100644 --- a/src/app/shared/log-out/log-out.component.spec.ts +++ b/src/app/shared/log-out/log-out.component.spec.ts @@ -5,8 +5,8 @@ import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { Store, StoreModule } from '@ngrx/store'; import { authReducer } from '../../core/auth/auth.reducer'; -import { EpersonMock } from '../testing/eperson-mock'; -import { Eperson } from '../../core/eperson/models/eperson.model'; +import { EPersonMock } from '../testing/eperson-mock'; +import { EPerson } from '../../core/eperson/models/eperson.model'; import { TranslateModule } from '@ngx-translate/core'; import { Router } from '@angular/router'; import { AppState } from '../../app.reducer'; @@ -18,7 +18,7 @@ describe('LogOutComponent', () => { let component: LogOutComponent; let fixture: ComponentFixture; let page: Page; - let user: Eperson; + let user: EPerson; const authState = { authenticated: false, @@ -28,7 +28,7 @@ describe('LogOutComponent', () => { const routerStub = new RouterStub(); beforeEach(() => { - user = EpersonMock; + user = EPersonMock; }); beforeEach(async(() => { diff --git a/src/app/shared/testing/auth-request-service-stub.ts b/src/app/shared/testing/auth-request-service-stub.ts index 4f525463c5..576b8c2ddf 100644 --- a/src/app/shared/testing/auth-request-service-stub.ts +++ b/src/app/shared/testing/auth-request-service-stub.ts @@ -2,13 +2,13 @@ import { Observable } from 'rxjs/Observable'; import { HttpOptions } from '../../core/dspace-rest-v2/dspace-rest-v2.service'; import { AuthStatus } from '../../core/auth/models/auth-status.model'; import { AuthTokenInfo } from '../../core/auth/models/auth-token-info.model'; -import { Eperson } from '../../core/eperson/models/eperson.model'; +import { EPerson } from '../../core/eperson/models/eperson.model'; import { isNotEmpty } from '../empty.util'; -import { EpersonMock } from './eperson-mock'; +import { EPersonMock } from './eperson-mock'; import { RemoteData } from '../../core/data/remote-data'; export class AuthRequestServiceStub { - protected mockUser: Eperson = EpersonMock; + protected mockUser: EPerson = EPersonMock; protected mockTokenInfo = new AuthTokenInfo('test_token'); public postToEndpoint(method: string, body: any, options?: HttpOptions): Observable { @@ -27,7 +27,7 @@ export class AuthRequestServiceStub { if (this.validateToken(token)) { authStatusStub.authenticated = true; authStatusStub.token = this.mockTokenInfo; - authStatusStub.eperson = Observable.of(new RemoteData(false, false, true, undefined, this.mockUser)); + authStatusStub.eperson = Observable.of(new RemoteData(false, false, true, undefined, this.mockUser)); } else { authStatusStub.authenticated = false; } @@ -46,7 +46,7 @@ export class AuthRequestServiceStub { if (this.validateToken(token)) { authStatusStub.authenticated = true; authStatusStub.token = this.mockTokenInfo; - authStatusStub.eperson = Observable.of(new RemoteData(false, false, true, undefined, 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 07157c8623..d6c20c9520 100644 --- a/src/app/shared/testing/auth-service-stub.ts +++ b/src/app/shared/testing/auth-service-stub.ts @@ -1,8 +1,8 @@ import { AuthStatus } from '../../core/auth/models/auth-status.model'; 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 { EPersonMock } from './eperson-mock'; +import { EPerson } from '../../core/eperson/models/eperson.model'; import { RemoteData } from '../../core/data/remote-data'; export class AuthServiceStub { @@ -20,7 +20,7 @@ export class AuthServiceStub { authStatus.okay = true; authStatus.authenticated = true; authStatus.token = this.token; - authStatus.eperson = Observable.of(new RemoteData(false, false, true, undefined, EpersonMock)); + authStatus.eperson = Observable.of(new RemoteData(false, false, true, undefined, EPersonMock)); return Observable.of(authStatus); } else { console.log('error'); @@ -28,9 +28,9 @@ export class AuthServiceStub { } } - public authenticatedUser(token: AuthTokenInfo): Observable { + public authenticatedUser(token: AuthTokenInfo): Observable { if (token.accessToken === 'token_test') { - return Observable.of(EpersonMock); + return Observable.of(EPersonMock); } else { throw(new Error('Message Error test')); } diff --git a/src/app/shared/testing/eperson-mock.ts b/src/app/shared/testing/eperson-mock.ts index 9cf938fcf2..f163a490b9 100644 --- a/src/app/shared/testing/eperson-mock.ts +++ b/src/app/shared/testing/eperson-mock.ts @@ -1,6 +1,6 @@ -import { Eperson } from '../../core/eperson/models/eperson.model'; +import { EPerson } from '../../core/eperson/models/eperson.model'; -export const EpersonMock: Eperson = Object.assign(new Eperson(),{ +export const EPersonMock: EPerson = Object.assign(new EPerson(),{ handle: null, groups: [], netid: 'test@test.com', From 2b1d4cd12a6c16e10ddd06d6e269bfd31a21b45c Mon Sep 17 00:00:00 2001 From: lotte Date: Fri, 14 Sep 2018 11:42:55 +0200 Subject: [PATCH 3/8] Fixed logout issue --- src/app/core/auth/auth.interceptor.ts | 2 +- src/app/core/auth/auth.service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/auth/auth.interceptor.ts b/src/app/core/auth/auth.interceptor.ts index 651e2fd096..f38abb90bc 100644 --- a/src/app/core/auth/auth.interceptor.ts +++ b/src/app/core/auth/auth.interceptor.ts @@ -35,7 +35,7 @@ export class AuthInterceptor implements HttpInterceptor { } private isSuccess(response: HttpResponseBase): boolean { - return response.status === 200; + return (response.status === 200 || response.status === 204); } private isAuthRequest(http: HttpRequest | HttpResponseBase): boolean { diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index ea73ff9e1b..f8fc863ee9 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -140,7 +140,7 @@ export class AuthService { const person$ = this.rdbService.buildSingle(status.eperson.toString()); return person$.pipe(map((eperson) => eperson.payload)); } else { - throw(new Error('Not authenticated')); + Observable.throw(new Error('Not authenticated')); } })) } From 97669d0c346b11055ae322069960d65364e989b2 Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 17 Sep 2018 10:20:12 +0200 Subject: [PATCH 4/8] upgrade ngrx to fix store devtools bug --- package.json | 6 +++--- yarn.lock | 32 ++++++++++++++++---------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index 0936b27ea4..e1ff94b428 100644 --- a/package.json +++ b/package.json @@ -90,8 +90,8 @@ "@ngx-translate/http-loader": "2.0.1", "@nicky-lenaers/ngx-scroll-to": "^0.6.0", "angular-idle-preload": "2.0.4", - "angular2-moment": "^1.9.0", "angular-sortablejs": "^2.5.0", + "angular2-moment": "^1.9.0", "angular2-text-mask": "8.0.4", "angulartics2": "^5.2.0", "body-parser": "1.18.2", @@ -99,7 +99,7 @@ "cerialize": "0.1.18", "compression": "1.7.1", "cookie-parser": "1.4.3", - "core-js": "2.5.3", + "core-js": "^2.5.7", "express": "4.16.2", "express-session": "1.15.6", "font-awesome": "4.7.0", @@ -112,8 +112,8 @@ "methods": "1.1.2", "moment": "^2.22.1", "morgan": "1.9.0", - "ng2-nouislider": "^1.7.11", "ng2-file-upload": "1.2.1", + "ng2-nouislider": "^1.7.11", "ngx-infinite-scroll": "0.8.2", "ngx-pagination": "3.0.3", "nouislider": "^11.0.0", diff --git a/yarn.lock b/yarn.lock index 344093d446..5caaac6090 100644 --- a/yarn.lock +++ b/yarn.lock @@ -90,20 +90,20 @@ resolved "https://registry.yarnpkg.com/@ng-dynamic-forms/ui-ng-bootstrap/-/ui-ng-bootstrap-5.4.7.tgz#66d037a226da96fe84c4dbac98e4dba859c551f8" "@ngrx/effects@^5.1.0": - version "5.1.0" - resolved "https://registry.yarnpkg.com/@ngrx/effects/-/effects-5.1.0.tgz#cef84576b2d0333f19188aedfe156fd301bff70a" + version "5.2.0" + resolved "https://registry.yarnpkg.com/@ngrx/effects/-/effects-5.2.0.tgz#aa762b69cb6fd4644d724a1cecd265caa42baf09" "@ngrx/router-store@^5.0.1": - version "5.0.1" - resolved "https://registry.yarnpkg.com/@ngrx/router-store/-/router-store-5.0.1.tgz#db872327bb958a2ebf296734c97de68672ec628a" + version "5.2.0" + resolved "https://registry.yarnpkg.com/@ngrx/router-store/-/router-store-5.2.0.tgz#bf4b174ce19a36eba8211fc1ddeaf1e35ae74368" "@ngrx/store-devtools@^5.1.0": - version "5.1.0" - resolved "https://registry.yarnpkg.com/@ngrx/store-devtools/-/store-devtools-5.1.0.tgz#7df8a6da652cc792000ad058ca4072a32e3629b1" + version "5.2.0" + resolved "https://registry.yarnpkg.com/@ngrx/store-devtools/-/store-devtools-5.2.0.tgz#2fff916a9aa349375826772b359dbb64b9e5d622" "@ngrx/store@^5.1.0": - version "5.1.0" - resolved "https://registry.yarnpkg.com/@ngrx/store/-/store-5.1.0.tgz#d957131e62041deede043524fd300db9fa835d68" + version "5.2.0" + resolved "https://registry.yarnpkg.com/@ngrx/store/-/store-5.2.0.tgz#627ed74c9cd95462930485d912a557117b23903e" "@ngtools/webpack@^1.10.0": version "1.10.0" @@ -437,16 +437,16 @@ angular-idle-preload@2.0.4: version "2.0.4" resolved "https://registry.yarnpkg.com/angular-idle-preload/-/angular-idle-preload-2.0.4.tgz#7b177c0f52918c090e5c345480b922297cd59a0d" +angular-sortablejs@^2.5.0: + version "2.5.2" + resolved "https://registry.yarnpkg.com/angular-sortablejs/-/angular-sortablejs-2.5.2.tgz#ffd651e47cc93a191db4c023f617db3789fd9af5" + angular2-moment@^1.9.0: version "1.9.0" resolved "https://registry.yarnpkg.com/angular2-moment/-/angular2-moment-1.9.0.tgz#d198a4d9bc825f61de19106ac7ea07a78569f5a1" dependencies: moment "^2.19.3" -angular-sortablejs@^2.5.0: - version "2.5.2" - resolved "https://registry.yarnpkg.com/angular-sortablejs/-/angular-sortablejs-2.5.2.tgz#ffd651e47cc93a191db4c023f617db3789fd9af5" - angular2-template-loader@0.6.2: version "0.6.2" resolved "https://registry.yarnpkg.com/angular2-template-loader/-/angular2-template-loader-0.6.2.tgz#c0d44e90fff0fac95e8b23f043acda7fd1c51d7c" @@ -1963,14 +1963,14 @@ copy-webpack-plugin@^4.4.1: p-limit "^1.0.0" serialize-javascript "^1.4.0" -core-js@2.5.3: - version "2.5.3" - resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.5.3.tgz#8acc38345824f16d8365b7c9b4259168e8ed603e" - core-js@^2.2.0, core-js@^2.4.0: version "2.5.1" resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.5.1.tgz#ae6874dc66937789b80754ff5428df66819ca50b" +core-js@^2.5.7: + version "2.5.7" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.5.7.tgz#f972608ff0cead68b841a16a932d0b183791814e" + core-js@~2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.3.0.tgz#fab83fbb0b2d8dc85fa636c4b9d34c75420c6d65" From e96eeeaa835fd9197d6d88d8543e9856fd3cb70b Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 18 Sep 2018 16:29:55 +0200 Subject: [PATCH 5/8] fixed redirect after login --- .../search-filters.component.ts | 1 - src/app/core/auth/auth.service.ts | 16 +++++++------ src/app/core/auth/server-auth.service.ts | 4 +++- src/app/header/header.component.spec.ts | 6 +++-- .../auth-nav-menu.component.spec.ts | 4 +++- .../auth-nav-menu/auth-nav-menu.component.ts | 23 +++++++++++++++---- 6 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filters.component.ts b/src/app/+search-page/search-filters/search-filters.component.ts index f4b63c332f..1da4e5860d 100644 --- a/src/app/+search-page/search-filters/search-filters.component.ts +++ b/src/app/+search-page/search-filters/search-filters.component.ts @@ -56,7 +56,6 @@ export class SearchFiltersComponent { * @returns {Observable} Emits true whenever a given filter config should be shown */ isActive(filter: SearchFilterConfig): Observable { - // console.log(filter.name); return this.filterService.getSelectedValuesForFilter(filter) .flatMap((isActive) => { if (isNotEmpty(isActive)) { diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index f8fc863ee9..f69dd2b58f 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -54,7 +54,7 @@ export class AuthService { protected storage: CookieService, protected store: Store, protected rdbService: RemoteDataBuildService - ) { + ) { this.store.select(isAuthenticated) .startWith(false) .subscribe((authenticated: boolean) => this._authenticated = authenticated); @@ -107,7 +107,7 @@ export class AuthService { if (status.authenticated) { return status; } else { - Observable.throw(new Error('Invalid email or password')); + throw(new Error('Invalid email or password')); } }) @@ -140,7 +140,7 @@ export class AuthService { const person$ = this.rdbService.buildSingle(status.eperson.toString()); return person$.pipe(map((eperson) => eperson.payload)); } else { - Observable.throw(new Error('Not authenticated')); + throw(new Error('Not authenticated')); } })) } @@ -216,7 +216,7 @@ export class AuthService { // Send a request that sign end the session let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'application/x-www-form-urlencoded'); - const options: HttpOptions = Object.create({headers, responseType: 'text'}); + const options: HttpOptions = Object.create({ headers, responseType: 'text' }); return this.authRequestService.getRequest('logout', options) .map((status: AuthStatus) => { if (!status.authenticated) { @@ -225,7 +225,6 @@ export class AuthService { throw(new Error('auth.errors.invalid-user')); } }) - } /** @@ -246,6 +245,7 @@ export class AuthService { public getToken(): AuthTokenInfo { let token: AuthTokenInfo; this.store.select(getAuthenticationToken) + .first() .subscribe((authTokenInfo: AuthTokenInfo) => { // Retrieve authentication token info and check if is valid token = authTokenInfo || null; @@ -291,7 +291,7 @@ export class AuthService { // Set the cookie expire date const expires = new Date(expireDate); - const options: CookieAttributes = {expires: expires}; + const options: CookieAttributes = { expires: expires }; // Save cookie with the token return this.storage.set(TOKENITEM, token, options); @@ -349,8 +349,10 @@ export class AuthService { this.router.navigated = false; const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); + this._window.nativeWindow.location.href = url; } else { this.router.navigate(['/']); + this._window.nativeWindow.location.href = '/'; } }) @@ -386,7 +388,7 @@ export class AuthService { // Set the cookie expire date const expires = new Date(expireDate); - const options: CookieAttributes = {expires: expires}; + const options: CookieAttributes = { expires: expires }; this.storage.set(REDIRECT_COOKIE, url, options); this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(url) ? url : '')); } diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 8d1bf1673c..18eba2e92d 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -44,7 +44,7 @@ export class ServerAuthService extends AuthService { // person$.subscribe(() => console.log('test')); return person$.pipe(map((eperson) => eperson.payload)); } else { - Observable.throw(new Error('Not authenticated')); + throw(new Error('Not authenticated')); } })) } @@ -71,8 +71,10 @@ export class ServerAuthService extends AuthService { this.router.navigated = false; const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); + this._window.nativeWindow.location.href = url; } else { this.router.navigate(['/']); + this._window.nativeWindow.location.href = '/'; } }) diff --git a/src/app/header/header.component.spec.ts b/src/app/header/header.component.spec.ts index 87fa2995d6..9d0dd04e40 100644 --- a/src/app/header/header.component.spec.ts +++ b/src/app/header/header.component.spec.ts @@ -19,6 +19,7 @@ import { HostWindowServiceStub } from '../shared/testing/host-window-service-stu import { RouterStub } from '../shared/testing/router-stub'; import { Router } from '@angular/router'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { NO_ERRORS_SCHEMA } from '@angular/core'; let comp: HeaderComponent; let fixture: ComponentFixture; @@ -35,11 +36,12 @@ describe('HeaderComponent', () => { NgbCollapseModule.forRoot(), NoopAnimationsModule, ReactiveFormsModule], - declarations: [HeaderComponent, AuthNavMenuComponent, LoadingComponent, LogInComponent, LogOutComponent], + declarations: [HeaderComponent], providers: [ { provide: HostWindowService, useValue: new HostWindowServiceStub(800) }, { provide: Router, useClass: RouterStub }, - ] + ], + schemas: [NO_ERRORS_SCHEMA] }) .compileComponents(); // compile template and css })); diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts index 1c59c02346..a036590ed5 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts @@ -13,7 +13,7 @@ import { HostWindowServiceStub } from '../testing/host-window-service-stub'; import { HostWindowService } from '../host-window.service'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { AuthTokenInfo } from '../../core/auth/models/auth-token-info.model'; -import { EPerson } from '../../core/eperson/models/eperson.model'; +import { AuthService } from '../../core/auth/auth.service'; describe('AuthNavMenuComponent', () => { @@ -54,6 +54,7 @@ describe('AuthNavMenuComponent', () => { ], providers: [ {provide: HostWindowService, useValue: window}, + {provide: AuthService, useValue: {setRedirectUrl: () => {}}} ], schemas: [ CUSTOM_ELEMENTS_SCHEMA @@ -223,6 +224,7 @@ describe('AuthNavMenuComponent', () => { ], providers: [ {provide: HostWindowService, useValue: window}, + {provide: AuthService, useValue: {setRedirectUrl: () => {}}} ], schemas: [ CUSTOM_ELEMENTS_SCHEMA 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 887729a7af..c657071987 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 @@ -7,10 +7,14 @@ import { fadeInOut, fadeOut } from '../animations/fade'; import { HostWindowService } from '../host-window.service'; import { AppState, routerStateSelector } from '../../app.reducer'; import { isNotUndefined } from '../empty.util'; -import { getAuthenticatedUser, isAuthenticated, isAuthenticationLoading } from '../../core/auth/selectors'; +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'; +import { AuthService, LOGIN_ROUTE, LOGOUT_ROUTE } from '../../core/auth/auth.service'; +import { Subscription } from 'rxjs/Subscription'; @Component({ selector: 'ds-auth-nav-menu', @@ -37,8 +41,12 @@ export class AuthNavMenuComponent implements OnInit { public user: Observable; + public sub: Subscription; + constructor(private store: Store, - private windowService: HostWindowService) { + private windowService: HostWindowService, + private authService: AuthService + ) { this.isXsOrSm$ = this.windowService.isXsOrSm(); } @@ -54,7 +62,12 @@ export class AuthNavMenuComponent implements OnInit { this.showAuth = this.store.select(routerStateSelector) .filter((router: RouterReducerState) => isNotUndefined(router) && isNotUndefined(router.state)) .map((router: RouterReducerState) => { - return !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); + const url = router.state.url; + const show = !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); + if (show) { + this.authService.setRedirectUrl(url); + } + return show; }); } } From a2bcdfbea9104a57a51a852bfc1cda005f84edca Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 20 Sep 2018 15:24:16 +0200 Subject: [PATCH 6/8] disabled reloads --- src/app/core/auth/auth.service.ts | 7 ++++--- src/app/core/auth/server-auth.service.ts | 2 -- src/app/shared/auth-nav-menu/auth-nav-menu.component.ts | 8 ++------ 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index f69dd2b58f..a7a1237e1c 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -245,7 +245,6 @@ export class AuthService { public getToken(): AuthTokenInfo { let token: AuthTokenInfo; this.store.select(getAuthenticationToken) - .first() .subscribe((authTokenInfo: AuthTokenInfo) => { // Retrieve authentication token info and check if is valid token = authTokenInfo || null; @@ -349,10 +348,12 @@ export class AuthService { this.router.navigated = false; const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); - this._window.nativeWindow.location.href = url; + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for */ + // this._window.nativeWindow.location.href = url; } else { this.router.navigate(['/']); - this._window.nativeWindow.location.href = '/'; + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for */ + // this._window.nativeWindow.location.href = '/'; } }) diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 18eba2e92d..9030443a1e 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -71,10 +71,8 @@ export class ServerAuthService extends AuthService { this.router.navigated = false; const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); - this._window.nativeWindow.location.href = url; } else { this.router.navigate(['/']); - this._window.nativeWindow.location.href = '/'; } }) 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 c657071987..f7e1b03c88 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 @@ -62,12 +62,8 @@ export class AuthNavMenuComponent implements OnInit { this.showAuth = this.store.select(routerStateSelector) .filter((router: RouterReducerState) => isNotUndefined(router) && isNotUndefined(router.state)) .map((router: RouterReducerState) => { - const url = router.state.url; - const show = !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); - if (show) { - this.authService.setRedirectUrl(url); - } - return show; + return !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); + }); } } From 7db30e88090104bde234663711eca76a8efd7c05 Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 20 Sep 2018 15:37:54 +0200 Subject: [PATCH 7/8] fixed tslint error --- .../shared/auth-nav-menu/auth-nav-menu.component.spec.ts | 4 ++-- src/app/shared/auth-nav-menu/auth-nav-menu.component.ts | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts index a036590ed5..e1a82f4a33 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts @@ -54,7 +54,7 @@ describe('AuthNavMenuComponent', () => { ], providers: [ {provide: HostWindowService, useValue: window}, - {provide: AuthService, useValue: {setRedirectUrl: () => {}}} + {provide: AuthService, useValue: {setRedirectUrl: () => { /*empty*/ }}} ], schemas: [ CUSTOM_ELEMENTS_SCHEMA @@ -224,7 +224,7 @@ describe('AuthNavMenuComponent', () => { ], providers: [ {provide: HostWindowService, useValue: window}, - {provide: AuthService, useValue: {setRedirectUrl: () => {}}} + {provide: AuthService, useValue: {setRedirectUrl: () => { /*empty*/ }}} ], schemas: [ CUSTOM_ELEMENTS_SCHEMA 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 f7e1b03c88..c657071987 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 @@ -62,8 +62,12 @@ export class AuthNavMenuComponent implements OnInit { this.showAuth = this.store.select(routerStateSelector) .filter((router: RouterReducerState) => isNotUndefined(router) && isNotUndefined(router.state)) .map((router: RouterReducerState) => { - return !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); - + const url = router.state.url; + const show = !router.state.url.startsWith(LOGIN_ROUTE) && !router.state.url.startsWith(LOGOUT_ROUTE); + if (show) { + this.authService.setRedirectUrl(url); + } + return show; }); } } From bfc70f4f881d22366410df0e8f47ed629adb8d2d Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 27 Sep 2018 10:37:40 +0200 Subject: [PATCH 8/8] Added links to issues --- src/app/core/auth/auth.service.ts | 6 ++++-- src/app/core/data/base-response-parsing.service.ts | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index a7a1237e1c..5f113b0262 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -137,6 +137,8 @@ export class AuthService { if (status.authenticated) { // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole... + // Review when https://jira.duraspace.org/browse/DS-4006 is fixed + // See https://github.com/DSpace/dspace-angular/issues/292 const person$ = this.rdbService.buildSingle(status.eperson.toString()); return person$.pipe(map((eperson) => eperson.payload)); } else { @@ -348,11 +350,11 @@ export class AuthService { this.router.navigated = false; const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for */ + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ // this._window.nativeWindow.location.href = url; } else { this.router.navigate(['/']); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for */ + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ // this._window.nativeWindow.location.href = '/'; } }) diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index bdd18f8a61..fdf5b4eb97 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/src/app/core/data/base-response-parsing.service.ts @@ -150,7 +150,8 @@ export abstract class BaseResponseParsingService { return obj[keys[0]]; } - /* TODO remove when REST response for epersons is fixed */ + // TODO Remove when https://jira.duraspace.org/browse/DS-4006 is fixed + // See https://github.com/DSpace/dspace-angular/issues/292 private fixBadEPersonRestResponse(obj: any): any { if (obj.type === ResourceType.EPerson) { const groups = obj.groups;