Skip to content

Commit

Permalink
Merge pull request #2488 from alexandrevryghem/fix-display-order-auth…
Browse files Browse the repository at this point in the history
…entication-methods_contribute-7.6

[Port dspace-7_x] Made it possible to reorder the login methods
  • Loading branch information
tdonohue authored Sep 11, 2023
2 parents 459a431 + 0905a53 commit bdf7414
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 108 deletions.
6 changes: 3 additions & 3 deletions src/app/core/auth/auth.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,20 @@ 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);
}
}

// 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;
Expand Down
8 changes: 4 additions & 4 deletions src/app/core/auth/auth.reducer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/auth/auth.reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions src/app/core/auth/models/auth.method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<LogInContainerComponent>;

const authMethod = new AuthMethod('password');
const authMethod = new AuthMethod(AuthMethodType.Password, 0);

let hardRedirectService: HardRedirectService;

Expand All @@ -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
],
Expand Down Expand Up @@ -113,6 +119,6 @@ describe('LogInContainerComponent', () => {
class TestComponent {

isStandalonePage = true;
authMethod = new AuthMethod('password');
authMethod = new AuthMethod(AuthMethodType.Password, 0);

}
14 changes: 6 additions & 8 deletions src/app/shared/log-in/log-in.component.html
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
<ds-themed-loading *ngIf="(loading | async) || (isAuthenticated | async)" class="m-5"></ds-themed-loading>
<div *ngIf="!(loading | async) && !(isAuthenticated | async)" class="px-4 py-3 mx-auto login-container">
<ng-container *ngFor="let authMethod of (authMethods); let i = index">
<div *ngIf="i === 1" class="text-center mt-2">
<span class="align-middle">{{"login.form.or-divider" | translate}}</span>
<ng-container *ngFor="let authMethod of getOrderedAuthMethods(authMethods | async); let last = last">
<div [class.d-none]="contentRef.innerText?.trim().length === 0">
<div #contentRef>
<ds-log-in-container [authMethod]="authMethod" [isStandalonePage]="isStandalonePage"></ds-log-in-container>
</div>
<div *ngIf="!last" class="dropdown-divider my-2"></div>
</div>
<ds-log-in-container [authMethod]="authMethod" [isStandalonePage]="isStandalonePage"></ds-log-in-container>
</ng-container>

<div class="dropdown-divider"></div>
<a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly">{{"login.form.new-user" | translate}}</a>
<a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly">{{"login.form.forgot-password" | translate}}</a>
</div>
2 changes: 1 addition & 1 deletion src/app/shared/log-in/log-in.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('LogInComponent', () => {
});

// refine the test module by declaring the test component
TestBed.configureTestingModule({
void TestBed.configureTestingModule({
imports: [
FormsModule,
ReactiveFormsModule,
Expand Down
46 changes: 20 additions & 26 deletions src/app/shared/log-in/log-in.component.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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';

Expand All @@ -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 {

Expand All @@ -37,7 +35,7 @@ export class LogInComponent implements OnInit {
* The list of authentication methods available
* @type {AuthMethod[]}
*/
public authMethods: AuthMethod[];
public authMethods: Observable<AuthMethod[]>;

/**
* Whether user is authenticated.
Expand All @@ -51,24 +49,17 @@ export class LogInComponent implements OnInit {
*/
public loading: Observable<boolean>;

/**
* Whether or not the current user (or anonymous) is authorized to register an account
*/
canRegister$: Observable<boolean>;

constructor(private store: Store<CoreState>,
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));
Expand All @@ -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 [];
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<button class="btn btn-lg btn-primary btn-block mt-2 text-white" (click)="redirectToExternalProvider()">
<button class="btn btn-lg btn-primary btn-block text-white" (click)="redirectToExternalProvider()">
<i class="fas fa-sign-in-alt"></i> {{getButtonLabel() | translate}}
</button>
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -25,17 +22,14 @@ describe('LogInExternalProviderComponent', () => {

let component: LogInExternalProviderComponent;
let fixture: ComponentFixture<LogInExternalProviderComponent>;
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';

Expand All @@ -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()
Expand All @@ -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() },
Expand All @@ -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();

});
Expand Down Expand Up @@ -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<LogInExternalProviderComponent>) {
// use injector to get services
const injector = fixture.debugElement.injector;
const store = injector.get(Store);

// add spies
this.navigateSpy = spyOn(store, 'dispatch');
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,12 @@
<button class="btn btn-lg btn-primary btn-block mt-3" type="submit" [attr.data-test]="'login-button' | dsBrowserOnly"
[disabled]="!form.valid"><i class="fas fa-sign-in-alt"></i> {{"login.form.submit" | translate}}</button>
</form>

<div class="mt-2">
<a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly">
{{ 'login.form.new-user' | translate }}
</a>
<a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly">
{{ 'login.form.forgot-password' | translate }}
</a>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@
border-top-right-radius: 0;
}

.dropdown-item {
white-space: normal;
padding: .25rem .75rem;
}
Loading

0 comments on commit bdf7414

Please sign in to comment.