From 32d5dd7354c2a9bc96c9d379f2bc34235afe818f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Thu, 14 Aug 2025 17:21:32 +0200 Subject: [PATCH] refactor: improve UI services basic authentication handling MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/charging-station/Helpers.ts | 6 +- .../ui-server/AbstractUIServer.ts | 12 +++- .../ui-server/UIServerUtils.ts | 66 ++++++++++++------- src/utils/Utils.ts | 5 ++ tests/utils/Utils.test.ts | 4 +- 5 files changed, 62 insertions(+), 31 deletions(-) diff --git a/src/charging-station/Helpers.ts b/src/charging-station/Helpers.ts index aa34494f..527a63dd 100644 --- a/src/charging-station/Helpers.ts +++ b/src/charging-station/Helpers.ts @@ -189,7 +189,7 @@ export const validateStationInfo = (chargingStation: ChargingStation): void => { } if ( chargingStation.stationInfo.chargingStationId == null || - isEmpty(chargingStation.stationInfo.chargingStationId.trim()) + isEmpty(chargingStation.stationInfo.chargingStationId) ) { throw new BaseError('Missing chargingStationId in stationInfo properties') } @@ -197,7 +197,7 @@ export const validateStationInfo = (chargingStation: ChargingStation): void => { if ( // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition chargingStation.stationInfo.hashId == null || - isEmpty(chargingStation.stationInfo.hashId.trim()) + isEmpty(chargingStation.stationInfo.hashId) ) { throw new BaseError(`${chargingStationId}: Missing hashId in stationInfo properties`) } @@ -213,7 +213,7 @@ export const validateStationInfo = (chargingStation: ChargingStation): void => { if ( // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition chargingStation.stationInfo.templateName == null || - isEmpty(chargingStation.stationInfo.templateName.trim()) + isEmpty(chargingStation.stationInfo.templateName) ) { throw new BaseError(`${chargingStationId}: Missing templateName in stationInfo properties`) } diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts index 9068d474..2795570d 100644 --- a/src/charging-station/ui-server/AbstractUIServer.ts +++ b/src/charging-station/ui-server/AbstractUIServer.ts @@ -159,16 +159,20 @@ export abstract class AbstractUIServer { } private isValidBasicAuth (req: IncomingMessage, next: (err?: Error) => void): boolean { - const [username, password] = getUsernameAndPasswordFromAuthorizationToken( + const usernameAndPassword = getUsernameAndPasswordFromAuthorizationToken( req.headers.authorization?.split(/\s+/).pop() ?? '', next ) + if (usernameAndPassword == null) { + return false + } + const [username, password] = usernameAndPassword return this.isValidUsernameAndPassword(username, password) } private isValidProtocolBasicAuth (req: IncomingMessage, next: (err?: Error) => void): boolean { const authorizationProtocol = req.headers['sec-websocket-protocol']?.split(/,\s+/).pop() - const [username, password] = getUsernameAndPasswordFromAuthorizationToken( + const usernameAndPassword = getUsernameAndPasswordFromAuthorizationToken( // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/restrict-template-expressions `${authorizationProtocol}${Array(((4 - (authorizationProtocol!.length % 4)) % 4) + 1).join( '=' @@ -177,6 +181,10 @@ export abstract class AbstractUIServer { .pop() ?? '', next ) + if (usernameAndPassword == null) { + return false + } + const [username, password] = usernameAndPassword return this.isValidUsernameAndPassword(username, password) } diff --git a/src/charging-station/ui-server/UIServerUtils.ts b/src/charging-station/ui-server/UIServerUtils.ts index 419c9f63..50ed36f3 100644 --- a/src/charging-station/ui-server/UIServerUtils.ts +++ b/src/charging-station/ui-server/UIServerUtils.ts @@ -7,56 +7,74 @@ import { isEmpty, logger, logPrefix } from '../../utils/index.js' export const getUsernameAndPasswordFromAuthorizationToken = ( authorizationToken: string, next: (err?: Error) => void -): [string, string] => { - if ( - !/^([0-9a-zA-Z+/]{4})*(([0-9a-zA-Z+/]{2}==)|([0-9a-zA-Z+/]{3}=))?$/.test(authorizationToken) - ) { - next(new BaseError('Invalid basic authentication token format')) +): [string, string] | undefined => { + try { + const authentication = Buffer.from(authorizationToken, 'base64').toString('utf8') + const separatorIndex = authentication.indexOf(':') + if (separatorIndex === -1) { + next(new BaseError('Invalid basic authentication token format: missing ":" separator')) + return undefined + } + const username = authentication.slice(0, separatorIndex) + const password = authentication.slice(separatorIndex + 1) + if (isEmpty(username)) { + next(new BaseError('Invalid basic authentication token format: empty username')) + return undefined + } + return [username, password] + } catch (error) { + next(new BaseError(`Invalid basic authentication token format: ${(error as Error).message}`)) + return undefined } - const authentication = Buffer.from(authorizationToken, 'base64').toString() - const authenticationParts = authentication.split(/:/) - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return [authenticationParts.shift()!, authenticationParts.join(':')] } export const handleProtocols = ( protocols: Set, _request: IncomingMessage ): false | string => { - let protocol: Protocol | undefined - let version: ProtocolVersion | undefined if (isEmpty(protocols)) { return false } - for (const fullProtocol of protocols) { - if (isProtocolAndVersionSupported(fullProtocol)) { - return fullProtocol + for (const protocol of protocols) { + if (isProtocolAndVersionSupported(protocol)) { + return protocol } } logger.error( `${logPrefix( ' UI WebSocket Server |' - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - )} Unsupported protocol: '${protocol}' or protocol version: '${version}'` + )} Unsupported protocol in client request: '${Array.from(protocols).join(', ')}'` ) return false } export const isProtocolAndVersionSupported = (protocolStr: string): boolean => { - const [protocol, version] = getProtocolAndVersion(protocolStr) + const protocolAndVersion = getProtocolAndVersion(protocolStr) + if (protocolAndVersion == null) { + return false + } + const [protocol, version] = protocolAndVersion return ( Object.values(Protocol).includes(protocol) && Object.values(ProtocolVersion).includes(version) ) } -export const getProtocolAndVersion = (protocolStr: string): [Protocol, ProtocolVersion] => { +export const getProtocolAndVersion = ( + protocolStr: string +): [Protocol, ProtocolVersion] | undefined => { + if (isEmpty(protocolStr)) { + return undefined + } + if (!protocolStr.startsWith(Protocol.UI)) { + return undefined + } const protocolIndex = protocolStr.indexOf(Protocol.UI) - const protocol = protocolStr.substring( - protocolIndex, - protocolIndex + Protocol.UI.length - ) as Protocol - const version = protocolStr.substring(protocolIndex + Protocol.UI.length) as ProtocolVersion - return [protocol, version] + const protocol = protocolStr.substring(protocolIndex, protocolIndex + Protocol.UI.length) + const version = protocolStr.substring(protocolIndex + Protocol.UI.length) + if (isEmpty(protocol) || isEmpty(version)) { + return undefined + } + return [protocol, version] as [Protocol, ProtocolVersion] } export const isLoopback = (address: string): boolean => { diff --git a/src/utils/Utils.ts b/src/utils/Utils.ts index 9bd4d1d9..285927bd 100644 --- a/src/utils/Utils.ts +++ b/src/utils/Utils.ts @@ -61,6 +61,7 @@ export const has = (property: PropertyKey, object: null | object | undefined): b const type = (value: unknown): string => { if (value === null) return 'Null' if (value === undefined) return 'Undefined' + if (typeof value === 'string') return 'String' if (Number.isNaN(value)) return 'NaN' if (Array.isArray(value)) return 'Array' return Object.prototype.toString.call(value).slice(8, -1) @@ -73,6 +74,10 @@ export const isEmpty = (value: unknown): boolean => { } if (!value) return true + if (valueType === 'String') { + return (value as string).trim().length === 0 + } + if (valueType === 'Object') { return Object.keys(value as Record).length === 0 } diff --git a/tests/utils/Utils.test.ts b/tests/utils/Utils.test.ts index 1358295d..ec467912 100644 --- a/tests/utils/Utils.test.ts +++ b/tests/utils/Utils.test.ts @@ -352,8 +352,8 @@ await describe('Utils test suite', async () => { await it('Verify isEmpty()', () => { expect(isEmpty('')).toBe(true) - expect(isEmpty(' ')).toBe(false) - expect(isEmpty(' ')).toBe(false) + expect(isEmpty(' ')).toBe(true) + expect(isEmpty(' ')).toBe(true) expect(isEmpty('test')).toBe(false) expect(isEmpty(' test')).toBe(false) expect(isEmpty('test ')).toBe(false) -- 2.43.0