From e786699dd36ea61c2a46baed8ff98567f0893918 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 18 Mar 2026 23:21:10 +0100 Subject: [PATCH] fix: use truncateId consistently for all user identifiers in logs Replace 16 bare idTag/identifier.value references and 4 manual substring(0,8) truncations with truncateId() across OCPP 1.6 ResponseService, IncomingRequestService, auth adapters, strategies, and helpers. User identifiers (RFID tags, auth tokens) are now consistently truncated in all log output to prevent sensitive data exposure. --- .../ocpp/1.6/OCPP16IncomingRequestService.ts | 3 ++- .../ocpp/1.6/OCPP16ResponseService.ts | 16 ++++++++-------- .../ocpp/auth/adapters/OCPP16AuthAdapter.ts | 4 ++-- .../ocpp/auth/adapters/OCPP20AuthAdapter.ts | 4 ++-- .../ocpp/auth/services/OCPPAuthServiceImpl.ts | 6 +++--- .../ocpp/auth/strategies/LocalAuthStrategy.ts | 6 ++++-- .../ocpp/auth/strategies/RemoteAuthStrategy.ts | 10 +++++----- .../ocpp/auth/utils/AuthHelpers.ts | 5 ++--- .../ocpp/auth/utils/AuthHelpers.test.ts | 3 ++- 9 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts index 18cf59b7..9471353a 100644 --- a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts @@ -113,6 +113,7 @@ import { isNotEmptyString, logger, sleep, + truncateId, } from '../../../utils/index.js' import { OCPPConstants } from '../OCPPConstants.js' import { OCPPIncomingRequestService } from '../OCPPIncomingRequestService.js' @@ -1241,7 +1242,7 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { `${chargingStation.logPrefix()} ${moduleName}.handleRequestRemoteStartTransaction: Remote start transaction REJECTED on ${ // eslint-disable-next-line @typescript-eslint/restrict-template-expressions chargingStation.stationInfo?.chargingStationId - }, idTag '${commandPayload.idTag}': no available connector found` + }, idTag '${truncateId(commandPayload.idTag)}': no available connector found` ) return OCPP16Constants.OCPP_RESPONSE_REJECTED } diff --git a/src/charging-station/ocpp/1.6/OCPP16ResponseService.ts b/src/charging-station/ocpp/1.6/OCPP16ResponseService.ts index 933d1916..2ba96a03 100644 --- a/src/charging-station/ocpp/1.6/OCPP16ResponseService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16ResponseService.ts @@ -34,7 +34,7 @@ import { ReservationTerminationReason, type ResponseHandler, } from '../../../types/index.js' -import { Constants, convertToInt, logger } from '../../../utils/index.js' +import { Constants, convertToInt, logger, truncateId } from '../../../utils/index.js' import { OCPPResponseService } from '../OCPPResponseService.js' import { OCPP16ServiceUtils } from './OCPP16ServiceUtils.js' @@ -182,9 +182,9 @@ export class OCPP16ResponseService extends OCPPResponseService { authorizeConnectorStatus.idTagAuthorized = false delete authorizeConnectorStatus.authorizeIdTag logger.debug( - `${chargingStation.logPrefix()} ${moduleName}.handleResponseAuthorize: idTag '${ + `${chargingStation.logPrefix()} ${moduleName}.handleResponseAuthorize: idTag '${truncateId( requestPayload.idTag - }' rejected with status '${payload.idTagInfo.status}'` + )}' rejected with status '${payload.idTagInfo.status}'` ) } } else { @@ -394,9 +394,9 @@ export class OCPP16ResponseService extends OCPPResponseService { if (reservation != null) { if (reservation.idTag !== requestPayload.idTag) { logger.warn( - `${chargingStation.logPrefix()} ${moduleName}.handleResponseStartTransaction: Reserved transaction ${payload.transactionId.toString()} started with a different idTag ${ + `${chargingStation.logPrefix()} ${moduleName}.handleResponseStartTransaction: Reserved transaction ${payload.transactionId.toString()} started with a different idTag ${truncateId( requestPayload.idTag - } than the reservation one ${reservation.idTag}` + )} than the reservation one ${truncateId(reservation.idTag)}` ) } if (hasReservationExpired(reservation)) { @@ -432,7 +432,7 @@ export class OCPP16ResponseService extends OCPPResponseService { `${chargingStation.logPrefix()} ${moduleName}.handleResponseStartTransaction: Transaction with id ${payload.transactionId.toString()} STARTED on ${ // eslint-disable-next-line @typescript-eslint/restrict-template-expressions chargingStation.stationInfo?.chargingStationId - }#${connectorId.toString()} for idTag '${requestPayload.idTag}'` + }#${connectorId.toString()} for idTag '${truncateId(requestPayload.idTag)}'` ) if (chargingStation.stationInfo?.powerSharedByConnectors === true) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -453,9 +453,9 @@ export class OCPP16ResponseService extends OCPPResponseService { `${chargingStation.logPrefix()} ${moduleName}.handleResponseStartTransaction: Starting transaction with id ${payload.transactionId.toString()} REJECTED on ${ // eslint-disable-next-line @typescript-eslint/restrict-template-expressions chargingStation.stationInfo?.chargingStationId - }#${connectorId.toString()} with status '${payload.idTagInfo.status}', idTag '${ + }#${connectorId.toString()} with status '${payload.idTagInfo.status}', idTag '${truncateId( requestPayload.idTag - }'${ + )}'${ OCPP16ServiceUtils.hasReservation(chargingStation, connectorId, requestPayload.idTag) ? // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `, reservationId '${requestPayload.reservationId?.toString()}'` diff --git a/src/charging-station/ocpp/auth/adapters/OCPP16AuthAdapter.ts b/src/charging-station/ocpp/auth/adapters/OCPP16AuthAdapter.ts index 0b34715c..83cd7fe2 100644 --- a/src/charging-station/ocpp/auth/adapters/OCPP16AuthAdapter.ts +++ b/src/charging-station/ocpp/auth/adapters/OCPP16AuthAdapter.ts @@ -15,7 +15,7 @@ import { StandardParametersKey, } from '../../../../types/index.js' import { OCPPVersion } from '../../../../types/ocpp/OCPPVersion.js' -import { logger } from '../../../../utils/index.js' +import { logger, truncateId } from '../../../../utils/index.js' import { AuthContext, AuthenticationMethod, @@ -55,7 +55,7 @@ export class OCPP16AuthAdapter implements OCPPAuthAdapter { try { logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: Authorizing identifier ${identifier.value} via OCPP 1.6` + `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: Authorizing identifier ${truncateId(identifier.value)} via OCPP 1.6` ) // Mark connector as authorizing if provided diff --git a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts index d481738e..ec5dbd58 100644 --- a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts +++ b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts @@ -20,7 +20,7 @@ import { OCPP20TriggerReasonEnumType, } from '../../../../types/ocpp/2.0/Transaction.js' import { OCPPVersion } from '../../../../types/ocpp/OCPPVersion.js' -import { logger } from '../../../../utils/index.js' +import { logger, truncateId } from '../../../../utils/index.js' import { AuthContext, AuthenticationMethod, @@ -64,7 +64,7 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { try { logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: Authorizing identifier ${identifier.value} via OCPP 2.0 TransactionEvent` + `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: Authorizing identifier ${truncateId(identifier.value)} via OCPP 2.0 TransactionEvent` ) // Check if remote authorization is configured diff --git a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts index 36a06059..a570d32b 100644 --- a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts +++ b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts @@ -92,7 +92,7 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { this.metrics.totalRequests++ logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.authenticate: Starting authentication for identifier: ${JSON.stringify(request.identifier)}` + `${this.chargingStation.logPrefix()} ${moduleName}.authenticate: Starting authentication for identifier: ${truncateId(request.identifier.value)}` ) // Try each strategy in priority order @@ -431,7 +431,7 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { */ public invalidateCache (identifier: UnifiedIdentifier): void { logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.invalidateCache: Invalidating cache for identifier: ${identifier.value}` + `${this.chargingStation.logPrefix()} ${moduleName}.invalidateCache: Invalidating cache for identifier: ${truncateId(identifier.value)}` ) // Invalidate in local strategy @@ -439,7 +439,7 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { if (localStrategy) { localStrategy.invalidateCache(identifier.value) logger.info( - `${this.chargingStation.logPrefix()} ${moduleName}.invalidateCache: Cache invalidated for identifier: ${identifier.value}` + `${this.chargingStation.logPrefix()} ${moduleName}.invalidateCache: Cache invalidated for identifier: ${truncateId(identifier.value)}` ) } else { logger.debug( diff --git a/src/charging-station/ocpp/auth/strategies/LocalAuthStrategy.ts b/src/charging-station/ocpp/auth/strategies/LocalAuthStrategy.ts index 181b95aa..a78b1475 100644 --- a/src/charging-station/ocpp/auth/strategies/LocalAuthStrategy.ts +++ b/src/charging-station/ocpp/auth/strategies/LocalAuthStrategy.ts @@ -70,7 +70,7 @@ export class LocalAuthStrategy implements AuthStrategy { try { logger.debug( - `${moduleName}: Authenticating ${request.identifier.value} for ${request.context}` + `${moduleName}: Authenticating ${truncateId(request.identifier.value)} for ${request.context}` ) // 1. Try local authorization list first (highest priority) @@ -117,7 +117,9 @@ export class LocalAuthStrategy implements AuthStrategy { } } - logger.debug(`${moduleName}: No local authorization found for ${request.identifier.value}`) + logger.debug( + `${moduleName}: No local authorization found for ${truncateId(request.identifier.value)}` + ) return undefined } catch (error) { const errorMessage = getErrorMessage(error) diff --git a/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts b/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts index 3e406e95..6a1fb91b 100644 --- a/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts +++ b/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts @@ -7,7 +7,7 @@ import type { } from '../interfaces/OCPPAuthService.js' import type { AuthConfiguration, AuthorizationResult, AuthRequest } from '../types/AuthTypes.js' -import { ensureError, getErrorMessage, logger } from '../../../../utils/index.js' +import { ensureError, getErrorMessage, logger, truncateId } from '../../../../utils/index.js' import { AuthenticationError, AuthenticationMethod, @@ -93,7 +93,7 @@ export class RemoteAuthStrategy implements AuthStrategy { try { logger.debug( - `${moduleName}: Authenticating ${request.identifier.value.substring(0, 8)}... via CSMS for ${request.context}` + `${moduleName}: Authenticating ${truncateId(request.identifier.value)} via CSMS for ${request.context}` ) // Get appropriate adapter for OCPP version @@ -126,7 +126,7 @@ export class RemoteAuthStrategy implements AuthStrategy { const isInLocalList = await this.localAuthListManager.getEntry(request.identifier.value) if (isInLocalList) { logger.debug( - `${moduleName}: Skipping cache for local list identifier: ${request.identifier.value.substring(0, 8)}...` + `${moduleName}: Skipping cache for local list identifier: ${truncateId(request.identifier.value)}` ) } else { this.cacheResult( @@ -149,7 +149,7 @@ export class RemoteAuthStrategy implements AuthStrategy { } logger.debug( - `${moduleName}: No remote authorization result for ${request.identifier.value.substring(0, 8)}...` + `${moduleName}: No remote authorization result for ${truncateId(request.identifier.value)}` ) return undefined } catch (error) { @@ -385,7 +385,7 @@ export class RemoteAuthStrategy implements AuthStrategy { const cacheTtl = ttl ?? result.cacheTtl ?? 300 // Default 5 minutes this.authCache.set(identifier, result, cacheTtl) logger.debug( - `${moduleName}: Cached result for ${identifier.substring(0, 8)}... (TTL: ${String(cacheTtl)}s)` + `${moduleName}: Cached result for ${truncateId(identifier)} (TTL: ${String(cacheTtl)}s)` ) } catch (error) { const errorMessage = getErrorMessage(error) diff --git a/src/charging-station/ocpp/auth/utils/AuthHelpers.ts b/src/charging-station/ocpp/auth/utils/AuthHelpers.ts index 593295b2..3358c720 100644 --- a/src/charging-station/ocpp/auth/utils/AuthHelpers.ts +++ b/src/charging-station/ocpp/auth/utils/AuthHelpers.ts @@ -6,10 +6,10 @@ import type { UnifiedIdentifier, } from '../types/AuthTypes.js' +import { truncateId } from '../../../../utils/index.js' import { AuthorizationStatus } from '../types/AuthTypes.js' /** - * Compute remaining TTL in seconds from an expiry date. * @param expiryDate - Expiry timestamp to compute TTL from * @returns TTL in seconds, or undefined if already expired or no date provided */ @@ -80,8 +80,7 @@ function createRejectedResult ( * @returns Formatted error string with truncated identifier */ function formatAuthError (error: Error, identifier: UnifiedIdentifier): string { - const identifierValue = identifier.value.substring(0, 8) + '...' - return `Authentication failed for identifier ${identifierValue} (${identifier.type}): ${error.message}` + return `Authentication failed for identifier ${truncateId(identifier.value)} (${identifier.type}): ${error.message}` } /** diff --git a/tests/charging-station/ocpp/auth/utils/AuthHelpers.test.ts b/tests/charging-station/ocpp/auth/utils/AuthHelpers.test.ts index b8fc286c..653561cd 100644 --- a/tests/charging-station/ocpp/auth/utils/AuthHelpers.test.ts +++ b/tests/charging-station/ocpp/auth/utils/AuthHelpers.test.ts @@ -151,7 +151,8 @@ await describe('AuthHelpers', async () => { const message = AuthHelpers.formatAuthError(error, identifier) - assert.ok(message.includes('SHORT...')) + assert.ok(message.includes('SHORT')) + assert.ok(!message.includes('SHORT...')) assert.ok(message.includes('Local')) assert.ok(message.includes('Invalid format')) }) -- 2.53.0