From 35c80937063d0a644a19a4f9420fab6831139e6b Mon Sep 17 00:00:00 2001 From: swapnil-verma-gl <92505353+swapnil-verma-gl@users.noreply.github.com> Date: Fri, 8 Nov 2024 14:06:40 +0530 Subject: [PATCH] [MNT-24614] Fixed APS basic auth login issue with ADF (#10364) * [MNT-24614] Fixed APS basic auth login issue with ADF * [MNT-24614] Addressed code review findings - Using includes api, and removed unneeded functions. Added missing return type to functions * [MNT-24614] Added unit tests * [MNT-24614] Added unit tests * [MNT-24614] Fixed casing of unit test titles --- .../src/lib/adf-http-client.service.spec.ts | 32 ++++ .../api/src/lib/adf-http-client.service.ts | 152 ++++++++---------- .../basic-alfresco-auth.service.spec.ts | 62 +++++++ .../basic-auth/basic-alfresco-auth.service.ts | 23 ++- 4 files changed, 180 insertions(+), 89 deletions(-) create mode 100644 lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.spec.ts diff --git a/lib/core/api/src/lib/adf-http-client.service.spec.ts b/lib/core/api/src/lib/adf-http-client.service.spec.ts index 7bf60f7e67..9c664b6e18 100644 --- a/lib/core/api/src/lib/adf-http-client.service.spec.ts +++ b/lib/core/api/src/lib/adf-http-client.service.spec.ts @@ -387,4 +387,36 @@ describe('AdfHttpClient', () => { req.flush(null, { status: 200, statusText: 'Ok' }); }); + + it('should set X-CSRF-TOKEN header if CSRF is enabled', () => { + const options: RequestOptions = { + path: '', + httpMethod: 'GET' + }; + angularHttpClient.disableCsrf = false; + + angularHttpClient.request('http://example.com', options, securityOptions, emitters).catch((error) => fail(error)); + + const req = controller.expectOne('http://example.com'); + + expect(req.request.headers.get('X-CSRF-TOKEN')).toBeDefined(); + + req.flush(null, { status: 200, statusText: 'Ok' }); + }); + + it('should not set X-CSRF-TOKEN header if CSRF is disabled', () => { + const options: RequestOptions = { + path: '', + httpMethod: 'GET' + }; + angularHttpClient.disableCsrf = true; + + angularHttpClient.request('http://example.com', options, securityOptions, emitters).catch((error) => fail(error)); + + const req = controller.expectOne('http://example.com'); + + expect(req.request.headers.get('X-CSRF-TOKEN')).toBeNull(); + + req.flush(null, { status: 200, statusText: 'Ok' }); + }); }); diff --git a/lib/core/api/src/lib/adf-http-client.service.ts b/lib/core/api/src/lib/adf-http-client.service.ts index 30865ab227..b015643e7b 100644 --- a/lib/core/api/src/lib/adf-http-client.service.ts +++ b/lib/core/api/src/lib/adf-http-client.service.ts @@ -17,15 +17,7 @@ import { SHOULD_ADD_AUTH_TOKEN } from '@alfresco/adf-core/auth'; import { Emitters as JsApiEmitters, HttpClient as JsApiHttpClient } from '@alfresco/js-api'; -import { - HttpClient, - HttpContext, - HttpErrorResponse, - HttpEvent, - HttpHeaders, - HttpParams, - HttpResponse -} from '@angular/common/http'; +import { HttpClient, HttpContext, HttpErrorResponse, HttpEvent, HttpHeaders, HttpParams, HttpResponse } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Observable, of, Subject, throwError } from 'rxjs'; import { catchError, map, takeUntil } from 'rxjs/operators'; @@ -52,8 +44,7 @@ export interface Emitters { @Injectable({ providedIn: 'root' }) -export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { - +export class AdfHttpClient implements ee.Emitter, JsApiHttpClient { on: ee.EmitterMethod; off: ee.EmitterMethod; once: ee.EmitterMethod; @@ -107,47 +98,43 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { const params = getQueryParamsWithCustomEncoder(options.queryParams, new AlfrescoApiParamEncoder()); const responseType = AdfHttpClient.getResponseType(options); const context = new HttpContext().set(SHOULD_ADD_AUTH_TOKEN, true); - const security: SecurityOptions = {...this.defaultSecurityOptions, ...sc}; + const security: SecurityOptions = { ...this.defaultSecurityOptions, ...sc }; const headers = this.getHeaders(options); if (!emitters) { emitters = this.getEventEmitters(); } - const request = this.httpClient.request( - options.httpMethod, - url, - { - context, - ...(body && {body}), - ...(responseType && {responseType}), - ...security, - ...(params && {params}), - headers, - observe: 'events', - reportProgress: true - } - ); + const request = this.httpClient.request(options.httpMethod, url, { + context, + ...(body && { body }), + ...(responseType && { responseType }), + ...security, + ...(params && { params }), + headers, + observe: 'events', + reportProgress: true + }); return this.requestWithLegacyEventEmitters(request, emitters, options.returnType); } post(url: string, options?: RequestOptions, sc?: SecurityOptions, emitters?: JsApiEmitters): Promise { - return this.request(url, {...options, httpMethod: 'POST'}, sc, emitters); + return this.request(url, { ...options, httpMethod: 'POST' }, sc, emitters); } put(url: string, options?: RequestOptions, sc?: SecurityOptions, emitters?: JsApiEmitters): Promise { - return this.request(url, {...options, httpMethod: 'PUT'}, sc, emitters); + return this.request(url, { ...options, httpMethod: 'PUT' }, sc, emitters); } get(url: string, options?: RequestOptions, sc?: SecurityOptions, emitters?: JsApiEmitters): Promise { - return this.request(url, {...options, httpMethod: 'GET'}, sc, emitters); + return this.request(url, { ...options, httpMethod: 'GET' }, sc, emitters); } delete(url: string, options?: RequestOptions, sc?: SecurityOptions, emitters?: JsApiEmitters): Promise { - return this.request(url, {...options, httpMethod: 'DELETE'}, sc, emitters); + return this.request(url, { ...options, httpMethod: 'DELETE' }, sc, emitters); } - private addPromiseListeners(promise: Promise, eventEmitter: any) { + private addPromiseListeners(promise: Promise, eventEmitter: any) { const eventPromise = Object.assign(promise, { on() { // eslint-disable-next-line prefer-spread, prefer-rest-params @@ -189,58 +176,59 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { } private requestWithLegacyEventEmitters(request$: Observable>, emitters: JsApiEmitters, returnType: any): Promise { - const abort$ = new Subject(); - const {eventEmitter, apiClientEmitter} = emitters; + const { eventEmitter, apiClientEmitter } = emitters; - const promise = request$.pipe( - map((res) => { - if (isHttpUploadProgressEvent(res)) { - const percent = Math.round((res.loaded / res.total) * 100); - eventEmitter.emit('progress', {loaded: res.loaded, total: res.total, percent}); - } + const promise = request$ + .pipe( + map((res) => { + if (isHttpUploadProgressEvent(res)) { + const percent = Math.round((res.loaded / res.total) * 100); + eventEmitter.emit('progress', { loaded: res.loaded, total: res.total, percent }); + } - if (isHttpResponseEvent(res)) { - eventEmitter.emit('success', res.body); - return AdfHttpClient.deserialize(res, returnType); - } - }), - catchError((err: HttpErrorResponse): Observable => { + if (isHttpResponseEvent(res)) { + eventEmitter.emit('success', res.body); + return AdfHttpClient.deserialize(res, returnType); + } + }), + catchError((err: HttpErrorResponse): Observable => { + // since we can't always determinate ahead of time if the response is going to be xml or plain text response + // we need to handle false positive cases here. - // since we can't always determinate ahead of time if the response is going to be xml or plain text response - // we need to handle false positive cases here. + if (err.status === 200) { + eventEmitter.emit('success', err.error.text); + return of(err.error.text); + } - if (err.status === 200) { - eventEmitter.emit('success', err.error.text); - return of(err.error.text); - } + eventEmitter.emit('error', err); + apiClientEmitter.emit('error', { ...err, response: { req: err } }); - eventEmitter.emit('error', err); - apiClientEmitter.emit('error', { ...err, response: { req: err } }); + if (err.status === 401) { + eventEmitter.emit('unauthorized'); + apiClientEmitter.emit('unauthorized'); + } - if (err.status === 401) { - eventEmitter.emit('unauthorized'); - apiClientEmitter.emit('unauthorized'); - } + // for backwards compatibility we need to convert it to error class as the HttpErrorResponse only implements Error interface, not extending it, + // and we need to be able to correctly pass instanceof Error conditions used inside repository + // we also need to pass error as Stringify string as we are detecting statusCodes using JSON.parse(error.message) in some places + const msg = typeof err.error === 'string' ? err.error : JSON.stringify(err.error); - // for backwards compatibility we need to convert it to error class as the HttpErrorResponse only implements Error interface, not extending it, - // and we need to be able to correctly pass instanceof Error conditions used inside repository - // we also need to pass error as Stringify string as we are detecting statusCodes using JSON.parse(error.message) in some places - const msg = typeof err.error === 'string' ? err.error : JSON.stringify(err.error); + // for backwards compatibility to handle cases in code where we try read response.error.response.body; - // for backwards compatibility to handle cases in code where we try read response.error.response.body; + const error = { + ...err, + body: err.error + }; - const error = { - ...err, body: err.error - }; + const alfrescoApiError = new AlfrescoApiResponseError(msg, err.status, error); + return throwError(alfrescoApiError); + }), + takeUntil(abort$) + ) + .toPromise(); - const alfrescoApiError = new AlfrescoApiResponseError(msg, err.status, error); - return throwError(alfrescoApiError); - }), - takeUntil(abort$) - ).toPromise(); - - (promise as any).abort = function() { + (promise as any).abort = function () { eventEmitter.emit('abort'); abort$.next(); abort$.complete(); @@ -261,7 +249,7 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { } if (isFormUrlEncoded) { - return new HttpParams({fromObject: removeNilValues(options.formParams)}); + return new HttpParams({ fromObject: removeNilValues(options.formParams) }); } return body; @@ -273,8 +261,8 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { const optionsHeaders = { ...options.headerParams, - ...(accept && {Accept: accept}), - ...((contentType) && {'Content-Type': contentType}) + ...(accept && { Accept: accept }), + ...(contentType && { 'Content-Type': contentType }) }; if (!this.disableCsrf) { @@ -319,7 +307,6 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { return Boolean(contentType?.match(/^application\/json(;.*)?$/i)); } - private setCsrfToken(optionsHeaders: any) { const token = this.createCSRFToken(); optionsHeaders['X-CSRF-TOKEN'] = token; @@ -332,12 +319,16 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { } private createCSRFToken(a?: any): string { - const randomValue = window.crypto.getRandomValues(new Uint32Array(1))[0]; + const randomValue = AdfHttpClient.getSecureRandomValue(); return a ? (a ^ ((randomValue * 16) >> (a / 4))).toString(16) : ([1e16] + (1e16).toString()).replace(/[01]/g, this.createCSRFToken); } - private static getResponseType(options: RequestOptions): 'blob' | 'json' | 'text' { + private static getSecureRandomValue(): number { + const max = Math.pow(2, 32); + return window.crypto.getRandomValues(new Uint32Array(1))[0] / max; + } + private static getResponseType(options: RequestOptions): 'blob' | 'json' | 'text' { const isBlobType = options.returnType?.toString().toLowerCase() === 'blob' || options.responseType?.toString().toLowerCase() === 'blob'; if (isBlobType) { @@ -359,7 +350,6 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { * @returns deserialized object */ private static deserialize(response: HttpResponse, returnType?: Constructor | 'blob'): any { - if (response === null) { return null; } @@ -390,9 +380,7 @@ export class AdfHttpClient implements ee.Emitter,JsApiHttpClient { return new returnType(body); } - private static deserializeBlobResponse(response: HttpResponse) { - return new Blob([response.body], {type: response.headers.get('Content-Type')}); + return new Blob([response.body], { type: response.headers.get('Content-Type') }); } } - diff --git a/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.spec.ts b/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.spec.ts new file mode 100644 index 0000000000..59273cf4e6 --- /dev/null +++ b/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.spec.ts @@ -0,0 +1,62 @@ +/*! + * @license + * Copyright © 2005-2024 Hyland Software, Inc. and its affiliates. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TestBed } from '@angular/core/testing'; +import { BasicAlfrescoAuthService } from './basic-alfresco-auth.service'; +import { AppConfigService, AppConfigValues } from '../../app-config/app-config.service'; +import { ProcessAuth } from './process-auth'; +import { ContentAuth } from './content-auth'; +import { HttpClientTestingModule } from '@angular/common/http/testing'; + +describe('BasicAlfrescoAuthService', () => { + let basicAlfrescoAuthService: BasicAlfrescoAuthService; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [BasicAlfrescoAuthService] + }); + basicAlfrescoAuthService = TestBed.inject(BasicAlfrescoAuthService); + spyOn(TestBed.inject(ProcessAuth), 'getToken').and.returnValue('Mock Process Auth ticket'); + spyOn(TestBed.inject(ContentAuth), 'getToken').and.returnValue('Mock Content Auth ticket'); + const appConfigSpy = spyOn(TestBed.inject(AppConfigService), 'get'); + appConfigSpy.withArgs(AppConfigValues.CONTEXTROOTBPM).and.returnValue('activiti-app'); + appConfigSpy.withArgs(AppConfigValues.CONTEXTROOTECM).and.returnValue('alfresco'); + }); + + it('should return content services ticket when requestUrl contains ECM context root', () => { + const ticket = basicAlfrescoAuthService.getTicketEcmBase64('http://www.exmple.com/alfresco/mock-api-url'); + const base64Segment = ticket.split('Basic ')[1]; + expect(atob(base64Segment)).toEqual('Mock Content Auth ticket'); + }); + + it('should return process services ticket when requestUrl contains ECM context root', () => { + const ticket = basicAlfrescoAuthService.getTicketEcmBase64('http://www.example.com/activiti-app/mock-api-url'); + expect(ticket).toEqual('Basic Mock Process Auth ticket'); + }); + + it('should return content services ticket when requestUrl contains both ECM and BPM context root, but ECM context root comes before', () => { + const ticket = basicAlfrescoAuthService.getTicketEcmBase64('http://www.exmple.com/alfresco/activiti-app/mock-api-url'); + const base64Segment = ticket.split('Basic ')[1]; + expect(atob(base64Segment)).toEqual('Mock Content Auth ticket'); + }); + + it('should return process services ticket when requestUrl contains both ECM and BPM context root, but BPM context root comes before', () => { + const ticket = basicAlfrescoAuthService.getTicketEcmBase64('http://www.example.com/activiti-app/alfresco/mock-api-url'); + expect(ticket).toEqual('Basic Mock Process Auth ticket'); + }); +}); diff --git a/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.ts b/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.ts index 30fb401ad1..adbfd3417f 100644 --- a/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.ts +++ b/lib/core/src/lib/auth/basic-auth/basic-alfresco-auth.service.ts @@ -373,15 +373,24 @@ export class BasicAlfrescoAuthService extends BaseAuthenticationService { getTicketEcmBase64(requestUrl: string): string | null { let ticket = null; - const contextRootBpm = this.appConfig.get(AppConfigValues.CONTEXTROOTBPM) || 'activiti-app'; - const contextRoot = this.appConfig.get(AppConfigValues.CONTEXTROOTECM) || 'alfresco'; + const bpmRoot = `/${this.appConfig.get(AppConfigValues.CONTEXTROOTBPM) || 'activiti-app'}/`; + const ecmRoot = `/${this.appConfig.get(AppConfigValues.CONTEXTROOTECM) || 'alfresco'}/`; - if (contextRoot && requestUrl.indexOf(contextRoot) !== -1) { - ticket = 'Basic ' + btoa(this.contentAuth.getToken()); - } else if (contextRootBpm && requestUrl.indexOf(contextRootBpm) !== -1) { - ticket = 'Basic ' + this.processAuth.getToken(); + if (requestUrl?.includes(ecmRoot) && !requestUrl.includes(bpmRoot)) { + ticket = this.getContentServicesTicket(); + } else if (requestUrl?.includes(bpmRoot) && !requestUrl.includes(ecmRoot)) { + ticket = this.getProcessServicesTicket(); + } else if (requestUrl?.includes(ecmRoot) && requestUrl.includes(bpmRoot)) { + ticket = requestUrl.indexOf(ecmRoot) < requestUrl.indexOf(bpmRoot) ? this.getContentServicesTicket() : this.getProcessServicesTicket(); } - return ticket; } + + private getProcessServicesTicket(): string { + return this.processAuth.getToken()?.startsWith('Basic ') ? this.processAuth.getToken() : 'Basic ' + this.processAuth.getToken(); + } + + private getContentServicesTicket(): string { + return 'Basic ' + btoa(this.contentAuth.getToken()); + } }