diff --git a/src/app/core/auth/auth.interceptor.ts b/src/app/core/auth/auth.interceptor.ts index 672879f436f..4ad856fd882 100644 --- a/src/app/core/auth/auth.interceptor.ts +++ b/src/app/core/auth/auth.interceptor.ts @@ -152,12 +152,12 @@ export class AuthInterceptor implements HttpInterceptor { let authMethodModel: AuthMethod; if (splittedRealm.length === 1) { - authMethodModel = new AuthMethod(methodName); + authMethodModel = new AuthMethod(methodName, Number(j)); authMethodModels.push(authMethodModel); } else if (splittedRealm.length > 1) { let location = splittedRealm[1]; location = this.parseLocation(location); - authMethodModel = new AuthMethod(methodName, location); + authMethodModel = new AuthMethod(methodName, Number(j), location); authMethodModels.push(authMethodModel); } } @@ -165,7 +165,7 @@ export class AuthInterceptor implements HttpInterceptor { // make sure the email + password login component gets rendered first authMethodModels = this.sortAuthMethods(authMethodModels); } else { - authMethodModels.push(new AuthMethod(AuthMethodType.Password)); + authMethodModels.push(new AuthMethod(AuthMethodType.Password, 0)); } return authMethodModels; diff --git a/src/app/core/auth/auth.reducer.spec.ts b/src/app/core/auth/auth.reducer.spec.ts index c0619adf793..41c03126538 100644 --- a/src/app/core/auth/auth.reducer.spec.ts +++ b/src/app/core/auth/auth.reducer.spec.ts @@ -598,9 +598,9 @@ describe('authReducer', () => { authMethods: [], idle: false }; - const authMethods = [ - new AuthMethod(AuthMethodType.Password), - new AuthMethod(AuthMethodType.Shibboleth, 'location') + const authMethods: AuthMethod[] = [ + new AuthMethod(AuthMethodType.Password, 0), + new AuthMethod(AuthMethodType.Shibboleth, 1, 'location'), ]; const action = new RetrieveAuthMethodsSuccessAction(authMethods); const newState = authReducer(initialState, action); @@ -632,7 +632,7 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - authMethods: [new AuthMethod(AuthMethodType.Password)], + authMethods: [new AuthMethod(AuthMethodType.Password, 0)], idle: false }; expect(newState).toEqual(state); diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index ba9c41326a6..437c19fd26e 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -236,7 +236,7 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut return Object.assign({}, state, { loading: false, blocking: false, - authMethods: [new AuthMethod(AuthMethodType.Password)] + authMethods: [new AuthMethod(AuthMethodType.Password, 0)] }); case AuthActionTypes.SET_REDIRECT_URL: diff --git a/src/app/core/auth/models/auth.method.ts b/src/app/core/auth/models/auth.method.ts index 0579ae0cd1b..b84e7a308af 100644 --- a/src/app/core/auth/models/auth.method.ts +++ b/src/app/core/auth/models/auth.method.ts @@ -2,11 +2,12 @@ import { AuthMethodType } from './auth.method-type'; export class AuthMethod { authMethodType: AuthMethodType; + position: number; location?: string; - // isStandalonePage? = true; + constructor(authMethodName: string, position: number, location?: string) { + this.position = position; - constructor(authMethodName: string, location?: string) { switch (authMethodName) { case 'ip': { this.authMethodType = AuthMethodType.Ip; diff --git a/src/app/shared/log-in/container/log-in-container.component.spec.ts b/src/app/shared/log-in/container/log-in-container.component.spec.ts index 29598e422ec..4700cf93071 100644 --- a/src/app/shared/log-in/container/log-in-container.component.spec.ts +++ b/src/app/shared/log-in/container/log-in-container.component.spec.ts @@ -13,13 +13,17 @@ import { AuthMethod } from '../../../core/auth/models/auth.method'; import { AuthServiceStub } from '../../testing/auth-service.stub'; import { createTestComponent } from '../../testing/utils.test'; import { HardRedirectService } from '../../../core/services/hard-redirect.service'; +import { AuthMethodType } from '../../../core/auth/models/auth.method-type'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { AuthorizationDataServiceStub } from '../../testing/authorization-service.stub'; +import { RouterTestingModule } from '@angular/router/testing'; describe('LogInContainerComponent', () => { let component: LogInContainerComponent; let fixture: ComponentFixture; - const authMethod = new AuthMethod('password'); + const authMethod = new AuthMethod(AuthMethodType.Password, 0); let hardRedirectService: HardRedirectService; @@ -35,13 +39,15 @@ describe('LogInContainerComponent', () => { ReactiveFormsModule, StoreModule.forRoot(authReducer), SharedModule, - TranslateModule.forRoot() + TranslateModule.forRoot(), + RouterTestingModule, ], declarations: [ TestComponent ], providers: [ { provide: AuthService, useClass: AuthServiceStub }, + { provide: AuthorizationDataService, useClass: AuthorizationDataServiceStub }, { provide: HardRedirectService, useValue: hardRedirectService }, LogInContainerComponent ], @@ -113,6 +119,6 @@ describe('LogInContainerComponent', () => { class TestComponent { isStandalonePage = true; - authMethod = new AuthMethod('password'); + authMethod = new AuthMethod(AuthMethodType.Password, 0); } diff --git a/src/app/shared/log-in/log-in.component.html b/src/app/shared/log-in/log-in.component.html index 6b1fdb9ff66..857b977360e 100644 --- a/src/app/shared/log-in/log-in.component.html +++ b/src/app/shared/log-in/log-in.component.html @@ -1,13 +1,11 @@
- -
- {{"login.form.or-divider" | translate}} + +
+
+ +
+
-
- - - {{"login.form.new-user" | translate}} - {{"login.form.forgot-password" | translate}}
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 0a13e8b701e..57ed3e46946 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -50,7 +50,7 @@ describe('LogInComponent', () => { }); // refine the test module by declaring the test component - TestBed.configureTestingModule({ + void TestBed.configureTestingModule({ imports: [ FormsModule, ReactiveFormsModule, diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index e6b2ba9dcf1..4bfae183326 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -1,5 +1,5 @@ -import { Component, Input, OnInit } from '@angular/core'; -import { Observable } from 'rxjs'; +import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core'; +import { map, Observable } from 'rxjs'; import { select, Store } from '@ngrx/store'; import { AuthMethod } from '../../core/auth/models/auth.method'; import { @@ -8,11 +8,8 @@ import { isAuthenticated, isAuthenticationLoading } from '../../core/auth/selectors'; -import { getForgotPasswordRoute, getRegisterRoute } from '../../app-routing-paths'; import { hasValue } from '../empty.util'; import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { FeatureID } from '../../core/data/feature-authorization/feature-id'; import { CoreState } from '../../core/core-state.model'; import { AuthMethodType } from '../../core/auth/models/auth.method-type'; @@ -23,7 +20,8 @@ import { AuthMethodType } from '../../core/auth/models/auth.method-type'; @Component({ selector: 'ds-log-in', templateUrl: './log-in.component.html', - styleUrls: ['./log-in.component.scss'] + styleUrls: ['./log-in.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class LogInComponent implements OnInit { @@ -37,7 +35,7 @@ export class LogInComponent implements OnInit { * The list of authentication methods available * @type {AuthMethod[]} */ - public authMethods: AuthMethod[]; + public authMethods: Observable; /** * Whether user is authenticated. @@ -51,24 +49,17 @@ export class LogInComponent implements OnInit { */ public loading: Observable; - /** - * Whether or not the current user (or anonymous) is authorized to register an account - */ - canRegister$: Observable; - constructor(private store: Store, private authService: AuthService, - private authorizationService: AuthorizationDataService) { + ) { } ngOnInit(): void { - - this.store.pipe( + this.authMethods = this.store.pipe( select(getAuthenticationMethods), - ).subscribe(methods => { // ignore the ip authentication method when it's returned by the backend - this.authMethods = methods.filter(a => a.authMethodType !== AuthMethodType.Ip); - }); + map((methods: AuthMethod[]) => methods.filter((authMethod: AuthMethod) => authMethod.authMethodType !== AuthMethodType.Ip)), + ); // set loading this.loading = this.store.pipe(select(isAuthenticationLoading)); @@ -82,15 +73,18 @@ export class LogInComponent implements OnInit { this.authService.clearRedirectUrl(); } }); - - this.canRegister$ = this.authorizationService.isAuthorized(FeatureID.EPersonRegistration); - } - - getRegisterRoute() { - return getRegisterRoute(); } - getForgotRoute() { - return getForgotPasswordRoute(); + /** + * Returns an ordered list of {@link AuthMethod}s based on their position. + * + * @param authMethods The {@link AuthMethod}s to sort + */ + getOrderedAuthMethods(authMethods: AuthMethod[] | null): AuthMethod[] { + if (hasValue(authMethods)) { + return [...authMethods].sort((method1: AuthMethod, method2: AuthMethod) => method1.position - method2.position); + } else { + return []; + } } } diff --git a/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.html b/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.html index 6046aec7251..378bb6ed67f 100644 --- a/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.html +++ b/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.html @@ -1,3 +1,3 @@ - diff --git a/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.spec.ts b/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.spec.ts index de4f62eb9eb..5d7a1759737 100644 --- a/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.spec.ts +++ b/src/app/shared/log-in/methods/log-in-external-provider/log-in-external-provider.component.spec.ts @@ -3,11 +3,8 @@ import { ActivatedRoute, Router } from '@angular/router'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { provideMockStore } from '@ngrx/store/testing'; -import { Store, StoreModule } from '@ngrx/store'; +import { StoreModule } from '@ngrx/store'; import { TranslateModule } from '@ngx-translate/core'; - -import { EPerson } from '../../../../core/eperson/models/eperson.model'; -import { EPersonMock } from '../../../testing/eperson.mock'; import { authReducer } from '../../../../core/auth/auth.reducer'; import { AuthService } from '../../../../core/auth/auth.service'; import { AuthServiceStub } from '../../../testing/auth-service.stub'; @@ -25,17 +22,14 @@ describe('LogInExternalProviderComponent', () => { let component: LogInExternalProviderComponent; let fixture: ComponentFixture; - let page: Page; - let user: EPerson; let componentAsAny: any; let setHrefSpy; - let orcidBaseUrl; - let location; + let orcidBaseUrl: string; + let location: string; let initialState: any; let hardRedirectService: HardRedirectService; beforeEach(() => { - user = EPersonMock; orcidBaseUrl = 'dspace-rest.test/orcid?redirectUrl='; location = orcidBaseUrl + 'http://dspace-angular.test/home'; @@ -59,7 +53,7 @@ describe('LogInExternalProviderComponent', () => { beforeEach(waitForAsync(() => { // refine the test module by declaring the test component - TestBed.configureTestingModule({ + void TestBed.configureTestingModule({ imports: [ StoreModule.forRoot({ auth: authReducer }, storeModuleConfig), TranslateModule.forRoot() @@ -69,7 +63,7 @@ describe('LogInExternalProviderComponent', () => { ], providers: [ { provide: AuthService, useClass: AuthServiceStub }, - { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Orcid, location) }, + { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Orcid, 0, location) }, { provide: 'isStandalonePage', useValue: true }, { provide: NativeWindowService, useFactory: NativeWindowMockFactory }, { provide: Router, useValue: new RouterStub() }, @@ -94,7 +88,6 @@ describe('LogInExternalProviderComponent', () => { componentAsAny = component; // create page - page = new Page(component, fixture); setHrefSpy = spyOnProperty(componentAsAny._window.nativeWindow.location, 'href', 'set').and.callThrough(); }); @@ -130,25 +123,3 @@ describe('LogInExternalProviderComponent', () => { }); }); - -/** - * I represent the DOM elements and attach spies. - * - * @class Page - */ -class Page { - - public emailInput: HTMLInputElement; - public navigateSpy: jasmine.Spy; - public passwordInput: HTMLInputElement; - - constructor(private component: LogInExternalProviderComponent, private fixture: ComponentFixture) { - // use injector to get services - const injector = fixture.debugElement.injector; - const store = injector.get(Store); - - // add spies - this.navigateSpy = spyOn(store, 'dispatch'); - } - -} diff --git a/src/app/shared/log-in/methods/password/log-in-password.component.html b/src/app/shared/log-in/methods/password/log-in-password.component.html index c1f1016cb8e..60477d141de 100644 --- a/src/app/shared/log-in/methods/password/log-in-password.component.html +++ b/src/app/shared/log-in/methods/password/log-in-password.component.html @@ -28,3 +28,12 @@ + + diff --git a/src/app/shared/log-in/methods/password/log-in-password.component.scss b/src/app/shared/log-in/methods/password/log-in-password.component.scss index 0eda382c0a6..955252b51e9 100644 --- a/src/app/shared/log-in/methods/password/log-in-password.component.scss +++ b/src/app/shared/log-in/methods/password/log-in-password.component.scss @@ -11,3 +11,7 @@ border-top-right-radius: 0; } +.dropdown-item { + white-space: normal; + padding: .25rem .75rem; +} diff --git a/src/app/shared/log-in/methods/password/log-in-password.component.spec.ts b/src/app/shared/log-in/methods/password/log-in-password.component.spec.ts index 5d9370f2048..2c9081d1c32 100644 --- a/src/app/shared/log-in/methods/password/log-in-password.component.spec.ts +++ b/src/app/shared/log-in/methods/password/log-in-password.component.spec.ts @@ -8,8 +8,6 @@ import { Store, StoreModule } from '@ngrx/store'; import { TranslateModule } from '@ngx-translate/core'; import { LogInPasswordComponent } from './log-in-password.component'; -import { EPerson } from '../../../../core/eperson/models/eperson.model'; -import { EPersonMock } from '../../../testing/eperson.mock'; import { authReducer } from '../../../../core/auth/auth.reducer'; import { AuthService } from '../../../../core/auth/auth.service'; import { AuthServiceStub } from '../../../testing/auth-service.stub'; @@ -18,19 +16,18 @@ import { AuthMethod } from '../../../../core/auth/models/auth.method'; import { AuthMethodType } from '../../../../core/auth/models/auth.method-type'; import { HardRedirectService } from '../../../../core/services/hard-redirect.service'; import { BrowserOnlyMockPipe } from '../../../testing/browser-only-mock.pipe'; +import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; +import { AuthorizationDataServiceStub } from '../../../testing/authorization-service.stub'; describe('LogInPasswordComponent', () => { let component: LogInPasswordComponent; let fixture: ComponentFixture; let page: Page; - let user: EPerson; let initialState: any; let hardRedirectService: HardRedirectService; beforeEach(() => { - user = EPersonMock; - hardRedirectService = jasmine.createSpyObj('hardRedirectService', { getCurrentRoute: {} }); @@ -50,7 +47,7 @@ describe('LogInPasswordComponent', () => { beforeEach(waitForAsync(() => { // refine the test module by declaring the test component - TestBed.configureTestingModule({ + void TestBed.configureTestingModule({ imports: [ FormsModule, ReactiveFormsModule, @@ -63,7 +60,8 @@ describe('LogInPasswordComponent', () => { ], providers: [ { provide: AuthService, useClass: AuthServiceStub }, - { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Password) }, + { provide: AuthorizationDataService, useClass: AuthorizationDataServiceStub }, + { provide: 'authMethodProvider', useValue: new AuthMethod(AuthMethodType.Password, 0) }, { provide: 'isStandalonePage', useValue: true }, { provide: HardRedirectService, useValue: hardRedirectService }, provideMockStore({ initialState }), @@ -76,7 +74,7 @@ describe('LogInPasswordComponent', () => { })); - beforeEach(() => { + beforeEach(async () => { // create component and test fixture fixture = TestBed.createComponent(LogInPasswordComponent); @@ -87,10 +85,8 @@ describe('LogInPasswordComponent', () => { page = new Page(component, fixture); // verify the fixture is stable (no pending tasks) - fixture.whenStable().then(() => { - page.addPageElements(); - }); - + await fixture.whenStable(); + page.addPageElements(); }); it('should create a FormGroup comprised of FormControls', () => { diff --git a/src/app/shared/log-in/methods/password/log-in-password.component.ts b/src/app/shared/log-in/methods/password/log-in-password.component.ts index 22b3b131308..61323940192 100644 --- a/src/app/shared/log-in/methods/password/log-in-password.component.ts +++ b/src/app/shared/log-in/methods/password/log-in-password.component.ts @@ -15,6 +15,9 @@ import { AuthMethod } from '../../../../core/auth/models/auth.method'; import { AuthService } from '../../../../core/auth/auth.service'; import { HardRedirectService } from '../../../../core/services/hard-redirect.service'; import { CoreState } from '../../../../core/core-state.model'; +import { getForgotPasswordRoute, getRegisterRoute } from '../../../../app-routing-paths'; +import { FeatureID } from '../../../../core/data/feature-authorization/feature-id'; +import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; /** * /users/sign-in @@ -66,21 +69,18 @@ export class LogInPasswordComponent implements OnInit { public form: UntypedFormGroup; /** - * @constructor - * @param {AuthMethod} injectedAuthMethodModel - * @param {boolean} isStandalonePage - * @param {AuthService} authService - * @param {HardRedirectService} hardRedirectService - * @param {FormBuilder} formBuilder - * @param {Store} store + * Whether the current user (or anonymous) is authorized to register an account */ + public canRegister$: Observable; + constructor( @Inject('authMethodProvider') public injectedAuthMethodModel: AuthMethod, @Inject('isStandalonePage') public isStandalonePage: boolean, private authService: AuthService, private hardRedirectService: HardRedirectService, private formBuilder: UntypedFormBuilder, - private store: Store + protected store: Store, + protected authorizationService: AuthorizationDataService, ) { this.authMethod = injectedAuthMethodModel; } @@ -115,6 +115,15 @@ export class LogInPasswordComponent implements OnInit { }) ); + this.canRegister$ = this.authorizationService.isAuthorized(FeatureID.EPersonRegistration); + } + + getRegisterRoute() { + return getRegisterRoute(); + } + + getForgotRoute() { + return getForgotPasswordRoute(); } /** diff --git a/src/app/shared/testing/auth-service.stub.ts b/src/app/shared/testing/auth-service.stub.ts index 7f3d040042f..6913305315c 100644 --- a/src/app/shared/testing/auth-service.stub.ts +++ b/src/app/shared/testing/auth-service.stub.ts @@ -6,10 +6,11 @@ import { EPerson } from '../../core/eperson/models/eperson.model'; import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; import { AuthMethod } from '../../core/auth/models/auth.method'; import { hasValue } from '../empty.util'; +import { AuthMethodType } from '../../core/auth/models/auth.method-type'; -export const authMethodsMock = [ - new AuthMethod('password'), - new AuthMethod('shibboleth', 'dspace.test/shibboleth') +export const authMethodsMock: AuthMethod[] = [ + new AuthMethod(AuthMethodType.Password, 0), + new AuthMethod(AuthMethodType.Shibboleth, 1, 'dspace.test/shibboleth'), ]; export class AuthServiceStub { diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 0bc24355543..d12a45b8a81 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2772,8 +2772,6 @@ "login.form.new-user": "New user? Click here to register.", - "login.form.or-divider": "or", - "login.form.oidc": "Log in with OIDC", "login.form.orcid": "Log in with ORCID",