From ca669077758a3c45fa3c812032979c4b5be99f17 Mon Sep 17 00:00:00 2001
From: andrej romanov <50377758+auumgn@users.noreply.github.com>
Date: Fri, 29 Mar 2024 13:21:23 +0200
Subject: [PATCH] add unit tests and final tweaks
---
.../landing-page/landing-page.component.html | 4 +-
.../landing-page.component.spec.ts | 153 ++++++++++++++++--
.../landing-page/landing-page.component.ts | 151 ++++++++---------
.../app/landing-page/landing-page.module.ts | 3 +-
.../app/landing-page/landing-page.service.ts | 21 +--
ui/src/app/shared/model/orcid-record.model.ts | 15 ++
.../shared/service/window-location.service.ts | 4 +
ui/src/app/user/users.component.spec.ts | 1 +
8 files changed, 244 insertions(+), 108 deletions(-)
create mode 100644 ui/src/app/shared/model/orcid-record.model.ts
diff --git a/ui/src/app/landing-page/landing-page.component.html b/ui/src/app/landing-page/landing-page.component.html
index 3d654fb5a..03715968c 100644
--- a/ui/src/app/landing-page/landing-page.component.html
+++ b/ui/src/app/landing-page/landing-page.component.html
@@ -17,7 +17,7 @@
>
- 
+ 
{{ issuer }}/{{ orcidRecord.orcid }}
@@ -57,7 +57,7 @@
{{ successfullyGrantedMessage }}
-
+
{{ issuer }}/{{ signedInIdToken.sub }}
diff --git a/ui/src/app/landing-page/landing-page.component.spec.ts b/ui/src/app/landing-page/landing-page.component.spec.ts
index 5b7ad0b17..42e67adaa 100644
--- a/ui/src/app/landing-page/landing-page.component.spec.ts
+++ b/ui/src/app/landing-page/landing-page.component.spec.ts
@@ -1,21 +1,146 @@
-import { ComponentFixture, TestBed } from '@angular/core/testing';
-
-import { LandingPageComponent } from './landing-page.component';
+import { ComponentFixture, TestBed } from '@angular/core/testing'
+import { HttpClientModule } from '@angular/common/http'
+import { LandingPageComponent } from './landing-page.component'
+import { LandingPageService } from './landing-page.service'
+import { OrcidRecord } from '../shared/model/orcid-record.model'
+import { of } from 'rxjs'
+import { Member } from '../member/model/member.model'
+import { WindowLocationService } from '../shared/service/window-location.service'
+import * as KEYUTIL from 'jsrsasign'
describe('LandingPageComponent', () => {
- let component: LandingPageComponent;
- let fixture: ComponentFixture;
+ let component: LandingPageComponent
+ let fixture: ComponentFixture
+ let landingPageService: jasmine.SpyObj
+ let windowLocationService: jasmine.SpyObj
beforeEach(() => {
TestBed.configureTestingModule({
- declarations: [LandingPageComponent]
- });
- fixture = TestBed.createComponent(LandingPageComponent);
- component = fixture.componentInstance;
- fixture.detectChanges();
- });
+ declarations: [LandingPageComponent],
+ imports: [HttpClientModule],
+ providers: [
+ {
+ provide: LandingPageService,
+ useValue: jasmine.createSpyObj('LandingPageService', [
+ 'getOrcidConnectionRecord',
+ 'getMemberInfo',
+ 'getPublicKey',
+ 'submitUserResponse',
+ 'getUserInfo',
+ 'submitUserResponse',
+ ]),
+ },
+ {
+ provide: WindowLocationService,
+ useValue: jasmine.createSpyObj('WindowLocationService', ['updateWindowLocation', 'getWindowLocationHash']),
+ },
+ ],
+ })
+ fixture = TestBed.createComponent(LandingPageComponent)
+ component = fixture.componentInstance
+ landingPageService = TestBed.inject(LandingPageService) as jasmine.SpyObj
+ windowLocationService = TestBed.inject(WindowLocationService) as jasmine.SpyObj
+ })
it('should create', () => {
- expect(component).toBeTruthy();
- });
-});
+ expect(component).toBeTruthy()
+ })
+
+ it('New record connection should redirect to the registry', () => {
+ windowLocationService.updateWindowLocation.and.returnValue()
+ landingPageService.getOrcidConnectionRecord.and.returnValue(of(new OrcidRecord('email', 'orcid')))
+ landingPageService.getMemberInfo.and.returnValue(
+ of(new Member('id', 'name', 'email', 'orcid', 'salesforceId', 'clientId'))
+ )
+ component.processRequest('someState', '', '')
+ expect(landingPageService.getOrcidConnectionRecord).toHaveBeenCalled()
+ expect(component.oauthUrl).toBe(
+ 'localhost.orcid.org/oauth/authorize?response_type=token&redirect_uri=/landing-page&client_id=name&scope=/read-limited /activities/update /person/update openid&prompt=login&state=someState'
+ )
+ expect(landingPageService.getPublicKey).toHaveBeenCalledTimes(0)
+ expect(windowLocationService.updateWindowLocation).toHaveBeenCalled()
+ })
+
+ it('New record connection should fail (user denied permission)', () => {
+ windowLocationService.updateWindowLocation.and.returnValue()
+ windowLocationService.getWindowLocationHash.and.returnValue('#error=access_denied')
+ landingPageService.getOrcidConnectionRecord.and.returnValue(of(new OrcidRecord('email', 'orcid')))
+ landingPageService.getMemberInfo.and.returnValue(
+ of(new Member('id', 'name', 'email', 'orcid', 'salesforceId', 'clientId'))
+ )
+ landingPageService.submitUserResponse.and.returnValue(of(''))
+ component.processRequest('someState', '', '')
+ expect(landingPageService.getOrcidConnectionRecord).toHaveBeenCalled()
+ expect(component.oauthUrl).toBe(
+ 'localhost.orcid.org/oauth/authorize?response_type=token&redirect_uri=/landing-page&client_id=name&scope=/read-limited /activities/update /person/update openid&prompt=login&state=someState'
+ )
+ expect(landingPageService.getPublicKey).toHaveBeenCalledTimes(0)
+ expect(windowLocationService.updateWindowLocation).toHaveBeenCalledTimes(0)
+ expect(landingPageService.submitUserResponse).toHaveBeenCalled()
+ expect(component.showError).toBeFalsy()
+ expect(component.showDenied).toBeTruthy()
+ })
+
+ it('New record connection should fail (generic error)', () => {
+ windowLocationService.updateWindowLocation.and.returnValue()
+ windowLocationService.getWindowLocationHash.and.returnValue('#error=123')
+ landingPageService.getOrcidConnectionRecord.and.returnValue(of(new OrcidRecord('email', 'orcid')))
+ landingPageService.getMemberInfo.and.returnValue(
+ of(new Member('id', 'name', 'email', 'orcid', 'salesforceId', 'clientId'))
+ )
+ landingPageService.submitUserResponse.and.returnValue(of(''))
+ component.processRequest('someState', '', '')
+ expect(landingPageService.getOrcidConnectionRecord).toHaveBeenCalled()
+ expect(component.oauthUrl).toBe(
+ 'localhost.orcid.org/oauth/authorize?response_type=token&redirect_uri=/landing-page&client_id=name&scope=/read-limited /activities/update /person/update openid&prompt=login&state=someState'
+ )
+ expect(landingPageService.getPublicKey).toHaveBeenCalledTimes(0)
+ expect(windowLocationService.updateWindowLocation).toHaveBeenCalledTimes(0)
+ expect(landingPageService.submitUserResponse).toHaveBeenCalledTimes(0)
+ expect(component.showError).toBeTruthy()
+ expect(component.showDenied).toBeFalsy()
+ })
+
+ it('Existing record connection should be identified', () => {
+ windowLocationService.updateWindowLocation.and.returnValue()
+ landingPageService.getOrcidConnectionRecord.and.returnValue(of(new OrcidRecord('email', 'orcid')))
+ landingPageService.getMemberInfo.and.returnValue(
+ of(new Member('id', 'name', 'email', 'orcid', 'salesforceId', 'clientId'))
+ )
+ landingPageService.getPublicKey.and.returnValue(of(['publicKey']))
+ landingPageService.submitUserResponse.and.returnValue(of(''))
+ landingPageService.getUserInfo.and.returnValue(of({ givenName: 'givenName', familyName: 'familyName' }))
+ spyOn(KEYUTIL.KEYUTIL, 'getKey').and.returnValue(new KEYUTIL.RSAKey())
+ spyOn(KEYUTIL.KJUR.jws.JWS, 'verifyJWT').and.returnValue(true)
+
+ component.processRequest('someState', 'it_token', '')
+
+ expect(landingPageService.getOrcidConnectionRecord).toHaveBeenCalled()
+ expect(landingPageService.getPublicKey).toHaveBeenCalled()
+ expect(windowLocationService.updateWindowLocation).toHaveBeenCalledTimes(0)
+ })
+
+ it('Check for wrong user', () => {
+ landingPageService.submitUserResponse.and.returnValue(of({ isDifferentUser: true }))
+ landingPageService.getPublicKey.and.returnValue(of(['publicKey']))
+ landingPageService.getUserInfo.and.returnValue(of({ givenName: 'givenName', familyName: 'familyName' }))
+ spyOn(KEYUTIL.KEYUTIL, 'getKey').and.returnValue(new KEYUTIL.RSAKey())
+ spyOn(KEYUTIL.KJUR.jws.JWS, 'verifyJWT').and.returnValue(true)
+ component.checkSubmitToken('token', 'state', 'access_token')
+ expect(landingPageService.submitUserResponse).toHaveBeenCalled()
+ expect(component.showConnectionExists).toBeFalsy()
+ expect(component.showConnectionExistsDifferentUser).toBeTruthy()
+ })
+
+ it('Check for existing connection', () => {
+ landingPageService.submitUserResponse.and.returnValue(of({ isSameUserThatAlreadyGranted: true }))
+ landingPageService.getPublicKey.and.returnValue(of(['publicKey']))
+ landingPageService.getUserInfo.and.returnValue(of({ givenName: 'givenName', familyName: 'familyName' }))
+ spyOn(KEYUTIL.KEYUTIL, 'getKey').and.returnValue(new KEYUTIL.RSAKey())
+ spyOn(KEYUTIL.KJUR.jws.JWS, 'verifyJWT').and.returnValue(true)
+ component.checkSubmitToken('token', 'state', 'access_token')
+ expect(landingPageService.submitUserResponse).toHaveBeenCalled()
+ expect(component.showConnectionExists).toBeTruthy()
+ expect(component.showConnectionExistsDifferentUser).toBeFalsy()
+ })
+})
diff --git a/ui/src/app/landing-page/landing-page.component.ts b/ui/src/app/landing-page/landing-page.component.ts
index 7f0d21dae..e2294c31a 100644
--- a/ui/src/app/landing-page/landing-page.component.ts
+++ b/ui/src/app/landing-page/landing-page.component.ts
@@ -6,22 +6,23 @@ import { LandingPageService } from './landing-page.service'
import { MemberService } from '../member/service/member.service'
import { IMember } from '../member/model/member.model'
import { ORCID_BASE_URL } from '../app.constants'
+import { WindowLocationService } from '../shared/service/window-location.service'
@Component({
selector: 'app-landing-page',
templateUrl: './landing-page.component.html',
})
export class LandingPageComponent implements OnInit {
- issuer: string = ORCID_BASE_URL
- oauthBaseUrl: string = ORCID_BASE_URL + '/oauth/authorize'
- redirectUri: string = '/landing-page'
+ issuer = ORCID_BASE_URL
+ oauthBaseUrl = ORCID_BASE_URL + '/oauth/authorize'
+ redirectUri = '/landing-page'
- loading: Boolean = true
- showConnectionExists: Boolean = false
- showConnectionExistsDifferentUser: Boolean = false
- showDenied: Boolean = false
- showError: Boolean = false
- showSuccess: Boolean = false
+ loading = true
+ showConnectionExists = false
+ showConnectionExistsDifferentUser = false
+ showDenied = false
+ showError = false
+ showSuccess = false
key: any
clientName: string | undefined
salesforceId: string | undefined
@@ -42,6 +43,7 @@ export class LandingPageComponent implements OnInit {
constructor(
private landingPageService: LandingPageService,
+ private windowLocationService: WindowLocationService,
protected memberService: MemberService
) {}
@@ -51,67 +53,73 @@ export class LandingPageComponent implements OnInit {
const state_param = this.getQueryParameterByName('state')
if (state_param) {
- this.landingPageService.getOrcidConnectionRecord(state_param).subscribe({
- next: (result: HttpResponse) => {
- this.orcidRecord = result.body
- this.landingPageService.getMemberInfo(state_param).subscribe({
- next: (res: IMember) => {
- this.clientName = res.clientName
- this.clientId = res.clientId
- this.salesforceId = res.salesforceId
- this.oauthUrl =
- this.oauthBaseUrl +
- '?response_type=token&redirect_uri=' +
- this.redirectUri +
- '&client_id=' +
- this.clientId +
- '&scope=/read-limited /activities/update /person/update openid&prompt=login&state=' +
- state_param
+ this.processRequest(state_param, id_token_fragment, access_token_fragment)
+ }
+ }
- this.incorrectDataMessage = $localize`:@@landingPage.success.ifYouFind:If you find that data added to your ORCID record is incorrect, please contact ${this.clientName}`
- this.linkAlreadyUsedMessage = $localize`:@@landingPage.connectionExists.differentUser.string:This authorization link has already been used. Please contact ${this.clientName} for a new authorization link.`
- this.allowToUpdateRecordMessage = $localize`:@@landingPage.denied.grantAccess.string:Allow ${this.clientName} to update my ORCID record.`
- this.successfullyGrantedMessage = $localize`:@@landingPage.success.youHaveSuccessfully.string:You have successfully granted ${this.clientName} permission to update your ORCID record, and your record has been updated with affiliation information.`
+ processRequest(state_param: string, id_token_fragment: string, access_token_fragment: string) {
+ this.landingPageService.getOrcidConnectionRecord(state_param).subscribe({
+ next: (result) => {
+ this.orcidRecord = result
+ this.landingPageService.getMemberInfo(state_param).subscribe({
+ next: (res: IMember) => {
+ this.clientName = res.clientName
+ this.clientId = res.clientId
+ this.salesforceId = res.salesforceId
+ this.oauthUrl =
+ this.oauthBaseUrl +
+ '?response_type=token&redirect_uri=' +
+ this.redirectUri +
+ '&client_id=' +
+ this.clientId +
+ '&scope=/read-limited /activities/update /person/update openid&prompt=login&state=' +
+ state_param
- // Check if id token exists in URL (user just granted permission)
- if (id_token_fragment != null && id_token_fragment !== '') {
- this.checkSubmitToken(id_token_fragment, state_param, access_token_fragment)
- } else {
- const error = this.getFragmentParameterByName('error')
- // Check if user denied permission
- if (error != null && error !== '') {
- if (error === 'access_denied') {
- this.submitUserDenied(state_param)
- } else {
- this.showErrorElement()
- }
+ this.incorrectDataMessage = $localize`:@@landingPage.success.ifYouFind:If you find that data added to your ORCID record is incorrect, please contact ${this.clientName}`
+ this.linkAlreadyUsedMessage = $localize`:@@landingPage.connectionExists.differentUser.string:This authorization link has already been used. Please contact ${this.clientName} for a new authorization link.`
+ this.allowToUpdateRecordMessage = $localize`:@@landingPage.denied.grantAccess.string:Allow ${this.clientName} to update my ORCID record.`
+ this.successfullyGrantedMessage = $localize`:@@landingPage.success.youHaveSuccessfully.string:You have successfully granted ${this.clientName} permission to update your ORCID record, and your record has been updated with affiliation information.`
+
+ // Check if id token exists in URL (user just granted permission)
+ if (id_token_fragment != null && id_token_fragment !== '') {
+ this.checkSubmitToken(id_token_fragment, state_param, access_token_fragment)
+ } else {
+ const error = this.getFragmentParameterByName('error')
+ // Check if user denied permission
+ if (error != null && error !== '') {
+ if (error === 'access_denied') {
+ this.submitUserDenied(state_param)
} else {
- window.location.replace(this.oauthUrl)
+ this.showErrorElement()
}
+ } else {
+ this.windowLocationService.updateWindowLocation(this.oauthUrl)
}
+ }
- this.startTimer(600)
- },
- error: (res: HttpErrorResponse) => {
- console.log('error')
- },
- })
- },
- error: (res: HttpErrorResponse) => {
- console.log('error')
- },
- })
- }
+ this.startTimer(600)
+ },
+ error: (res: HttpErrorResponse) => {
+ console.log('error')
+ },
+ })
+ },
+ error: (res: HttpErrorResponse) => {
+ console.log('error')
+ },
+ })
}
getFragmentParameterByName(name: string): string {
+ // eslint-disable-next-line
name = name.replace(/[\[]/, '\\[').replace(/[\]]/, '\\]')
const regex = new RegExp('[\\#&]' + name + '=([^&#]*)'),
- results = regex.exec(window.location.hash)
+ results = regex.exec(this.windowLocationService.getWindowLocationHash())
return results === null ? '' : decodeURIComponent(results[1].replace(/\+/g, ' '))
}
getQueryParameterByName(name: string): string | null {
+ // eslint-disable-next-line
name = name.replace(/[\[\]]/g, '\\$&')
const regex = new RegExp('[?&]' + name + '(=([^&#]*)|&|#|$)'),
results = regex.exec(window.location.href)
@@ -125,8 +133,8 @@ export class LandingPageComponent implements OnInit {
}
checkSubmitToken(id_token: string, state: string, access_token: string) {
- this.landingPageService.getPublicKey().subscribe(
- (res) => {
+ this.landingPageService.getPublicKey().subscribe({
+ next: (res) => {
const pubKey = KEYUTIL.getKey(res.keys[0]) as RSAKey
const response = KJUR.jws.JWS.verifyJWT(id_token, pubKey, {
alg: ['RS256'],
@@ -178,25 +186,6 @@ export class LandingPageComponent implements OnInit {
this.showErrorElement()
}
},
- () => {
- this.showErrorElement()
- }
- )
- }
-
- submitIdTokenData(id_token: string, state: string, access_token: string) {
- this.landingPageService.submitUserResponse({ id_token, state }).subscribe({
- next: () => {
- this.landingPageService.getUserInfo(access_token).subscribe({
- next: (res: HttpResponse) => {
- this.signedInIdToken = res
- this.showSuccessElement()
- },
- error: () => {
- this.showErrorElement()
- },
- })
- },
error: () => {
this.showErrorElement()
},
@@ -204,14 +193,14 @@ export class LandingPageComponent implements OnInit {
}
submitUserDenied(state: string) {
- this.landingPageService.submitUserResponse({ denied: true, state }).subscribe(
- () => {
+ this.landingPageService.submitUserResponse({ denied: true, state }).subscribe({
+ next: () => {
this.showDeniedElement()
},
- () => {
+ error: () => {
this.showErrorElement()
- }
- )
+ },
+ })
}
startTimer(seconds: number) {
diff --git a/ui/src/app/landing-page/landing-page.module.ts b/ui/src/app/landing-page/landing-page.module.ts
index 6af3902e6..53fc89b5d 100644
--- a/ui/src/app/landing-page/landing-page.module.ts
+++ b/ui/src/app/landing-page/landing-page.module.ts
@@ -7,9 +7,10 @@ import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'
import { MatProgressBarModule } from '@angular/material/progress-bar'
import { LandingPageComponent } from './landing-page.component'
import { LANDING_PAGE_ROUTE } from './landing-page.route'
+import { CommonModule } from '@angular/common'
@NgModule({
- imports: [MatProgressSpinnerModule, MatProgressBarModule, RouterModule.forChild(LANDING_PAGE_ROUTE)],
+ imports: [CommonModule, MatProgressSpinnerModule, MatProgressBarModule, RouterModule.forChild(LANDING_PAGE_ROUTE)],
declarations: [LandingPageComponent],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
})
diff --git a/ui/src/app/landing-page/landing-page.service.ts b/ui/src/app/landing-page/landing-page.service.ts
index 6758e7289..e18311a9e 100644
--- a/ui/src/app/landing-page/landing-page.service.ts
+++ b/ui/src/app/landing-page/landing-page.service.ts
@@ -1,17 +1,18 @@
import { Injectable } from '@angular/core'
import { HttpClient, HttpClientModule, HttpHeaders } from '@angular/common/http'
-import { Observable } from 'rxjs'
+import { Observable, map } from 'rxjs'
import { ORCID_BASE_URL } from '../app.constants'
+import { OrcidRecord } from '../shared/model/orcid-record.model'
@Injectable({ providedIn: 'root' })
export class LandingPageService {
private headers: HttpHeaders
- idTokenUri: string = '/services/assertionservice/api/id-token'
- recordConnectionUri: string = '/services/assertionservice/api/assertion/record/'
- memberInfoUri: string = '/services/memberservice/api/members/authorized/'
- userInfoUri: string = ORCID_BASE_URL + '/oauth/userinfo'
- publicKeyUri: string = ORCID_BASE_URL + '/oauth/jwks'
+ idTokenUri = '/services/assertionservice/api/id-token'
+ recordConnectionUri = '/services/assertionservice/api/assertion/record/'
+ memberInfoUri = '/services/memberservice/api/members/authorized/'
+ userInfoUri = ORCID_BASE_URL + '/oauth/userinfo'
+ publicKeyUri = ORCID_BASE_URL + '/oauth/jwks'
constructor(private http: HttpClient) {
this.headers = new HttpHeaders({
@@ -24,17 +25,17 @@ export class LandingPageService {
return this.http.post(this.idTokenUri, JSON.stringify(data), { headers: this.headers })
}
- getOrcidConnectionRecord(state: String): Observable {
+ getOrcidConnectionRecord(state: string): Observable {
const requestUrl = this.recordConnectionUri + state
- return this.http.get(requestUrl, { observe: 'response' })
+ return this.http.get(requestUrl).pipe(map((response: any) => response.body))
}
- getMemberInfo(state: String): Observable {
+ getMemberInfo(state: string): Observable {
const requestUrl = this.memberInfoUri + state
return this.http.get(requestUrl)
}
- getUserInfo(access_token: String): Observable {
+ getUserInfo(access_token: string): Observable {
const headers = new HttpHeaders({
Authorization: 'Bearer ' + access_token,
'Content-Type': 'application/json',
diff --git a/ui/src/app/shared/model/orcid-record.model.ts b/ui/src/app/shared/model/orcid-record.model.ts
new file mode 100644
index 000000000..a21fe3feb
--- /dev/null
+++ b/ui/src/app/shared/model/orcid-record.model.ts
@@ -0,0 +1,15 @@
+import { Moment } from 'moment'
+import { EventType } from 'src/app/app.constants'
+
+export class OrcidRecord {
+ constructor(
+ public email: string,
+ public orcid: string,
+ public tokens?: any,
+ public last_notified?: Moment,
+ public revoke_notification_sent_date?: Moment,
+ public eminder_notification_sent_date?: Moment,
+ public created?: Moment,
+ public modified?: Moment
+ ) {}
+}
diff --git a/ui/src/app/shared/service/window-location.service.ts b/ui/src/app/shared/service/window-location.service.ts
index b7c93ca2f..35d980d8b 100644
--- a/ui/src/app/shared/service/window-location.service.ts
+++ b/ui/src/app/shared/service/window-location.service.ts
@@ -17,4 +17,8 @@ export class WindowLocationService {
getWindowLocationHref(): string {
return window.location.href
}
+
+ getWindowLocationHash(): string {
+ return window.location.hash
+ }
}
diff --git a/ui/src/app/user/users.component.spec.ts b/ui/src/app/user/users.component.spec.ts
index e33bfa34b..3b0a1c70d 100644
--- a/ui/src/app/user/users.component.spec.ts
+++ b/ui/src/app/user/users.component.spec.ts
@@ -44,6 +44,7 @@ describe('UsersComponent', () => {
declarations: [UsersComponent, HasAnyAuthorityDirective, LocalizePipe],
imports: [
ReactiveFormsModule,
+ RouterTestingModule,
RouterModule.forChild([{ path: 'users', component: UsersComponent }]),
FormsModule,
],